Respond with more helpful error messages for unsigned requests

6 files changed, 45 insertions, 10 deletions

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