s3api: Allow CORS preflight requests

Unfortunately, we can't identify the user, so we can't map to an
account, so we can't respect whatever CORS metadata might be set on the
container.

As a result, the allowed origins must be configured cluster-wide. Add a
new config option, cors_preflight_allow_origin, for that; default it
to blank (ie, deny preflights from all origins, preserving existing
behavior), but allow either a comma-separated list of origins or
* (to allow all origins).

Change-Id: I985143bf03125a05792e79bc5e5f83722d6431b3
Co-Authored-By: Matthew Oliver <matt@oliver.net.au>
This commit is contained in:
Tim Burke 2020-02-27 16:25:44 -08:00
parent 81db980690
commit 27a734c78a
7 changed files with 161 additions and 11 deletions

View File

@ -92,6 +92,7 @@ use = egg:swift#symlink
use = egg:swift#s3api
s3_acl = yes
check_bucket_owner = yes
cors_preflight_allow_origin = *
# Example to create root secret: `openssl rand -base64 32`
[filter:keymaster]

View File

@ -82,6 +82,7 @@ use = egg:swift#symlink
# To enable, add the s3api middleware to the pipeline before tempauth
[filter:s3api]
use = egg:swift#s3api
cors_preflight_allow_origin = *
# Example to create root secret: `openssl rand -base64 32`
[filter:keymaster]

View File

@ -629,6 +629,12 @@ use = egg:swift#s3api
# AWS allows clock skew up to 15 mins; note that older versions of swift/swift3
# allowed at most 5 mins.
# allowable_clock_skew = 900
#
# CORS preflight requests don't contain enough information for us to
# identify the account that should be used for the real request, so
# the allowed origins must be set cluster-wide. (default: blank; all
# preflight requests will be denied)
# cors_preflight_allow_origin =
# You can override the default log routing for this filter here:
# log_name = s3api

View File

@ -157,7 +157,7 @@ from swift.common.middleware.s3api.s3response import ErrorResponse, \
InternalError, MethodNotAllowed, S3ResponseBase, S3NotImplemented
from swift.common.utils import get_logger, register_swift_info, \
config_true_value, config_positive_int_value, split_path, \
closing_if_possible
closing_if_possible, list_from_csv
from swift.common.middleware.s3api.utils import Config
from swift.common.middleware.s3api.acl_handlers import get_acl_handler
@ -277,12 +277,43 @@ class S3ApiMiddleware(object):
wsgi_conf.get('min_segment_size', 5242880))
self.conf.allowable_clock_skew = config_positive_int_value(
wsgi_conf.get('allowable_clock_skew', 15 * 60))
self.conf.cors_preflight_allow_origin = list_from_csv(wsgi_conf.get(
'cors_preflight_allow_origin', ''))
if '*' in self.conf.cors_preflight_allow_origin and \
len(self.conf.cors_preflight_allow_origin) > 1:
raise ValueError('if cors_preflight_allow_origin should include '
'all domains, * must be the only entry')
self.logger = get_logger(
wsgi_conf, log_route=wsgi_conf.get('log_name', 's3api'))
self.check_pipeline(wsgi_conf)
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/')):
# 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:
start_response('401 Unauthorized', [
('Allow', 'GET, HEAD, PUT, POST, DELETE, OPTIONS'),
])
return [b'']
start_response('200 OK', [
('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'),
])
return [b'']
try:
req_class = get_request_class(env, self.conf.s3_acl)
req = req_class(env, self.app, self.conf)

View File

@ -133,28 +133,69 @@ function makeTests (params) {
Bucket: 'private-with-cors',
Key: 'obj'
})
.then(CorsBlocked)], // Pre-flight failed
['PUT',
() => MakeS3Request(service, 'putObject', {
.then(HasStatus(200, 'OK'))
.then(CheckS3Headers)
.then(HasHeaders(['x-amz-meta-mtime']))
.then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
.then(HasHeaders({
'Content-Type': 'application/octet-stream',
Etag: '"0f343b0931126a20f133d67c2b018a3b"'
}))
.then(BodyHasLength(1024))],
['PUT then DELETE',
() => Promise.resolve('put-target-' + Math.random()).then((objectName) => {
return MakeS3Request(service, 'putObject', {
Bucket: 'private-with-cors',
Key: 'put-target',
Key: objectName,
Body: 'test'
})
.then(CorsBlocked)], // Pre-flight failed
.then(HasStatus(200, 'OK'))
.then(CheckS3Headers)
.then(HasHeaders({
'Content-Type': 'text/html; charset=UTF-8',
Etag: '"098f6bcd4621d373cade4e832627b4f6"'
}))
.then(HasNoBody)
.then((resp) => {
return MakeS3Request(service, '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',
Key: 'obj',
IfMatch: '0f343b0931126a20f133d67c2b018a3b'
})
.then(CorsBlocked)], // Pre-flight failed
.then(HasStatus(200, 'OK'))
.then(CheckS3Headers)
.then(HasHeaders(['x-amz-meta-mtime']))
.then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
.then(HasHeaders({
'Content-Type': 'application/octet-stream',
Etag: '"0f343b0931126a20f133d67c2b018a3b"'
}))
.then(BodyHasLength(1024))],
['GET Range',
() => MakeS3Request(service, 'getObject', {
Bucket: 'private-with-cors',
Key: 'obj',
Range: 'bytes=100-199'
})
.then(CorsBlocked)], // Pre-flight failed
.then(HasStatus(206, 'Partial Content'))
.then(CheckS3Headers)
.then(HasHeaders(['x-amz-meta-mtime']))
.then(DoesNotHaveHeaders(['X-Object-Meta-Mtime']))
.then(HasHeaders({
'Content-Type': 'application/octet-stream',
Etag: '"0f343b0931126a20f133d67c2b018a3b"'
}))
.then(BodyHasLength(100))]
]
}

View File

@ -1724,6 +1724,47 @@ class TestS3ApiObj(S3ApiTestCase):
'test:write', 'READ', src_path='')
self.assertEqual(status.split()[0], '400')
def test_cors_preflight(self):
req = Request.blank(
'/bucket/cors-object',
environ={'REQUEST_METHOD': 'OPTIONS'},
headers={'Origin': 'http://example.com',
'Access-Control-Request-Method': 'GET',
'Access-Control-Request-Headers': 'authorization'})
self.s3api.conf.cors_preflight_allow_origin = ['*']
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'),
'Access-Control-Allow-Headers': 'authorization',
'Vary': 'Origin, Access-Control-Request-Headers',
})
# test more allow_origins
self.s3api.conf.cors_preflight_allow_origin = ['http://example.com',
'http://other.com']
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'),
'Access-Control-Allow-Headers': 'authorization',
'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)
self.assertEqual(status, '401 Unauthorized')
self.assertEqual(headers, {
'Allow': 'GET, HEAD, PUT, POST, DELETE, OPTIONS',
})
def test_cors_headers(self):
# note: Access-Control-Allow-Methods would normally be expected in
# response to an OPTIONS request but its included here in GET/PUT tests

View File

@ -118,6 +118,7 @@ class TestS3ApiMiddleware(S3ApiTestCase):
'min_segment_size': 5242880,
'multi_delete_concurrency': 2,
's3_acl': False,
'cors_preflight_allow_origin': [],
})
s3api = S3ApiMiddleware(None, {})
self.assertEqual(expected, s3api.conf)
@ -140,10 +141,38 @@ class TestS3ApiMiddleware(S3ApiTestCase):
'min_segment_size': 1000000,
'multi_delete_concurrency': 1,
's3_acl': True,
'cors_preflight_allow_origin': 'foo.example.com,bar.example.com',
}
s3api = S3ApiMiddleware(None, conf)
conf['cors_preflight_allow_origin'] = \
conf['cors_preflight_allow_origin'].split(',')
self.assertEqual(conf, s3api.conf)
# test allow_origin list with a '*' fails.
conf = {
'storage_domain': 'somewhere',
'location': 'us-west-1',
'force_swift_request_proxy_log': True,
'dns_compliant_bucket_names': False,
'allow_multipart_uploads': False,
'allow_no_owner': True,
'allowable_clock_skew': 300,
'auth_pipeline_check': False,
'check_bucket_owner': True,
'max_bucket_listing': 500,
'max_multi_delete_objects': 600,
'max_parts_listing': 70,
'max_upload_part_num': 800,
'min_segment_size': 1000000,
'multi_delete_concurrency': 1,
's3_acl': True,
'cors_preflight_allow_origin': 'foo.example.com,bar.example.com,*',
}
with self.assertRaises(ValueError) as ex:
S3ApiMiddleware(None, conf)
self.assertIn("if cors_preflight_allow_origin should include all "
"domains, * must be the only entry", str(ex.exception))
def check_bad_positive_ints(**kwargs):
bad_conf = dict(conf, **kwargs)
self.assertRaises(ValueError, S3ApiMiddleware, None, bad_conf)