From 8afb7b55d0527f8c6af7690b162ebaabe9b5d9f5 Mon Sep 17 00:00:00 2001
From: Hubert Chathi <hubertc@matrix.org>
Date: Wed, 18 May 2022 06:19:30 -0400
Subject: [PATCH] Make handling of federation Authorization header (more)
 compliant with RFC7230 (#12774)

The main differences are:
- values with delimiters (such as colons) should be quoted, so always
  quote the origin, since it could contain a colon followed by a port
  number
- should allow more than one space after "X-Matrix"
- quoted values with backslash-escaped characters should be unescaped
- names should be case insensitive
---
 changelog.d/12774.misc                        |  1 +
 synapse/federation/transport/server/_base.py  |  8 +++--
 synapse/http/matrixfederationclient.py        |  2 +-
 .../federation/transport/server/test__base.py | 29 ++++++++++++++++++-
 4 files changed, 35 insertions(+), 5 deletions(-)
 create mode 100644 changelog.d/12774.misc

diff --git a/changelog.d/12774.misc b/changelog.d/12774.misc
new file mode 100644
index 000000000..8651f2e0e
--- /dev/null
+++ b/changelog.d/12774.misc
@@ -0,0 +1 @@
+Make handling of federation Authorization header (more) compliant with RFC7230.
diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py
index 103861644..84100a5a5 100644
--- a/synapse/federation/transport/server/_base.py
+++ b/synapse/federation/transport/server/_base.py
@@ -169,14 +169,16 @@ def _parse_auth_header(header_bytes: bytes) -> Tuple[str, str, str, Optional[str
     """
     try:
         header_str = header_bytes.decode("utf-8")
-        params = header_str.split(" ")[1].split(",")
+        params = re.split(" +", header_str)[1].split(",")
         param_dict: Dict[str, str] = {
-            k: v for k, v in [param.split("=", maxsplit=1) for param in params]
+            k.lower(): v for k, v in [param.split("=", maxsplit=1) for param in params]
         }
 
         def strip_quotes(value: str) -> str:
             if value.startswith('"'):
-                return value[1:-1]
+                return re.sub(
+                    "\\\\(.)", lambda matchobj: matchobj.group(1), value[1:-1]
+                )
             else:
                 return value
 
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index 725b5c33b..0b9475deb 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -747,7 +747,7 @@ class MatrixFederationHttpClient:
         for key, sig in request["signatures"][self.server_name].items():
             auth_headers.append(
                 (
-                    'X-Matrix origin=%s,key="%s",sig="%s",destination="%s"'
+                    'X-Matrix origin="%s",key="%s",sig="%s",destination="%s"'
                     % (
                         self.server_name,
                         key,
diff --git a/tests/federation/transport/server/test__base.py b/tests/federation/transport/server/test__base.py
index ac3695a8c..e63885c1c 100644
--- a/tests/federation/transport/server/test__base.py
+++ b/tests/federation/transport/server/test__base.py
@@ -17,7 +17,7 @@ from typing import Dict, List, Tuple
 
 from synapse.api.errors import Codes
 from synapse.federation.transport.server import BaseFederationServlet
-from synapse.federation.transport.server._base import Authenticator
+from synapse.federation.transport.server._base import Authenticator, _parse_auth_header
 from synapse.http.server import JsonResource, cancellable
 from synapse.server import HomeServer
 from synapse.types import JsonDict
@@ -112,3 +112,30 @@ class BaseFederationServletCancellationTests(
             expect_cancellation=False,
             expected_body={"result": True},
         )
+
+
+class BaseFederationAuthorizationTests(unittest.TestCase):
+    def test_authorization_header(self) -> None:
+        """Tests that the Authorization header is parsed correctly."""
+
+        # test a "normal" Authorization header
+        self.assertEqual(
+            _parse_auth_header(
+                b'X-Matrix origin=foo,key="ed25519:1",sig="sig",destination="bar"'
+            ),
+            ("foo", "ed25519:1", "sig", "bar"),
+        )
+        # test an Authorization with extra spaces, upper-case names, and escaped
+        # characters
+        self.assertEqual(
+            _parse_auth_header(
+                b'X-Matrix  ORIGIN=foo,KEY="ed25\\519:1",SIG="sig",destination="bar"'
+            ),
+            ("foo", "ed25519:1", "sig", "bar"),
+        )
+        self.assertEqual(
+            _parse_auth_header(
+                b'X-Matrix origin=foo,key="ed25519:1",sig="sig",destination="bar",extra_field=ignored'
+            ),
+            ("foo", "ed25519:1", "sig", "bar"),
+        )