From e5fd323ae0376b76d9239ee0d0ba66b5fec4fe22 Mon Sep 17 00:00:00 2001
From: Tim Burke <tim.burke@gmail.com>
Date: Mon, 24 Apr 2017 16:06:02 -0700
Subject: [PATCH] Don't log complete signatures for pre-signed URLs

Trim the Signature and X-Amz-Signature query params after to prevent the
proxy-logging middlewares from logging enough information to reconstruct
a valid request.

This is particularly important for long-lived pre-signed URLs or when
using central logging servers.

Change-Id: I6b15407b56f1ca236f26152799f201427ea27102
Closes-Bug: #1685798
---
 swift3/middleware.py                | 25 ++++++++++++++++-
 swift3/test/unit/test_middleware.py | 56 +++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+), 1 deletion(-)

diff --git a/swift3/middleware.py b/swift3/middleware.py
index afbb4c9..dd8aeb8 100644
--- a/swift3/middleware.py
+++ b/swift3/middleware.py
@@ -53,6 +53,7 @@ following for an SAIO setup::
 """
 
 from paste.deploy import loadwsgi
+from six.moves import urllib
 
 from swift.common.wsgi import PipelineWrapper, loadcontext
 
@@ -65,6 +66,27 @@ from swift3.cfg import CONF
 from swift3.utils import LOGGER
 from swift.common.utils import get_logger, register_swift_info
 
+class SignatureScrubber(object):
+    def __init__(self, app, scrub_before_app_call=False):
+        self.app = app
+        self.scrub_before_app_call = scrub_before_app_call
+
+    def scrub(self, env):
+        params = dict(urllib.parse.parse_qsl(env['QUERY_STRING'], True))
+        for key in ('Signature', 'X-Amz-Signature'):
+            if key in params:
+                params[key] = params[key][:8] + '...'
+        env['QUERY_STRING'] = urllib.parse.urlencode(params)
+
+    def __call__(self, env, start_response):
+        if self.scrub_before_app_call:
+            self.scrub(env)
+        try:
+            return self.app(env, start_response)
+        finally:
+            if not self.scrub_before_app_call:
+                self.scrub(env)
+
 
 class Swift3Middleware(object):
     """Swift3 S3 compatibility middleware"""
@@ -194,6 +216,7 @@ def filter_factory(global_conf, **local_conf):
     )
 
     def swift3_filter(app):
-        return Swift3Middleware(app, CONF)
+        return SignatureScrubber(Swift3Middleware(
+            SignatureScrubber(app, True), CONF))
 
     return swift3_filter
diff --git a/swift3/test/unit/test_middleware.py b/swift3/test/unit/test_middleware.py
index f31a5f6..4a6710a 100644
--- a/swift3/test/unit/test_middleware.py
+++ b/swift3/test/unit/test_middleware.py
@@ -904,6 +904,62 @@ class TestSwift3Middleware(Swift3TestCase):
         status, headers, body = self.call_swift3(req)
         self.assertEqual(status.split()[0], '403', body)
 
+    def _test_signature_scrubbing(self, req):
+        def check_params(params):
+            made_assertions = False
+            for key in ('Signature', 'X-Amz-Signature'):
+                if key not in params:
+                    continue
+                self.assertEqual(11, len(params[key]))  # Too short to be good!
+                self.assertTrue(params[key].endswith('...'), params[key])
+                made_assertions = True
+            self.assertTrue(made_assertions, 'Found neither Signature nor '
+                                             'X-Amz-Signature in request')
+
+        def fake_app(env, start_response):
+            params = dict(urllib.parse.parse_qsl(env['QUERY_STRING'], True))
+            check_params(params)
+
+            # Not representative of what an app would do, but simulate some
+            # early return within swift3, like we never called the app
+            params['Signature'] = env['swift3.auth_details']['signature']
+            params['X-Amz-Signature'] = env['swift3.auth_details']['signature']
+            env['QUERY_STRING'] = urllib.parse.urlencode(params)
+
+            start_response('200 OK', [])
+            return ['']
+
+        app = filter_factory(CONF)(fake_app)
+        req.get_response(app)
+        check_params(req.params)
+
+    def test_signature_scrubbing_v2(self):
+        # example sig from AWS docs
+        sig = '5IH4YojJfFxDkvu1Rk4Ta%2BQmPX8%3D'
+        expire = '2147483647'
+        utc_date = datetime.utcnow()
+        self._test_signature_scrubbing(Request.blank(
+            '/bucket/object?Signature=%s&Expires=%s&'
+            'AWSAccessKeyId=test:tester&Timestamp=%s' %
+            (sig, expire, utc_date.isoformat().rsplit('.')[0]),
+            environ={'REQUEST_METHOD': 'GET'},
+            headers={'Date': self.get_date_header()}))
+
+    def test_signature_scrubbing_v4(self):
+        # example sig from AWS docs
+        sig = ('733255ef022bec3f2a8701cd61d4b371'
+               'f3f28c9f193a1f02279211d48d5193d7')
+        self._test_signature_scrubbing(Request.blank(
+            '/bucket/object'
+            '?X-Amz-Algorithm=AWS4-HMAC-SHA256'
+            '&X-Amz-Credential=test:tester/20T20Z/US/s3/aws4_request'
+            '&X-Amz-Date=%s'
+            '&X-Amz-Expires=1000'
+            '&X-Amz-SignedHeaders=host'
+            '&X-Amz-Signature=%s' %
+            (self.get_v4_amz_date_header(), sig),
+            headers={'Date': self.get_date_header()}))
+
     def test_swift3_with_only_s3_token(self):
         self.swift = FakeSwift()
         self.keystone_auth = KeystoneAuth(
-- 
2.12.2