[OSSA 2013-014] auth_token middleware neglects to check expiry of signed token

[Eoghan Glynn]

Unless I'm mistaken the keystoneclient auth_token middleware seems to be neglecting to check the expiry of signed tokens.

Instead, it only checks if the proposed token has been explicitly revoked:

  https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/middleware/auth_token.py#L1047

Surely the expiration timestamp needs to be checked also and the token rejected if expired.

[Eoghan Glynn (eglynn)]

Verified that ayoung's patch addresses the issue in my devstack env.

[Adam Young (ayoung)]

This version is a little clearer in what it is testing.

[Eoghan Glynn (eglynn)]

Confirmed second patch also, so +1 from me.

[Adam Young (ayoung)]

Backport for folsom. auth_token middleware in Folsom was in Keystone, not in the keystone client.

[Thierry Carrez (ttx)]

Adding more keystone core eyes to +1 the proposed patches (keystone/folsom and python-keystoneclient)

[Thierry Carrez (ttx)]

Making sure I understand this correctly...

Your patch fixes _cache_put() so that it doesn't store expired tokens... But _cache_get() seems to properly implement expiration check so it will not return expired tokens. So is this really exploitable ? Or exploitable only if the token is not in the cache already ?

[Adam Young (ayoung)]

It seems like that is the case on first glance, but further investigation of the code shows otherwise. The cache_get function does not return anything if the value is expired, but then the calling code treats that as a cache miss, and re-verifies the token. For a UUID token, this would mean going back to the server, and thus get a status of "Invalid." However, with PKI tokens, it just reruns the validation code in process which was not checking the expiration time. So the original cache validation is not effective in checking token expiration.

[Thierry Carrez (ttx)]

OK, so this only affects PKI tokens ?

[Eoghan Glynn (eglynn)]

@ttx: only affects signed tokens (there's a separate codepath in the auth_token middleware for verifying UUID tokens) .

[Dolph Mathews (dolph)]

The test included in the patch couldn't be any simpler to illustrate the issue; Big +1 for that! (and it fails without the corresponding fix)

I'm lost though on why the patch is so big? Why do ALL the examples/pki/ files have to be recreated, rather than just adding the two new files? (auth_token_scoped_expired.json and auth_token_scoped_expired.xml). +2 otherwise!

[Adam Young (ayoung)]

the pem files are the signed versions of those files. We don't regenerate them on the fly. So, even though the only change in the JSON file is the date, it completely changes the signature.

[Thierry Carrez (ttx)]

Proposed impact description:

Title: Expiration check failure in Keystone PKI tokens validation
Reporter: Eoghan Glynn (Red Hat)
Products/Affects: Keystone (Folsom only), python-keystoneclient (0.2.0+)

Description:
Eoghan Glynn from Red Hat reported a vulnerability in expiry checks for PKI tokens in the Keystone authentication middleware. Expired tokens for authenticated users could continue to be used as long as the user wasn't disabled, potentially resulting in the bypass of intended security policies. Only setups using PKI tokens are affected.

Note:
The affected code was added to Keystone in the Folsom release, but was moved to python-keystoneclient during the Grizzly development cycle.

[Eoghan Glynn (eglynn)]

Proposed tweak to wording of impact description:

s/Title: Expiration check failure/Title: Missing expiration check/

Otherwise looks good to me.

[Dolph Mathews (dolph)]

ayoung: Ah, I see the date changes now (s/2012/2112/). That makes sense. +1 for patch.

On description: s/as long as the user wasn't disabled/as long as the token wasn't otherwise revoked/

[Eoghan Glynn (eglynn)]

@dolph

> On description: s/as long as the user wasn't disabled/as long as the token wasn't otherwise revoked/

That brings up interesting point.

If I'm reading the keystone code correctly, only *non-expired* revoked tokens are returned from Driver.list_revoked_tokens(), at least in the SQL case:

  https://github.com/openstack/keystone/blob/master/keystone/token/backends/sql.py#L128

So a token that has been both revoked *and* expired would not be reported as revoked (presumably the assumption here is that the token would fall in any case on the expiry-check hurdle, so no need to pollute the reported revoked list with really old tokens).

IIUC this makes the vulnerability a tad worse, as the effect of revocation is reversed if the token is re-used post-expiry.

[Eoghan Glynn (eglynn)]

I've confirmed that the effect of signed token revocation is reversed when the token is expires (as envisaged in the previous comment).

IMO this should be reflected in the impact description, e.g. by adding the line:

"The effect of signed token revocation is also reversed when the token expires, in the sense that a revoked token is once again treated as being valid."

[Thierry Carrez (ttx)]

This one is getting better and better :) New version:

---------------------------------------------------------
Title: Missing expiration check in Keystone PKI tokens validation
Reporter: Eoghan Glynn (Red Hat)
Products/Affects: Keystone (Folsom only), python-keystoneclient (0.2.0+)

