py3: Work with proper native string paths in crypto meta

Previously, we would work with these paths as WSGI strings -- this would
work fine when all data were read and written on the same major version
of Python, but fail pretty badly during and after upgrading Python.

In particular, if a py3 proxy-server tried to read existing data that
was written down by a py2 proxy-server, it would hit an error and
respond 500. Worse, if an un-upgraded py2 proxy tried to read data that
was freshly-written by a py3 proxy, it would serve corrupt data back to
the client (including a corrupt/invalid ETag and Content-Type).

Now, ensure that both py2 and py3 write down paths as native strings.
Make an effort to still work with WSGI-string metadata, though it can be
ambiguous as to whether a string is a WSGI string or not. The heuristic
used is if

 * the path from metadata does not match the (native-string) request
   path and
 * the path from metadata (when interpreted as a WSGI string) can be
   "un-wsgi-fied" without any encode/decode errors and
 * the native-string path from metadata *does* match the native-string
   request path

then trust the path from the request. By contrast, we usually prefer the
path from metadata in case there was a pipeline misconfiguration (see
related bug).

Add the ability to read and write a new, unambiguous version of metadata
that always has the path as a native string. To support rolling
upgrades, a new config option is added: meta_version_to_write. This
defaults to 2 to support rolling upgrades without configuration changes,
but the default may change to 3 in a future release.

UpgradeImpact
=============
When upgrading from Swift 2.20.0 or Swift 2.19.1 or earlier, set

    meta_version_to_write = 1

in your keymaster's configuration. Regardless of prior Swift version, set

    meta_version_to_write = 3

after upgrading all proxy servers.

When switching from Python 2 to Python 3, first upgrade Swift while on
Python 2, then upgrade to Python 3.

Change-Id: I00c6693c42c1a0220b64d8016d380d5985339658
Closes-Bug: #1888037
Related-Bug: #1813725
(cherry picked from commit 7d42931)

etc/keymaster.conf-sample

@@ -1,4 +1,13 @@
 [keymaster]
+# Over time, the format of crypto metadata on disk may change slightly to resolve
+# ambiguities. In general, you want to be writing the newest version, but to
+# ensure that all writes can still be read during rolling upgrades, there's the
+# option to write older formats as well.
+# Before upgrading from Swift 2.20.0 or earlier, ensure this is set to 1
+# Before upgrading from Swift 2.25.0 or earlier, ensure this is set to at most 2
+# After upgrading all proxy servers, set this to 3 (currently the highest version)
+# meta_version_to_write = 3
+
 # Sets the root secret from which encryption keys are derived. This must be set
 # before first use to a value that is a base64 encoding of at least 32 bytes.
 # The security of all encrypted data critically depends on this key, therefore
@@ -16,6 +25,15 @@
 # backends that use Keystone for authentication. Currently, the only
 # implemented backend is for Barbican.
 
+# Over time, the format of crypto metadata on disk may change slightly to resolve
+# ambiguities. In general, you want to be writing the newest version, but to
+# ensure that all writes can still be read during rolling upgrades, there's the
+# option to write older formats as well.
+# Before upgrading from Swift 2.20.0 or earlier, ensure this is set to 1
+# Before upgrading from Swift 2.25.0 or earlier, ensure this is set to at most 2
+# After upgrading all proxy servers, set this to 3 (currently the highest version)
+# meta_version_to_write = 3
+
 # The api_class tells Castellan which key manager to use to access the external
 # key management system. The default value that accesses Barbican is
 # castellan.key_manager.barbican_key_manager.BarbicanKeyManager.
@@ -79,6 +97,15 @@
 # The kmip_keymaster section is used to configure a keymaster that fetches an
 # encryption root secret from a KMIP service.
 
+# Over time, the format of crypto metadata on disk may change slightly to resolve
+# ambiguities. In general, you want to be writing the newest version, but to
+# ensure that all writes can still be read during rolling upgrades, there's the
+# option to write older formats as well.
+# Before upgrading from Swift 2.20.0 or earlier, ensure this is set to 1
+# Before upgrading from Swift 2.25.0 or earlier, ensure this is set to at most 2
+# After upgrading all proxy servers, set this to 3 (currently the highest version)
+# meta_version_to_write = 3
+
 # The value of the ``key_id`` option should be the unique identifier for a
 # secret that will be retrieved from the KMIP service. The secret should be an
 # AES-256 symmetric key.

etc/proxy-server.conf-sample

@@ -1052,6 +1052,18 @@ use = egg:swift#copy
 [filter:keymaster]
 use = egg:swift#keymaster
 
+# Over time, the format of crypto metadata on disk may change slightly to resolve
+# ambiguities. In general, you want to be writing the newest version, but to
+# ensure that all writes can still be read during rolling upgrades, there's the
+# option to write older formats as well.
+# Before upgrading from Swift 2.20.0 or Swift 2.19.1 or earlier, ensure this is set to 1
+# Before upgrading from Swift 2.25.0 or earlier, ensure this is set to at most 2
+# After upgrading all proxy servers, set this to 3 (currently the highest version)
+#
+# The default is currently 2 to support upgrades with no configuration changes,
+# but may change to 3 in the future.
+meta_version_to_write = 2
+
 # Sets the root secret from which encryption keys are derived. This must be set
 # before first use to a value that is a base64 encoding of at least 32 bytes.
 # The security of all encrypted data critically depends on this key, therefore

swift/common/middleware/crypto/keymaster.py

@@ -14,10 +14,11 @@
 # limitations under the License.
 import hashlib
 import hmac
