Need to verify content of v4-signed PUTs

[Tim Burke]

When we added support for v4 signatures, we (correctly) require that the client provide a X-Amz-Content-SHA256 header and use it in computing the expected signature. However, we never verify that the content sent actually matches the SHA! As a result, an attacker that manages to capture the headers for a PUT request has a 5-minute window to overwrite the object with arbitrary content of the same length:

[11:50:08] $ echo 'GOOD' > good.txt

[11:50:12] $ echo 'BAD!' > bad.txt

[11:50:36] $ s3cmd put --debug good.txt s3://bucket
DEBUG: s3cmd version 1.6.1
DEBUG: ConfigParser: Reading file '/Users/tburke/.s3cfg'
DEBUG: ConfigParser: access_key->te...8_chars...r
DEBUG: ConfigParser: secret_key->te...4_chars...g
DEBUG: ConfigParser: host_base->saio:8080
DEBUG: ConfigParser: host_bucket->saio:8080
DEBUG: ConfigParser: use_https->False
DEBUG: Updating Config.Config cache_file ->
DEBUG: Updating Config.Config follow_symlinks -> False
DEBUG: Updating Config.Config verbosity -> 10
DEBUG: Unicodising 'put' using UTF-8
DEBUG: Unicodising 'good.txt' using UTF-8
DEBUG: Unicodising 's3://bucket' using UTF-8
DEBUG: Command: put
DEBUG: DeUnicodising u'good.txt' using UTF-8
INFO: Compiling list of local files...
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: Unicodising '' using UTF-8
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: Applying --exclude/--include
DEBUG: CHECK: good.txt
DEBUG: PASS: u'good.txt'
INFO: Running stat() and reading/calculating MD5 values on 1 files, this may take some time...
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: doing file I/O to read md5 of good.txt
DEBUG: DeUnicodising u'good.txt' using UTF-8
INFO: Summary: 1 local files to upload
DEBUG: attr_header: {'x-amz-meta-s3cmd-attrs': 'uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212'}
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: String 'good.txt' encoded to 'good.txt'
DEBUG: CreateRequest: resource[uri]=/good.txt
DEBUG: Using signature v4
DEBUG: get_hostname(bucket): saio:8080
DEBUG: canonical_headers = content-length:5
content-type:text/plain
host:saio:8080
x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date:20180420T185102Z
x-amz-meta-s3cmd-attrs:uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212
x-amz-storage-class:STANDARD

DEBUG: Canonical Request:
PUT
/bucket/good.txt

content-length:5
content-type:text/plain
host:saio:8080
x-amz-content-sha256:e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
x-amz-date:20180420T185102Z
x-amz-meta-s3cmd-attrs:uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212
x-amz-storage-class:STANDARD

content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855
----------------------
DEBUG: signature-v4 headers: {'x-amz-content-sha256': 'e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855', 'content-length': '5', 'x-amz-storage-class': 'STANDARD', 'x-amz-meta-s3cmd-attrs': 'uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212', 'x-amz-date': '20180420T185102Z', 'content-type': 'text/plain', 'Authorization': 'AWS4-HMAC-SHA256 Credential=test:tester/20180420/US/s3/aws4_request,SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class,Signature=e79e1dd2fcd3ba125d3186abdbaf428992c478ad59380eab4d81510cfc494e43'}
DEBUG: Unicodising 'good.txt' using UTF-8
upload: 'good.txt' -> 's3://bucket/good.txt' [1 of 1]
DEBUG: DeUnicodising u'good.txt' using UTF-8
DEBUG: Using signature v4
DEBUG: get_hostname(bucket): saio:8080
DEBUG: canonical_headers = content-length:5
content-type:text/plain
host:saio:8080
x-amz-content-sha256:d43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7
x-amz-date:20180420T185102Z
x-amz-meta-s3cmd-attrs:uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212
x-amz-storage-class:STANDARD

DEBUG: Canonical Request:
PUT
/bucket/good.txt

content-length:5
content-type:text/plain
host:saio:8080
x-amz-content-sha256:d43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7
x-amz-date:20180420T185102Z
x-amz-meta-s3cmd-attrs:uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212
x-amz-storage-class:STANDARD

