swift-temp-url wrong HMAC full url (with WARNING)

Bug #1607519 reported by clayg
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
OpenStack Object Storage (swift)
Fix Released
Low
Unassigned

Bug Description

If you want to get a tempurl for an object at a uri you might try:

vagrant@saio:~$ swift-temp-url GET 30 https://swift-cluster.example.com/v1/AUTH_account/c/o secret
WARNING: "https://swift-cluster.example.com/v1/AUTH_account/c/o" does not refer to an object (e.g. /v1/account/container/object).
WARNING: Non-object paths will be rejected by tempurl.
https://swift-cluster.example.com/v1/AUTH_account/c/o?temp_url_sig=c245fb4062684d423a6bb53bdc8f9644bf1f4ea0&temp_url_expires=1469736000

It outputs a valid *looking* tempurl - but the warning does indicate something wrong (it used the wrong path for the HMAC)!

Expected output:

vagrant@saio:~$ swift-temp-url GET 30 https://swift-cluster.example.com/v1/AUTH_account/c/o secret
https://swift-cluster.example.com/v1/AUTH_account/c/o?temp_url_sig=f7a815b64a34dbdcbda138c14078e6854f526313&temp_url_expires=1469736364

clayg (clay-gerrard)
summary: - swift-temp-url prints WARNING with full url
+ swift-temp-url wrong HMAC full url (with WARNING)
Revision history for this message
Mohit Motiani (mohit-motiani) wrote :

I was able to reproduce it.
Warning message needs to update in this case. Also, a valid looking URI should not be generated if object or container does not exist. Instead, tempURL should be discarded.

#in below case, object does not exist. still it generates a valid URI.
ubuntu@saio-experiment:~/swift$ swift-temp-url GET 3600 /v1/AUTH_test/test_container/file4.txt secrete_key_b
/v1/AUTH_test/test_container/file4.txt?temp_url_sig=a7b362cbb3a5c3482b1cc26ee84a1741de5f3c9d&temp_url_expires=1469742082

#in below case, container and object does not exist. still it generates a valid URI.
ubuntu@saio-experiment:~/swift$ swift-temp-url GET 3600 /v1/AUTH_test/test_container1/file4.txt secrete_key_b
/v1/AUTH_test/test_container1/file4.txt?temp_url_sig=6cffe72513448d2de0b31700fb71427b727e3628&temp_url_expires=1469742091

Changed in swift:
assignee: nobody → Mohit Motiani (mohit-motiani)
Revision history for this message
clayg (clay-gerrard) wrote :
Changed in swift:
assignee: Mohit Motiani (mohit-motiani) → nobody
Revision history for this message
Donagh McCabe (donagh-mccabe) wrote :

This is an invalid usage of swift-temp-url:

    swift-temp-url GET 30 https://blah/v1/AUTH_account/c/o secret

The third argument is https://blah/v1/AUTH_account/c/o. That is not a valid path -- it's a full URL. The correct value should be /v1/AUTH_account/c/o. So the correct usage should have been:

    swift-temp-url GET 30 /v1/AUTH_account/c/o secret

The user is expected to create a full URL by adding the schema and netloc. The help text shows one way to do this. Note: that help text was patched very recently to add double-quotes around the path so that he "&" in the query parameters is not interpreted by your shell.

The purpose of the "WARNING: Non-object paths will be rejected by tempurl." message is to remind the user that tempurl only works on objects, so if the path does not include an object name, don't be surprised that the result won't work. I've found this reminder helpful - I'm not sure why it prints the signature and not simply exit after printing the warning.

> in below case, container and object does not exist. still it generates a valid URI.
The purpose of the tool is to generate signatures for a given path -- it does not attempt to confirm that the object actually exists.

Working with tempurl can be frustrating -- it's easy to make a mistake. For the above example, a usefull enhancement for the tool would be to recognise that a full URL (including scehema/netloc) was provided and to remove them from the result (or remove them from the signature generation, but add them back to the end result).

Another useful feature would be someway to indicate that the time is an absolute time --- this would make swift-temp-url useful as a validation tool for home-grown signature-generating code. An easy way would be examine the time value -- anything less than the current time could be considered to be elapsed seconds, anything larger can be assumed to be an absolute time.

Note: similar problems may exist in bin/swift

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

Reviewed: https://review.openstack.org/348173
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=350f10bf3be9ce6c9eec4b3046afc9a05cf08454
Submitter: Jenkins
Branch: master

