From b6c4f8456f5dfda2d03ac8c01c7bcc137e3b603c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marek=20Marczykowski-G=C3=B3recki?=
 <marmarek@invisiblethingslab.com>
Date: Sun, 9 Jun 2019 18:03:18 +0200
Subject: [PATCH] api/admin: make admin.vm.Console call go through qubesd

Ask qubesd for admin.vm.Console call. This allows to intercept it with
admin-permission event. While at it, extract tty path extraction to
python, where libvirt domain object is already available.

Fixes QubesOS/qubes-issues#5030
---
 qubes-rpc/admin.vm.Console | 20 ++++++++++++++----
 qubes/api/admin.py         | 18 ++++++++++++++++
 qubes/tests/api_admin.py   | 43 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 77 insertions(+), 4 deletions(-)

diff --git a/qubes-rpc/admin.vm.Console b/qubes-rpc/admin.vm.Console
index 6ce5bdf5..4dcb2c7d 100755
--- a/qubes-rpc/admin.vm.Console
+++ b/qubes-rpc/admin.vm.Console
@@ -1,11 +1,23 @@
 #!/bin/bash
 
-# TODO: handle 'admin-permission' event for qubesd
-
 lock="/var/run/qubes/$QREXEC_REQUESTED_TARGET.terminal.lock"
 
-qvm-check --quiet --running "$QREXEC_REQUESTED_TARGET" > /dev/null 2>&1 || { echo "Error: domain '$QREXEC_REQUESTED_TARGET' does not exist or is not running"; exit 1; }
+# use temporary file, because env variables deal poorly with \0 inside
+tmpfile=$(mktemp)
+trap "rm -f $tmpfile" EXIT
+qubesd-query -e \
+        "$QREXEC_REMOTE_DOMAIN" \
+        "admin.vm.Console" \
+        "$QREXEC_REQUESTED_TARGET" \
+        "$1" >$tmpfile
+
+# exit if qubesd returned an error (not '0\0')
+if [ "$(head -c 2 $tmpfile | xxd -p)" != "3000" ]; then
+    cat "$tmpfile"
+    exit 1
+fi
+path=$(tail -c +3 "$tmpfile")
 
 # Create an exclusive lock to ensure that multiple qubes cannot access to the same socket
 # In the case of multiple qrexec calls it returns a specific exit code
-sudo flock -n -E 200 -x "$lock" socat - OPEN:"$(virsh -c xen ttyconsole "$QREXEC_REQUESTED_TARGET")"
+sudo flock -n -E 200 -x "$lock" socat - OPEN:"$path"
diff --git a/qubes/api/admin.py b/qubes/api/admin.py
index 47c75c00..effe329b 100644
--- a/qubes/api/admin.py
+++ b/qubes/api/admin.py
@@ -29,6 +29,7 @@ import string
 import subprocess
 
 import libvirt
+import lxml.etree
 import pkg_resources
 import yaml
 
@@ -575,6 +576,23 @@ class QubesAdminAPI(qubes.api.AbstractQubesAPI):
             raise qubes.exc.QubesTagNotFoundError(self.dest, self.arg)
         self.app.save()
 
+    @qubes.api.method('admin.vm.Console', no_payload=True,
+        scope='local', write=True)
+    @asyncio.coroutine
+    def vm_console(self):
+        self.enforce(not self.arg)
+
+        self.fire_event_for_permission()
+
+        if not self.dest.is_running():
+            raise qubes.exc.QubesVMNotRunningError(self.dest)
+
+        xml_desc = lxml.etree.fromstring(self.dest.libvirt_domain.XMLDesc())
+        ttypath = xml_desc.xpath('string(/domain/devices/console/@tty)')
+        # this value is returned to /etc/qubes-rpc/admin.vm.Console script,
+        # which will call socat on it
+        return ttypath
+
     @qubes.api.method('admin.pool.List', no_payload=True,
         scope='global', read=True)
     @asyncio.coroutine
diff --git a/qubes/tests/api_admin.py b/qubes/tests/api_admin.py
index d984f8f1..8cd99969 100644
--- a/qubes/tests/api_admin.py
+++ b/qubes/tests/api_admin.py
@@ -2475,6 +2475,47 @@ class TC_00_VMs(AdminAPITestCase):
                 b'test-vm1', b'private', b'abc')
         self.assertFalse(self.app.save.called)
 
+    def test_690_vm_console(self):
+        self.vm._libvirt_domain = unittest.mock.Mock()
+        xml_desc = (
+            '<domain type=\'xen\' id=\'42\'>\n'
+            '<name>test-vm1</name>\n'
+            '<devices>\n'
+            '<console type=\'pty\' tty=\'/dev/pts/42\'>\n'
+            '<source path=\'/dev/pts/42\'/>\n'
+            '<target type=\'xen\' port=\'0\'/>\n'
+            '</console>\n'
+            '</devices>\n'
+            '</domain>\n'
+        )
+        self.vm._libvirt_domain.configure_mock(
+            **{'XMLDesc.return_value': xml_desc,
+               'isActive.return_value': True}
+        )
+        self.app.vmm.configure_mock(offline_mode=False)
+        value = self.call_mgmt_func(b'admin.vm.Console', b'test-vm1')
+        self.assertEqual(value, '/dev/pts/42')
+
+    def test_691_vm_console_not_running(self):
+        self.vm._libvirt_domain = unittest.mock.Mock()
+        xml_desc = (
+            '<domain type=\'xen\' id=\'42\'>\n'
+            '<name>test-vm1</name>\n'
+            '<devices>\n'
+            '<console type=\'pty\' tty=\'/dev/pts/42\'>\n'
+            '<source path=\'/dev/pts/42\'/>\n'
+            '<target type=\'xen\' port=\'0\'/>\n'
+            '</console>\n'
+            '</devices>\n'
+            '</domain>\n'
+        )
+        self.vm._libvirt_domain.configure_mock(
+            **{'XMLDesc.return_value': xml_desc,
+               'isActive.return_value': False}
+        )
+        with self.assertRaises(qubes.exc.QubesVMNotRunningError):
+            self.call_mgmt_func(b'admin.vm.Console', b'test-vm1')
+
     def test_990_vm_unexpected_payload(self):
         methods_with_no_payload = [
             b'admin.vm.List',
@@ -2505,6 +2546,7 @@ class TC_00_VMs(AdminAPITestCase):
             b'admin.vm.Pause',
             b'admin.vm.Unpause',
             b'admin.vm.Kill',
+            b'admin.vm.Console',
             b'admin.Events',
             b'admin.vm.feature.List',
             b'admin.vm.feature.Get',
@@ -2549,6 +2591,7 @@ class TC_00_VMs(AdminAPITestCase):
             b'admin.vm.Pause',
             b'admin.vm.Unpause',
             b'admin.vm.Kill',
+            b'admin.vm.Console',
             b'admin.Events',
             b'admin.vm.feature.List',
         ]