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

[Le Tian Ren]

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?

[Jeremy Stanley (fungi)]

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

[Lance Bragstad (lbragstad)]

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

[Jeremy Stanley (fungi)]

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)]

Was able to recreate locally on master.

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)

[OpenStack Infra (hudson-openstack)]

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)]

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)]

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

[Stanislaw Pitucha (stanislaw-pitucha)]

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.