+import six
 
 from swift.common.exceptions import UnknownSecretIdError
 from swift.common.middleware.crypto.crypto_utils import CRYPTO_KEY_CALLBACK
-from swift.common.swob import Request, HTTPException, wsgi_to_bytes
+from swift.common.swob import Request, HTTPException, wsgi_to_str, str_to_wsgi
 from swift.common.utils import readconf, strict_b64decode, get_logger, \
     split_path
 from swift.common.wsgi import WSGIContext
@@ -33,7 +34,8 @@ class KeyMasterContext(WSGIContext):
 
       <path_key> = HMAC_SHA256(<root_secret>, <path>)
     """
-    def __init__(self, keymaster, account, container, obj):
+    def __init__(self, keymaster, account, container, obj,
+                 meta_version_to_write='2'):
         """
         :param keymaster: a Keymaster instance
         :param account: account name
@@ -47,8 +49,11 @@ def __init__(self, keymaster, account, container, obj):
         self.obj = obj
         self._keys = {}
         self.alternate_fetch_keys = None
+        self.meta_version_to_write = meta_version_to_write
 
     def _make_key_id(self, path, secret_id, version):
+        if version in ('1', '2'):
+            path = str_to_wsgi(path)
         key_id = {'v': version, 'path': path}
         if secret_id:
             # stash secret_id so that decrypter can pass it back to get the
@@ -76,8 +81,9 @@ def fetch_crypto_keys(self, key_id=None, *args, **kwargs):
         if key_id:
             secret_id = key_id.get('secret_id')
             version = key_id['v']
-            if version not in ('1', '2'):
+            if version not in ('1', '2', '3'):
                 raise ValueError('Unknown key_id version: %s' % version)
+
             if version == '1' and not key_id['path'].startswith(
                     '/' + self.account + '/'):
                 # Well shoot. This was the bug that made us notice we needed
@@ -90,7 +96,32 @@ def fetch_crypto_keys(self, key_id=None, *args, **kwargs):
 
             check_path = (
                 self.account, self.container or key_cont, self.obj or key_obj)
+            if version in ('1', '2') and (
+                    key_acct, key_cont, key_obj) != check_path:
+                # Older py3 proxies may have written down crypto meta as WSGI
+                # strings; we still need to be able to read that
+                try:
+                    if six.PY2:
+                        alt_path = tuple(
+                            part.decode('utf-8').encode('latin1')
+                            for part in (key_acct, key_cont, key_obj))
+                    else:
+                        alt_path = tuple(
+                            part.encode('latin1').decode('utf-8')
+                            for part in (key_acct, key_cont, key_obj))
+                except UnicodeError:
+                    # Well, it was worth a shot
+                    pass
+                else:
+                    if check_path == alt_path or (
+                            check_path[:2] == alt_path[:2] and not self.obj):
+                        # This object is affected by bug #1888037
+                        key_acct, key_cont, key_obj = alt_path
+
             if (key_acct, key_cont, key_obj) != check_path:
+                # Pipeline may have been misconfigured, with copy right of
+                # encryption. In that case, path in meta may not be the
+                # request path.
                 self.keymaster.logger.info(
                     "Path stored in meta (%r) does not match path from "
                     "request (%r)! Using path from meta.",
@@ -100,9 +131,11 @@ def fetch_crypto_keys(self, key_id=None, *args, **kwargs):
         else:
             secret_id = self.keymaster.active_secret_id
             # v1 had a bug where we would claim the path was just the object
-            # name if the object started with a slash. Bump versions to
-            # establish that we can trust the path.
-            version = '2'
+            # name if the object started with a slash.
+            # v1 and v2 had a bug on py3 where we'd write the path in meta as
+            # a WSGI string (ie, as Latin-1 chars decoded from UTF-8 bytes).
+            # Bump versions to establish that we can trust the path.
+            version = self.meta_version_to_write
             key_acct, key_cont, key_obj = (
                 self.account, self.container, self.obj)
 
@@ -215,6 +248,11 @@ def __init__(self, app, conf):
             raise ValueError('No secret loaded for active_root_secret_id %s' %
                              self.active_secret_id)
 
+        self.meta_version_to_write = conf.get('meta_version_to_write') or '2'
+        if self.meta_version_to_write not in ('1', '2', '3'):
+            raise ValueError('Unknown/unsupported metadata version: %r' %
+                             self.meta_version_to_write)
+
     @property
     def root_secret(self):
         # Returns the default root secret; this is here for historical reasons
@@ -261,13 +299,15 @@ def __call__(self, env, start_response):
         req = Request(env)
 
         try:
-            parts = req.split_path(2, 4, True)
+            parts = [wsgi_to_str(part) for part in req.split_path(2, 4, True)]
         except ValueError:
             return self.app(env, start_response)
 
         if req.method in ('PUT', 'POST', 'GET', 'HEAD'):
             # handle only those request methods that may require keys
-            km_context = KeyMasterContext(self, *parts[1:])
+            km_context = KeyMasterContext(
+                self, *parts[1:],
+                meta_version_to_write=self.meta_version_to_write)
             try:
                 return km_context.handle_request(req, start_response)
             except HTTPException as err_resp:
@@ -292,8 +332,9 @@ def create_key(self, path, secret_id=None):
             self.logger.warning('Unrecognised secret id: %s' % secret_id)
             raise UnknownSecretIdError(secret_id)
         else:
-            return hmac.new(key, wsgi_to_bytes(path),
-                            digestmod=hashlib.sha256).digest()
+            if not six.PY2:
+                path = path.encode('utf-8')
+            return hmac.new(key, path, digestmod=hashlib.sha256).digest()
 
 
 class KeyMaster(BaseKeyMaster):