From e9b97e42b1dffe7b1d7317d73fcfa912f724d016 Mon Sep 17 00:00:00 2001 From: Pawel Marczewski Date: Mon, 20 Jan 2020 18:14:05 +0100 Subject: [PATCH] import: check exact size of copied data The import will error out if there is not enough data, or too much data provided. --- qubes-rpc/admin.vm.volume.Import | 45 ++++++-- qubes/api/internal.py | 9 +- qubes/tests/__init__.py | 1 + qubes/tests/api_admin.py | 22 ++++ qubes/tests/rpc_import.py | 180 +++++++++++++++++++++++++++++++ rpm_spec/core-dom0.spec.in | 1 + 6 files changed, 247 insertions(+), 11 deletions(-) create mode 100644 qubes/tests/rpc_import.py diff --git a/qubes-rpc/admin.vm.volume.Import b/qubes-rpc/admin.vm.volume.Import index db57c9e1..88704596 100755 --- a/qubes-rpc/admin.vm.volume.Import +++ b/qubes-rpc/admin.vm.volume.Import @@ -1,4 +1,4 @@ -#!/bin/sh +#!/bin/bash # # This Admin API call is implemented as a custom script, instead of dumb # passthrough to qubesd because it may get huge amount of data (whole root.img @@ -32,39 +32,64 @@ set -e +# make dd output consistent +export LC_ALL=C + # use temporary file, because env variables deal poorly with \0 inside tmpfile=$(mktemp) -trap "rm -f $tmpfile" EXIT +trap 'rm -f $tmpfile' EXIT requested_size="" -if [[ ${0##*/} == admin.vm.volume.ImportWithSize ]]; then - read requested_size +if [[ "${0##*/}" == admin.vm.volume.ImportWithSize ]]; then + read -r requested_size fi echo -n "$requested_size" | qubesd-query -c /var/run/qubesd.internal.sock \ "$QREXEC_REMOTE_DOMAIN" \ "internal.vm.volume.ImportBegin" \ "$QREXEC_REQUESTED_TARGET" \ - "$1" >$tmpfile + "$1" >"$tmpfile" # exit if qubesd returned an error (not '0\0') -if [ "$(head -c 2 $tmpfile | xxd -p)" != "3000" ]; then +if [ "$(head -c 2 "$tmpfile" | xxd -p)" != "3000" ]; then cat "$tmpfile" exit 1 fi size=$(tail -c +3 "$tmpfile"|cut -d ' ' -f 1) path=$(tail -c +3 "$tmpfile"|cut -d ' ' -f 2) +error="" + # now process stdin into this path -if sudo dd bs=128K of="$path" count="$size" iflag=count_bytes,fullblock \ - conv=sparse,notrunc,nocreat,fdatasync status=none; then +if ! sudo dd bs=128K of="$path" count="$size" iflag=count_bytes,fullblock \ + conv=sparse,notrunc,nocreat,fdatasync 2>"$tmpfile"; then + error="error copying data" +fi + +# Examine dd's output and check if number of bytes copied matches +if [ -z "$error" ]; then + bytes_copied=$(tail -n1 "$tmpfile" | cut -d ' ' -f 1) + if [ "$bytes_copied" -ne "$size" ]; then + error="not enough data (copied $bytes_copied bytes, expected $size bytes)" + fi +fi + +# Check if there is nothing more to be read from stdin +if [ -z "$error" ]; then + if ! dd of="$tmpfile" bs=1 count=1 status=none || \ + [ "$(stat -c %s "$tmpfile")" -ne 0 ]; then + error="too much data (expected $size bytes)" + fi +fi + +if [ -z "$error" ]; then status="ok" else - status="fail" + status="fail\n$error" fi # send status notification to qubesd, and pass its response to the caller -echo -n "$status" | qubesd-query -c /var/run/qubesd.internal.sock \ +echo -ne "$status" | qubesd-query -c /var/run/qubesd.internal.sock \ "$QREXEC_REMOTE_DOMAIN" \ "internal.vm.volume.ImportEnd" \ "$QREXEC_REQUESTED_TARGET" \ diff --git a/qubes/api/internal.py b/qubes/api/internal.py index 297edfcb..c4fbb9bc 100644 --- a/qubes/api/internal.py +++ b/qubes/api/internal.py @@ -118,6 +118,8 @@ class QubesInternalAPI(qubes.api.AbstractQubesAPI): This is second half of admin.vm.volume.Import handling. It is called when actual import is finished. Response from this method is sent do the client (as a response for admin.vm.volume.Import call). + + The payload is either 'ok', or 'fail\n'. ''' self.enforce(self.arg in self.dest.volumes.keys()) success = untrusted_payload == b'ok' @@ -134,7 +136,12 @@ class QubesInternalAPI(qubes.api.AbstractQubesAPI): success=success) if not success: - raise qubes.exc.QubesException('Data import failed') + error = '' + parts = untrusted_payload.decode('ascii').split('\n', 1) + if len(parts) > 1: + error = parts[1] + raise qubes.exc.QubesException( + 'Data import failed: {}'.format(error)) @qubes.api.method('internal.SuspendPre', no_payload=True) @asyncio.coroutine diff --git a/qubes/tests/__init__.py b/qubes/tests/__init__.py index 06260466..2ed87dad 100644 --- a/qubes/tests/__init__.py +++ b/qubes/tests/__init__.py @@ -1403,6 +1403,7 @@ def load_tests(loader, tests, pattern): # pylint: disable=unused-argument 'qubes.tests.api_admin', 'qubes.tests.api_misc', 'qubes.tests.api_internal', + 'qubes.tests.rpc_import', ): tests.addTests(loader.loadTestsFromName(modname)) diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py index 185b487f..c9423fea 100644 --- a/qubes/tests/api_admin.py +++ b/qubes/tests/api_admin.py @@ -1796,6 +1796,28 @@ netvm default=True type=vm \n''' self.assertEventFired( self.emitter, 'admin-permission:admin.vm.volume.ImportWithSize') + def test_510_vm_volume_import_end_success(self): + import_data_end_mock, self.vm.storage.import_data_end = \ + self.coroutine_mock() + self.call_internal_mgmt_func( + b'internal.vm.volume.ImportEnd', b'test-vm1', b'private', + payload=b'ok') + self.assertEqual(import_data_end_mock.mock_calls, [ + unittest.mock.call('private', success=True) + ]) + + def test_510_vm_volume_import_end_failure(self): + import_data_end_mock, self.vm.storage.import_data_end = \ + self.coroutine_mock() + with self.assertRaisesRegexp( + qubes.exc.QubesException, 'error message'): + self.call_internal_mgmt_func( + b'internal.vm.volume.ImportEnd', b'test-vm1', b'private', + payload=b'fail\nerror message') + self.assertEqual(import_data_end_mock.mock_calls, [ + unittest.mock.call('private', success=False) + ]) + def setup_for_clone(self): self.pool = unittest.mock.MagicMock() self.app.pools['test'] = self.pool diff --git a/qubes/tests/rpc_import.py b/qubes/tests/rpc_import.py new file mode 100644 index 00000000..5d9f2666 --- /dev/null +++ b/qubes/tests/rpc_import.py @@ -0,0 +1,180 @@ + +# +# The Qubes OS Project, https://www.qubes-os.org/ +# +# Copyright (C) 2020 Paweł Marczewski +# +# This library is free software; you can redistribute it and/or +# modify it under the terms of the GNU Lesser General Public +# License as published by the Free Software Foundation; either +# version 2.1 of the License, or (at your option) any later version. +# +# This library is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +# Lesser General Public License for more details. +# +# You should have received a copy of the GNU Lesser General Public +# License along with this library; if not, see . +# + +import unittest +import tempfile +import shutil +import os +import subprocess + + +import qubes.tests + + +class TestRpcImport(qubes.tests.QubesTestCase): + ''' + Tests for qubes-rpc/admin.vm.volume.Import script. + + It is a shell script that calls internal API methods via qubesd-query. + These tests mock all the calls. + ''' + + + QUBESD_QUERY = '''\ +#!/bin/sh -e + +method=$4 +echo "$@" > command-$method +cat > payload-$method +cat response-$method +''' + + RPC_FILE_PATH = os.path.abspath(os.path.join( + os.path.dirname(__file__), + '../../qubes-rpc/admin.vm.volume.Import')) + + def setUp(self): + self.tmpdir = tempfile.mkdtemp() + self.addCleanup(shutil.rmtree, self.tmpdir) + with open(os.path.join(self.tmpdir, 'qubesd-query'), 'w') \ + as qubesd_query_f: + qubesd_query_f.write(self.QUBESD_QUERY) + os.chmod(os.path.join(self.tmpdir, 'qubesd-query'), 0o700) + + shutil.copy( + self.RPC_FILE_PATH, + os.path.join(self.tmpdir, 'admin.vm.volume.Import')) + shutil.copy( + self.RPC_FILE_PATH, + os.path.join(self.tmpdir, 'admin.vm.volume.ImportWithSize')) + + # pylint:disable=invalid-name + def mockMethod(self, method, response): + with open(os.path.join(self.tmpdir, 'response-' + method), 'wb') \ + as response_f: + response_f.write(response) + + # pylint:disable=invalid-name + def assertMethodCalled(self, method, arg, expected_payload=b''): + try: + with open(os.path.join(self.tmpdir, 'command-' + method), 'rb') \ + as command_f: + command = command_f.read() + with open(os.path.join(self.tmpdir, 'payload-' + method), 'rb') \ + as payload_f: + payload = payload_f.read() + except FileNotFoundError: + self.fail('{} was not called'.format(method)) + + self.assertListEqual(command.decode().split(), [ + '-c', '/var/run/qubesd.internal.sock', + 'remote', method, 'target', arg + ]) + self.assertEqual(payload, expected_payload) + + # pylint:disable=invalid-name + def assertFileData(self, path, expected_data): + with open(path, 'rb') as data_f: + data = data_f.read() + self.assertEquals(data, expected_data) + + def setup_import(self, size): + self.target = os.path.join(self.tmpdir, 'target') + os.mknod(self.target) + + self.mockMethod( + 'internal.vm.volume.ImportBegin', + '\x30\x00{} {}'.format(size, self.target).encode()) + + self.mockMethod( + 'internal.vm.volume.ImportEnd', + b'\x30\x00import-end') + + def run_rpc(self, command, arg, data): + with open(os.path.join(self.tmpdir, 'data'), 'w+b') as data_f: + data_f.write(data) + data_f.seek(0) + env = { + 'PATH': self.tmpdir + ':' + os.getenv('PATH'), + 'QREXEC_REMOTE_DOMAIN': 'remote', + 'QREXEC_REQUESTED_TARGET': 'target', + } + output = subprocess.check_output( + [command, arg], + env=env, + cwd=self.tmpdir, + stdin=data_f + ) + + self.assertEqual(output, b'\x30\x00import-end') + + def test_00_import(self): + data = b'abcd' * 1024 + size = len(data) + self.setup_import(size) + + self.run_rpc('admin.vm.volume.Import', 'volume', data) + + self.assertMethodCalled('internal.vm.volume.ImportBegin', 'volume') + self.assertMethodCalled('internal.vm.volume.ImportEnd', 'volume', + b'ok') + self.assertFileData(self.target, data) + + def test_01_import_with_size(self): + data = b'abcd' * 1024 + size = len(data) + self.setup_import(size) + + self.run_rpc('admin.vm.volume.ImportWithSize', 'volume', + str(size).encode() + b'\n' + data) + + self.assertMethodCalled('internal.vm.volume.ImportBegin', 'volume', + str(size).encode()) + self.assertMethodCalled('internal.vm.volume.ImportEnd', 'volume', + b'ok') + self.assertFileData(self.target, data) + + def test_02_import_not_enough_data(self): + data = b'abcd' * 1024 + size = len(data) + 1 + self.setup_import(size) + + self.run_rpc('admin.vm.volume.Import', 'volume', data) + + self.assertMethodCalled('internal.vm.volume.ImportBegin', 'volume') + self.assertMethodCalled( + 'internal.vm.volume.ImportEnd', 'volume', + b'fail\n' + + ('not enough data (copied {} bytes, expected {} bytes)' + .format(len(data), size).encode())) + + def test_03_import_too_much_data(self): + data = b'abcd' * 1024 + size = len(data) - 1 + self.setup_import(size) + + output = self.run_rpc('admin.vm.volume.Import', 'volume', data) + + self.assertMethodCalled('internal.vm.volume.ImportBegin', 'volume') + self.assertMethodCalled( + 'internal.vm.volume.ImportEnd', 'volume', + b'fail\n' + + ('too much data (expected {} bytes)' + .format(size).encode())) diff --git a/rpm_spec/core-dom0.spec.in b/rpm_spec/core-dom0.spec.in index 76ade37e..76339017 100644 --- a/rpm_spec/core-dom0.spec.in +++ b/rpm_spec/core-dom0.spec.in @@ -300,6 +300,7 @@ fi %{python3_sitelib}/qubes/tests/ext.py %{python3_sitelib}/qubes/tests/firewall.py %{python3_sitelib}/qubes/tests/init.py +%{python3_sitelib}/qubes/tests/rpc_import.py %{python3_sitelib}/qubes/tests/storage.py %{python3_sitelib}/qubes/tests/storage_file.py %{python3_sitelib}/qubes/tests/storage_reflink.py