Issues regarding application credentials

[Arjen]

While looking into the application credential API we came across several issues. Since they are all closely related I will file them under this issue:

- No secret strength requirements. To configure a password strength requirement for users, one can use `password_regex`. However, this is not possible for application credentials, which makes it possible to create a credentials with the secret 'a':

$ openstack application credential create test-secret-strength --secret a
+--------------+----------------------------------+
| Field | Value |
+--------------+----------------------------------+
| description | None |
| expires_at | None |
| id | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
| name | test-secret-strength |
| project_id | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
| roles | member reader |
| secret | a |
| system | None |
| unrestricted | False |
| user_id | xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx |
+--------------+----------------------------------+

To attack this, you'd still need to know the ID, but combined with https://bugs.launchpad.net/keystone/+bug/1901207 the impact of this issue is increased.

- No lockout feature. For normal login, the settings `lockout_failure_attempts` and `lockout_duration` are used. These do not affect the application credential API. This increases the attack surface unnecessarily in my opinion. Combined with weak secrets and https://bugs.launchpad.net/keystone/+bug/1901207 the probability of a successful attack is increased.

- Only part of secret is verified. It looks like only the first 72 characters of the secret of an application credential are used to verify it. Characters after that are not used in the verification. The default length of a secret seems to be 86 characters. Even though brute forcing 72 characters is still pretty impossible, this doesn't sound like intended behaviour to me.

[Jeremy Stanley (fungi)]

Since this report concerns a possible security risk, an incomplete
security advisory task has been added while the core security
reviewers for the affected project or projects confirm the bug and
discuss the scope of any vulnerability along with potential
solutions.

[Jeremy Stanley (fungi)]

Breaking down the list of issues here with proposed report classifications:

1. (D/security hardening) request to implement secret strength requirements for app credentials

2. (D/security hardening) request to implement brute-force mitigation via lockout for app credentials

3. (C1/impractical) app credentials are truncated to 72 characters prior to comparison

