SYN-75 sign at the request level rather than the transaction level

7 files changed, 70 insertions, 52 deletions

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"))
         )