content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class
d43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7
----------------------
DEBUG: signature-v4 headers: {'x-amz-content-sha256': 'd43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7', 'content-length': '5', 'x-amz-storage-class': 'STANDARD', 'x-amz-meta-s3cmd-attrs': 'uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212', 'x-amz-date': '20180420T185102Z', 'content-type': 'text/plain', 'Authorization': 'AWS4-HMAC-SHA256 Credential=test:tester/20180420/US/s3/aws4_request,SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class,Signature=63a27138d8f6fd0320a15f8ef8bf95474246c80a38ed68693c58173cefd8589b'}
DEBUG: get_hostname(bucket): saio:8080
DEBUG: ConnMan.get(): creating new connection: http://saio:8080
DEBUG: non-proxied HTTPConnection(saio:8080)
DEBUG: format_uri(): /bucket/good.txt
 5 of 5 100% in 0s 373.44 B/sDEBUG: ConnMan.put(): connection put back to pool (http://saio:8080#1)
DEBUG: Response: {'status': 200, 'headers': {'content-length': '0', 'x-amz-id-2': 'tx98be5ca4733e430eb4a76-005ada3696', 'x-trans-id': 'tx98be5ca4733e430eb4a76-005ada3696', 'last-modified': 'Fri, 20 Apr 2018 18:51:03 GMT', 'etag': '"f9d9dc2bab2572ba95cfd67b596a6d1a"', 'x-amz-request-id': 'tx98be5ca4733e430eb4a76-005ada3696', 'date': 'Fri, 20 Apr 2018 18:51:02 GMT', 'content-type': 'text/html; charset=UTF-8', 'x-openstack-request-id': 'tx98be5ca4733e430eb4a76-005ada3696'}, 'reason': 'OK', 'data': '', 'size': 5L}
 5 of 5 100% in 0s 56.02 B/s done
DEBUG: MD5 sums: computed=f9d9dc2bab2572ba95cfd67b596a6d1a, received="f9d9dc2bab2572ba95cfd67b596a6d1a"
/Users/tburke/.virtualenvs/Python27/lib/python2.7/site-packages/magic/identify.py:62: RuntimeWarning: Implicitly cleaning up <magic.api.LP_Cookie object at 0x110369050>
  CleanupWarning)

