From fad36d8ef79e52d8bf67c89bde876911990abf8f Mon Sep 17 00:00:00 2001 From: Alex Meade Date: Thu, 10 Jan 2013 12:24:02 -0500 Subject: [PATCH] Add migration to quote encrypted image location urls Migration 015_quote_swift_credentials.py failed to take into account encrypted location values. This patch adds a similar migration that if the value is encrypted then: 1. Decrypts the value 2. Quotes the url string 3. Encrypts the value 4. Inserts back into DB fixes bug 1081043 Change-Id: Iadfe5f5082b3932307eef8e76571e36c6ff9fce7 --- .../017_quote_encrypted_swift_credentials.py | 215 ++++++++++++++++++ glance/tests/unit/test_migrations.py | 125 ++++++++++ 2 files changed, 340 insertions(+) create mode 100644 glance/db/sqlalchemy/migrate_repo/versions/017_quote_encrypted_swift_credentials.py diff --git a/glance/db/sqlalchemy/migrate_repo/versions/017_quote_encrypted_swift_credentials.py b/glance/db/sqlalchemy/migrate_repo/versions/017_quote_encrypted_swift_credentials.py new file mode 100644 index 0000000000..e2bb81c0ff --- /dev/null +++ b/glance/db/sqlalchemy/migrate_repo/versions/017_quote_encrypted_swift_credentials.py @@ -0,0 +1,215 @@ +# vim: tabstop=4 shiftwidth=4 softtabstop=4 + +# Copyright 2013 OpenStack LLC. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +""" +This migration handles migrating encrypted image location values from +the unquoted form to the quoted form. + +If 'metadata_encryption_key' is specified in the config then this +migration performs the following steps for every entry in the images table: +1. Decrypt the location value with the metadata_encryption_key +2. Changes the value to its quoted form +3. Encrypts the new value with the metadata_encryption_key +4. Inserts the new value back into the row + +Fixes bug #1081043 +""" + +import types +import urlparse + +import sqlalchemy + +from glance.common import crypt +from glance.common import exception +from glance.openstack.common import cfg +import glance.openstack.common.log as logging +import glance.store.swift + +LOG = logging.getLogger(__name__) +CONF = cfg.CONF + +CONF.import_opt('metadata_encryption_key', 'glance.registry') + + +def upgrade(migrate_engine): + migrate_location_credentials(migrate_engine, to_quoted=True) + + +def downgrade(migrate_engine): + migrate_location_credentials(migrate_engine, to_quoted=False) + + +def migrate_location_credentials(migrate_engine, to_quoted): + """ + Migrate location credentials for encrypted swift uri's between the + quoted and unquoted forms. + + :param migrate_engine: The configured db engine + :param to_quoted: If True, migrate location credentials from + unquoted to quoted form. If False, do the + reverse. + """ + if not CONF.metadata_encryption_key: + msg = _("'metadata_encryption_key' was not specified in the config" + " file or a config file was not specified. This means that" + " this migration is a NOOP.") + LOG.info(msg) + return + + meta = sqlalchemy.schema.MetaData() + meta.bind = migrate_engine + + images_table = sqlalchemy.Table('images', meta, autoload=True) + + images = list(images_table.select().execute()) + + for image in images: + try: + fixed_uri = fix_uri_credentials(image['location'], to_quoted) + images_table.update()\ + .where(images_table.c.id == image['id'])\ + .values(location=fixed_uri).execute() + except exception.Invalid: + msg = _("Failed to decrypt location value for image %s") + LOG.warn(msg % image['id']) + + +def decrypt_location(uri): + return crypt.urlsafe_decrypt(CONF.metadata_encryption_key, uri) + + +def encrypt_location(uri): + return crypt.urlsafe_encrypt(CONF.metadata_encryption_key, uri, 64) + + +def fix_uri_credentials(uri, to_quoted): + """ + Fix the given uri's embedded credentials by round-tripping with + StoreLocation. + + If to_quoted is True, the uri is assumed to have credentials that + have not been quoted, and the resulting uri will contain quoted + credentials. + + If to_quoted is False, the uri is assumed to have credentials that + have been quoted, and the resulting uri will contain credentials + that have not been quoted. + """ + if not uri: + return + location = glance.store.swift.StoreLocation({}) + if to_quoted: + # The legacy parse_uri doesn't unquote credentials + location.parse_uri = types.MethodType(legacy_parse_uri, location) + else: + # The legacy _get_credstring doesn't quote credentials + location._get_credstring = types.MethodType(legacy__get_credstring, + location) + decrypted_uri = None + try: + decrypted_uri = decrypt_location(uri) + #NOTE (ameade): If a uri is not encrypted or incorrectly encoded then we + # we raise an exception. + except (TypeError, ValueError) as e: + raise exception.Invalid(str(e)) + + location.parse_uri(decrypted_uri) + return encrypt_location(location.get_uri()) + + +def legacy__get_credstring(self): + if self.user: + return '%s:%s@' % (self.user, self.key) + return '' + + +def legacy_parse_uri(self, uri): + """ + Parse URLs. This method fixes an issue where credentials specified + in the URL are interpreted differently in Python 2.6.1+ than prior + versions of Python. It also deals with the peculiarity that new-style + Swift URIs have where a username can contain a ':', like so: + + swift://account:user:pass@authurl.com/container/obj + """ + # Make sure that URIs that contain multiple schemes, such as: + # swift://user:pass@http://authurl.com/v1/container/obj + # are immediately rejected. + if uri.count('://') != 1: + reason = _("URI cannot contain more than one occurrence of a scheme." + "If you have specified a URI like " + "swift://user:pass@http://authurl.com/v1/container/obj" + ", you need to change it to use the swift+http:// scheme, " + "like so: " + "swift+http://user:pass@authurl.com/v1/container/obj") + + LOG.error(_("Invalid store uri %(uri)s: %(reason)s") % locals()) + raise exception.BadStoreUri(message=reason) + + pieces = urlparse.urlparse(uri) + assert pieces.scheme in ('swift', 'swift+http', 'swift+https') + self.scheme = pieces.scheme + netloc = pieces.netloc + path = pieces.path.lstrip('/') + if netloc != '': + # > Python 2.6.1 + if '@' in netloc: + creds, netloc = netloc.split('@') + else: + creds = None + else: + # Python 2.6.1 compat + # see lp659445 and Python issue7904 + if '@' in path: + creds, path = path.split('@') + else: + creds = None + netloc = path[0:path.find('/')].strip('/') + path = path[path.find('/'):].strip('/') + if creds: + cred_parts = creds.split(':') + + # User can be account:user, in which case cred_parts[0:2] will be + # the account and user. Combine them into a single username of + # account:user + if len(cred_parts) == 1: + reason = (_("Badly formed credentials '%(creds)s' in Swift " + "URI") % locals()) + LOG.error(reason) + raise exception.BadStoreUri() + elif len(cred_parts) == 3: + user = ':'.join(cred_parts[0:2]) + else: + user = cred_parts[0] + key = cred_parts[-1] + self.user = user + self.key = key + else: + self.user = None + path_parts = path.split('/') + try: + self.obj = path_parts.pop() + self.container = path_parts.pop() + if not netloc.startswith('http'): + # push hostname back into the remaining to build full authurl + path_parts.insert(0, netloc) + self.auth_or_store_url = '/'.join(path_parts) + except IndexError: + reason = _("Badly formed S3 URI: %s") % uri + LOG.error(message=reason) + raise exception.BadStoreUri() diff --git a/glance/tests/unit/test_migrations.py b/glance/tests/unit/test_migrations.py index 1571ca8592..4c44ab60f4 100644 --- a/glance/tests/unit/test_migrations.py +++ b/glance/tests/unit/test_migrations.py @@ -33,6 +33,7 @@ from migrate.versioning.repository import Repository from sqlalchemy import * from sqlalchemy.pool import NullPool +from glance.common import crypt from glance.common import exception #NOTE(bcwaldon): import this to prevent circular import from glance.db.sqlalchemy import api @@ -42,6 +43,10 @@ from glance.openstack.common import cfg from glance.tests import utils +CONF = cfg.CONF +CONF.import_opt('metadata_encryption_key', 'glance.registry') + + class TestMigrations(utils.BaseTestCase): """Test sqlalchemy-migrate migrations""" @@ -481,3 +486,123 @@ class TestMigrations(utils.BaseTestCase): num_image_members = conn.execute(sel).scalar() self.assertEqual(orig_num_image_members + 2, num_image_members) conn.close() + + def test_quote_encrypted_locations_16_to_17(self): + self.metadata_encryption_key = 'a' * 16 + for key, engine in self.engines.items(): + self.config(sql_connection=TestMigrations.TEST_DATABASES[key]) + self.config(metadata_encryption_key=self.metadata_encryption_key) + self._check_16_to_17(engine) + + def _check_16_to_17(self, engine): + """ + Check that migrating swift location credentials to quoted form + and back works. + """ + migration_api.version_control(version=0) + migration_api.upgrade(16) + + conn = engine.connect() + images_table = Table('images', MetaData(), autoload=True, + autoload_with=engine) + + def get_locations(): + conn = engine.connect() + locations = [x[0] for x in + conn.execute( + select(['location'], from_obj=[images_table]))] + conn.close() + return locations + + unquoted = 'swift://acct:usr:pass@example.com/container/obj-id' + encrypted_unquoted = crypt.urlsafe_encrypt( + self.metadata_encryption_key, + unquoted, 64) + + quoted = 'swift://acct%3Ausr:pass@example.com/container/obj-id' + + # Insert image with an unquoted image location + now = datetime.datetime.now() + kwargs = dict(deleted=False, + created_at=now, + updated_at=now, + status='active', + is_public=True, + min_disk=0, + min_ram=0) + kwargs.update(location=encrypted_unquoted, id=1) + conn.execute(images_table.insert(), [kwargs]) + conn.close() + + migration_api.upgrade(17) + + actual_location = crypt.urlsafe_decrypt(self.metadata_encryption_key, + get_locations()[0]) + + self.assertEqual(actual_location, quoted) + + migration_api.downgrade(16) + + actual_location = crypt.urlsafe_decrypt(self.metadata_encryption_key, + get_locations()[0]) + + self.assertEqual(actual_location, unquoted) + + def test_no_data_loss_16_to_17(self): + self.metadata_encryption_key = 'a' * 16 + for key, engine in self.engines.items(): + self.config(sql_connection=TestMigrations.TEST_DATABASES[key]) + self.config(metadata_encryption_key=self.metadata_encryption_key) + self._check_no_data_loss_16_to_17(engine) + + def _check_no_data_loss_16_to_17(self, engine): + """ + Check that migrating swift location credentials to quoted form + and back does not result in data loss. + """ + migration_api.version_control(version=0) + migration_api.upgrade(16) + + conn = engine.connect() + images_table = Table('images', MetaData(), autoload=True, + autoload_with=engine) + + def get_locations(): + conn = engine.connect() + locations = [x[0] for x in + conn.execute( + select(['location'], from_obj=[images_table]))] + conn.close() + return locations + + locations = ['file://ab', + 'file://abc', + 'swift://acct3A%foobar:pass@example.com/container/obj-id'] + + # Insert images with an unquoted image location + now = datetime.datetime.now() + kwargs = dict(deleted=False, + created_at=now, + updated_at=now, + status='active', + is_public=True, + min_disk=0, + min_ram=0) + for i, location in enumerate(locations): + kwargs.update(location=location, id=i) + conn.execute(images_table.insert(), [kwargs]) + conn.close() + + def assert_locations(): + actual_locations = get_locations() + for location in locations: + if not location in actual_locations: + self.fail(_("location: %s data lost") % location) + + migration_api.upgrade(17) + + assert_locations() + + migration_api.downgrade(16) + + assert_locations()