Keystone token hashing is MD5

Bug #1174499 reported by Adam Young on 2013-04-29
22
This bug affects 2 people
Affects Status Importance Assigned to Milestone
OpenStack Dashboard (Horizon)
Low
Brant Knudson
OpenStack Identity (keystone)
Wishlist
Brant Knudson
django-openstack-auth
Wishlist
Brant Knudson
openstack-api-site
Undecided
Unassigned
python-keystoneclient
Wishlist
Morgan Fainberg

Bug Description

https://github.com/openstack/python-keystoneclient/blob/master/keystoneclient/common/cms.py

def cms_hash_token(token_id):
    """
return: for ans1_token, returns the hash of the passed in token
otherwise, returns what it was passed in.
"""
    if token_id is None:
        return None
    if is_ans1_token(token_id):
        hasher = hashlib.md5()
        hasher.update(token_id)
        return hasher.hexdigest()
    else:
        return token_id

MD5 is a deprecated mechanism, it should be replaces with at least SHA1, if not SHA256.
Keystone should be able to support multiple Hash types, and the auth_token middleware should query Keystone to find out which type is in use.

Thierry Carrez (ttx) wrote :

This is definitely a good strengthening action, but I don't think it qualifies as a vulnerability. MD5 is weaker than other hashing schemes, but practical collisions are still a bit hard to do, especially when you don't control the entirety of the cleartext.

If nobody complains, I'll open this bug and tag it "security" so that it gets wider attention, but it would not get an embargo or an OSSA.

Changed in keystone:
importance: Undecided → Wishlist
status: New → Confirmed
Adam Young (ayoung) wrote :

That was my take as well, just being cautious. Considering that the Hash is generated by an operation completely controlled by Keystone, I don't think it is possible to intentionally create a collision on the server. In auth_token middleware, the Hash is based on a PKI verified token, so, again, a random document will never be hashed. Thus, this is a hardening issue.

Thierry Carrez (ttx) wrote :

Agreed and opened

information type: Private Security → Public
tags: added: security
Robert Clark (robert-clark) wrote :

Would be nice to see support for multiple hash algorithms, certainly we shouldn't be using MD5 any more. While there might not be any obvious attack vectors we should look to harden this.

I don't think an OSSN is appropriate here either.

Bryan D. Payne (bdpayne) wrote :

I suspect that fixing this involves addressing some backwards compatibility changes too? Allowing a user to choose the hash alg would be nice, but I'm not sure if there are assumptions elsewhere based on hash length that could complication this.

Any thoughts on the "right" fix here from the Keystone core team?

Adam Young (ayoung) wrote :

First , Keystone needs a way to publish which hash algorithm it supports. auth_token middleware and other clients then can query this value and use it when requesting the tokens via the hash.

Kurt Seifried (kseifried) wrote :

One note:
http://cve.mitre.org/cgi-bin/cvekey.cgi?keyword=md5

Second note:
based on Adam Young's comments it sounds like even the ability to forge arbitrary MD5 hashes would not be directly exploitable as 1) the MD5 hashes are created by keystone and 2) the hashed value is a PKI token so there's an additional level pf protection. Is this correct?

Adam Young (ayoung) wrote :

Kurt, those statements are correct.

Kurt Seifried (kseifried) wrote :

Ok treating this as security hardening, no CVE will be assigned unless new information about exploitability becomes available.

Li Ma (nick-ma-z) on 2013-08-20
Changed in keystone:
assignee: nobody → Li Ma (nick-ma-b)
Li Ma (nick-ma-z) on 2013-08-20
Changed in keystone:
status: Confirmed → In Progress
Li Ma (nick-ma-z) on 2013-08-22
Changed in keystone:
assignee: Li Ma (nick-ma-b) → nobody
Li Ma (nick-ma-z) on 2013-08-22
Changed in keystone:
assignee: nobody → Li Ma (nick-ma-b)
Li Ma (nick-ma-z) wrote :

I suggest to just change MD5 to SHA1. It's not that important to provide multiple choices of hash algorithms.
(1) The hash function is used internally and not exposed to end-users.
(2) A stable and secure hash algorithm can deal with the situation very well.

According to the security and performance, SHA1 is the promisinig trade-off algorithm and it is also widely used in thousands of production-ready systems.

Bryan D. Payne (bdpayne) wrote :

Please do note use SHA-1. The appropriate choice is SHA-256 or SHA-512. Thanks!

