Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Infinte Redirect Loop For SSO Login When Behind A Reverse Proxy #10492

Closed
warricksothr opened this issue Jul 28, 2021 · 15 comments
Closed

Infinte Redirect Loop For SSO Login When Behind A Reverse Proxy #10492

warricksothr opened this issue Jul 28, 2021 · 15 comments
Labels
X-Needs-Info This issue is blocked awaiting information from the reporter

Comments

@warricksothr
Copy link
Contributor

Description

When SSO is enabled, the public_baseurl is set to an https scheme endpoint, and the site is running
behind a reverse proxy that terminates ssl and forwards all traffic as http the SSO login redirect will
infinitely loop because the request is for an http endpoint but the public_baseurl is pointed at an
https endpoint. The root of the issue is the simple startswith bytes check inside the SSO redirect
that is meant to make sure that cookies are set on the right domain (#9436).

The public_baseurl mentions that it should be set to the same scheme as what is behind the reverse proxy
however that results in clients like element web being unable to resolve resources behind the reverse proxy
as they aren't always obeying 301 and 302 redirects for resources and exposing the http endpoint externally is
undesirable.

Workaround

The current work around used on my homeserver (matrix.nulloctet.com) is to point the reverse proxy at the https
backend for synapse and leave the public_baseurl as https://matrix.nulloctet.com/. This results in no loop because
the web request is on the same scheme as the public_baseurl. However I am not a fan of terminating an TLS connection only to remote proxy another TLS connection behind the scene. I also do not want to run synapse
completely public as that would be a regression in functionality and would be a special snowflake among the other
services I run.

Proposed Solution

The check of the request and the public_baseurl should completely ignore the scheme, as only the dns name
is required to match for cookies to be correctly set.

Steps to reproduce

  • Set public_baseurl to use https scheme
  • Setup; reverse proxy that forwards all https traffic to the http endpoint of synapse
  • Enable an SSO provider
  • Attempt to login with that provider

Version information

  • Homeserver: matrix.nulloctet.com

If not matrix.org:

Versions Tested:

  • Version: 1.38.1

  • Version: 1.37.1

  • Version: 1.35.0

  • Install method: Docker-Compose

  • Platform: Ubuntu Linux 20.04 HWE with Docker CE
@clokep
Copy link
Member

clokep commented Jul 28, 2021

@warricksothr Have you seen the docs on ensuring that X-Forwarded-Proto is configured properly by your reverse proxy? I believe that fixes this issue.

@clokep clokep added the X-Needs-Info This issue is blocked awaiting information from the reporter label Jul 28, 2021
@warricksothr
Copy link
Contributor Author

@clokep I have my server defined with the X-Forwarded-Proto as defined in the reverse proxy definition

Here is the relevant nginx location block with the workaround I mentioned above. I tested this previously with the proxy_pass pointed to the http port with the correct matching scheme.

        location ~* ^(\/_matrix|\/_synapse\/client) {
                proxy_pass https://localhost:8448;
                proxy_set_header X-Forwarded-For $remote_addr;
                proxy_set_header X-Forwarded-Proto $scheme;
                proxy_set_header Host $host;
        }

Unfortunately the code that is responsible for this check just looks at just the request to see if it is secure and doesn't check any additional headers.

def get_request_uri(request: IRequest) -> bytes:
    """Return the full URI that was requested by the client"""
    return b"%s://%s%s" % (
        b"https" if request.isSecure() else b"http",
        _get_requested_host(request),
        # despite its name, "request.uri" is only the path and query-string.
        request.uri,
    )

and then does an assertion that the request and the public_baseurl start with the same bytes

...
        requested_uri = get_request_uri(request)
        baseurl_bytes = self._public_baseurl.encode("utf-8")
        if not requested_uri.startswith(baseurl_bytes):
...

This does bring up a potentially cleaner fix than the one I have proposed in #10494 however it might also be a bit more opaque and error prone when that header is missing.

@clokep
Copy link
Member

clokep commented Jul 28, 2021

Unfortunately the code that is responsible for this check just looks at just the request to see if it is secure and doesn't check any additional headers.

I believe the isSecure check takes into account the multiple headers:

def _process_forwarded_headers(self):
headers = self.requestHeaders.getRawHeaders(b"x-forwarded-for")
if not headers:
return
# for now, we just use the first x-forwarded-for header. Really, we ought
# to start from the client IP address, and check whether it is trusted; if it
# is, work backwards through the headers until we find an untrusted address.
# see https://github.com/matrix-org/synapse/issues/9471
self._forwarded_for = _XForwardedForAddress(
headers[0].split(b",")[0].strip().decode("ascii")
)
# if we got an x-forwarded-for header, also look for an x-forwarded-proto header
header = self.getHeader(b"x-forwarded-proto")
if header is not None:
self._forwarded_https = header.lower() == b"https"
else:
# this is done largely for backwards-compatibility so that people that
# haven't set an x-forwarded-proto header don't get a redirect loop.
logger.warning(
"forwarded request lacks an x-forwarded-proto header: assuming https"
)
self._forwarded_https = True
def isSecure(self):
if self._forwarded_https:
return True
return super().isSecure()

@warricksothr
Copy link
Contributor Author

Hmm... I'll have to do a bit more digging then. Thanks for the information!

@warricksothr
Copy link
Contributor Author

warricksothr commented Jul 28, 2021

I agree, this code definitely is supposed to take that into account. My configuration is setup with the appropriate header on the proxied request, but I still received the following log entries.

**2021-07-27 21:08:25,279 - synapse.rest.client.v1.login - 555 - INFO - GET-26 - Requested URI http://matrix.nulloctet.com/_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F is not canonical: redirecting to https://matrix.nulloctet.com/_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F**
synapse.access.http.8008 - 389 - INFO - GET-26 - ::ffff:172.24.0.1 - 8008 - {None} Processed request: 0.001sec/-0.000sec (0.000sec, 0.000sec) (0.000sec/0.000sec/0) 20B 302 "GET /_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F HTTP/1.0" "Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/92.0.4515.107 Safari/537.36" [0 dbevts]
2021-07-27 21:08:25,358 - synapse.rest.client.v1.login - 555 - INFO - GET-27 - Requested URI http://matrix.nulloctet.com/_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F is not canonical: redirecting to https://matrix.nulloctet.com/_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F

🤔

@jerrykan
Copy link
Contributor

jerrykan commented Aug 5, 2021

I just upgrade from Synapse v1.28.0 to v.1.39.0 and hit this issue. Using the Apache setting ProxyPreserveHost On seems to have resolved the problem. Ideally it would be good if Synapse looked at the first entry in X-Forwarded-Host request header so this extra settings isn't required (we have multiple reverse proxies sitting in front of Synapse, so we had to make the change on each of those).

@richvdh
Copy link
Member

richvdh commented Aug 5, 2021

I'm pretty sure this is a configuration error in the reverse-proxy. If there's any doubt, I suggest taking a tcpdump of the traffic between the reverse-proxy and Synapse, and inspecting the headers.

In the meantime, I'm going to close this.

@richvdh richvdh closed this as completed Aug 5, 2021
@warricksothr
Copy link
Contributor Author

Apologies for the delay, I did switch back to the http port to do some debugging, and here's part of a TCP dump of the traffic across the poxy during an OIDC redirect.

.GET /_matrix/client/r0/login/sso/redirect/oidc-keycloak?redirectUrl=https%3A%2F%2Felement.nulloctet.com%2F HTTP/1.0
X-Forwarded-For: <redacted ipv4 address>
X-Forwarded-Proto: https
Host: matrix.nulloctet.com
Connection: close
user-agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko) Version/14.1.2 Safari/605.1.15
accept: text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8
accept-language: en-us
accept-encoding: gzip, deflate, br

