Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 17:38:49 +0100
Michal Prívozník  wrote:

> On 2/22/23 16:51, Stefano Brivio wrote:
> > On Wed, 22 Feb 2023 14:30:21 +
> > Daniel P. Berrangé  wrote:
> >   
> >> On Wed, Feb 22, 2023 at 02:21:29PM +0100, Stefano Brivio wrote:  
> >>> qemuSecurityCommandRun() causes an explicit domain transition of the
> >>> new process, but passt ships with its own SELinux policy, with
> >>> external interfaces for libvirtd, so we simply need to transition
> >>> from virtd_t to passt_t as passt is executed. The qemu type
> >>> enforcement rules have little to do with it.
> >>>
> >>> That is, if we switch to svirt_t, passt will run in the security
> >>> context that's intended for QEMU, which allows a number of
> >>> operations not needed by passt. On the other hand, with a switch
> >>> to svirt_t, passt won't be able to create its own PID file.
> >>>
> >>> Usage of those new interfaces is implemented by this change in
> >>> selinux-policy:
> >>>   https://github.com/fedora-selinux/selinux-policy/pull/1613
> >>>
> >>> Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
> >>> set the label, preserving the correct MCS range for the given VM
> >>> instance. This is a temporary measure: eventually, we'll need a more
> >>> generic and elegant mechanism for helper binaries.
> >>
> >> I'd really prefer to see the security manager used from the
> >> start, rather than committing code with a TODO that should
> >> be practical to implement straight away.  
> > 
> > Feel free, by all means.
> > 
> > As to myself, I can't realistically implement missing parts of the
> > security manager code that I'm entirely unfamiliar with, and seriously
> > test the whole thing before the 9.1.0 release freeze, which happens to
> > be... today.
> > 
> > So I see four alternatives: 1. accept this fix, 2. somebody else fixes
> > this "properly", 3. drop the passt networking back-end, 4. ship a
> > completely broken feature.  
> 
> That ship already sailed. Passt was merged in libvirt-9.0.0.

