Make the checks in strutils.mask_password more secure (CVE-2014-7231)

Bug #1345233 reported by Amrith Kumar on 2014-07-19
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Compute (nova)
Undecided
Unassigned
Havana
Undecided
Unassigned
Icehouse
Undecided
Unassigned
oslo.utils
Medium
Amrith Kumar

Bug Description

Relates to findings while fixing https://bugs.launchpad.net/ossa/+bug/1343604

mask_password() needs to be more robust and catch many more common formats of strings that could include passwords.

An example is that it does not catch something like '--password=top-secret' but does catch '--password="top-secret"'. See below; the logged messages are being generated by using mask_password().

/usr/sbin/mysqld --password=top-secret

2014-07-19 18:35:01.415 20588 ERROR openstack.common.processutils [-] Running cmd (subprocess): /usr/sbin/mysqld --password=secret

They did catch

/usr/sbin/mysqld --password="top-secret"

2014-07-19 18:35:48.686 20605 ERROR openstack.common.processutils [-] Running cmd (subprocess): /usr/sbin/mysqld --password="***"

CVE References

Amrith Kumar (amrith) on 2014-07-20
Changed in oslo:
assignee: nobody → Amrith (amrith)
Changed in ossa:
assignee: nobody → Amrith (amrith)

We are going to take care of the OSSA task within the former report:
https://bugs.launchpad.net/ossa/+bug/1343604

Can we mark this bug as a duplicate of 1343604 ?

Changed in ossa:
assignee: Amrith (amrith) → nobody

Oh forget about the duplicate idea, this is another issue. We may cover both within a single OSSA though.

@Amrith, while those two bugs where under private review here in LP you may have wait for a disclosure time before pushing fix to gerrit.

Now that the former referenced bug report is public, and as it contains all the details of this strutils.mask_password bug, I think it's better to open this report as well.

Jeremy Stanley (fungi) wrote :

Yes, this bug should be marked public-security rather than private-security, since the vulnerability was already explicitly mentioned publicly in bug 1343604.

Thierry Carrez (ttx) on 2014-07-28
information type: Private Security → Public Security
Changed in ossa:
status: New → Confirmed
importance: Undecided → Medium
Changed in ossa:
assignee: nobody → Tristan Cacqueray (tristan-cacqueray)
Thierry Carrez (ttx) on 2014-07-31
Changed in ossa:
status: Confirmed → Triaged
Changed in oslo:
importance: Undecided → Medium
status: New → Triaged

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

Changed in oslo:
status: Triaged → In Progress

Tristan, Doug,

I've pushed a partial fix for this bug. I would like to fix it more thoroughly but in the interest of time, and of getting something into Juno, I've pushed a partial.

Thanks,

-amrith

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

Changed in oslo:
assignee: Amrith (amrith) → Davanum Srinivas (DIMS) (dims-v)

Reviewed: https://review.openstack.org/114614
Committed: https://git.openstack.org/cgit/openstack/oslo.utils/commit/?id=e0425691d90bce0bbe847a9ff49468ce0fab5486
Submitter: Jenkins
Branch: master

commit e0425691d90bce0bbe847a9ff49468ce0fab5486
Author: Davanum Srinivas <email address hidden>
Date: Fri Aug 15 13:53:57 2014 -0400

    Make strutils.mask_password more secure

    Make some enhancements to strutils.mask_password to allow it to catch
    more cases of passwords in strings. Test cases have been added to test
    for these newly added situations.

    The following is a listing of patterns that will be handled. The
    keyword that mask_password uses (a list of four now) is represented by
    <key> and the password is shown as <password>. Quotes (both single and
    double) are represented as <quote>.

    --<key> <password>
    --<key> <quote><password><quote>
    <key> = <password>
    <key> = <quote><password><quote>

    All existing tests and patterns are still handled.

    Originally submitted in If5ea2d91b1d87c995f50d07a1281879493bd7adb

    Change-Id: Ifa9a753821484defb5784b136470e3a78ebed3e3
    Partial-Bug: #1345233

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

Changed in oslo:
assignee: Davanum Srinivas (DIMS) (dims-v) → Amrith (amrith)

Reviewed: https://review.openstack.org/113407
Committed: https://git.openstack.org/cgit/openstack/oslo-incubator/commit/?id=66142c3471fec8a4903395514f6bf55e3c54344f
Submitter: Jenkins
Branch: master

commit 66142c3471fec8a4903395514f6bf55e3c54344f
Author: Amrith Kumar <email address hidden>
Date: Mon Aug 11 16:29:31 2014 -0400

    Make strutils.mask_password more secure

    Make some enhancements to strutils.mask_password to allow it to catch
    more cases of passwords in strings. Test cases have been added to test
    for these newly added situations.

    The following is a listing of patterns that will be handled. The
    keyword that mask_password uses (a list of four now) is represented by
    <key> and the password is shown as <password>. Quotes (both single and
    double) are represented as <quote>.

    --<key> <password>
    --<key> <quote><password><quote>
    <key> = <password>
    <key> = <quote><password><quote>

    All existing tests and patterns are still handled.

    Change-Id: If5ea2d91b1d87c995f50d07a1281879493bd7adb
    Partial-Bug: #1345233

Change abandoned by amrith (<email address hidden>) on branch: master
Review: https://review.openstack.org/115618
Reason: abandoned based on Yuriy's comment.

affects: oslo-incubator → oslo.utils
Amrith Kumar (amrith) wrote :

I've manually marked this as "Fix Released" because the change appears to have made it to all the paces where we need it (for now). The changes had been submitted with "Partial Fix" and therefore LP didn't do this automatically (I'm guessing).

Changed in oslo.utils:
status: In Progress → Fix Released
summary: - Make the checks in strutils.mask_password more secure
+ Make the checks in strutils.mask_password more secure (CVE-2014-7231)
Matt Riedemann (mriedem) wrote :

The oslo-sync for nova in juno to fix this bug was: https://review.openstack.org/#/c/116981/

Matt Riedemann (mriedem) wrote :

The oslo-sync for nova in stable/icehouse to fix this bug was: https://review.openstack.org/#/c/121383/

Matt Riedemann (mriedem) wrote :

The backport / sync for nova in stable/havana to fix this bug was: https://review.openstack.org/#/c/121096/

Alan Pevec (apevec) on 2015-06-17
Changed in nova:
status: New → Invalid
no longer affects: nova/juno
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers