openstack/swift
Code Issues Proposed changes

Emit metrics for tempurl & formpost digest usage

Browse Source 
Without this, it's really hard for operators to feel comfortable
removing support for now-deprecated digests.

This patch adds new statsd metrics in the form of:

  formpost.digests.<digest>
  tempurl.digest.<digest>

Something like:

  formpost.digessts.sha1
  tempurl.digests.sha512

Change-Id: I203607a0576582330241172d05bf8fd223bbbb9d
This commit is contained in:
Tim Burke 2022-06-29 15:44:33 -07:00 committed by Matthew Oliver
parent 6af4449268
commit 475cdba65b
4 changed files with 31 additions and 14 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

11
swift/common/middleware/formpost.py
View File

@ -206,11 +206,12 @@ class FormPost(object):
:param conf: The configuration dict for the middleware.
"""
def __init__(self, app, conf):
def __init__(self, app, conf, logger=None):
#: The next WSGI application/filter in the paste.deploy pipeline.
self.app = app
#: The filter configuration dict.
self.conf = conf
self.logger = logger or get_logger(conf, log_route='formpost')
# Defaulting to SUPPORTED_DIGESTS just so we don't completely
# deprecate sha1 yet. We'll change this to DEFAULT_ALLOWED_DIGESTS
# later.
@ -413,13 +414,12 @@ class FormPost(object):
has_valid_sig = False
signature = attributes.get('signature', '')
try:
hash_algorithm, signature = extract_digest_and_algorithm(signature)
hash_name, signature = extract_digest_and_algorithm(signature)
except ValueError:
raise FormUnauthorized('invalid signature')
if hash_algorithm not in self.allowed_digests:
if hash_name not in self.allowed_digests:
raise FormUnauthorized('invalid signature')
if six.PY2:
hash_algorithm = getattr(hashlib, hash_algorithm)
hash_algorithm = getattr(hashlib, hash_name) if six.PY2 else hash_name
for key in keys:
# Encode key like in swift.common.utls.get_hmac.
@ -430,6 +430,7 @@ class FormPost(object):
has_valid_sig = True
if not has_valid_sig:
raise FormUnauthorized('invalid signature')
self.logger.increment('formpost.digests.%s' % hash_name)
substatus = [None]
subheaders = [None]

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

@ -423,11 +423,12 @@ class TempURL(object):
:param conf: The configuration dict for the middleware.
"""
def __init__(self, app, conf):
def __init__(self, app, conf, logger=None):
#: The next WSGI application/filter in the paste.deploy pipeline.
self.app = app
#: The filter configuration dict.
self.conf = conf
self.logger = logger or get_logger(conf, log_route='tempurl')
self.allowed_digests = conf.get(
'allowed_digests', DEFAULT_ALLOWED_DIGESTS.split())
@ -560,6 +561,7 @@ class TempURL(object):
break
if not is_valid_hmac:
return self._invalid(env, start_response)
self.logger.increment('tempurl.digests.%s' % hash_algorithm)
# disallowed headers prevent accidentally allowing upload of a pointer
# to data that the PUT tempurl would not otherwise allow access for.
# It should be safe to provide a GET tempurl for data that an
@ -867,4 +869,4 @@ def filter_factory(global_conf, **local_conf):
register_sensitive_param('temp_url_sig')
return lambda app: TempURL(app, conf)
return lambda app: TempURL(app, conf, logger)

20
test/unit/common/middleware/test_formpost.py
View File

@ -134,6 +134,7 @@ class TestFormPost(unittest.TestCase):
self.app = FakeApp()
self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth)
self.logger = self.formpost.logger = debug_logger()
def _make_request(self, path, tempurl_keys=(), **kwargs):
req = Request.blank(path, **kwargs)
@ -1489,10 +1490,8 @@ class TestFormPost(unittest.TestCase):
self._fake_cache_env('AUTH_test', [key]))
env['swift.infocache'][get_cache_key(
'AUTH_test', 'container')] = {'meta': {}}
self.app = FakeApp(iter([('201 Created', {}, b''),
('201 Created', {}, b'')]))
self.auth = tempauth.filter_factory({})(self.app)
self.formpost = formpost.filter_factory({})(self.auth)
self.auth.app = app = FakeApp(iter([('201 Created', {}, b''),
('201 Created', {}, b'')]))
status = [None]
headers = [None]
exc_info = [None]
@ -1514,14 +1513,21 @@ class TestFormPost(unittest.TestCase):
self.assertIsNone(location)
self.assertIsNone(exc_info)
self.assertTrue(b'201 Created' in body)
self.assertEqual(len(self.app.requests), 2)
self.assertEqual(self.app.requests[0].body, b'Test File\nOne\n')
self.assertEqual(self.app.requests[1].body, b'Test\nFile\nTwo\n')
self.assertEqual(len(app.requests), 2)
self.assertEqual(app.requests[0].body, b'Test File\nOne\n')
self.assertEqual(app.requests[1].body, b'Test\nFile\nTwo\n')
for digest in ('sha1', 'sha256', 'sha512'):
do_test(digest, True)
do_test(digest, False)
# NB: one increment per *upload*, not client request
self.assertEqual(self.logger.get_increment_counts(), {
'formpost.digests.sha1': 4,
'formpost.digests.sha256': 4,
'formpost.digests.sha512': 4,
})
def test_prefixed_and_not_prefixed_sigs_unsupported(self):
def do_test(digest, prefixed):
key = b'abc'

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

@ -78,6 +78,7 @@ class TestTempURL(unittest.TestCase):
self.app = FakeApp()
self.auth = tempauth.filter_factory({'reseller_prefix': ''})(self.app)
self.tempurl = tempurl.filter_factory({})(self.auth)
self.logger = self.tempurl.logger = debug_logger()
def _make_request(self, path, environ=None, keys=(), container_keys=None,
**kwargs):
@ -162,6 +163,7 @@ class TestTempURL(unittest.TestCase):
tempurl1 = tempurl.filter_factory({
'allowed_digests': 'sha1'})(self.auth)
tempurl1.logger = self.logger
sig = hmac.new(key, hmac_body, hashlib.sha1).hexdigest()
self.assert_valid_sig(expires, path, [key], sig, tempurl=tempurl1)
@ -176,6 +178,12 @@ class TestTempURL(unittest.TestCase):
key, hmac_body, hashlib.sha512).digest())
self.assert_valid_sig(expires, path, [key], b'sha512:' + sig)
self.assertEqual(self.logger.get_increment_counts(), {
'tempurl.digests.sha1': 1,
'tempurl.digests.sha256': 2,
'tempurl.digests.sha512': 1
})
def test_get_valid_key2(self):
method = 'GET'
expires = int(time() + 86400)