loosen validation on matching trusted dashboard

[Steve Martinelli]

In the current implementation for verifying where the SSO request came from, the host is grabbed from the 'origin' query parameter, and compared to the list of 'trusted_dashboards' in the config file.

  origin = context['query_string'].get('origin')
  host = urllib.parse.unquote_plus(origin)
  if host in CONF.federation.trusted_dashboard:
    ...

https://github.com/openstack/keystone/blob/master/keystone/contrib/federation/controllers.py#L278-L287

This works, but unless the entry is marked perfectly in the config file, it won't match. We should loosen the validation that is performed, and maybe even use the HTTP Referer instead (and no longer require the 'origin' parameter from horizon).

We should be able to decompose the Refer to figure out the scheme + hostname + path, and use that hostname to check against the trusted dashboards.

[Lin Hua Cheng (lin-hua-cheng)]

Steve, I can take a stab at this for Liberty.

[Lin Hua Cheng (lin-hua-cheng)]

It would be neat to generate the redirect URL to horizon instead of reading from the origin parameter, this could be done by just using the HOST name and generate the URL using the template: http://<HOST>/auth/websso/. But I realized this would not always work because Horizon can be configured in different WEBROOT too.

For example: http://<HOST>/<horizon_web_root>/auth/websso/

Too bad for that.

As for using the Referer, I tried printing the Referer from the context and this is what I got:
  'HTTP_REFERER': 'http://localhost:5000/v3/auth/OS-FEDERATION/websso/redirect'

It seems like it doesn't work. I also read that using HTTP_REFERER is not reliable and vulnerable to attacks as well since an malicious user can simple replace this HTTP header.

So.. It looks like what we got is actually the right way to go.

What I could do to loosen the validation is just match the PROTOCOL://HOST_NAME of trusted_dashboard against the value in origin instead of matching the whole string.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/175169

[David Stanek (dstanek)]

++ On not trusting the HTTP headers if there are used in any way for security. Both HTTP_HOST and HTTP_REFERER are easy to fake. Generally speaking all HTTP_* vars are just pulled from the client request and stuffed into the environment. They should be treated like any other user provided data.

[David Stanek (dstanek)]

I am a little concerned about the patch that was just merged. It makes it possible to exploit Keystone through a broken dashboard. For example, if the dashboard allowed redirects to other sites (unvalidated redirects) then we would be vulnerable. I don't know that Horizon (or any other dashboard) has this issue currently, but this makes my Spidey senses tingle.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/175169
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=fb6920e5fe1fef2fa32afe602d2bf93f18d48a3f
Submitter: Jenkins
Branch: master

commit fb6920e5fe1fef2fa32afe602d2bf93f18d48a3f
Author: lin-hua-cheng <email address hidden>
Date: Sat Apr 18 15:37:00 2015 -0700

    Loosen validation on matching trusted dashboard

    Instead of performing an exact matching for the validation if
    origin is in trusted dashboard, loosen the check by comparing
    the scheme and netloc of the URL instead.

    Change-Id: I995214058c4071d9089a8f00b9b8002fd125fda0
    Closes-Bug: #1440958

[Lin Hua Cheng (lin-hua-cheng)]

The host name to check is still the same, it still looks at the value of the "origin" query parameter. But instead of exact match to trusted dashboard, it just matches by <scheme>://<netloc>.

Can you provide more details on the scenario that concerns you? I'll be happy to revert it back if it does open up possible exploits.

[David Stanek (dstanek)]

The possible attack is someone forging the querystring value to a dashboard URL that has its own security problem. Specifice an unvalidated redirect or forward. At this point I don't they that Horizon has this problem, but it's possible that a cloud deploys their own dashboard.

I thought it was worth mentioning because in this case the security of this particular operation seems to be dependent on Keystone and the dashboard.

[Lin Hua Cheng (lin-hua-cheng)]

Instead of matching only the <scheme>://<netloc> of the redirect_url with the URLs in trusted_dashboard, how about changing the check into <redirect_url>.startswith (<url in trusted_dashboard>)

this provides the flexibility to the deployer to make the validation loose by providing just the <scheme>://<netloc> or restrictive to check the full redirect URL.

[David Stanek (dstanek)]

In theory, relaxing the exact match makes us vulnerable to an attack if this functionality is used with a dashboard that allows unvalidated redirects.

Could a user spoof this by setting the dashboard URL to something like: http://dashboard/redirect?url=http://hacked_site ?

And if they can what could they steal?

[Lin Hua Cheng (lin-hua-cheng)]

if horizon (djanog) redirects to http://hacked_site after login, it would just perform a simple redirect [1] to the hacked site. Horizon stores the session information of the login user in the cookie, but the cookie will be scoped to the domain of horizon. So the bad site it redirected to will not be able to access any of the session information.

[1] https://github.com/django/django/blob/master/django/contrib/auth/views.py#L47-L53

[Marek Denis (marek-denis)]

If the problem is that it's hard to add redirect urls in keystone config i think we should improve error msgs and logging msgs (even though they seemed to specify what was the input, so it should be easily to sketch up a script that compares values with ones from keystone.conf) instead of loosing restrictions on those checks.

[Lin Hua Cheng (lin-hua-cheng)]

after discussing in IRC it was decided to just revert the patch to loosen the validation, and then improve the logging to make it easier to resolve issue when validation fails on the trusted_dashboard.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/180343
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=b48c820e3015a0d6264df6a0a87bf1a3dbea61c4
Submitter: Jenkins
Branch: master

commit b48c820e3015a0d6264df6a0a87bf1a3dbea61c4
Author: Lin Hua Cheng <email address hidden>
Date: Tue May 5 22:33:24 2015 +0000

    Revert "Loosen validation on matching trusted dashboard"

    Loosening the validation introduce a security hole for unvalidated redirect.

    For example: redirect_url=http://dashboard/sso?next=http://hacksite

    This reverts commit fb6920e5fe1fef2fa32afe602d2bf93f18d48a3f.

    Change-Id: I7e85b2b879f4c66c3664e8610d3ddbb999a5ac75
    Closes-Bug: #1440958