By default, disallow inbound X-Timestamp headers

With the X-Timestamp validation added in commit e619411, end users
could upload objects with

    X-Timestamp: 9999999999.99999_ffffffffffffffff

(the maximum value) and Swift would be unable to delete them.

Now, inbound X-Timestamp headers will be moved to
X-Backend-Inbound-X-Timestamp, effectively rendering them harmless.

The primary reason to allow X-Timestamp before was to prevent
Last-Modified changes for objects coming from either:

  * container_sync or
  * a migration from another storage system.

To enable the former use-case, the container_sync middleware will now
translate X-Backend-Inbound-X-Timestamp headers back to X-Timestamp
after verifying the request.

Additionally, a new option is added to the gatekeeper filter config:

    # shunt_inbound_x_timestamp = true

To enable the latter use-case (or any other use-case not mentioned), set
this to false.

Upgrade Consideration
=====================

If your cluster workload requires that clients be allowed to specify
objects' X-Timestamp values, disable the shunt_inbound_x_timestamp
option before upgrading.

UpgradeImpact
Change-Id: I8799d5eb2ae9d795ba358bb422f69c70ee8ebd2c

etc/proxy-server.conf-sample

@@ -674,6 +674,12 @@ use = egg:swift#account_quotas
 
 [filter:gatekeeper]
 use = egg:swift#gatekeeper
+# Set this to false if you want to allow clients to set arbitrary X-Timestamps
+# on uploaded objects. This may be used to preserve timestamps when migrating
+# from a previous storage system, but risks allowing users to upload
+# difficult-to-delete data.
+# shunt_inbound_x_timestamp = true
+#
 # You can override the default log routing for this filter here:
 # set log_name = gatekeeper
 # set log_facility = LOG_LOCAL0

swift/common/middleware/container_sync.py

@@ -97,6 +97,11 @@ class ContainerSync(object):
                         req.environ.setdefault('swift.log_info', []).append(
                             'cs:no-local-user-key')
                     else:
+                        # x-timestamp headers get shunted by gatekeeper
+                        if 'x-backend-inbound-x-timestamp' in req.headers:
+                            req.headers['x-timestamp'] = req.headers.pop(
+                                'x-backend-inbound-x-timestamp')
+
                         expected = self.realms_conf.get_sig(
                             req.method, req.path,
                             req.headers.get('x-timestamp', '0'), nonce,

swift/common/middleware/gatekeeper.py

@@ -32,7 +32,7 @@ automatically inserted close to the start of the pipeline by the proxy server.
 
 
 from swift.common.swob import Request
-from swift.common.utils import get_logger
+from swift.common.utils import get_logger, config_true_value
 from swift.common.request_helpers import remove_items, get_sys_meta_prefix
 import re
 
@@ -69,6 +69,8 @@ class GatekeeperMiddleware(object):
         self.logger = get_logger(conf, log_route='gatekeeper')
         self.inbound_condition = make_exclusion_test(inbound_exclusions)
         self.outbound_condition = make_exclusion_test(outbound_exclusions)
+        self.shunt_x_timestamp = config_true_value(
+            conf.get('shunt_inbound_x_timestamp', 'true'))
 
     def __call__(self, env, start_response):
         req = Request(env)
@@ -76,6 +78,13 @@ class GatekeeperMiddleware(object):
         if removed:
             self.logger.debug('removed request headers: %s' % removed)
 
+        if 'X-Timestamp' in req.headers and self.shunt_x_timestamp:
+            ts = req.headers.pop('X-Timestamp')
+            req.headers['X-Backend-Inbound-X-Timestamp'] = ts
+            # log in a similar format as the removed headers
+            self.logger.debug('shunted request headers: %s' %
+                              [('X-Timestamp', ts)])
+
         def gatekeeper_response(status, response_headers, exc_info=None):
             removed = filter(
                 lambda h: self.outbound_condition(h[0]),

test/functional/test_object.py

@@ -167,11 +167,28 @@ class TestObject(unittest2.TestCase):
                               'Content-Length': '0',
                               'X-Timestamp': '-1'})
             return check_response(conn)