commit 350f10bf3be9ce6c9eec4b3046afc9a05cf08454
Author: Christian Schwede <email address hidden>
Date: Thu Jul 28 08:32:17 2016 +0000

    Deprecate swift-temp-url

    python-swiftclient includes an improved and tested method to generate
    tempurls. The command syntax is essentially the same, therefore we can
    deprecate this one by importing that method.

    python-swiftclient is not added as a requirement; if the import fails
    due to a missing swiftclient module it will just raise a deprecation
    warning.

    Closes-Bug: #1607523
    Closes-Bug: #1607519

    Change-Id: Ifa8bf636f20f82db4845b02d1b58699edaa39356

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

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

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix proposed to swift (feature/hummingbird)

Fix proposed to branch: feature/hummingbird
Review: https://review.openstack.org/400985

Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix merged to swift (feature/hummingbird)
Download full text (78.0 KiB)

Reviewed: https://review.openstack.org/400985
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=0c3f8f87104af8717115c5badffd243dbaa1c430
Submitter: Jenkins
Branch: feature/hummingbird

commit 2d25fe6ad3573b2a06b6b3e5e66493d7b0c55693
Author: Tim Burke <email address hidden>
Date: Mon Jul 25 15:06:23 2016 -0700

    Reduce backend requests for SLO If-Match / HEAD requests

    ... by storing SLO Etag and size in sysmeta.

    Previously, we had to GET the manifest for every HEAD or conditional
    request to an SLO. Worse, since SLO PUTs require that we HEAD every
    segment, we'd GET all included sub-SLO manifests. This was necessary so
    we could recompute the large object's Etag and content-length.

    Since we already know both of those during PUT, we'll now store it in
    object sysmeta. This allows us to:

     * satisfy HEAD requests based purely off of the manifest's HEAD
       response, and
     * perform the If-(None-)Match comparison on the object server, without
       any additional subrequests.

    Note that the large object content-length can't just be parsed from
    content-type -- with fast-POST enabled, the content-type coming out of
    the object-server won't necessarily include swift_bytes.

    Also note that we must still fall back to GETting the manifest if the
    sysmeta headers were not found. Otherwise, we'd break existing large
    objects.

    Change-Id: Ia6ad32354105515560b005cea750aa64a88c96f9

commit ae7dddd801e28217d7dc46bd45cd6b621f29340c
Author: Ondřej Nový <email address hidden>
Date: Mon Nov 21 22:13:11 2016 +0100

    Added comment for "user" option in drive-audit config

    Change-Id: I24362826bee85ac3304e9b63504c9465da673014

commit c3e1d847f4b9d6cc6212aae4dc1b1e6dff45fb40
Author: Thiago da Silva <email address hidden>
Date: Thu Nov 17 17:17:00 2016 -0500

    breaking down tests.py into smaller pieces

    tests.py is currently at ~5500 lines of code, it's
    time to break it down into smaller files.

    I started with an easy middleware set of tests
    (i.e., versioned writes, ~600 lines of code ) so I can get
    some feedback. There are more complicated tests that cover
    multiple middlewares for example, it is not so clear where
    those should go.

    Change-Id: I2aa6c18ee5b68d0aae73cc6add8cac6fbf7f33da
    Signed-off-by: Thiago da Silva <email address hidden>

commit 5d7a3a4172f0f11ab870252eec784cf24b247dea
Author: Ondřej Nový <email address hidden>
Date: Sat Nov 19 23:24:30 2016 +0100

    Removed "in-process-" from func env tox name

    This shorten shebang in infra, because we are hitting 128 bytes limit.

    Change-Id: I02477d81b836df71780942189d37d616944c4dce

commit 9ea340256996a03c8c744201297b47a0e91fe65b
Author: Kota Tsuyuzaki <email address hidden>
Date: Fri Nov 18 01:50:11 2016 -0800

    Don't overwrite built-in 'id'

    This is a follow up for https://review.openstack.org/#/c/399237

    'id' is assigned as a builtin function so that we should not use 'id'
    for the local variable name.

    Change-Id: Ic27460d49e68f6cd50bda1d5b3810e01ccb07a37

commit bf...

tags: added: in-feature-hummingbird
Revision history for this message
OpenStack Infra (hudson-openstack) wrote : Fix included in openstack/swift 2.10.0

This issue was fixed in the openstack/swift 2.10.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.