session fixation vulnerability

[Thomas Biege]

Hi,
it looks like openstack-dashboard is vulnerable to a session fixation attack. Here is a HTTP dialog:

1. eviled Login with no Cookie header
-------------------------------------
POST http://test:80/auth/login/ HTTP/1.0
User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
Host: test
Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
Accept-Language: de-DE,de;q=0.9,en;q=0.8
Accept-Encoding: gzip, deflate
Referer: http://test
Cookie: csrftoken=f5ce126776d4ede0bff352d2e53e3e28;
sessionid=f7467b5b66ac49c48c74386d558c3319
Proxy-Connection: Keep-Alive
Content-length: 141
Content-Type: application/x-www-form-urlencoded

csrfmiddlewaretoken=f5ce126776d4ede0bff352d2e53e3e28&method=Login&region=http%3A%2F%2F127.0.0.1%3A5000%2Fv2.0&username=eviled&password=eviled

2. eviled gets response with sessionid Cookie
----------------------------------------------
HTTP/1.1 302 FOUND
Date: Wed, 21 Mar 2012 15:59:09 GMT
Server: Apache/2.2.12 (Linux/SUSE)
Vary: Accept-Language,Cookie
Location: http://test
Content-Language: en
Set-Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319; Path=/
Content-length: 0
Connection: close
Content-Type: text/html; charset=utf-8

3. eviled logout
-----------------
GET http://test:80/auth/logout/ HTTP/1.0
User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
Host: test
Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
Accept-Language: de-DE,de;q=0.9,en;q=0.8
Accept-Encoding: gzip, deflate
Referer: http://test
Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319;
csrftoken=f5ce126776d4ede0bff352d2e53e3e28
Pragma: no-cache
Cache-Control: no-cache
Proxy-Connection: Keep-Alive

4. Admin Login:
---------------
GET http://test:80/auth/logout/ HTTP/1.0
User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
Host: test
Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
Accept-Language: de-DE,de;q=0.9,en;q=0.8
Accept-Encoding: gzip, deflate
Referer: http://test/nova/
Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319;
csrftoken=f5ce126776d4ede0bff352d2e53e3e28
Pragma: no-cache
Cache-Control: no-cache
Proxy-Connection: Keep-Alive

5. Admin request sets the same CSRF token and sessionid Cookie
--------------------------------------------------------------
HTTP/1.1 200 OK
Date: Wed, 21 Mar 2012 15:59:13 GMT
Server: Apache/2.2.12 (Linux/SUSE)
Vary: Cookie,Accept-Language
Content-Language: en
Set-Cookie: csrftoken=f5ce126776d4ede0bff352d2e53e3e28; expires=Wed,
20-Mar-2013 15:59:13 GMT; Max-Age=31449600; Path=/
Set-Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319; Path=/
Connection: close
Content-Type: text/html; charset=utf

[Thomas Biege (thomas-suse-deactivatedaccount)]

The following output is from the owasp.pl tool of my test-suite [1].

thomas:~/test-suite> ./owasp.pl output=short openstack_horizon_lpd978896.ini

[...removed output for session_fixation_public()...]

    DBG:
OWASP_SM_003::session_fixation_private(http://test:80/auth/login/,
POST, method=Login&username=admin&password=openstack)
    DBG: 1. try to login with 'method=Login&username=eviled&password=eviled'
(attacker)
    DBG: OWASP_SM_003::session_fixation_private: cookie =
'sessionid=a917fb9e5c35b03b690aa24ed1815a16; Path=/'
    DBG: 2. try to login with 'method=Login&username=admin&password=openstack'
(victim)
    DBG: OWASP_SM_003::session_fixation_private: session cookie =
'sessionid=a917fb9e5c35b03b690aa24ed1815a16; Path=/'
    DBG: OWASP_SM_003::session_fixation_private: new cookie =
'sessionid=a917fb9e5c35b03b690aa24ed1815a16; Path=/'
    DBG: 3. compare cookie 'sessionid'
    DBG: OWASP_SM_003::session_fixation_private: logged out
