Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support HS unbinding 3pids #67

Merged
merged 27 commits into from
Jun 8, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions sydent/db/threepid_associations.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
from sydent.threepid import ThreepidAssociation, threePidAssocFromDict

import json
import logging


logger = logging.getLogger(__name__)


class LocalAssociationStore:
Expand Down Expand Up @@ -60,6 +64,34 @@ def getAssociationsAfterId(self, afterId, limit):

return (assocs, maxId)

def removeAssociation(self, threepid, mxid):
cur = self.sydent.db.cursor()

cur.execute(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we bother with this, ooi?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it's a REPLACE INTO, so we don't need to insert an empty record if there's nothing there to start with

"SELECT COUNT(*) FROM local_threepid_associations "
"WHERE medium = ? AND address = ? AND mxid = ?",
(threepid['medium'], threepid['address'], mxid)
)
row = cur.fetchone()
if row[0] > 0:
ts = time_msec()
cur.execute(
"REPLACE INTO local_threepid_associations "
"('medium', 'address', 'mxid', 'ts', 'notBefore', 'notAfter') "
" values (?, ?, NULL, ?, null, null)",
(threepid['medium'], threepid['address'], ts),
)
logger.info(
"Deleting local assoc for %s/%s/%s replaced %d rows",
threepid['medium'], threepid['address'], mxid, cur.rowcount,
)
self.sydent.db.commit()
else:
logger.info(
"No local assoc found for %s/%s/%s",
threepid['medium'], threepid['address'], mxid,
)


class GlobalAssociationStore:
def __init__(self, sydent):
Expand Down Expand Up @@ -164,3 +196,16 @@ def lastIdFromServer(self, server):
return None

return row[0]

def removeAssociation(self, medium, address):
cur = self.sydent.db.cursor()
cur.execute(
"DELETE FROM global_threepid_associations WHERE "
"medium = ? AND address = ?",
(medium, address),
)
logger.info(
"Deleted %d rows from global associations for %s/%s",
cur.rowcount, medium, address,
)
self.sydent.db.commit()
12 changes: 6 additions & 6 deletions sydent/db/threepid_associations.sql
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

CREATE TABLE IF NOT EXISTS local_threepid_associations (id integer primary key, medium varchar(16) not null, address varchar(256) not null, mxid varchar(256) not null, ts integer not null, notBefore bigint not null, notAfter bigint not null);
CREATE UNIQUE INDEX IF NOT EXISTS medium_address on local_threepid_associations(medium, address);
CREATE TABLE IF NOT EXISTS local_threepid_associations (id integer primary key autoincrement, medium varchar(16) not null, address varchar(256) not null, mxid varchar(256) not null, ts integer, notBefore bigint, notAfter bigint);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you write a comment which explains what it means for the nullable fields to be null?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer in this schema file but I've commented it in the upgrade script

CREATE UNIQUE INDEX IF NOT EXISTS local_threepid_medium_address on local_threepid_associations(medium, address);

CREATE TABLE IF NOT EXISTS global_threepid_associations (id integer primary key, medium varchar(16) not null, address varchar(256) not null, mxid varchar(256) not null, ts integer not null, notBefore bigint not null, notAfter integer not null, originServer varchar(255) not null, originId integer not null, sgAssoc text not null);
CREATE INDEX IF NOT EXISTS medium_address on global_threepid_associations (medium, address);
CREATE INDEX IF NOT EXISTS medium_lower_address on global_threepid_associations (medium, lower(address));
CREATE UNIQUE INDEX IF NOT EXISTS originServer_originId on global_threepid_associations (originServer, originId);
CREATE TABLE IF NOT EXISTS global_threepid_associations (id integer primary key autoincrement, medium varchar(16) not null, address varchar(256) not null, mxid varchar(256) not null, ts integer not null, notBefore bigint not null, notAfter integer not null, originServer varchar(255) not null, originId integer not null, sgAssoc text not null);
CREATE INDEX IF NOT EXISTS global_threepid_medium_address on global_threepid_associations (medium, address);
CREATE INDEX IF NOT EXISTS global_threepid_medium_lower_address on global_threepid_associations (medium, lower(address));
CREATE UNIQUE INDEX IF NOT EXISTS global_threepid_originServer_originId on global_threepid_associations (originServer, originId);
109 changes: 109 additions & 0 deletions sydent/db/version.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,109 @@
# -*- coding: utf-8 -*-

# Copyright 2018 New Vector Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


import logging

logger = logging.getLogger(__name__)


class VersionStore:
def __init__(self, sydent):
self.sydent = sydent

def upgradeSchema(self):
curVer = self._getSchemaVersion()

if curVer < 1:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we do something to make new installations have schema v1 ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this wasn't thought through very well. Hopefully this is more sensible now.

cur = self.sydent.db.cursor()

