From b95a178584cac07018f47e571f48993878da7284 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Tue, 30 Sep 2014 15:15:10 +0100 Subject: SYN-75 Verify signatures on server to server transactions --- synapse/federation/transport.py | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index afc777ec9e..5d595b7433 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -144,7 +144,7 @@ class TransportLayer(object): @defer.inlineCallbacks @log_function - def send_transaction(self, transaction, on_send_callback=None): + def send_transaction(self, transaction, json_data_callback=None): """ Sends the given Transaction to it's destination Args: @@ -163,24 +163,26 @@ class TransportLayer(object): if transaction.destination == self.server_name: raise RuntimeError("Transport layer cannot send to itself!") - data = transaction.get_dict() + if json_data_callback is None: + def json_data_callback(): + return transaction.get_dict() # FIXME (erikj): This is a bit of a hack to make the Pdu age # keys work def cb(destination, method, path_bytes, producer): - if not on_send_callback: - return + json_data = json_data_callback() + del json_data["destination"] + del json_data["transaction_id"] + producer.reset(json_data) - transaction = json.loads(producer.body) - - new_transaction = on_send_callback(transaction) - - producer.reset(new_transaction) + json_data = transaction.get_dict() + del json_data["destination"] + del json_data["transaction_id"] code, response = yield self.client.put_json( transaction.destination, path=PREFIX + "/send/%s/" % transaction.transaction_id, - data=data, + data=json_data, on_send_callback=cb, ) -- cgit 1.5.1 From 10ef8e6e4bb9d50fd2c636cfbb66d3dd6d6f94e9 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 13 Oct 2014 11:49:40 +0100 Subject: SYN-75 sign at the request level rather than the transaction level --- synapse/federation/replication.py | 13 ---------- synapse/federation/transport.py | 18 +++---------- synapse/federation/units.py | 5 ++++ synapse/http/client.py | 52 ++++++++++++++++++++++++++++++++----- tests/federation/test_federation.py | 4 +-- tests/handlers/test_presence.py | 26 +++++++++---------- tests/handlers/test_typing.py | 4 +-- 7 files changed, 70 insertions(+), 52 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/federation/replication.py b/synapse/federation/replication.py index b4235585a3..2346d55045 100644 --- a/synapse/federation/replication.py +++ b/synapse/federation/replication.py @@ -25,8 +25,6 @@ from .persistence import PduActions, TransactionActions from synapse.util.logutils import log_function -from syutil.crypto.jsonsign import sign_json - import logging @@ -66,8 +64,6 @@ class ReplicationLayer(object): hs, self.transaction_actions, transport_layer ) - self.keyring = hs.get_keyring() - self.handler = None self.edu_handlers = {} self.query_handlers = {} @@ -296,10 +292,6 @@ class ReplicationLayer(object): @defer.inlineCallbacks @log_function def on_incoming_transaction(self, transaction_data): - yield self.keyring.verify_json_for_server( - transaction_data["origin"], transaction_data - ) - transaction = Transaction(**transaction_data) for p in transaction.pdus: @@ -500,7 +492,6 @@ class _TransactionQueue(object): """ def __init__(self, hs, transaction_actions, transport_layer): - self.signing_key = hs.config.signing_key[0] self.server_name = hs.hostname self.transaction_actions = transaction_actions self.transport_layer = transport_layer @@ -615,9 +606,6 @@ class _TransactionQueue(object): # Actually send the transaction - server_name = self.server_name - signing_key = self.signing_key - # FIXME (erikj): This is a bit of a hack to make the Pdu age # keys work def json_data_cb(): @@ -627,7 +615,6 @@ class _TransactionQueue(object): for p in data["pdus"]: if "age_ts" in p: p["age"] = now - int(p["age_ts"]) - data = sign_json(data, server_name, signing_key) return data code, response = yield self.transport_layer.send_transaction( diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 1f864f5fa7..48fc9fbf5e 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -163,27 +163,15 @@ class TransportLayer(object): if transaction.destination == self.server_name: raise RuntimeError("Transport layer cannot send to itself!") - if json_data_callback is None: - def json_data_callback(): - return transaction.get_dict() - - # FIXME (erikj): This is a bit of a hack to make the Pdu age - # keys work - def cb(destination, method, path_bytes, producer): - json_data = json_data_callback() - del json_data["destination"] - del json_data["transaction_id"] - producer.reset(json_data) - + # FIXME: This is only used by the tests. The actual json sent is + # generated by the json_data_callback. json_data = transaction.get_dict() - del json_data["destination"] - del json_data["transaction_id"] code, response = yield self.client.put_json( transaction.destination, path=PREFIX + "/send/%s/" % transaction.transaction_id, data=json_data, - on_send_callback=cb, + json_data_callback=json_data_callback, ) logger.debug( diff --git a/synapse/federation/units.py b/synapse/federation/units.py index 1ca123d1bf..ecca35ac43 100644 --- a/synapse/federation/units.py +++ b/synapse/federation/units.py @@ -190,6 +190,11 @@ class Transaction(JsonEncodedObject): "destination", ] + internal_keys = [ + "transaction_id", + "destination", + ] + required_keys = [ "transaction_id", "origin", diff --git a/synapse/http/client.py b/synapse/http/client.py index 0e8fa2eb25..62fe14fa5e 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -26,6 +26,8 @@ from syutil.jsonutil import encode_canonical_json from synapse.api.errors import CodeMessageException, SynapseError +from syutil.crypto.jsonsign import sign_json + from StringIO import StringIO import json @@ -147,7 +149,7 @@ class BaseHttpClient(object): class MatrixHttpClient(BaseHttpClient): - """ Wrapper around the twisted HTTP client api. Implements + """ Wrapper around the twisted HTTP client api. Implements Attributes: agent (twisted.web.client.Agent): The twisted Agent used to send the @@ -156,8 +158,38 @@ class MatrixHttpClient(BaseHttpClient): RETRY_DNS_LOOKUP_FAILURES = "__retry_dns" + def __init__(self, hs): + self.signing_key = hs.config.signing_key[0] + self.server_name = hs.hostname + BaseHttpClient.__init__(self, hs) + + def sign_request(self, destination, method, url_bytes, headers_dict, + content=None): + request = { + "method": method, + "uri": url_bytes, + "origin": self.server_name, + "destination": destination, + } + + if content is not None: + request["content"] = content + + request = sign_json(request, self.server_name, self.signing_key) + + auth_headers = [] + + for key,sig in request["signatures"][self.server_name].items(): + auth_headers.append( + "X-Matrix origin=%s,key=\"%s\",sig=\"%s\"" % ( + self.server_name, key, sig, + ) + ) + + headers_dict["Authorization"] = auth_headers + @defer.inlineCallbacks - def put_json(self, destination, path, data, on_send_callback=None): + def put_json(self, destination, path, data={}, json_data_callback=None): """ Sends the specifed json data using PUT Args: @@ -166,6 +198,8 @@ class MatrixHttpClient(BaseHttpClient): path (str): The HTTP path. data (dict): A dict containing the data that will be used as the request body. This will be encoded as JSON. + json_data_callback (callable): A callable returning the dict to + use as the request body. Returns: Deferred: Succeeds when we get a 2xx HTTP response. The result @@ -173,13 +207,16 @@ class MatrixHttpClient(BaseHttpClient): CodeMessageException is raised. """ - if not on_send_callback: - def on_send_callback(destination, method, path_bytes, producer): - pass + if not json_data_callback: + def json_data_callback(): + return data def body_callback(method, url_bytes, headers_dict): - producer = _JsonProducer(data) - on_send_callback(destination, method, path, producer) + json_data = json_data_callback() + self.sign_request( + destination, method, url_bytes, headers_dict, json_data + ) + producer = _JsonProducer(json_data) return producer response = yield self._create_request( @@ -221,6 +258,7 @@ class MatrixHttpClient(BaseHttpClient): logger.debug("Query bytes: %s Retry DNS: %s", args, retry_on_dns_fail) def body_callback(method, url_bytes, headers_dict): + self.sign_request(destination, method, url_bytes, headers_dict) return None response = yield self._create_request( diff --git a/tests/federation/test_federation.py b/tests/federation/test_federation.py index 01f07ff36d..91edeaa4b9 100644 --- a/tests/federation/test_federation.py +++ b/tests/federation/test_federation.py @@ -186,7 +186,7 @@ class FederationTestCase(unittest.TestCase): }, ] }, - on_send_callback=ANY, + json_data_callback=ANY, ) @defer.inlineCallbacks @@ -218,7 +218,7 @@ class FederationTestCase(unittest.TestCase): } ], }, - on_send_callback=ANY, + json_data_callback=ANY, ) @defer.inlineCallbacks diff --git a/tests/handlers/test_presence.py b/tests/handlers/test_presence.py index c9406b70c4..15022b8d05 100644 --- a/tests/handlers/test_presence.py +++ b/tests/handlers/test_presence.py @@ -300,7 +300,7 @@ class PresenceInvitesTestCase(unittest.TestCase): "observed_user": "@cabbage:elsewhere", } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -329,7 +329,7 @@ class PresenceInvitesTestCase(unittest.TestCase): "observed_user": "@apple:test", } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -365,7 +365,7 @@ class PresenceInvitesTestCase(unittest.TestCase): "observed_user": "@durian:test", } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -786,7 +786,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -802,7 +802,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -928,7 +928,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -943,7 +943,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -973,7 +973,7 @@ class PresencePushTestCase(unittest.TestCase): ], } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1175,7 +1175,7 @@ class PresencePollingTestCase(unittest.TestCase): "poll": [ "@potato:remote" ], }, ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1188,7 +1188,7 @@ class PresencePollingTestCase(unittest.TestCase): "push": [ {"user_id": "@clementine:test" }], }, ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1217,7 +1217,7 @@ class PresencePollingTestCase(unittest.TestCase): "push": [ {"user_id": "@fig:test" }], }, ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1250,7 +1250,7 @@ class PresencePollingTestCase(unittest.TestCase): "unpoll": [ "@potato:remote" ], }, ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -1282,7 +1282,7 @@ class PresencePollingTestCase(unittest.TestCase): ], }, ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index ff40343b8c..064b04c217 100644 --- a/tests/handlers/test_typing.py +++ b/tests/handlers/test_typing.py @@ -175,7 +175,7 @@ class TypingNotificationsTestCase(unittest.TestCase): "typing": True, } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) @@ -226,7 +226,7 @@ class TypingNotificationsTestCase(unittest.TestCase): "typing": False, } ), - on_send_callback=ANY, + json_data_callback=ANY, ), defer.succeed((200, "OK")) ) -- cgit 1.5.1 From 66848557672977e17f0d5ba785b7567b305ccdbe Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 13 Oct 2014 14:37:46 +0100 Subject: Verify signatures for server2server requests --- synapse/federation/__init__.py | 1 + synapse/federation/transport.py | 110 ++++++++++++++++++++++++++++-------- synapse/http/client.py | 10 +++- tests/federation/test_federation.py | 1 + tests/utils.py | 3 + 5 files changed, 100 insertions(+), 25 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/federation/__init__.py b/synapse/federation/__init__.py index 1351b68fd6..0112588656 100644 --- a/synapse/federation/__init__.py +++ b/synapse/federation/__init__.py @@ -22,6 +22,7 @@ from .transport import TransportLayer def initialize_http_replication(homeserver): transport = TransportLayer( + homeserver, homeserver.hostname, server=homeserver.get_resource_for_federation(), client=homeserver.get_http_client() diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 48fc9fbf5e..7b2631fbc8 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -54,7 +54,7 @@ class TransportLayer(object): we receive data. """ - def __init__(self, server_name, server, client): + def __init__(self, homeserver, server_name, server, client): """ Args: server_name (str): Local home server host @@ -63,6 +63,7 @@ class TransportLayer(object): client (synapse.protocol.http.HttpClient): the http client used to send requests """ + self.keyring = homeserver.get_keyring() self.server_name = server_name self.server = server self.client = client @@ -195,6 +196,66 @@ class TransportLayer(object): defer.returnValue(response) + @defer.inlineCallbacks + def _authenticate_request(self, request): + json_request = { + "method": request.method, + "uri": request.uri, + "destination": self.server_name, + "signatures": {}, + } + + content = None + origin = None + + if request.method == "PUT": + #TODO: Handle other method types? other content types? + content_bytes = request.content.read() + content = json.loads(content_bytes) + json_request["content"] = content + + def parse_auth_header(header_str): + params = auth.split(" ")[1].split(",") + param_dict = dict(kv.split("=") for kv in params) + def strip_quotes(value): + if value.startswith("\""): + return value[1:-1] + else: + return value + origin = strip_quotes(param_dict["origin"]) + key = strip_quotes(param_dict["key"]) + sig = strip_quotes(param_dict["sig"]) + return (origin, key, sig) + + auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") + + if not auth_headers: + #TODO(markjh): Send a 401 response? + raise Exception("Missing auth headers") + + for auth in auth_headers: + if auth.startswith("X-Matrix"): + (origin, key, sig) = parse_auth_header(auth) + json_request["origin"] = origin + json_request["signatures"].setdefault(origin,{})[key] = sig + + from syutil.jsonutil import encode_canonical_json + logger.debug("Checking %s %s", + origin, encode_canonical_json(json_request)) + yield self.keyring.verify_json_for_server(origin, json_request) + + defer.returnValue((origin, content)) + + def _with_authentication(self, handler): + @defer.inlineCallbacks + def new_handler(request, *args, **kwargs): + (origin, content) = yield self._authenticate_request(request) + response = yield handler( + origin, content, request.args, *args, **kwargs + ) + defer.returnValue(response) + return new_handler + @log_function def register_received_handler(self, handler): """ Register a handler that will be fired when we receive data. @@ -208,7 +269,7 @@ class TransportLayer(object): self.server.register_path( "PUT", re.compile("^" + PREFIX + "/send/([^/]*)/$"), - self._on_send_request + self._with_authentication(self._on_send_request) ) @log_function @@ -226,9 +287,9 @@ class TransportLayer(object): self.server.register_path( "GET", re.compile("^" + PREFIX + "/pull/$"), - lambda request: handler.on_pull_request( - request.args["origin"][0], - request.args["v"] + self._with_authentication( + lambda origin, content, query: + handler.on_pull_request(query["origin"][0], query["v"]) ) ) @@ -237,8 +298,9 @@ class TransportLayer(object): self.server.register_path( "GET", re.compile("^" + PREFIX + "/pdu/([^/]*)/([^/]*)/$"), - lambda request, pdu_origin, pdu_id: handler.on_pdu_request( - pdu_origin, pdu_id + self._with_authentication( + lambda origin, content, query, pdu_origin, pdu_id: + handler.on_pdu_request(pdu_origin, pdu_id) ) ) @@ -246,38 +308,47 @@ class TransportLayer(object): self.server.register_path( "GET", re.compile("^" + PREFIX + "/state/([^/]*)/$"), - lambda request, context: handler.on_context_state_request( - context + self._with_authentication( + lambda origin, content, query, context: + handler.on_context_state_request(context) ) ) self.server.register_path( "GET", re.compile("^" + PREFIX + "/backfill/([^/]*)/$"), - lambda request, context: self._on_backfill_request( - context, request.args["v"], - request.args["limit"] + self._with_authentication( + lambda origin, content, query, context: + self._on_backfill_request( + context, query["v"], query["limit"] + ) ) ) self.server.register_path( "GET", re.compile("^" + PREFIX + "/context/([^/]*)/$"), - lambda request, context: handler.on_context_pdus_request(context) + self._with_authentication( + lambda origin, content, query, context: + handler.on_context_pdus_request(context) + ) ) # This is when we receive a server-server Query self.server.register_path( "GET", re.compile("^" + PREFIX + "/query/([^/]*)$"), - lambda request, query_type: handler.on_query_request( - query_type, {k: v[0] for k, v in request.args.items()} + self._with_authentication( + lambda origin, content, query, query_type: + handler.on_query_request( + query_type, {k: v[0] for k, v in query.items()} + ) ) ) @defer.inlineCallbacks @log_function - def _on_send_request(self, request, transaction_id): + def _on_send_request(self, origin, content, query, transaction_id): """ Called on PUT /send// Args: @@ -292,12 +363,7 @@ class TransportLayer(object): """ # Parse the request try: - data = request.content.read() - - l = data[:20].encode("string_escape") - logger.debug("Got data: \"%s\"", l) - - transaction_data = json.loads(data) + transaction_data = content logger.debug( "Decoded %s: %s", diff --git a/synapse/http/client.py b/synapse/http/client.py index 62fe14fa5e..9f54b74e3a 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -177,16 +177,20 @@ class MatrixHttpClient(BaseHttpClient): request = sign_json(request, self.server_name, self.signing_key) + from syutil.jsonutil import encode_canonical_json + logger.debug("Signing " + " " * 11 + "%s %s", + self.server_name, encode_canonical_json(request)) + auth_headers = [] for key,sig in request["signatures"][self.server_name].items(): - auth_headers.append( + auth_headers.append(bytes( "X-Matrix origin=%s,key=\"%s\",sig=\"%s\"" % ( self.server_name, key, sig, ) - ) + )) - headers_dict["Authorization"] = auth_headers + headers_dict[b"Authorization"] = auth_headers @defer.inlineCallbacks def put_json(self, destination, path, data={}, json_data_callback=None): diff --git a/tests/federation/test_federation.py b/tests/federation/test_federation.py index 91edeaa4b9..8d277d6612 100644 --- a/tests/federation/test_federation.py +++ b/tests/federation/test_federation.py @@ -221,6 +221,7 @@ class FederationTestCase(unittest.TestCase): json_data_callback=ANY, ) + @defer.inlineCallbacks def test_recv_edu(self): recv_observer = Mock() diff --git a/tests/utils.py b/tests/utils.py index 797818be72..83dbd4f4d3 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -76,6 +76,9 @@ class MockHttpResource(HttpServer): mock_content.configure_mock(**config) mock_request.content = mock_content + mock_request.method = http_method + mock_request.uri = path + # return the right path if the event requires it mock_request.path = path -- cgit 1.5.1 From 75e517a2da4890b55f492897599185c91ed0826c Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 13 Oct 2014 15:41:20 +0100 Subject: Remove debug logging, raise a proper SynapseError if the auth header is missing --- synapse/federation/transport.py | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 7b2631fbc8..93134ee274 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -24,6 +24,7 @@ over a different (albeit still reliable) protocol. from twisted.internet import defer from synapse.api.urls import FEDERATION_PREFIX as PREFIX +from synapse.api.errors import Codes, SynapseError from synapse.util.logutils import log_function import logging @@ -230,8 +231,9 @@ class TransportLayer(object): auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") if not auth_headers: - #TODO(markjh): Send a 401 response? - raise Exception("Missing auth headers") + raise SynapseError( + 401, "Missing Authorization headers", Codes.FORBIDDEN, + ) for auth in auth_headers: if auth.startswith("X-Matrix"): @@ -239,9 +241,6 @@ class TransportLayer(object): json_request["origin"] = origin json_request["signatures"].setdefault(origin,{})[key] = sig - from syutil.jsonutil import encode_canonical_json - logger.debug("Checking %s %s", - origin, encode_canonical_json(json_request)) yield self.keyring.verify_json_for_server(origin, json_request) defer.returnValue((origin, content)) -- cgit 1.5.1 From 25d80f35f10239b280cf374f60ccb552087fcf44 Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 13 Oct 2014 15:53:18 +0100 Subject: Raise a SynapseError if the authorisation header is missing or malformed --- synapse/federation/transport.py | 46 ++++++++++++++++++++++++----------------- tests/utils.py | 4 ++++ 2 files changed, 31 insertions(+), 19 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 93134ee274..7a4c1f6443 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -211,36 +211,44 @@ class TransportLayer(object): if request.method == "PUT": #TODO: Handle other method types? other content types? - content_bytes = request.content.read() - content = json.loads(content_bytes) - json_request["content"] = content + try: + content_bytes = request.content.read() + content = json.loads(content_bytes) + json_request["content"] = content + except: + raise SynapseError(400, "Unable to parse JSON", Codes.BAD_JSON) def parse_auth_header(header_str): - params = auth.split(" ")[1].split(",") - param_dict = dict(kv.split("=") for kv in params) - def strip_quotes(value): - if value.startswith("\""): - return value[1:-1] - else: - return value - origin = strip_quotes(param_dict["origin"]) - key = strip_quotes(param_dict["key"]) - sig = strip_quotes(param_dict["sig"]) - return (origin, key, sig) + try: + params = auth.split(" ")[1].split(",") + param_dict = dict(kv.split("=") for kv in params) + def strip_quotes(value): + if value.startswith("\""): + return value[1:-1] + else: + return value + origin = strip_quotes(param_dict["origin"]) + key = strip_quotes(param_dict["key"]) + sig = strip_quotes(param_dict["sig"]) + return (origin, key, sig) + except: + raise SynapseError( + 400, "Malformed Authorization Header", Codes.FORBIDDEN + ) auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") - if not auth_headers: - raise SynapseError( - 401, "Missing Authorization headers", Codes.FORBIDDEN, - ) - for auth in auth_headers: if auth.startswith("X-Matrix"): (origin, key, sig) = parse_auth_header(auth) json_request["origin"] = origin json_request["signatures"].setdefault(origin,{})[key] = sig + if not json_request["signatures"]: + raise SynapseError( + 401, "Missing Authorization headers", Codes.FORBIDDEN, + ) + yield self.keyring.verify_json_for_server(origin, json_request) defer.returnValue((origin, content)) diff --git a/tests/utils.py b/tests/utils.py index 83dbd4f4d3..60fd6085ac 100644 --- a/tests/utils.py +++ b/tests/utils.py @@ -79,6 +79,10 @@ class MockHttpResource(HttpServer): mock_request.method = http_method mock_request.uri = path + mock_request.requestHeaders.getRawHeaders.return_value=[ + "X-Matrix origin=test,key=,sig=" + ] + # return the right path if the event requires it mock_request.path = path -- cgit 1.5.1 From 07639c79d9536cf293c550e5849ce6b5dd82189e Mon Sep 17 00:00:00 2001 From: Mark Haines Date: Mon, 13 Oct 2014 16:39:15 +0100 Subject: Respond with more helpful error messages for unsigned requests --- synapse/api/errors.py | 1 + synapse/crypto/keyclient.py | 4 ++-- synapse/crypto/keyring.py | 33 +++++++++++++++++++++++++++++++-- synapse/federation/transport.py | 4 ++-- synapse/storage/_base.py | 11 +++++++---- synapse/storage/keys.py | 2 ++ 6 files changed, 45 insertions(+), 10 deletions(-) (limited to 'synapse/federation/transport.py') diff --git a/synapse/api/errors.py b/synapse/api/errors.py index 88175602c4..6d7d499fea 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -19,6 +19,7 @@ import logging class Codes(object): + UNAUTHORIZED = "M_UNAUTHORIZED" FORBIDDEN = "M_FORBIDDEN" BAD_JSON = "M_BAD_JSON" NOT_JSON = "M_NOT_JSON" diff --git a/synapse/crypto/keyclient.py b/synapse/crypto/keyclient.py index c26f16a038..5949ea0573 100644 --- a/synapse/crypto/keyclient.py +++ b/synapse/crypto/keyclient.py @@ -43,7 +43,7 @@ def fetch_server_key(server_name, ssl_context_factory): return except Exception as e: logger.exception(e) - raise IOError("Cannot get key for " % server_name) + raise IOError("Cannot get key for %s" % server_name) class SynapseKeyClientError(Exception): @@ -93,7 +93,7 @@ class SynapseKeyClientProtocol(HTTPClient): def on_timeout(self): logger.debug("Timeout waiting for response from %s", self.transport.getHost()) - self.on_remote_key.errback(IOError("Timeout waiting for response")) + self.remote_key.errback(IOError("Timeout waiting for response")) self.transport.abortConnection() diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index ce19c69bd5..3c85295274 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -20,6 +20,7 @@ from syutil.crypto.signing_key import ( is_signing_algorithm_supported, decode_verify_key_bytes ) from syutil.base64util import decode_base64, encode_base64 +from synapse.api.errors import SynapseError, Codes from OpenSSL import crypto @@ -38,8 +39,36 @@ class Keyring(object): @defer.inlineCallbacks def verify_json_for_server(self, server_name, json_object): key_ids = signature_ids(json_object, server_name) - verify_key = yield self.get_server_verify_key(server_name, key_ids) - verify_signed_json(json_object, server_name, verify_key) + if not key_ids: + raise SynapseError( + 400, + "No supported algorithms in signing keys", + Codes.UNAUTHORIZED, + ) + try: + verify_key = yield self.get_server_verify_key(server_name, key_ids) + except IOError: + raise SynapseError( + 502, + "Error downloading keys for %s" % (server_name,), + Codes.UNAUTHORIZED, + ) + except: + raise SynapseError( + 401, + "No key for %s with id %s" % (server_name, key_ids), + Codes.UNAUTHORIZED, + ) + try: + verify_signed_json(json_object, server_name, verify_key) + except: + raise SynapseError( + 401, + "Invalid signature for server %s with key %s:%s" % ( + server_name, verify_key.alg, verify_key.version + ), + Codes.UNAUTHORIZED, + ) @defer.inlineCallbacks def get_server_verify_key(self, server_name, key_ids): diff --git a/synapse/federation/transport.py b/synapse/federation/transport.py index 7a4c1f6443..755eee8cf6 100644 --- a/synapse/federation/transport.py +++ b/synapse/federation/transport.py @@ -233,7 +233,7 @@ class TransportLayer(object): return (origin, key, sig) except: raise SynapseError( - 400, "Malformed Authorization Header", Codes.FORBIDDEN + 400, "Malformed Authorization header", Codes.UNAUTHORIZED ) auth_headers = request.requestHeaders.getRawHeaders(b"Authorization") @@ -246,7 +246,7 @@ class TransportLayer(object): if not json_request["signatures"]: raise SynapseError( - 401, "Missing Authorization headers", Codes.FORBIDDEN, + 401, "Missing Authorization headers", Codes.UNAUTHORIZED, ) yield self.keyring.verify_json_for_server(origin, json_request) diff --git a/synapse/storage/_base.py b/synapse/storage/_base.py index 889de2bedc..dba50f1213 100644 --- a/synapse/storage/_base.py +++ b/synapse/storage/_base.py @@ -121,7 +121,7 @@ class SQLBaseStore(object): # "Simple" SQL API methods that operate on a single table with no JOINs, # no complex WHERE clauses, just a dict of values for columns. - def _simple_insert(self, table, values, or_replace=False): + def _simple_insert(self, table, values, or_replace=False, or_ignore=False): """Executes an INSERT query on the named table. Args: @@ -130,13 +130,16 @@ class SQLBaseStore(object): or_replace : bool; if True performs an INSERT OR REPLACE """ return self.runInteraction( - self._simple_insert_txn, table, values, or_replace=or_replace + self._simple_insert_txn, table, values, or_replace=or_replace, + or_ignore=or_ignore, ) @log_function - def _simple_insert_txn(self, txn, table, values, or_replace=False): + def _simple_insert_txn(self, txn, table, values, or_replace=False, + or_ignore=False): sql = "%s INTO %s (%s) VALUES(%s)" % ( - ("INSERT OR REPLACE" if or_replace else "INSERT"), + ("INSERT OR REPLACE" if or_replace else + "INSERT OR IGNORE" if or_ignore else "INSERT"), table, ", ".join(k for k in values), ", ".join("?" for k in values) diff --git a/synapse/storage/keys.py b/synapse/storage/keys.py index 253dc17be2..8189e071a3 100644 --- a/synapse/storage/keys.py +++ b/synapse/storage/keys.py @@ -65,6 +65,7 @@ class KeyStore(SQLBaseStore): "ts_added_ms": time_now_ms, "tls_certificate": buffer(tls_certificate_bytes), }, + or_ignore=True, ) @defer.inlineCallbacks @@ -113,4 +114,5 @@ class KeyStore(SQLBaseStore): "ts_added_ms": time_now_ms, "verify_key": buffer(verify_key.encode()), }, + or_ignore=True, ) -- cgit 1.5.1