[ report taxonomy: https://security.openstack.org/vmt-process.html#incident-report-taxonomy ]

For #1 and #2 I'm assuming the Keystone docs don't claim application credentials provide these protections currently, and so they're effectively security-related feature requests. #3 could be construed as a defect worthy a CVE assignment, but as vulnerabilities go it's fairly impractical to exploit as you note, so I don't think we need to issue any advisory for it. Also it doesn't seem to me that any of these items need to be discussed in private under embargo, so we could switch this bug to public. Does anyone strongly disagree with the above assessment?

[Gage Hugo (gagehugo)]

Agreed with bug report and Jeremy, there doesn't seem to be anything directly exploitable here, we can make this public.

1 & 2 seem to be requests for security hardening similar to how keystone handles PCI-DSS features for user passwords. 3 might indeed be unintended behavior and should be investigated.

[Jeremy Stanley (fungi)]

Given nobody has objected to the proposed classifications in my comment #2 from October, I'll go ahead and mark our security advisory task Won't Fix for this. We can revisit the decision if anyone disagrees.

[Nick Tait (nickthetait)]

I'm with Gage on classifications. Should #3 be split into its own bug report for keystone?

[Nick Tait (nickthetait)]

Checking back in, #3 deserves a CVE IMO. Happy to assign that on Red Hat's behalf. WDYT Gague?

[Gage Hugo (gagehugo)]

Anyone can request a CVE, feel free to request one for this.

As Jeremy pointed out, #3 is pretty impossible to exploit, so we likely won't issue an advisory.

[Arjen (i2rcnasjfnk3)]

Even though successfully exploiting #3 is pretty unlikely, I personally do agree a CVE would be applicable. I feel it’d be appropriate if Red Hat would request it rather than myself.

[Nick Tait (nickthetait)]

CVE-2021-3563 has been assigned to #3.

Arjen, is it OK to list you as the reporter? What name should I use? Are you affiliated with an organization?

[Nick Tait (nickthetait)]

https://access.redhat.com/security/cve/CVE-2021-3563

[Nick Tait (nickthetait)]

Appreciate the report, I've added you.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.opendev.org/c/openstack/keystone/+/803641

[Lance Bragstad (lbragstad)]

I was able to verify the hash truncation issue using a functional test in keystone [0].

We can re-use that test moving forward to develop a fix.

[0] https://review.opendev.org/c/openstack/keystone/+/803641

[Luis Flores (luis-flores-ibm)]

There is an update about the fix for this vulnerability ?

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/828595
Committed: https://opendev.org/openstack/keystone/commit/3288af579de8ee312c36fb78ac9309ce8c554827
Submitter: "Zuul (22348)"
Branch: master

commit 3288af579de8ee312c36fb78ac9309ce8c554827
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 23.0.0.0rc1 release candidate.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/890417
Committed: https://opendev.org/openstack/keystone/commit/1b3536a7a4d72e7f7b95cc1874a450accad3ec8d
Submitter: "Zuul (22348)"
Branch: stable/zed

commit 1b3536a7a4d72e7f7b95cc1874a450accad3ec8d
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    Conflicts:
    * tox.ini

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
    (cherry picked from commit 3288af579de8ee312c36fb78ac9309ce8c554827)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/keystone/+/890661

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/890936
Committed: https://opendev.org/openstack/keystone/commit/6730c761d18aa547998f2add833c13f45f257fe7
Submitter: "Zuul (22348)"
Branch: master

commit 6730c761d18aa547998f2add833c13f45f257fe7
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Aug 9 20:41:05 2023 +0200

    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    Change-Id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/2023.1
Review: https://review.opendev.org/c/openstack/keystone/+/891115

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/zed
Review: https://review.opendev.org/c/openstack/keystone/+/891116

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/891115
Committed: https://opendev.org/openstack/keystone/commit/df54af90d03b14ebcd6e662bc8ece1fc52ea7c1d
Submitter: "Zuul (22348)"
Branch: stable/2023.1

commit df54af90d03b14ebcd6e662bc8ece1fc52ea7c1d
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Aug 9 20:41:05 2023 +0200

    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    Change-Id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/891116
Committed: https://opendev.org/openstack/keystone/commit/65f1fb6b4a54386f473369b05c8d10d77fb6710c
Submitter: "Zuul (22348)"
Branch: stable/zed

commit 65f1fb6b4a54386f473369b05c8d10d77fb6710c
Author: Dmitriy Rabotyagov <email address hidden>
Date: Wed Aug 9 20:41:05 2023 +0200

    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    Change-Id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/yoga
Review: https://review.opendev.org/c/openstack/keystone/+/892333

[OpenStack Infra (hudson-openstack)]

Change abandoned by "David Wilde <email address hidden>" on branch: stable/yoga
Review: https://review.opendev.org/c/openstack/keystone/+/892333
Reason: Looks like this backport is happening elsewhere

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/890661
Committed: https://opendev.org/openstack/keystone/commit/7852ca24a4eb86cb271ef7ec5e07f8f9c97f926d
Submitter: "Zuul (22348)"
Branch: stable/yoga

commit 7852ca24a4eb86cb271ef7ec5e07f8f9c97f926d
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length & Properly trimm bcrypt hashed passwords

    This is the squash of 2 patches related to bcrypt hashing settings.

    1.
    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    Conflicts:
    * tox.ini

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    2.
    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    original change id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)
    (cherry picked from commit 65f1fb6b4a54386f473369b05c8d10d77fb6710c)

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
    (cherry picked from commit 3288af579de8ee312c36fb78ac9309ce8c554827)
    (cherry picked from commit 1b3536a7a4d72e7f7b95cc1874a450accad3ec8d)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/xena
Review: https://review.opendev.org/c/openstack/keystone/+/892865

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/892865
Committed: https://opendev.org/openstack/keystone/commit/a38ba2a70cdf7e72c0f17bb80d895ccc1e39a010
Submitter: "Zuul (22348)"
Branch: stable/xena

commit a38ba2a70cdf7e72c0f17bb80d895ccc1e39a010
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length & Properly trimm bcrypt hashed passwords

    This is the squash of 2 patches related to bcrypt hashing settings.

    1.
    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    Conflicts:
    * tox.ini

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    2.
    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    original change id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)
    (cherry picked from commit 65f1fb6b4a54386f473369b05c8d10d77fb6710c)

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
    (cherry picked from commit 3288af579de8ee312c36fb78ac9309ce8c554827)
    (cherry picked from commit 1b3536a7a4d72e7f7b95cc1874a450accad3ec8d)
    (cherry picked from commit 7852ca24a4eb86cb271ef7ec5e07f8f9c97f926d)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/wallaby
