Support for a whitelist of --redirect-domains

[colemickens]

Per my comments here, I'd like to add support for --redirect-domains to take a comma-delimited list of whitelisted redirect domains.

Example scenario:

• https://secret-static-content.mydomain.tld -> httpd fronting a static directory
• https://more-secret-content.mydomain.tld -> httpd fronting a different static directory
• https://auth.mydomain.tld/oauth2 -> oauth2_proxy instsance

Currently, I can't seem to get things to work properly in terms of handling the redirect after the callback succeeds.

It seems like it's due to the filtering that happens here:

• oauth2_proxy/oauthproxy.go

  Line 419 in 6d6cb7e

  if redirect == "" || !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {

The same logic seems duplicated here:

• oauth2_proxy/oauthproxy.go

  Line 551 in 6d6cb7e

  if !strings.HasPrefix(redirect, "/") || strings.HasPrefix(redirect, "//") {

I propose a PR such that:

• there is a new --redirect-domains flag that takes a list of domains. example: (--redirect-domains=mickens.io,mickens.us).

• the redirect filter is extended to allow any URLs that have a Host that ends with a value in the --redirect-domain list.

Does this sound reasonable?

[colemickens]

Actually, I'm not sure I understand why the redirect locations needs to be limited... is that a security concern as well? Thanks!

[jehiah]

@colemickens Thanks for this PR; i'll hopefully get to look through it soon.

There is a security aspect to restricting redirect domains. This is to protect against an attacker crafting a URL with a malicious redirect parameter that will send you through the oauth2_proxy auth flow as you would expect, but then redirect to a malicious site after authentication. A user should only end on an authorized URL after authentication. This eliminates many avenues of phishing and other social engineering.

[ploxiln]

also discussed in #378

[colemickens]

Not a PR yet, just curious if it would be desired/accepted before I write it. I've confirmed that just removing those filtering lines mentioned unblocks my scenario, but I don't want to continue to use that if it's insecure and would prefer to just use upstream instead of my fork anyway.

@jehiah I'm not sure I understand your comments about the redirect, though. Regardless of the scrubbing done today, the "redirect" that is handed to GitHub is always the oauth2_proxy callback endpoint (by nature, GitHub will only allow redirect to that URL anyway). The thing being scrubbed here is the "second" redirect that oauth2_proxy handles inside the callback function (by decomposing state into the nonce+rd to then return a 302 to bounce the user back to their original URL. At that point, the correct endpoint had been given the oauth authorization_code, so there shouldn't be a need to further filter. No?

[ploxiln]

The concern is not the authorization code (or token), it's social engineering aka "phishing". For example:

Attacker sends user a email or something with a link "access the secure benefits application form on <a href="secure.com/oauth2/start?rd=evil.com/fake-form">secure.com</a>". After clicking the link, the user can verify they are on "secure.com", click sign-in, go through through the authentic google or github authorization, and then not realize that after all that they end up at evil.com (which could be a subtle typo of secure.com).

not as much detail there, but the original issue and PR for this: #228 #359

[colemickens]

Okay, thank you, that makes more sense. In that case, it seems like manually white listing some redirect host suffixes would be acceptable. I'll add this sometime soon unless someone jumps in and says it's a bad idea. Thanks all.