Skip to content

Commit

Permalink
backup: fix handling already encoded passphrase
Browse files Browse the repository at this point in the history
When passphrase is retrieved from VM, it is already encoded. Do not try
to encode it again.

QubesOS/qubes-issues#2931
  • Loading branch information
marmarek committed Jul 29, 2017
1 parent cdbe1d1 commit 1556814
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 9 deletions.
1 change: 0 additions & 1 deletion qubes/api/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1146,7 +1146,6 @@ def _load_backup_profile(self, profile_name, skip_passphrase=False):
except KeyError:
raise qubes.exc.QubesException(
'Invalid backup profile - invalid passphrase_vm')
# TODO .decode()?
passphrase, _ = yield from passphrase_vm.run_service_for_stdio(
'qubes.BackupPassphrase+' + self.arg)
else:
Expand Down
14 changes: 8 additions & 6 deletions qubes/backup.py
Original file line number Diff line number Diff line change
Expand Up @@ -200,6 +200,7 @@ def launch_scrypt(action, input_name, output_name, passphrase):
:param input_name: input path or '-' for stdin
:param output_name: output path or '-' for stdout
:param passphrase: passphrase
:type passphrase: bytes
:return: subprocess.Popen object
'''
command_line = ['scrypt', action, input_name, output_name]
Expand All @@ -217,7 +218,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.encode('utf-8') + b'\n')
pty.write(passphrase + b'\n')
pty.flush()
# save it here, so garbage collector would not close it (which would kill
# the child)
Expand Down Expand Up @@ -492,8 +493,8 @@ def _prepare_backup_header(self):
backup_header.save(header_file_path)
# Start encrypt, scrypt will also handle integrity
# protection
scrypt_passphrase = u'{filename}!{passphrase}'.format(
filename=HEADER_FILENAME, passphrase=self.passphrase)
scrypt_passphrase = '{filename}!'.format(
filename=HEADER_FILENAME).encode() + self.passphrase
scrypt = yield from launch_scrypt(
'enc', header_file_path, header_file_path + '.hmac',
scrypt_passphrase)
Expand Down Expand Up @@ -545,11 +546,10 @@ def _split_and_send(self, input_stream, file_basename,
# Start encrypt, scrypt will also handle integrity
# protection
scrypt_passphrase = \
u'{backup_id}!{filename}!{passphrase}'.format(
'{backup_id}!{filename}!'.format(
backup_id=self.backup_id,
filename=os.path.relpath(chunkfile[:-4],
self.tmpdir),
passphrase=self.passphrase)
self.tmpdir)).encode() + self.passphrase
try:
scrypt = yield from launch_scrypt(
"enc", "-", chunkfile, scrypt_passphrase)
Expand Down Expand Up @@ -691,6 +691,8 @@ def backup_do(self):
# pylint: disable=too-many-statements
if self.passphrase is None:
raise qubes.exc.QubesException("No passphrase set")
if not isinstance(self.passphrase, bytes):
self.passphrase = self.passphrase.encode('utf-8')
qubes_xml = self.app.store
self.tmpdir = tempfile.mkdtemp()
shutil.copy(qubes_xml, os.path.join(self.tmpdir, 'qubes.xml'))
Expand Down
4 changes: 2 additions & 2 deletions qubes/tests/api_admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1930,7 +1930,7 @@ def test_621_backup_execute_passphrase_service(self, mock_backup):

@asyncio.coroutine
def service_passphrase(*args, **kwargs):
return ('pass-from-vm', None)
return (b'pass-from-vm', None)

mock_backup.return_value.backup_do.side_effect = self.dummy_coro
self.vm.run_service_for_stdio = unittest.mock.Mock(
Expand All @@ -1950,7 +1950,7 @@ def service_passphrase(*args, **kwargs):
target_vm=self.vm,
target_dir='/home/user',
compressed=True,
passphrase='pass-from-vm')
passphrase=b'pass-from-vm')
mock_backup.return_value.backup_do.assert_called_once_with()
self.vm.run_service_for_stdio.assert_called_with(
'qubes.BackupPassphrase+testprofile')
Expand Down

0 comments on commit 1556814

Please sign in to comment.