Implementation for blocking fraudulent 3PID /unbind attempts

This commit is contained in:
Max Dor
2019-02-01 02:34:52 +01:00
parent 4237eeb3b6
commit 635f6fdbe7
21 changed files with 361 additions and 37 deletions

View File

@@ -85,7 +85,7 @@ public class HttpMxisd {
.post(SessionValidateHandler.Path, sessValidateHandler)
.get(SessionTpidGetValidatedHandler.Path, SaneHandler.around(new SessionTpidGetValidatedHandler(m.getSession())))
.post(SessionTpidBindHandler.Path, SaneHandler.around(new SessionTpidBindHandler(m.getSession(), m.getInvitationManager())))
.post(SessionTpidUnbindHandler.Path, SaneHandler.around(new SessionTpidUnbindHandler()))
.post(SessionTpidUnbindHandler.Path, SaneHandler.around(new SessionTpidUnbindHandler(m.getSession())))
.get(RemoteIdentityAPIv1.SESSION_REQUEST_TOKEN, SaneHandler.around(new RemoteSessionStartHandler(m.getSession(), m.getConfig().getView())))
.get(RemoteIdentityAPIv1.SESSION_CHECK, SaneHandler.around(new RemoteSessionCheckHandler(m.getSession(), m.getConfig().getView())))

View File

@@ -102,7 +102,7 @@ public class Mxisd {
idStrategy = new RecursivePriorityLookupStrategy(cfg.getLookup(), ThreePidProviders.get(), bridgeFetcher);
pMgr = new ProfileManager(ProfileProviders.get(), clientDns, httpClient);
notifMgr = new NotificationManager(cfg.getNotification(), NotificationHandlers.get());
sessMgr = new SessionMananger(cfg.getSession(), cfg.getMatrix(), store, notifMgr, httpClient);
sessMgr = new SessionMananger(cfg.getSession(), cfg.getMatrix(), store, notifMgr, idStrategy, httpClient);
invMgr = new InvitationManager(cfg.getInvite(), store, idStrategy, signMgr, fedDns, notifMgr);
authMgr = new AuthManager(cfg, AuthProviders.get(), idStrategy, invMgr, clientDns, httpClient);
dirMgr = new DirectoryManager(cfg.getDirectory(), clientDns, httpClient, DirectoryProviders.get());

View File

@@ -125,6 +125,34 @@ public class SessionConfig {
}
public static class PolicyUnbind {
public static class PolicyUnbindFraudulent {
private boolean sendWarning = true;
public boolean getSendWarning() {
return sendWarning;
}
public void setSendWarning(boolean sendWarning) {
this.sendWarning = sendWarning;
}
}
private PolicyUnbindFraudulent fraudulent = new PolicyUnbindFraudulent();
public PolicyUnbindFraudulent getFraudulent() {
return fraudulent;
}
public void setFraudulent(PolicyUnbindFraudulent fraudulent) {
this.fraudulent = fraudulent;
}
}
public Policy() {
validation.enabled = true;
validation.forLocal.enabled = true;
@@ -139,6 +167,7 @@ public class SessionConfig {
}
private PolicyTemplate validation = new PolicyTemplate();
private PolicyUnbind unbind = new PolicyUnbind();
public PolicyTemplate getValidation() {
return validation;
@@ -148,6 +177,14 @@ public class SessionConfig {
this.validation = validation;
}
public PolicyUnbind getUnbind() {
return unbind;
}
public void setUnbind(PolicyUnbind unbind) {
this.unbind = unbind;
}
}
private Policy policy = new Policy();

View File

@@ -115,7 +115,7 @@ public class EmailSendGridConfig {
public static class Templates {
public static class TemplateSession {
public static class TemplateSessionValidation {
private EmailTemplate local = new EmailTemplate();
private EmailTemplate remote = new EmailTemplate();
@@ -137,6 +137,43 @@ public class EmailSendGridConfig {
}
}
public static class TemplateSessionUnbind {
private EmailTemplate fraudulent = new EmailTemplate();
public EmailTemplate getFraudulent() {
return fraudulent;
}
public void setFraudulent(EmailTemplate fraudulent) {
this.fraudulent = fraudulent;
}
}
public static class TemplateSession {
private TemplateSessionValidation validation = new TemplateSessionValidation();
private TemplateSessionUnbind unbind = new TemplateSessionUnbind();
public TemplateSessionValidation getValidation() {
return validation;
}
public void setValidation(TemplateSessionValidation validation) {
this.validation = validation;
}
public TemplateSessionUnbind getUnbind() {
return unbind;
}
public void setUnbind(TemplateSessionUnbind unbind) {
this.unbind = unbind;
}
}
private EmailTemplate invite = new EmailTemplate();
private TemplateSession session = new TemplateSession();
private Map<String, EmailTemplate> generic = new HashMap<>();

View File

@@ -32,6 +32,7 @@ public class EmailTemplateConfig extends GenericTemplateConfig {
getGeneric().put("matrixId", "classpath:/threepids/email/mxid-template.eml");
getSession().getValidation().setLocal("classpath:/threepids/email/validate-local-template.eml");
getSession().getValidation().setRemote("classpath:/threepids/email/validate-remote-template.eml");
getSession().getUnbind().setFraudulent("classpath:/threepids/email/unbind-fraudulent.eml");
}
public EmailTemplateConfig build() {

View File

@@ -62,7 +62,22 @@ public class GenericTemplateConfig {
}
public static class SessionUnbind {
private String fraudulent;
public String getFraudulent() {
return fraudulent;
}
public void setFraudulent(String fraudulent) {
this.fraudulent = fraudulent;
}
}
private SessionValidation validation = new SessionValidation();
private SessionUnbind unbind = new SessionUnbind();
public SessionValidation getValidation() {
return validation;
@@ -72,6 +87,14 @@ public class GenericTemplateConfig {
this.validation = validation;
}
public SessionUnbind getUnbind() {
return unbind;
}
public void setUnbind(SessionUnbind unbind) {
this.unbind = unbind;
}
}
private String invite;

View File

@@ -32,6 +32,7 @@ public class PhoneSmsTemplateConfig extends GenericTemplateConfig {
getGeneric().put("matrixId", "classpath:/threepids/email/mxid-template.eml");
getSession().getValidation().setLocal("classpath:/threepids/sms/validate-local-template.txt");
getSession().getValidation().setRemote("classpath:/threepids/sms/validate-remote-template.txt");
getSession().getUnbind().setFraudulent("classpath:/threepids/sms/unbind-fraudulent.txt");
}
public PhoneSmsTemplateConfig build() {

View File

@@ -21,10 +21,9 @@
package io.kamax.mxisd.http.undertow.handler.identity.v1;
import com.google.gson.JsonObject;
import io.kamax.mxisd.exception.FeatureNotAvailable;
import io.kamax.mxisd.exception.NotAllowedException;
import io.kamax.mxisd.http.IsAPIv1;
import io.kamax.mxisd.http.undertow.handler.BasicHttpHandler;
import io.kamax.mxisd.session.SessionMananger;
import io.undertow.server.HttpServerExchange;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -33,38 +32,19 @@ public class SessionTpidUnbindHandler extends BasicHttpHandler {
public static final String Path = IsAPIv1.Base + "/3pid/unbind";
private transient final Logger log = LoggerFactory.getLogger(SessionTpidUnbindHandler.class);
private static final Logger log = LoggerFactory.getLogger(SessionTpidUnbindHandler.class);
private final SessionMananger sessMgr;
public SessionTpidUnbindHandler(SessionMananger sessMgr) {
this.sessMgr = sessMgr;
}
@Override
public void handleRequest(HttpServerExchange exchange) {
JsonObject body = parseJsonObject(exchange);
// TODO also check for HS header to know which domain attempting the unbind
if (body.entrySet().size() == 2 && body.has("mxisd") && body.has("threepid")) {
/* This is a HS request to remove a 3PID and is considered:
* - An attack on user privacy
* - A baffling spec breakage requiring IS and HS 3PID info to be independent [1]
* - A baffling spec breakage that 3PID (un)bind is only one way [2]
*
* Given the lack of response on our extensive feedback on the proposal [3] which has not landed in the spec yet [4],
* We'll be denying such unbind requests and will inform users using their 3PID that a fraudulent attempt of
* removing their 3PID binding has been attempting but blocked.
*
* [1]: https://matrix.org/docs/spec/client_server/r0.4.0.html#adding-account-administrative-contact-information
* [2]: https://matrix.org/docs/spec/identity_service/r0.1.0.html#privacy
* [3]: https://docs.google.com/document/d/135g2muVxmuml0iUnLoTZxk8M2ZSt3kJzg81chGh51yg/edit
* [4]: https://github.com/matrix-org/matrix-doc/issues/1194
*/
log.warn("A remote host attempted to unbind without proper authorization. Request was denied");
// TODO notify the 3PID owner
throw new NotAllowedException("You have attempted to alter 3PID bindings, which can only be done by the 3PID owner directly. " +
"We have informed the 3PID owner of your fraudulent attempt.");
}
throw new FeatureNotAvailable("Unbind using a 3PID session is not defined in the spec");
sessMgr.unbind(body);
writeBodyAsUtf8(exchange, "{}");
}
}

View File

@@ -20,6 +20,7 @@
package io.kamax.mxisd.notification;
import io.kamax.matrix.ThreePid;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.invitation.IThreePidInviteReply;
import io.kamax.mxisd.threepid.session.IThreePidSession;
@@ -38,4 +39,6 @@ public interface NotificationHandler {
void sendForRemoteValidation(IThreePidSession session);
void sendForFraudulentUnbind(ThreePid tpid);
}

View File

@@ -20,6 +20,7 @@
package io.kamax.mxisd.notification;
import io.kamax.matrix.ThreePid;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.config.threepid.notification.NotificationConfig;
import io.kamax.mxisd.exception.NotImplementedException;
@@ -81,4 +82,8 @@ public class NotificationManager {
ensureMedium(session.getThreePid().getMedium()).sendForRemoteValidation(session);
}
public void sendForFraudulentUnbind(ThreePid tpid) throws NotImplementedException {
ensureMedium(tpid.getMedium()).sendForFraudulentUnbind(tpid);
}
}

View File

@@ -28,12 +28,15 @@ import io.kamax.matrix.MatrixID;
import io.kamax.matrix.ThreePid;
import io.kamax.matrix.ThreePidMedium;
import io.kamax.matrix._MatrixID;
import io.kamax.matrix.json.GsonUtil;
import io.kamax.mxisd.config.MatrixConfig;
import io.kamax.mxisd.config.SessionConfig;
import io.kamax.mxisd.exception.*;
import io.kamax.mxisd.http.io.identity.RequestTokenResponse;
import io.kamax.mxisd.http.undertow.handler.identity.v1.RemoteIdentityAPIv1;
import io.kamax.mxisd.lookup.SingleLookupReply;
import io.kamax.mxisd.lookup.ThreePidValidation;
import io.kamax.mxisd.lookup.strategy.LookupStrategy;
import io.kamax.mxisd.matrix.IdentityServerUtils;
import io.kamax.mxisd.notification.NotificationManager;
import io.kamax.mxisd.storage.IStorage;
@@ -71,6 +74,7 @@ public class SessionMananger {
private MatrixConfig mxCfg;
private IStorage storage;
private NotificationManager notifMgr;
private LookupStrategy lookupMgr;
private GsonParser parser = new GsonParser();
private PhoneNumberUtil phoneUtil = PhoneNumberUtil.getInstance(); // FIXME refactor for sessions handling their own stuff
@@ -78,11 +82,19 @@ public class SessionMananger {
// FIXME export into central class, set version
private CloseableHttpClient client;
public SessionMananger(SessionConfig cfg, MatrixConfig mxCfg, IStorage storage, NotificationManager notifMgr, CloseableHttpClient client) {
public SessionMananger(
SessionConfig cfg,
MatrixConfig mxCfg,
IStorage storage,
NotificationManager notifMgr,
LookupStrategy lookupMgr,
CloseableHttpClient client
) {
this.cfg = cfg;
this.mxCfg = mxCfg;
this.storage = storage;
this.notifMgr = notifMgr;
this.lookupMgr = lookupMgr;
this.client = client;
}
@@ -259,6 +271,54 @@ public class SessionMananger {
}
}
public void unbind(JsonObject reqData) {
// TODO also check for HS header to know which domain attempting the unbind
if (reqData.entrySet().size() == 2 && reqData.has("mxid") && reqData.has("threepid")) {
/* This is a HS request to remove a 3PID and is considered:
* - An attack on user privacy
* - A baffling spec breakage requiring IS and HS 3PID info to be independent [1]
* - A baffling spec breakage that 3PID (un)bind is only one way [2]
*
* Given the lack of response on our extensive feedback on the proposal [3] which has not landed in the spec yet [4],
* We'll be denying such unbind requests and will inform users using their 3PID that a fraudulent attempt of
* removing their 3PID binding has been attempted and blocked.
*
* [1]: https://matrix.org/docs/spec/client_server/r0.4.0.html#adding-account-administrative-contact-information
* [2]: https://matrix.org/docs/spec/identity_service/r0.1.0.html#privacy
* [3]: https://docs.google.com/document/d/135g2muVxmuml0iUnLoTZxk8M2ZSt3kJzg81chGh51yg/edit
* [4]: https://github.com/matrix-org/matrix-doc/issues/1194
*/
log.warn("A remote host attempted to unbind without proper authorization. Request was denied");
if (!cfg.getPolicy().getUnbind().getFraudulent().getSendWarning()) {
log.info("Not sending notification to 3PID owner as per configuration");
} else {
log.info("Sending notification to 3PID owner as per configuration");
ThreePid tpid = GsonUtil.get().fromJson(GsonUtil.getObj(reqData, "threepid"), ThreePid.class);
Optional<SingleLookupReply> lookup = lookupMgr.findLocal(tpid.getMedium(), tpid.getAddress());
if (!lookup.isPresent()) {
log.info("No 3PID owner found, not sending any notification");
} else {
log.info("3PID owner found, sending notification");
try {
notifMgr.sendForFraudulentUnbind(tpid);
log.info("Notification sent");
} catch (NotImplementedException e) {
log.warn("Unable to send notification: {}", e.getMessage());
} catch (RuntimeException e) {
log.warn("Unable to send notification due to unknown error. See stacktrace below", e);
}
}
}
}
log.info("Denying request");
throw new NotAllowedException("You have attempted to alter 3PID bindings, which can only be done by the 3PID owner directly. " +
"We have informed the 3PID owner of your fraudulent attempt.");
}
public IThreePidSession createRemote(String sid, String secret) {
ThreePidSession session = getSessionIfValidated(sid, secret);
log.info("Creating remote 3PID session for {} with local session [{}] to {}", session.getThreePid(), sid);

View File

@@ -20,6 +20,7 @@
package io.kamax.mxisd.threepid.generator;
import io.kamax.matrix.ThreePid;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.config.MatrixConfig;
import io.kamax.mxisd.config.ServerConfig;
@@ -82,4 +83,10 @@ public abstract class GenericTemplateNotificationGenerator extends PlaceholderNo
return populateForRemoteValidation(session, getTemplateContent(cfg.getSession().getValidation().getRemote()));
}
@Override
public String getForFraudulentUnbind(ThreePid tpid) {
log.info("Generating notification content for fraudulent unbind");
return populateForFraudulentUndind(tpid, getTemplateContent(cfg.getSession().getUnbind().getFraudulent()));
}
}

View File

@@ -20,6 +20,7 @@
package io.kamax.mxisd.threepid.generator;
import io.kamax.matrix.ThreePid;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.invitation.IThreePidInviteReply;
import io.kamax.mxisd.threepid.session.IThreePidSession;
@@ -38,4 +39,6 @@ public interface NotificationGenerator {
String getForRemoteValidation(IThreePidSession session);
String getForFraudulentUnbind(ThreePid tpid);
}

View File

@@ -106,4 +106,8 @@ public abstract class PlaceholderNotificationGenerator {
return populateForValidation(session, input);
}
protected String populateForFraudulentUndind(ThreePid tpid, String input) {
return populateForCommon(tpid, input);
}
}

View File

@@ -20,6 +20,7 @@
package io.kamax.mxisd.threepid.notification;
import io.kamax.matrix.ThreePid;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.exception.ConfigurationException;
import io.kamax.mxisd.invitation.IThreePidInviteReply;
@@ -76,4 +77,9 @@ public abstract class GenericNotificationHandler<A extends ThreePidConnector, B
send(connector, session.getThreePid().getAddress(), generator.getForRemoteValidation(session));
}
@Override
public void sendForFraudulentUnbind(ThreePid tpid) {
send(connector, tpid.getAddress(), generator.getForFraudulentUnbind(tpid));
}
}

View File

@@ -22,6 +22,7 @@ package io.kamax.mxisd.threepid.notification.email;
import com.sendgrid.SendGrid;
import com.sendgrid.SendGridException;
import io.kamax.matrix.ThreePid;
import io.kamax.matrix.ThreePidMedium;
import io.kamax.mxisd.as.IMatrixIdInvite;
import io.kamax.mxisd.config.MxisdConfig;
@@ -107,7 +108,7 @@ public class EmailSendGridNotificationHandler extends PlaceholderNotificationGen
@Override
public void sendForValidation(IThreePidSession session) {
EmailTemplate template = cfg.getTemplates().getSession().getLocal();
EmailTemplate template = cfg.getTemplates().getSession().getValidation().getLocal();
Email email = getEmail();
email.setSubject(populateForValidation(session, template.getSubject()));
email.setText(populateForValidation(session, getFromFile(template.getBody().getText())));
@@ -118,7 +119,7 @@ public class EmailSendGridNotificationHandler extends PlaceholderNotificationGen
@Override
public void sendForRemoteValidation(IThreePidSession session) {
EmailTemplate template = cfg.getTemplates().getSession().getLocal();
EmailTemplate template = cfg.getTemplates().getSession().getValidation().getRemote();
Email email = getEmail();
email.setSubject(populateForRemoteValidation(session, template.getSubject()));
email.setText(populateForRemoteValidation(session, getFromFile(template.getBody().getText())));
@@ -127,6 +128,17 @@ public class EmailSendGridNotificationHandler extends PlaceholderNotificationGen
send(session.getThreePid().getAddress(), email);
}
@Override
public void sendForFraudulentUnbind(ThreePid tpid) {
EmailTemplate template = cfg.getTemplates().getSession().getUnbind().getFraudulent();
Email email = getEmail();
email.setSubject(populateForCommon(tpid, template.getSubject()));
email.setText(populateForCommon(tpid, getFromFile(template.getBody().getText())));
email.setHtml(populateForCommon(tpid, getFromFile(template.getBody().getHtml())));
send(tpid.getAddress(), email);
}
private void send(String recipient, Email email) {
if (StringUtils.isBlank(cfg.getIdentity().getFrom())) {
throw new FeatureNotAvailable("3PID Email identity: sender address is empty - " +