Revoking an unscoped token does not revoke all tokens scoped from the unscoped token

[Richard]

If you create an unscoped token (A) and you then use token A to
create a project-scoped token (B) you now have
token (A) [audit_id] = audit_id_a
token (A) [audit_chain_id] = [audit_id_a]

token (B) [audit_id] = audit_id_b
token (B) [audit_chain_id] = [audit_id_a, audit_id_b]

If you Revoke(token A) then token B should also be invalid.
However, this is not the case currently as there are two reasons
for this.
There is a bug that doesn't correctly catch this in revoke_models
because it accidently changes a list to a list in a tuple:
https://github.com/openstack/keystone/blob/master/keystone/models/revoke_model.py#L200-L201
This needs to have the comma removed from
not in (token_values['audit_chain_id'],) to not in (token_values['audit_chain_id'])

The second and main reason is because this functionality is never exposed to the user
and in the code it is never run here:
https://github.com/openstack/keystone/blob/master/keystone/token/provider.py#L255-L277
because revoke_chain=False in the parameter is never set to True in a call anywhere in
the code.

[Jeremy Stanley (fungi)]

Since this report concerns a possible security risk, an incomplete security advisory task has been added while the core security reviewers for the affected project or projects confirm the bug and discuss the scope of any vulnerability along with potential solutions.

[Tristan Cacqueray (tristan-cacqueray)]

@keystone-coresec, please triage this bug report.

[Lance Bragstad (lbragstad)]

I would triage this as a Class D vulnerability. Keystone never implemented the logic to revoke a token based on a revoke_chain (i.e. revoke(A) resulting in token B being invalid because it contains token A's audit_id in its list of audit_ids).

Fixing this bug would require keystone to add functionality for how tokens are revoked, which wouldn't be backportable. I think this would be a great feature request that would act as a hardening opportunity for keystone's current revocation model.

I'll let other coresec folks weigh in on this.

[Morgan Fainberg (mdrnstm)]

On a not of history, we did implement this at one point and it broke
OpenStack in a ton of ways, as when someone logs out of horizon, it revoked
all tokens and long running tasks failed.

This could be improved, but it make break things again

On May 25, 2017 3:11 PM, "Lance Bragstad" <email address hidden> wrote:

> I would triage this as a Class D vulnerability. Keystone never
> implemented the logic to revoke a token based on a revoke_chain (i.e.
> revoke(A) resulting in token B being invalid because it contains token
> A's audit_id in its list of audit_ids).
>
> Fixing this bug would require keystone to add functionality for how
> tokens are revoked, which wouldn't be backportable. I think this would
> be a great feature request that would act as a hardening opportunity for
> keystone's current revocation model.
>
> I'll let other coresec folks weigh in on this.
>
> --
> You received this bug notification because you are a member of Keystone
> Core security contacts, which is subscribed to the bug report.
> Matching subscriptions: Private security bugs
> https://bugs.launchpad.net/bugs/1680911
>
> Title:
> Revoking an unscoped token does not revoke all tokens scoped from the
> unscoped token
>
> Status in OpenStack Identity (keystone):
> New
> Status in OpenStack Security Advisory:
> Incomplete
>
> Bug description:
> This issue is being treated as a potential security risk under
> embargo. Please do not make any public mention of embargoed (private)
> security vulnerabilities before their coordinated publication by the
> OpenStack Vulnerability Management Team in the form of an official
> OpenStack Security Advisory. This includes discussion of the bug or
> associated fixes in public forums such as mailing lists, code review
> systems and bug trackers. Please also avoid private disclosure to
> other individuals not already approved for access to this information,
> and provide this same reminder to those who are made aware of the
> issue prior to publication. All discussion should remain confined to
> this private bug report, and any proposed fixes should be added to the
> bug as attachments.
>
> If you create an unscoped token (A) and you then use token A to
> create a project-scoped token (B) you now have
> token (A) [audit_id] = audit_id_a
> token (A) [audit_chain_id] = [audit_id_a]
>
> token (B) [audit_id] = audit_id_b
> token (B) [audit_chain_id] = [audit_id_a, audit_id_b]
>
> If you Revoke(token A) then token B should also be invalid.
> However, this is not the case currently as there are two reasons
> for this.
> There is a bug that doesn't correctly catch this in revoke_models
> because it accidently changes a list to a list in a tuple:
> https://github.com/openstack/keystone/blob/master/keystone/
> models/revoke_model.py#L200-L201
> This needs to have the comma removed from
> not in (token_values['audit_chain_id'],) to not in
> (token_values['audit_chain_id'])
>
> The second and main reason is because this functionality is never
> exposed to the user
> and in the code it is never run here:
> https://github.com/openstack/keystone/blob/master/keystone/
> token/provide...

[Tristan Cacqueray (tristan-cacqueray)]

Subscribing ossg-coresec for embargo exception (https://security.openstack.org/vmt-process.html#embargo-exceptions)

In the event of a manual revocation, maybe there could be a process to also revoke all children scoped token that could be documented as a Security Note?

[Luke Hinds (lhinds)]

Tristian, based on Morgan and Lance's comments, an embargo exception is OK by me.

[Lance Bragstad (lbragstad)]

Following up here, is there any outstanding action items on behalf of keystone to take here? Outside of someone signing up or pursuing the feature work?

[Jeremy Stanley (fungi)]

I don't think so, looks like we just missed lifting the embargo. Based on comments above, I'll do that now. Thanks for the reminder.

[Jeremy Stanley (fungi)]

This report is being treated as a security hardening opportunity, class D in the OpenStack VMT report taxonomy: https://security.openstack.org/vmt-process.html#incident-report-taxonomy

[Morgan Fainberg (mdrnstm)]

Marking this as wont fix. This really is not something we can address in a meaningful way at this time. It expands through a huge set of issues across all of openstack and is not in line with the direction of the project now.

[Morgan Fainberg (mdrnstm)]

Especially since we have eliminated most cases of revoke-by-id. Revocation is easier to handle in the cases of revoke-by-user-at-time, etc.