Bryan D. Payne (bdpayne) wrote :

Sorry, that should say "please do not use SHA-1".

Jeffrey Walton (noloader) wrote :

> I suggest to just change MD5 to SHA1.
> ...
> According to the security and performance, SHA1 is the promising trade-off

Would it be possible to make it configurable? US Federal and a number of institutions with NIST like SOPs will want SHA256 or above. For them, its a question of compliance and not performance.

Paul McMillan (paul-mcmillan) wrote :

As Bryan said, we should use SHA256 or above, and avoid making it configurable. This will be complaint with institutional guidelines.

Donald Stufft (dstufft) wrote :

Add in another +1 for sha256 or above. There's really no reason to move to anything less than that. The "pain" is caused by the migration itself not the particular algorithm used. It's just as easy to move to sha256 (or higher) as it is sha1 and sha256+ have better safety margins.

Dolph Mathews (dolph) wrote :

+1 for SHA-256. I really don't want to migrate to SHA-1, and then migrate to something stronger next release anyway.

> The "pain" is caused by the migration itself not the particular algorithm used.

++

I believe the hashing algorithm is effectively a (undocumented?) part of the API, so backwards-compatibility will have to be maintained. This is NOT necessarily internal to OpenStack.

Changed in python-keystoneclient:
status: New → Confirmed
Dolph Mathews (dolph) wrote :

Resetting status because I'm not aware of anything in review? Please correct if I'm mistaken.

Changed in keystone:
status: In Progress → Confirmed
Changed in python-keystoneclient:
importance: Undecided → Wishlist
Li Ma (nick-ma-z) on 2013-08-23
Changed in python-keystoneclient:
assignee: nobody → Li Ma (nick-ma-b)
Dirk Mueller (dmllr) wrote :

I have a patch for this btw, I can post it for review if interested.

Li Ma (nick-ma-z) on 2013-08-29
Changed in keystone:
assignee: Li Ma (nick-ma-b) → nobody
Changed in python-keystoneclient:
assignee: Li Ma (nick-ma-b) → nobody
Dolph Mathews (dolph) wrote :

Happy to have them in review, but fixes for this should probably wait for Icehouse before merging.

Dirk Mueller (dmllr) wrote :

I'll post a patch for review shortly (want to finish testing it a real scenario first).

Changed in keystone:
assignee: nobody → Dirk Mueller (dmllr)

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

Changed in python-keystoneclient:
assignee: nobody → Dirk Mueller (dmllr)
status: Confirmed → In Progress

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

Changed in keystone:
status: Confirmed → In Progress
Li Ma (nick-ma-z) wrote :

Any progress here?

+1 add a configuration option to let users decide which algorithm to apply, and make it MD5 by default.

On 02/28/2014 09:06 PM, Li Ma wrote:
> Any progress here?
>
> +1 add a configuration option to let users decide which algorithm to
> apply, and make it MD5 by default.
>

Goal is to get away from on line token validation all together.

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

Changed in python-keystoneclient:
assignee: Dirk Mueller (dmllr) → Brant Knudson (blk-u)

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

Changed in keystone:
assignee: Dirk Mueller (dmllr) → Brant Knudson (blk-u)
Dolph Mathews (dolph) on 2014-03-25
Changed in python-keystoneclient:
milestone: none → 0.8.0
Dirk Mueller (dmllr) wrote :

Sorry, just came back from an extended leave, will refresh my patchset for this bug.

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

Dolph Mathews (dolph) on 2014-04-08
Changed in openstack-api-site:
status: New → Confirmed
tags: added: identity-api

Reviewed: https://review.openstack.org/86202
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=82359492dc14e679d48e6801da304027e508533c
Submitter: Jenkins
Branch: master

commit 82359492dc14e679d48e6801da304027e508533c
Author: Brant Knudson <email address hidden>
Date: Tue Apr 8 19:50:09 2014 -0500

    Hash functions support different hash algorithms

    The token hash functions always used MD5. With this change, the
    hash function can be passed in to the hash functions.

    SecurityImpact
    Related-Bug: #1174499

    Change-Id: Ia08c2d6252bb034087a244b47d5bcbea7dcfa70b

Dolph Mathews (dolph) on 2014-04-17
Changed in python-keystoneclient:
milestone: 0.8.0 → none

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