=====> OWASP_SM_003::session_fixation_private: CWE-384 (Session Fixation): code
= 1 (msg = 'FAIL:Vulnerable to Session Fixation Attack by authenticated users
(http://test:80/auth/login/)')

2 tests in 2 secs.

[1] https://gitorious.org/test-suite/test-suite

[Thierry Carrez (ttx)]

Thought that identifiers in cookies were safe from session fixation ?
Adding PTL to discuss impact.

[Thomas Biege (thomas-suse-deactivatedaccount)]

Exploitation is harder using cookies, see http://en.wikipedia.org/wiki/Cross-site_cooking

Someone an argue that the bug is in the browser of course, but nevertheless creating a new session cookie after login seems to be missing. Not sure if it is django's responsibility or if it is designed to be done by the dashboard.

Another attack vector can be a XSS attack, see
https://www.owasp.org/index.php/Session_fixation

[Devin Carlen (devcamcar)]

Added Gabriel Hurley and Tres Henry to discuss.

[Devin Carlen (devcamcar)]

Added Paul McMillan who has experience with the core Django pieces involved here.

[Paul McMillan (paul-mcmillan)]

The Django security team is reviewing this issue. The same reporter sent us a similar report this morning via the <email address hidden> alias.

We've dealt with several similar issues in the past. I haven't had a chance to personally verify this bug. If it is valid, it's a Django issue, and not something that should be fixed in Horizon.

The CSRF token portion of this report is irrelevant, given how Django's CSRF tokens work.

The impact is relatively limited for most real-world deployments - generally restricted to any of the following:
 - there are existing XSS/CSRF bugs in the Django app
 - there are login-CSRF bugs (XSS or CSRF on related subdomains is the most common case)
 - an attacker can otherwise read/set arbitrary cookies on your browser prior to logging into the site (wifi mitm, physical access)

[Paul McMillan (paul-mcmillan)]

After thoroughly investigating the Django end of things, I'm forced to conclude this is a Horizon bug, and a serious one. I can reproduce the effect in Horizon, and during testing noticed that Horizon's use of Django's sessions and its connection to keystone seem to bypass some of the assumptions I've made about the protection provided by Django.

In light of that, I don't currently have enough information about this bug to estimate how serious it is, other than that it is at least as serious as indicated above, likely much worse, and I need to spend more time looking at the code and testing. I will try to work on that tomorrow.

[Paul McMillan (paul-mcmillan)]

I'm working on a patch for this now.

[Thomas Biege (thomas-suse-deactivatedaccount)]

Thanks!

Do we have a CVE-ID for this already?

[Thierry Carrez (ttx)]

@Thomas: not yet, we need to work on statement of impact first.

@Paul: Since you did the confirmation, could you describe the attack vector and impact more precisely ? In comment #6 you say it's minor since it requires the ability to pre-set cookies, and in comment #8 you say it's a serious issue...

[Paul McMillan (paul-mcmillan)]

(sorry to be slow responding here)

@Thierry: The most serious other issue I discovered was that there are some circumstances in which it is be possible for user data and/or auth to leak from one session to another.

The fix here is to remove the keystone auth from the middleware, and use it as a proper Django auth backend. This means that Horizon uses the same extremely well tested and audited mechanisms that Django uses. It also means that Horizon will get the same security upgrades that Django does, if a bug is discovered in these mechanisms. It also means that we have consistent behavior across alternate session backends.

I'm in the process of testing my patch for this now.

[Thierry Carrez (ttx)]

@Paul: please attach here when you have the patch for this. A complete impact description based on your findings would also be very cool. Thanks!

[Thierry Carrez (ttx)]

@Paul: any progress on the patch ? I'd rather fix this early than late :)

[Paul McMillan (paul-mcmillan)]

The larger fix for the issue has been giving me trouble, and being out sick for a couple days didn't help.

I think at this point, I should fix it in less comprehensive way we discussed, since I'm worried about bugs the large fix may introduce this late in the release process. I'll get that posted ASAP.

[Paul McMillan (paul-mcmillan)]

Here's the simple version of the patch. It is tested against trunk, but should be extremely similar for backporting.

It fixes both the session fixation bug, and the bug where user sessions could run together.

If we agree this is a reasonable approach, I can go ahead and try out the private gerrit stuff.

[Paul McMillan (paul-mcmillan)]

Once we've dealt with the immediate issue, I'll work with the Horizon team to get the more comprehensive authentication backend rewrite in place for the Folsom release.

[Paul McMillan (paul-mcmillan)]

It's also worth noting that the automatic testing tools may still have a false-positive for this issue when:

signed cookie session backend is in use
the testing tool is logging in and out of one test account
the testing tool does not wait at least 1 second between logins

This is an expected property of that session backend, and is not a bug. In all other cases, the session id should be different.

[Thierry Carrez (ttx)]

Patch looks lean and clean to me. Devin, Gabriel, could you have a look and +1 it ?

@Paul: could you please review the following impact description and check that it describes the issue accurately:

"""
Title: Horizon session fixation and reuse
Impact: Critical
Reporter: Thomas Biege, SUSE
Products: Horizon
Affects: All versions

Description:
Thomas Biege from SUSE reported a vulnerability in OpenStack Dashboard (Horizon). Under specific circumstances it is possible to reuse session cookies from another user, potentially resulting in information leakage between Horizon sessions, including authentication information.
"""

Feel free to suggest corrections that would make the description more precise.

[Gabriel Hurley (gabriel-hurley)]

Patch looks good to me.

As per the impact description, I'd word the last sentence as such: "Under specific circumstances it is possible to reuse session cookies from another user, potentially allowing access to unauthorized information and capabilities."

e.g. the attacker couldn't actually steal your password, but they could change it for you if you're an admin...

[Devin Carlen (devcamcar)]

Patch gets a +1 from me

[Russell Bryant (russellb)]

Thanks for the patch reviews! I'll send out the advance notification with the description updated to reflect Gabriel's comments tomorrow.

Does anyone have a problem with Thursday, April 31st as the CRD (coordinated release date) ?

[Devin Carlen (devcamcar)]

Sounds good.

On Apr 26, 2012, at 6:29 PM, Russell Bryant wrote:

> Thanks for the patch reviews! I'll send out the advance notification
> with the description updated to reflect Gabriel's comments tomorrow.
>
> Does anyone have a problem with Thursday, April 31st as the CRD
> (coordinated release date) ?
>
> --
> You received this bug notification because you are subscribed to the bug
> report.
> https://bugs.launchpad.net/bugs/978896
>
> Title:
> session fixation vulnerability
>
> Status in OpenStack Dashboard (Horizon):
> Confirmed
>
> Bug description:
> Hi,
> it looks like openstack-dashboard is vulnerable to a session fixation attack. Here is a HTTP dialog:
>
> 1. eviled Login with no Cookie header
> -------------------------------------
> POST http://test:80/auth/login/ HTTP/1.0
> User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
> Host: test
> Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
> image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
> Accept-Language: de-DE,de;q=0.9,en;q=0.8
> Accept-Encoding: gzip, deflate
> Referer: http://test
> Cookie: csrftoken=f5ce126776d4ede0bff352d2e53e3e28;
> sessionid=f7467b5b66ac49c48c74386d558c3319
> Proxy-Connection: Keep-Alive
> Content-length: 141
> Content-Type: application/x-www-form-urlencoded
>
> csrfmiddlewaretoken=f5ce126776d4ede0bff352d2e53e3e28&method=Login&region=http%3A%2F%2F127.0.0.1%3A5000%2Fv2.0&username=eviled&password=eviled
>
> 2. eviled gets response with sessionid Cookie
> ----------------------------------------------
> HTTP/1.1 302 FOUND
> Date: Wed, 21 Mar 2012 15:59:09 GMT
> Server: Apache/2.2.12 (Linux/SUSE)
> Vary: Accept-Language,Cookie
> Location: http://test
> Content-Language: en
> Set-Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319; Path=/
> Content-length: 0
> Connection: close
> Content-Type: text/html; charset=utf-8
>
> 3. eviled logout
> -----------------
> GET http://test:80/auth/logout/ HTTP/1.0
> User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
> Host: test
> Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
> image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
> Accept-Language: de-DE,de;q=0.9,en;q=0.8
> Accept-Encoding: gzip, deflate
> Referer: http://test
> Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319;
> csrftoken=f5ce126776d4ede0bff352d2e53e3e28
> Pragma: no-cache
> Cache-Control: no-cache
> Proxy-Connection: Keep-Alive
>
> 4. Admin Login:
> ---------------
> GET http://test:80/auth/logout/ HTTP/1.0
> User-Agent: Opera/9.80 (X11; Linux x86_64; U; de) Presto/2.10.229 Version/11.61
> Host: test
> Accept: text/html, application/xml;q=0.9, application/xhtml+xml, image/png,
> image/webp, image/jpeg, image/gif, image/x-xbitmap, */*;q=0.1
> Accept-Language: de-DE,de;q=0.9,en;q=0.8
> Accept-Encoding: gzip, deflate
> Referer: http://test/nova/
> Cookie: sessionid=f7467b5b66ac49c48c74386d558c3319;
> csrftoken=f5ce126776d4ede0bff352d2e53e3e28
> Pragma: no-cache
> Cache-Control: no-cache
> Proxy-Connection: Keep-Alive
>
> 5. Admin request sets the s...

[Paul McMillan (paul-mcmillan)]

Sounds good to me. Should we try the private gerrit thing, or do we prefer to just patch manually on the release date?

[Thierry Carrez (ttx)]

Patch & modified description look good to me.

There is no such thing as a private gerrit yet. We need to have a look to check how secure the gerrit "draft" feature is, and if it could be abused to manage embargoed updates. For now we just get the patch pre-reviewed here, then on disclosure date we push the patch to gerrit and fast-track it using a willing Horizon coredev.

[Russell Bryant (russellb)]

I meant Thursday, May 3rd ...

[Thierry Carrez (ttx)]

CRD is OK for me. I'll let you send it since I'm awa

[Thierry Carrez (ttx)]

y on Monday-Tuesday so can't deal with any possible fire.

[Russell Bryant (russellb)]

Advance notification email sent

[Gabriel Hurley (gabriel-hurley)]

FYI, this patch applies cleanly to the stable/essex branch and passes the unit tests (including the included unit test that verifies this issue). It should be good-to-go for backport.

[Russell Bryant (russellb)]

How about stable/diablo? Is that branch affected? If so, does this patch apply?

[Gabriel Hurley (gabriel-hurley)]

I can't say if stable/diablo is affected offhand (and I don't have the infrstructure set up to test it in any effective manner), but I can guarantee this patch will not apply cleanly. The very significant changes in Horizon between Diablo and Essex simply don't line up.

[Paul McMillan (paul-mcmillan)]

Looking at the code history, Horizon has been vulnerable to this for as far back as I can trace (11 months). So yes, stable/diablo is affected, and while the patch won't apply cleanly, the relevant code (other than possibly moving around a bit) hasn't changed terribly much. I'm afraid I don't have a setup for testing that either, but I can certainly review the backport.

[Russell Bryant (russellb)]

Thanks.

Are you guys comfortable releasing this without a patch for diablo? Or do we need to find someone to do the backport before Thursday?

[Thierry Carrez (ttx)]

Horizon wasn't a core project in Diablo. So we could say there is no such thing as "Horizon Diablo". And we can certainly make a case that non-core projects/releases are not security-supported.

[Thierry Carrez (ttx)]

So I'd say that "all versions are affected" and provide a fix for Essex and Folsom.

[Russell Bryant (russellb)]

Works for me.

[Gabriel Hurley (gabriel-hurley)]

Makes sense. Sounds good!

[Russell Bryant (russellb)]

Jamie Strandboge is reporting unit test failures with this patch applied to Essex. Can someone please follow up on that today to see if the patch needs to be updated before the release tomorrow?

[Thierry Carrez (ttx)]

Forwarded the email to Gabriel and Paul.

[Gabriel Hurley (gabriel-hurley)]

I applied the patch to stable/essex and ran the unit tests without seeing any failures. That would lead me to believe there's something different in the upstream package/environment that's causing the issue. It's hard to debug that without more details.

Alternatively, the patch could simply be amended to wrap that del call inside an "if hasattr(request, "_cached_user")..." to allow it to fail more gracefully. That said, without knowing why that attribute is not present I can't definitely say that gracefully passing by that line won't cause other problems.

Perhaps Paul can comment on that.

[Paul McMillan (paul-mcmillan)]

I'm not sure how the patch works in one place but not the other...

I'm not terribly thrilled about wrapping the del call inside a conditional. If that value is not set, it indicates that our tests are doing unexpected things (most likely not getting all the way logged in before logging out).

If we wrap it in a conditional, we PROBABLY don't make things worse, from a vulnerability standpoint (if that doesn't exist, it means the user wasn't logged in), but it worries me.

[Thierry Carrez (ttx)]

Added Jamie directly to the bug.

The difference could come from the fact that you test on top of stable/essex while Jamie tests on top of 2012.1 release. There are 3 extra commits (+ a merge commit) that were backported since April 5: https://github.com/openstack/horizon/commits/stable/essex

That said, it's really not obvious why those extra commits would cause those failures.

[Jamie Strandboge (jdstrand)]

I just tried each commit since April 3rd (unpatched horizon in Ubuntu already has those from the 3rd) and they have no effect on the test results. It is only after I remove the patch for CVE-2012-2144 do the tests start passing again. My testing is on Ubuntu 12.04 and consists of:
$ apt-get source horizon
$ apt-get build-dep horizon
$ cd horizon-2012.1
$ cat /tmp/session_fixation.diff | patch -p1
patching file horizon/exceptions.py
patching file horizon/middleware.py
patching file horizon/tests/auth_tests.py
patching file horizon/users.py
patching file horizon/views/auth.py
patching file horizon/views/auth_forms.py
$ PYTHONPATH=. bash run_tests.sh -N
...
Ran 191 tests in 3.604s

FAILED (errors=5)

This mimics what we use in our builds. I (re-)verified that the patch I'm using is the same as the one in this bug. Attached is the failure output.

[Russell Bryant (russellb)]

This issue was scheduled to be opened up and patches released in just over an hour. I suggest we hold off until we get to the bottom of these failures.

[Russell Bryant (russellb)]

If you guys in horizon-core feel that we should push it out anyway, just proceed with pushing the patches to gerrit as usual when you're ready and I'll follow up with the security advisory.

[Jamie Strandboge (jdstrand)]

Perhaps it is worth noting that Ubuntu uses django 1.3.1 (specifically 1.3.1-4ubuntu1).

[Jamie Strandboge (jdstrand)]

That may be it. If I build my package on Debian unstable (which has django 1.4), it passes.
$ PYTHONPATH=. bash run_tests.sh -N
...
Ran 191 tests in 4.918s

OK

[Thierry Carrez (ttx)]

Question is, is the patch not applicable to Django 1.3... or is the test fail just a false negative ? We might need to wait for Paul to answer that, and he is west coast.

[Paul McMillan (paul-mcmillan)]

I didn't think that anything relevant changed between Django 1.3 and 1.4, but it sounds like it has. I'm working on replicating this problem now. I hope to have a tested fix later today.

[Paul McMillan (paul-mcmillan)]

I reproduced the problem, and it seems to be related to less-than-complete test cases that weren't accessing the (lazy) user object before attempting to log out. I've trivially modified the patch to deal with that case (should never occur in real use), and feel that the modification makes the patch slightly safer overall.

I'll be putting this in gerrit shortly.

[Russell Bryant (russellb)]

Opened up the bug. Paul is pushing the patches to gerrit now.

[Russell Bryant (russellb)]

Patches merged and the advisory has been sent. I'm going to unsubscribe the vulnerability management team now.

https://lists.launchpad.net/openstack/msg11263.html