deleted stacks retain stored credentials

Bug #1266523 reported by Steven Hardy
10
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Heat
Fix Released
Medium
Steven Hardy

Bug Description

Since stacks became soft deleted, we no longer delete the contents of the user_creds record on stack delete, and we probably should as storing credentials for an indefinite period after the stack has been deleted seems wrong and we should have no need to use them.

So we should either modify the parser.Stack code to cope with loading a stack where no user_creds record exists, or overwrite the credentials with an empty context when the stack is deleted.

Revision history for this message
Clint Byrum (clint-fewbar) wrote : Re: [Bug 1266523] [NEW] deleted stacks retain stored credentials

If the only reason to do soft deletes is so that the stack is somehow
recoverable, seems like deleting the credentials would be a bad idea.

Otherwise, why do we do soft deletes?

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

The reason we do soft deletes is for to allow an audit-trail, ie so you can query events for deleted stacks.

AFAIK there are no plans to enable some sort of stack-undelete feature, and even if we did, the API request would have to contain credentials anyway, so we could re-store the credentials at that point.

My concern is that we don't want to store credentials for deleted stacks to minimise the impact if some sort of unthinkably horrible security bug happens - it would be bad enough having a compromise expose credentials related to active stacks, but imagine if somehow an attacker could access credentials for every user who's ever used the service?

This btw is another good reason to deprecate and eventually remove the deferred_auth_method=password functionality - using trusts is much more secure because the lifetime of the trust is the same as the stack (we delete the trust on stack delete), the scope of delegated privileges can be controlled via trusts_delegated_roles, and even if a stored trust id was obtained by an attacker, they can't use it because only the trustee can consume it.

Steven Hardy (shardy)
Changed in heat:
assignee: nobody → Steven Hardy (shardy)
milestone: none → icehouse-3
importance: Undecided → Medium
status: New → Triaged
Revision history for this message
Steve McLellan (sjmc7) wrote :

FWIW, I'm +1 for this for the reasons you (Steven) outline above - I can see use cases (and I have one at the moment) where auditing is less important than the security implications; in addition, the credentials become less useful the longer they're kept because users get deleted, passwords change etc. If auditing IS important, it's still not necessary to retain the full set of credentials.

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

@Steve McLellan - if you care about security, I'd strongly suggest evaluating the deferred_auth_method=trusts option, where you have much more granular control over what is delegated from the stack owner to the stack service user for deferred operations, and as mentioned above the impact of any third party obtaining the trust ID is infinitely smaller than them obtaining username/password credentials.

FWIW, I'm hoping we can make deferred_auth_method=trusts the default for Icehouse - it was merged during the Havana cycle and I believe it's stable enough (feedback/testing welcome :)

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to heat (master)

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

Changed in heat:
status: Triaged → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

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

Thierry Carrez (ttx)
Changed in heat:
milestone: icehouse-3 → icehouse-rc1
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to heat (master)

Reviewed: https://review.openstack.org/76928
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=609150dd9ccf29220085ab2533511f4bfb11cc54
Submitter: Jenkins
Branch: master

commit 609150dd9ccf29220085ab2533511f4bfb11cc54
Author: Steven Hardy <email address hidden>
Date: Wed Feb 26 11:20:19 2014 +0000

    Make user_creds_id a parser.Stack attribute

    Expose the user_creds_id column via a stack attribute, like all
    the other columns, this avoids loading another stack object from
    the DB just to retrieve the id, and also provides a convenient
    interface to update the id when required (via Stack.store())

    Partial-Bug: #1266523
    Change-Id: Id404f9402b24988266acd881d1e775aa3cfee60d

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/76929
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=5c511c36b8a7a7bd0891ea60d1403ee84b27e462
Submitter: Jenkins
Branch: master

commit 5c511c36b8a7a7bd0891ea60d1403ee84b27e462
Author: Steven Hardy <email address hidden>
Date: Thu Feb 27 13:51:24 2014 +0000

    fix DB API user_creds_get for non-existent ID

    If you pass a non-existent ID to the user_creds_get call, it throws
    an exception "TypeError: 'NoneType' object is not iterable", but it
    should probably return None to be consistent with other get methods

    Partial-Bug: #1266523
    Change-Id: I8887afa04d80b91ff4479c6ede3818da4d67d726

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/76930
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=d39ee0c3077886720775baad144f33b141c2ce3b
Submitter: Jenkins
Branch: master

commit d39ee0c3077886720775baad144f33b141c2ce3b
Author: Steven Hardy <email address hidden>
Date: Thu Feb 27 14:05:57 2014 +0000

    Add user_creds_delete to the DB API

    Partial-Bug: #1266523
    Change-Id: I8c49c6630ecaa856078146e032ba7a2fa61786d4

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Reviewed: https://review.openstack.org/76931
Committed: https://git.openstack.org/cgit/openstack/heat/commit/?id=5f79b9e43f0e7f484dc5cc91032b8ebf55c9abd1
Submitter: Jenkins
Branch: master

commit 5f79b9e43f0e7f484dc5cc91032b8ebf55c9abd1
Author: Steven Hardy <email address hidden>
Date: Thu Feb 27 18:21:38 2014 +0000

    Delete user_creds on stack delete

    Make the user_creds_id nullable and delete the user_creds record associated
    with a stack when we delete the stack. Since stacks aren't really deleted
    (only soft-deleted), this means the potentially sensitive credentials data
    will be permanently deleted, while leaving the stack itself for audit
    purposes (e.g listing events of a soft-deleted stack).

    The changes made here would also make it easier in future if we wish to
    validate the resources on create/update and only create the trust (or
    store credentials) when a stack actually contains resources which require
    deferred operations.

    Change-Id: Ibd4ea3e549fce4de8de0e8da501e459c421bb11f
    Closes-Bug: #1266523

Changed in heat:
status: In Progress → Fix Committed
Thierry Carrez (ttx)
Changed in heat:
status: Fix Committed → Fix Released
Thierry Carrez (ttx)
Changed in heat:
milestone: icehouse-rc1 → 2014.1
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.