Review: https://review.opendev.org/c/openstack/keystone/+/893549

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/893549
Committed: https://opendev.org/openstack/keystone/commit/11e1258ccda688fd6f1414ac664c50df56ca2989
Submitter: "Zuul (22348)"
Branch: stable/wallaby

commit 11e1258ccda688fd6f1414ac664c50df56ca2989
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length & Properly trimm bcrypt hashed passwords

    This is the squash of 2 patches related to bcrypt hashing settings.

    1.
    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    Conflicts:
    * tox.ini

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    2.
    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    original change id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)
    (cherry picked from commit 65f1fb6b4a54386f473369b05c8d10d77fb6710c)

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
    (cherry picked from commit 3288af579de8ee312c36fb78ac9309ce8c554827)
    (cherry picked from commit 1b3536a7a4d72e7f7b95cc1874a450accad3ec8d)
    (cherry picked from commit 7852ca24a4eb86cb271ef7ec5e07f8f9c97f926d)
    (cherry picked from commit a38ba2a70cdf7e72c0f17bb80d895ccc1e39a010)

[OpenStack Infra (hudson-openstack)]

Related fix proposed to branch: stable/victoria
Review: https://review.opendev.org/c/openstack/keystone/+/893944

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/c/openstack/keystone/+/893944
Committed: https://opendev.org/openstack/keystone/commit/0d5186cb49f452423240e8c6234ca2b753918fe7
Submitter: "Zuul (22348)"
Branch: stable/victoria

commit 0d5186cb49f452423240e8c6234ca2b753918fe7
Author: Dave Wilde (d34dh0r53) <email address hidden>
Date: Wed Feb 9 11:28:59 2022 -0600

    Force algo specific maximum length & Properly trimm bcrypt hashed passwords

    This is the squash of 2 patches related to bcrypt hashing settings.

    1.
    Force algo specific maximum length

    The bcrypt algorithm that we use for password hashing silently
    length limits the size of the password that is hashed giving the
    user a false sense of security [0]. This patch adds a check
    in the verify_length_and_trunc_password function for the hash in
    use and updates the max_length accordingly, this will override
    the configured value and log a warning if the password is truncated.

    Conflicts:
    * tox.ini

    [0]: https://passlib.readthedocs.io/en/stable/lib/passlib.hash.bcrypt.html#security-issues

    2.
    Properly trimm bcrypt hashed passwords

    bcrypt hashing algorythm has a limitation on length of passwords it
    can hash on 72 bytes. In [1] a password trimm to 54 symbols has been
    implemented, which resulted in password being invalidated after the
    keystone upgrade, since passwords are trimmed differently by bcrypt
    itself, as well as len(str()) is not always equal to
    len(str().encode()) as trimming should be done based on bytes and not
    string itself.

    With the change we return a byte object from
    `verify_length_and_trunc_password`, so it does not need to
    be encoded afterwards, since we need to strip based on bytes
    rather then on length of the string.

    [1] https://review.opendev.org/c/openstack/keystone/+/828595

    Closes-Bug: #2028809
    Related-Bug: #1901891
    original change id: Iea95a3c2df041a0046647b3d3dadead1a6d054d1
    (cherry picked from commit 6730c761d18aa547998f2add833c13f45f257fe7)
    (cherry picked from commit 65f1fb6b4a54386f473369b05c8d10d77fb6710c)

    Closes-bug: #1901891
    Change-Id: I8d0bb2438b23227b5a66b94af6f8e198084fcd8d
    (cherry picked from commit 3288af579de8ee312c36fb78ac9309ce8c554827)
    (cherry picked from commit 1b3536a7a4d72e7f7b95cc1874a450accad3ec8d)
    (cherry picked from commit 7852ca24a4eb86cb271ef7ec5e07f8f9c97f926d)
    (cherry picked from commit a38ba2a70cdf7e72c0f17bb80d895ccc1e39a010)
    (cherry picked from commit 11e1258ccda688fd6f1414ac664c50df56ca2989)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 21.0.1 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone 22.0.1 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone victoria-eom release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone wallaby-eom release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/keystone xena-eom release.