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

@@ -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" \

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<error message>'.
         '''
         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

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))
 

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

qubes/tests/rpc_import.py

@@ -0,0 +1,180 @@
+
+#
+# The Qubes OS Project, https://www.qubes-os.org/
+#
+# Copyright (C) 2020 Paweł Marczewski <pawel@invisiblethingslab.com>
+#
+# 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 <https://www.gnu.org/licenses/>.
+#
+
+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()))

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