Skip to content
This repository has been archived by the owner on Sep 26, 2019. It is now read-only.

Commit

Permalink
Fix date validation
Browse files Browse the repository at this point in the history
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 <dtam@swiftstack.com>
Co-Authored-By: Tim Burke <tim.burke@gmail.com>

Change-Id: Ibeff8503fa147e1cf08c1b5374aecee7a4c0bee2
  • Loading branch information
Kota Tsuyuzaki committed Dec 9, 2015
1 parent b90a661 commit 4fce274
Show file tree
Hide file tree
Showing 16 changed files with 386 additions and 152 deletions.
12 changes: 9 additions & 3 deletions swift3/request.py
Expand Up @@ -186,9 +186,11 @@ def _validate_headers(self):
raise InvalidArgument('Content-Length',
self.environ['CONTENT_LENGTH'])

if 'Date' in self.headers:
date_header = self.headers.get('x-amz-date',
self.headers.get('Date', None))
if date_header:
now = datetime.datetime.utcnow()
date = email.utils.parsedate(self.headers['Date'])
date = email.utils.parsedate(date_header)
if 'Expires' in self.params:
try:
d = email.utils.formatdate(float(self.params['Expires']))
Expand All @@ -213,7 +215,11 @@ def _validate_headers(self):
if abs(d1 - now) > delta:
raise RequestTimeTooSkewed()
else:
raise AccessDenied()
raise AccessDenied('AWS authentication requires a valid Date '
'or x-amz-date header')
else:
raise AccessDenied('AWS authentication requires a valid Date '
'or x-amz-date header')

if 'Content-MD5' in self.headers:
value = self.headers['Content-MD5']
Expand Down
2 changes: 1 addition & 1 deletion swift3/subresource.py
Expand Up @@ -355,7 +355,7 @@ def __init__(self, grantee, permission):
if permission.upper() not in PERMISSIONS:
raise S3NotImplemented()
if not isinstance(grantee, Grantee):
raise
raise ValueError()
self.grantee = grantee
self.permission = permission

Expand Down
9 changes: 8 additions & 1 deletion swift3/test/unit/__init__.py
Expand Up @@ -14,6 +14,9 @@
# limitations under the License.

import unittest
from datetime import datetime
import email
import time

from swift.common import swob

Expand Down Expand Up @@ -90,12 +93,16 @@ def _test_method_error(self, method, path, response_class, headers={}):
uri += path

self.swift.register(method, uri, response_class, headers, None)
headers.update({'Authorization': 'AWS test:tester:hmac'})
headers.update({'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()})
req = swob.Request.blank(path, environ={'REQUEST_METHOD': method},
headers=headers)
status, headers, body = self.call_swift3(req)
return self._get_error_code(body)

def get_date_header(self):
return email.utils.formatdate(time.mktime(datetime.now().timetuple()))

def call_app(self, req, app=None, expect_exception=False):
if app is None:
app = self.app
Expand Down
16 changes: 12 additions & 4 deletions swift3/test/unit/test_acl.py
Expand Up @@ -44,7 +44,8 @@ def _check_acl(self, owner, body):
def test_bucket_acl_GET(self):
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'GET'},
headers={'Authorization': 'AWS test:tester:hmac'})
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()})
status, headers, body = self.call_swift3(req)
self._check_acl('test:tester', body)

Expand All @@ -63,7 +64,8 @@ def test_bucket_acl_PUT(self):
xml = tostring(elem)
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'PUT'},
headers={'Authorization': 'AWS test:tester:hmac'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()},
body=xml)
status, headers, body = self.call_swift3(req)
self.assertEquals(status.split()[0], '200')
Expand All @@ -72,6 +74,7 @@ def test_bucket_acl_PUT(self):
environ={'REQUEST_METHOD': 'PUT',
'wsgi.input': StringIO(xml)},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header(),
'Transfer-Encoding': 'chunked'})
self.assertIsNone(req.content_length)
self.assertIsNone(req.message_length())
Expand All @@ -82,6 +85,7 @@ def test_bucket_canned_acl_PUT(self):
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'PUT'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header(),
'X-AMZ-ACL': 'public-read'})
status, headers, body = self.call_swift3(req)
self.assertEquals(status.split()[0], '200')
Expand All @@ -91,6 +95,7 @@ def test_bucket_canned_acl_PUT_with_s3acl(self):
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'PUT'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header(),
'X-AMZ-ACL': 'public-read'})
with mock.patch('swift3.request.handle_acl_header') as mock_handler:
status, headers, body = self.call_swift3(req)
Expand All @@ -113,6 +118,7 @@ def test_bucket_fails_with_both_acl_header_and_xml_PUT(self):
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'PUT'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header(),
'X-AMZ-ACL': 'public-read'},
body=xml)
status, headers, body = self.call_swift3(req)
Expand All @@ -122,14 +128,16 @@ def test_bucket_fails_with_both_acl_header_and_xml_PUT(self):
def test_object_acl_GET(self):
req = Request.blank('/bucket/object?acl',
environ={'REQUEST_METHOD': 'GET'},
headers={'Authorization': 'AWS test:tester:hmac'})
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()})
status, headers, body = self.call_swift3(req)
self._check_acl('test:tester', body)

def test_invalid_xml(self):
req = Request.blank('/bucket?acl',
environ={'REQUEST_METHOD': 'PUT'},
headers={'Authorization': 'AWS test:tester:hmac'},
headers={'Authorization': 'AWS test:tester:hmac',
'Date': self.get_date_header()},
body='invalid')
status, headers, body = self.call_swift3(req)
self.assertEquals(self._get_error_code(body), 'MalformedACLError')
Expand Down

0 comments on commit 4fce274

Please sign in to comment.