commit bf4ff96472991675f76c95dde8c027417d0deafd
Author: Brant Knudson <email address hidden>
Date: Wed Apr 9 19:13:09 2014 -0500

    Configurable token hash algorithm

    Tokens were always hashed with MD5. This change allows tokens to
    be hashed with SHA256 (or any other algorithm supported by the
    keystoneclient token hash function). This is for security
    hardening.

    There's a new configuration option 'hash_algorithm' in the [token]
    section. This is the algorithm to use for hashing PKI tokens, so is
    used

    a) when storing the token in the db
    b) as the hash in the revocation list

    hash_algorithm defaults to 'md5' for backwards compatibility.

    SecurityImpact
    DocImpact
    Closes-Bug: #1174499

    Change-Id: Iafe3c975d59818c8f362647f7ea5149a03deee47

Changed in keystone:
status: In Progress → Fix Committed

Reviewed: https://review.openstack.org/90251
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=bef7f497f0fdcb7d9f529c8b0a811d79b4465f3a
Submitter: Jenkins
Branch: master

commit bef7f497f0fdcb7d9f529c8b0a811d79b4465f3a
Author: Brant Knudson <email address hidden>
Date: Thu Apr 24 18:29:07 2014 -0500

    Enhance tests for auth_token middleware

    There was code in _verify_uuid_token that was not covered by unit
    tests. This change increases the coverage.

    Change-Id: I63e171a0a8e63ae599c967adc9ff09670063b807
    Related-Bug: #1174499

Reviewed: https://review.openstack.org/92021
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=f2adf271e719647b8e3f8bd13dce84a35dfcb932
Submitter: Jenkins
Branch: master

commit f2adf271e719647b8e3f8bd13dce84a35dfcb932
Author: Brant Knudson <email address hidden>
Date: Sun May 4 14:52:53 2014 -0500

    Fix client fixtures

    Some of the client fixtures used for testing were invalid. v2
    tokens must have 'access'/'token'/'expires', and v3 tokens must
    have 'token'/'expires_at'.

    Change-Id: I2614c7deed47c9758c2031418110108308634296
    Related-Bug: #1174499

Reviewed: https://review.openstack.org/92499
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=41b3abd42dbadab536bfd3793211ca001e7ffc8d
Submitter: Jenkins
Branch: master

commit 41b3abd42dbadab536bfd3793211ca001e7ffc8d
Author: Brant Knudson <email address hidden>
Date: Tue May 6 19:33:46 2014 -0500

    auth_token hashes PKI token once

    auth_token was hashing the PKI token multiple times. With this
    change, the token is hashed once.

    Change-Id: I70d3339d09deb2d3528f141d37138971038f4075
    Related-Bug: #1174499

Changed in python-keystoneclient:
assignee: Brant Knudson (blk-u) → Morgan Fainberg (mdrnstm)

Reviewed: https://review.openstack.org/80398
Committed: https://git.openstack.org/cgit/openstack/python-keystoneclient/commit/?id=22db04bb6bee3ab15a90510bb6c1780d2a254300
Submitter: Jenkins
Branch: master

commit 22db04bb6bee3ab15a90510bb6c1780d2a254300
Author: Brant Knudson <email address hidden>
Date: Tue May 6 19:36:59 2014 -0500

    auth_token middleware hashes tokens with configurable algorithm

    The auth_token middleware always hashed PKI Tokens with MD5. This
    change makes it so that PKI tokens can be hashed with SHA256 or any
    other algorithm supported by hashlib.new(). This is for security
    hardening.

    auth_token has a new config option 'hash_algorithms' that is set
    to the list of algorithms that will be used for hashing PKI tokens.
    This will typically be set to a single hash algorithm which must
    match the hash algorithm set in Keystone. Otherwise the tokens
    in the revocation list will not match, leading to revoked tokens
    being still usable.

    During a transition from one algorithm to another,
    'hash_algorithms' is set to both the new algorithm and the old
    algorithm. Both of the hash algorithms will be used to match
    against the revocation list and cache. Once the tokens using the
    old algorithm have expired the old algorithm can be removed from
    the list.

    'hash_algorithms' defaults to ['md5'] for backwards compatibility.

    DocImpact
    SecurityImpact
    Closes-Bug: #1174499

    Change-Id: Ie524125dc5f6f1076bfd47db3a414b178e4dac80

