Password visible in clear text in keystone.log when user created and keystone debug logging is enabled

Bug #1465922 reported by Le Tian Ren on 2015-06-17
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Bandit
Wishlist
Unassigned
OpenStack Identity (keystone)
Medium
Brant Knudson
Juno
Medium
Brant Knudson
Kilo
Medium
Brant Knudson
OpenStack Security Advisory
Undecided
Unassigned

Bug Description

grep CLEARTEXTPASSWORD keystone.log

2015-06-16 06:44:39.770 20986 DEBUG keystone.common.controller [-] RBAC: Authorizing identity:create_user(user={u'domain_id': u'default', u'password': u'CLEARTEXTPASSWORD', u'enabled': True, u'default_project_id': u'0175b43419064ae38c4b74006baaeb8d', u'name': u'DermotJ'}) _build_policy_check_credentials /usr/lib/python2.7/site-packages/keystone/common/controller.py:57

Issue code: https://github.com/openstack/keystone/blob/master/keystone/common/controller.py#L57

    LOG.debug('RBAC: Authorizing %(action)s(%(kwargs)s)', {
        'action': action,
        'kwargs': ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs])})

Shadow the values of sensitive fields like 'password' by some meaningless garbled text like "XXXXX" is one way to fix.

Well, in addition to this, I think we should never pass the 'password' with its original value along the code and save it in any persistence, instead we should convert it to a strong hash value as early as possible. With the help of a good hash system, we never have to need the original value of the password, right?

Le Tian Ren (gpanda+) on 2015-06-17
description: updated
Jeremy Stanley (fungi) wrote :

The OpenStack VMT currently considers (based on existing precedent) disclosure of sensitive information in debug-level logging a security hardening opportunity, and does not issue security advisories for it. This is class D in our report taxonomy: https://security.openstack.org/vmt-process.html#incident-report-taxonomy

information type: Private Security → Public
Changed in ossa:
status: New → Won't Fix
Lance Bragstad (lbragstad) wrote :

I believe the same it true in Keystone based on what Jeremy has linked above.

Changed in keystone:
status: New → Won't Fix
Jeremy Stanley (fungi) wrote :

Well, there's not necessarily a reason to avoid masking sensitive data in debug level logs as a security hardening measure, we just have enough cases of this already in various places along with documentation saying not to disclose debug logs to untrusted parties that we don't issue security advisories when yet another is discovered.

Brant Knudson (blk-u) wrote :

Was able to recreate locally on master.

Changed in keystone:
status: Won't Fix → Confirmed
importance: Undecided → Medium
assignee: nobody → Brant Knudson (blk-u)
Changed in keystone:
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/193703
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=c2c3a0ff86314bee3d62f69d30206ff7584f229f
Submitter: Jenkins
Branch: master

commit c2c3a0ff86314bee3d62f69d30206ff7584f229f
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:40:30 2015 -0500

    Add test showing password logged

    There was no test that showed that the password is logged when a
    user is created or admin changes user password.

    Change-Id: I5ffa04e9ac359355cff47a622731f1bf6a27ea7b
    Partial-Bug: #1465922

Reviewed: https://review.openstack.org/193695
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=fbdb100e656b19958589fa659bf9d95303e76ab8
Submitter: Jenkins
Branch: master

commit fbdb100e656b19958589fa659bf9d95303e76ab8
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:18:18 2015 -0500

    Mask passwords in debug log on user password operations

    When a user is created, they change their password, or admin
    changes their password and debug logging is enabled, the value of
    the user's password was logged. The value should be masked.

    Change-Id: I07b7441378fb630f01204d6b656b218f6b94dd5a
    Closes-Bug: #1465922

Changed in keystone:
status: In Progress → Fix Committed
Brant Knudson (blk-u) on 2015-07-13
tags: added: kilo-backport-potential

Fix proposed to branch: stable/kilo
Review: https://review.openstack.org/201323

Fix proposed to branch: stable/juno
Review: https://review.openstack.org/201329

Dolph Mathews (dolph) on 2015-07-14
tags: removed: keystone kilo-backport-potential

Reviewed: https://review.openstack.org/201322
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=fba2d5c15e298e0936800a0e3d1ff7588235c359
Submitter: Jenkins
Branch: stable/kilo

