From 5babb68031e6e07b5b84db092bd06b8ecc79b464 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 19 Oct 2016 01:54:44 +0200 Subject: [PATCH 01/15] tests/backupcompatibility: verify if all files got restored There is still no verification of disk images content, nor VM properties... --- qubes/tests/int/backupcompatibility.py | 55 +++++++++++++------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/qubes/tests/int/backupcompatibility.py b/qubes/tests/int/backupcompatibility.py index cfede7c6..bb98af23 100644 --- a/qubes/tests/int/backupcompatibility.py +++ b/qubes/tests/int/backupcompatibility.py @@ -405,6 +405,11 @@ class TC_00_BackupCompatibility( output.close() + def assertRestored(self, name): + with self.assertNotRaises((KeyError, qubes.exc.QubesException)): + vm = self.app.domains[name] + vm.storage.verify() + def test_100_r1(self): self.create_v1_files(r2b2=False) @@ -418,12 +423,11 @@ class TC_00_BackupCompatibility( 'use-default-netvm': True, }, ) - with self.assertNotRaises(KeyError): - vm = self.app.domains["test-template-clone"] - vm = self.app.domains["test-testproxy"] - vm = self.app.domains["test-work"] - vm = self.app.domains["test-standalonevm"] - vm = self.app.domains["test-custom-template-appvm"] + self.assertRestored("test-template-clone") + self.assertRestored("test-testproxy") + self.assertRestored("test-work") + self.assertRestored("test-standalonevm") + self.assertRestored("test-custom-template-appvm") self.assertEqual(self.app.domains["test-custom-template-appvm"] .template, self.app.domains["test-template-clone"]) @@ -439,13 +443,12 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - with self.assertNotRaises(KeyError): - vm = self.app.domains["test-template-clone"] - vm = self.app.domains["test-testproxy"] - vm = self.app.domains["test-work"] - vm = self.app.domains["test-testhvm"] - vm = self.app.domains["test-standalonevm"] - vm = self.app.domains["test-custom-template-appvm"] + self.assertRestored("test-template-clone") + self.assertRestored("test-testproxy") + self.assertRestored("test-work") + self.assertRestored("test-testhvm") + self.assertRestored("test-standalonevm") + self.assertRestored("test-custom-template-appvm") self.assertEqual(self.app.domains["test-custom-template-appvm"] .template, self.app.domains["test-template-clone"]) @@ -457,13 +460,12 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - with self.assertNotRaises(KeyError): - vm = self.app.domains["test-template-clone"] - vm = self.app.domains["test-testproxy"] - vm = self.app.domains["test-work"] - vm = self.app.domains["test-testhvm"] - vm = self.app.domains["test-standalonevm"] - vm = self.app.domains["test-custom-template-appvm"] + self.assertRestored("test-template-clone") + self.assertRestored("test-testproxy") + self.assertRestored("test-work") + self.assertRestored("test-testhvm") + self.assertRestored("test-standalonevm") + self.assertRestored("test-custom-template-appvm") self.assertEqual(self.app.domains["test-custom-template-appvm"] .template, self.app.domains["test-template-clone"]) @@ -475,13 +477,12 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - with self.assertNotRaises(KeyError): - vm = self.app.domains["test-template-clone"] - vm = self.app.domains["test-testproxy"] - vm = self.app.domains["test-work"] - vm = self.app.domains["test-testhvm"] - vm = self.app.domains["test-standalonevm"] - vm = self.app.domains["test-custom-template-appvm"] + self.assertRestored("test-template-clone") + self.assertRestored("test-testproxy") + self.assertRestored("test-work") + self.assertRestored("test-testhvm") + self.assertRestored("test-standalonevm") + self.assertRestored("test-custom-template-appvm") self.assertEqual(self.app.domains["test-custom-template-appvm"] .template, self.app.domains["test-template-clone"]) From fbecd08a5856e6eb5da684af705efce9fba06964 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 19 Oct 2016 01:57:59 +0200 Subject: [PATCH 02/15] tests/backup: exclude some VMs during restore --- qubes/tests/int/backup.py | 27 +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 deletions(-) diff --git a/qubes/tests/int/backup.py b/qubes/tests/int/backup.py index bbefd1f1..d317ba22 100644 --- a/qubes/tests/int/backup.py +++ b/qubes/tests/int/backup.py @@ -176,7 +176,7 @@ class BackupTestsMixin(qubes.tests.SystemTestsMixin): #self.reload_db() def restore_backup(self, source=None, appvm=None, options=None, - expect_errors=None): + expect_errors=None, manipulate_restore_info=None): if source is None: backupfile = os.path.join(self.backupdir, sorted(os.listdir(self.backupdir))[-1]) @@ -190,6 +190,8 @@ class BackupTestsMixin(qubes.tests.SystemTestsMixin): for key, value in options.items(): setattr(restore_op.options, key, value) restore_info = restore_op.get_restore_info() + if callable(manipulate_restore_info): + restore_info = manipulate_restore_info(restore_info) self.log.debug(restore_op.get_restore_summary(restore_info)) with self.assertNotRaises(qubes.exc.QubesException): @@ -357,7 +359,28 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.QubesTestCase): # create backup with internal dependencies (template, netvm etc) # try restoring only AppVMs (but not templates, netvms) - should # handle according to options set - self.skipTest('test not implemented') + exclude = [ + self.make_vm_name('test-net'), + self.make_vm_name('template') + ] + def exclude_some(restore_info): + for name in exclude: + restore_info.pop(name) + return restore_info + vms = self.create_backup_vms() + orig_hashes = self.vm_checksum(vms) + self.make_backup(vms, compression_filter="bzip2") + self.remove_vms(reversed(vms)) + self.restore_backup(manipulate_restore_info=exclude_some) + for vm in vms: + if vm.name == self.make_vm_name('test1'): + # netvm was set to 'test-inst-test-net' - excluded + vm.netvm = qubes.property.DEFAULT + elif vm.name == self.make_vm_name('custom'): + # template was set to 'test-inst-template' - excluded + vm.template = self.app.default_template + vms = [vm for vm in vms if vm.name not in exclude] + self.assertCorrectlyRestored(vms, orig_hashes) def test_100_backup_dom0_no_restore(self): # do not write it into dom0 home itself... From 673fe4423ab630adbf48c3095dd192357d58953d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 19 Oct 2016 01:58:43 +0200 Subject: [PATCH 03/15] tests: handle LVM thin pool --- qubes/tests/__init__.py | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 5649190e..bed46c59 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -63,6 +63,17 @@ VMPREFIX = 'test-inst-' CLSVMPREFIX = 'test-cls-' +if 'DEFAULT_LVM_POOL' in os.environ.keys(): + DEFAULT_LVM_POOL = os.environ['DEFAULT_LVM_POOL'] +else: + DEFAULT_LVM_POOL = 'qubes_dom0/pool00' + + +POOL_CONF = {'name': 'test-lvm', + 'driver': 'lvm_thin', + 'volume_group': DEFAULT_LVM_POOL.split('/')[0], + 'thin_pool': DEFAULT_LVM_POOL.split('/')[1]} + #: :py:obj:`True` if running in dom0, :py:obj:`False` otherwise in_dom0 = False @@ -493,6 +504,30 @@ class SystemTestsMixin(object): ) self.app.default_netvm = netvm_clone + + def _find_pool(self, volume_group, thin_pool): + ''' Returns the pool matching the specified ``volume_group`` & + ``thin_pool``, or None. + ''' + pools = [p for p in self.app.pools + if issubclass(p.__class__, qubes.storage.lvm.ThinPool)] + for pool in pools: + if pool.volume_group == volume_group \ + and pool.thin_pool == thin_pool: + return pool + return None + + def init_lvm_pool(self): + volume_group, thin_pool = DEFAULT_LVM_POOL.split('/', 1) + path = "/dev/mapper/{!s}-{!s}".format(volume_group, thin_pool) + if not os.path.exists(path): + self.skipTest('LVM thin pool {!r} does not exist'. + format(DEFAULT_LVM_POOL)) + self.pool = self._find_pool(volume_group, thin_pool) + if not self.pool: + self.pool = self.app.add_pool(**POOL_CONF) + self.created_pool = True + def reload_db(self): self.app = qubes.Qubes(qubes.tests.XMLPATH) From 6ee200236cad390a76db00c726250f74ba28232e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 19 Oct 2016 01:59:11 +0200 Subject: [PATCH 04/15] tests/backup: verify migration into LVM thin pool --- qubes/tests/int/backupcompatibility.py | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/qubes/tests/int/backupcompatibility.py b/qubes/tests/int/backupcompatibility.py index bb98af23..99f4bf3b 100644 --- a/qubes/tests/int/backupcompatibility.py +++ b/qubes/tests/int/backupcompatibility.py @@ -486,3 +486,17 @@ class TC_00_BackupCompatibility( self.assertEqual(self.app.domains["test-custom-template-appvm"] .template, self.app.domains["test-template-clone"]) + + +class TC_01_BackupCompatibilityIntoLVM(TC_00_BackupCompatibility): + def setUp(self): + super(TC_01_BackupCompatibilityIntoLVM, self).setUp() + self.init_lvm_pool() + + def restore_backup(self, source=None, appvm=None, options=None, + expect_errors=None, manipulate_restore_info=None): + if options is None: + options = {} + options['override_pool'] = self.pool.name + super(TC_01_BackupCompatibilityIntoLVM, self).restore_backup(source, + appvm, options, expect_errors, manipulate_restore_info) From d7c355eadbd6f28e5ebaac5016b408531a355d79 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 19 Oct 2016 22:41:49 +0200 Subject: [PATCH 05/15] backup: make wait_backup_feedback/handle_streams less ugly Have a generic function `handle_streams`, instead of `wait_backup_feedback` with open coded process names and manual iteration over them. No functional change, besides minor logging change. --- qubes/backup.py | 198 +++++++++++++++++++----------------------------- 1 file changed, 76 insertions(+), 122 deletions(-) diff --git a/qubes/backup.py b/qubes/backup.py index aeda2dde..637f24c5 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -680,21 +680,23 @@ class Backup(object): i += 1 chunkfile_p = open(chunkfile, 'wb') - common_args = { - 'backup_target': chunkfile_p, - 'hmac': hmac, - 'vmproc': vmproc, - 'addproc': tar_sparse, - 'progress_callback': self._add_vm_progress, - 'size_limit': self.chunk_size, - } - run_error = wait_backup_feedback( - in_stream=pipe, streamproc=encryptor, - **common_args) + run_error = handle_streams( + pipe, + {'hmac_data': hmac.stdin, + 'backup_target': chunkfile_p, + }, + {'hmac': hmac, + 'vmproc': vmproc, + 'addproc': tar_sparse, + 'streamproc': encryptor, + }, + self.chunk_size, + self._add_vm_progress + ) chunkfile_p.close() self.log.debug( - "Wait_backup_feedback returned: {}".format(run_error)) + "12 returned: {}".format(run_error)) if self.canceled: try: @@ -783,96 +785,52 @@ class Backup(object): self.app.save() - - -def wait_backup_feedback(progress_callback, in_stream, streamproc, - backup_target, hmac=None, vmproc=None, - addproc=None, - size_limit=None): - ''' - Wait for backup chunk to finish - - Monitor all the processes (streamproc, hmac, vmproc, addproc) for errors - - Copy stdout of streamproc to backup_target and hmac stdin if available - - Compute progress based on total_backup_sz and send progress to - progress_callback function - - Returns if - - one of the monitored processes error out (streamproc, hmac, vmproc, - addproc), along with the processe that failed - - all of the monitored processes except vmproc finished successfully - (vmproc termination is controlled by the python script) - - streamproc does not delivers any data anymore (return with the error - "") - - size_limit is provided and is about to be exceeded +def handle_streams(stream_in, streams_out, processes, size_limit=None, + progress_callback=None): ''' + Copy stream_in to all streams_out and monitor all mentioned processes. + If any of them terminate with non-zero code, interrupt the process. Copy + at most `size_limit` data (if given). + :param stream_in: file-like object to read data from + :param streams_out: dict of file-like objects to write data to + :param processes: dict of subprocess.Popen objects to monitor + :param size_limit: int maximum data amount to process + :param progress_callback: callable function to report progress, will be + given copied data size (it should accumulate internally) + :return: failed process name, failed stream name, "size_limit" or None ( + no error) + ''' buffer_size = 409600 - run_error = None - run_count = 1 bytes_copied = 0 - log = logging.getLogger('qubes.backup') + while True: + if size_limit: + to_copy = min(buffer_size, size_limit - bytes_copied) + if to_copy <= 0: + return "size_limit" + else: + to_copy = buffer_size + buf = stream_in.read(to_copy) + if not len(buf): + # done + return None - while run_count > 0 and run_error is None: - if size_limit and bytes_copied + buffer_size > size_limit: - return "size_limit" - - buf = in_stream.read(buffer_size) if callable(progress_callback): progress_callback(len(buf)) + for name, stream in streams_out.items(): + if stream is None: + continue + try: + stream.write(buf) + except IOError: + return name bytes_copied += len(buf) - run_count = 0 - if hmac: - retcode = hmac.poll() - if retcode is not None: - if retcode != 0: - run_error = "hmac" - else: - run_count += 1 - - if addproc: - retcode = addproc.poll() - if retcode is not None: - if retcode != 0: - run_error = "addproc" - else: - run_count += 1 - - if vmproc: - retcode = vmproc.poll() - if retcode is not None: - if retcode != 0: - run_error = "VM" - log.debug(vmproc.stdout.read()) - else: - # VM should run until the end - pass - - if streamproc: - retcode = streamproc.poll() - if retcode is not None: - if retcode != 0: - run_error = "streamproc" - break - elif retcode == 0 and len(buf) <= 0: - return "" - run_count += 1 - - else: - if len(buf) <= 0: - return "" - - try: - backup_target.write(buf) - except IOError as e: - if e.errno == errno.EPIPE: - run_error = "target" - else: - raise - - if hmac: - hmac.stdin.write(buf) - - return run_error + for name, proc in processes.items(): + if proc is None: + continue + if proc.poll(): + return name class ExtractWorker2(Process): @@ -1127,6 +1085,10 @@ class ExtractWorker2(Process): self.tar2_current_file = filename pipe = open(self.restore_pipe, 'wb') + monitor_processes = { + 'vmproc': self.vmproc, + 'addproc': self.tar2_process, + } common_args = { 'backup_target': pipe, 'hmac': None, @@ -1144,28 +1106,23 @@ class ExtractWorker2(Process): (["-z"] if self.compressed else []), stdin=open(filename, 'rb'), stdout=subprocess.PIPE) - - run_error = wait_backup_feedback( - progress_callback=self.progress_callback, - in_stream=self.decryptor_process.stdout, - streamproc=self.decryptor_process, - **common_args) + in_stream = self.decryptor_process.stdout + monitor_processes['decryptor'] = self.decryptor_process elif self.compressed: self.decompressor_process = subprocess.Popen( ["gzip", "-d"], stdin=open(filename, 'rb'), stdout=subprocess.PIPE) - - run_error = wait_backup_feedback( - progress_callback=self.progress_callback, - in_stream=self.decompressor_process.stdout, - streamproc=self.decompressor_process, - **common_args) + in_stream = self.decompressor_process.stdout + monitor_processes['decompresor'] = self.decompressor_process else: - run_error = wait_backup_feedback( - progress_callback=self.progress_callback, - in_stream=open(filename, "rb"), streamproc=None, - **common_args) + in_stream = open(filename, 'rb') + + run_error = handle_streams( + in_stream, + {'target': pipe}, + monitor_processes, + progress_callback=self.progress_callback) try: pipe.close() @@ -1177,7 +1134,7 @@ class ExtractWorker2(Process): # ignore the error else: raise - if len(run_error): + if run_error: if run_error == "target": self.collect_tar_output() details = "\n".join(self.tar2_stderr) @@ -1310,19 +1267,16 @@ class ExtractWorker3(ExtractWorker2): self.log.debug("Releasing next chunck") self.tar2_current_file = filename - common_args = { - 'backup_target': input_pipe, - 'hmac': None, - 'vmproc': self.vmproc, - 'addproc': self.tar2_process - } + run_error = handle_streams( + open(filename, 'rb'), + {'target': input_pipe}, + {'vmproc': self.vmproc, + 'addproc': self.tar2_process, + 'decryptor': self.decryptor_process, + }, + progress_callback=self.progress_callback) - run_error = wait_backup_feedback( - progress_callback=self.progress_callback, - in_stream=open(filename, "rb"), streamproc=None, - **common_args) - - if len(run_error): + if run_error: if run_error == "target": self.collect_tar_output() details = "\n".join(self.tar2_stderr) From 418d74968092bfc806d864535f64d220d13e206e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 20 Oct 2016 12:56:51 +0200 Subject: [PATCH 06/15] backup: use 'scrypt' tool for backup encryption and integrity protection `openssl dgst` and `openssl enc` used previously poorly handle key stretching - in case of `openssl enc` encryption key is derived using single MD5 iteration, without even any salt. This hardly prevent brute force or even rainbow tables attacks. To make things worse, the same key is used for encryption and integrity protection which ease brute force even further. All this is still about brute force attacks, so when using long, high entropy passphrase, it should be still relatively safe. But lets do better. According to discussion in QubesOS/qubes-issues#971, scrypt algorithm is a good choice for key stretching (it isn't the best of all existing, but a good one and widely adopted). At the same time, lets switch away from `openssl` tool, as it is very limited and apparently not designed for production use. Use `scrypt` tool, which is very simple and does exactly what we need - encrypt the data and integrity protect it. Its archive format have own (simple) header with data required by the `scrypt` algorithm, including salt. Internally data is encrypted with AES256-CTR and integrity protected with HMAC-SHA256. For details see: https://github.com/tarsnap/scrypt/blob/master/FORMAT This means change of backup format. Mainly: 1. HMAC is stored in scrypt header, so don't use separate file for it. Instead have data in files with `.enc` extension. 2. For compatibility leave `backup-header` and `backup-header.hmac`. But `backup-header.hmac` is really scrypt-encrypted version of `backup-header`. 3. For each file, prepend its identifier to the passphrase, to authenticate filename itself too. Having this we can guard against reordering archive files within a single backup and across backups. This identifier is built as: backup ID (from backup-header)!filename! For backup-header itself, there is no backup ID (just 'backup-header!'). Fixes QubesOS/qubes-issues#971 --- qubes/backup.py | 402 +++++++++++++++++++++++++--------------- rpm_spec/core-dom0.spec | 1 + 2 files changed, 258 insertions(+), 145 deletions(-) diff --git a/qubes/backup.py b/qubes/backup.py index 637f24c5..361d3c2f 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -24,6 +24,8 @@ from __future__ import unicode_literals import itertools import logging +import termios + from qubes.utils import size_to_human import sys import stat @@ -50,7 +52,9 @@ QUEUE_FINISHED = "FINISHED" HEADER_FILENAME = 'backup-header' DEFAULT_CRYPTO_ALGORITHM = 'aes-256-cbc' -DEFAULT_HMAC_ALGORITHM = 'SHA512' +# 'scrypt' is not exactly HMAC algorithm, but a tool we use to +# integrity-protect the data +DEFAULT_HMAC_ALGORITHM = 'scrypt' DEFAULT_COMPRESSION_FILTER = 'gzip' CURRENT_BACKUP_FORMAT_VERSION = '4' # Maximum size of error message get from process stderr (including VM process) @@ -213,6 +217,63 @@ class SendWorker(Process): self.log.debug("Finished sending thread") +def launch_proc_with_pty(args, stdin=None, stdout=None, stderr=None, echo=True): + """Similar to pty.fork, but handle stdin/stdout according to parameters + instead of connecting to the pty + + :return tuple (subprocess.Popen, pty_master) + """ + + def set_ctty(ctty_fd, master_fd): + os.setsid() + os.close(master_fd) + fcntl.ioctl(ctty_fd, termios.TIOCSCTTY, 0) + if not echo: + termios_p = termios.tcgetattr(ctty_fd) + # termios_p.c_lflags + termios_p[3] &= ~termios.ECHO + termios.tcsetattr(ctty_fd, termios.TCSANOW, termios_p) + (pty_master, pty_slave) = os.openpty() + p = subprocess.Popen(args, stdin=stdin, stdout=stdout, stderr=stderr, + preexec_fn=lambda: set_ctty(pty_slave, pty_master)) + os.close(pty_slave) + return p, os.fdopen(pty_master, 'w+') + + +def launch_scrypt(action, input_name, output_name, passphrase): + ''' + Launch 'scrypt' process, pass passphrase to it and return + subprocess.Popen object. + + :param action: 'enc' or 'dec' + :param input_name: input path or '-' for stdin + :param output_name: output path or '-' for stdout + :param passphrase: passphrase + :return: subprocess.Popen object + ''' + command_line = ['scrypt', action, input_name, output_name] + (p, pty) = launch_proc_with_pty(command_line, + stdin=subprocess.PIPE if input_name == '-' else None, + stdout=subprocess.PIPE if output_name == '-' else None, + stderr=subprocess.PIPE, + echo=False) + if action == 'enc': + prompts = ('Please enter passphrase: ', 'Please confirm passphrase: ') + else: + prompts = ('Please enter passphrase: ',) + for prompt in prompts: + actual_prompt = p.stderr.read(len(prompt)) + if actual_prompt != prompt: + raise qubes.exc.QubesException( + 'Unexpected prompt from scrypt: {}'.format(actual_prompt)) + pty.write(passphrase + '\n') + pty.flush() + # save it here, so garbage collector would not close it (which would kill + # the child) + p.pty = pty + return p + + class Backup(object): class FileToBackup(object): def __init__(self, file_path, subdir=None, name=None): @@ -476,15 +537,18 @@ class Backup(object): compression_filter=self.compression_filter, ) backup_header.save(header_file_path) + # Start encrypt, scrypt will also handle integrity + # protection + scrypt_passphrase = HEADER_FILENAME + '!' + self.passphrase.encode( + 'utf-8') + scrypt = launch_scrypt( + 'enc', header_file_path, header_file_path + '.hmac', + scrypt_passphrase) - hmac = subprocess.Popen( - ["openssl", "dgst", "-" + self.hmac_algorithm, - "-hmac", self.passphrase], - stdin=open(header_file_path, "r"), - stdout=open(header_file_path + ".hmac", "w")) - if hmac.wait() != 0: + if scrypt.wait() != 0: raise qubes.exc.QubesException( - "Failed to compute hmac of header file") + "Failed to compute hmac of header file: " + + scrypt.stderr.read()) return HEADER_FILENAME, HEADER_FILENAME + ".hmac" @@ -640,60 +704,39 @@ class Backup(object): self.log.debug(" ".join(tar_cmdline)) - # Tips: Popen(bufsize=0) - # Pipe: tar-sparse | encryptor [| hmac] | tar | backup_target - # Pipe: tar-sparse [| hmac] | tar | backup_target + # Pipe: tar-sparse | scrypt | tar | backup_target # TODO: log handle stderr tar_sparse = subprocess.Popen( - tar_cmdline, stdin=subprocess.PIPE) + tar_cmdline) self.processes_to_kill_on_cancel.append(tar_sparse) # Wait for compressor (tar) process to finish or for any # error of other subprocesses i = 0 + pipe = open(backup_pipe, 'rb') run_error = "paused" - encryptor = None - if self.encrypted: - # Start encrypt - # If no cipher is provided, - # the data is forwarded unencrypted !!! - encryptor = subprocess.Popen([ - "openssl", "enc", - "-e", "-" + self.crypto_algorithm, - "-pass", "pass:" + passphrase], - stdin=open(backup_pipe, 'rb'), - stdout=subprocess.PIPE) - pipe = encryptor.stdout - else: - pipe = open(backup_pipe, 'rb') while run_error == "paused": - - # Start HMAC - hmac = subprocess.Popen([ - "openssl", "dgst", "-" + self.hmac_algorithm, - "-hmac", passphrase], - stdin=subprocess.PIPE, - stdout=subprocess.PIPE) - # Prepare a first chunk - chunkfile = backup_tempfile + "." + "%03d" % i + chunkfile = backup_tempfile + ".%03d.enc" % i i += 1 - chunkfile_p = open(chunkfile, 'wb') + + # Start encrypt, scrypt will also handle integrity + # protection + scrypt_passphrase = os.path.relpath(chunkfile[:-4], + self.tmpdir) + '!' + passphrase + scrypt = launch_scrypt( + "enc", "-", chunkfile, scrypt_passphrase) run_error = handle_streams( pipe, - {'hmac_data': hmac.stdin, - 'backup_target': chunkfile_p, - }, - {'hmac': hmac, - 'vmproc': vmproc, + {'backup_target': scrypt.stdin}, + {'vmproc': vmproc, 'addproc': tar_sparse, - 'streamproc': encryptor, + 'scrypt': scrypt, }, self.chunk_size, self._add_vm_progress ) - chunkfile_p.close() self.log.debug( "12 returned: {}".format(run_error)) @@ -703,12 +746,7 @@ class Backup(object): tar_sparse.terminate() except OSError: pass - try: - hmac.terminate() - except OSError: - pass tar_sparse.wait() - hmac.wait() to_send.put(QUEUE_ERROR) send_proc.join() shutil.rmtree(self.tmpdir) @@ -724,29 +762,16 @@ class Backup(object): "Failed to perform backup: error in " + run_error) + scrypt.stdin.close() + scrypt.wait() + self.log.debug("scrypt return code: {}".format( + scrypt.poll())) + # Send the chunk to the backup target self._queue_put_with_check( send_proc, vmproc, to_send, os.path.relpath(chunkfile, self.tmpdir)) - # Close HMAC - hmac.stdin.close() - hmac.wait() - self.log.debug("HMAC proc return code: {}".format( - hmac.poll())) - - # Write HMAC data next to the chunk file - hmac_data = hmac.stdout.read() - self.log.debug( - "Writing hmac to {}.hmac".format(chunkfile)) - with open(chunkfile + ".hmac", 'w') as hmac_file: - hmac_file.write(hmac_data) - - # Send the HMAC to the backup target - self._queue_put_with_check( - send_proc, vmproc, to_send, - os.path.relpath(chunkfile, self.tmpdir) + ".hmac") - if tar_sparse.poll() is None or run_error == "size_limit": run_error = "paused" else: @@ -1310,6 +1335,8 @@ def get_supported_hmac_algo(hmac_algorithm=None): # Start with provided default if hmac_algorithm: yield hmac_algorithm + if hmac_algorithm != 'scrypt': + yield 'scrypt' proc = subprocess.Popen(['openssl', 'list-message-digest-algorithms'], stdout=subprocess.PIPE) for algo in proc.stdout.readlines(): @@ -1529,6 +1556,10 @@ class BackupRestore(object): def _verify_hmac(self, filename, hmacfile, algorithm=None): def load_hmac(hmac_text): + if filter(lambda x: ord(x) not in range(128), + hmac_text): + raise qubes.exc.QubesException( + "Invalid content of {}".format(hmacfile)) hmac_text = hmac_text.strip().split("=") if len(hmac_text) > 1: hmac_text = hmac_text[1].strip() @@ -1547,6 +1578,17 @@ class BackupRestore(object): "ERROR: expected hmac for {}, but got {}". format(filename, hmacfile)) + if algorithm == 'scrypt': + # in case of 'scrypt' _verify_hmac is only used for backup header + assert filename == HEADER_FILENAME + self._verify_and_decrypt(hmacfile, HEADER_FILENAME + '.dec') + if open(os.path.join(self.tmpdir, filename)).read() != \ + open(os.path.join(self.tmpdir, filename + '.dec')).read(): + raise qubes.exc.QubesException( + 'Invalid hmac on {}'.format(filename)) + else: + return True + hmac_proc = subprocess.Popen( ["openssl", "dgst", "-" + algorithm, "-hmac", passphrase], stdin=open(os.path.join(self.tmpdir, filename), 'rb'), @@ -1572,6 +1614,72 @@ class BackupRestore(object): "Is the passphrase correct?". format(filename, load_hmac(hmac_stdout))) + def _verify_and_decrypt(self, filename, output=None): + assert filename.endswith('.enc') or filename.endswith('.hmac') + fullname = os.path.join(self.tmpdir, filename) + (origname, _) = os.path.splitext(filename) + if output: + fulloutput = os.path.join(self.tmpdir, output) + else: + fulloutput = os.path.join(self.tmpdir, origname) + passphrase = origname + '!' + self.passphrase.encode('utf-8') + p = launch_scrypt('dec', fullname, fulloutput, passphrase) + (_, stderr) = p.communicate() + if p.returncode != 0: + os.unlink(fulloutput) + raise qubes.exc.QubesException('failed to decrypt {}: {}'.format( + fullname, stderr)) + # encrypted file is no longer needed + os.unlink(fullname) + return origname + + def _retrieve_backup_header_files(self, files, allow_none=False): + (retrieve_proc, filelist_pipe, error_pipe) = \ + self._start_retrieval_process( + files, len(files), 1024 * 1024) + filelist = filelist_pipe.read() + retrieve_proc_returncode = retrieve_proc.wait() + if retrieve_proc in self.processes_to_kill_on_cancel: + self.processes_to_kill_on_cancel.remove(retrieve_proc) + extract_stderr = error_pipe.read(MAX_STDERR_BYTES) + + # wait for other processes (if any) + for proc in self.processes_to_kill_on_cancel: + if proc.wait() != 0: + raise qubes.exc.QubesException( + "Backup header retrieval failed (exit code {})".format( + proc.wait()) + ) + + if retrieve_proc_returncode != 0: + if not filelist and 'Not found in archive' in extract_stderr: + if allow_none: + return None + else: + raise qubes.exc.QubesException( + "unable to read the qubes backup file {0} ({1}): {2}".format( + self.backup_location, + retrieve_proc.wait(), + extract_stderr + )) + actual_files = filelist.splitlines() + if sorted(actual_files) != sorted(files): + raise qubes.exc.QubesException( + 'unexpected files in archive: got {!r}, expeced {!r}'.format( + actual_files, files + )) + for f in files: + if not os.path.exists(os.path.join(self.tmpdir, f)): + if allow_none: + return None + else: + raise qubes.exc.QubesException( + 'Unable to retrieve file {} from backup {}: {}'.format( + f, self.backup_location, extract_stderr + ) + ) + return files + def _retrieve_backup_header(self): """Retrieve backup header and qubes.xml. Only backup header is analyzed, qubes.xml is left as-is @@ -1588,82 +1696,47 @@ class BackupRestore(object): header_data.version = 1 return header_data - (retrieve_proc, filelist_pipe, error_pipe) = \ - self._start_retrieval_process( - ['backup-header', 'backup-header.hmac', - 'qubes.xml.000', 'qubes.xml.000.hmac'], 4, 1024 * 1024) + header_files = self._retrieve_backup_header_files( + ['backup-header', 'backup-header.hmac'], allow_none=True) - expect_tar_error = False - - filename = filelist_pipe.readline().strip() - hmacfile = filelist_pipe.readline().strip() - # tar output filename before actually extracting it, so wait for the - # next one before trying to access it - if not self.backup_vm: - filelist_pipe.readline().strip() - - self.log.debug("Got backup header and hmac: {}, {}".format( - filename, hmacfile)) - - if not filename or filename == "EOF" or \ - not hmacfile or hmacfile == "EOF": - retrieve_proc.wait() - proc_error_msg = error_pipe.read(MAX_STDERR_BYTES) - raise qubes.exc.QubesException( - "Premature end of archive while receiving " - "backup header. Process output:\n" + proc_error_msg) - file_ok = False - hmac_algorithm = DEFAULT_HMAC_ALGORITHM - for hmac_algo in get_supported_hmac_algo(hmac_algorithm): - try: - if self._verify_hmac(filename, hmacfile, hmac_algo): - file_ok = True - hmac_algorithm = hmac_algo - break - except qubes.exc.QubesException: - # Ignore exception here, try the next algo - pass - if not file_ok: - raise qubes.exc.QubesException( - "Corrupted backup header (hmac verification " - "failed). Is the password correct?") - if os.path.basename(filename) == HEADER_FILENAME: - filename = os.path.join(self.tmpdir, filename) - header_data = BackupHeader(open(filename, 'r').read()) - os.unlink(filename) - else: - # if no header found, create one with guessed HMAC algo + if not header_files: + # R2-Beta3 didn't have backup header, so if none is found, + # assume it's version=2 and use values present at that time header_data = BackupHeader( version=2, - hmac_algorithm=hmac_algorithm, # place explicitly this value, because it is what format_version # 2 have + hmac_algorithm='SHA1', crypto_algorithm='aes-256-cbc', # TODO: set encrypted to something... ) - # when tar do not find expected file in archive, it exit with - # code 2. This will happen because we've requested backup-header - # file, but the archive do not contain it. Ignore this particular - # error. - if not self.backup_vm: - expect_tar_error = True + else: + filename = HEADER_FILENAME + hmacfile = HEADER_FILENAME + '.hmac' + self.log.debug("Got backup header and hmac: {}, {}".format( + filename, hmacfile)) - if retrieve_proc.wait() != 0 and not expect_tar_error: - raise qubes.exc.QubesException( - "unable to read the qubes backup file {0} ({1}): {2}".format( - self.backup_location, - retrieve_proc.wait(), - error_pipe.read(MAX_STDERR_BYTES) - )) - if retrieve_proc in self.processes_to_kill_on_cancel: - self.processes_to_kill_on_cancel.remove(retrieve_proc) - # wait for other processes (if any) - for proc in self.processes_to_kill_on_cancel: - if proc.wait() != 0: + file_ok = False + hmac_algorithm = DEFAULT_HMAC_ALGORITHM + for hmac_algo in get_supported_hmac_algo(hmac_algorithm): + try: + if self._verify_hmac(filename, hmacfile, hmac_algo): + file_ok = True + break + except qubes.exc.QubesException as e: + self.log.debug( + 'Failed to verify {} using {}: {}'.format( + hmacfile, hmac_algo, str(e))) + # Ignore exception here, try the next algo + pass + if not file_ok: raise qubes.exc.QubesException( - "Backup header retrieval failed (exit code {})".format( - proc.wait()) - ) + "Corrupted backup header (hmac verification " + "failed). Is the password correct?") + filename = os.path.join(self.tmpdir, filename) + header_data = BackupHeader(open(filename, 'r').read()) + os.unlink(filename) + return header_data def _start_inner_extraction_worker(self, queue, relocate): @@ -1696,6 +1769,9 @@ class BackupRestore(object): elif format_version in [3, 4]: extractor_params['compression_filter'] = \ self.header_data.compression_filter + if format_version == 4: + # encryption already handled + extractor_params['encrypted'] = False extract_proc = ExtractWorker3(**extractor_params) else: raise NotImplementedError( @@ -1714,7 +1790,14 @@ class BackupRestore(object): offline_mode=True) return backup_app else: - self._verify_hmac("qubes.xml.000", "qubes.xml.000.hmac") + if self.header_data.version in [2, 3]: + self._retrieve_backup_header_files( + ['qubes.xml.000', 'qubes.xml.000.hmac']) + self._verify_hmac("qubes.xml.000", "qubes.xml.000.hmac") + else: + self._retrieve_backup_header_files(['qubes.xml.000.enc']) + self._verify_and_decrypt('qubes.xml.000.enc') + queue = Queue() queue.put("qubes.xml.000") queue.put(QUEUE_FINISHED) @@ -1762,6 +1845,7 @@ class BackupRestore(object): try: filename = None + hmacfile = None nextfile = None while True: if self.canceled: @@ -1785,30 +1869,58 @@ class BackupRestore(object): if not filename or filename == "EOF": break - hmacfile = filelist_pipe.readline().strip() - - if self.canceled: - break # if reading archive directly with tar, wait for next filename - # tar prints filename before processing it, so wait for # the next one to be sure that whole file was extracted if not self.backup_vm: nextfile = filelist_pipe.readline().strip() - self.log.debug("Getting hmac:" + hmacfile) - if not hmacfile or hmacfile == "EOF": - # Premature end of archive, either of tar1_command or - # vmproc exited with error - break + if self.header_data.version in [2, 3]: + if not self.backup_vm: + hmacfile = nextfile + nextfile = filelist_pipe.readline().strip() + else: + hmacfile = filelist_pipe.readline().strip() + + if self.canceled: + break + + self.log.debug("Getting hmac:" + hmacfile) + if not hmacfile or hmacfile == "EOF": + # Premature end of archive, either of tar1_command or + # vmproc exited with error + break + else: # self.header_data.version == 4 + if not filename.endswith('.enc'): + raise qubes.exc.QubesException( + 'Invalid file extension found in archive: {}'. + format(filename)) if not any(map(lambda x: filename.startswith(x), vms_dirs)): self.log.debug("Ignoring VM not selected for restore") os.unlink(os.path.join(self.tmpdir, filename)) - os.unlink(os.path.join(self.tmpdir, hmacfile)) + if hmacfile: + os.unlink(os.path.join(self.tmpdir, hmacfile)) continue - if self._verify_hmac(filename, hmacfile): - to_extract.put(os.path.join(self.tmpdir, filename)) + if self.header_data.version in [2, 3]: + self._verify_hmac(filename, hmacfile) + else: + # _verify_and_decrypt will write output to a file with + # '.enc' extension cut off. This is safe because: + # - `scrypt` tool will override output, so if the file was + # already there (received from the VM), it will be removed + # - incoming archive extraction will refuse to override + # existing file, so if `scrypt` already created one, + # it can not be manipulated by the VM + # - when the file is retrieved from the VM, it appears at + # the final form - if it's visible, VM have no longer + # influence over its content + # + # This all means that if the file was correctly verified + # + decrypted, we will surely access the right file + filename = self._verify_and_decrypt(filename) + to_extract.put(os.path.join(self.tmpdir, filename)) if self.canceled: raise BackupCanceledError("Restore canceled", diff --git a/rpm_spec/core-dom0.spec b/rpm_spec/core-dom0.spec index ecfd4391..bce75a54 100644 --- a/rpm_spec/core-dom0.spec +++ b/rpm_spec/core-dom0.spec @@ -83,6 +83,7 @@ Requires: createrepo Requires: gnome-packagekit Requires: cronie Requires: bsdtar +Requires: scrypt # for qubes-hcl-report Requires: dmidecode Requires: PyQt4 From 4ad15c082b52293fe04f59f37adb4207768bacaf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 25 Oct 2016 21:28:59 +0200 Subject: [PATCH 07/15] backup: add 'backup_id' to integrity protection This prevent switching parts of backup of the same VM between different backups made by the same user (or actually: with the same passphrase). QubesOS/qubes-issues#971 --- qubes/backup.py | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/qubes/backup.py b/qubes/backup.py index 361d3c2f..6e8f91b8 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -80,6 +80,7 @@ class BackupHeader(object): 'compression-filter': 'compression_filter', 'crypto-algorithm': 'crypto_algorithm', 'hmac-algorithm': 'hmac_algorithm', + 'backup-id': 'backup_id' } bool_options = ['encrypted', 'compressed'] int_options = ['version'] @@ -91,7 +92,8 @@ class BackupHeader(object): compressed=None, compression_filter=None, hmac_algorithm=None, - crypto_algorithm=None): + crypto_algorithm=None, + backup_id=None): # repeat the list to help code completion... self.version = version self.encrypted = encrypted @@ -101,6 +103,7 @@ class BackupHeader(object): self.compression_filter = compression_filter self.hmac_algorithm = hmac_algorithm self.crypto_algorithm = crypto_algorithm + self.backup_id = backup_id if header_data is not None: self.load(header_data) @@ -152,6 +155,8 @@ class BackupHeader(object): expected_attrs += ['crypto_algorithm'] if self.version >= 3 and self.compressed: expected_attrs += ['compression_filter'] + if self.version >= 4: + expected_attrs += ['backup_id'] for key in expected_attrs: if getattr(self, key) is None: raise qubes.exc.QubesException( @@ -353,6 +358,10 @@ class Backup(object): #: callback for progress reporting. Will be called with one argument #: - progress in percents self.progress_callback = None + #: backup ID, needs to be unique (for a given user), + #: not necessary unpredictable; automatically generated + self.backup_id = datetime.datetime.now().strftime( + '%Y%m%dT%H%M%S-' + str(os.getpid())) for key, value in kwargs.iteritems(): if hasattr(self, key): @@ -535,6 +544,7 @@ class Backup(object): encrypted=self.encrypted, compressed=self.compressed, compression_filter=self.compression_filter, + backup_id=self.backup_id, ) backup_header.save(header_file_path) # Start encrypt, scrypt will also handle integrity @@ -722,8 +732,12 @@ class Backup(object): # Start encrypt, scrypt will also handle integrity # protection - scrypt_passphrase = os.path.relpath(chunkfile[:-4], - self.tmpdir) + '!' + passphrase + scrypt_passphrase = \ + '{backup_id}!{filename}!{passphrase}'.format( + backup_id=self.backup_id, + filename=os.path.relpath(chunkfile[:-4], + self.tmpdir), + passphrase=passphrase) scrypt = launch_scrypt( "enc", "-", chunkfile, scrypt_passphrase) @@ -1622,7 +1636,14 @@ class BackupRestore(object): fulloutput = os.path.join(self.tmpdir, output) else: fulloutput = os.path.join(self.tmpdir, origname) - passphrase = origname + '!' + self.passphrase.encode('utf-8') + if origname == HEADER_FILENAME: + passphrase = origname + '!' + self.passphrase.encode('utf-8') + else: + passphrase = \ + '{backup_id}!{filename}!{passphrase}'.format( + backup_id=self.header_data.backup_id, + filename=origname, + passphrase=self.passphrase.encode('utf-8')) p = launch_scrypt('dec', fullname, fulloutput, passphrase) (_, stderr) = p.communicate() if p.returncode != 0: From 51b66208f308215732fcf4478b5bc782e02662f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 25 Oct 2016 21:31:39 +0200 Subject: [PATCH 08/15] backup: verify if archive chunks are not reordered Now, when file name is also integrity protected (prefixed to the passphrase), we can make sure that input files are given in the same order. And are parts of the same VM. QubesOS/qubes-issues#971 --- qubes/backup.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/qubes/backup.py b/qubes/backup.py index 6e8f91b8..a1ed07fd 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -1303,7 +1303,19 @@ class ExtractWorker3(ExtractWorker2): os.remove(filename) continue else: + (basename, ext) = os.path.splitext(self.tar2_current_file) + previous_chunk_number = int(ext[1:]) + expected_filename = basename + '.%03d' % ( + previous_chunk_number+1) + if expected_filename != filename: + self.cleanup_tar2(wait=True, terminate=True) + self.log.error( + 'Unexpected file in archive: {}, expected {}'.format( + filename, expected_filename)) + os.remove(filename) + continue self.log.debug("Releasing next chunck") + self.tar2_current_file = filename run_error = handle_streams( From 49e718cf57dcecd2b7b8bf888f7250db6ca1ace4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 25 Oct 2016 22:11:39 +0200 Subject: [PATCH 09/15] backup: mark 'encryption' option as deprecated - all backups are encrypted QubesOS/qubes-issues#971 --- qubes/backup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qubes/backup.py b/qubes/backup.py index a1ed07fd..74abd01b 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -376,7 +376,9 @@ class Backup(object): self.log = logging.getLogger('qubes.backup') - self.compression_filter = DEFAULT_COMPRESSION_FILTER + if not self.encrypted: + self.log.warning('\'encrypted\' option is ignored, backup is ' + 'always encrypted') if exclude_list is None: exclude_list = [] From fc00dd211e4bd1c30caaa67416a763aad2aa961b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Tue, 25 Oct 2016 22:12:36 +0200 Subject: [PATCH 10/15] tests/backup: test backup with non-ASCII passphrase --- qubes/tests/int/backup.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/qubes/tests/int/backup.py b/qubes/tests/int/backup.py index d317ba22..2220068e 100644 --- a/qubes/tests/int/backup.py +++ b/qubes/tests/int/backup.py @@ -161,7 +161,8 @@ class BackupTestsMixin(qubes.tests.SystemTestsMixin): else: raise - backup.passphrase = 'qubes' + if 'passphrase' not in kwargs: + backup.passphrase = 'qubes' backup.target_dir = target try: @@ -176,7 +177,8 @@ class BackupTestsMixin(qubes.tests.SystemTestsMixin): #self.reload_db() def restore_backup(self, source=None, appvm=None, options=None, - expect_errors=None, manipulate_restore_info=None): + expect_errors=None, manipulate_restore_info=None, + passphrase='qubes'): if source is None: backupfile = os.path.join(self.backupdir, sorted(os.listdir(self.backupdir))[-1]) @@ -185,7 +187,7 @@ class BackupTestsMixin(qubes.tests.SystemTestsMixin): with self.assertNotRaises(qubes.exc.QubesException): restore_op = qubes.backup.BackupRestore( - self.app, backupfile, appvm, "qubes") + self.app, backupfile, appvm, passphrase) if options: for key, value in options.items(): setattr(restore_op.options, key, value) From 043d20c05de68a7c58e6d754c1fc152662dfd81d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 26 Oct 2016 20:45:24 +0200 Subject: [PATCH 11/15] backup: fix handling non-ascii characters in backup passphrase Fixes QubesOS/qubes-issues#2398 --- qubes/backup.py | 25 ++++++++++++------------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/qubes/backup.py b/qubes/backup.py index 74abd01b..832e9832 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -271,7 +271,7 @@ def launch_scrypt(action, input_name, output_name, passphrase): if actual_prompt != prompt: raise qubes.exc.QubesException( 'Unexpected prompt from scrypt: {}'.format(actual_prompt)) - pty.write(passphrase + '\n') + pty.write(passphrase.encode('utf-8') + b'\n') pty.flush() # save it here, so garbage collector would not close it (which would kill # the child) @@ -551,8 +551,8 @@ class Backup(object): backup_header.save(header_file_path) # Start encrypt, scrypt will also handle integrity # protection - scrypt_passphrase = HEADER_FILENAME + '!' + self.passphrase.encode( - 'utf-8') + scrypt_passphrase = u'{filename}!{passphrase}'.format( + filename=HEADER_FILENAME, passphrase=self.passphrase) scrypt = launch_scrypt( 'enc', header_file_path, header_file_path + '.hmac', scrypt_passphrase) @@ -610,8 +610,6 @@ class Backup(object): backup_app.domains[qid].features['backup-size'] = vm_info.size backup_app.save() - passphrase = self.passphrase.encode('utf-8') - vmproc = None tar_sparse = None if self.target_vm is not None: @@ -735,11 +733,11 @@ class Backup(object): # Start encrypt, scrypt will also handle integrity # protection scrypt_passphrase = \ - '{backup_id}!{filename}!{passphrase}'.format( + u'{backup_id}!{filename}!{passphrase}'.format( backup_id=self.backup_id, filename=os.path.relpath(chunkfile[:-4], self.tmpdir), - passphrase=passphrase) + passphrase=self.passphrase) scrypt = launch_scrypt( "enc", "-", chunkfile, scrypt_passphrase) @@ -1651,13 +1649,14 @@ class BackupRestore(object): else: fulloutput = os.path.join(self.tmpdir, origname) if origname == HEADER_FILENAME: - passphrase = origname + '!' + self.passphrase.encode('utf-8') + passphrase = u'{filename}!{passphrase}'.format( + filename=origname, + passphrase=self.passphrase) else: - passphrase = \ - '{backup_id}!{filename}!{passphrase}'.format( - backup_id=self.header_data.backup_id, - filename=origname, - passphrase=self.passphrase.encode('utf-8')) + passphrase = u'{backup_id}!{filename}!{passphrase}'.format( + backup_id=self.header_data.backup_id, + filename=origname, + passphrase=self.passphrase) p = launch_scrypt('dec', fullname, fulloutput, passphrase) (_, stderr) = p.communicate() if p.returncode != 0: From af182c4fd1871e1d98242d5168b226eb93ebb795 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 26 Oct 2016 21:37:26 +0200 Subject: [PATCH 12/15] backup: fixup restore options just before restoring VMs When user included/excluded some VMs for restoration, it may be neceesarry to fix dependencies between them (for example when default template is no longer going to be restored). Also fix handling conflicting names. --- qubes/backup.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/qubes/backup.py b/qubes/backup.py index 832e9832..06441e33 100644 --- a/qubes/backup.py +++ b/qubes/backup.py @@ -2021,7 +2021,7 @@ class BackupRestore(object): vm_info.problems.add(self.VMToRestore.EXCLUDED) if not self.options.verify_only and \ - vm in self.app.domains: + vm_info.name in self.app.domains: if self.options.rename_conflicting: new_name = self.generate_new_name_for_conflicting_vm( vm, restore_info @@ -2343,6 +2343,8 @@ class BackupRestore(object): # FIXME handle locking + restore_info = self.restore_info_verify(restore_info) + self._restore_vms_metadata(restore_info) # Perform VM restoration in backup order From 64ac7f6e8de7f2982573db2c1a10446b7e038149 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Wed, 26 Oct 2016 21:39:44 +0200 Subject: [PATCH 13/15] tests/backup: check non-ASCII passphrase QubesOS/qubes-issues#2398 --- qubes/tests/int/backup.py | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/qubes/tests/int/backup.py b/qubes/tests/int/backup.py index 2220068e..83acb86b 100644 --- a/qubes/tests/int/backup.py +++ b/qubes/tests/int/backup.py @@ -384,6 +384,14 @@ class TC_00_Backup(BackupTestsMixin, qubes.tests.QubesTestCase): vms = [vm for vm in vms if vm.name not in exclude] self.assertCorrectlyRestored(vms, orig_hashes) + def test_020_encrypted_backup_non_ascii(self): + vms = self.create_backup_vms() + orig_hashes = self.vm_checksum(vms) + self.make_backup(vms, encrypted=True, passphrase=u'zażółć gęślą jaźń') + self.remove_vms(reversed(vms)) + self.restore_backup(passphrase=u'zażółć gęślą jaźń') + self.assertCorrectlyRestored(vms, orig_hashes) + def test_100_backup_dom0_no_restore(self): # do not write it into dom0 home itself... os.mkdir('/var/tmp/test-backup') From 36bd834c012f98bef9c44eb04afb64e469e72d97 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 27 Oct 2016 20:49:00 +0200 Subject: [PATCH 14/15] core2migration: try to set properties to "default" when possible Core3 keep information whether property have default value for all the properties (not only few like netvm or kernel). Try to use this feature as much as possible. --- qubes/core2migration.py | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/qubes/core2migration.py b/qubes/core2migration.py index 8ddd943d..3021f59d 100644 --- a/qubes/core2migration.py +++ b/qubes/core2migration.py @@ -162,12 +162,23 @@ class Core2Qubes(qubes.Qubes): 'template_qid'))] vm_class = AppVM # simple attributes - for attr in ['installed_by_rpm', 'include_in_backups', - 'qrexec_timeout', 'internal', 'label', 'name', - 'vcpus', 'memory', 'maxmem', 'default_user', - 'debug', 'pci_strictreset', 'mac', 'autostart']: + for attr, default in { + 'installed_by_rpm': 'False', + 'include_in_backups': 'True', + 'qrexec_timeout': '60', + 'internal': 'False', + 'label': None, + 'name': None, + 'vcpus': '2', + 'memory': '400', + 'maxmem': '4000', + 'default_user': 'user', + 'debug': 'False', + 'pci_strictreset': 'True', + 'mac': None, + 'autostart': 'False'}.items(): value = element.get(attr) - if value: + if value and value != default: kwargs[attr] = value # attributes with default value for attr in ["kernel", "kernelopts"]: From 8cf19e3c92f22e20b2a7ef9eef9c6fe72bbf5055 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?= Date: Thu, 27 Oct 2016 20:51:21 +0200 Subject: [PATCH 15/15] tests/backupcompatibility: verify restored VM properties --- qubes/tests/int/backupcompatibility.py | 226 +++++++++++++++++++++---- 1 file changed, 190 insertions(+), 36 deletions(-) diff --git a/qubes/tests/int/backupcompatibility.py b/qubes/tests/int/backupcompatibility.py index 99f4bf3b..419b1580 100644 --- a/qubes/tests/int/backupcompatibility.py +++ b/qubes/tests/int/backupcompatibility.py @@ -405,10 +405,27 @@ class TC_00_BackupCompatibility( output.close() - def assertRestored(self, name): + def assertRestored(self, name, **kwargs): with self.assertNotRaises((KeyError, qubes.exc.QubesException)): vm = self.app.domains[name] vm.storage.verify() + for prop, value in kwargs.items(): + if prop == 'klass': + self.assertIsInstance(vm, value) + elif value is qubes.property.DEFAULT: + self.assertTrue(vm.property_is_default(prop), + 'VM {} - property {} not default'.format(vm.name, prop)) + else: + actual_value = getattr(vm, prop) + if isinstance(actual_value, qubes.vm.BaseVM): + self.assertEqual(value, actual_value.name, + 'VM {} - property {}'.format(vm.name, prop)) + elif isinstance(actual_value, qubes.Label): + self.assertEqual(value, actual_value.name, + 'VM {} - property {}'.format(vm.name, prop)) + else: + self.assertEqual(value, actual_value, + 'VM {} - property {}'.format(vm.name, prop)) def test_100_r1(self): self.create_v1_files(r2b2=False) @@ -423,14 +440,49 @@ class TC_00_BackupCompatibility( 'use-default-netvm': True, }, ) - self.assertRestored("test-template-clone") - self.assertRestored("test-testproxy") - self.assertRestored("test-work") - self.assertRestored("test-standalonevm") - self.assertRestored("test-custom-template-appvm") - self.assertEqual(self.app.domains["test-custom-template-appvm"] - .template, - self.app.domains["test-template-clone"]) + common_props = { + 'installed_by_rpm': False, + 'kernel': qubes.property.DEFAULT, + 'kernelopts': qubes.property.DEFAULT, + 'qrexec_timeout': qubes.property.DEFAULT, + 'netvm': qubes.property.DEFAULT, + 'default_user': qubes.property.DEFAULT, + 'internal': qubes.property.DEFAULT, + 'include_in_backups': True, + 'debug': False, + 'maxmem': 4000, # 4063 caped by 10*400 + 'memory': 400, + } + self.assertRestored("test-template-clone", + klass=qubes.vm.templatevm.TemplateVM, + label='gray', + provides_network=False, + **common_props) + testproxy_props = common_props.copy() + testproxy_props.update( + label='yellow', + provides_network=True, + memory=200, + maxmem=2000, + template=self.app.default_template.name, + ) + self.assertRestored("test-testproxy", + klass=qubes.vm.appvm.AppVM, + **testproxy_props) + self.assertRestored("test-work", + klass=qubes.vm.appvm.AppVM, + template=self.app.default_template.name, + label='green', + **common_props) + self.assertRestored("test-standalonevm", + klass=qubes.vm.standalonevm.StandaloneVM, + label='red', + **common_props) + self.assertRestored("test-custom-template-appvm", + klass=qubes.vm.appvm.AppVM, + template='test-template-clone', + label='yellow', + **common_props) def test_200_r2b2(self): self.create_v1_files(r2b2=True) @@ -443,15 +495,51 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - self.assertRestored("test-template-clone") - self.assertRestored("test-testproxy") - self.assertRestored("test-work") - self.assertRestored("test-testhvm") - self.assertRestored("test-standalonevm") - self.assertRestored("test-custom-template-appvm") - self.assertEqual(self.app.domains["test-custom-template-appvm"] - .template, - self.app.domains["test-template-clone"]) + common_props = { + 'installed_by_rpm': False, + 'kernel': qubes.property.DEFAULT, + 'kernelopts': qubes.property.DEFAULT, + 'qrexec_timeout': qubes.property.DEFAULT, + 'netvm': qubes.property.DEFAULT, + 'default_user': qubes.property.DEFAULT, + 'internal': qubes.property.DEFAULT, + 'include_in_backups': True, + 'debug': False, + 'maxmem': 1535, + 'memory': 400, + } + template_clone_props = common_props.copy() + template_clone_props.update( + label='green', + provides_network=False, + ) + self.assertRestored("test-template-clone", + klass=qubes.vm.templatevm.TemplateVM, + **template_clone_props) + testproxy_props = common_props.copy() + testproxy_props.update( + label='red', + provides_network=True, + memory=200, + template=self.app.default_template.name, + ) + self.assertRestored("test-testproxy", + klass=qubes.vm.appvm.AppVM, + **testproxy_props) + self.assertRestored("test-work", + klass=qubes.vm.appvm.AppVM, + template=self.app.default_template.name, + label='green', + **common_props) + self.assertRestored("test-standalonevm", + klass=qubes.vm.standalonevm.StandaloneVM, + label='blue', + **common_props) + self.assertRestored("test-custom-template-appvm", + klass=qubes.vm.appvm.AppVM, + template='test-template-clone', + label='yellow', + **common_props) def test_210_r2(self): self.create_v3_backup(False) @@ -460,15 +548,48 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - self.assertRestored("test-template-clone") - self.assertRestored("test-testproxy") - self.assertRestored("test-work") - self.assertRestored("test-testhvm") - self.assertRestored("test-standalonevm") - self.assertRestored("test-custom-template-appvm") - self.assertEqual(self.app.domains["test-custom-template-appvm"] - .template, - self.app.domains["test-template-clone"]) + common_props = { + 'installed_by_rpm': False, + 'kernel': qubes.property.DEFAULT, + 'kernelopts': qubes.property.DEFAULT, + 'qrexec_timeout': qubes.property.DEFAULT, + 'netvm': qubes.property.DEFAULT, + 'default_user': qubes.property.DEFAULT, + 'internal': qubes.property.DEFAULT, + 'include_in_backups': True, + 'debug': False, + 'maxmem': 1535, + 'memory': 400, + } + self.assertRestored("test-template-clone", + klass=qubes.vm.templatevm.TemplateVM, + label='green', + provides_network=False, + **common_props) + testproxy_props = common_props.copy() + testproxy_props.update( + label='red', + provides_network=True, + memory=200, + template=self.app.default_template.name, + ) + self.assertRestored("test-testproxy", + klass=qubes.vm.appvm.AppVM, + **testproxy_props) + self.assertRestored("test-work", + klass=qubes.vm.appvm.AppVM, + template=self.app.default_template.name, + label='green', + **common_props) + self.assertRestored("test-standalonevm", + klass=qubes.vm.standalonevm.StandaloneVM, + label='blue', + **common_props) + self.assertRestored("test-custom-template-appvm", + klass=qubes.vm.appvm.AppVM, + template='test-template-clone', + label='yellow', + **common_props) def test_220_r2_encrypted(self): self.create_v3_backup(True) @@ -477,15 +598,48 @@ class TC_00_BackupCompatibility( 'use-default-template': True, 'use-default-netvm': True, }) - self.assertRestored("test-template-clone") - self.assertRestored("test-testproxy") - self.assertRestored("test-work") - self.assertRestored("test-testhvm") - self.assertRestored("test-standalonevm") - self.assertRestored("test-custom-template-appvm") - self.assertEqual(self.app.domains["test-custom-template-appvm"] - .template, - self.app.domains["test-template-clone"]) + common_props = { + 'installed_by_rpm': False, + 'kernel': qubes.property.DEFAULT, + 'kernelopts': qubes.property.DEFAULT, + 'qrexec_timeout': qubes.property.DEFAULT, + 'netvm': qubes.property.DEFAULT, + 'default_user': qubes.property.DEFAULT, + 'internal': qubes.property.DEFAULT, + 'include_in_backups': True, + 'debug': False, + 'maxmem': 1535, # 4063 caped by 10*400 + 'memory': 400, + } + self.assertRestored("test-template-clone", + klass=qubes.vm.templatevm.TemplateVM, + label='green', + provides_network=False, + **common_props) + testproxy_props = common_props.copy() + testproxy_props.update( + label='red', + provides_network=True, + memory=200, + template=self.app.default_template.name, + ) + self.assertRestored("test-testproxy", + klass=qubes.vm.appvm.AppVM, + **testproxy_props) + self.assertRestored("test-work", + klass=qubes.vm.appvm.AppVM, + template=self.app.default_template.name, + label='green', + **common_props) + self.assertRestored("test-standalonevm", + klass=qubes.vm.standalonevm.StandaloneVM, + label='blue', + **common_props) + self.assertRestored("test-custom-template-appvm", + klass=qubes.vm.appvm.AppVM, + template='test-template-clone', + label='yellow', + **common_props) class TC_01_BackupCompatibilityIntoLVM(TC_00_BackupCompatibility):