openstack/swift
Code Issues Proposed changes

Merge "Trim sensitive information in the logs (CVE-2017-8761)"

Browse Source
This commit is contained in:
Zuul 2022-02-10 11:30:01 +00:00 committed by Gerrit Code Review
commit dc061dd95e
10 changed files with 421 additions and 100 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
doc/source/development_middleware.rst
View File

@ -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

24
etc/proxy-server.conf-sample
View File

@ -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

30
swift/common/middleware/proxy_logging.py
View File

@ -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

7
swift/common/middleware/s3api/s3api.py
View File

@ -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)

4
swift/common/middleware/tempurl.py
View File

@ -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)

33
swift/common/registry.py
View File

@ -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)

17
test/unit/common/middleware/s3api/test_s3api.py
View File

@ -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)

151
test/unit/common/middleware/test_proxy_logging.py
View File

@ -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,10 +931,10 @@ 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.ProxyLoggingMiddleware(FakeApp(), {})
app = proxy_logging.filter_factory({})(FakeApp())
app.access_logger = debug_logger()
req = Request.blank('/', environ={'REQUEST_METHOD': 'GET'})
resp = app(req.environ, start_response)
@ -932,26 +942,26 @@ class TestProxyLogging(unittest.TestCase):
log_parts = self._log_parts(app)
self.assertEqual(log_parts[9], '-')
# Has x-auth-token header
app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {})
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...')
self.assertEqual(log_parts[9], 'b05bf940-0464-4c...', log_parts)
# Truncate to first 8 characters
app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {
'reveal_sensitive_prefix': '8'})
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.ProxyLoggingMiddleware(FakeApp(), {
'reveal_sensitive_prefix': '8'})
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})
@ -961,8 +971,8 @@ class TestProxyLogging(unittest.TestCase):
self.assertEqual(log_parts[9], 'b05bf940...')
# Token length and reveal_sensitive_prefix are same (no truncate)
app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {
'reveal_sensitive_prefix': str(len(auth_token))})
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})
@ -972,8 +982,9 @@ class TestProxyLogging(unittest.TestCase):
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 = 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})
@ -983,16 +994,16 @@ class TestProxyLogging(unittest.TestCase):
self.assertEqual(log_parts[9], auth_token)
# Don't log x-auth-token
app = proxy_logging.ProxyLoggingMiddleware(FakeApp(), {
'reveal_sensitive_prefix': '0'})
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.ProxyLoggingMiddleware(FakeApp(), {
'reveal_sensitive_prefix': '0'})
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})
@ -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)

28
test/unit/common/middleware/test_tempurl.py
View File

@ -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'

89
test/unit/common/test_registry.py
View File

@ -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)