From 3f0f28bca95f95e227ed9b7aff27e48a445564fd Mon Sep 17 00:00:00 2001 From: magic_rb Date: Mon, 16 Oct 2023 17:29:45 +0200 Subject: [PATCH 1/6] Fix http/s proxy authentication Signed-off-by: magic_rb --- changelog.d/16504.bugfix | 1 + synapse/http/connectproxyclient.py | 3 +-- 2 files changed, 2 insertions(+), 2 deletions(-) create mode 100644 changelog.d/16504.bugfix diff --git a/changelog.d/16504.bugfix b/changelog.d/16504.bugfix new file mode 100644 index 000000000000..60839c474b29 --- /dev/null +++ b/changelog.d/16504.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.41 where HTTP(S) forward proxy authorization would fail when using basic HTTP authentication with a long `username:password` string. diff --git a/synapse/http/connectproxyclient.py b/synapse/http/connectproxyclient.py index 636efc33e8f3..5bedca922505 100644 --- a/synapse/http/connectproxyclient.py +++ b/synapse/http/connectproxyclient.py @@ -59,8 +59,7 @@ def as_proxy_authorization_value(self) -> bytes: a Proxy-Authorization header. """ # Encode as base64 and prepend the authorization type - return b"Basic " + base64.encodebytes(self.username_password) - + return b"Basic " + base64.b64encode(self.username_password) @attr.s(auto_attribs=True) class BearerProxyCredentials(ProxyCredentials): From 47861e9a141b7bb098860321e1507e90145339cd Mon Sep 17 00:00:00 2001 From: David Robertson Date: Mon, 23 Oct 2023 19:21:23 +0100 Subject: [PATCH 2/6] Add test case to detect dodgy b64 encoding --- tests/http/test_proxyagent.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py index 8164b0b78e31..b48c2c293a35 100644 --- a/tests/http/test_proxyagent.py +++ b/tests/http/test_proxyagent.py @@ -217,6 +217,20 @@ def test_parse_proxy( ) +class TestBasicProxyCredentials(TestCase): + def test_long_user_pass_string_encoded_without_newlines(self) -> None: + """Reproduces https://github.com/matrix-org/synapse/pull/16504.""" + creds = BasicProxyCredentials( + b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass@proxy.local:9988" + ) + auth_value = creds.as_proxy_authorization_value() + self.assertNotIn(b"\n", auth_value) + self.assertEqual( + creds.as_proxy_authorization_value(), + b"Basic: bG9vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vbmd1c2VyOnBhc3M=", + ) + + class MatrixFederationAgentTests(TestCase): def setUp(self) -> None: self.reactor = ThreadedMemoryReactorClock() From c78c49dee7f2d56a30a3dc0b6ce264eb10d60efd Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 24 Oct 2023 08:29:32 -0400 Subject: [PATCH 3/6] Lint --- synapse/http/connectproxyclient.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/http/connectproxyclient.py b/synapse/http/connectproxyclient.py index 5bedca922505..59b914b87ecd 100644 --- a/synapse/http/connectproxyclient.py +++ b/synapse/http/connectproxyclient.py @@ -61,6 +61,7 @@ def as_proxy_authorization_value(self) -> bytes: # Encode as base64 and prepend the authorization type return b"Basic " + base64.b64encode(self.username_password) + @attr.s(auto_attribs=True) class BearerProxyCredentials(ProxyCredentials): access_token: bytes From 2c06981097ee32bb326d53940dfbc38cf157faf1 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 24 Oct 2023 13:49:32 +0100 Subject: [PATCH 4/6] Fixup my dodgy test (sorry) --- tests/http/test_proxyagent.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py index b48c2c293a35..ccea9369036f 100644 --- a/tests/http/test_proxyagent.py +++ b/tests/http/test_proxyagent.py @@ -220,16 +220,23 @@ def test_parse_proxy( class TestBasicProxyCredentials(TestCase): def test_long_user_pass_string_encoded_without_newlines(self) -> None: """Reproduces https://github.com/matrix-org/synapse/pull/16504.""" - creds = BasicProxyCredentials( + proxy_connection_string = ( b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass@proxy.local:9988" ) + _, _, _, creds = parse_proxy(proxy_connection_string) + self.assertIsInstance(creds, BasicProxyCredentials) + auth_value = creds.as_proxy_authorization_value() self.assertNotIn(b"\n", auth_value) self.assertEqual( creds.as_proxy_authorization_value(), - b"Basic: bG9vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vbmd1c2VyOnBhc3M=", + b"Basic bG9vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vb29vbmd1c2VyOnBhc3M=", + ) + basic_auth_payload = creds.as_proxy_authorization_value().split(b" ")[1] + self.assertEqual( + base64.b64decode(basic_auth_payload), + b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass", ) - class MatrixFederationAgentTests(TestCase): def setUp(self) -> None: From f922cafa9f6e0117e1a7264d6d5de8f3ea881fcc Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 24 Oct 2023 13:53:09 +0100 Subject: [PATCH 5/6] Lint --- tests/http/test_proxyagent.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py index ccea9369036f..72b385b0743a 100644 --- a/tests/http/test_proxyagent.py +++ b/tests/http/test_proxyagent.py @@ -220,9 +220,7 @@ def test_parse_proxy( class TestBasicProxyCredentials(TestCase): def test_long_user_pass_string_encoded_without_newlines(self) -> None: """Reproduces https://github.com/matrix-org/synapse/pull/16504.""" - proxy_connection_string = ( - b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass@proxy.local:9988" - ) + proxy_connection_string = b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass@proxy.local:9988" _, _, _, creds = parse_proxy(proxy_connection_string) self.assertIsInstance(creds, BasicProxyCredentials) @@ -238,6 +236,7 @@ def test_long_user_pass_string_encoded_without_newlines(self) -> None: b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass", ) + class MatrixFederationAgentTests(TestCase): def setUp(self) -> None: self.reactor = ThreadedMemoryReactorClock() From 4c62938bd7ae4965da6357810e7149732b79eec8 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 24 Oct 2023 13:55:01 +0100 Subject: [PATCH 6/6] Appease mypy --- tests/http/test_proxyagent.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py index 72b385b0743a..41dfd5dc17b9 100644 --- a/tests/http/test_proxyagent.py +++ b/tests/http/test_proxyagent.py @@ -222,6 +222,7 @@ def test_long_user_pass_string_encoded_without_newlines(self) -> None: """Reproduces https://github.com/matrix-org/synapse/pull/16504.""" proxy_connection_string = b"looooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooooonguser:pass@proxy.local:9988" _, _, _, creds = parse_proxy(proxy_connection_string) + assert creds is not None # for mypy's benefit self.assertIsInstance(creds, BasicProxyCredentials) auth_value = creds.as_proxy_authorization_value()