Fix restore encrypted backup

For the backups created prior to Victoria which may be encrypted, the restore
function in Victoria or later release should correctly decrypt the backup data.

Backup encryption from Victoria is disabled.

Bump the backup docker image tag to 1.1.0.

Change-Id: I7abf5563b22ab1914fa355c089a3793da07f2215
This commit is contained in:
Lingxian Kong 2020-10-29 10:55:22 +13:00
parent 69c8467854
commit bd2b256a37
8 changed files with 76 additions and 38 deletions

View File

@ -47,7 +47,16 @@ class BaseRunner(object):
self.restore_content_length = 0 self.restore_content_length = 0
self.command = self.cmd % kwargs self.command = self.cmd % kwargs
self.restore_command = (self.decrypt_cmd +
if self.location.endswith('.enc') and not self.encrypt_key:
raise Exception("Encryption key not provided with an encrypted "
"backup.")
self.restore_command = ''
# Only decrypt if the object name ends with .enc
if self.location.endswith('.enc'):
self.restore_command = self.decrypt_cmd
self.restore_command = (self.restore_command +
self.unzip_cmd + self.unzip_cmd +
(self.restore_cmd % kwargs)) (self.restore_cmd % kwargs))
self.prepare_command = self.prepare_cmd % kwargs self.prepare_command = self.prepare_cmd % kwargs
@ -78,12 +87,21 @@ class BaseRunner(object):
@property @property
def encrypt_cmd(self): def encrypt_cmd(self):
return (' | openssl enc -aes-256-cbc -md sha512 -pbkdf2 -iter 10000 ' """Encryption command.
'-salt -pass pass:%s' %
self.encrypt_key) if self.encrypt_key else '' Since Victoria, trove no longer encrypts the backup data for the end
user. This could be improved by giving users the capability to specify
password when creating the backups.
"""
return ""
@property @property
def decrypt_cmd(self): def decrypt_cmd(self):
"""Decryption command.
Since Victoria, trove no longer encrypts the backup data for the end
user. This command is only for backward compatibility.
"""
if self.encrypt_key: if self.encrypt_key:
return ('openssl enc -d -aes-256-cbc -md sha512 -pbkdf2 -iter ' return ('openssl enc -d -aes-256-cbc -md sha512 -pbkdf2 -iter '
'10000 -salt -pass pass:%s | ' '10000 -salt -pass pass:%s | '

View File

@ -39,7 +39,12 @@ cli_opts = [
choices=['innobackupex', 'mariabackup', 'pg_basebackup', 'xtrabackup'] choices=['innobackupex', 'mariabackup', 'pg_basebackup', 'xtrabackup']
), ),
cfg.BoolOpt('backup'), cfg.BoolOpt('backup'),
cfg.StrOpt('backup-encryption-key'), cfg.StrOpt(
'backup-encryption-key',
help='This is only for backward compatibility. The backups '
'created prior to Victoria may be encrypted. Trove guest '
'agent is responsible for passing the key.'
),
cfg.StrOpt('db-user'), cfg.StrOpt('db-user'),
cfg.StrOpt('db-password'), cfg.StrOpt('db-password'),
cfg.StrOpt('db-host'), cfg.StrOpt('db-host'),

View File

@ -25,6 +25,7 @@ from oslo_config.cfg import NoSuchOptError
from oslo_log import log as logging from oslo_log import log as logging
from oslo_middleware import cors from oslo_middleware import cors
from osprofiler import opts as profiler from osprofiler import opts as profiler
from oslo_log import versionutils
from trove.common.i18n import _ from trove.common.i18n import _
from trove.version import version_info as version from trove.version import version_info as version
@ -325,13 +326,36 @@ common_opts = [
cfg.StrOpt('backup_swift_container', default='database_backups', cfg.StrOpt('backup_swift_container', default='database_backups',
help='Swift container to put backups in.'), help='Swift container to put backups in.'),
cfg.BoolOpt('backup_use_gzip_compression', default=True, cfg.BoolOpt('backup_use_gzip_compression', default=True,
help='Compress backups using gzip.'), help='Compress backups using gzip.',
cfg.BoolOpt('backup_use_openssl_encryption', default=True, deprecated_for_removal=True,
help='Encrypt backups using OpenSSL.'), deprecated_since=versionutils.deprecated.VICTORIA,
cfg.StrOpt('backup_aes_cbc_key', default='default_aes_cbc_key', deprecated_reason='Backup data compression is enabled by '
help='Default OpenSSL aes_cbc key.'), 'default. This option is ignored.'),
cfg.BoolOpt('backup_use_snet', default=False, cfg.BoolOpt(
help='Send backup files over snet.'), 'backup_use_openssl_encryption', default=True,
help='Encrypt backups using OpenSSL.',
deprecated_for_removal=True,
deprecated_since=versionutils.deprecated.VICTORIA,
deprecated_reason='Trove should not encrypt backup data on '
'behalf of the user. This option is ignored.'
),
cfg.StrOpt(
'backup_aes_cbc_key', default='',
help='Default OpenSSL aes_cbc key for decrypting backup data created '
'prior to Victoria.',
deprecated_for_removal=True,
deprecated_since=versionutils.deprecated.VICTORIA,
deprecated_reason='This option is only for backward compatibility. '
'Backups created after Victoria are not encrypted '
'any more.'
),
cfg.BoolOpt(
'backup_use_snet', default=False,
help='Send backup files over snet.',
deprecated_for_removal=True,
deprecated_since=versionutils.deprecated.VICTORIA,
deprecated_reason='This option is not supported any more.'
),
cfg.IntOpt('backup_chunk_size', default=2 ** 16, cfg.IntOpt('backup_chunk_size', default=2 ** 16,
help='Chunk size (in bytes) to stream to the Swift container. ' help='Chunk size (in bytes) to stream to the Swift container. '
'This should be in multiples of 128 bytes, since this is the ' 'This should be in multiples of 128 bytes, since this is the '
@ -616,7 +640,7 @@ mysql_opts = [
help='Database docker image.' help='Database docker image.'
), ),
cfg.StrOpt( cfg.StrOpt(
'backup_docker_image', default='openstacktrove/db-backup-mysql:1.0.0', 'backup_docker_image', default='openstacktrove/db-backup-mysql:1.1.0',
help='The docker image used for backup and restore. For mysql, ' help='The docker image used for backup and restore. For mysql, '
'the minor version is added to the image name as a suffix before ' 'the minor version is added to the image name as a suffix before '
'creating container, e.g. openstacktrove/db-backup-mysql5.7:1.0.0' 'creating container, e.g. openstacktrove/db-backup-mysql5.7:1.0.0'
@ -1063,7 +1087,7 @@ postgresql_opts = [
), ),
cfg.StrOpt( cfg.StrOpt(
'backup_docker_image', 'backup_docker_image',
default='openstacktrove/db-backup-postgresql:1.0.0', default='openstacktrove/db-backup-postgresql:1.1.0',
help='The docker image used for backup and restore.' help='The docker image used for backup and restore.'
), ),
cfg.BoolOpt('icmp', default=False, cfg.BoolOpt('icmp', default=False,
@ -1384,7 +1408,7 @@ mariadb_opts = [
), ),
cfg.StrOpt( cfg.StrOpt(
'backup_docker_image', 'backup_docker_image',
default='openstacktrove/db-backup-mariadb:1.0.0', default='openstacktrove/db-backup-mariadb:1.1.0',
help='The docker image used for backup and restore.' help='The docker image used for backup and restore.'
), ),
] ]

View File

@ -760,6 +760,12 @@ class Manager(periodic_task.PeriodicTasks):
LOG.info("Starting to restore database from backup %s, " LOG.info("Starting to restore database from backup %s, "
"backup_info: %s", backup_info['id'], backup_info) "backup_info: %s", backup_info['id'], backup_info)
if (backup_info["location"].endswith('.enc') and
not CONF.backup_aes_cbc_key):
self.status.set_status(service_status.ServiceStatuses.FAILED)
raise exception.TroveError('Decryption key not configured for '
'encrypted backup.')
try: try:
self.app.restore_backup(context, backup_info, restore_location) self.app.restore_backup(context, backup_info, restore_location)
except Exception: except Exception:

View File

@ -678,6 +678,9 @@ class BaseMySqlApp(service.BaseDbApp):
f'--restore-from={backup_info["location"]} ' f'--restore-from={backup_info["location"]} '
f'--restore-checksum={backup_info["checksum"]}' f'--restore-checksum={backup_info["checksum"]}'
) )
if CONF.backup_aes_cbc_key:
command = (f"{command} "
f"--backup-encryption-key={CONF.backup_aes_cbc_key}")
LOG.debug('Stop the database and clean up the data before restore ' LOG.debug('Stop the database and clean up the data before restore '
'from %s', backup_id) 'from %s', backup_id)

View File

@ -267,6 +267,9 @@ class PgSqlApp(service.BaseDbApp):
f'--restore-checksum={backup_info["checksum"]} ' f'--restore-checksum={backup_info["checksum"]} '
f'--pg-wal-archive-dir {WAL_ARCHIVE_DIR}' f'--pg-wal-archive-dir {WAL_ARCHIVE_DIR}'
) )
if CONF.backup_aes_cbc_key:
command = (f"{command} "
f"--backup-encryption-key={CONF.backup_aes_cbc_key}")
LOG.debug('Stop the database and clean up the data before restore ' LOG.debug('Stop the database and clean up the data before restore '
'from %s', backup_id) 'from %s', backup_id)

