From 8511b96a319836700b4829816cdae27c3630060d Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 9 Jun 2023 08:59:18 +0200
Subject: [PATCH] qemu_passt: Precreate passt logfile
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

There are a few situations where passt itself is unable to create
a file because it runs under QEMU user (e.g. just like our
example from formatdomain.rst suggests: /var/log/passt.log). If
libvirtd runs with sufficient permissions (e.g. as root) it can
create the file and set seclabels on it so that passt can then
open it.

Ideally, we would just pass pre-opened FD, but this wasn't viewed
as secure enough [1]. So lets just create the file and set
seclabels.

For the case when both libvirtd and passt have the same
permissions, well then we fail before even needing to fork() and
exec().

1: https://archives.passt.top/passt-dev/20230606225836.63aecebe@elisabeth/
Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=2209191
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_passt.c | 40 +++++++++++++++++++++++++++++++++++-----
 1 file changed, 35 insertions(+), 5 deletions(-)

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 99636a3a49..25b22d8ad9 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -20,6 +20,8 @@
 
 #include <config.h>
 
+#include <fcntl.h>
+
 #include "qemu_dbus.h"
 #include "qemu_extdevice.h"
 #include "qemu_security.h"
@@ -136,9 +138,13 @@ void
 qemuPasstStop(virDomainObj *vm,
               virDomainNetDef *net)
 {
+    qemuDomainObjPrivate *priv = vm->privateData;
+    virQEMUDriver *driver = priv->driver;
     g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
     g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
 
+    qemuSecurityDomainRestorePathLabel(driver, vm, net->backend.logFile);
+
     qemuPasstKill(pidfile, passtSocketName);
 }
 
@@ -166,10 +172,12 @@ qemuPasstStart(virDomainObj *vm,
 {
     qemuDomainObjPrivate *priv = vm->privateData;
     virQEMUDriver *driver = priv->driver;
+    g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
     g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
     g_autoptr(virCommand) cmd = NULL;
     g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
     char macaddr[VIR_MAC_STRING_BUFLEN];
+    bool needUnlink = false;
     size_t i;
 
     cmd = virCommandNew(PASST);
@@ -191,8 +199,25 @@ qemuPasstStart(virDomainObj *vm,
     if (net->sourceDev)
         virCommandAddArgList(cmd, "--interface", net->sourceDev, NULL);
 
-    if (net->backend.logFile)
+    if (net->backend.logFile) {
+        VIR_AUTOCLOSE logfd = -1;
+        /* The logFile location is not restricted to a per-domain directory. It
+         * can be anywhere. Pre-create it as passt may not have enough perms to
+         * do so. */
+        if (qemuDomainOpenFile(cfg, vm->def, net->backend.logFile,
+                               O_CREAT | O_TRUNC | O_APPEND | O_RDWR,
+                               &needUnlink) < 0) {
+            return -1;
+        }
+
+        if (qemuSecurityDomainSetPathLabel(driver, vm,
+                                           net->backend.logFile, false) < 0) {
+            goto error;
+        }
+
+        /* Worse, passt deliberately doesn't support FD passing. */
         virCommandAddArgList(cmd, "--log-file", net->backend.logFile, NULL);
+    }
 
     /* Add IP address info */
     for (i = 0; i < net->guestIP.nips; i++) {
@@ -203,7 +228,7 @@ qemuPasstStart(virDomainObj *vm,
          * a single IPv4 and single IPv6 address
          */
         if (!(addr = virSocketAddrFormat(&ip->address)))
-            return -1;
+            goto error;
 
         virCommandAddArgList(cmd, "--address", addr, NULL);
 
@@ -231,14 +256,14 @@ qemuPasstStart(virDomainObj *vm,
             /* validation guarantees this will never happen */
             virReportError(VIR_ERR_INTERNAL_ERROR,
                            _("Invalid portForward proto value %1$u"), pf->proto);
-            return -1;
+            goto error;
         }
 
         if (VIR_SOCKET_ADDR_VALID(&pf->address)) {
             g_autofree char *addr = NULL;
 
             if (!(addr = virSocketAddrFormat(&pf->address)))
-                return -1;
+                goto error;
 
             virBufferAddStr(&buf, addr);
             emitsep = true;
@@ -284,7 +309,7 @@ qemuPasstStart(virDomainObj *vm,
 
 
     if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
-        return -1;
+        goto error;
 
     if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, true, NULL) < 0)
         goto error;
@@ -292,6 +317,11 @@ qemuPasstStart(virDomainObj *vm,
     return 0;
 
  error:
+    if (needUnlink && unlink(net->backend.logFile) < 0) {
+        VIR_WARN("Unable to unlink '%s': %s",
+                 net->backend.logFile, g_strerror(errno));
+    }
+
     qemuPasstKill(pidfile, passtSocketName);
     return -1;
 }