From b9276e21eee569c244ef40cab0285fc067dbf702 Mon Sep 17 00:00:00 2001
From: Quentin Gliech <quenting@element.io>
Date: Tue, 25 Feb 2025 11:34:33 +0100
Subject: [PATCH] Fix MSC4108 'rendez-vous' responses with some reverse proxy
 in the front of Synapse (#18178)

MSC4108 relies on ETag to determine if something has changed on the
rendez-vous channel.
Strong and correct ETag comparison works if the response body is
bit-for-bit identical, which isn't the case if a proxy in the middle
compresses the response on the fly.

This adds a `no-transform` directive to the `Cache-Control` header,
which tells proxies not to transform the response body.

Additionally, some proxies (nginx) will switch to `Transfer-Encoding:
chunked` if it doesn't know the Content-Length of the response, and
'weakening' the ETag if that's the case. I've added `Content-Length`
headers to all responses, to hopefully solve that.

This basically fixes QR-code login when nginx or cloudflare is involved,
with gzip/zstd/deflate compression enabled.
---
 changelog.d/18178.bugfix             | 1 +
 rust/src/rendezvous/mod.rs           | 6 +++++-
 tests/rest/client/test_rendezvous.py | 6 ++++--
 3 files changed, 10 insertions(+), 3 deletions(-)
 create mode 100644 changelog.d/18178.bugfix

diff --git a/changelog.d/18178.bugfix b/changelog.d/18178.bugfix
new file mode 100644
index 0000000000..d91327803b
--- /dev/null
+++ b/changelog.d/18178.bugfix
@@ -0,0 +1 @@
+Fix MSC4108 QR-code login not working with some reverse-proxy setups.
diff --git a/rust/src/rendezvous/mod.rs b/rust/src/rendezvous/mod.rs
index 23de668102..3148e0f67a 100644
--- a/rust/src/rendezvous/mod.rs
+++ b/rust/src/rendezvous/mod.rs
@@ -47,7 +47,7 @@ fn prepare_headers(headers: &mut HeaderMap, session: &Session) {
     headers.typed_insert(AccessControlAllowOrigin::ANY);
     headers.typed_insert(AccessControlExposeHeaders::from_iter([ETAG]));
     headers.typed_insert(Pragma::no_cache());
-    headers.typed_insert(CacheControl::new().with_no_store());
+    headers.typed_insert(CacheControl::new().with_no_store().with_no_transform());
     headers.typed_insert(session.etag());
     headers.typed_insert(session.expires());
     headers.typed_insert(session.last_modified());
@@ -192,10 +192,12 @@ impl RendezvousHandler {
             "url": uri,
         })
         .to_string();
+        let length = response.len() as _;
 
         let mut response = Response::new(response.as_bytes());
         *response.status_mut() = StatusCode::CREATED;
         response.headers_mut().typed_insert(ContentType::json());
+        response.headers_mut().typed_insert(ContentLength(length));
         prepare_headers(response.headers_mut(), &session);
         http_response_to_twisted(twisted_request, response)?;
 
@@ -299,6 +301,7 @@ impl RendezvousHandler {
         // proxy/cache setup which strips the ETag header if there is no Content-Type set.
         // Specifically, we noticed this behaviour when placing Synapse behind Cloudflare.
         response.headers_mut().typed_insert(ContentType::text());
+        response.headers_mut().typed_insert(ContentLength(0));
 
         http_response_to_twisted(twisted_request, response)?;
 
@@ -316,6 +319,7 @@ impl RendezvousHandler {
         response
             .headers_mut()
             .typed_insert(AccessControlAllowOrigin::ANY);
+        response.headers_mut().typed_insert(ContentLength(0));
         http_response_to_twisted(twisted_request, response)?;
 
         Ok(())
diff --git a/tests/rest/client/test_rendezvous.py b/tests/rest/client/test_rendezvous.py
index ab701680a6..83a5cbdc15 100644
--- a/tests/rest/client/test_rendezvous.py
+++ b/tests/rest/client/test_rendezvous.py
@@ -117,10 +117,11 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase):
         headers = dict(channel.headers.getAllRawHeaders())
         self.assertIn(b"ETag", headers)
         self.assertIn(b"Expires", headers)
+        self.assertIn(b"Content-Length", headers)
         self.assertEqual(headers[b"Content-Type"], [b"application/json"])
         self.assertEqual(headers[b"Access-Control-Allow-Origin"], [b"*"])
         self.assertEqual(headers[b"Access-Control-Expose-Headers"], [b"etag"])
-        self.assertEqual(headers[b"Cache-Control"], [b"no-store"])
+        self.assertEqual(headers[b"Cache-Control"], [b"no-store, no-transform"])
         self.assertEqual(headers[b"Pragma"], [b"no-cache"])
         self.assertIn("url", channel.json_body)
         self.assertTrue(channel.json_body["url"].startswith("https://"))
@@ -141,9 +142,10 @@ class RendezvousServletTestCase(unittest.HomeserverTestCase):
         self.assertEqual(headers[b"ETag"], [etag])
         self.assertIn(b"Expires", headers)
         self.assertEqual(headers[b"Content-Type"], [b"text/plain"])
+        self.assertEqual(headers[b"Content-Length"], [b"7"])
         self.assertEqual(headers[b"Access-Control-Allow-Origin"], [b"*"])
         self.assertEqual(headers[b"Access-Control-Expose-Headers"], [b"etag"])
-        self.assertEqual(headers[b"Cache-Control"], [b"no-store"])
+        self.assertEqual(headers[b"Cache-Control"], [b"no-store, no-transform"])
         self.assertEqual(headers[b"Pragma"], [b"no-cache"])
         self.assertEqual(channel.text_body, "foo=bar")
 
-- 
GitLab