From 10cdb4360ebda8cf02bda44c2892d80b08cc8518 Mon Sep 17 00:00:00 2001 From: Anatoly Sablin Date: Tue, 10 Dec 2019 00:10:13 +0300 Subject: [PATCH] Fix homeserver verification with wildcards certificates. Disable v2 by default. Add migration to fix the accepted table (due to sqlite unable to change constraint, drop table and create again). Fix displaying the expiration period of the new token. Remove duplicated code. Use v1 single lookup when receive the request with `none` algorithm and the only one argument. Hide v2 endpoint if v2 API disabled. --- docs/MSC2140_MSC2134.md | 14 +++- ma1sd.example.yaml | 54 ++++++------- src/main/java/io/kamax/mxisd/HttpMxisd.java | 31 ++++--- .../io/kamax/mxisd/auth/AccountManager.java | 81 +------------------ .../io/kamax/mxisd/config/MatrixConfig.java | 2 +- .../auth/v2/AccountRegisterHandler.java | 2 +- .../identity/v2/HashLookupHandler.java | 36 ++++++++- .../mxisd/matrix/HomeserverVerifier.java | 3 +- .../storage/ormlite/OrmLiteSqlStorage.java | 24 +++++- .../storage/ormlite/dao/AcceptedDao.java | 2 +- .../storage/ormlite/dao/ChangelogDao.java | 52 ++++++++++++ 11 files changed, 176 insertions(+), 125 deletions(-) create mode 100644 src/main/java/io/kamax/mxisd/storage/ormlite/dao/ChangelogDao.java diff --git a/docs/MSC2140_MSC2134.md b/docs/MSC2140_MSC2134.md index 8d5c17e..4d6d672 100644 --- a/docs/MSC2140_MSC2134.md +++ b/docs/MSC2140_MSC2134.md @@ -7,7 +7,7 @@ Default values: ```.yaml matrix: v1: true # deprecated - v2: true + v2: false ``` To disable change value to `false`. @@ -19,8 +19,14 @@ matrix: ``` NOTE: Riot Web version 1.5.5 and below checks the v1 for backward compatibility. +NOTE: v2 disabled by default in order to preserve backward compatibility. + ## Terms +###### Requires: No. + +Administrator can omit terms configuration. In this case the terms checking will be disabled. + Example: ```.yaml policy: @@ -45,7 +51,7 @@ Where: - `version` -- the terms version. - `lang` -- the term language. - `name` -- the name of the term. -- `url` -- the url of the term. +- `url` -- the url of the term. Might be any url (i.e. from another host) for a html page. - `regexp` -- regexp patterns for API which should be available only after accepting the terms. API will be checks for accepted terms only with authorization. @@ -72,6 +78,10 @@ There is only one exception: [`POST /_matrix/identity/v2/terms`](https://matrix. Hashes and the pepper updates together according to the `rotationPolicy`. +###### Requires: No. + +In case the `none` algorithms ma1sd will be lookup using the v1 bulk API. + ```.yaml hashing: enabled: true # enable or disable the hash lookup MSC2140 (default is false) diff --git a/ma1sd.example.yaml b/ma1sd.example.yaml index 05b40a0..9aaeaa4 100644 --- a/ma1sd.example.yaml +++ b/ma1sd.example.yaml @@ -22,7 +22,7 @@ matrix: domain: '' v1: true # deprecated - v2: true # MSC2140 API v2 + v2: false # MSC2140 API v2. Disabled by default in order to preserve backward compatibility. ################ @@ -115,16 +115,16 @@ threepid: #### MSC2134 (hash lookup) -hashing: - enabled: false # enable or disable the hash lookup MSC2140 (default is false) - pepperLength: 20 # length of the pepper value (default is 20) - rotationPolicy: per_requests # or `per_seconds` how often the hashes will be updating - hashStorageType: sql # or `in_memory` where the hashes will be stored - algorithms: - - none # the same as v1 bulk lookup - - sha256 # hash the 3PID and pepper. - delay: 2m # how often hashes will be updated if rotation policy = per_seconds (default is 10s) - requests: 10 # how many lookup requests will be performed before updating hashes if rotation policy = per_requests (default is 10) +#hashing: +# enabled: false # enable or disable the hash lookup MSC2140 (default is false) +# pepperLength: 20 # length of the pepper value (default is 20) +# rotationPolicy: per_requests # or `per_seconds` how often the hashes will be updating +# hashStorageType: sql # or `in_memory` where the hashes will be stored +# algorithms: +# - none # the same as v1 bulk lookup +# - sha256 # hash the 3PID and pepper. +# delay: 2m # how often hashes will be updated if rotation policy = per_seconds (default is 10s) +# requests: 10 # how many lookup requests will be performed before updating hashes if rotation policy = per_requests (default is 10) ### hash lookup for synapseSql provider. # synapseSql: @@ -132,19 +132,19 @@ hashing: # query: 'select user_id as mxid, medium, address from user_threepids' # query for retrive 3PIDs for hashes. #### MSC2140 (Terms) -policy: - policies: - term_name: # term name - version: 1.0 # version - terms: - en: # lang - name: term name en # localized name - url: https://ma1sd.host.tld/term_en.html # localized url - fe: # lang - name: term name fr # localized name - url: https://ma1sd.host.tld/term_fr.html # localized url - regexp: - - '/_matrix/identity/v2/account.*' - - '/_matrix/identity/v2/hash_details' - - '/_matrix/identity/v2/lookup' - +#policy: +# policies: +# term_name: # term name +# version: 1.0 # version +# terms: +# en: # lang +# name: term name en # localized name +# url: https://ma1sd.host.tld/term_en.html # localized url +# fe: # lang +# name: term name fr # localized name +# url: https://ma1sd.host.tld/term_fr.html # localized url +# regexp: +# - '/_matrix/identity/v2/account.*' +# - '/_matrix/identity/v2/hash_details' +# - '/_matrix/identity/v2/lookup' +# diff --git a/src/main/java/io/kamax/mxisd/HttpMxisd.java b/src/main/java/io/kamax/mxisd/HttpMxisd.java index c3aff61..82b59a3 100644 --- a/src/main/java/io/kamax/mxisd/HttpMxisd.java +++ b/src/main/java/io/kamax/mxisd/HttpMxisd.java @@ -189,23 +189,32 @@ public class HttpMxisd { } private void accountEndpoints(RoutingHandler routingHandler) { - routingHandler.post(AccountRegisterHandler.Path, SaneHandler.around(new AccountRegisterHandler(m.getAccMgr()))); - wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new AccountGetUserInfoHandler(m.getAccMgr())), - AccountGetUserInfoHandler.Path, true); - wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new AccountLogoutHandler(m.getAccMgr())), - AccountLogoutHandler.Path, true); + MatrixConfig matrixConfig = m.getConfig().getMatrix(); + if (matrixConfig.isV2()) { + routingHandler.post(AccountRegisterHandler.Path, SaneHandler.around(new AccountRegisterHandler(m.getAccMgr()))); + wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new AccountGetUserInfoHandler(m.getAccMgr())), + AccountGetUserInfoHandler.Path, true); + wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new AccountLogoutHandler(m.getAccMgr())), + AccountLogoutHandler.Path, true); + } } private void termsEndpoints(RoutingHandler routingHandler) { - routingHandler.get(GetTermsHandler.PATH, new GetTermsHandler(m.getConfig().getPolicy())); - routingHandler.post(AcceptTermsHandler.PATH, sane(new AcceptTermsHandler(m.getAccMgr()))); + MatrixConfig matrixConfig = m.getConfig().getMatrix(); + if (matrixConfig.isV2()) { + routingHandler.get(GetTermsHandler.PATH, new GetTermsHandler(m.getConfig().getPolicy())); + routingHandler.post(AcceptTermsHandler.PATH, sane(new AcceptTermsHandler(m.getAccMgr()))); + } } private void hashEndpoints(RoutingHandler routingHandler) { - wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new HashDetailsHandler(m.getHashManager())), - HashDetailsHandler.PATH, true); - wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.POST, - sane(new HashLookupHandler(m.getIdentity(), m.getHashManager())), HashLookupHandler.Path, true); + MatrixConfig matrixConfig = m.getConfig().getMatrix(); + if (matrixConfig.isV2()) { + wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.GET, sane(new HashDetailsHandler(m.getHashManager())), + HashDetailsHandler.PATH, true); + wrapWithTokenAndAuthorizationHandlers(routingHandler, Methods.POST, + sane(new HashLookupHandler(m.getIdentity(), m.getHashManager())), HashLookupHandler.Path, true); + } } private void addEndpoints(RoutingHandler routingHandler, HttpString method, boolean useAuthorization, ApiHandler... handlers) { diff --git a/src/main/java/io/kamax/mxisd/auth/AccountManager.java b/src/main/java/io/kamax/mxisd/auth/AccountManager.java index 666dbe5..5b59ca8 100644 --- a/src/main/java/io/kamax/mxisd/auth/AccountManager.java +++ b/src/main/java/io/kamax/mxisd/auth/AccountManager.java @@ -10,6 +10,7 @@ import io.kamax.mxisd.exception.BadRequestException; import io.kamax.mxisd.exception.InvalidCredentialsException; import io.kamax.mxisd.exception.NotFoundException; import io.kamax.mxisd.matrix.HomeserverFederationResolver; +import io.kamax.mxisd.matrix.HomeserverVerifier; import io.kamax.mxisd.storage.IStorage; import io.kamax.mxisd.storage.ormlite.dao.AccountDao; import org.apache.http.HttpStatus; @@ -22,18 +23,10 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.security.cert.Certificate; -import java.security.cert.CertificateParsingException; -import java.security.cert.X509Certificate; import java.time.Instant; -import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.Objects; import java.util.UUID; -import javax.net.ssl.HostnameVerifier; -import javax.net.ssl.SSLPeerUnverifiedException; -import javax.net.ssl.SSLSession; public class AccountManager { @@ -80,7 +73,7 @@ public class AccountManager { homeserverURL + "/_matrix/federation/v1/openid/userinfo?access_token=" + openIdToken.getAccessToken()); String userId; try (CloseableHttpClient httpClient = HttpClients.custom() - .setSSLHostnameVerifier(new MatrixHostnameVerifier(homeserverTarget.getDomain())).build()) { + .setSSLHostnameVerifier(new HomeserverVerifier(homeserverTarget.getDomain())).build()) { try (CloseableHttpResponse response = httpClient.execute(getUserInfo)) { int statusCode = response.getStatusLine().getStatusCode(); if (statusCode == HttpStatus.SC_OK) { @@ -170,74 +163,4 @@ public class AccountManager { public MatrixConfig getMatrixConfig() { return matrixConfig; } - - public static class MatrixHostnameVerifier implements HostnameVerifier { - - private static final String ALT_DNS_NAME_TYPE = "2"; - private static final String ALT_IP_ADDRESS_TYPE = "7"; - - private final String matrixHostname; - - public MatrixHostnameVerifier(String matrixHostname) { - this.matrixHostname = matrixHostname; - } - - @Override - public boolean verify(String hostname, SSLSession session) { - try { - Certificate peerCertificate = session.getPeerCertificates()[0]; - if (peerCertificate instanceof X509Certificate) { - X509Certificate x509Certificate = (X509Certificate) peerCertificate; - if (x509Certificate.getSubjectAlternativeNames() == null) { - return false; - } - for (String altSubjectName : getAltSubjectNames(x509Certificate)) { - if (match(altSubjectName)) { - return true; - } - } - } - } catch (SSLPeerUnverifiedException | CertificateParsingException e) { - LOGGER.error("Unable to check remote host", e); - return false; - } - - return false; - } - - private List getAltSubjectNames(X509Certificate x509Certificate) { - List subjectNames = new ArrayList<>(); - try { - for (List subjectAlternativeNames : x509Certificate.getSubjectAlternativeNames()) { - if (subjectAlternativeNames == null - || subjectAlternativeNames.size() < 2 - || subjectAlternativeNames.get(0) == null - || subjectAlternativeNames.get(1) == null) { - continue; - } - String subjectType = subjectAlternativeNames.get(0).toString(); - switch (subjectType) { - case ALT_DNS_NAME_TYPE: - case ALT_IP_ADDRESS_TYPE: - subjectNames.add(subjectAlternativeNames.get(1).toString()); - break; - default: - LOGGER.trace("Unusable subject type: " + subjectType); - } - } - } catch (CertificateParsingException e) { - LOGGER.error("Unable to parse the certificate", e); - return Collections.emptyList(); - } - return subjectNames; - } - - private boolean match(String altSubjectName) { - if (altSubjectName.startsWith("*.")) { - return altSubjectName.toLowerCase().endsWith(matrixHostname.toLowerCase()); - } else { - return matrixHostname.equalsIgnoreCase(altSubjectName); - } - } - } } diff --git a/src/main/java/io/kamax/mxisd/config/MatrixConfig.java b/src/main/java/io/kamax/mxisd/config/MatrixConfig.java index 0d21f07..285d958 100644 --- a/src/main/java/io/kamax/mxisd/config/MatrixConfig.java +++ b/src/main/java/io/kamax/mxisd/config/MatrixConfig.java @@ -64,7 +64,7 @@ public class MatrixConfig { private String domain; private Identity identity = new Identity(); private boolean v1 = true; - private boolean v2 = true; + private boolean v2 = false; public String getDomain() { return domain; diff --git a/src/main/java/io/kamax/mxisd/http/undertow/handler/auth/v2/AccountRegisterHandler.java b/src/main/java/io/kamax/mxisd/http/undertow/handler/auth/v2/AccountRegisterHandler.java index f10000e..81dbb3b 100644 --- a/src/main/java/io/kamax/mxisd/http/undertow/handler/auth/v2/AccountRegisterHandler.java +++ b/src/main/java/io/kamax/mxisd/http/undertow/handler/auth/v2/AccountRegisterHandler.java @@ -48,7 +48,7 @@ public class AccountRegisterHandler extends BasicHttpHandler { if (LOGGER.isInfoEnabled()) { LOGGER.info("Registration from domain: {}, expired at {}", openIdToken.getMatrixServerName(), - new Date(openIdToken.getExpiresIn())); + new Date(System.currentTimeMillis() + openIdToken.getExpiresIn())); } String token = accountManager.register(openIdToken); diff --git a/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v2/HashLookupHandler.java b/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v2/HashLookupHandler.java index 8fb55f3..2b21c84 100644 --- a/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v2/HashLookupHandler.java +++ b/src/main/java/io/kamax/mxisd/http/undertow/handler/identity/v2/HashLookupHandler.java @@ -31,6 +31,8 @@ import io.kamax.mxisd.http.undertow.handler.ApiHandler; import io.kamax.mxisd.http.undertow.handler.identity.share.LookupHandler; import io.kamax.mxisd.lookup.BulkLookupRequest; import io.kamax.mxisd.lookup.HashLookupRequest; +import io.kamax.mxisd.lookup.SingleLookupReply; +import io.kamax.mxisd.lookup.SingleLookupRequest; import io.kamax.mxisd.lookup.ThreePidMapping; import io.kamax.mxisd.lookup.strategy.LookupStrategy; import io.undertow.server.HttpServerExchange; @@ -40,6 +42,7 @@ import org.slf4j.LoggerFactory; import java.util.ArrayList; import java.util.List; +import java.util.Optional; public class HashLookupHandler extends LookupHandler implements ApiHandler { @@ -89,6 +92,18 @@ public class HashLookupHandler extends LookupHandler implements ApiHandler { throw new InvalidParamException(); } + ClientHashLookupAnswer answer = null; + if (input.getAddresses() != null && input.getAddresses().size() > 0) { + if (input.getAddresses().size() == 1) { + answer = noneSingleLookup(request, input); + } else { + answer = noneBulkLookup(request, input); + } + } + respondJson(exchange, answer != null ? answer : new ClientHashLookupAnswer()); + } + + private ClientHashLookupAnswer noneBulkLookup(HashLookupRequest request, ClientHashLookupRequest input) throws Exception { BulkLookupRequest bulkLookupRequest = new BulkLookupRequest(); List mappings = new ArrayList<>(); for (String address : input.getAddresses()) { @@ -107,7 +122,26 @@ public class HashLookupHandler extends LookupHandler implements ApiHandler { } log.info("Finished bulk lookup request from {}", request.getRequester()); - respondJson(exchange, answer); + return answer; + } + + private ClientHashLookupAnswer noneSingleLookup(HashLookupRequest request, ClientHashLookupRequest input) { + SingleLookupRequest singleLookupRequest = new SingleLookupRequest(); + String address = input.getAddresses().get(0); + String[] parts = address.split(" "); + singleLookupRequest.setThreePid(parts[0]); + singleLookupRequest.setType(parts[1]); + + ClientHashLookupAnswer answer = new ClientHashLookupAnswer(); + + Optional singleLookupReply = strategy.find(singleLookupRequest); + if (singleLookupReply.isPresent()) { + SingleLookupReply reply = singleLookupReply.get(); + answer.getMappings().put(address, reply.getMxid().toString()); + } + log.info("Finished single lookup request from {}", request.getRequester()); + + return answer; } private void sha256Algorithm(HttpServerExchange exchange, HashLookupRequest request, ClientHashLookupRequest input) { diff --git a/src/main/java/io/kamax/mxisd/matrix/HomeserverVerifier.java b/src/main/java/io/kamax/mxisd/matrix/HomeserverVerifier.java index 78c943b..e8e9c03 100644 --- a/src/main/java/io/kamax/mxisd/matrix/HomeserverVerifier.java +++ b/src/main/java/io/kamax/mxisd/matrix/HomeserverVerifier.java @@ -77,7 +77,8 @@ public class HomeserverVerifier implements HostnameVerifier { private boolean match(String altSubjectName) { if (altSubjectName.startsWith("*.")) { - return altSubjectName.toLowerCase().endsWith(matrixHostname.toLowerCase()); + String subjectNameWithoutMask = altSubjectName.substring(1); // remove wildcard + return matrixHostname.toLowerCase().endsWith(subjectNameWithoutMask.toLowerCase()); } else { return matrixHostname.equalsIgnoreCase(altSubjectName); } diff --git a/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqlStorage.java b/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqlStorage.java index 235234b..c3a17c2 100644 --- a/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqlStorage.java +++ b/src/main/java/io/kamax/mxisd/storage/ormlite/OrmLiteSqlStorage.java @@ -39,6 +39,7 @@ import io.kamax.mxisd.storage.IStorage; import io.kamax.mxisd.storage.dao.IThreePidSessionDao; import io.kamax.mxisd.storage.ormlite.dao.ASTransactionDao; import io.kamax.mxisd.storage.ormlite.dao.AccountDao; +import io.kamax.mxisd.storage.ormlite.dao.ChangelogDao; import io.kamax.mxisd.storage.ormlite.dao.HashDao; import io.kamax.mxisd.storage.ormlite.dao.HistoricalThreePidInviteIO; import io.kamax.mxisd.storage.ormlite.dao.AcceptedDao; @@ -52,6 +53,7 @@ import java.sql.SQLException; import java.time.Instant; import java.util.ArrayList; import java.util.Collection; +import java.util.Date; import java.util.List; import java.util.Optional; import java.util.UUID; @@ -73,13 +75,18 @@ public class OrmLiteSqlStorage implements IStorage { } + public static class Migrations { + public static final String FIX_ACCEPTED_DAO = "2019_12_09__2254__fix_accepted_dao"; + } + private Dao invDao; private Dao expInvDao; private Dao sessionDao; private Dao asTxnDao; private Dao accountDao; - private Dao acceptedDao; + private Dao acceptedDao; private Dao hashDao; + private Dao changelogDao; public OrmLiteSqlStorage(MxisdConfig cfg) { this(cfg.getStorage().getBackend(), cfg.getStorage().getProvider().getSqlite().getDatabase()); @@ -96,6 +103,7 @@ public class OrmLiteSqlStorage implements IStorage { withCatcher(() -> { ConnectionSource connPool = new JdbcConnectionSource("jdbc:" + backend + ":" + path); + changelogDao = createDaoAndTable(connPool, ChangelogDao.class); invDao = createDaoAndTable(connPool, ThreePidInviteIO.class); expInvDao = createDaoAndTable(connPool, HistoricalThreePidInviteIO.class); sessionDao = createDaoAndTable(connPool, ThreePidSessionDao.class); @@ -103,9 +111,23 @@ public class OrmLiteSqlStorage implements IStorage { accountDao = createDaoAndTable(connPool, AccountDao.class); acceptedDao = createDaoAndTable(connPool, AcceptedDao.class); hashDao = createDaoAndTable(connPool, HashDao.class); + runMigration(connPool); }); } + private void runMigration(ConnectionSource connPol) throws SQLException { + ChangelogDao fixAcceptedDao = changelogDao.queryForId(Migrations.FIX_ACCEPTED_DAO); + if (fixAcceptedDao == null) { + fixAcceptedDao(connPol); + changelogDao.create(new ChangelogDao(Migrations.FIX_ACCEPTED_DAO, new Date(), "Recreate the accepted table.")); + } + } + + private void fixAcceptedDao(ConnectionSource connPool) throws SQLException { + TableUtils.dropTable(acceptedDao, true); + TableUtils.createTableIfNotExists(connPool, AcceptedDao.class); + } + private Dao createDaoAndTable(ConnectionSource connPool, Class c) throws SQLException { Dao dao = DaoManager.createDao(connPool, c); TableUtils.createTableIfNotExists(connPool, c); diff --git a/src/main/java/io/kamax/mxisd/storage/ormlite/dao/AcceptedDao.java b/src/main/java/io/kamax/mxisd/storage/ormlite/dao/AcceptedDao.java index eb8c907..f7a1a56 100644 --- a/src/main/java/io/kamax/mxisd/storage/ormlite/dao/AcceptedDao.java +++ b/src/main/java/io/kamax/mxisd/storage/ormlite/dao/AcceptedDao.java @@ -26,7 +26,7 @@ import com.j256.ormlite.table.DatabaseTable; @DatabaseTable(tableName = "accepted") public class AcceptedDao { - @DatabaseField(canBeNull = false, id = true) + @DatabaseField(canBeNull = false) private String url; @DatabaseField(canBeNull = false) diff --git a/src/main/java/io/kamax/mxisd/storage/ormlite/dao/ChangelogDao.java b/src/main/java/io/kamax/mxisd/storage/ormlite/dao/ChangelogDao.java new file mode 100644 index 0000000..53a39fb --- /dev/null +++ b/src/main/java/io/kamax/mxisd/storage/ormlite/dao/ChangelogDao.java @@ -0,0 +1,52 @@ +package io.kamax.mxisd.storage.ormlite.dao; + +import com.j256.ormlite.field.DatabaseField; +import com.j256.ormlite.table.DatabaseTable; + +import java.util.Date; + +@DatabaseTable(tableName = "changelog") +public class ChangelogDao { + + @DatabaseField(id = true) + private String id; + + @DatabaseField + private Date createdAt; + + @DatabaseField + private String comment; + + public ChangelogDao() { + } + + public ChangelogDao(String id, Date createdAt, String comment) { + this.id = id; + this.createdAt = createdAt; + this.comment = comment; + } + + public String getId() { + return id; + } + + public void setId(String id) { + this.id = id; + } + + public Date getCreatedAt() { + return createdAt; + } + + public void setCreatedAt(Date createdAt) { + this.createdAt = createdAt; + } + + public String getComment() { + return comment; + } + + public void setComment(String comment) { + this.comment = comment; + } +}