+
+        def head(url, token, parsed, conn):
+            conn.request('HEAD', '%s/%s/%s' % (parsed.path, self.container,
+                                               'too_small_x_timestamp'),
+                         '', {'X-Auth-Token': token,
+                              'Content-Length': '0'})
+            return check_response(conn)
+        ts_before = time.time()
         resp = retry(put)
         body = resp.read()
-        self.assertEqual(resp.status, 400)
+        ts_after = time.time()
-        self.assertIn(
+        if resp.status == 400:
-            'X-Timestamp should be a UNIX timestamp float value', body)
+            # shunt_inbound_x_timestamp must be false
+            self.assertIn(
+                'X-Timestamp should be a UNIX timestamp float value', body)
+        else:
+            self.assertEqual(resp.status, 201)
+            self.assertEqual(body, '')
+            resp = retry(head)
+            resp.read()
+            self.assertGreater(float(resp.headers['x-timestamp']), ts_before)
+            self.assertLess(float(resp.headers['x-timestamp']), ts_after)
 
     def test_too_big_x_timestamp(self):
         def put(url, token, parsed, conn):
@@ -181,11 +198,28 @@ class TestObject(unittest2.TestCase):
                               'Content-Length': '0',
                               'X-Timestamp': '99999999999.9999999999'})
             return check_response(conn)
+
+        def head(url, token, parsed, conn):
+            conn.request('HEAD', '%s/%s/%s' % (parsed.path, self.container,
+                                               'too_big_x_timestamp'),
+                         '', {'X-Auth-Token': token,
+                              'Content-Length': '0'})
+            return check_response(conn)
+        ts_before = time.time()
         resp = retry(put)
         body = resp.read()
-        self.assertEqual(resp.status, 400)
+        ts_after = time.time()
-        self.assertIn(
+        if resp.status == 400:
-            'X-Timestamp should be a UNIX timestamp float value', body)
+            # shunt_inbound_x_timestamp must be false
+            self.assertIn(
+                'X-Timestamp should be a UNIX timestamp float value', body)
+        else:
+            self.assertEqual(resp.status, 201)
+            self.assertEqual(body, '')
+            resp = retry(head)
+            resp.read()
+            self.assertGreater(float(resp.headers['x-timestamp']), ts_before)
+            self.assertLess(float(resp.headers['x-timestamp']), ts_after)
 
     def test_x_delete_after(self):
         def put(url, token, parsed, conn):

test/unit/common/middleware/test_container_sync.py

@@ -42,7 +42,10 @@ class FakeApp(object):
             body = 'Response to Authorized Request'
         else:
             body = 'Pass-Through Response'
-        start_response('200 OK', [('Content-Length', str(len(body)))])
+        headers = [('Content-Length', str(len(body)))]
+        if 'HTTP_X_TIMESTAMP' in env:
+            headers.append(('X-Timestamp', env['HTTP_X_TIMESTAMP']))
+        start_response('200 OK', headers)
         return body
 
 
@@ -214,18 +217,20 @@ cluster_dfw1 = http://dfw1.host/v1/
             req.environ.get('swift.log_info'))
 
     def test_valid_sig(self):
+        ts = '1455221706.726999_0123456789abcdef'
         sig = self.sync.realms_conf.get_sig(
-            'GET', '/v1/a/c', '0', 'nonce',
+            'GET', '/v1/a/c', ts, 'nonce',
             self.sync.realms_conf.key('US'), 'abc')
-        req = swob.Request.blank(
+        req = swob.Request.blank('/v1/a/c', headers={
-            '/v1/a/c', headers={'x-container-sync-auth': 'US nonce ' + sig})
+            'x-container-sync-auth': 'US nonce ' + sig,
+            'x-backend-inbound-x-timestamp': ts})
         req.environ[_get_cache_key('a', 'c')[1]] = {'sync_key': 'abc'}
         resp = req.get_response(self.sync)
         self.assertEqual(resp.status, '200 OK')
         self.assertEqual(resp.body, 'Response to Authorized Request')