Changed in python-keystoneclient:
status: In Progress → Fix Committed
Dolph Mathews (dolph) on 2014-05-29
Changed in python-keystoneclient:
milestone: none → 0.9.0
Dolph Mathews (dolph) on 2014-05-29
Changed in python-keystoneclient:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2014-06-11
Changed in keystone:
milestone: none → juno-1
status: Fix Committed → Fix Released
Gary W. Smith (gary-w-smith) wrote :

Are any changes needed to horizon for this bug?

Adam Young (ayoung) wrote :

Yes, Horizon needs some chane, and I am not certain what the best approach is. Ideally, Horizon would get the Keystone configuration option, but Keystone has no way of publishing it to a remote server. The "Pass a hash" approach for Horizon was never intended to be a long term solution, and so I'm reluctant to put too much effort in to making it even more usable. I suspect that the best compromise is a configuration field for Horizon to establish the hash algorithm to use.

Gary W. Smith (gary-w-smith) wrote :

Thanks Adam. Marking the horizon as low priority in keeping with the priority of the other parts of this change, and due to the comments above about its low risk (such as #6 and #7).

Changed in horizon:
status: New → Confirmed
importance: Undecided → Low
Brant Knudson (blk-u) wrote :

Change proposed for django-openstack-auth : https://review.openstack.org/#/c/116509/

Changed in django-openstack-auth:
status: New → In Progress
assignee: nobody → Brant Knudson (blk-u)

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

Changed in horizon:
assignee: nobody → Brant Knudson (blk-u)
status: Confirmed → In Progress
Brant Knudson (blk-u) wrote :

Change proposed to horizon: https://review.openstack.org/#/c/116510/

Reviewed: https://review.openstack.org/116509
Committed: https://git.openstack.org/cgit/openstack/django_openstack_auth/commit/?id=ed1e31eca6cd34677feb6674973c4f8989b2b4e4
Submitter: Jenkins
Branch: master

commit ed1e31eca6cd34677feb6674973c4f8989b2b4e4
Author: Brant Knudson <email address hidden>
Date: Sat Aug 23 11:35:25 2014 -0500

    Configurable token hashing algorithm

    The user's authentication token was hashed using the MD5 algorithm.
    The MD5 algorithm shouldn't be used because of the potential for
    hash collisions. Some security standards mandate a SHA2 algorithm
    or better must be used.

    With this change the algorithm to use for hashing tokens can be
    configured by setting the OPENSTACK_TOKEN_HASH_ALGORITHM
    configuration option to a hash algorithm supported by Python's
    hashlib library[1]. For example, a deployer could set the option to
    'sha256' to meet a SHA2 security standard.

    The algorithm chosen must match the hash algorithm that the
    identity server is configured to use (Keystone and the auth_token
    middleware can be configured to use any hash algorithm supported by
    hashlib).

    This is for security hardening.

    [1] https://docs.python.org/2/library/hashlib.html

    DocImpact
    SecurityImpact

    Change-Id: I9e3eba7e0a12ae40a08d0ed851ea916ec6591bcc
    Closes-Bug: #1174499

Changed in django-openstack-auth:
status: In Progress → Fix Committed
Akihiro Motoki (amotoki) on 2014-09-20
Changed in django-openstack-auth:
milestone: none → 1.1.7
status: Fix Committed → Fix Released
importance: Undecided → Wishlist
Akihiro Motoki (amotoki) wrote :

The required fix in Horizon side is just an update of the example settings file and the documentation of the new parameter.
Thus I think it is a good candidate for Juno-RC2 (we already plan RC2 for translation import).

tags: added: juno-rc-potential

Reviewed: https://review.openstack.org/116510
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=372d033d89c0f5d305959a6ad5fd3e1159cc91ed
Submitter: Jenkins
Branch: master

commit 372d033d89c0f5d305959a6ad5fd3e1159cc91ed
Author: Brant Knudson <email address hidden>
Date: Sun Aug 24 10:04:10 2014 -0500

    Document token hash algorithm option

    With https://review.openstack.org/#/c/116509/ ,
    django-openstack-auth will support a new option for the token hash
    algorithm. This adds the documentation to Horizon's local settings
    example file.

    This is for security hardening. The token hash algorithm defaults
    to MD5, which is considered too weak due to the potential for hash
    collisions. Some security standards require a SHA2 hash algorithm to
    be used.

    DocImpact
    SecurityImpact

    Change-Id: I6774b9b7215d191259586e4721e357487bb777cd
    Closes-Bug: #1174499

Changed in horizon:
status: In Progress → Fix Committed
Akihiro Motoki (amotoki) on 2014-10-10
Changed in horizon:
milestone: none → kilo-1
Thierry Carrez (ttx) on 2014-10-10
Changed in horizon:
milestone: kilo-1 → juno-rc2
Thierry Carrez (ttx) on 2014-10-10
tags: removed: juno-rc-potential security

Reviewed: https://review.openstack.org/127452
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=3a64723917366eff4d8896b2b2d3d82fa462d25d
Submitter: Jenkins
Branch: proposed/juno

commit 3a64723917366eff4d8896b2b2d3d82fa462d25d
Author: Brant Knudson <email address hidden>
Date: Sun Aug 24 10:04:10 2014 -0500

    Document token hash algorithm option

    With https://review.openstack.org/#/c/116509/ ,
    django-openstack-auth will support a new option for the token hash
    algorithm. This adds the documentation to Horizon's local settings
    example file.

    This is for security hardening. The token hash algorithm defaults
    to MD5, which is considered too weak due to the potential for hash
    collisions. Some security standards require a SHA2 hash algorithm to
    be used.

    DocImpact
    SecurityImpact

    Change-Id: I6774b9b7215d191259586e4721e357487bb777cd
    Closes-Bug: #1174499
    (cherry picked from commit 372d033d89c0f5d305959a6ad5fd3e1159cc91ed)

Changed in horizon:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2014-10-16
Changed in keystone:
milestone: juno-1 → 2014.2
Thierry Carrez (ttx) on 2014-10-16
Changed in horizon:
milestone: juno-rc2 → 2014.2

Change abandoned by Matthias Runge (<email address hidden>) on branch: master
Review: https://review.openstack.org/128916
Reason: this is ancient and has been merged a looooong time ago.

Diane Fleming (diane-fleming) wrote :

Not clear what needs to change in the API docs.

Changed in openstack-api-site:
status: Confirmed → Won't Fix
status: Won't Fix → Incomplete

Reviewed: https://review.openstack.org/128916
Committed: https://git.openstack.org/cgit/openstack/horizon/commit/?id=36254d20706cebecf70048e7272bbc8bab8ade10
Submitter: Jenkins
Branch: master

commit 3a64723917366eff4d8896b2b2d3d82fa462d25d
Author: Brant Knudson <email address hidden>
Date: Sun Aug 24 10:04:10 2014 -0500

    Document token hash algorithm option

    With https://review.openstack.org/#/c/116509/ ,
    django-openstack-auth will support a new option for the token hash
    algorithm. This adds the documentation to Horizon's local settings
    example file.

    This is for security hardening. The token hash algorithm defaults
    to MD5, which is considered too weak due to the potential for hash
    collisions. Some security standards require a SHA2 hash algorithm to
    be used.

    DocImpact
    SecurityImpact

    Change-Id: I6774b9b7215d191259586e4721e357487bb777cd
    Closes-Bug: #1174499
    (cherry picked from commit 372d033d89c0f5d305959a6ad5fd3e1159cc91ed)

commit f1a8abb9250f158ba1f04cf2055f717e78ef8184
Author: Akihiro Motoki <email address hidden>
Date: Sun Oct 5 03:53:51 2014 +0900

    Warn OPENSTACK_QUANTUM_NETWORK setting as deprecated

    Change-Id: If2f762fe665b9a88153a77a658f52bcd56185c53
    Closes-Bug: #1377498
    (cherry picked from commit 530e5fee789ce5ed19d90a6b4901f01e8efde5ff)

commit 9b0ba951c07af13aa4c386b19876474b971e7946
Author: Akihiro Motoki <email address hidden>
Date: Sun Oct 5 14:23:43 2014 +0900

    Import translations from Transifex for Juno

    * Import ~100% completed translations
      (translations available for 12 languages)
    * Update language list in openstack_dashboard settings.py
    * Update English POT files
    * Update Transifex resource name in .tx/config for Juno.
    * Remove compiled message catalogs (Related-Bug: #1196982)

    Closes-Bug: #1376542

    The instruction on how to compile message catalogs will be
    covered by https://review.openstack.org/#/c/126169/
    or the installation guide.

    Change-Id: Ib36562168009fa34b9818e99154df350678abd4b

Atsushi SAKAI (sakaia) wrote :

I agree Diane's #53 Opinion.
It should be handled on Config Reference.
Also this option "hash_algorithm" is already fixed.

Changed in openstack-api-site:
status: Incomplete → Invalid
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers