SHA1 signatures are weak, use SHA256 in docs

Bug #1733634 reported by Brian King
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Undecided
Unassigned

Bug Description

This bug tracker is for errors with the documentation, use the following as a template and remove or add fields as you see fit. Convert [ ] into [x] to check boxes:

- [X] This doc is inaccurate in this way: ______
- [ ] This is a doc addition request.
- [ ] I have a fix to the document that I can paste below including example: input and output.

Hello,

SHA1 signatures are considered weak nowadays. I believe Swift has support for using SHA256 signatures to generate TempURLs (although this should be independently verified before updating). Recommend updating the docs to use SHA256 once this is confirmed as a feature in Swift.

-----------------------------------
Release: 2.16.1.dev7 on 2017-11-20 14:15
SHA: 3135878d2fe9909f49fcadeeb9cc6c6933d06127
Source: https://git.openstack.org/cgit/openstack/swift/tree/doc/source/api/temporary_url_middleware.rst
URL: https://docs.openstack.org/swift/latest/api/temporary_url_middleware.html

Revision history for this message
Samuel Merritt (torgomatic) wrote :

I don't see SHA256 support in TempURL. As far as I see, it's SHA-1 only.

Revision history for this message
Brian King (inflatador) wrote :

Interesting. I confirmed that TempURL SHA256 signatures do in fact work with Rackspace Cloud Files , which is based on Swift.

Does anyone have access to a vanilla Swift install with TempURL enabled so they can confirm or deny whether or not it works?

Revision history for this message
Tim Burke (1-tim-z) wrote :

Yeah, sha256 definitely isn't supported -- the only place we even mention it in code is in the encryption-of-object-data-at-rest feature. Got any docs for the cloud files work? https://developer.rackspace.com/docs/cloud-files/v1/use-cases/public-access-to-your-cloud-files-account/#tempurl only seems to mention sha1. I'd be interested in adding support for other, stronger digests (with an eye toward deprecating and eventually getting rid of sha1-based signatures), but I'd hate to come up with an incompatible API "just because".

Really, we probably should have started this work a while ago -- the writing's been on the wall for years: https://www.schneier.com/blog/archives/2012/10/when_will_we_se.html

Revision history for this message
Brian King (inflatador) wrote :

@Tim I'll try and get the Cloud Files docs updatesd I wrote a script to test the use of sha256 in Rackspace Cloud Files and it seemed to work ( https://github.com/inflatador/ptemper ).

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (master)

Related fix proposed to branch: master
Review: https://review.openstack.org/525770

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (master)

Reviewed: https://review.openstack.org/525770
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=5a4d3bdfc49d61ee6e333c8eb1dad4383ae9308b
Submitter: Zuul
Branch: master

commit 5a4d3bdfc49d61ee6e333c8eb1dad4383ae9308b
Author: Tim Burke <email address hidden>
Date: Tue Dec 5 20:19:37 2017 +0000

    tempurl: Make the digest algorithm configurable

    ... and add support for SHA-256 and SHA-512 by default. This allows us
    to start moving toward replacing SHA-1-based signatures. We've known
    this would eventually be necessary for a while [1], and earlier this
    year we've seen SHA-1 collisions [2].

    Additionally, allow signatures to be base64-encoded, provided they start
    with a digest name followed by a colon. Trailing padding is optional for
    base64-encoded signatures, and both normal and "url-safe" modes are
    supported. For example, all of the following SHA-1 signatures are
    equivalent:

       da39a3ee5e6b4b0d3255bfef95601890afd80709
       sha1:2jmj7l5rSw0yVb/vlWAYkK/YBwk=
       sha1:2jmj7l5rSw0yVb/vlWAYkK/YBwk
       sha1:2jmj7l5rSw0yVb_vlWAYkK_YBwk=
       sha1:2jmj7l5rSw0yVb_vlWAYkK_YBwk

    (Note that "normal" base64 encodings will require that you url encode
    all "+" characters as "%2B" so they aren't misinterpretted as spaces.)

    This was done for two reasons:

       1. A hex-encoded SHA-512 is rather lengthy at 128 characters -- 88
          isn't *that* much better, but it's something.
       2. This will allow us to more-easily add support for different
          digests with the same bit length in the future.

    Base64-encoding is required for SHA-512 signatures; hex-encoding is
    supported for SHA-256 signatures so we aren't needlessly breaking from
    what Rackspace is doing.

    [1] https://www.schneier.com/blog/archives/2012/10/when_will_we_se.html
    [2] https://security.googleblog.com/2017/02/announcing-first-sha1-collision.html

    Change-Id: Ia9dd1a91cc3c9c946f5f029cdefc9e66bcf01046
    Related-Bug: #1733634

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/deep)

Related fix proposed to branch: feature/deep
Review: https://review.openstack.org/541233

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/deep)
Download full text (12.3 KiB)

Reviewed: https://review.openstack.org/541233
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=e473955900dde31dc170bf485e718fdf65955650
Submitter: Zuul
Branch: feature/deep

commit d9eec63a588e1e1e574b71d67b5d7456c3ad468d
Author: James E. Blair <email address hidden>
Date: Wed Jan 24 17:05:13 2018 -0800

    Zuul: Remove project name

    Zuul no longer requires the project-name for in-repo configuration.
    Omitting it makes forking or renaming projects easier.

    Change-Id: I2226881d18e69bf25ad93ee8b8db67248e14f697

commit f4cfe81e593b52e11e67916073a050cd2dde2e00
Author: John Dickinson <email address hidden>
Date: Tue Jan 16 14:51:29 2018 -0800

    authors/changelog updates for 2.17.0 release

    Change-Id: I577d169022916676a20a9ac24c7cc7b63ae46778

commit 8140b7e7ad8eaef0ca110c0f5e185a046b69fd6f
Author: Kazuhiro MIYAHARA <email address hidden>
Date: Thu Feb 1 09:43:46 2018 +0000

    Fix inconsistency of account info in expirer's unit tests

    In expirer's unit tests, FakeInternalClient instances simulates
    expirer's task queue behavior. But get_account_info method of
    the FakeInternalClient returns container count = 1 and object
    count = 2, even if it simulate different count of containers or
    objects.

    This patch fixes the behavior. The return values of get_account_info
    will be equal to simulated container and object counts.

    This patch will make review for expirer's task queue upgrade patch [1]
    more easy.

    [1]: https://review.openstack.org/#/c/517389

    Change-Id: Id5339ea7e10e4577ff22daeb91ec90f08704c98d

commit 924a043c70ba38c5d81758b5e0c29fa73674404a
Author: Samuel Merritt <email address hidden>
Date: Wed Jan 31 16:40:21 2018 -0800

    Remove some cruft from ratelimit tests

    The tests were carefully setting up a mock for the 'http_connect'
    function in the ratelimit module, but there is no such function
    imported by the ratelimit module.

    As far as I can tell, this has been the case since the ratelimit
    middleware first appeared in 72d40bd (Mon Oct 4 14:11:48 2010 -0700).

    Change-Id: If047184c6435aa1647050f50b499dc9feff4318d

commit 5cac37284e1d3f3019309ca86f73e3dbb176df59
Author: Kazuhiro MIYAHARA <email address hidden>
Date: Thu Jan 25 10:49:00 2018 +0000

    Refactor expirer unit tests

    In expirer's unit tests, fake InternalClient classes are defined
    and its instance simulates expirer's task queue behaviors.

    To make review for expirer's task queue update patch [1] easy,
    this patch refactors the implementation of the fake InternalClient
    classes. In this patch, unit tests are refactored by the following
    two approaches:
        #1: Summarizing duplicated fake InternalClient implementation
        #2: Make task account name variable
    The #2 approach is for multiple task accounts in [1].

    The patch [1] will be rebased after this patch merged.

    [1]: https://review.openstack.org/#/c/517389

    Change-Id: I10a7151cfdd43460ad38c47f672d3c31b77e7990

commit 1c306660a53447bb0a696873bd3f88e66e3781eb
Author: OpenStack Proposal Bot <openstack-...

tags: added: in-feature-deep
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix proposed to swift (feature/s3api)

Related fix proposed to branch: feature/s3api
Review: https://review.openstack.org/548052

Revision history for this message
OpenStack Infra (hudson-openstack) wrote :

Related fix proposed to branch: feature/s3api
Review: https://review.openstack.org/548193

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Change abandoned on swift (feature/s3api)

Change abandoned by Kota Tsuyuzaki (<email address hidden>) on branch: feature/s3api
Review: https://review.openstack.org/548052
Reason: This is affected by the new s3api functests added in recent. Use https://review.openstack.org/#/c/548193/ instead.

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Related fix merged to swift (feature/s3api)
Download full text (30.7 KiB)

Reviewed: https://review.openstack.org/548193
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=9d43f4e190802cb0cad507a65245e4dd70fda7db
Submitter: Zuul
Branch: feature/s3api

