From 460dcf7562b7fa6d3244c28097ddd77287782274 Mon Sep 17 00:00:00 2001 From: Tim Burke Date: Thu, 20 May 2021 17:49:10 -0700 Subject: [PATCH] s3api: Allow CORS preflights for pre-signed URLs Looks like browsers *do* send the query string in the OPTIONS request. Change-Id: Id10e6e32890f1c9a09c91990e5a6ee729bf4d973 Related-Change: I985143bf03125a05792e79bc5e5f83722d6431b3 --- swift/common/middleware/s3api/s3api.py | 36 ++++++++++++++----- test/cors/test-s3-obj.js | 25 +++++++++++++ test/unit/common/middleware/s3api/test_obj.py | 35 ++++++++++++++++++ 3 files changed, 87 insertions(+), 9 deletions(-) diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py index 1d0bc28545..42cef6024d 100644 --- a/swift/common/middleware/s3api/s3api.py +++ b/swift/common/middleware/s3api/s3api.py @@ -144,6 +144,7 @@ https://github.com/swiftstack/s3compat in detail. from cgi import parse_header import json from paste.deploy import loadwsgi +from six.moves.urllib.parse import parse_qs from swift.common.constraints import valid_api_version from swift.common.middleware.listing_formats import \ @@ -290,13 +291,24 @@ class S3ApiMiddleware(object): wsgi_conf, log_route=wsgi_conf.get('log_name', 's3api')) self.check_pipeline(wsgi_conf) + def is_s3_cors_preflight(self, env): + if env['REQUEST_METHOD'] != 'OPTIONS' or not env.get('HTTP_ORIGIN'): + # Not a CORS preflight + return False + acrh = env.get('HTTP_ACCESS_CONTROL_REQUEST_HEADERS', '').lower() + if 'authorization' in acrh and \ + not env['PATH_INFO'].startswith(('/v1/', '/v1.0/')): + return True + q = parse_qs(env.get('QUERY_STRING', '')) + if 'AWSAccessKeyId' in q or 'X-Amz-Credential' in q: + return True + # Not S3, apparently + return False + def __call__(self, env, start_response): origin = env.get('HTTP_ORIGIN') - acrh = env.get('HTTP_ACCESS_CONTROL_REQUEST_HEADERS', '').lower() - if self.conf.cors_preflight_allow_origin and origin and \ - env['REQUEST_METHOD'] == 'OPTIONS' and \ - 'authorization' in acrh and \ - not env['PATH_INFO'].startswith(('/v1/', '/v1.0/')): + if self.conf.cors_preflight_allow_origin and \ + self.is_s3_cors_preflight(env): # I guess it's likely going to be an S3 request? *shrug* if self.conf.cors_preflight_allow_origin != ['*'] and \ origin not in self.conf.cors_preflight_allow_origin: @@ -305,15 +317,21 @@ class S3ApiMiddleware(object): ]) return [b''] - start_response('200 OK', [ + headers = [ ('Allow', 'GET, HEAD, PUT, POST, DELETE, OPTIONS'), ('Access-Control-Allow-Origin', origin), ('Access-Control-Allow-Methods', 'GET, HEAD, PUT, POST, DELETE, OPTIONS'), - ('Access-Control-Allow-Headers', - ', '.join(set(list_from_csv(acrh)))), ('Vary', 'Origin, Access-Control-Request-Headers'), - ]) + ] + acrh = set(list_from_csv( + env.get('HTTP_ACCESS_CONTROL_REQUEST_HEADERS', '').lower())) + if acrh: + headers.append(( + 'Access-Control-Allow-Headers', + ', '.join(acrh))) + + start_response('200 OK', headers) return [b''] try: diff --git a/test/cors/test-s3-obj.js b/test/cors/test-s3-obj.js index dfa119ef9e..1dbc8189ce 100644 --- a/test/cors/test-s3-obj.js +++ b/test/cors/test-s3-obj.js @@ -166,6 +166,31 @@ function makeTests (params) { .then(CheckTransactionIdHeaders) .then(HasNoBody) })], + ['presigned PUT then DELETE', + () => Promise.resolve('put-target-' + Math.random()).then((objectName) => { + return MakeRequest('PUT', service.getSignedUrl('putObject', { + Bucket: 'private-with-cors', + Key: objectName, + ContentType: 'application/octet-stream' + // Consciously go for an unsigned payload + }), {'Content-Type': 'application/octet-stream'}, 'test') + .then(HasStatus(200, 'OK')) + .then(CheckS3Headers) + .then(HasHeaders({ + 'Content-Type': 'text/html; charset=UTF-8', + Etag: '"098f6bcd4621d373cade4e832627b4f6"' + })) + .then(HasNoBody) + .then((resp) => { + return MakeRequest('DELETE', service.getSignedUrl('deleteObject', { + Bucket: 'private-with-cors', + Key: objectName + })) + }) + .then(HasStatus(204, 'No Content')) + .then(CheckTransactionIdHeaders) + .then(HasNoBody) + })], ['GET If-Match matching', () => MakeS3Request(service, 'getObject', { Bucket: 'private-with-cors', diff --git a/test/unit/common/middleware/s3api/test_obj.py b/test/unit/common/middleware/s3api/test_obj.py index a2c886836d..ff1091eb3d 100644 --- a/test/unit/common/middleware/s3api/test_obj.py +++ b/test/unit/common/middleware/s3api/test_obj.py @@ -1771,6 +1771,41 @@ class TestS3ApiObj(S3ApiTestCase): 'Vary': 'Origin, Access-Control-Request-Headers', }) + # test presigned urls + req = Request.blank( + '/bucket/cors-object?AWSAccessKeyId=test%3Atester&' + 'Expires=1621558415&Signature=MKMdW3FpYcoFEJlTLF3EhP7AJgc%3D', + environ={'REQUEST_METHOD': 'OPTIONS'}, + headers={'Origin': 'http://example.com', + 'Access-Control-Request-Method': 'PUT'}) + status, headers, body = self.call_s3api(req) + self.assertEqual(status, '200 OK') + self.assertDictEqual(headers, { + 'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS', + 'Access-Control-Allow-Origin': 'http://example.com', + 'Access-Control-Allow-Methods': ('GET, HEAD, PUT, POST, DELETE, ' + 'OPTIONS'), + 'Vary': 'Origin, Access-Control-Request-Headers', + }) + req = Request.blank( + '/bucket/cors-object?X-Amz-Algorithm=AWS4-HMAC-SHA256&' + 'X-Amz-Credential=test%3Atester%2F20210521%2Fus-east-1%2Fs3%2F' + 'aws4_request&X-Amz-Date=20210521T003835Z&X-Amz-Expires=900&' + 'X-Amz-Signature=e413549f2cbeddb457c5fddb2d28820ce58de514bb900' + '5d588800d7ebb1a6a2d&X-Amz-SignedHeaders=host', + environ={'REQUEST_METHOD': 'OPTIONS'}, + headers={'Origin': 'http://example.com', + 'Access-Control-Request-Method': 'DELETE'}) + status, headers, body = self.call_s3api(req) + self.assertEqual(status, '200 OK') + self.assertDictEqual(headers, { + 'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS', + 'Access-Control-Allow-Origin': 'http://example.com', + 'Access-Control-Allow-Methods': ('GET, HEAD, PUT, POST, DELETE, ' + 'OPTIONS'), + 'Vary': 'Origin, Access-Control-Request-Headers', + }) + # Wrong protocol self.s3api.conf.cors_preflight_allow_origin = ['https://example.com'] status, headers, body = self.call_s3api(req)