Fix response body of /3pid/bind to match spec

- synapse did not check/validate the response as per spec until 0.99.5 it seems
- mxisd was never compliant also
This commit is contained in:
Max Dor
2019-05-30 13:26:38 +02:00
parent 544f8e59f0
commit 0ddd086bda
11 changed files with 65 additions and 22 deletions

View File

@@ -105,7 +105,7 @@ public class HttpMxisd {
.get(SessionValidateHandler.Path, SaneHandler.around(new SessionValidationGetHandler(m.getSession(), m.getConfig()))) .get(SessionValidateHandler.Path, SaneHandler.around(new SessionValidationGetHandler(m.getSession(), m.getConfig())))
.post(SessionValidateHandler.Path, SaneHandler.around(new SessionValidationPostHandler(m.getSession()))) .post(SessionValidateHandler.Path, SaneHandler.around(new SessionValidationPostHandler(m.getSession())))
.get(SessionTpidGetValidatedHandler.Path, SaneHandler.around(new SessionTpidGetValidatedHandler(m.getSession()))) .get(SessionTpidGetValidatedHandler.Path, SaneHandler.around(new SessionTpidGetValidatedHandler(m.getSession())))
.post(SessionTpidBindHandler.Path, SaneHandler.around(new SessionTpidBindHandler(m.getSession(), m.getInvite()))) .post(SessionTpidBindHandler.Path, SaneHandler.around(new SessionTpidBindHandler(m.getSession(), m.getInvite(), m.getSign())))
.post(SessionTpidUnbindHandler.Path, SaneHandler.around(new SessionTpidUnbindHandler(m.getSession()))) .post(SessionTpidUnbindHandler.Path, SaneHandler.around(new SessionTpidUnbindHandler(m.getSession())))
.post(SignEd25519Handler.Path, SaneHandler.around(new SignEd25519Handler(m.getConfig(), m.getInvite(), m.getSign()))) .post(SignEd25519Handler.Path, SaneHandler.around(new SignEd25519Handler(m.getConfig(), m.getInvite(), m.getSign())))

View File

@@ -107,7 +107,7 @@ public class Mxisd {
store = new OrmLiteSqlStorage(cfg); store = new OrmLiteSqlStorage(cfg);
keyMgr = CryptoFactory.getKeyManager(cfg.getKey()); keyMgr = CryptoFactory.getKeyManager(cfg.getKey());
signMgr = CryptoFactory.getSignatureManager(keyMgr); signMgr = CryptoFactory.getSignatureManager(cfg, keyMgr);
clientDns = new ClientDnsOverwrite(cfg.getDns().getOverwrite()); clientDns = new ClientDnsOverwrite(cfg.getDns().getOverwrite());
synapse = new Synapse(cfg.getSynapseSql()); synapse = new Synapse(cfg.getSynapseSql());

View File

@@ -83,6 +83,12 @@ public class MxisdConfig {
} }
public static MxisdConfig forDomain(String domain) {
MxisdConfig cfg = new MxisdConfig();
cfg.getMatrix().setDomain(domain);
return cfg;
}
private AppServiceConfig appsvc = new AppServiceConfig(); private AppServiceConfig appsvc = new AppServiceConfig();
private AuthenticationConfig auth = new AuthenticationConfig(); private AuthenticationConfig auth = new AuthenticationConfig();
private DirectoryConfig directory = new DirectoryConfig(); private DirectoryConfig directory = new DirectoryConfig();
@@ -309,6 +315,13 @@ public class MxisdConfig {
this.wordpress = wordpress; this.wordpress = wordpress;
} }
public MxisdConfig inMemory() {
getKey().setPath(":memory:");
getStorage().getProvider().getSqlite().setDatabase(":memory:");
return this;
}
public MxisdConfig build() { public MxisdConfig build() {
if (StringUtils.isBlank(getServer().getName())) { if (StringUtils.isBlank(getServer().getName())) {
getServer().setName(getMatrix().getDomain()); getServer().setName(getMatrix().getDomain());

View File

@@ -21,6 +21,7 @@
package io.kamax.mxisd.crypto; package io.kamax.mxisd.crypto;
import io.kamax.mxisd.config.KeyConfig; import io.kamax.mxisd.config.KeyConfig;
import io.kamax.mxisd.config.MxisdConfig;
import io.kamax.mxisd.crypto.ed25519.Ed25519KeyManager; import io.kamax.mxisd.crypto.ed25519.Ed25519KeyManager;
import io.kamax.mxisd.crypto.ed25519.Ed25519SignatureManager; import io.kamax.mxisd.crypto.ed25519.Ed25519SignatureManager;
import io.kamax.mxisd.storage.crypto.FileKeyStore; import io.kamax.mxisd.storage.crypto.FileKeyStore;
@@ -54,8 +55,8 @@ public class CryptoFactory {
return new Ed25519KeyManager(store); return new Ed25519KeyManager(store);
} }
public static SignatureManager getSignatureManager(Ed25519KeyManager keyMgr) { public static SignatureManager getSignatureManager(MxisdConfig cfg, Ed25519KeyManager keyMgr) {
return new Ed25519SignatureManager(keyMgr); return new Ed25519SignatureManager(cfg, keyMgr);
} }
} }

View File

@@ -30,6 +30,18 @@ import java.util.Objects;
public interface SignatureManager { public interface SignatureManager {
/**
* Sign the message with the default domain and add the signature to the <code>signatures</code> key.
* <p>
* If the key does not exist yet, it is created. If the key exist, the produced signature will be merged with any
* existing ones.
*
* @param message The message to sign with the default domain and add the produced signature to
* @return The provided message with the new signature
* @throws IllegalArgumentException If the <code>signatures</code> key exists and its value is not a JSON object
*/
JsonObject signMessageGson(JsonObject message) throws IllegalArgumentException;
/** /**
* Sign the message and add the signature to the <code>signatures</code> key. * Sign the message and add the signature to the <code>signatures</code> key.
* <p> * <p>
@@ -39,7 +51,7 @@ public interface SignatureManager {
* @param domain The domain under which the signature should be added * @param domain The domain under which the signature should be added
* @param message The message to sign and add the produced signature to * @param message The message to sign and add the produced signature to
* @return The provided message with the new signature * @return The provided message with the new signature
* @throws IllegalArgumentException If the <code>signatures</code> value is not a JSON object * @throws IllegalArgumentException If the <code>signatures</code> key exists and its value is not a JSON object
*/ */
default JsonObject signMessageGson(String domain, JsonObject message) throws IllegalArgumentException { default JsonObject signMessageGson(String domain, JsonObject message) throws IllegalArgumentException {
JsonElement signEl = message.remove(EventKey.Signatures.get()); JsonElement signEl = message.remove(EventKey.Signatures.get());

View File

@@ -23,6 +23,8 @@ package io.kamax.mxisd.crypto.ed25519;
import com.google.gson.JsonObject; import com.google.gson.JsonObject;
import io.kamax.matrix.codec.MxBase64; import io.kamax.matrix.codec.MxBase64;
import io.kamax.matrix.json.MatrixJson; import io.kamax.matrix.json.MatrixJson;
import io.kamax.mxisd.config.MxisdConfig;
import io.kamax.mxisd.config.ServerConfig;
import io.kamax.mxisd.crypto.KeyIdentifier; import io.kamax.mxisd.crypto.KeyIdentifier;
import io.kamax.mxisd.crypto.Signature; import io.kamax.mxisd.crypto.Signature;
import io.kamax.mxisd.crypto.SignatureManager; import io.kamax.mxisd.crypto.SignatureManager;
@@ -35,12 +37,19 @@ import java.security.SignatureException;
public class Ed25519SignatureManager implements SignatureManager { public class Ed25519SignatureManager implements SignatureManager {
private final ServerConfig cfg;
private final Ed25519KeyManager keyMgr; private final Ed25519KeyManager keyMgr;
public Ed25519SignatureManager(Ed25519KeyManager keyMgr) { public Ed25519SignatureManager(MxisdConfig cfg, Ed25519KeyManager keyMgr) {
this.cfg = cfg.getServer();
this.keyMgr = keyMgr; this.keyMgr = keyMgr;
} }
@Override
public JsonObject signMessageGson(JsonObject message) throws IllegalArgumentException {
return signMessageGson(cfg.getName(), message);
}
@Override @Override
public JsonObject signMessageGson(String domain, String message) { public JsonObject signMessageGson(String domain, String message) {
Signature sign = sign(message); Signature sign = sign(message);

View File

@@ -21,11 +21,15 @@
package io.kamax.mxisd.http.undertow.handler.identity.v1; package io.kamax.mxisd.http.undertow.handler.identity.v1;
import com.google.gson.JsonObject; import com.google.gson.JsonObject;
import io.kamax.matrix.json.GsonUtil;
import io.kamax.mxisd.crypto.SignatureManager;
import io.kamax.mxisd.exception.BadRequestException; import io.kamax.mxisd.exception.BadRequestException;
import io.kamax.mxisd.http.IsAPIv1; import io.kamax.mxisd.http.IsAPIv1;
import io.kamax.mxisd.http.io.identity.BindRequest; import io.kamax.mxisd.http.io.identity.BindRequest;
import io.kamax.mxisd.http.io.identity.SingeLookupReplyJson;
import io.kamax.mxisd.http.undertow.handler.BasicHttpHandler; import io.kamax.mxisd.http.undertow.handler.BasicHttpHandler;
import io.kamax.mxisd.invitation.InvitationManager; import io.kamax.mxisd.invitation.InvitationManager;
import io.kamax.mxisd.lookup.SingleLookupReply;
import io.kamax.mxisd.session.SessionManager; import io.kamax.mxisd.session.SessionManager;
import io.undertow.server.HttpServerExchange; import io.undertow.server.HttpServerExchange;
import io.undertow.util.QueryParameterUtils; import io.undertow.util.QueryParameterUtils;
@@ -42,14 +46,16 @@ public class SessionTpidBindHandler extends BasicHttpHandler {
public static final String Path = IsAPIv1.Base + "/3pid/bind"; public static final String Path = IsAPIv1.Base + "/3pid/bind";
private transient final Logger log = LoggerFactory.getLogger(SessionTpidBindHandler.class); private static final Logger log = LoggerFactory.getLogger(SessionTpidBindHandler.class);
private SessionManager mgr; private SessionManager mgr;
private InvitationManager invMgr; private InvitationManager invMgr;
private SignatureManager signMgr;
public SessionTpidBindHandler(SessionManager mgr, InvitationManager invMgr) { public SessionTpidBindHandler(SessionManager mgr, InvitationManager invMgr, SignatureManager signMgr) {
this.mgr = mgr; this.mgr = mgr;
this.invMgr = invMgr; this.invMgr = invMgr;
this.signMgr = signMgr;
} }
@Override @Override
@@ -74,8 +80,9 @@ public class SessionTpidBindHandler extends BasicHttpHandler {
} }
try { try {
mgr.bind(bindReq.getSid(), bindReq.getSecret(), bindReq.getUserId()); SingleLookupReply lookup = mgr.bind(bindReq.getSid(), bindReq.getSecret(), bindReq.getUserId());
respond(exchange, new JsonObject()); JsonObject response = signMgr.signMessageGson(GsonUtil.makeObj(new SingeLookupReplyJson(lookup)));
respond(exchange, response);
} catch (BadRequestException e) { } catch (BadRequestException e) {
log.info("requested session was not validated"); log.info("requested session was not validated");

View File

@@ -32,6 +32,7 @@ import io.kamax.mxisd.exception.NotImplementedException;
import io.kamax.mxisd.exception.SessionNotValidatedException; import io.kamax.mxisd.exception.SessionNotValidatedException;
import io.kamax.mxisd.exception.SessionUnknownException; import io.kamax.mxisd.exception.SessionUnknownException;
import io.kamax.mxisd.lookup.SingleLookupReply; import io.kamax.mxisd.lookup.SingleLookupReply;
import io.kamax.mxisd.lookup.SingleLookupRequest;
import io.kamax.mxisd.lookup.ThreePidValidation; import io.kamax.mxisd.lookup.ThreePidValidation;
import io.kamax.mxisd.lookup.strategy.LookupStrategy; import io.kamax.mxisd.lookup.strategy.LookupStrategy;
import io.kamax.mxisd.notification.NotificationManager; import io.kamax.mxisd.notification.NotificationManager;
@@ -43,6 +44,7 @@ import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger; import org.slf4j.Logger;
import org.slf4j.LoggerFactory; import org.slf4j.LoggerFactory;
import java.time.Instant;
import java.util.Optional; import java.util.Optional;
import static io.kamax.mxisd.config.SessionConfig.Policy.PolicyTemplate; import static io.kamax.mxisd.config.SessionConfig.Policy.PolicyTemplate;
@@ -151,7 +153,7 @@ public class SessionManager {
return new ThreePidValidation(session.getThreePid(), session.getValidationTime()); return new ThreePidValidation(session.getThreePid(), session.getValidationTime());
} }
public void bind(String sid, String secret, String mxidRaw) { public SingleLookupReply bind(String sid, String secret, String mxidRaw) {
// We make sure we have an acceptable User ID // We make sure we have an acceptable User ID
if (StringUtils.isEmpty(mxidRaw)) { if (StringUtils.isEmpty(mxidRaw)) {
throw new IllegalArgumentException("No Matrix User ID provided"); throw new IllegalArgumentException("No Matrix User ID provided");
@@ -165,11 +167,16 @@ public class SessionManager {
// Only accept binds if the domain matches our own // Only accept binds if the domain matches our own
if (!StringUtils.equalsIgnoreCase(mxCfg.getDomain(), mxid.getDomain())) { if (!StringUtils.equalsIgnoreCase(mxCfg.getDomain(), mxid.getDomain())) {
throw new NotAllowedException("Only Matrix IDs from domain " + mxCfg + " can be bound"); throw new NotAllowedException("Only Matrix IDs from domain " + mxCfg.getDomain() + " can be bound");
} }
log.info("Session {}: Binding of {}:{} to Matrix ID {} is accepted", log.info("Session {}: Binding of {}:{} to Matrix ID {} is accepted",
session.getId(), session.getThreePid().getMedium(), session.getThreePid().getAddress(), mxid.getId()); session.getId(), session.getThreePid().getMedium(), session.getThreePid().getAddress(), mxid.getId());
SingleLookupRequest request = new SingleLookupRequest();
request.setType(session.getThreePid().getMedium());
request.setThreePid(session.getThreePid().getAddress());
return new SingleLookupReply(request, mxid, Instant.now(), Instant.now().minusSeconds(5 * 60), Instant.now().plusSeconds(5 * 60));
} }
public void unbind(JsonObject reqData) { public void unbind(JsonObject reqData) {

View File

@@ -33,11 +33,7 @@ public class MxisdDefaultTest {
@Test @Test
public void defaultConfig() { public void defaultConfig() {
MxisdConfig cfg = new MxisdConfig(); MxisdConfig cfg = MxisdConfig.forDomain(domain).inMemory();
cfg.getMatrix().setDomain(domain);
cfg.getKey().setPath(":memory:");
cfg.getStorage().getProvider().getSqlite().setDatabase(":memory:");
Mxisd m = new Mxisd(cfg); Mxisd m = new Mxisd(cfg);
m.start(); m.start();

View File

@@ -41,10 +41,7 @@ public class MxisdTest {
@Before @Before
public void before() { public void before() {
MxisdConfig cfg = new MxisdConfig(); MxisdConfig cfg = MxisdConfig.forDomain("localhost").inMemory();
cfg.getMatrix().setDomain("localhost");
cfg.getKey().setPath(":memory:");
cfg.getStorage().getProvider().getSqlite().setDatabase(":memory:");
MemoryThreePid mem3pid = new MemoryThreePid(); MemoryThreePid mem3pid = new MemoryThreePid();
mem3pid.setMedium("email"); mem3pid.setMedium("email");

View File

@@ -24,6 +24,7 @@ import com.google.gson.JsonObject;
import io.kamax.matrix.event.EventKey; import io.kamax.matrix.event.EventKey;
import io.kamax.matrix.json.GsonUtil; import io.kamax.matrix.json.GsonUtil;
import io.kamax.matrix.json.MatrixJson; import io.kamax.matrix.json.MatrixJson;
import io.kamax.mxisd.config.MxisdConfig;
import io.kamax.mxisd.crypto.Signature; import io.kamax.mxisd.crypto.Signature;
import io.kamax.mxisd.crypto.SignatureManager; import io.kamax.mxisd.crypto.SignatureManager;
import io.kamax.mxisd.crypto.ed25519.Ed25519Key; import io.kamax.mxisd.crypto.ed25519.Ed25519Key;
@@ -52,7 +53,7 @@ public class SignatureManagerTest {
KeyStore store = new MemoryKeyStore(); KeyStore store = new MemoryKeyStore();
store.add(key); store.add(key);
return new Ed25519SignatureManager(new Ed25519KeyManager(store)); return new Ed25519SignatureManager(MxisdConfig.forDomain("localhost").inMemory().build(), new Ed25519KeyManager(store));
} }
@BeforeClass @BeforeClass