Date validation bypassed if 'Date' header missing (CVE-2015-8466)

I didn't see this patch in detail yet but attach Darryl's patch I received via E-mail for discussion.

Darryl, thanks for reporting this. As you said, this is a problem for guard of requests when date header is missing.

The change seems reasonable but this change breaks a lot of tests because almost of unit tests doesn't care the date header so that I wonder if you could also fix those tests. I guess that's not so difficult, we can add 'Date' or 'X-Amx-Date' headers into request instance in those tests.

If you could do that, I wanna review the patch via this thread. If you need my help to make it, please poke me anywhere (e.g. IRC)

Thanks!

Here's my improved patch. It modifies all the tests to include the Date header to ensure it passes the new validation. I also added some tests in test_request

Patch seems good, and unit tests fail without the change to request.py. I wouldn't mind seeing a test with x-amz-date and no date header (in particular, I was surprised that test_date_header_with_x_amz_date_valid failed when I rolled back request.py), but it's not the sort of thing that should block this from landing.

Still need to make sure the func tests pass, though. Will get that set up tonight or tomorrow.

Functional tests (both tempauth and keystone) pass. I'm happy with it.

Good job Darryl!

Changes looks good to me but git am failed to patch current newest master in my environment even though it looks a formatted patch for git. I might be missing something but I wonder if you could help me (or rebasing needed?) to push/merge in upstream.

I'm assuming I can push this by myself (absolutely with co-authed-by: darryl) and +2 +A to land asap like as any other security patches.

I just tried rebasing the patch against master. Let me know if it doesn't work.

I believe I made an error in the prior patch. I deleted it and I'll fix it now.

Updated patch.

@Darryl

Thanks for working this, I confirmed the latest patch worked well in my environment.
Code looks good to me so I think we can merge it into upstream.

However, I'm not sure whether or not we should follow on openstack security vulnerability management Process even if it's thirdparty project.

There is a SecurityImpact tag In Darryl's commit message and it seems to trigger to push a notification to openstack security team but if we should maintain this bug as security issue, I think we should add some CVE information for this in launchpad. So I need a help about that process before submitting this to public.

1: https://security.openstack.org/vmt-process.html

Hi Kota, there is no rule for projects who doesn't have the "vulnerability:managed" big tent tag, basically it's up to you if you want to follow the vmt process.

MITRE assigned CVE-2015-8466 for this bug.

Thanks Tristan!

I'll make this public with the CVE tag in commit message.

@All

Note that I don't know there are any stakeholders for swift3 and Tristan told me swift3 don't have to follow OpenStack VMT process so I diceded to skip "Embargoed Disclosure" process for this.

I think some distributors might want to get this fix but we already has a notification (swift3 had a security issue) with assigned CVE . I expects someone find the notification and will try to merge this if needed. That is because I decided to this to be open.

If you have any question or suggestion for this, please contact me.

Patch pushed to https://review.openstack.org/#/c/255067/

Reviewed: https://review.openstack.org/255067
Committed: https://git.openstack.org/cgit/openstack/swift3/commit/?id=4fce274c50112e02360993c4eeaafe811fcc757c
Submitter: Jenkins
Branch: master

commit 4fce274c50112e02360993c4eeaafe811fcc757c
Author: Kota Tsuyuzaki <email address hidden>
Date: Wed Nov 25 14:16:06 2015 -0800

    Fix date validation

    According to [1] when an Authorization header is specified, either a
    Date or x-amz-date header needs to be specified, with the x-amz-date
    header taking precedence.

    Now, the x-amz-date header is validated first, and if both headers are
    missing, an AccessDenied error should be returned. This should prevent
    replay attacks occurring on valid requests that are missing the Date
    header.

    [1]
    http://docs.aws.amazon.com/AmazonS3/latest/API/RESTCommonRequestHeaders.
    html

    N.B. This also fixes some pylint issues and dependencies

    Closes-Bug: 1497424
    SecurityImpact
    [CVE-2015-8466]

    Co-Authored-By: Darryl Tam <email address hidden>
    Co-Authored-By: Tim Burke <email address hidden>

    Change-Id: Ibeff8503fa147e1cf08c1b5374aecee7a4c0bee2

Hi Kota, it seems like most stakeholder doesn't read openstack-security mailing list, and CVE assigned to opensource project are better advertised on <email address hidden> mailing list.

Is there a non-too technical impact description ? And do we know what swift3 release are affected ?

Here's my attempt at a less technical description of the issue and its impact:

In S3, requests are signed, with the signature depending on an assigned private key for a user and specified message headers, including Date and/or x-amz-date. If the signature does not match, the request should be rejected.

For these signed requests, a x-amz-date or Date header must be specified, with the x-amz-date header taking precedence. If the timestamp of the signed request differs from the current time a particular time limit (in Swift3 this is set to plus or minus five minutes), the request should be rejected.

In affected versions of Swift3, the x-amz-date is not validated, and only the Date header (if present) is validated against the current time.

Note that in all cases, requests are still checked for a valid signature.

If a client sends both Date header and x-amz-date header, affected versions will check the signature against the time in the Date header instead of the x-amz-date header. If the Date and x-amz-date headers are different, the time period in which Swift3 would accept the request would be different. This could result in a valid requests being rejected and invalid requests being accepted depending on the time the requests are sent.

If a client sends a Date header only, it is not affected by this issue.

If a client sends a x-amz-date header without a Date header, affected versions validate regardless of the current time. This would allow the same request to be repeated and accepted by Swift3 at any time.

In order to be affected, a user would need to use a S3 client that sends an x-amz-date header without a matching Date header. If the request is captured, it could later be repeated and accepted either (a) at any time (if the Date header is not present) or, (b) within the time limit set by the Date header.

The update will validate x-amz-date headers, if they exist, against the current time, which preventing replay attacks when outside of the time limit.