[11:51:02] $ curl -v http://saio:8080/bucket/good.txt -T bad.txt -H 'x-amz-content-sha256: d43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7' -H 'x-amz-storage-class: STANDARD' -H 'x-amz-meta-s3cmd-attrs: uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212' -H 'x-amz-date: 20180420T185102Z' -H 'content-type: text/plain' -H 'Authorization: AWS4-HMAC-SHA256 Credential=test:tester/20180420/US/s3/aws4_request,SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class,Signature=63a27138d8f6fd0320a15f8ef8bf95474246c80a38ed68693c58173cefd8589b'
* Trying 192.168.8.80...
* TCP_NODELAY set
* Connected to saio (192.168.8.80) port 8080 (#0)
> PUT /bucket/good.txt HTTP/1.1
> Host: saio:8080
> User-Agent: curl/7.54.0
> Accept: application/json;q=1, text/*;q=.9, */*;q=.8
> x-amz-content-sha256: d43cf775e7609f1274a4cd97b7649be036b01a6e22d6a04038ecd51811652cf7
> x-amz-storage-class: STANDARD
> x-amz-meta-s3cmd-attrs: uid:501/gname:staff/uname:tburke/gid:20/mode:33188/mtime:1524250212/atime:1524250212/md5:f9d9dc2bab2572ba95cfd67b596a6d1a/ctime:1524250212
> x-amz-date: 20180420T185102Z
> content-type: text/plain
> Authorization: AWS4-HMAC-SHA256 Credential=test:tester/20180420/US/s3/aws4_request,SignedHeaders=content-length;content-type;host;x-amz-content-sha256;x-amz-date;x-amz-meta-s3cmd-attrs;x-amz-storage-class,Signature=63a27138d8f6fd0320a15f8ef8bf95474246c80a38ed68693c58173cefd8589b
> Content-Length: 5
> Expect: 100-continue
>
< HTTP/1.1 100 Continue
* We are completely uploaded and fine
< HTTP/1.1 200 OK
< Content-Length: 0
< x-amz-id-2: tx348d466b04cd425b81760-005ada3718
< Last-Modified: Fri, 20 Apr 2018 18:53:13 GMT
< ETag: "6cd890020ad6ab38782de144aa831f24"
< x-amz-request-id: tx348d466b04cd425b81760-005ada3718
< Content-Type: text/html; charset=UTF-8
< X-Trans-Id: tx348d466b04cd425b81760-005ada3718
< X-Openstack-Request-Id: tx348d466b04cd425b81760-005ada3718
< Date: Fri, 20 Apr 2018 18:53:13 GMT
<
* Connection #0 to host saio left intact

---

I've attached a fix, but it could use tests :-/

[Tim Burke (1-tim-z)]

Realized this also affects s3api since it was imported.

[Tim Burke (1-tim-z)]

Looks like the old patch doesn't apply any more... rebased on top of current master, https://github.com/openstack/swift/commit/f7ca182

[Jeremy Stanley (fungi)]

It looks like this was never triaged by the OpenStack VMT, so I'm doing that now. 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)]

Is there a means of capturing the headers of a PUT request without an already compromised channel (hacked Swift infrastructure, compromised client system, man-in-the-middle HTTPS...)? Is this commonly deployed via plain, unsecured HTTP for example? If not, I'd be inclined to consider this a security hardening opportunity and continue working the bug in public rather than under embargo.

[John Dickinson (notmyname)]

Jeremy, that sounds reasonable.

[Jeremy Stanley (fungi)]

Since it seems like exploiting this situation requires some additional access to begin with, I'm setting the OpenStack VMT's security advisory task to "won't fix" for now and tagging the report as a security hardening opportunity. If new information comes to light, we can of course revisit that choice (which also becomes much easier with the report now public).

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/629301
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=3a8f5dbf9c49fdf1cf2d0b7ba35b82f25f88e634
Submitter: Zuul
Branch: master

commit 3a8f5dbf9c49fdf1cf2d0b7ba35b82f25f88e634
Author: Tim Burke <email address hidden>
Date: Tue Dec 11 15:29:35 2018 -0800

    Verify client input for v4 signatures

    Previously, we would use the X-Amz-Content-SHA256 value when calculating
    signatures, but wouldn't actually check the content that was sent. This
    would allow a malicious third party that managed to capture the headers
    for an object upload to overwrite that with arbitrary content provided
    they could do so within the 5-minute clock-skew window.

    Now, we wrap the wsgi.input that's sent on to the proxy-server app to
    hash content as it's read and raise an error if there's a mismatch. Note
    that clients using presigned-urls to upload have no defense against a
    similar replay attack.

    Notwithstanding the above security consideration, this *also* provides
    better assurances that the client's payload was received correctly. Note
    that this *does not* attempt to send an etag in footers, however, so the
    proxy-to-object-server connection is not guarded against bit-flips.

    In the future, Swift will hopefully grow a way to perform SHA256
    verification on the object-server. This would offer two main benefits:

      - End-to-end message integrity checking.
      - Move CPU load of calculating the hash from the proxy (which is
        somewhat CPU-bound) to the object-server (which tends to have CPU to
        spare).

    Change-Id: I61eb12455c37376be4d739eee55a5f439216f0e9
    Closes-Bug: 1765834

[OpenStack Infra (hudson-openstack)]

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

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: feature/losf
Review: https://review.openstack.org/648245

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/648245
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=6afc1130fd753306d64745c9bee7712182b273d3
Submitter: Zuul
Branch: feature/losf

commit 89e5927f7dd94fc28b3847944eb7dd227d516fa8
Author: Thiago da Silva <email address hidden>
Date: Tue Mar 26 10:46:02 2019 -0400

    Fix mocking time

    When running on Centos the side_effect was returning a MagicMock
    object instead of the intended int.

    Change-Id: I73713a9a96dc415073a637d85a40304021f76072

commit 50715acb1838fbde628e447e7b02545ce8469180
Author: OpenStack Release Bot <email address hidden>
Date: Mon Mar 25 17:07:54 2019 +0000

    Update master for stable/stein

    Add file to the reno documentation build to show release notes for
    stable/stein.

    Use pbr instruction to increment the minor version number
    automatically so that master versions are higher than the versions on
    stable/stein.

    Change-Id: I6109bff3227f87d914abf7bd1d76143aaf91419d
    Sem-Ver: feature

commit 179fa7ccd4d6faeacc989715887b69f9422a17b2
Author: John Dickinson <email address hidden>
Date: Mon Mar 18 17:09:31 2019 -0700

    authors/changelog update for 2.21.0 release

    Change-Id: Iac51a69c71491e5a8db435aae396178a6c592c73

commit 64eec5fc93eb670e581cbb3a6dedb6a7aa501e99
Author: Tim Burke <email address hidden>
Date: Thu Mar 7 14:36:02 2019 -0800

    Fix how we UTF-8-ify func tests

    I noticed while poking at the DLO func tests that we don't actually use
    non-ascii chars when we set up the test env.

    By patching the create name function earlier (in SetUpClass) we can
    ensure we get some more interesting characters in our object names.

    Change-Id: I9480ddf74463310aeb11ad876b79527888d8c871

commit fe3a20f2e4b745bf7d81f9bda97082b593e8794a
Author: Tim Burke <email address hidden>
Date: Tue Mar 19 14:52:19 2019 -0700

    Remove uncalled function

    Change-Id: Ica67815f0ddf4b00bce1ffe183735490c7f7c0b5
    Related-Change: I5629de9f2e9b2331ed3f455d253efc69d030df72

commit adc568c97f5b30d9d4628eaf448f81d736ad4e51
Author: John Dickinson <email address hidden>
Date: Fri Mar 15 15:18:36 2019 -0700

    Fix bulk responses when using xml and Expect 100-continue

    When we fixed bulk response heartbeating in https://review.openstack.org/#/c/510715/,
    code review raised the issue of moving the xml header down to after the
    early-exit clauses. At the time, it didn't seem to break anything, so
    it was left in place. However, that insight was correct.

    The purpose of the earlier patch was to force eventlet to use chunked
    transfer encoding on the response in order to prevent eventlet from
    buffering the whole response, thus defeating the purpose of the
    heartbeat responses.

    Moving the first line of the body lower (ie after the early exit
    checks), allows other headers in a chunked transfer encoding response
    to be appropriately processed before sending the headers. Sending the
    xml declaration early causes it to get intermingled in the 100-continue
    protocol, thus breaking the chunked transfer encoding semantics.

    Closes-Bug: #1819...

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/rocky
Review: https://review.opendev.org/689883

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.opendev.org/689883
Committed: https://git.openstack.org/cgit/openstack/swift/commit/?id=423f96293b6eb43504b5370e893c280697ed23c9
Submitter: Zuul
Branch: stable/rocky

commit 423f96293b6eb43504b5370e893c280697ed23c9
Author: Tim Burke <email address hidden>
Date: Tue Dec 11 15:29:35 2018 -0800

    Verify client input for v4 signatures

    This is a combination of 2 commits.

    ==========

    Previously, we would use the X-Amz-Content-SHA256 value when calculating
    signatures, but wouldn't actually check the content that was sent. This
    would allow a malicious third party that managed to capture the headers
    for an object upload to overwrite that with arbitrary content provided
    they could do so within the 5-minute clock-skew window.

    Now, we wrap the wsgi.input that's sent on to the proxy-server app to
    hash content as it's read and raise an error if there's a mismatch. Note
    that clients using presigned-urls to upload have no defense against a
    similar replay attack.

    Notwithstanding the above security consideration, this *also* provides
    better assurances that the client's payload was received correctly. Note
    that this *does not* attempt to send an etag in footers, however, so the
    proxy-to-object-server connection is not guarded against bit-flips.

    In the future, Swift will hopefully grow a way to perform SHA256
    verification on the object-server. This would offer two main benefits:

      - End-to-end message integrity checking.
      - Move CPU load of calculating the hash from the proxy (which is
        somewhat CPU-bound) to the object-server (which tends to have CPU to
        spare).

    Closes-Bug: 1765834
    (cherry picked from commit 3a8f5dbf9c49fdf1cf2d0b7ba35b82f25f88e634)

    ----------

    s3api: Allow clients to upload with UNSIGNED-PAYLOAD

    (Some versions of?) awscli/boto3 will do v4 signatures but send a
    Content-MD5 for end-to-end validation. Since a X-Amz-Content-SHA256
    is still required to calculate signatures, it uses UNSIGNED-PAYLOAD
    similar to how signatures work for pre-signed URLs.

    Look for UNSIGNED-PAYLOAD and skip SHA256 validation if set.

    (cherry picked from commit 82e446a8a0c0fd6a81f06717b76ed3d1be26a281)
    (cherry picked from commit 6ed165cf3f65329beaef9977a5fec24ce3ac0b39)

    ==========

    Change-Id: I61eb12455c37376be4d739eee55a5f439216f0e9

[OpenStack Infra (hudson-openstack)]

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