fix(sso): respect X-Forwarded-* headers for redirect URL. Fixes #13031

[MasonM]

Fixes #13031

Motivation

When running Argo in plain text mode behind a TLS termination proxy with SSO enabled, you will always be redirected to the workflows page after logging in, regardless of which page you came from. This is happening because server/auth/sso/sso.go is using the protocol and host of the request to determine the redirect URL, which can be different than the original request when a proxy is involved.

Modifications

The fix involves reading from the X-Forwarded-Host and X-Forwarded-Proto headers when present. This is almost the same as the fix that oauth2-proxy made when they faced a similar issue in oauth2-proxy/oauth2-proxy#957. The only major difference is that oauth2-proxy only reads those headers when the --reverse-proxy option is set, but I didn't add a corresponding option here. The only reason I can think of for doing so is to preserve backwards-compatibility, since it's theoretically possible this could break someone's setup. I can't think of any real-world situations where that can happen, but let me know if I should put this behind a option and I'm happy to do so.

Verification

Testing instructions:

1. Run make start UI_SECURE=true PROFILE=sso
2. Visit https://localhost:8080/cron-workflows
3. Click the "Login" button
4. Click "Log in with Example"
5. Click "Grant Access"
6. Confirm you're returned to https://localhost:8080/cron-workflows

Screen.Recording.2024-09-16.at.9.56.43.AM.mov

[jmeridth]

We are having the exact same issue. Thank you for coding this up.

[MasonM]

FYI: I split off the dev environment improvements to #13674

[agilgur5]

my main concern with changing configuration based on proxy is that it becomes susceptible to MITMs or similar -- anyone can send such a header.

Whereas redirectURL is configured by an admin

[agilgur5]

Hmm this was previously using the request URL (r.URL.Scheme), which I believe could be similarly spoofed 🤔 .
HandleCallback wasn't using that though, but was using r.Host, although that's technically not exactly the same as the host header in Go (related #12691)

[MasonM]

Yes, but that was already an inherent problem with this setup, because if you're running a TLS termination in front of Argo, you have to trust everything in between you and the proxy (including the proxy). The only way of protecting against a malicious proxy is through E2E encryption.

Also, if someone does MITM the connection and has the ability to inject arbitrary headers, then they almost certainly have the ability to completely rewrite the response body too, in which case there's no need for them to redirect the user to a malicious page. They can just directly serve up whatever content they want,.

[agilgur5]

With the specific setup of the issue, yes, but this potentially extends it further too. Even with E2EE, a maliciously crafted request could potentially send someone to an uncontrolled/malicious URL, similar to an open redirect.

[MasonM]

Even with E2EE, a maliciously crafted request could potentially send someone to an uncontrolled/malicious URL, similar to an open redirect.

I can't think of any attack vector where that can happen that wasn't already a viable attack vector. If E2E encryption is used, the only means for an attacker to inject arbitrary headers is to either compromise the Argo server or the end user's device, and in both cases the attacker isn't going to bother with open redirect vulnerabilities because they can just rewrite the page to be whatever they want.

Regardless, I entered #13747 with an alternative approach: use relative redirect URLs. That sidesteps the whole issue.

[agilgur5]

If E2E encryption is used, the only means for an attacker to inject arbitrary headers is to either compromise the Argo server or the end user's device

The simplest and one of the most common variants is to, for instance, write an informative blog post that sends requests to the API, and one of those requests is maliciously crafted. The user then copy+pastes it unknowingly.

In CVSS, this kind of exploit requires "User Interaction", but does not require compromising the user's device

[MasonM]

Sorry, I'm still not seeing how an attacker could inject arbitrary headers. I know an attacker can control the ?redirect= parameter via something like https://localhost:8080/oauth2/redirect?redirect=https://evil.com, but that's useless if they can't control the request headers too. Here's what would happen if an attacker were to try to pull this off:

1. Attacker tricks user into visiting https://localhost:8080/oauth2/redirect?redirect=https://evil.com
2. Argo stores https://evil.com in the cookie associated with the state parameter and redirects the user to the OAuth2 provider
3. User logs in
4. OAuth2 provider redirects user to https://localhost:8080/oauth2/callback
5. Argo reads the cookie and rejects https://evil.com because it doesn't match request headers, so it redirects them to https://localhost:8080/

Even if the attacker can execute Javascript and send AJAX requests, the CORS policy won't allow it. Try running this in a DevTools console at https://example.com :

fetch('https://localhost:8080/oauth2/redirect?redirect=https://evil.com', {headers: {"X-Forwarded-Host": "evil.com"}})

You should see an error like this:

Access to fetch at 'https://localhost:8080/oauth2/redirect?redirect=https://evil.com' from origin 'https://example.com' has been blocked by CORS policy: Response to preflight request doesn't pass access control check: Redirect is not allowed for a preflight request.

Can you give a concrete example of the kind of attack you're thinking of?

[agilgur5]

Let me know if I should split this off to a separate PR.

FYI: I split off the dev environment improvements to #13674

Thanks Mason. I requested this over Slack as it is would make it easier to review and backport, and is a general best practice for smaller PRs.

[MasonM]

/retest

[MasonM]

/retest

[MasonM]

/retest

[MasonM]

/retest

[MasonM]

/retest

[MasonM]

/retest