# add auto_increment to the primary key of local_threepid_associations to ensure ids are never re-used,
# allow the mxid column to be null to represent the deletion of a binding
# and remove not null constraints on ts, notBefore and notAfter
logger.info("Migrating schema from v0 to v1")
cur.execute("DROP INDEX IF EXISTS medium_address")
cur.execute("DROP INDEX IF EXISTS local_threepid_medium_address")
cur.execute("ALTER TABLE local_threepid_associations RENAME TO old_local_threepid_associations");
cur.execute(
"CREATE TABLE local_threepid_associations (id integer primary key autoincrement, "
"medium varchar(16) not null, "
"address varchar(256) not null, "
"mxid varchar(256), "
"ts integer, "
"notBefore bigint, "
"notAfter bigint)"
)
cur.execute(
"INSERT INTO local_threepid_associations (medium, address, mxid, ts, notBefore, notAfter) "
"SELECT medium, address, mxid, ts, notBefore, notAfter FROM old_local_threepid_associations"
)
cur.execute(
"CREATE UNIQUE INDEX local_threepid_medium_address on local_threepid_associations(medium, address)"
)
cur.execute("DROP TABLE old_local_threepid_associations")

# same for global_threepid_associations
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, except we're keeping the not null constraints.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good point: fixed

cur.execute("DROP INDEX IF EXISTS global_threepid_medium_address")
cur.execute("DROP INDEX IF EXISTS global_threepid_medium_lower_address")
cur.execute("DROP INDEX IF EXISTS global_threepid_originServer_originId")
cur.execute("DROP INDEX IF EXISTS medium_lower_address")
cur.execute("DROP INDEX IF EXISTS threepid_originServer_originId")
cur.execute("ALTER TABLE global_threepid_associations RENAME TO old_global_threepid_associations");
cur.execute(
"CREATE TABLE IF NOT EXISTS global_threepid_associations "
"(id integer primary key autoincrement, "
"medium varchar(16) not null, "
"address varchar(256) not null, "
"mxid varchar(256) not null, "
"ts integer not null, "
"notBefore bigint not null, "
"notAfter integer not null, "
"originServer varchar(255) not null, "
"originId integer not null, "
"sgAssoc text not null)"
)
cur.execute(
"INSERT INTO global_threepid_associations "
"(medium, address, mxid, ts, notBefore, notAfter, originServer, originId, sgAssoc) "
"SELECT medium, address, mxid, ts, notBefore, notAfter, originServer, originId, sgAssoc "
"FROM old_global_threepid_associations"
)
cur.execute("CREATE INDEX global_threepid_medium_address on global_threepid_associations (medium, address)")
cur.execute(
"CREATE INDEX global_threepid_medium_lower_address on "
"global_threepid_associations (medium, lower(address))"
)
cur.execute(
"CREATE UNIQUE INDEX global_threepid_originServer_originId on "
"global_threepid_associations (originServer, originId)"
)
cur.execute("DROP TABLE old_global_threepid_associations")
self.sydent.db.commit()
logger.info("v0 -> v1 schema migration complete")
self._setSchemaVersion(1)

def _getSchemaVersion(self):
cur = self.sydent.db.cursor()
res = cur.execute("PRAGMA user_version");
row = cur.fetchone()
return row[0]

def _setSchemaVersion(self, ver):
cur = self.sydent.db.cursor()
# NB. pragma doesn't support variable substitution so we
# do it in python (as a decimal so we don't risk SQL injection)
res = cur.execute("PRAGMA user_version = %d" % (ver,));

58 changes: 49 additions & 9 deletions sydent/http/httpclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
import logging

from StringIO import StringIO
from twisted.internet import defer, reactor
from twisted.web.client import FileBodyProducer, Agent
from twisted.internet import defer, reactor, ssl
from twisted.internet._sslverify import _OpenSSLECCurve, _defaultCurveName, ClientTLSOptions
from twisted.web.client import FileBodyProducer, Agent, readBody
from twisted.web.http_headers import Headers
from twisted.web.iweb import IPolicyForHTTPS
from zope.interface import implementer
from OpenSSL import SSL

logger = logging.getLogger(__name__)

