RFE: block_device_info dict should have a password key rather than clear password

Bug #1321785 reported by Matt Riedemann on 2014-05-21
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Matt Riedemann
Matt Riedemann

Bug Description

See bug 1319943 and the related patch https://review.openstack.org/#/c/93787/ for details, but right now the block_device_info dict passed around in the nova virt driver can contain a clear text password for the auth_password key.

That bug and patch are masking the password when logged in the immediate known locations, but this could continue to crop up so we should change the design such that the block_device_info dict doesn't contain the password but rather a key to a store that nova can retrieve the password for use.

Comment from Daniel Berrange in the patch above:

"Long term I think we need to figure out a way to remove the passwords from any data dicts we pass around. Ideally the block device info would merely contain something like a UUID to identify a password, which Nova could use to fetch the actual password from a secure password manager service at time of use. Thus we wouldn't have to worry about random objects/dicts containing actual passwords. Obviously this isn't something we can do now, but could you file an RFE to address this from a design POV, because masking passwords at time of logging call is not really a viable long term strategy IMHO."

Changed in nova:
status: New → Confirmed
importance: Undecided → Wishlist
Matt Riedemann (mriedem) wrote :

Ideally we'd turn connection_info into a nova object and it would have a __repr__ method that would mask the values by default. Then we could use the ConnectionInfo object in the BlockDeviceMapping object and we wouldn't have to worry about this when things get logged.

tags: added: volumes
Matt Riedemann (mriedem) wrote :
Matt Riedemann (mriedem) on 2015-09-30
no longer affects: nova/icehouse
Matt Riedemann (mriedem) on 2015-10-09
Changed in oslo.versionedobjects:
status: New → Confirmed
importance: Undecided → Medium
assignee: nobody → Matt Riedemann (mriedem)
Matt Riedemann (mriedem) wrote :

oslo.versionedobjects change: https://review.openstack.org/#/c/233151/

Changed in oslo.versionedobjects:
status: Confirmed → In Progress
ChangBo Guo(gcb) (glongwave) wrote :

The fix for oslo.versionedobjects was merged.

Changed in oslo.versionedobjects:
status: In Progress → Fix Committed
Changed in oslo.versionedobjects:
milestone: none → 1.1.0
status: Fix Committed → Fix Released
Sean Dague (sdague) wrote :

I'm assuming we just pick this up from oslo.vo and there isn't a nova change?

Changed in nova:
status: Confirmed → Fix Released
Daniel Berrange (berrange) wrote :

The ovo change merely added a new SensitiveString field type. We still have to actually convert Nova to use that new field type where needed

Changed in nova:
status: Fix Released → Confirmed
Matt Riedemann (mriedem) on 2016-03-05
Changed in nova:
importance: Wishlist → Medium
assignee: nobody → Matt Riedemann (mriedem)
tags: added: unified-objects

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

Changed in nova:
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/288927
Committed: https://git.openstack.org/cgit/openstack/nova/commit/?id=db50aaca0acdf7814a8d6673545db94738907131
Submitter: Jenkins
Branch: master

commit db50aaca0acdf7814a8d6673545db94738907131
Author: Matt Riedemann <email address hidden>
Date: Sat Mar 5 13:40:52 2016 -0500

    Use SensitiveStringField for BlockDeviceMapping.connection_info

    bd977f400a1192d5cf7c2b52ef91615c0828813c added the SensitiveStringField
    to oslo.versionedobjects. SensitiveStringField is a StringField, which
    is what BlockDeviceMapping.connection_info was already using. The difference
    is that a SensitiveStringField masks passwords in the 'stringify' method,
    which is what's used when __repr__ is called on the object.

    Since BDM.connection_info can contain credentials, and the connection_info
    dict gets passed around quite a bit in the compute manager and virt drivers,
    it has from time to time gotten logged without first masking passwords.

    This makes the object handle masking the password so we don't have to do it
    explicitly anymore.

    There is no version bump on the BlockDeviceMapping object since nothing has
    functionally changed.

    Change-Id: I66a0b5f6834034e2fcbefc4510e3aa018edec310
    Closes-Bug: #1321785

Changed in nova:
status: In Progress → Fix Released

This issue was fixed in the openstack/nova release candidate.

To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers