From 9b7433055de93b89046c921a01d001c701095848 Mon Sep 17 00:00:00 2001 From: Marcus Hoffmann Date: Mon, 15 Nov 2021 21:44:58 +0100 Subject: [PATCH 1/7] MSC3383: include destination in X-Matrix auth header MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit https://github.com/matrix-org/matrix-doc/pull/3383 Co-Authored-By: Jan Christian Grünhage Signed-off-by: Jan Christian Grünhage Signed-off-by: Marcus Hoffmann --- changelog.d/11398.feature | 1 + scripts-dev/federation_client.py | 7 ++++++- synapse/federation/transport/server/_base.py | 18 +++++++++++++++--- synapse/http/matrixfederationclient.py | 12 ++++++++++-- 4 files changed, 32 insertions(+), 6 deletions(-) create mode 100644 changelog.d/11398.feature diff --git a/changelog.d/11398.feature b/changelog.d/11398.feature new file mode 100644 index 000000000000..5a37c841a1f3 --- /dev/null +++ b/changelog.d/11398.feature @@ -0,0 +1 @@ +Implement [MSC3383](https://github.com/matrix-org/matrix-doc/pull/3383) for including the destination in server-to-server authentication headers. Contributed by @Bubu and @jcgruenhage for Famedly GmbH. diff --git a/scripts-dev/federation_client.py b/scripts-dev/federation_client.py index 6f76c08fcff2..230de8830ec3 100755 --- a/scripts-dev/federation_client.py +++ b/scripts-dev/federation_client.py @@ -105,7 +105,12 @@ def request( authorization_headers = [] for key, sig in signed_json["signatures"][origin_name].items(): - header = 'X-Matrix origin=%s,key="%s",sig="%s"' % (origin_name, key, sig) + header = 'X-Matrix origin=%s,key="%s",sig="%s",destination="%s"' % ( + origin_name, + key, + sig, + destination, + ) authorization_headers.append(header.encode("ascii")) print("Authorization: %s" % header, file=sys.stderr) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index cef65929c529..dcdefab0ed58 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -82,10 +82,15 @@ async def authenticate_request(self, request, content): for auth in auth_headers: if auth.startswith(b"X-Matrix"): - (origin, key, sig) = _parse_auth_header(auth) + (origin, key, sig, destination) = _parse_auth_header(auth) json_request["origin"] = origin json_request["signatures"].setdefault(origin, {})[key] = sig + # if the origin_server sent a destination along it needs to match our own server_name + if destination is not None and destination != self.server_name: + raise AuthenticationError( + 400, "Destination mismatch in auth header", Codes.UNAUTHORIZED + ) if ( self.federation_domain_whitelist is not None and origin not in self.federation_domain_whitelist @@ -140,7 +145,7 @@ def _parse_auth_header(header_bytes): header_bytes (bytes): header value Returns: - Tuple[str, str, str]: origin, key id, signature. + Tuple[str, str, str, Optional[str]]: origin, key id, signature, destination. Raises: AuthenticationError if the header could not be parsed @@ -163,7 +168,14 @@ def strip_quotes(value): key = strip_quotes(param_dict["key"]) sig = strip_quotes(param_dict["sig"]) - return origin, key, sig + + # get the destination server_name from the auth header if it exists + if param_dict.get("destination") is not None: + destination = strip_quotes(param_dict.get("destination")) + else: + destination = None + + return origin, key, sig, destination except Exception as e: logger.warning( "Error parsing auth header '%s': %s", diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 203d723d4120..205c9bc77ead 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -715,6 +715,9 @@ def build_auth_headers( Returns: A list of headers to be added as "Authorization:" headers """ + if destination is None and destination_is is None: + raise ValueError("destination and destination_is cannot both be None!") + request: JsonDict = { "method": method.decode("ascii"), "uri": url_bytes.decode("ascii"), @@ -737,8 +740,13 @@ def build_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) + 'X-Matrix origin=%s,key="%s",sig="%s",destination="%s"' + % ( + self.server_name, + key, + sig, + request.get("destination") or request["destination_is"], + ) ).encode("ascii") ) return auth_headers From 2cb31e82a8038ab485503745055c2e41b43c3f1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Mon, 22 Nov 2021 13:56:36 +0100 Subject: [PATCH 2/7] use HTTPStatus constants in federation base transport --- synapse/federation/transport/server/_base.py | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index dcdefab0ed58..1f5b81f1a889 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -15,6 +15,7 @@ import functools import logging import re +from http import HTTPStatus from synapse.api.errors import Codes, FederationDeniedError, SynapseError from synapse.api.urls import FEDERATION_V1_PREFIX @@ -77,7 +78,9 @@ async def authenticate_request(self, request, content): if not auth_headers: raise NoAuthenticationError( - 401, "Missing Authorization headers", Codes.UNAUTHORIZED + HTTPStatus.UNAUTHORIZED, + "Missing Authorization headers", + Codes.UNAUTHORIZED, ) for auth in auth_headers: @@ -89,7 +92,9 @@ async def authenticate_request(self, request, content): # if the origin_server sent a destination along it needs to match our own server_name if destination is not None and destination != self.server_name: raise AuthenticationError( - 400, "Destination mismatch in auth header", Codes.UNAUTHORIZED + HTTPStatus.BAD_REQUEST, + "Destination mismatch in auth header", + Codes.UNAUTHORIZED, ) if ( self.federation_domain_whitelist is not None @@ -99,7 +104,9 @@ async def authenticate_request(self, request, content): if origin is None or not json_request["signatures"]: raise NoAuthenticationError( - 401, "Missing Authorization headers", Codes.UNAUTHORIZED + HTTPStatus.UNAUTHORIZED, + "Missing Authorization headers", + Codes.UNAUTHORIZED, ) await self.keyring.verify_json_for_server( @@ -183,7 +190,7 @@ def strip_quotes(value): e, ) raise AuthenticationError( - 400, "Malformed Authorization header", Codes.UNAUTHORIZED + HTTPStatus.BAD_REQUEST, "Malformed Authorization header", Codes.UNAUTHORIZED ) From 8c8d84eae0c2992eefebe365c8d3698a3ebec289 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Mon, 22 Nov 2021 14:00:48 +0100 Subject: [PATCH 3/7] set type hints for auth header parsing method --- synapse/federation/transport/server/_base.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index 1f5b81f1a889..9f0ef43abb1b 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -16,6 +16,7 @@ import logging import re from http import HTTPStatus +from typing import Optional, Tuple from synapse.api.errors import Codes, FederationDeniedError, SynapseError from synapse.api.urls import FEDERATION_V1_PREFIX @@ -145,14 +146,14 @@ async def _reset_retry_timings(self, origin): logger.exception("Error resetting retry timings on %s", origin) -def _parse_auth_header(header_bytes): +def _parse_auth_header(header_bytes: bytes) -> Tuple[str, str, str, Optional[str]]: """Parse an X-Matrix auth header Args: - header_bytes (bytes): header value + header_bytes: header value Returns: - Tuple[str, str, str, Optional[str]]: origin, key id, signature, destination. + origin, key id, signature, destination. Raises: AuthenticationError if the header could not be parsed From 69f606cde47528f56468c4179846d0d830643c70 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Sat, 27 Nov 2021 12:14:39 +0100 Subject: [PATCH 4/7] add type annotations to federation auth header parsing --- synapse/federation/transport/server/_base.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index 9f0ef43abb1b..d3041b403232 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -16,7 +16,7 @@ import logging import re from http import HTTPStatus -from typing import Optional, Tuple +from typing import Dict, Optional, Tuple from synapse.api.errors import Codes, FederationDeniedError, SynapseError from synapse.api.urls import FEDERATION_V1_PREFIX @@ -161,7 +161,9 @@ 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(",") - param_dict = dict(kv.split("=") for kv in params) + param_dict: Dict[str, str] = { + k: v for k, v in [param.split("=", maxsplit=1) for param in params] + } def strip_quotes(value): if value.startswith('"'): From ad2d91c04ed193a3a1c9a14693c621671d779f84 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Tue, 19 Apr 2022 15:40:52 +0200 Subject: [PATCH 5/7] avoid redirect in changelog item --- changelog.d/11398.feature | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/changelog.d/11398.feature b/changelog.d/11398.feature index 5a37c841a1f3..a910f4da1496 100644 --- a/changelog.d/11398.feature +++ b/changelog.d/11398.feature @@ -1 +1 @@ -Implement [MSC3383](https://github.com/matrix-org/matrix-doc/pull/3383) for including the destination in server-to-server authentication headers. Contributed by @Bubu and @jcgruenhage for Famedly GmbH. +Implement [MSC3383](https://github.com/matrix-org/matrix-spec-proposals/pull/3383) for including the destination in server-to-server authentication headers. Contributed by @Bubu and @jcgruenhage for Famedly GmbH. From a807f8a8d765055ed71908d5f7f4437fd7edf32a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Tue, 19 Apr 2022 15:41:33 +0200 Subject: [PATCH 6/7] address note about error code mentioned in the MSC, for when the expected destination and actual destination don't match up --- synapse/federation/transport/server/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index ceec3a5bb7ae..cbccf4573358 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -101,7 +101,7 @@ async def authenticate_request( # if the origin_server sent a destination along it needs to match our own server_name if destination is not None and destination != self.server_name: raise AuthenticationError( - HTTPStatus.BAD_REQUEST, + HTTPStatus.UNAUTHORIZED, "Destination mismatch in auth header", Codes.UNAUTHORIZED, ) From 1d73c70e52649fa514d27e319fc1302d4bbf3435 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20Christian=20Gr=C3=BCnhage?= Date: Tue, 19 Apr 2022 16:42:24 +0200 Subject: [PATCH 7/7] fix linting issues --- synapse/federation/transport/server/_base.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/federation/transport/server/_base.py b/synapse/federation/transport/server/_base.py index cbccf4573358..d629a3ecb5dd 100644 --- a/synapse/federation/transport/server/_base.py +++ b/synapse/federation/transport/server/_base.py @@ -15,9 +15,9 @@ import functools import logging import re -from http import HTTPStatus import time -from typing import TYPE_CHECKING, Any, Awaitable, Callable, Optional, Tuple, cast +from http import HTTPStatus +from typing import TYPE_CHECKING, Any, Awaitable, Callable, Dict, Optional, Tuple, cast from synapse.api.errors import Codes, FederationDeniedError, SynapseError from synapse.api.urls import FEDERATION_V1_PREFIX @@ -189,8 +189,9 @@ def strip_quotes(value: str) -> str: sig = strip_quotes(param_dict["sig"]) # get the destination server_name from the auth header if it exists - if param_dict.get("destination") is not None: - destination = strip_quotes(param_dict.get("destination")) + destination = param_dict.get("destination") + if destination is not None: + destination = strip_quotes(destination) else: destination = None