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

[Amrith Kumar]

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="***"

[Tristan Cacqueray (tristan-cacqueray)]

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 ?

[Tristan Cacqueray (tristan-cacqueray)]

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

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

[OpenStack Infra (hudson-openstack)]

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

[Amrith Kumar (amrith)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

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

[Thierry Carrez (ttx)]

Follow-up on bug 1377981

[Amrith Kumar (amrith)]

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

[Matt Riedemann (mriedem)]

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

[Matt Riedemann (mriedem)]

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

[Matt Riedemann (mriedem)]

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