Token with "placeholder" ID issued

Bug #1328067 reported by Steven Hardy
10
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Identity (keystone)
Won't Fix
Medium
Morgan Fainberg
keystonemiddleware
Fix Released
Medium
Jamie Lennox
python-keystoneclient
Fix Released
Medium
Jamie Lennox

Bug Description

We're seeing test failures, where it seems that an invalid token is issued, with the ID of "placeholder"

http://logs.openstack.org/69/97569/2/check/check-tempest-dsvm-full/565d328/logs/screen-h-eng.txt.gz

See context_auth_token_info which is being passed using the auth_token keystone.token_info request environment variable (ref https://review.openstack.org/#/c/97568/ which is the previous patch in the chain from the log referenced above).

It seems like auth_token is getting a token, but there's some sort of race in the backend which prevents an actual token being stored? Trying to use "placeholder" as a token ID doesn't work, so it seems like this default assigned in the controller is passed back to auth_token, which treats it as a valid token, even though it's not.

https://github.com/openstack/keystone/blob/master/keystone/token/controllers.py#L121

I'm not sure how to debug this further, as I can't reproduce this problem locally.

Dolph Mathews (dolph)
Changed in keystone:
importance: Undecided → Critical
Changed in python-keystoneclient:
importance: Undecided → Critical
Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

This bug is based upon an assumption that the decoded token data in the ENV that is provide by auth_token middleware has the correct token id. This assumption is incorrect.

The decoded token data from a v2 PKI token will always contain the id 'placeholder'. This is because the token_id is created before the document is signed, and is part of the signed document.

The bug is specifically caused in the review https://review.openstack.org/#/c/97569/ because the token id is sourced from the decoded PKI token document instead of directly from the request header X-Auth-Token.

The fix from the Keystone / Keystoneclient side is one of three options:

1. auth_token middleware can do something interesting and populate the correct location in the env variable for the token.

2. auth_token middleware can remove the field from the env variable to avoid confusion (better)

3. V2 tokens should not be issued with the token_id as part of the token document, but independent of the document itself (in both UUID and PKI cases). The token_id would be derived not from the env location but from the X-auth-token header. It would be possible to provide a friendly env-variable for grabbing the ID if desired (but not really required). This mirrors the way V3.0 builds the token document. [best solution]

This definitely needs to be fixed. It definitely needs to be fixed sooner rather than later. I am unsure if this is really a "critical" bug.

Revision history for this message
Brad Topol (btopol) wrote :

Option 2 seems simplest if it truly is "good enough"

Revision history for this message
Steven Hardy (shardy) wrote :

Hi Morgan,

Many thanks for the analysis - I've been testing with UUID tokens primarily, which explains why I've not been able to reproduce locally.

With the info provided I can probably work around the issue in 97569 by using the context (header) auth_token, like I'm already doing for v3 tokens.

From a consumer-of-keystoneclient perspective, it would certainly nice to reduce the amount of mangling required to reuse the token_info from the env as an auth_ref input - perhaps long-term we could have a different env var after the keystoneclient-auth-token BP is implemented, maybe a keystone.token_auth_ref variable, which would be more directly consumable as an argument to a keystoneclient.Client constructor?

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

Steven,

Absolutely agree. Making for less mangling is good.

I'm going to go ahead and drop the priority on this down some for now because it's more of a case of data is available where it shouldn't be (and it's wrong) not that it's used in it's current state.

We should fix this during the j development cycle (regardless of the direction of fix we go with)

Changed in keystone:
importance: Critical → Medium
Changed in python-keystoneclient:
importance: Critical → Medium
Changed in keystone:
status: New → Triaged
Changed in python-keystoneclient:
status: New → Triaged
Revision history for this message
Adam Young (ayoung) wrote :

Be aware that if you change the body of the token, you will get a different hash value if you attempt to hash it (duh) and that the hash value is used as the key in both the revocation list and in memcached. It is impossible to put the hashed value into the body of the token.

Changed in keystone:
milestone: none → juno-rc1
Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

This is not something we can likely fix in the Keystone server due to exactly what Adam commented on. And V2 tokens expect to contain the ID.

We can make auth_token not provide "placeholder" ever to the underlying service. (Valid bug)

Extracting the token_id from the body for V2 tokens when using PKI is likely incorrect when it comes to the client (if this is done).

Revision history for this message
Jamie Lennox (jamielennox) wrote :

It's not a complete fix by any means, but i've noticed this as well and made it possible to override the value of the token_id [1] in an AccessInfo. What i intend to do is always override the value with the one that comes in from the X-Subject-Token header in auth_token middleware. There is also a patch in review to pass through a full Auth Plugin from auth_token middleware that will correctly respect this [2].

So If you correctly consume the plugin or the headers coming from auth_token middleware and use the AccessInfo object rather than parse the token content yourself it should at least not cause any problems.

I don't think there is anything that we can do from a server perspective.

[1] https://review.openstack.org/#/c/113415/
[2] https://review.openstack.org/#/c/107222/

Changed in keystonemiddleware:
assignee: nobody → Jamie Lennox (jamielennox)
Changed in python-keystoneclient:
assignee: nobody → Jamie Lennox (jamielennox)
milestone: none → 0.11.0
status: Triaged → Fix Committed
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to keystone (master)

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

Changed in keystone:
assignee: nobody → Morgan Fainberg (mdrnstm)
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on keystone (master)

Change abandoned by Morgan Fainberg (<email address hidden>) on branch: master
Review: https://review.openstack.org/119673
Reason: This change cannot go in really. it's unfortunate but not API compat.

Revision history for this message
Morgan Fainberg (mdrnstm) wrote :

This would cause a breakage in the backwards compatibility of the Keystone API. The V2 token requires an id, however, under PKI tokens the id in the token body is part of the signing/hashing that is used to generate the token id. This means that we cannot have an accurate ID in the v2 token body.

When using PKI tokens do not use the id encoded in the token body.

Changed in keystone:
status: In Progress → Won't Fix
milestone: juno-rc1 → none
Changed in keystonemiddleware:
status: New → Fix Committed
milestone: none → 1.2.0
importance: Undecided → Medium
Dolph Mathews (dolph)
Changed in python-keystoneclient:
status: Fix Committed → Fix Released
Dolph Mathews (dolph)
Changed in keystonemiddleware:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.