commit fba2d5c15e298e0936800a0e3d1ff7588235c359
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:40:30 2015 -0500

    Add test showing password logged

    There was no test that showed that the password is logged when a
    user is created or admin changes user password.

    Change-Id: I5ffa04e9ac359355cff47a622731f1bf6a27ea7b
    Partial-Bug: #1465922
    (cherry picked from commit c2c3a0ff86314bee3d62f69d30206ff7584f229f)

tags: added: in-stable-kilo

Reviewed: https://review.openstack.org/201323
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=c4dc1331e111f6fce070163129cef008a204e99f
Submitter: Jenkins
Branch: stable/kilo

commit c4dc1331e111f6fce070163129cef008a204e99f
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:18:18 2015 -0500

    Mask passwords in debug log on user password operations

    When a user is created, they change their password, or admin
    changes their password and debug logging is enabled, the value of
    the user's password was logged. The value should be masked.

    Change-Id: I07b7441378fb630f01204d6b656b218f6b94dd5a
    Closes-Bug: #1465922
    (cherry picked from commit fbdb100e656b19958589fa659bf9d95303e76ab8)

Changed in keystone:
milestone: none → liberty-2
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2015-10-15
Changed in keystone:
milestone: liberty-2 → 8.0.0

Reviewed: https://review.openstack.org/201328
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=a7037547fecf0998ea09b1139123c3c1ef97472d
Submitter: Jenkins
Branch: stable/juno

commit a7037547fecf0998ea09b1139123c3c1ef97472d
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:40:30 2015 -0500

    Add test showing password logged

    There was no test that showed that the password is logged when a
    user is created or admin changes user password.

    Conflicts:
     keystone/tests/unit/test_v3_identity.py

    Change-Id: I5ffa04e9ac359355cff47a622731f1bf6a27ea7b
    Partial-Bug: #1465922
    (cherry picked from commit c2c3a0ff86314bee3d62f69d30206ff7584f229f)
    (cherry picked from commit fba2d5c15e298e0936800a0e3d1ff7588235c359)

tags: added: in-stable-juno

Reviewed: https://review.openstack.org/201329
Committed: https://git.openstack.org/cgit/openstack/keystone/commit/?id=c15cbc48d63e9cb6a6994ffc73fded2464a8651c
Submitter: Jenkins
Branch: stable/juno

commit c15cbc48d63e9cb6a6994ffc73fded2464a8651c
Author: Brant Knudson <email address hidden>
Date: Fri Jun 19 14:18:18 2015 -0500

    Mask passwords in debug log on user password operations

    When a user is created, they change their password, or admin
    changes their password and debug logging is enabled, the value of
    the user's password was logged. The value should be masked.

    Conflicts:
     keystone/common/controller.py
     keystone/tests/unit/test_v3_identity.py

    Change-Id: I07b7441378fb630f01204d6b656b218f6b94dd5a
    Closes-Bug: #1465922
    (cherry picked from commit fbdb100e656b19958589fa659bf9d95303e76ab8)
    (cherry picked from commit c4dc1331e111f6fce070163129cef008a204e99f)

Robert Clark (robert-clark) wrote :

We would typically issue an OSSN for such behaviour, it's somewhat boilerplate but it's important to document the issue, particularly as a number of production workloads run in debug mode.

I also think it's interesting that Bandit didn't catch this, it's pretty good at finding these sorts of issues.

Robert Clark (robert-clark) wrote :

Added bandit bug so those guys can take a look to see if this can be caught in future.

For Bandit I'm marking "Won't fix":
Offending code is:

    LOG.debug('RBAC: Authorizing %(action)s(%(kwargs)s)', {
        'action': action,
        'kwargs': ', '.join(['%s=%s' % (k, kwargs[k]) for k in kwargs])})

There's no indication of what the kwargs are, so without a runtime integration (tainting) or symbolic execution this is not realistic to pick up. At the call sites the kwargs are not a literal dict, but rather values from up the call chain, so not easy to analyse.

It would be great to pick this up, but it's way out of reach for the engine currently or in reasonable future.

Changed in bandit:
status: New → Won't Fix
importance: Undecided → Wishlist
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers