From f2c279bae94689e2062beb6d0030d168a0b4cbdf Mon Sep 17 00:00:00 2001 From: Matthew Oliver Date: Thu, 3 Feb 2022 16:29:53 +1100 Subject: [PATCH] Trim sensitive information in the logs (CVE-2017-8761) Several headers and query params were previously revealed in logs but are now redacted: * X-Auth-Token header (previously redacted in the {auth_token} field, but not the {headers} field) * temp_url_sig query param (used by tempurl middleware) * Authorization header and X-Amz-Signature and Signature query parameters (used by s3api middleware) This patch adds some new middleware helper methods to track headers and query parameters that should be redacted by proxy-logging. While instantiating the middleware, authors can call either: register_sensitive_header('case-insensitive-header-name') register_sensitive_param('case-sensitive-query-param-name') to add items that should be redacted. The redaction uses proxy-logging's existing reveal_sensitive_prefix config option to determine how much to reveal. Note that query params will still be logged in their entirety if eventlet_debug is enabled. UpgradeImpact ============= The reveal_sensitive_prefix config option now applies to more items; operators should review their currently-configured value to ensure it is appropriate for these new contexts. In particular, operators should consider reducing the value if it is more than 20 or so, even if that previously offered sufficient protection for auth tokens. Co-Authored-By: Tim Burke Closes-Bug: #1685798 Change-Id: I88b8cfd30292325e0870029058da6fb38026ae1a --- doc/source/development_middleware.rst | 14 +- etc/proxy-server.conf-sample | 24 +- swift/common/middleware/proxy_logging.py | 30 +- swift/common/middleware/s3api/s3api.py | 7 +- swift/common/middleware/tempurl.py | 4 +- swift/common/registry.py | 33 +++ .../common/middleware/s3api/test_s3api.py | 17 +- .../common/middleware/test_proxy_logging.py | 275 +++++++++++++----- test/unit/common/middleware/test_tempurl.py | 28 +- test/unit/common/test_registry.py | 89 +++++- 10 files changed, 421 insertions(+), 100 deletions(-) diff --git a/doc/source/development_middleware.rst b/doc/source/development_middleware.rst index 24d6fb7c79..1f7e9e3693 100644 --- a/doc/source/development_middleware.rst +++ b/doc/source/development_middleware.rst @@ -178,13 +178,25 @@ Middleware may advertize its availability and capabilities via Swift's :ref:`discoverability` support by using :func:`.register_swift_info`:: - from swift.common.utils import register_swift_info + from swift.common.registry import register_swift_info def webhook_factory(global_conf, **local_conf): register_swift_info('webhook') def webhook_filter(app): return WebhookMiddleware(app) return webhook_filter +If a middleware handles sensitive information in headers or query parameters +that may need redaction when logging, use the :func:`.register_sensitive_header` +and :func:`.register_sensitive_param` functions. This should be done in the +filter factory:: + + from swift.common.registry import register_sensitive_header + def webhook_factory(global_conf, **local_conf): + register_sensitive_header('webhook-api-key') + def webhook_filter(app): + return WebhookMiddleware(app) + return webhook_filter + -------------- Swift Metadata diff --git a/etc/proxy-server.conf-sample b/etc/proxy-server.conf-sample index 759bd7cd25..ef49c430f7 100644 --- a/etc/proxy-server.conf-sample +++ b/etc/proxy-server.conf-sample @@ -86,6 +86,9 @@ bind_port = 8080 # cors_expose_headers = # # client_timeout = 60.0 +# +# Note: enabling evenlet_debug might reveal sensitive information, for example +# signatures for temp urls # eventlet_debug = false # # You can set scheduling priority of processes. Niceness values range from -20 @@ -998,16 +1001,17 @@ use = egg:swift#proxy_logging # list like this: access_log_headers_only = Host, X-Object-Meta-Mtime # access_log_headers_only = # -# By default, the X-Auth-Token is logged. To obscure the value, -# set reveal_sensitive_prefix to the number of characters to log. -# For example, if set to 12, only the first 12 characters of the -# token appear in the log. An unauthorized access of the log file -# won't allow unauthorized usage of the token. However, the first -# 12 or so characters is unique enough that you can trace/debug -# token usage. Set to 0 to suppress the token completely (replaced -# by '...' in the log). -# Note: reveal_sensitive_prefix will not affect the value -# logged with access_log_headers=True. +# The default log format includes several sensitive values in logs: +# * X-Auth-Token header +# * temp_url_sig query parameter +# * Authorization header +# * X-Amz-Signature query parameter +# To prevent an unauthorized access of the log file leading to an unauthorized +# access of cluster data, only a portion of these values are written, with the +# remainder replaced by '...' in the log. Set reveal_sensitive_prefix to the +# number of characters to log. Set to 0 to suppress the values entirely; set +# to something large (1000, say) to write full values. Note that some values +# may start appearing in full at values as low as 33. # reveal_sensitive_prefix = 16 # # What HTTP methods are allowed for StatsD logging (comma-sep); request methods diff --git a/swift/common/middleware/proxy_logging.py b/swift/common/middleware/proxy_logging.py index 74245a0a18..29920aaac4 100644 --- a/swift/common/middleware/proxy_logging.py +++ b/swift/common/middleware/proxy_logging.py @@ -84,6 +84,8 @@ from swift.common.utils import (get_logger, get_remote_client, LogStringFormatter) from swift.common.storage_policy import POLICIES +from swift.common.registry import get_sensitive_headers, \ + get_sensitive_params, register_sensitive_header class ProxyLoggingMiddleware(object): @@ -200,6 +202,24 @@ class ProxyLoggingMiddleware(object): return value[:self.reveal_sensitive_prefix] + '...' return value + def obscure_req(self, req): + for header in get_sensitive_headers(): + if header in req.headers: + req.headers[header] = \ + self.obscure_sensitive(req.headers[header]) + + obscure_params = get_sensitive_params() + new_params = [] + any_obscured = False + for k, v in req.params.items(): + if k in obscure_params: + new_params.append((k, self.obscure_sensitive(v))) + any_obscured = True + else: + new_params.append((k, v)) + if any_obscured: + req.params = new_params + def log_request(self, req, status_int, bytes_received, bytes_sent, start_time, end_time, resp_headers=None, ttfb=0, wire_status_int=None): @@ -215,6 +235,7 @@ class ProxyLoggingMiddleware(object): :param resp_headers: dict of the response headers :param wire_status_int: the on the wire status int """ + self.obscure_req(req) resp_headers = resp_headers or {} logged_headers = None if self.log_hdrs: @@ -270,8 +291,7 @@ class ProxyLoggingMiddleware(object): req.environ.get('SERVER_PROTOCOL'), 'status_int': status_int, 'auth_token': - self.obscure_sensitive( - req.headers.get('x-auth-token')), + req.headers.get('x-auth-token'), 'bytes_recvd': bytes_received, 'bytes_sent': bytes_sent, 'transaction_id': req.environ.get('swift.trans_id'), @@ -445,6 +465,12 @@ def filter_factory(global_conf, **local_conf): conf = global_conf.copy() conf.update(local_conf) + # Normally it would be the middleware that uses the header that + # would register it, but because there could be 3rd party auth middlewares + # that use 'x-auth-token' or 'x-storage-token' we special case it here. + register_sensitive_header('x-auth-token') + register_sensitive_header('x-storage-token') + def proxy_logger(app): return ProxyLoggingMiddleware(app, conf) return proxy_logger diff --git a/swift/common/middleware/s3api/s3api.py b/swift/common/middleware/s3api/s3api.py index 2046d0b70b..a3518fbe65 100644 --- a/swift/common/middleware/s3api/s3api.py +++ b/swift/common/middleware/s3api/s3api.py @@ -160,7 +160,8 @@ from swift.common.utils import get_logger, config_true_value, \ config_positive_int_value, split_path, 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 -from swift.common.registry import register_swift_info +from swift.common.registry import register_swift_info, \ + register_sensitive_header, register_sensitive_param class ListingEtagMiddleware(object): @@ -464,6 +465,10 @@ def filter_factory(global_conf, **local_conf): s3_acl=config_true_value(conf.get('s3_acl', False)), ) + register_sensitive_header('authorization') + register_sensitive_param('Signature') + register_sensitive_param('X-Amz-Signature') + def s3api_filter(app): return S3ApiMiddleware(ListingEtagMiddleware(app), conf) diff --git a/swift/common/middleware/tempurl.py b/swift/common/middleware/tempurl.py index c7169eba0f..629c175423 100644 --- a/swift/common/middleware/tempurl.py +++ b/swift/common/middleware/tempurl.py @@ -315,7 +315,7 @@ from swift.common.swob import header_to_environ_key, HTTPUnauthorized, \ HTTPBadRequest, wsgi_to_str from swift.common.utils import split_path, get_valid_utf8_str, \ get_hmac, streq_const_time, quote, get_logger, strict_b64decode -from swift.common.registry import register_swift_info +from swift.common.registry import register_swift_info, register_sensitive_param DISALLOWED_INCOMING_HEADERS = 'x-object-manifest x-symlink-target' @@ -873,4 +873,6 @@ def filter_factory(global_conf, **local_conf): register_swift_info('tempurl', **info_conf) conf.update(info_conf) + register_sensitive_param('temp_url_sig') + return lambda app: TempURL(app, conf) diff --git a/swift/common/registry.py b/swift/common/registry.py index 9e515ae237..912d6590ba 100644 --- a/swift/common/registry.py +++ b/swift/common/registry.py @@ -16,6 +16,7 @@ # Used by get_swift_info and register_swift_info to store information about # the swift cluster. from copy import deepcopy +import six _swift_info = {} _swift_admin_info = {} @@ -84,3 +85,35 @@ def register_swift_info(name='swift', admin=False, **kwargs): if "." in key: raise ValueError('Cannot use "." in a swift_info key: %s' % key) dict_to_use[name][key] = val + + +_sensitive_headers = set() +_sensitive_params = set() + + +def get_sensitive_headers(): + return frozenset(_sensitive_headers) + + +def register_sensitive_header(header): + if not isinstance(header, str): + raise TypeError + if six.PY2: + header.decode('ascii') + else: + header.encode('ascii') + _sensitive_headers.add(header) + + +def get_sensitive_params(): + return frozenset(_sensitive_params) + + +def register_sensitive_param(query_param): + if not isinstance(query_param, str): + raise TypeError + if six.PY2: + query_param.decode('ascii') + else: + query_param.encode('ascii') + _sensitive_params.add(query_param) diff --git a/test/unit/common/middleware/s3api/test_s3api.py b/test/unit/common/middleware/s3api/test_s3api.py index 09c9cac0df..85a22a2296 100644 --- a/test/unit/common/middleware/s3api/test_s3api.py +++ b/test/unit/common/middleware/s3api/test_s3api.py @@ -760,8 +760,8 @@ class TestS3ApiMiddleware(S3ApiTestCase): def test_mfa(self): self._test_unsupported_header('x-amz-mfa') - @mock.patch.object(registry, '_swift_admin_info', new_callable=dict) - def test_server_side_encryption(self, mock_info): + @mock.patch.object(registry, '_swift_admin_info', dict()) + def test_server_side_encryption(self): sse_header = 'x-amz-server-side-encryption' self._test_unsupported_header(sse_header, 'AES256') self._test_unsupported_header(sse_header, 'aws:kms') @@ -868,6 +868,19 @@ class TestS3ApiMiddleware(S3ApiTestCase): self.assertEqual(elem.find('./Method').text, 'POST') self.assertEqual(elem.find('./ResourceType').text, 'ACL') + @mock.patch.object(registry, '_sensitive_headers', set()) + @mock.patch.object(registry, '_sensitive_params', set()) + def test_registered_sensitive_info(self): + self.assertFalse(registry.get_sensitive_headers()) + self.assertFalse(registry.get_sensitive_params()) + filter_factory(self.conf) + sensitive = registry.get_sensitive_headers() + self.assertIn('authorization', sensitive) + sensitive = registry.get_sensitive_params() + self.assertIn('X-Amz-Signature', sensitive) + self.assertIn('Signature', sensitive) + + @mock.patch.object(registry, '_swift_info', dict()) def test_registered_defaults(self): conf_from_file = {k: str(v) for k, v in self.conf.items()} filter_factory(conf_from_file) diff --git a/test/unit/common/middleware/test_proxy_logging.py b/test/unit/common/middleware/test_proxy_logging.py index f1b9d32593..2fe7bfa438 100644 --- a/test/unit/common/middleware/test_proxy_logging.py +++ b/test/unit/common/middleware/test_proxy_logging.py @@ -23,8 +23,10 @@ from six.moves.urllib.parse import unquote from swift.common.utils import get_logger, split_path, StatsdClient from swift.common.middleware import proxy_logging +from swift.common.registry import register_sensitive_header, \ + register_sensitive_param, get_sensitive_headers from swift.common.swob import Request, Response -from swift.common import constraints +from swift.common import constraints, registry from swift.common.storage_policy import StoragePolicy from test.debug_logger import debug_logger from test.unit import patch_policies @@ -712,6 +714,14 @@ class TestProxyLogging(unittest.TestCase): self.assertTrue(callable(factory)) self.assertTrue(callable(factory(FakeApp()))) + def test_sensitive_headers_registered(self): + with mock.patch.object(registry, '_sensitive_headers', set()): + self.assertNotIn('x-auth-token', get_sensitive_headers()) + self.assertNotIn('x-storage-token', get_sensitive_headers()) + proxy_logging.filter_factory({})(FakeApp()) + self.assertIn('x-auth-token', get_sensitive_headers()) + self.assertIn('x-storage-token', get_sensitive_headers()) + def test_unread_body(self): app = proxy_logging.ProxyLoggingMiddleware( FakeApp(['some', 'stuff']), {}) @@ -921,89 +931,90 @@ class TestProxyLogging(unittest.TestCase): def test_log_auth_token(self): auth_token = 'b05bf940-0464-4c0e-8c70-87717d2d73e8' + with mock.patch.object(registry, '_sensitive_headers', set()): + # Default - reveal_sensitive_prefix is 16 + # No x-auth-token header + app = proxy_logging.filter_factory({})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], '-') + # Has x-auth-token header + app = proxy_logging.filter_factory({})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_AUTH_TOKEN': auth_token}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], 'b05bf940-0464-4c...', log_parts) - # Default - reveal_sensitive_prefix is 16 - # No x-auth-token header - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], '-') - # Has x-auth-token header - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', - 'HTTP_X_AUTH_TOKEN': auth_token}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], 'b05bf940-0464-4c...') + # Truncate to first 8 characters + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': '8'})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], '-') + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': '8'})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_AUTH_TOKEN': auth_token}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], 'b05bf940...') - # Truncate to first 8 characters - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': '8'}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], '-') - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': '8'}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', - 'HTTP_X_AUTH_TOKEN': auth_token}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], 'b05bf940...') + # Token length and reveal_sensitive_prefix are same (no truncate) + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': str(len(auth_token))})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_AUTH_TOKEN': auth_token}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], auth_token) - # Token length and reveal_sensitive_prefix are same (no truncate) - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': str(len(auth_token))}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', - 'HTTP_X_AUTH_TOKEN': auth_token}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], auth_token) + # No effective limit on auth token + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': constraints.MAX_HEADER_SIZE} + )(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_AUTH_TOKEN': auth_token}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], auth_token) - # No effective limit on auth token - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': constraints.MAX_HEADER_SIZE}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', - 'HTTP_X_AUTH_TOKEN': auth_token}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], auth_token) + # Don't log x-auth-token + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': '0'})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], '-') + app = proxy_logging.filter_factory( + {'reveal_sensitive_prefix': '0'})(FakeApp()) + app.access_logger = debug_logger() + req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', + 'HTTP_X_AUTH_TOKEN': auth_token}) + resp = app(req.environ, start_response) + resp_body = b''.join(resp) + log_parts = self._log_parts(app) + self.assertEqual(log_parts[9], '...') - # Don't log x-auth-token - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': '0'}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], '-') - app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), { - 'reveal_sensitive_prefix': '0'}) - app.access_logger = debug_logger() - req = Request.blank('/', environ={'REQUEST_METHOD': 'GET', - 'HTTP_X_AUTH_TOKEN': auth_token}) - resp = app(req.environ, start_response) - resp_body = b''.join(resp) - log_parts = self._log_parts(app) - self.assertEqual(log_parts[9], '...') - - # Avoids pyflakes error, "local variable 'resp_body' is assigned to - # but never used - self.assertTrue(resp_body is not None) + # Avoids pyflakes error, "local variable 'resp_body' is assigned to + # but never used + self.assertTrue(resp_body is not None) def test_ensure_fields(self): app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {}) @@ -1151,3 +1162,109 @@ class TestProxyLogging(unittest.TestCase): b''.join(resp) log_parts = self._log_parts(app) self.assertEqual(log_parts[20], '1') + + def test_obscure_req(self): + app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {}) + app.access_logger = debug_logger() + + params = [('param_one', + 'some_long_string_that_might_need_to_be_obscured'), + ('param_two', + "super_secure_param_that_needs_to_be_obscured")] + headers = {'X-Auth-Token': 'this_is_my_auth_token', + 'X-Other-Header': 'another_header_that_we_may_obscure'} + + req = Request.blank('a/c/o', environ={'REQUEST_METHOD': 'GET'}, + headers=headers) + req.params = params + + # if nothing is sensitive, nothing will be obscured + with mock.patch.object(registry, '_sensitive_params', set()): + with mock.patch.object(registry, '_sensitive_headers', set()): + app.obscure_req(req) + # show that nothing changed + for header, expected_value in headers.items(): + self.assertEqual(req.headers[header], expected_value) + + for param, expected_value in params: + self.assertEqual(req.params[param], expected_value) + + # If an obscured param or header doesn't exist in a req, that's fine + with mock.patch.object(registry, '_sensitive_params', set()): + with mock.patch.object(registry, '_sensitive_headers', set()): + register_sensitive_header('X-Not-Exist') + register_sensitive_param('non-existent-param') + app.obscure_req(req) + + # show that nothing changed + for header, expected_value in headers.items(): + self.assertEqual(req.headers[header], expected_value) + + for param, expected_value in params: + self.assertEqual(req.params[param], expected_value) + + def obscured_test(params, headers, params_to_add, headers_to_add, + expected_params, expected_headers): + with mock.patch.object(registry, '_sensitive_params', set()): + with mock.patch.object(registry, '_sensitive_headers', set()): + app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {}) + app.access_logger = debug_logger() + req = Request.blank('a/c/o', + environ={'REQUEST_METHOD': 'GET'}, + headers=dict(headers)) + req.params = params + for param in params_to_add: + register_sensitive_param(param) + + for header in headers_to_add: + register_sensitive_header(header) + + app.obscure_req(req) + for header, expected_value in expected_headers.items(): + self.assertEqual(req.headers[header], expected_value) + + for param, expected_value in expected_params: + self.assertEqual(req.params[param], expected_value) + + # first just 1 param + expected_params = list(params) + expected_params[0] = ('param_one', 'some_long_string...') + obscured_test(params, headers, ['param_one'], [], expected_params, + headers) + # case sensitive + expected_params = list(params) + obscured_test(params, headers, ['Param_one'], [], expected_params, + headers) + # Other param + expected_params = list(params) + expected_params[1] = ('param_two', 'super_secure_par...') + obscured_test(params, headers, ['param_two'], [], expected_params, + headers) + # both + expected_params[0] = ('param_one', 'some_long_string...') + obscured_test(params, headers, ['param_two', 'param_one'], [], + expected_params, headers) + + # Now the headers + # first just 1 header + expected_headers = headers.copy() + expected_headers["X-Auth-Token"] = 'this_is_my_auth_...' + obscured_test(params, headers, [], ['X-Auth-Token'], params, + expected_headers) + # case insensitive + obscured_test(params, headers, [], ['x-auth-token'], params, + expected_headers) + # Other headers + expected_headers = headers.copy() + expected_headers["X-Other-Header"] = 'another_header_t...' + obscured_test(params, headers, [], ['X-Other-Header'], params, + expected_headers) + # both + expected_headers["X-Auth-Token"] = 'this_is_my_auth_...' + obscured_test(params, headers, [], ['X-Auth-Token', 'X-Other-Header'], + params, expected_headers) + + # all together + obscured_test(params, headers, ['param_two', 'param_one'], + ['X-Auth-Token', 'X-Other-Header'], + expected_params, expected_headers) diff --git a/test/unit/common/middleware/test_tempurl.py b/test/unit/common/middleware/test_tempurl.py index 00546d7c3f..115f0e5255 100644 --- a/test/unit/common/middleware/test_tempurl.py +++ b/test/unit/common/middleware/test_tempurl.py @@ -38,10 +38,11 @@ import six from six.moves.urllib.parse import quote from time import time, strftime, gmtime -from swift.common.middleware import tempauth, tempurl +from swift.common.middleware import tempauth, tempurl, proxy_logging from swift.common.header_key_dict import HeaderKeyDict from swift.common.swob import Request, Response from swift.common import utils, registry +from test.debug_logger import debug_logger class FakeApp(object): @@ -206,6 +207,31 @@ class TestTempURL(unittest.TestCase): for sig in (sig1, sig2): self.assert_valid_sig(expires, path, account_keys, sig, environ) + def test_signature_trim(self): + # Insert proxy logging into the pipeline + p_logging = proxy_logging.filter_factory({})(self.app) + self.auth = tempauth.filter_factory({'reseller_prefix': ''})( + p_logging) + self.tempurl = tempurl.filter_factory({})(self.auth) + + sig = 'valid_sigs_will_be_exactly_40_characters' + expires = int(time() + 1000) + p_logging.access_logger.logger = debug_logger('fake') + resp = self._make_request( + '/v1/a/c/o?temp_url_sig=%s&temp_url_expires=%d' % (sig, expires)) + + with mock.patch('swift.common.middleware.tempurl.TempURL._get_keys', + return_value=[('key', tempurl.CONTAINER_SCOPE)]): + with mock.patch( + 'swift.common.middleware.tempurl.TempURL._get_hmacs', + return_value=[(sig, tempurl.CONTAINER_SCOPE)]): + resp.get_response(self.tempurl) + trimmed_sig_qs = '%s...' % sig[:16] + info_lines = p_logging.access_logger. \ + logger.get_lines_for_level('info') + + self.assertIn(trimmed_sig_qs, info_lines[0]) + @mock.patch('swift.common.middleware.tempurl.time', return_value=0) def test_get_valid_with_filename(self, mock_time): method = 'GET' diff --git a/test/unit/common/test_registry.py b/test/unit/common/test_registry.py index 6fc17b3ba5..6960c47502 100644 --- a/test/unit/common/test_registry.py +++ b/test/unit/common/test_registry.py @@ -15,14 +15,19 @@ from swift.common import registry, utils +import mock import unittest class TestSwiftInfo(unittest.TestCase): - def tearDown(self): - registry._swift_info = {} - registry._swift_admin_info = {} + def setUp(self): + patcher = mock.patch.object(registry, '_swift_info', dict()) + patcher.start() + self.addCleanup(patcher.stop) + patcher = mock.patch.object(registry, '_swift_admin_info', dict()) + patcher.start() + self.addCleanup(patcher.stop) def test_register_swift_info(self): registry.register_swift_info(foo='bar') @@ -211,3 +216,81 @@ class TestSwiftInfo(unittest.TestCase): self.assertEqual(registry._swift_info['swift']['foo'], 'bar') self.assertEqual(registry.get_swift_info(admin=True), utils.get_swift_info(admin=True)) + + +class TestSensitiveRegistry(unittest.TestCase): + def setUp(self): + patcher = mock.patch.object(registry, '_sensitive_headers', set()) + patcher.start() + self.addCleanup(patcher.stop) + patcher = mock.patch.object(registry, '_sensitive_params', set()) + patcher.start() + self.addCleanup(patcher.stop) + + def test_register_sensitive_header(self): + self.assertFalse(registry._sensitive_headers) + + registry.register_sensitive_header('Some-Header') + expected_headers = {'Some-Header'} + self.assertEqual(expected_headers, registry._sensitive_headers) + + expected_headers.add("New-Header") + registry.register_sensitive_header("New-Header") + self.assertEqual(expected_headers, registry._sensitive_headers) + + for header_not_str in (1, None, 1.1): + with self.assertRaises(TypeError): + registry.register_sensitive_header(header_not_str) + self.assertEqual(expected_headers, registry._sensitive_headers) + + with self.assertRaises(UnicodeError): + registry.register_sensitive_header('\xe2\x98\x83') + self.assertEqual(expected_headers, registry._sensitive_headers) + + def test_register_sensitive_param(self): + self.assertFalse(registry._sensitive_params) + + registry.register_sensitive_param('some_param') + expected_params = {'some_param'} + self.assertEqual(expected_params, registry._sensitive_params) + + expected_params.add("another") + registry.register_sensitive_param("another") + self.assertEqual(expected_params, registry._sensitive_params) + + for param_not_str in (1, None, 1.1): + with self.assertRaises(TypeError): + registry.register_sensitive_param(param_not_str) + self.assertEqual(expected_params, registry._sensitive_params) + + with self.assertRaises(UnicodeError): + registry.register_sensitive_param('\xe2\x98\x83') + self.assertEqual(expected_params, registry._sensitive_params) + + def test_get_sensitive_headers(self): + self.assertFalse(registry.get_sensitive_headers()) + + registry.register_sensitive_header('Header1') + self.assertEqual(registry.get_sensitive_headers(), {'Header1'}) + self.assertEqual(registry.get_sensitive_headers(), + registry._sensitive_headers) + + registry.register_sensitive_header('Header2') + self.assertEqual(registry.get_sensitive_headers(), + {'Header1', 'Header2'}) + self.assertEqual(registry.get_sensitive_headers(), + registry._sensitive_headers) + + def test_get_sensitive_params(self): + self.assertFalse(registry.get_sensitive_params()) + + registry.register_sensitive_param('Param1') + self.assertEqual(registry.get_sensitive_params(), {'Param1'}) + self.assertEqual(registry.get_sensitive_params(), + registry._sensitive_params) + + registry.register_sensitive_param('param') + self.assertEqual(registry.get_sensitive_params(), + {'Param1', 'param'}) + self.assertEqual(registry.get_sensitive_params(), + registry._sensitive_params)