Expand All @@ -29,15 +33,33 @@ class SimpleHttpClient(object):
A simple, no-frills HTTP client based on the class of the same name
from synapse
"""
def __init__(self, sydent):
def __init__(self, sydent, context_factory=None):
self.sydent = sydent
# The default context factory in Twisted 14.0.0 (which we require) is
# BrowserLikePolicyForHTTPS which will do regular cert validation
# 'like a browser'
self.agent = Agent(
reactor,
connectTimeout=15,
if context_factory is None:
# The default context factory in Twisted 14.0.0 (which we require) is
# BrowserLikePolicyForHTTPS which will do regular cert validation
# 'like a browser'
self.agent = Agent(
reactor,
connectTimeout=15,
)
else:
self.agent = Agent(
reactor,
connectTimeout=15,
contextFactory=context_factory
)

@defer.inlineCallbacks
def get_json(self, uri):
logger.debug("HTTP GET %s", uri)

response = yield self.agent.request(
"GET",
uri.encode("ascii"),
)
body = yield readBody(response)
defer.returnValue(json.loads(body))

@defer.inlineCallbacks
def post_json_get_nothing(self, uri, post_json, opts):
Expand All @@ -57,3 +79,21 @@ def post_json_get_nothing(self, uri, post_json, opts):
)
defer.returnValue(response)

@implementer(IPolicyForHTTPS)
class FederationPolicyForHTTPS(object):
def creatorForNetloc(self, hostname, port):
context = SSL.Context(SSL.SSLv23_METHOD)
try:
_ecCurve = _OpenSSLECCurve(_defaultCurveName)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you do matrix-org/synapse#3157 to this please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

_ecCurve.addECKeyToContext(context)
except Exception:
logger.exception("Failed to enable elliptic curve for TLS")
context.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3)

context.set_cipher_list("!ADH:HIGH+kEDH:!AECDH:HIGH+kEECDH")
return ClientTLSOptions(hostname, context)


class FederationHttpClient(SimpleHttpClient):
def __init__(self, sydent):
super(FederationHttpClient, self).__init__(sydent, FederationPolicyForHTTPS())
3 changes: 3 additions & 0 deletions sydent/http/httpserver.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
# -*- coding: utf-8 -*-

# Copyright 2014 OpenMarket Ltd
# Copyright 2018 New Vector Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand Down Expand Up @@ -48,6 +49,7 @@ def __init__(self, sydent):

threepid = Resource()
bind = self.sydent.servlets.threepidBind
unbind = self.sydent.servlets.threepidUnbind

pubkey = Resource()
ephemeralPubkey = Resource()
Expand All @@ -74,6 +76,7 @@ def __init__(self, sydent):

v1.putChild('3pid', threepid)
threepid.putChild('bind', bind)
threepid.putChild('unbind', unbind)
threepid.putChild('getValidated3pid', getValidated3pid)

email.putChild('requestToken', emailReqCode)
Expand Down
6 changes: 5 additions & 1 deletion sydent/http/servlets/replication.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,11 @@ def render_POST(self, request):

assocObj = threePidAssocFromDict(sgAssoc)

globalAssocsStore.addAssociation(assocObj, json.dumps(sgAssoc), peer.servername, originId, commit=False)
if assocObj.mxid is not None:
globalAssocsStore.addAssociation(assocObj, json.dumps(sgAssoc), peer.servername, originId, commit=False)
else:
logger.info("Incoming deletion: removing associations for %s / %s", assocObj.medium, assocObj.address)
globalAssocsStore.removeAssociation(assocObj.medium, assocObj.address)
logger.info("Stored association origin ID %s from %s", originId, peer.servername)
except:
failedIds.append(originId)
Expand Down
83 changes: 83 additions & 0 deletions sydent/http/servlets/threepidunbindservlet.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
# -*- coding: utf-8 -*-

# Copyright 2014 OpenMarket Ltd
# Copyright 2018 New Vector Ltd
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

import json

from twisted.web.resource import Resource
from twisted.web import server
from twisted.internet import defer

from sydent.http.servlets import get_args, jsonwrap
from sydent.hs_federation.verifier import NoAuthenticationError
from signedjson.sign import SignatureVerifyException
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before twisted, please

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


class ThreePidUnbindServlet(Resource):
def __init__(self, sydent):
self.sydent = sydent

def render_POST(self, request):
self._async_render_POST(request)
return server.NOT_DONE_YET

@defer.inlineCallbacks
def _async_render_POST(self, request):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does this not need a generic Exception handler?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, true

try:
body = json.load(request.content)
except ValueError:
request.setResponseCode(400)
request.write(json.dumps({'errcode': 'M_BAD_JSON', 'error': 'Malformed JSON'}))
request.finish()
return

missing = [k for k in ("threepid", "mxid") if k not in body]
if len(missing) > 0:
request.setResponseCode(400)
msg = "Missing parameters: "+(",".join(missing))
request.write(json.dumps({'errcode': 'M_MISSING_PARAMS', 'error': msg}))
request.finish()
return

threepid = body['threepid']
mxid = body['mxid']

if 'medium' not in threepid or 'address' not in threepid:
request.setResponseCode(400)
request.write(json.dumps({'errcode': 'M_MISSING_PARAMS', 'error': 'Threepid lacks medium / address'}))
request.finish()
return

try:
origin_server_name = yield self.sydent.sig_verifier.authenticate_request(request, body)
except SignatureVerifyException as ex:
request.setResponseCode(401)
request.write(json.dumps({'errcode': 'M_FORBIDDEN', 'error': ex.message}))
request.finish()
return
except NoAuthenticationError as ex:
request.setResponseCode(401)
request.write(json.dumps({'errcode': 'M_FORBIDDEN', 'error': ex.message}))
request.finish()
return

if not mxid.endswith(':' + origin_server_name):
request.setResponseCode(403)
request.write(json.dumps({'errcode': 'M_FORBIDDEN', 'error': 'Origin server name does not match mxid'}))
request.finish()

res = self.sydent.threepidBinder.removeBinding(threepid, mxid)
request.write(json.dumps({}))
request.finish()
Loading