commit b3f1558acd2f5a4df3f039070ca5bbc33935e822
Author: Kazuhiro MIYAHARA <email address hidden>
Date: Tue Feb 13 05:16:27 2018 +0000

    Fix expirer's invalid task object names in unit tests

    Object-expirer's task name should be in format of
    "<timestamp>-<account>/<container>/<obj>". In object-expirer
    implementation, ValueError is catched and handled when expirer's task
    objects have invalid name. But in actual swift cluster, invalid task
    object name is not created because task object is created by
    object-server.
    However, without the ValueError catching, some unit tests fail,
    because the unit tests create invalid task object names.

    This patch fixes invalid task object names in unit tests. The
    ValueError catch is remained for unexpected errors, but in the case
    the task will be skipped.

    This patch will help to refactor expirer's task object parsing.

    Change-Id: I8fab8fd180481ce9e97c945904c5c89eec037110

commit 4b19ac772364778a4b96d7e18834db9a7645f482
Author: Tim Burke <email address hidden>
Date: Thu Feb 1 14:30:19 2018 -0800

    py3: port common/storage_policy.py

    Change-Id: I7030280a8495628df9ed8edcc8abc31f901da72e

commit 25540a415e7e36bb08a01a14ca41e2d7591e62cb
Author: Tim Burke <email address hidden>
Date: Thu Feb 22 11:08:49 2018 -0800

    Tighten up assertions around expirer's concurrency

    In particular, test that each work item is only done *once*.

    Change-Id: I9cc610bffb2aa9a2f2b05f4c49e574ab56d05201
    Related-Change: Ic0075a3718face8c509ed0524b63d9171f5b7d7a

commit 5017864133b5af289f205afaf76ffe4518688b3f
Author: melissaml <ma.lei@99cloud.net>
Date: Mon Feb 26 15:48:31 2018 +0800

    Fix the incorrect reference links

    TrivialFix
    [1] is the installation guide for OpenStack components, obviously,
    we need [1] in the docs.

    [1] https://docs.openstack.org/latest/install/

    Change-Id: I3c6fe7327f5552cc2b8f0f0e42b41f8e989a0a7e

commit 58f5d89066adbd54403ad315ffe1f9b2f05aa583
Author: Kazuhiro MIYAHARA <email address hidden>
Date: Tue Feb 13 03:36:04 2018 +0000

    Remove confusing assertion from expirer's unit test

    In test_expirer.TestObjectExpirer.test_process_based_concurrency,
    an assertion checks that expirer execute tasks in round-robin order
    for target containers. But the assertion depends on task object path,
    because task assignation for each process depends on md5 of task
    object path. The dependency makes the assetion confusing.

    Now, we have test_expirer.TestObjectExpirer.test_round_robin_order which
    is added in [1]. So this patch remove the confusing assertion.

    This patch will help to refactor expirer's task object parsing.
    I will push patch for the refactoring after this patch.

    [1]: https://review.openstack.org/#/c/538171

    Change-Id: Ic0075a3718face8c509ed0524b63d9171f5b7d7a

commit 6060af8db96e23d32f3ecc3d02f7f...

tags: added: in-feature-s3api
Changed in swift:
status: New → In Progress
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (master)

Reviewed: https://review.opendev.org/c/openstack/swift/+/525771
Committed: https://opendev.org/openstack/swift/commit/118cf2ba8af97dbbd78271126e22cb80f18f9adc
Submitter: "Zuul (22348)"
Branch: master

commit 118cf2ba8af97dbbd78271126e22cb80f18f9adc
Author: Tim Burke <email address hidden>
Date: Tue Dec 5 21:52:51 2017 +0000

    tempurl: Deprecate sha1 signatures

    We've known this would eventually be necessary for a while [1], and
    way back in 2017 we started seeing SHA-1 collisions [2].

    [1] https://www.schneier.com/blog/archives/2012/10/when_will_we_se.html
    [2] https://security.googleblog.com/2017/02/announcing-first-sha1-collision.html

    UpgradeImpact:
    ==============
    "sha1" has been removed from the default set of `allowed_digests` in the
    tempurl middleware config. If your cluster still has clients requiring
    the use of SHA-1,

    - explicitly configure `allowed_digests` to include "sha1" and
    - encourage your clients to move to more-secure algorithms.

    Depends-On: https://review.opendev.org/c/openstack/tempest/+/832771
    Change-Id: I6e6fa76671c860191a2ce921cb6caddc859b1066
    Related-Change: Ia9dd1a91cc3c9c946f5f029cdefc9e66bcf01046
    Closes-Bug: #1733634

Changed in swift:
status: In Progress → Fix Released
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.30.0

This issue was fixed in the openstack/swift 2.30.0 release.

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.