-        self.assertTrue(
+        self.assertIn('cs:valid', req.environ.get('swift.log_info'))
-            'cs:valid' in req.environ.get('swift.log_info'),
+        self.assertIn('X-Timestamp', resp.headers)
-            req.environ.get('swift.log_info'))
+        self.assertEqual(ts, resp.headers['X-Timestamp'])
 
     def test_valid_sig2(self):
         sig = self.sync.realms_conf.get_sig(

test/unit/common/middleware/test_gatekeeper.py

@@ -74,10 +74,13 @@ class TestGatekeeper(unittest.TestCase):
     x_backend_headers = {'X-Backend-Replication': 'true',
                          'X-Backend-Replication-Headers': 'stuff'}
 
+    x_timestamp_headers = {'X-Timestamp': '1455952805.719739'}
+
     forbidden_headers_out = dict(sysmeta_headers.items() +
                                  x_backend_headers.items())
     forbidden_headers_in = dict(sysmeta_headers.items() +
                                 x_backend_headers.items())
+    shunted_headers_in = dict(x_timestamp_headers.items())
 
     def _assertHeadersEqual(self, expected, actual):
         for key in expected:
@@ -106,20 +109,63 @@ class TestGatekeeper(unittest.TestCase):
     def _test_reserved_header_removed_inbound(self, method):
         headers = dict(self.forbidden_headers_in)
         headers.update(self.allowed_headers)
+        headers.update(self.shunted_headers_in)
         req = Request.blank('/v/a/c', environ={'REQUEST_METHOD': method},
                             headers=headers)
         fake_app = FakeApp()
         app = self.get_app(fake_app, {})
         resp = req.get_response(app)
         self.assertEqual('200 OK', resp.status)
-        self._assertHeadersEqual(self.allowed_headers, fake_app.req.headers)
+        expected_headers = dict(self.allowed_headers)
-        self._assertHeadersAbsent(self.forbidden_headers_in,
+        # shunt_inbound_x_timestamp should be enabled by default
-                                  fake_app.req.headers)
+        expected_headers.update({'X-Backend-Inbound-' + k: v
+                                 for k, v in self.shunted_headers_in.items()})
+        self._assertHeadersEqual(expected_headers, fake_app.req.headers)
+        unexpected_headers = dict(self.forbidden_headers_in.items() +
+                                  self.shunted_headers_in.items())
+        self._assertHeadersAbsent(unexpected_headers, fake_app.req.headers)
 
     def test_reserved_header_removed_inbound(self):
         for method in self.methods:
             self._test_reserved_header_removed_inbound(method)
 
+    def _test_reserved_header_shunted_inbound(self, method):
+        headers = dict(self.shunted_headers_in)
+        headers.update(self.allowed_headers)
+        req = Request.blank('/v/a/c', environ={'REQUEST_METHOD': method},
+                            headers=headers)
+        fake_app = FakeApp()
+        app = self.get_app(fake_app, {}, shunt_inbound_x_timestamp='true')
+        resp = req.get_response(app)
+        self.assertEqual('200 OK', resp.status)
+        expected_headers = dict(self.allowed_headers)
+        expected_headers.update({'X-Backend-Inbound-' + k: v
+                                 for k, v in self.shunted_headers_in.items()})
+        self._assertHeadersEqual(expected_headers, fake_app.req.headers)
+        self._assertHeadersAbsent(self.shunted_headers_in,
+                                  fake_app.req.headers)
+
+    def test_reserved_header_shunted_inbound(self):
+        for method in self.methods:
+            self._test_reserved_header_shunted_inbound(method)
+
+    def _test_reserved_header_shunt_bypassed_inbound(self, method):
+        headers = dict(self.shunted_headers_in)
+        headers.update(self.allowed_headers)
+        req = Request.blank('/v/a/c', environ={'REQUEST_METHOD': method},
+                            headers=headers)
+        fake_app = FakeApp()
+        app = self.get_app(fake_app, {}, shunt_inbound_x_timestamp='false')
+        resp = req.get_response(app)
+        self.assertEqual('200 OK', resp.status)
+        expected_headers = dict(self.allowed_headers.items() +
+                                self.shunted_headers_in.items())
+        self._assertHeadersEqual(expected_headers, fake_app.req.headers)
+
+    def test_reserved_header_shunt_bypassed_inbound(self):
+        for method in self.methods:
+            self._test_reserved_header_shunt_bypassed_inbound(method)
+
     def _test_reserved_header_removed_outbound(self, method):
         headers = dict(self.forbidden_headers_out)
         headers.update(self.allowed_headers)