Description:
Eoghan Glynn from Red Hat reported a vulnerability in expiry checks for PKI tokens in the Keystone authentication middleware. Expired tokens for authenticated users could continue to be used, potentially resulting in the bypass of intended security policies. The effect of PKI token revocation is also reversed when the token expires, in the sense that a revoked token is once again treated as being valid. Only setups using PKI tokens are affected.

Note:
The affected code was added to Keystone in the Folsom release, but was moved to python-keystoneclient during the Grizzly development cycle.

[Eoghan Glynn (eglynn)]

+1 on the latest impact description.

[Dolph Mathews (dolph)]

Wow.

+1 for description.

[Thierry Carrez (ttx)]

I don't think I have the formal +1 from two keystone-core devs yet. Could you comment to that effect ?

[Adam Young (ayoung)]

You have 2. I am one and Dolph is the other.

[Thierry Carrez (ttx)]

One could argue that I need two other than the patch author, but you guys decide (and Eoghan +1ed too).
Waiting for a CVE assignment, will probably push downstream early next week.

[Dolph Mathews (dolph)]

I asked bknudson to review.

[Brant Knudson (blk-u)]

I reviewed the patch in comment #3 and it looks good to me. +1.

[Brant Knudson (blk-u)]

I reviewed the statement in comment #18 and it looks good to me. +1

Only suggestion is I think "PKI token validation" reads better than "PKI tokens validation".

[Thierry Carrez (ttx)]

This is CVE-2013-2104

[Thierry Carrez (ttx)]

Sent downstream. Proposed public disclosure date/time: Tuesday, May 28, 1500UTC.

[Alex Meade (alex-meade)]

@Thierry Feel free to co-credit me on this since you asked :)

Glad I wasn't the first to find this. I like the keystone client fix in comment #3

[Thierry Carrez (ttx)]

@Alex: will add "Alex Meade (Rackspace)" as co-reporter on the final advisory.

[Adam Young (ayoung)]

I am going to need to update the patch once https://review.openstack.org/#/c/30147 is merged, as the certs in the unit tests will be different from the ones used to sign the tokens. The content of the patch will be same, with the exception of data in keystone/tests/signing

[Thierry Carrez (ttx)]

OK, please update the bug as soon as possible once that merges, will communicate updated patch to stakeholders.

[Thierry Carrez (ttx)]

@ayoung: Thanks for the rebase. It looks like the python-keystoneclient patch also needs a rebase on top of 11263ac3 (to account for changes in tests/test_auth_token_middleware.py). Any chance you could have a look ?

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/folsom
Review: https://review.openstack.org/30743

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/30742
Committed: http://github.com/openstack/python-keystoneclient/commit/8fe7a822d3fd9f435bb4a73a838b380659196cf7
Submitter: Jenkins
Branch: master

commit 8fe7a822d3fd9f435bb4a73a838b380659196cf7
Author: Adam Young <email address hidden>
Date: Tue May 28 09:50:51 2013 -0400

    Check Expiry

    Explicitly checks the expiry on the tokens, and rejects tokens that
    have expired

    had to regenerate the sample data for the tokens as they all had been
    generated with values that are now expired.

    bug 1179615

    Change-Id: Ie06500d446f55fd0ad67ea540c92d8cfc57483f4

[Matthew Thode (prometheanfire)]

Would it be possible to get a patch rebased against python-keystoneclient-0.2.3 (so I can get a fix out for it)?
also, keystone-folsom is fixed in gentoo

[Jamie Strandboge (jdstrand)]

Ubuntu is also interested in a patch against 0.2.3. Thanks!

[Thierry Carrez (ttx)]

FYI we are still investigating why the proposed stable/folsom patch fails integration tests, please stay put.

[Thierry Carrez (ttx)]

Failure reason found, slightly modified patch coming up

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/30743
Committed: http://github.com/openstack/keystone/commit/8d23da1302dde9d38bbc227d9aba30da919b60c8
Submitter: Jenkins
Branch: stable/folsom

commit 8d23da1302dde9d38bbc227d9aba30da919b60c8
Author: Adam Young <email address hidden>
Date: Mon May 13 16:07:51 2013 -0400

    Check token Expiration

    Backport for Folsom.

    Bug 1179615

    Change-Id: I8516d87ffc72cf35d3bff6fc21cb5324da4ad2bb

[Thierry Carrez (ttx)]

OSSA 2013-014

[Thierry Carrez (ttx)]

My try at a minimal backport over python-keystoneclient 0.2.3...

[Thierry Carrez (ttx)]

python-keystoneclient 0.2.4 (including this fix) is available on Pypi

[Matthew Thode (prometheanfire)]

Thanks for the release, makes it easier. I made a more full backport patch for python-keystoneclient-0.2.3 just now as well (not really tested, but here you go).

bug fully fixed in gentoo as of sys-auth/keystone-2012.2.4-r4 and dev-python/python-keystoneclient-0.2.4