To me this looks correct, both X-Forwarded-For and X-Forwarded-Proto are set, yet a redirect is being triggered.

@warricksothr
Copy link
Contributor Author

@richvdh is there anything else you'd want to see from a TCP dump, I didn't see a whole lot but am not necessarily sure what I'd be looking for. I have a snapshot of 30 seconds of traffic during which the OIDC redirect is triggered that I'd be willing to share. However it is full of ipv4 addresses so I'm not willing to share such a log publicly without redacting all of those addresses.

@warricksothr
Copy link
Contributor Author

Here's the full reverse proxy configuration for reference

server {
        listen 173.208.198.250:80 ;
        listen [::]:80 ;
        server_name matrix.nulloctet.com;

        location / {
                return 301 https://$host$request_uri;
        }
}

server {
        listen [::]:443 ssl ipv6only=on;
        listen 173.208.198.250:443 ssl;
        ssl_certificate /etc/letsencrypt/live/matrix.nulloctet.com/fullchain.pem;
        ssl_certificate_key /etc/letsencrypt/live/matrix.nulloctet.com/privkey.pem;
        include /etc/letsencrypt/options-ssl-nginx.conf;
        ssl_dhparam /etc/letsencrypt/ssl-dhparams.pem;

        root /var/www/html;
        server_name matrix.nulloctet.com;

        # This value also needs to be changed in the homeserver.yaml file
        client_max_body_size 500M;

        location ~* ^(\/_matrix|\/_synapse\/client) {
                proxy_pass http://localhost:8447;
                proxy_set_header X-Forwarded-For $remote_addr;
                proxy_set_header X-Forwarded-Proto $scheme;
                proxy_set_header Host $host;
        }
}

@richvdh
Copy link
Member

richvdh commented Aug 6, 2021

silly question maybe, but have you set x_forwarded: true in the listener configuration in the homeserver config file? cf https://matrix-org.github.io/synapse/latest/reverse_proxy.html#homeserver-configuration

@warricksothr
Copy link
Contributor Author

warricksothr commented Aug 6, 2021

Not a silly question at all. I don't know how I missed this configuration parameter for the http port. With x_forwarded: true set, redirecting is working correctly. Thank you @richvdh and @clokep for your help!

This syanpse configuration file is a few years old, I'll need to go through it again to make sure I haven't missed other settings as I've upgraded and enabled additional functionality.

@richvdh
Copy link
Member

richvdh commented Aug 6, 2021

great, glad you got it sorted.

I think it's a shame that https://matrix-org.github.io/synapse/latest/reverse_proxy.html#homeserver-configuration is easy to overlook. It might be better to move that section above the "Reverse-proxy configuration examples". If you'd like to make a PR to do so, that would be great!

@warricksothr
Copy link
Contributor Author

Sure, I'll open a PR later today for the documentation change to move that section toward the top of the Reverse Proxy documentation.

@VPaulV
Copy link

VPaulV commented Jul 5, 2023

Had the same problem, despite the proper config had infinite loop. Was able to resolve by updating nginx to the last stable version

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
X-Needs-Info This issue is blocked awaiting information from the reporter
Projects
None yet
Development

No branches or pull requests

5 participants