View File

@ -470,8 +470,7 @@ class ListConfigurations(ConfigurationsTestBase):
@test @test
def test_changing_configuration_with_nondynamic_parameter(self): def test_changing_configuration_with_nondynamic_parameter(self):
# test that changing a non-dynamic parameter is applied to instance """test_changing_configuration_with_nondynamic_parameter"""
# and show that the instance requires a restart
expected_configs = self.expected_default_datastore_configs() expected_configs = self.expected_default_datastore_configs()
values = json.dumps(expected_configs.get('nondynamic_parameter')) values = json.dumps(expected_configs.get('nondynamic_parameter'))
instance_info.dbaas.configurations.update(configuration_info.id, instance_info.dbaas.configurations.update(configuration_info.id,
@ -486,6 +485,7 @@ class ListConfigurations(ConfigurationsTestBase):
@test(depends_on=[test_changing_configuration_with_nondynamic_parameter]) @test(depends_on=[test_changing_configuration_with_nondynamic_parameter])
@time_out(20) @time_out(20)
def test_waiting_for_instance_in_restart_required(self): def test_waiting_for_instance_in_restart_required(self):
"""test_waiting_for_instance_in_restart_required"""
def result_is_not_active(): def result_is_not_active():
instance = instance_info.dbaas.instances.get( instance = instance_info.dbaas.instances.get(
instance_info.id) instance_info.id)
@ -733,26 +733,6 @@ class DeleteConfigurations(ConfigurationsTestBase):
@test(depends_on=[test_unassign_configuration_from_instances]) @test(depends_on=[test_unassign_configuration_from_instances])
@time_out(120) @time_out(120)
def test_restart_service_after_unassign_return_active(self):
"""test_restart_service_after_unassign_return_active"""
def result_is_not_active():
instance = instance_info.dbaas.instances.get(
instance_info.id)
if instance.status in CONFIG.running_status:
return False
else:
return True
poll_until(result_is_not_active)
config = instance_info.dbaas.configurations.list()
print(config)
instance = instance_info.dbaas.instances.get(instance_info.id)
resp, body = instance_info.dbaas.client.last_response
assert_equal(resp.status, 200)
assert_equal('RESTART_REQUIRED', instance.status)
@test(depends_on=[test_restart_service_after_unassign_return_active])
@time_out(120)
def test_restart_service_should_return_active(self): def test_restart_service_should_return_active(self):
"""test that after restarting the instance it becomes active""" """test that after restarting the instance it becomes active"""
instance_info.dbaas.instances.restart(instance_info.id) instance_info.dbaas.instances.restart(instance_info.id)

View File

@ -98,7 +98,6 @@ class TestConfig(object):
"valid_values": { "valid_values": {
"connect_timeout": 120, "connect_timeout": 120,
"local_infile": 0, "local_infile": 0,
"innodb_log_checksums": False
}, },
"appending_values": { "appending_values": {
"join_buffer_size": 1048576, "join_buffer_size": 1048576,