...and isn't that a good enough reason to have it actually working in
9.1.0? I really fail to understand the reasoning here. :( Allow me to
quote from yourself:

On Thu, 16 Feb 2023 17:27:11 +0100
Michal Prívozník  wrote:

> And as we are getting close to the release I'd like to make this work
> with what we have now.

and I think this makes *a lot of sense*.

-- 
Stefano



Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Michal Prívozník
On 2/22/23 16:51, Stefano Brivio wrote:
> On Wed, 22 Feb 2023 14:30:21 +
> Daniel P. Berrangé  wrote:
> 
>> On Wed, Feb 22, 2023 at 02:21:29PM +0100, Stefano Brivio wrote:
>>> qemuSecurityCommandRun() causes an explicit domain transition of the
>>> new process, but passt ships with its own SELinux policy, with
>>> external interfaces for libvirtd, so we simply need to transition
>>> from virtd_t to passt_t as passt is executed. The qemu type
>>> enforcement rules have little to do with it.
>>>
>>> That is, if we switch to svirt_t, passt will run in the security
>>> context that's intended for QEMU, which allows a number of
>>> operations not needed by passt. On the other hand, with a switch
>>> to svirt_t, passt won't be able to create its own PID file.
>>>
>>> Usage of those new interfaces is implemented by this change in
>>> selinux-policy:
>>>   https://github.com/fedora-selinux/selinux-policy/pull/1613
>>>
>>> Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
>>> set the label, preserving the correct MCS range for the given VM
>>> instance. This is a temporary measure: eventually, we'll need a more
>>> generic and elegant mechanism for helper binaries.  
>>
>> I'd really prefer to see the security manager used from the
>> start, rather than committing code with a TODO that should
>> be practical to implement straight away.
> 
> Feel free, by all means.
> 
> As to myself, I can't realistically implement missing parts of the
> security manager code that I'm entirely unfamiliar with, and seriously
> test the whole thing before the 9.1.0 release freeze, which happens to
> be... today.
> 
> So I see four alternatives: 1. accept this fix, 2. somebody else fixes
> this "properly", 3. drop the passt networking back-end, 4. ship a
> completely broken feature.

That ship already sailed. Passt was merged in libvirt-9.0.0. We should
rather fix this properly, if we can.

Michal



Re: Entering freeze for libvirt-9.1.0

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 15:23:04 +0100
Jiri Denemark  wrote:

> I have just tagged v9.1.0-rc1 in the repository and pushed signed
> tarballs and source RPMs to https://libvirt.org/sources/
> 
> Please give the release candidate some testing and in case you find a
> serious issue which should have a fix in the upcoming release, feel
> free to reply to this thread to make sure the issue is more visible.

The "passt" network back-end is entirely non-functional on distributions
shipping with SELinux: the binary helper can't be executed. The
'virsh start' command reports:

  error: internal error: Could not start 'passt': libvirt:  error : cannot 
execute binary /usr/bin/passt: Permission denied

and the guest doesn't start. This is on Fedora 37, but it should be
universally reproducible.

I provided more details on the thread at:
  https://listman.redhat.com/archives/libvir-list/2023-February/238096.html

This is the relevant snippet from my domain XML file:


  
  
  
  


-- 
Stefano



Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 14:30:21 +
Daniel P. Berrangé  wrote:

> On Wed, Feb 22, 2023 at 02:21:29PM +0100, Stefano Brivio wrote:
> > qemuSecurityCommandRun() causes an explicit domain transition of the
> > new process, but passt ships with its own SELinux policy, with
> > external interfaces for libvirtd, so we simply need to transition
> > from virtd_t to passt_t as passt is executed. The qemu type
> > enforcement rules have little to do with it.
> > 
> > That is, if we switch to svirt_t, passt will run in the security
> > context that's intended for QEMU, which allows a number of
> > operations not needed by passt. On the other hand, with a switch
> > to svirt_t, passt won't be able to create its own PID file.
> > 
> > Usage of those new interfaces is implemented by this change in
> > selinux-policy:
> >   https://github.com/fedora-selinux/selinux-policy/pull/1613
> > 
> > Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
> > set the label, preserving the correct MCS range for the given VM
> > instance. This is a temporary measure: eventually, we'll need a more
> > generic and elegant mechanism for helper binaries.  
> 
> I'd really prefer to see the security manager used from the
> start, rather than committing code with a TODO that should
> be practical to implement straight away.

Feel free, by all means.

As to myself, I can't realistically implement missing parts of the
security manager code that I'm entirely unfamiliar with, and seriously
test the whole thing before the 9.1.0 release freeze, which happens to
be... today.

So I see four alternatives: 1. accept this fix, 2. somebody else fixes
this "properly", 3. drop the passt networking back-end, 4. ship a
completely broken feature.

-- 
Stefano



Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Laine Stump

On 2/22/23 9:30 AM, Daniel P. Berrangé wrote:

On Wed, Feb 22, 2023 at 02:21:29PM +0100, Stefano Brivio wrote:

qemuSecurityCommandRun() causes an explicit domain transition of the
new process, but passt ships with its own SELinux policy, with
external interfaces for libvirtd, so we simply need to transition
from virtd_t to passt_t as passt is executed. The qemu type
enforcement rules have little to do with it.

That is, if we switch to svirt_t, passt will run in the security
context that's intended for QEMU, which allows a number of
operations not needed by passt. On the other hand, with a switch
to svirt_t, passt won't be able to create its own PID file.

Usage of those new interfaces is implemented by this change in
selinux-policy:
   https://github.com/fedora-selinux/selinux-policy/pull/1613

Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
set the label, preserving the correct MCS range for the given VM
instance. This is a temporary measure: eventually, we'll need a more
generic and elegant mechanism for helper binaries.


I'd really prefer to see the security manager used from the
start, rather than committing code with a TODO that should
be practical to implement straight away.


As the person who started all this by naively believing that we would 
just need to "add something to the selinux-policy package" to make 
selinux+libvirt+passt work, and now that Stefano has done the heavy 
lifting to figure out something that actually *does* work, I will say 
that I'll help in whatever way I can to make that happen in as short a 
time as possible.




Re: [PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Daniel P . Berrangé
On Wed, Feb 22, 2023 at 02:21:29PM +0100, Stefano Brivio wrote:
> qemuSecurityCommandRun() causes an explicit domain transition of the
> new process, but passt ships with its own SELinux policy, with
> external interfaces for libvirtd, so we simply need to transition
> from virtd_t to passt_t as passt is executed. The qemu type
> enforcement rules have little to do with it.
> 
> That is, if we switch to svirt_t, passt will run in the security
> context that's intended for QEMU, which allows a number of
> operations not needed by passt. On the other hand, with a switch
> to svirt_t, passt won't be able to create its own PID file.
> 
> Usage of those new interfaces is implemented by this change in
> selinux-policy:
>   https://github.com/fedora-selinux/selinux-policy/pull/1613
> 
> Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
> set the label, preserving the correct MCS range for the given VM
> instance. This is a temporary measure: eventually, we'll need a more
> generic and elegant mechanism for helper binaries.

I'd really prefer to see the security manager used from the
start, rather than committing code with a TODO that should
be practical to implement straight away.


With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-22 Thread Alex Bennée


Reinoud Zandijk  writes:

> On Wed, Feb 22, 2023 at 09:51:57AM +, Daniel P. Berrangé wrote:
>> On Wed, Feb 22, 2023 at 09:11:13AM +, Bernhard Beschow wrote:
>> > Are there any plans or ideas to support 128 bit architectures
>> > such as CHERI in the future? There is already a QEMU fork
>> > implementing CHERI for RISC V [1]. Also ARM has developed an
>> > experimental hardware implementation of CHERI within the Morello
>> > project where Linaro is involved as well, although the QEMU
>> > implementation is performed by the University of Cambridge [2].
>> 
>> If 128 bit hardware exists and has real world non-toy usage,
>> then a request to support it in QEMU is essentially inevitable.
>> 
>> > I'm asking because once we deeply bake in the assumption that
>> > host size >= guest size then adding such architectures will
>> > become much harder.
>> 
>> Yep, that is a risk.
>
> I can second that. Better keep it in the code and deal with it. Maybe those
> specific parts can be implemented in such a way that it can easily become a
> noop on host size >= guest size.

AIUI these don't expose bigger addresses or natural atomic sizes which
is where things get tricky for the TCG. All the extra bits are used for
authentication or permission checks.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro



Entering freeze for libvirt-9.1.0

2023-02-22 Thread Jiri Denemark
I have just tagged v9.1.0-rc1 in the repository and pushed signed
tarballs and source RPMs to https://libvirt.org/sources/

Please give the release candidate some testing and in case you find a
serious issue which should have a fix in the upcoming release, feel
free to reply to this thread to make sure the issue is more visible.

If you have not done so yet, please update NEWS.rst to document any
significant change you made since the last release.

Thanks,

Jirka



Re: [PATCH 3/3] qemu_passt: Remove passt socket file on exit

2023-02-22 Thread Laine Stump

On 2/21/23 2:19 PM, Stefano Brivio wrote:

Just like it can't remove its own PID files, passt can't unlink its
own socket upon exit (unless the initialisation fails), because it
has no access to the filesystem at runtime.

Remove the socket file in qemuPasstKill().

Fixes: a56f0168d576 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio 


Reviewed-by: Laine Stump 

(it's independent of the rest of the series, so I pushed it)

---
  src/qemu/qemu_passt.c | 9 ++---
  1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index c7012e349a..0e028ca752 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -103,7 +103,7 @@ qemuPasstAddNetProps(virDomainObj *vm,
  
  
  static void

-qemuPasstKill(const char *pidfile)
+qemuPasstKill(const char *pidfile, const char *passtSocketName)
  {
  virErrorPtr orig_err;
  pid_t pid = 0;
@@ -115,6 +115,8 @@ qemuPasstKill(const char *pidfile)
  virProcessKillPainfully(pid, true);
  unlink(pidfile);
  
+unlink(passtSocketName);

+
  virErrorRestore(&orig_err);
  }
  
@@ -124,8 +126,9 @@ qemuPasstStop(virDomainObj *vm,

virDomainNetDef *net)
  {
  g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
+g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
  
-qemuPasstKill(pidfile);

+qemuPasstKill(pidfile, passtSocketName);
  }
  
  
@@ -284,6 +287,6 @@ qemuPasstStart(virDomainObj *vm,

  return 0;
  
   error:

-qemuPasstKill(pidfile);
+qemuPasstKill(pidfile, passtSocketName);
  return -1;
  }




[PATCH v2 0/3] qemu_passt: Fixes for passt lifecycle handling

2023-02-22 Thread Stefano Brivio
This series implements fixes in the handling of passt's lifecycle.

v2: In 1/3, preserve the VM-specific MCS range by explicitly setting a
label, as suggested by Daniel, with a temporary workaround sketched
by Michal.

Stefano Brivio (3):
  qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on
start
  qemu_passt: Set UID and GID to configured values for qemu driver, if
any
  qemu_passt: Remove passt socket file on exit

 src/qemu/qemu_passt.c | 46 +++
 1 file changed, 38 insertions(+), 8 deletions(-)

-- 
2.39.1



Re: [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event

2023-02-22 Thread Laine Stump

On 2/22/23 5:21 AM, Michal Prívozník wrote:

On 2/22/23 01:35, Laine Stump wrote:

When a QEMU netdev is of type "stream", if the socket it uses for
connectivity to the host network gets closed, then QEMU will send a
NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
created is backed by a passt process, and if the socket was closed,
that means the passt process has disappeared.

When we receive this event, we can respond by starting a new passt
process with the same options (including socket path) we originally
used. If we have previously created the stream netdev device with a
"reconnect" option, then QEMU will automatically reconnect to this new
passt process. (If we hadn't used "reconnect", then QEMU will never
try to reconnect to the new passt process, so there's no point in
starting it.)

Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
(ie "host side") of the network device, and so it sends the
"netdev-id" to specify which device was disconnected. But libvirt's
virDomainNetDef (the object used to keep track of network devices) is
the internal representation of both the host-side "netdev", and the
guest side device, and virDomainNetDef doesn't directly keep track of
the netdev-id, only of the device's "alias" (which is the "id"
parameter of the *guest* side of the device). Fortunately, by convention
libvirt always names the host-side of devices as "host" + alias, so in
order to search for the affected NetDef, all we need to do is trim the
1st 4 characters from the netdev-id and look for the NetDef having
that resulting trimmed string as its alias. (Contrast this to
NIC_RX_FILTER_CHANGED, which is an event received for the guest side
of the device, and so directly contains the device alias.)

Resolves: https://bugzilla.redhat.com/2172098
Signed-off-by: Laine Stump 
---
  src/qemu/qemu_domain.c   |  1 +
  src/qemu/qemu_domain.h   |  1 +
  src/qemu/qemu_driver.c   | 82 
  src/qemu/qemu_monitor.c  | 11 +
  src/qemu/qemu_monitor.h  |  6 +++
  src/qemu/qemu_monitor_json.c | 16 +++
  src/qemu/qemu_process.c  | 18 
  7 files changed, 135 insertions(+)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index e9bc0f375d..4cf9a259ea 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -11238,6 +11238,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
  break;
  case QEMU_PROCESS_EVENT_WATCHDOG:
  case QEMU_PROCESS_EVENT_DEVICE_DELETED:
+case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
  case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
  case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
  case QEMU_PROCESS_EVENT_MONITOR_EOF:
diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index 1053d1d4cb..6adc067681 100644
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -447,6 +447,7 @@ typedef enum {
  QEMU_PROCESS_EVENT_WATCHDOG = 0,
  QEMU_PROCESS_EVENT_GUESTPANIC,
  QEMU_PROCESS_EVENT_DEVICE_DELETED,
+QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
  QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
  QEMU_PROCESS_EVENT_SERIAL_CHANGED,
  QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE,
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 6154fe9bfe..47d6a0dd95 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -40,6 +40,7 @@
  #include "qemu_hostdev.h"
  #include "qemu_hotplug.h"
  #include "qemu_monitor.h"
+#include "qemu_passt.h"
  #include "qemu_process.h"
  #include "qemu_migration.h"
  #include "qemu_migration_params.h"
@@ -3622,6 +3623,84 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
  }
  
  
+static void

+processNetdevStreamDisconnectedEvent(virDomainObj *vm,
+ const char *netdevId)
+{
+virDomainDeviceDef dev;
+virDomainNetDef *def;
+qemuDomainObjPrivate *priv;
+virQEMUCaps *qemuCaps;
+const char *devAlias = NULL;
+
+/* The event sends us the "netdev-id", but we don't store the
+ * netdev-id in the NetDef and thus can't use it to find the
+ * correct NetDef. We *do* keep the device alias in the NetDef.
+ * By definition, the netdev-id is "host" + devAlias, so we just
+ * need to remove "host" from the front of netdev-id to get
+ * something we can use to find the proper NetDef.
+ */
+if (STREQLEN(netdevId, "host", 4))
+devAlias = &netdevId[4];


This is open coding STRSKIP():

   devAlias = STRSKIP(netdevId, "host");


Ah yes, right you are! Changed.




+
+if (!devAlias) {
+VIR_WARN("Received NETDEV_STREAM_DISCONNECTED event for unrecognized netdev 
%s from domain %p %s",
+  netdevId, vm, vm->def->name);
+return;
+}
+
+VIR_DEBUG("Received NETDEV_STREAM_DISCONNECTED event for device %s from domain 
%p %s",
+  devAlias, vm, vm->def->name);
+
+if (virDomainObjBeginJob(vm, VIR_JOB_QUERY) < 0)
+return;
+
+if (!virD

[PATCH v2 2/3] qemu_passt: Set UID and GID to configured values for qemu driver, if any

2023-02-22 Thread Stefano Brivio
qemuSecurityCommandRun() would have dealt with this (if UID and GID
had been passed). With virCommandRun() we need separate, explicit
calls.

Fixes: a56f0168d576 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio 
---
 src/qemu/qemu_passt.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 81f48dd630..61e7047354 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -157,6 +157,7 @@ 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);
@@ -174,6 +175,11 @@ qemuPasstStart(virDomainObj *vm,
 virCommandClearCaps(cmd);
 virCommandSetErrorBuffer(cmd, &errbuf);
 
+if (cfg->user != (uid_t) -1)
+virCommandSetUID(cmd, cfg->user);
+if (cfg->group != (gid_t) -1)
+virCommandSetGID(cmd, cfg->group);
+
 virCommandAddArgList(cmd,
  "--one-off",
  "--socket", passtSocketName,
-- 
2.39.1



[PATCH v2 3/3] qemu_passt: Remove passt socket file on exit

2023-02-22 Thread Stefano Brivio
Just like it can't remove its own PID files, passt can't unlink its
own socket upon exit (unless the initialisation fails), because it
has no access to the filesystem at runtime.

Remove the socket file in qemuPasstKill().

Fixes: a56f0168d576 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio 
---
 src/qemu/qemu_passt.c | 7 +--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 61e7047354..d5df3bb3f7 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -108,7 +108,7 @@ qemuPasstAddNetProps(virDomainObj *vm,
 
 
 static void
-qemuPasstKill(const char *pidfile)
+qemuPasstKill(const char *pidfile, const char *passtSocketName)
 {
 virErrorPtr orig_err;
 pid_t pid = 0;
@@ -120,6 +120,8 @@ qemuPasstKill(const char *pidfile)
 virProcessKillPainfully(pid, true);
 unlink(pidfile);
 
+unlink(passtSocketName);
+
 virErrorRestore(&orig_err);
 }
 
@@ -129,8 +131,9 @@ qemuPasstStop(virDomainObj *vm,
   virDomainNetDef *net)
 {
 g_autofree char *pidfile = qemuPasstCreatePidFilename(vm, net);
+g_autofree char *passtSocketName = qemuPasstCreateSocketPath(vm, net);
 
-qemuPasstKill(pidfile);
+qemuPasstKill(pidfile, passtSocketName);
 }
 
 
-- 
2.39.1



[PATCH v2 1/3] qemu_passt: Don't make passt transition to svirt_t/libvirt_domain on start

2023-02-22 Thread Stefano Brivio
qemuSecurityCommandRun() causes an explicit domain transition of the
new process, but passt ships with its own SELinux policy, with
external interfaces for libvirtd, so we simply need to transition
from virtd_t to passt_t as passt is executed. The qemu type
enforcement rules have little to do with it.

That is, if we switch to svirt_t, passt will run in the security
context that's intended for QEMU, which allows a number of
operations not needed by passt. On the other hand, with a switch
to svirt_t, passt won't be able to create its own PID file.

Usage of those new interfaces is implemented by this change in
selinux-policy:
  https://github.com/fedora-selinux/selinux-policy/pull/1613

Replace qemuSecurityCommandRun() with virCommandRun(), and explicitly
set the label, preserving the correct MCS range for the given VM
instance. This is a temporary measure: eventually, we'll need a more
generic and elegant mechanism for helper binaries.

Fixes: a56f0168d576 ("qemu: hook up passt config to qemu domains")
Signed-off-by: Stefano Brivio 
---
 src/qemu/qemu_passt.c | 33 +++--
 1 file changed, 27 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_passt.c b/src/qemu/qemu_passt.c
index 1217a6a087..81f48dd630 100644
--- a/src/qemu/qemu_passt.c
+++ b/src/qemu/qemu_passt.c
@@ -30,6 +30,11 @@
 #include "virlog.h"
 #include "virpidfile.h"
 
+#ifdef WITH_SELINUX
+# include 
+# include 
+#endif
+
 #define VIR_FROM_THIS VIR_FROM_NONE
 
 VIR_LOG_INIT("qemu.passt");
@@ -158,8 +163,11 @@ qemuPasstStart(virDomainObj *vm,
 g_autofree char *errbuf = NULL;
 char macaddr[VIR_MAC_STRING_BUFLEN];
 size_t i;
-int exitstatus = 0;
-int cmdret = 0;
+#ifdef WITH_SELINUX
+virSecurityLabelDef *seclabel;
+context_t s;
+const char *newLabel;
+#endif
 
 cmd = virCommandNew(PASST);
 
@@ -271,18 +279,31 @@ qemuPasstStart(virDomainObj *vm,
 if (qemuExtDeviceLogCommand(driver, vm, cmd, "passt") < 0)
 return -1;
 
-if (qemuSecurityCommandRun(driver, vm, cmd, -1, -1, &exitstatus, &cmdret) 
< 0)
-goto error;
+#ifdef WITH_SELINUX
+/* TODO: Implement a new security manager function for helper binaries,
+ * possibly deriving domain names from security attributes of the helper
+ * binary itself.
+ */
+seclabel = virDomainDefGetSecurityLabelDef(vm->def, "selinux");
+s = context_new(seclabel->label);
+context_type_set(s, "passt_t");
+newLabel = context_str(s);
+
+virCommandSetSELinuxLabel(cmd, newLabel);
+#endif
 
-if (cmdret < 0 || exitstatus != 0) {
+if (virCommandRun(cmd, NULL)) {
 virReportError(VIR_ERR_INTERNAL_ERROR,
_("Could not start 'passt': %s"), NULLSTR(errbuf));
 goto error;
 }
 
+context_free(s);
+
 return 0;
 
  error:
-qemuPasstKill(pidfile);
+context_free(s);
+qemuPasstKill(pidfile, passtSocketName);
 return -1;
 }
-- 
2.39.1



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-22 Thread Reinoud Zandijk
On Wed, Feb 22, 2023 at 09:51:57AM +, Daniel P. Berrangé wrote:
> On Wed, Feb 22, 2023 at 09:11:13AM +, Bernhard Beschow wrote:
> > Are there any plans or ideas to support 128 bit architectures
> > such as CHERI in the future? There is already a QEMU fork
> > implementing CHERI for RISC V [1]. Also ARM has developed an
> > experimental hardware implementation of CHERI within the Morello
> > project where Linaro is involved as well, although the QEMU
> > implementation is performed by the University of Cambridge [2].
> 
> If 128 bit hardware exists and has real world non-toy usage,
> then a request to support it in QEMU is essentially inevitable.
> 
> > I'm asking because once we deeply bake in the assumption that
> > host size >= guest size then adding such architectures will
> > become much harder.
> 
> Yep, that is a risk.

I can second that. Better keep it in the code and deal with it. Maybe those
specific parts can be implemented in such a way that it can easily become a
noop on host size >= guest size.

With regards,
Reinoud



Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Daniel P . Berrangé
On Wed, Feb 22, 2023 at 12:38:23PM +0100, Stefano Brivio wrote:
> On Wed, 22 Feb 2023 11:35:16 +
> Daniel P. Berrangé  wrote:
> 
> > On Wed, Feb 22, 2023 at 12:21:09PM +0100, Michal Prívozník wrote:
> > > On 2/22/23 11:05, Stefano Brivio wrote:  
> > > > On Wed, 22 Feb 2023 09:46:42 +
> > > > Daniel P. Berrangé  wrote:
> > > >   
> > > >> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:  
> > > >>> On Tue, 21 Feb 2023 19:43:33 +
> > > >>> Daniel P. Berrangé  wrote:
> > > >>> 
> > >  On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:
> > > > qemuSecurityCommandRun() causes an explicit domain transition of the
> > > > new process, but passt ships with its own SELinux policy, with
> > > > external interfaces for libvirtd, so we simply need to transition
> > > > from virtd_t to passt_t as passt is executed. The qemu type
> > > > enforcement rules have little to do with it.  
> > > 
> > >  Can you clarify the difference here ?
> > > >>>
> > > >>> Between...?
> > > >>>
> > > >>> I mean, virCommandRun() will just keep things running under virtd_t, 
> > > >>> so
> > > >>> that passt later can transition to passt_t.
> > > >>>
> > > >>> With qemuSecurityCommandRun(), there would be a transition from 
> > > >>> virtd_t
> > > >>> to svirt_t (it's the function that's called to start qemu, but
> > > >>> shouldn't be called here), and not to passt_t.
> > > >>>
> > > >>> But I'm not really sure that's what you were asking for.
> > > >>
> > > >> Yes, it is.
> > > >>  
> > > >>> 
> > >  Runing passt under 'svirt_t' is not desirable as that allows
> > >  many actions that are only relevant to QEMU.
> > > >>>
> > > >>> Right, that's what this patch avoids. There are also actions, such as
> > > >>> starting passt or killing it, that we don't want to allow QEMU to do.
> > > >>> 
> > >  Running passt under the MCS label that is associated with the
> > >  VM is highly desirable though. Two passt instances belonging
> > >  to separate VMs are isolated from each other if they each use
> > >  the VM specific MCS label, than if they use the global default
> > >  MCS label.
> > > 
> > >  To use the VM specific MCS label would require libvirt to
> > >  explicitly set the desired selinux label on exec, it can't
> > >  happen automatically via an SELinux transition rule.
> > > 
> > >  We do stil want to use the passt_t type though.
> > > 
> > >  IOW, if we have a VM running
> > > 
> > >    svirt_t:s0:c710,c716
> > > 
> > >  IMHO we would its corresponding passt instance to be
> > >  running
> > > 
> > >    passt_t:s0:c710,c716
> > > 
> > > 
> > >  not
> > > 
> > >    passt_t:s0:c0.c1023
> > > >>>
> > > >>> Practically speaking, it doesn't make a huge difference for passt
> > > >>> because it unshares any relevant namespace right after it starts --
> > > >>> that's *in theory* a strictly stronger isolation compared to what
> > > >>> SELinux provides (at least once we reach the main loop).
> > > >>
> > > >> Even docker/podman will apply SELinux isolation per container,
> > > >> rather than only relying on namespaces.   
> > > > 
> > > > Sure, I'm not saying it's not desirable -- but still, many (most?)
> > > > host-facing services they rely on are not isolated in this sense. Same
> > > > for the current implementation of libvirt with dnsmasq, for example.
> > > >   
> > > >>> But it makes sense, and I guess we should relabel to a specific MCS
> > > >>> with still 'virtd_t' as a type, then later the domain would transition
> > > >>> to passt_t. This could probably be done as an extension of
> > > >>> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> > > >>>
> > > >>> Anyway, right now, I think this provides better security than
> > > >>> 'setenforce 0', which is the only way to run passt from libvirt at the
> > > >>> moment on some distributions.
> > > >>
> > > >> If running with SELinux permissive, this patch has no effect, as it
> > > >> is unconfined regardless. That's not a situation we care about. If
> > > >> people want to run without protection they get to keep the pieces
> > > >> when it all goes wrong.  
> > > > 
> > > > The current implementation simply does not and cannot work with SELinux
> > > > in enforcing mode, so users have no other choice.
> > > >   
> > > >>> I'm not sure how you handle these cases on libvirt, but generally
> > > >>> speaking, this patch is a vast improvement on the current situation,
> > > >>> and I can follow up with an extension or a different version of
> > > >>> qemuSecurityCommandRun() later.
> > > >>
> > > >> No, I don't think it is a vast improvement.
> > > >>
> > > >> The goal of sVirt is to guarantee isolation between VMs.
> > > >>
> > > >> Running passt under svirt_t:MCS is not ideal because the svirt_t
> > > >> policy allows things that passt should not 

Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 11:35:16 +
Daniel P. Berrangé  wrote:

> On Wed, Feb 22, 2023 at 12:21:09PM +0100, Michal Prívozník wrote:
> > On 2/22/23 11:05, Stefano Brivio wrote:  
> > > On Wed, 22 Feb 2023 09:46:42 +
> > > Daniel P. Berrangé  wrote:
> > >   
> > >> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:  
> > >>> On Tue, 21 Feb 2023 19:43:33 +
> > >>> Daniel P. Berrangé  wrote:
> > >>> 
> >  On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:
> > > qemuSecurityCommandRun() causes an explicit domain transition of the
> > > new process, but passt ships with its own SELinux policy, with
> > > external interfaces for libvirtd, so we simply need to transition
> > > from virtd_t to passt_t as passt is executed. The qemu type
> > > enforcement rules have little to do with it.  
> > 
> >  Can you clarify the difference here ?
> > >>>
> > >>> Between...?
> > >>>
> > >>> I mean, virCommandRun() will just keep things running under virtd_t, so
> > >>> that passt later can transition to passt_t.
> > >>>
> > >>> With qemuSecurityCommandRun(), there would be a transition from virtd_t
> > >>> to svirt_t (it's the function that's called to start qemu, but
> > >>> shouldn't be called here), and not to passt_t.
> > >>>
> > >>> But I'm not really sure that's what you were asking for.
> > >>
> > >> Yes, it is.
> > >>  
> > >>> 
> >  Runing passt under 'svirt_t' is not desirable as that allows
> >  many actions that are only relevant to QEMU.
> > >>>
> > >>> Right, that's what this patch avoids. There are also actions, such as
> > >>> starting passt or killing it, that we don't want to allow QEMU to do.
> > >>> 
> >  Running passt under the MCS label that is associated with the
> >  VM is highly desirable though. Two passt instances belonging
> >  to separate VMs are isolated from each other if they each use
> >  the VM specific MCS label, than if they use the global default
> >  MCS label.
> > 
> >  To use the VM specific MCS label would require libvirt to
> >  explicitly set the desired selinux label on exec, it can't
> >  happen automatically via an SELinux transition rule.
> > 
> >  We do stil want to use the passt_t type though.
> > 
> >  IOW, if we have a VM running
> > 
> >    svirt_t:s0:c710,c716
> > 
> >  IMHO we would its corresponding passt instance to be
> >  running
> > 
> >    passt_t:s0:c710,c716
> > 
> > 
> >  not
> > 
> >    passt_t:s0:c0.c1023
> > >>>
> > >>> Practically speaking, it doesn't make a huge difference for passt
> > >>> because it unshares any relevant namespace right after it starts --
> > >>> that's *in theory* a strictly stronger isolation compared to what
> > >>> SELinux provides (at least once we reach the main loop).
> > >>
> > >> Even docker/podman will apply SELinux isolation per container,
> > >> rather than only relying on namespaces.   
> > > 
> > > Sure, I'm not saying it's not desirable -- but still, many (most?)
> > > host-facing services they rely on are not isolated in this sense. Same
> > > for the current implementation of libvirt with dnsmasq, for example.
> > >   
> > >>> But it makes sense, and I guess we should relabel to a specific MCS
> > >>> with still 'virtd_t' as a type, then later the domain would transition
> > >>> to passt_t. This could probably be done as an extension of
> > >>> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> > >>>
> > >>> Anyway, right now, I think this provides better security than
> > >>> 'setenforce 0', which is the only way to run passt from libvirt at the
> > >>> moment on some distributions.
> > >>
> > >> If running with SELinux permissive, this patch has no effect, as it
> > >> is unconfined regardless. That's not a situation we care about. If
> > >> people want to run without protection they get to keep the pieces
> > >> when it all goes wrong.  
> > > 
> > > The current implementation simply does not and cannot work with SELinux
> > > in enforcing mode, so users have no other choice.
> > >   
> > >>> I'm not sure how you handle these cases on libvirt, but generally
> > >>> speaking, this patch is a vast improvement on the current situation,
> > >>> and I can follow up with an extension or a different version of
> > >>> qemuSecurityCommandRun() later.
> > >>
> > >> No, I don't think it is a vast improvement.
> > >>
> > >> The goal of sVirt is to guarantee isolation between VMs.
> > >>
> > >> Running passt under svirt_t:MCS is not ideal because the svirt_t
> > >> policy allows things that passt should not get. It does still
> > >> guarantee isolation between VMs, because the MCS label is present.  
> > > 
> > > It's a bit more than that: it's not ideal because libvirt simply won't
> > > start passt. There's no isolation and no VMs.
> > >   
> > >> Switching to running passt_t:c0.c1023 will be more co

Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Michal Prívozník
On 2/22/23 12:30, Stefano Brivio wrote:
>>
>> I don't think we need such drastic measure. I think you can use:
>>
>> qemuPasstStart()
>> {
>>
>>
>>   seclabel = virDomainDefGetSecurityLabelDef(vm->def, "selinux");
>>   s = context_new(seclabel->label);
>>   context_type_set(s, "virt_t);
>>   newLabel = context_str(s);
>>
>>   virCommandSetSELinuxLabel(cmd, newLabel);
>>
>>   virCommandRun();
>> }
> 
> Yes, I actually tried something like this and it seemed to work, but I
> didn't propose it as it looks (is) gross.
> 

Agreed, it's not something I'd show to my kids, but it works.

> On the other hand, if you think it's acceptable as a temporary measure,
> let me test it (in a bit). Thanks for the snippet.
> 

Forgot to mention, it should be wrapped in #ifdef WITH_SELINUX as we
offer users to compile without SELinux support (e.g. FreeBSD which does
support QEMU but doesn't have SELinux, what a surprise, right?).

Michal



Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Daniel P . Berrangé
On Wed, Feb 22, 2023 at 12:21:09PM +0100, Michal Prívozník wrote:
> On 2/22/23 11:05, Stefano Brivio wrote:
> > On Wed, 22 Feb 2023 09:46:42 +
> > Daniel P. Berrangé  wrote:
> > 
> >> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:
> >>> On Tue, 21 Feb 2023 19:43:33 +
> >>> Daniel P. Berrangé  wrote:
> >>>   
>  On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:  
> > qemuSecurityCommandRun() causes an explicit domain transition of the
> > new process, but passt ships with its own SELinux policy, with
> > external interfaces for libvirtd, so we simply need to transition
> > from virtd_t to passt_t as passt is executed. The qemu type
> > enforcement rules have little to do with it.
> 
>  Can you clarify the difference here ?  
> >>>
> >>> Between...?
> >>>
> >>> I mean, virCommandRun() will just keep things running under virtd_t, so
> >>> that passt later can transition to passt_t.
> >>>
> >>> With qemuSecurityCommandRun(), there would be a transition from virtd_t
> >>> to svirt_t (it's the function that's called to start qemu, but
> >>> shouldn't be called here), and not to passt_t.
> >>>
> >>> But I'm not really sure that's what you were asking for.  
> >>
> >> Yes, it is.
> >>
> >>>   
>  Runing passt under 'svirt_t' is not desirable as that allows
>  many actions that are only relevant to QEMU.  
> >>>
> >>> Right, that's what this patch avoids. There are also actions, such as
> >>> starting passt or killing it, that we don't want to allow QEMU to do.
> >>>   
>  Running passt under the MCS label that is associated with the
>  VM is highly desirable though. Two passt instances belonging
>  to separate VMs are isolated from each other if they each use
>  the VM specific MCS label, than if they use the global default
>  MCS label.
> 
>  To use the VM specific MCS label would require libvirt to
>  explicitly set the desired selinux label on exec, it can't
>  happen automatically via an SELinux transition rule.
> 
>  We do stil want to use the passt_t type though.
> 
>  IOW, if we have a VM running
> 
>    svirt_t:s0:c710,c716
> 
>  IMHO we would its corresponding passt instance to be
>  running
> 
>    passt_t:s0:c710,c716
> 
> 
>  not
> 
>    passt_t:s0:c0.c1023  
> >>>
> >>> Practically speaking, it doesn't make a huge difference for passt
> >>> because it unshares any relevant namespace right after it starts --
> >>> that's *in theory* a strictly stronger isolation compared to what
> >>> SELinux provides (at least once we reach the main loop).  
> >>
> >> Even docker/podman will apply SELinux isolation per container,
> >> rather than only relying on namespaces. 
> > 
> > Sure, I'm not saying it's not desirable -- but still, many (most?)
> > host-facing services they rely on are not isolated in this sense. Same
> > for the current implementation of libvirt with dnsmasq, for example.
> > 
> >>> But it makes sense, and I guess we should relabel to a specific MCS
> >>> with still 'virtd_t' as a type, then later the domain would transition
> >>> to passt_t. This could probably be done as an extension of
> >>> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> >>>
> >>> Anyway, right now, I think this provides better security than
> >>> 'setenforce 0', which is the only way to run passt from libvirt at the
> >>> moment on some distributions.  
> >>
> >> If running with SELinux permissive, this patch has no effect, as it
> >> is unconfined regardless. That's not a situation we care about. If
> >> people want to run without protection they get to keep the pieces
> >> when it all goes wrong.
> > 
> > The current implementation simply does not and cannot work with SELinux
> > in enforcing mode, so users have no other choice.
> > 
> >>> I'm not sure how you handle these cases on libvirt, but generally
> >>> speaking, this patch is a vast improvement on the current situation,
> >>> and I can follow up with an extension or a different version of
> >>> qemuSecurityCommandRun() later.  
> >>
> >> No, I don't think it is a vast improvement.
> >>
> >> The goal of sVirt is to guarantee isolation between VMs.
> >>
> >> Running passt under svirt_t:MCS is not ideal because the svirt_t
> >> policy allows things that passt should not get. It does still
> >> guarantee isolation between VMs, because the MCS label is present.
> > 
> > It's a bit more than that: it's not ideal because libvirt simply won't
> > start passt. There's no isolation and no VMs.
> > 
> >> Switching to running passt_t:c0.c1023 will be more correct in terms
> >> of what permissions passt should be allowed, but it has disabled
> >> isolation of passt between VMs. This is a clear degradation of
> >> capabilities from the POV of sVirt's goals.
> > 
> > It's not a degradation because VMs can't start passt with SELinux in
> > enforcing mode, without this patch. No serv

Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 12:21:09 +0100
Michal Prívozník  wrote:

> On 2/22/23 11:05, Stefano Brivio wrote:
> > On Wed, 22 Feb 2023 09:46:42 +
> > Daniel P. Berrangé  wrote:
> >   
> >> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:  
> >>> On Tue, 21 Feb 2023 19:43:33 +
> >>> Daniel P. Berrangé  wrote:
> >>> 
>  On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:
> > qemuSecurityCommandRun() causes an explicit domain transition of the
> > new process, but passt ships with its own SELinux policy, with
> > external interfaces for libvirtd, so we simply need to transition
> > from virtd_t to passt_t as passt is executed. The qemu type
> > enforcement rules have little to do with it.  
> 
>  Can you clarify the difference here ?
> >>>
> >>> Between...?
> >>>
> >>> I mean, virCommandRun() will just keep things running under virtd_t, so
> >>> that passt later can transition to passt_t.
> >>>
> >>> With qemuSecurityCommandRun(), there would be a transition from virtd_t
> >>> to svirt_t (it's the function that's called to start qemu, but
> >>> shouldn't be called here), and not to passt_t.
> >>>
> >>> But I'm not really sure that's what you were asking for.
> >>
> >> Yes, it is.
> >>  
> >>> 
>  Runing passt under 'svirt_t' is not desirable as that allows
>  many actions that are only relevant to QEMU.
> >>>
> >>> Right, that's what this patch avoids. There are also actions, such as
> >>> starting passt or killing it, that we don't want to allow QEMU to do.
> >>> 
>  Running passt under the MCS label that is associated with the
>  VM is highly desirable though. Two passt instances belonging
>  to separate VMs are isolated from each other if they each use
>  the VM specific MCS label, than if they use the global default
>  MCS label.
> 
>  To use the VM specific MCS label would require libvirt to
>  explicitly set the desired selinux label on exec, it can't
>  happen automatically via an SELinux transition rule.
> 
>  We do stil want to use the passt_t type though.
> 
>  IOW, if we have a VM running
> 
>    svirt_t:s0:c710,c716
> 
>  IMHO we would its corresponding passt instance to be
>  running
> 
>    passt_t:s0:c710,c716
> 
> 
>  not
> 
>    passt_t:s0:c0.c1023
> >>>
> >>> Practically speaking, it doesn't make a huge difference for passt
> >>> because it unshares any relevant namespace right after it starts --
> >>> that's *in theory* a strictly stronger isolation compared to what
> >>> SELinux provides (at least once we reach the main loop).
> >>
> >> Even docker/podman will apply SELinux isolation per container,
> >> rather than only relying on namespaces.   
> > 
> > Sure, I'm not saying it's not desirable -- but still, many (most?)
> > host-facing services they rely on are not isolated in this sense. Same
> > for the current implementation of libvirt with dnsmasq, for example.
> >   
> >>> But it makes sense, and I guess we should relabel to a specific MCS
> >>> with still 'virtd_t' as a type, then later the domain would transition
> >>> to passt_t. This could probably be done as an extension of
> >>> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> >>>
> >>> Anyway, right now, I think this provides better security than
> >>> 'setenforce 0', which is the only way to run passt from libvirt at the
> >>> moment on some distributions.
> >>
> >> If running with SELinux permissive, this patch has no effect, as it
> >> is unconfined regardless. That's not a situation we care about. If
> >> people want to run without protection they get to keep the pieces
> >> when it all goes wrong.  
> > 
> > The current implementation simply does not and cannot work with SELinux
> > in enforcing mode, so users have no other choice.
> >   
> >>> I'm not sure how you handle these cases on libvirt, but generally
> >>> speaking, this patch is a vast improvement on the current situation,
> >>> and I can follow up with an extension or a different version of
> >>> qemuSecurityCommandRun() later.
> >>
> >> No, I don't think it is a vast improvement.
> >>
> >> The goal of sVirt is to guarantee isolation between VMs.
> >>
> >> Running passt under svirt_t:MCS is not ideal because the svirt_t
> >> policy allows things that passt should not get. It does still
> >> guarantee isolation between VMs, because the MCS label is present.  
> > 
> > It's a bit more than that: it's not ideal because libvirt simply won't
> > start passt. There's no isolation and no VMs.
> >   
> >> Switching to running passt_t:c0.c1023 will be more correct in terms
> >> of what permissions passt should be allowed, but it has disabled
> >> isolation of passt between VMs. This is a clear degradation of
> >> capabilities from the POV of sVirt's goals.  
> > 
> > It's not a degradation because VMs can't start passt with SELinux in
> > enfo

Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Michal Prívozník
On 2/22/23 11:05, Stefano Brivio wrote:
> On Wed, 22 Feb 2023 09:46:42 +
> Daniel P. Berrangé  wrote:
> 
>> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:
>>> On Tue, 21 Feb 2023 19:43:33 +
>>> Daniel P. Berrangé  wrote:
>>>   
 On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:  
> qemuSecurityCommandRun() causes an explicit domain transition of the
> new process, but passt ships with its own SELinux policy, with
> external interfaces for libvirtd, so we simply need to transition
> from virtd_t to passt_t as passt is executed. The qemu type
> enforcement rules have little to do with it.

 Can you clarify the difference here ?  
>>>
>>> Between...?
>>>
>>> I mean, virCommandRun() will just keep things running under virtd_t, so
>>> that passt later can transition to passt_t.
>>>
>>> With qemuSecurityCommandRun(), there would be a transition from virtd_t
>>> to svirt_t (it's the function that's called to start qemu, but
>>> shouldn't be called here), and not to passt_t.
>>>
>>> But I'm not really sure that's what you were asking for.  
>>
>> Yes, it is.
>>
>>>   
 Runing passt under 'svirt_t' is not desirable as that allows
 many actions that are only relevant to QEMU.  
>>>
>>> Right, that's what this patch avoids. There are also actions, such as
>>> starting passt or killing it, that we don't want to allow QEMU to do.
>>>   
 Running passt under the MCS label that is associated with the
 VM is highly desirable though. Two passt instances belonging
 to separate VMs are isolated from each other if they each use
 the VM specific MCS label, than if they use the global default
 MCS label.

 To use the VM specific MCS label would require libvirt to
 explicitly set the desired selinux label on exec, it can't
 happen automatically via an SELinux transition rule.

 We do stil want to use the passt_t type though.

 IOW, if we have a VM running

   svirt_t:s0:c710,c716

 IMHO we would its corresponding passt instance to be
 running

   passt_t:s0:c710,c716


 not

   passt_t:s0:c0.c1023  
>>>
>>> Practically speaking, it doesn't make a huge difference for passt
>>> because it unshares any relevant namespace right after it starts --
>>> that's *in theory* a strictly stronger isolation compared to what
>>> SELinux provides (at least once we reach the main loop).  
>>
>> Even docker/podman will apply SELinux isolation per container,
>> rather than only relying on namespaces. 
> 
> Sure, I'm not saying it's not desirable -- but still, many (most?)
> host-facing services they rely on are not isolated in this sense. Same
> for the current implementation of libvirt with dnsmasq, for example.
> 
>>> But it makes sense, and I guess we should relabel to a specific MCS
>>> with still 'virtd_t' as a type, then later the domain would transition
>>> to passt_t. This could probably be done as an extension of
>>> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
>>>
>>> Anyway, right now, I think this provides better security than
>>> 'setenforce 0', which is the only way to run passt from libvirt at the
>>> moment on some distributions.  
>>
>> If running with SELinux permissive, this patch has no effect, as it
>> is unconfined regardless. That's not a situation we care about. If
>> people want to run without protection they get to keep the pieces
>> when it all goes wrong.
> 
> The current implementation simply does not and cannot work with SELinux
> in enforcing mode, so users have no other choice.
> 
>>> I'm not sure how you handle these cases on libvirt, but generally
>>> speaking, this patch is a vast improvement on the current situation,
>>> and I can follow up with an extension or a different version of
>>> qemuSecurityCommandRun() later.  
>>
>> No, I don't think it is a vast improvement.
>>
>> The goal of sVirt is to guarantee isolation between VMs.
>>
>> Running passt under svirt_t:MCS is not ideal because the svirt_t
>> policy allows things that passt should not get. It does still
>> guarantee isolation between VMs, because the MCS label is present.
> 
> It's a bit more than that: it's not ideal because libvirt simply won't
> start passt. There's no isolation and no VMs.
> 
>> Switching to running passt_t:c0.c1023 will be more correct in terms
>> of what permissions passt should be allowed, but it has disabled
>> isolation of passt between VMs. This is a clear degradation of
>> capabilities from the POV of sVirt's goals.
> 
> It's not a degradation because VMs can't start passt with SELinux in
> enforcing mode, without this patch. No service, no degradation.
> 
> I looked into options to rework
> virSecurityManagerSetChildProcessLabel() and friends with a more
> flexible modeling/building of labels -- just doing some trick all the
> way down from qemuSecurityCommandRun() implies a number of layering
> violations that I would lik

Re: [libvirt PATCH 0/3] Support for restarting passt backend

2023-02-22 Thread Michal Prívozník
On 2/22/23 01:35, Laine Stump wrote:
> If the passt process that implements the backend of a QEMU emulated
> network device terminates, QEMU itself is incapable of restarting a
> new process so that the networking can begin... working again.
> 
> However, what QEMU can and does do is:
> 
> 1) send the NETDEV_STREAM_DISCONNECTED event to libvirt when it sees
>that the socket has been closed (this event has been in QEMU for as
>long as support for "-netdev stream", which is used for connecting
>to passt)
> 
> 2) as of QEMU 8.0.0, the qemu commandline for -netdev stream accepts
>the "reconnect" option, which tells QEMU to attempt reconnecting to
>the same socket it previously used, repeating the attempt every "n"
>seconds (the only argument to reconnect) until it is successful.
> 
> If libvirt adds the reconnect option to the qemu commandline, and then
> responds to a NETDEV_STREAM_DISCONNECTED event by re-running the same
> passt command that it ran when the device was originally connected,
> then a guest will be able to recover in the (very unlikely, according
> to Stefano!) event that the original passt process unexpectedly exits,
> or is killed by some external entity.
> 
> Patch 2/3 handles (2) above, while patch 3/3 handles (1). (patch 1/3
> is a short guest appearance by pkrempa. Thanks pkrempa!).
> 
> This resolves https://bugzilla.redhat.com/2172098
> 
> Along with Stefano's series fixing up selinux issues related to
> running the passt process, they make the passt backend very usable.
> 
> Laine Stump (2):
>   qemu: add reconnect=5 to passt qemu commandline options when available
>   qemu: respond to NETDEV_STREAM_DISCONNECTED event
> 
> Peter Krempa (1):
>   qemu: capabilities: Introduce QEMU_CAPS_NETDEV_STREAM_RECONNECT
> 
>  src/qemu/qemu_capabilities.c  |  4 +
>  src/qemu/qemu_capabilities.h  |  3 +
>  src/qemu/qemu_domain.c|  1 +
>  src/qemu/qemu_domain.h|  1 +
>  src/qemu/qemu_driver.c| 82 +++
>  src/qemu/qemu_monitor.c   | 11 +++
>  src/qemu/qemu_monitor.h   |  6 ++
>  src/qemu/qemu_monitor_json.c  | 16 
>  src/qemu/qemu_passt.c | 11 +++
>  src/qemu/qemu_process.c   | 18 
>  .../caps_8.0.0.x86_64.xml |  1 +
>  .../net-user-passt.x86_64-7.2.0.args  | 37 +
>  .../net-user-passt.x86_64-latest.args |  2 +-
>  tests/qemuxml2argvtest.c  |  1 +
>  14 files changed, 193 insertions(+), 1 deletion(-)
>  create mode 100644 tests/qemuxml2argvdata/net-user-passt.x86_64-7.2.0.args
> 

Reviewed-by: Michal Privoznik 

Michal



Re: [libvirt PATCH 3/3] qemu: respond to NETDEV_STREAM_DISCONNECTED event

2023-02-22 Thread Michal Prívozník
On 2/22/23 01:35, Laine Stump wrote:
> When a QEMU netdev is of type "stream", if the socket it uses for
> connectivity to the host network gets closed, then QEMU will send a
> NETDEV_STREAM_DISCONNECTED event. We know that any stream netdev we've
> created is backed by a passt process, and if the socket was closed,
> that means the passt process has disappeared.
> 
> When we receive this event, we can respond by starting a new passt
> process with the same options (including socket path) we originally
> used. If we have previously created the stream netdev device with a
> "reconnect" option, then QEMU will automatically reconnect to this new
> passt process. (If we hadn't used "reconnect", then QEMU will never
> try to reconnect to the new passt process, so there's no point in
> starting it.)
> 
> Note that NETDEV_STREAM_DISCONNECTED is an event sent for the netdev
> (ie "host side") of the network device, and so it sends the
> "netdev-id" to specify which device was disconnected. But libvirt's
> virDomainNetDef (the object used to keep track of network devices) is
> the internal representation of both the host-side "netdev", and the
> guest side device, and virDomainNetDef doesn't directly keep track of
> the netdev-id, only of the device's "alias" (which is the "id"
> parameter of the *guest* side of the device). Fortunately, by convention
> libvirt always names the host-side of devices as "host" + alias, so in
> order to search for the affected NetDef, all we need to do is trim the
> 1st 4 characters from the netdev-id and look for the NetDef having
> that resulting trimmed string as its alias. (Contrast this to
> NIC_RX_FILTER_CHANGED, which is an event received for the guest side
> of the device, and so directly contains the device alias.)
> 
> Resolves: https://bugzilla.redhat.com/2172098
> Signed-off-by: Laine Stump 
> ---
>  src/qemu/qemu_domain.c   |  1 +
>  src/qemu/qemu_domain.h   |  1 +
>  src/qemu/qemu_driver.c   | 82 
>  src/qemu/qemu_monitor.c  | 11 +
>  src/qemu/qemu_monitor.h  |  6 +++
>  src/qemu/qemu_monitor_json.c | 16 +++
>  src/qemu/qemu_process.c  | 18 
>  7 files changed, 135 insertions(+)
> 
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index e9bc0f375d..4cf9a259ea 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -11238,6 +11238,7 @@ qemuProcessEventFree(struct qemuProcessEvent *event)
>  break;
>  case QEMU_PROCESS_EVENT_WATCHDOG:
>  case QEMU_PROCESS_EVENT_DEVICE_DELETED:
> +case QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED:
>  case QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED:
>  case QEMU_PROCESS_EVENT_SERIAL_CHANGED:
>  case QEMU_PROCESS_EVENT_MONITOR_EOF:
> diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
> index 1053d1d4cb..6adc067681 100644
> --- a/src/qemu/qemu_domain.h
> +++ b/src/qemu/qemu_domain.h
> @@ -447,6 +447,7 @@ typedef enum {
>  QEMU_PROCESS_EVENT_WATCHDOG = 0,
>  QEMU_PROCESS_EVENT_GUESTPANIC,
>  QEMU_PROCESS_EVENT_DEVICE_DELETED,
> +QEMU_PROCESS_EVENT_NETDEV_STREAM_DISCONNECTED,
>  QEMU_PROCESS_EVENT_NIC_RX_FILTER_CHANGED,
>  QEMU_PROCESS_EVENT_SERIAL_CHANGED,
>  QEMU_PROCESS_EVENT_JOB_STATUS_CHANGE,
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 6154fe9bfe..47d6a0dd95 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -40,6 +40,7 @@
>  #include "qemu_hostdev.h"
>  #include "qemu_hotplug.h"
>  #include "qemu_monitor.h"
> +#include "qemu_passt.h"
>  #include "qemu_process.h"
>  #include "qemu_migration.h"
>  #include "qemu_migration_params.h"
> @@ -3622,6 +3623,84 @@ processDeviceDeletedEvent(virQEMUDriver *driver,
>  }
>  
>  
> +static void
> +processNetdevStreamDisconnectedEvent(virDomainObj *vm,
> + const char *netdevId)
> +{
> +virDomainDeviceDef dev;
> +virDomainNetDef *def;
> +qemuDomainObjPrivate *priv;
> +virQEMUCaps *qemuCaps;
> +const char *devAlias = NULL;
> +
> +/* The event sends us the "netdev-id", but we don't store the
> + * netdev-id in the NetDef and thus can't use it to find the
> + * correct NetDef. We *do* keep the device alias in the NetDef.
> + * By definition, the netdev-id is "host" + devAlias, so we just
> + * need to remove "host" from the front of netdev-id to get
> + * something we can use to find the proper NetDef.
> + */
> +if (STREQLEN(netdevId, "host", 4))
> +devAlias = &netdevId[4];

This is open coding STRSKIP():

  devAlias = STRSKIP(netdevId, "host");

> +
> +if (!devAlias) {
> +VIR_WARN("Received NETDEV_STREAM_DISCONNECTED event for unrecognized 
> netdev %s from domain %p %s",
> +  netdevId, vm, vm->def->name);
> +return;
> +}
> +
> +VIR_DEBUG("Received NETDEV_STREAM_DISCONNECTED event for device %s from 
> domain %p %s",
> +  devAlias, vm,

Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Stefano Brivio
On Wed, 22 Feb 2023 09:46:42 +
Daniel P. Berrangé  wrote:

> On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:
> > On Tue, 21 Feb 2023 19:43:33 +
> > Daniel P. Berrangé  wrote:
> >   
> > > On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:  
> > > > qemuSecurityCommandRun() causes an explicit domain transition of the
> > > > new process, but passt ships with its own SELinux policy, with
> > > > external interfaces for libvirtd, so we simply need to transition
> > > > from virtd_t to passt_t as passt is executed. The qemu type
> > > > enforcement rules have little to do with it.
> > > 
> > > Can you clarify the difference here ?  
> > 
> > Between...?
> > 
> > I mean, virCommandRun() will just keep things running under virtd_t, so
> > that passt later can transition to passt_t.
> > 
> > With qemuSecurityCommandRun(), there would be a transition from virtd_t
> > to svirt_t (it's the function that's called to start qemu, but
> > shouldn't be called here), and not to passt_t.
> > 
> > But I'm not really sure that's what you were asking for.  
> 
> Yes, it is.
> 
> >   
> > > Runing passt under 'svirt_t' is not desirable as that allows
> > > many actions that are only relevant to QEMU.  
> > 
> > Right, that's what this patch avoids. There are also actions, such as
> > starting passt or killing it, that we don't want to allow QEMU to do.
> >   
> > > Running passt under the MCS label that is associated with the
> > > VM is highly desirable though. Two passt instances belonging
> > > to separate VMs are isolated from each other if they each use
> > > the VM specific MCS label, than if they use the global default
> > > MCS label.
> > > 
> > > To use the VM specific MCS label would require libvirt to
> > > explicitly set the desired selinux label on exec, it can't
> > > happen automatically via an SELinux transition rule.
> > > 
> > > We do stil want to use the passt_t type though.
> > > 
> > > IOW, if we have a VM running
> > > 
> > >   svirt_t:s0:c710,c716
> > > 
> > > IMHO we would its corresponding passt instance to be
> > > running
> > > 
> > >   passt_t:s0:c710,c716
> > > 
> > > 
> > > not
> > > 
> > >   passt_t:s0:c0.c1023  
> > 
> > Practically speaking, it doesn't make a huge difference for passt
> > because it unshares any relevant namespace right after it starts --
> > that's *in theory* a strictly stronger isolation compared to what
> > SELinux provides (at least once we reach the main loop).  
> 
> Even docker/podman will apply SELinux isolation per container,
> rather than only relying on namespaces. 

Sure, I'm not saying it's not desirable -- but still, many (most?)
host-facing services they rely on are not isolated in this sense. Same
for the current implementation of libvirt with dnsmasq, for example.

> > But it makes sense, and I guess we should relabel to a specific MCS
> > with still 'virtd_t' as a type, then later the domain would transition
> > to passt_t. This could probably be done as an extension of
> > qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> > 
> > Anyway, right now, I think this provides better security than
> > 'setenforce 0', which is the only way to run passt from libvirt at the
> > moment on some distributions.  
> 
> If running with SELinux permissive, this patch has no effect, as it
> is unconfined regardless. That's not a situation we care about. If
> people want to run without protection they get to keep the pieces
> when it all goes wrong.

The current implementation simply does not and cannot work with SELinux
in enforcing mode, so users have no other choice.

> > I'm not sure how you handle these cases on libvirt, but generally
> > speaking, this patch is a vast improvement on the current situation,
> > and I can follow up with an extension or a different version of
> > qemuSecurityCommandRun() later.  
> 
> No, I don't think it is a vast improvement.
> 
> The goal of sVirt is to guarantee isolation between VMs.
> 
> Running passt under svirt_t:MCS is not ideal because the svirt_t
> policy allows things that passt should not get. It does still
> guarantee isolation between VMs, because the MCS label is present.

It's a bit more than that: it's not ideal because libvirt simply won't
start passt. There's no isolation and no VMs.

> Switching to running passt_t:c0.c1023 will be more correct in terms
> of what permissions passt should be allowed, but it has disabled
> isolation of passt between VMs. This is a clear degradation of
> capabilities from the POV of sVirt's goals.

It's not a degradation because VMs can't start passt with SELinux in
enforcing mode, without this patch. No service, no degradation.

I looked into options to rework
virSecurityManagerSetChildProcessLabel() and friends with a more
flexible modeling/building of labels -- just doing some trick all the
way down from qemuSecurityCommandRun() implies a number of layering
violations that I would like to avoid.

It all looks doable, but implementing

Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-22 Thread Daniel P . Berrangé
On Wed, Feb 22, 2023 at 09:11:13AM +, Bernhard Beschow wrote:
> 
> 
> Am 30. Januar 2023 20:45:47 UTC schrieb "Alex Bennée" 
> :
> >
> >Daniel P. Berrangé  writes:
> >
> >> On Mon, Jan 30, 2023 at 11:47:02AM +, Peter Maydell wrote:
> >>> On Mon, 30 Jan 2023 at 11:44, Thomas Huth  wrote:
> >>> >
> >>> > Testing 32-bit host OS support takes a lot of precious time during the 
> >>> > QEMU
> >>> > contiguous integration tests, and considering that many OS vendors 
> >>> > stopped
> >>> > shipping 32-bit variants of their OS distributions and most hardware 
> >>> > from
> >>> > the past >10 years is capable of 64-bit
> >>> 
> >>> True for x86, not necessarily true for other architectures.
> >>> Are you proposing to deprecate x86 32-bit, or all 32-bit?
> >>> I'm not entirely sure about whether we're yet at a point where
> >>> I'd want to deprecate-and-drop 32-bit arm host support.
> >>
> >> Do we have a feeling on which aspects of 32-bit cause us the support
> >> burden ? The boring stuff like compiler errors from mismatched integer
> >> sizes is mostly quick & easy to detect simply through a cross compile.
> >>
> >> I vaguely recall someone mentioned problems with atomic ops in the past,
> >> or was it 128-bit ints, caused implications for the codebase ?
> >
> >Atomic operations on > TARGET_BIT_SIZE and cputlb when
> >TCG_OVERSIZED_GUEST is set. Also the core TCG code and a bunch of the
> >backends have TARGET_LONG_BITS > TCG_TARGET_REG_BITS ifdefs peppered
> >throughout.
> 
> Are there any plans or ideas to support 128 bit architectures
> such as CHERI in the future? There is already a QEMU fork
> implementing CHERI for RISC V [1]. Also ARM has developed an
> experimental hardware implementation of CHERI within the Morello
> project where Linaro is involved as well, although the QEMU
> implementation is performed by the University of Cambridge [2].

If 128 bit hardware exists and has real world non-toy usage,
then a request to support it in QEMU is essentially inevitable.

> I'm asking because once we deeply bake in the assumption that
> host size >= guest size then adding such architectures will
> become much harder.

Yep, that is a risk.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [PATCH 1/3] qemu_passt: Don't make passt transition to svirt_t/virt_domain on start

2023-02-22 Thread Daniel P . Berrangé
On Tue, Feb 21, 2023 at 10:49:46PM +0100, Stefano Brivio wrote:
> On Tue, 21 Feb 2023 19:43:33 +
> Daniel P. Berrangé  wrote:
> 
> > On Tue, Feb 21, 2023 at 08:19:05PM +0100, Stefano Brivio wrote:
> > > qemuSecurityCommandRun() causes an explicit domain transition of the
> > > new process, but passt ships with its own SELinux policy, with
> > > external interfaces for libvirtd, so we simply need to transition
> > > from virtd_t to passt_t as passt is executed. The qemu type
> > > enforcement rules have little to do with it.  
> > 
> > Can you clarify the difference here ?
> 
> Between...?
> 
> I mean, virCommandRun() will just keep things running under virtd_t, so
> that passt later can transition to passt_t.
> 
> With qemuSecurityCommandRun(), there would be a transition from virtd_t
> to svirt_t (it's the function that's called to start qemu, but
> shouldn't be called here), and not to passt_t.
> 
> But I'm not really sure that's what you were asking for.

Yes, it is.

> 
> > Runing passt under 'svirt_t' is not desirable as that allows
> > many actions that are only relevant to QEMU.
> 
> Right, that's what this patch avoids. There are also actions, such as
> starting passt or killing it, that we don't want to allow QEMU to do.
> 
> > Running passt under the MCS label that is associated with the
> > VM is highly desirable though. Two passt instances belonging
> > to separate VMs are isolated from each other if they each use
> > the VM specific MCS label, than if they use the global default
> > MCS label.
> > 
> > To use the VM specific MCS label would require libvirt to
> > explicitly set the desired selinux label on exec, it can't
> > happen automatically via an SELinux transition rule.
> > 
> > We do stil want to use the passt_t type though.
> > 
> > IOW, if we have a VM running
> > 
> >   svirt_t:s0:c710,c716
> > 
> > IMHO we would its corresponding passt instance to be
> > running
> > 
> >   passt_t:s0:c710,c716
> > 
> > 
> > not
> > 
> >   passt_t:s0:c0.c1023
> 
> Practically speaking, it doesn't make a huge difference for passt
> because it unshares any relevant namespace right after it starts --
> that's *in theory* a strictly stronger isolation compared to what
> SELinux provides (at least once we reach the main loop).

Even docker/podman will apply SELinux isolation per container,
rather than only relying on namespaces. 

> But it makes sense, and I guess we should relabel to a specific MCS
> with still 'virtd_t' as a type, then later the domain would transition
> to passt_t. This could probably be done as an extension of
> qemuSecurityCommandRun(), I haven't looked into it yet. I will.
> 
> Anyway, right now, I think this provides better security than
> 'setenforce 0', which is the only way to run passt from libvirt at the
> moment on some distributions.

If running with SELinux permissive, this patch has no effect, as it
is unconfined regardless. That's not a situation we care about. If
people want to run without protection they get to keep the pieces
when it all goes wrong.

> I'm not sure how you handle these cases on libvirt, but generally
> speaking, this patch is a vast improvement on the current situation,
> and I can follow up with an extension or a different version of
> qemuSecurityCommandRun() later.

No, I don't think it is a vast improvement.

The goal of sVirt is to guarantee isolation between VMs.

Running passt under svirt_t:MCS is not ideal because the svirt_t
policy allows things that passt should not get. It does still
guarantee isolation between VMs, because the MCS label is present.

Switching to running passt_t:c0.c1023 will be more correct in terms
of what permissions passt should be allowed, but it has disabled
isolation of passt between VMs. This is a clear degradation of
capabilities from the POV of sVirt's goals.

With regards,
Daniel
-- 
|: https://berrange.com  -o-https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o-https://fstop138.berrange.com :|
|: https://entangle-photo.org-o-https://www.instagram.com/dberrange :|



Re: [libvirt PATCH 0/2] A couple simple cleanups

2023-02-22 Thread Michal Prívozník
On 2/22/23 01:29, Laine Stump wrote:
> Noticed these while doing other work.
> 
> Laine Stump (2):
>   qemu: add check for QEMU_CAPS_NETDEV_STREAM during validation
>   qemu: remove extraneous error log when qemuPasstStart() fails during
> hotplug
> 
>  src/qemu/qemu_hotplug.c   |  5 +-
>  src/qemu/qemu_validate.c  |  6 ++
>  .../net-user-passt.x86_64-latest.xml  | 61 +++
>  tests/qemuxml2xmltest.c   |  2 +-
>  4 files changed, 69 insertions(+), 5 deletions(-)
>  create mode 100644 tests/qemuxml2xmloutdata/net-user-passt.x86_64-latest.xml
> 

Reviewed-by: Michal Privoznik 

Michal



Re: [libvirt PATCH v2] qemu: implement QEMU NBD source reconnect delay attribute

2023-02-22 Thread Christian Nautze
I had a first version on gitlab
https://gitlab.com/libvirt/libvirt/-/merge_requests/216
where I added the attribute to the source. Peter Krempa noted that
this most likely has to be moved to the reconnect element.
I actually prefer adding it to reconnect (with choice in the schema).
If there are no objections I will make the necessary change...

On Tue, 21 Feb 2023 at 23:10, Jonathon Jongsma  wrote:

> On 2/17/23 10:50 AM, Christian Nautze wrote:
> >  Currently it's only possible to set this parameter during domain
> >  creation via QEMU commandline passthrough feature.
> >  With the new delay attribute it's also possible to set this
> >  parameter if you want to attach a new NBD disk
> >  using "virsh attach-device domain device.xml" e.g.:
> >
> >
> >  
> >  
> >
> >
> >  
> >  
> >
> >
> > Signed-off-by: Christian Nautze 
> >
>
>
> I wonder about the choice between using an attribute vs a child element
> for this value. Most of the things that use child elements for are more
> complex values. For example, 'host' is an address and a port, 'cookies'
> is a collection of cookie elements, even the existing 'reconnect'
> element has two sub-values. On the other hand, things that are a simple
> value tend to be specified as attributes instead. NBD sources in
> particular have attributes 'name', 'tls', and 'tlsHostname'. Since this
> is a single integer value, I wonder if it would make more sense to use
> another nbd-specific attribute such as 'reconnectDelay'. For example:
>
>
>
> This might also reduce potential confusion about how to specify the
> 'reconnect' element, but has the downside of having two different
> reconnect-related elements. To be honest, I'm not sure whether there is
> an overal philosophy on attributes vs. elements, so I will happily defer
> to more veteran libvirt developers.
>
> If we do use the  sub-element, I think the schema would need
> to use  to indicate that it's either ('enabled'+'timeout') OR
> 'delay', not an arbitrary subset of these attributes.
>
> Jonathon
>
>
> > ---
> > Change: reconnect element: drop mandatory 'enabled' attribute when using
> 'delay'
> > ---
> >   docs/formatdomain.rst | 11 +++--
> >   src/conf/domain_conf.c| 12 ++
> >   src/conf/schemas/domaincommon.rng |  3 +++
> >   src/conf/schemas/storagecommon.rng| 13 ---
> >   src/conf/storage_source_conf.c|  1 +
> >   src/conf/storage_source_conf.h|  4 
> >   src/qemu/qemu_block.c |  4 +++-
> >   src/qemu/qemu_domain.c|  9 
> >   .../disk-network-nbd.x86_64-latest.args   | 23 +++
> >   tests/qemuxml2argvdata/disk-network-nbd.xml   |  8 +++
> >   tests/qemuxml2xmloutdata/disk-network-nbd.xml |  9 
> >   11 files changed, 81 insertions(+), 16 deletions(-)
> >
> > diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst
> > index 36c6d87907..ee30c51cea 100644
> > --- a/docs/formatdomain.rst
> > +++ b/docs/formatdomain.rst
> > @@ -2946,13 +2946,20 @@ paravirtualized driver is specified via the
> ``disk`` element.
> > are intended to be default, then the entire element may be
> omitted.
> >  ``reconnect``
> > For disk type ``vhostuser`` configures reconnect timeout if the
> connection
> > -  is lost. It has two mandatory attributes:
> > +  is lost. This is set with the two mandatory attributes
> ``enabled`` and
> > +  ``timeout``.
> > +  For disk type ``network`` and protocol ``nbd`` the QEMU NBD
> reconnect delay
> > +  can be set via attribute ``delay``:
> >
> > ``enabled``
> >If the reconnect feature is enabled, accepts ``yes`` and
> ``no``
> > ``timeout``
> >The amount of seconds after which hypervisor tries to
> reconnect.
> > -
> > +  ``delay``
> > + Only for NBD hosts. The amount of seconds during which all
> requests are
> > + paused and will be rerun after a successful reconnect. After
> that time, any
> > + delayed requests and all future requests before a successful
> reconnect
> > + will immediately fail. If not set the default QEMU value is 0.
> >
> >  For a "file" or "volume" disk type which represents a cdrom or
> floppy (the
> >  ``device`` attribute), it is possible to define policy what to do
> with the
> > diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
> > index a5578324b9..3f2ba2aab8 100644
> > --- a/src/conf/domain_conf.c
> > +++ b/src/conf/domain_conf.c
> > @@ -7146,6 +7146,15 @@ virDomainDiskSourceNetworkParse(xmlNodePtr node,
> >   src->tlsFromConfig = !!value;
> >   }
> >
> > +if (src->protocol == VIR_STORAGE_NET_PROTOCOL_NBD) {
> > +xmlNodePtr cur;
> > +if ((cur = virXPathNode("./reconnect", ctxt))) {
> > +

Re: New wiki.libvirt.org site is live

2023-02-22 Thread Erik Skultety
On Tue, Feb 21, 2023 at 04:27:32PM +, Daniel P. Berrangé wrote:
> FYI, following Peter's work to import the old mediawiki content into
> the libvirt-wiki.git repo, the https://wiki.libvirt.org site has now
> been switched over to GitLab Pages. It should be working for everyone
> assuming DNS has propagated.
> 
> This new wiki isn't really a wiki of course, and we're not actually
> recommending people add new content to it. This just gets us off
> mediawiki as a service so we take advantage of gitlab which is more
> contributor friendly and avoids single points of failure.
> 
> Feel free to submit patches to correct obvious mistakes in existing
> pages.
> 
> Our long term preference remains for all content to make its way over
> to libvirt.git, probably under the /docs/kbase directory for most of
> what was wiki based before.

Kudos to everyone involved.

Regards,
Erik



Re: [RFC PATCH] docs/about/deprecated: Deprecate 32-bit host systems

2023-02-22 Thread Bernhard Beschow



Am 30. Januar 2023 20:45:47 UTC schrieb "Alex Bennée" :
>
>Daniel P. Berrangé  writes:
>
>> On Mon, Jan 30, 2023 at 11:47:02AM +, Peter Maydell wrote:
>>> On Mon, 30 Jan 2023 at 11:44, Thomas Huth  wrote:
>>> >
>>> > Testing 32-bit host OS support takes a lot of precious time during the 
>>> > QEMU
>>> > contiguous integration tests, and considering that many OS vendors stopped
>>> > shipping 32-bit variants of their OS distributions and most hardware from
>>> > the past >10 years is capable of 64-bit
>>> 
>>> True for x86, not necessarily true for other architectures.
>>> Are you proposing to deprecate x86 32-bit, or all 32-bit?
>>> I'm not entirely sure about whether we're yet at a point where
>>> I'd want to deprecate-and-drop 32-bit arm host support.
>>
>> Do we have a feeling on which aspects of 32-bit cause us the support
>> burden ? The boring stuff like compiler errors from mismatched integer
>> sizes is mostly quick & easy to detect simply through a cross compile.
>>
>> I vaguely recall someone mentioned problems with atomic ops in the past,
>> or was it 128-bit ints, caused implications for the codebase ?
>
>Atomic operations on > TARGET_BIT_SIZE and cputlb when
>TCG_OVERSIZED_GUEST is set. Also the core TCG code and a bunch of the
>backends have TARGET_LONG_BITS > TCG_TARGET_REG_BITS ifdefs peppered
>throughout.

Are there any plans or ideas to support 128 bit architectures such as CHERI in 
the future? There is already a QEMU fork implementing CHERI for RISC V [1]. 
Also ARM has developed an experimental hardware implementation of CHERI within 
the Morello project where Linaro is involved as well, although the QEMU 
implementation is performed by the University of Cambridge [2].

I'm asking because once we deeply bake in the assumption that host size >= 
guest size then adding such architectures will become much harder.

Best regards,
Bernhard

[1] https://github.com/CTSRD-CHERI/qemu
[2] 
https://git.morello-project.org/university-of-cambridge/mirrors/qemu/-/tree/qemu-morello-rebased

>
>>
>> With regards,
>> Daniel
>
>