Re: [PATCH 4/8] host/rootfs: run xdg-desktop-portal-spectrum-host as non-root

[Alyssa Ross <hi@alyssa.is>]

[-- Attachment #1: Type: text/plain, Size: 4186 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 12/10/25 07:47, Alyssa Ross wrote:
>> Signed-off-by: Alyssa Ross <hi@alyssa.is>
>> ---
>>  host/rootfs/file-list.mk                      |  1 +
>>  host/rootfs/image/etc/dbus-portal.conf.in     | 11 +++++++++++
>>  .../template/data/service/dbus/run            |  8 +++++++-
>>  .../xdg-desktop-portal-spectrum-host/run      |  2 ++
>>  host/rootfs/image/usr/bin/run-appimage        |  1 +
>>  host/rootfs/image/usr/bin/run-flatpak         |  1 +
>>  host/rootfs/image/usr/bin/vm-import           |  1 +
>>  host/rootfs/image/usr/bin/vm-start            | 19 ++++++++++++++++++-
>>  8 files changed, 42 insertions(+), 2 deletions(-)
>>  create mode 100644 host/rootfs/image/etc/dbus-portal.conf.in
>> 
>> diff --git a/host/rootfs/file-list.mk b/host/rootfs/file-list.mk
>> index f69775d2..59d83b7e 100644
>> --- a/host/rootfs/file-list.mk
>> +++ b/host/rootfs/file-list.mk
>> @@ -2,6 +2,7 @@
>>  # SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>>  
>>  FILES = \
>> +	image/etc/dbus-portal.conf.in \
>>  	image/etc/fonts/fonts.conf \
>>  	image/etc/fstab \
>>  	image/etc/init \
>> diff --git a/host/rootfs/image/etc/dbus-portal.conf.in b/host/rootfs/image/etc/dbus-portal.conf.in
>> new file mode 100644
>> index 00000000..3e0e6725
>> --- /dev/null
>> +++ b/host/rootfs/image/etc/dbus-portal.conf.in
>> @@ -0,0 +1,11 @@
>> +<?xml version="1.0" encoding="UTF-8"?>
>> +<!-- SPDX-License-Identifier: CC0-1.0 -->
>> +<!-- SPDX-FileCopyrightText: 2025 Alyssa Ross <hi@alyssa.is> -->
>> +<!DOCTYPE busconfig SYSTEM "busconfig.dtd">
>> +<busconfig>
>> +  <include>/usr/share/dbus-1/session.conf</include>
>> +
>> +  <policy context="default">
>> +    <allow user="@XDP_SPECTRUM_USER@"/>
>> +  </policy>
>> +</busconfig>
>> diff --git a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> index 83e97c65..20f1daff 100755
>> --- a/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> +++ b/host/rootfs/image/etc/s6-linux-init/run-image/service/vm-services/template/data/service/dbus/run
>> @@ -4,11 +4,17 @@
>>  
>>  importas -i VM VM
>>  
>> +if {
>> +  redirfd -w 1 data/dbus.conf
>> +  sed "s/@XDP_SPECTRUM_USER@/xdp-spectrum-${VM}/g" /etc/dbus-portal.conf.in
>> +}
>
> This makes me nervous.  I know that $VM is trusted, but I'd feel
> better if this was validated with a case command.  There's a bug in
> case that makes this not work properly, but that's fixed in execline
> git right now.

I don't think this is necessary, because as you say it's trusted.  There
shouldn't be any way to invoke this script with elevated permissions
anyway, so it's not doing anything that whatever is invoking it couldn't
just do themself.

>> diff --git a/host/rootfs/image/usr/bin/vm-start b/host/rootfs/image/usr/bin/vm-start
>> index 67480e52..c8031eec 100755
>> --- a/host/rootfs/image/usr/bin/vm-start
>> +++ b/host/rootfs/image/usr/bin/vm-start
>> @@ -20,4 +20,21 @@ foreground {
>>    redirfd -w 2 /dev/null
>>    s6-svwait -U /run/service/vmm/instance/${1}
>>  }
>> -ch-remote --api-socket /run/vm/by-id/${1}/vmm boot
>> +foreground { ch-remote --api-socket /run/vm/by-id/${1}/vmm boot }
>> +importas -Siu ?
>> +if {
>> +  if -t { test $? -eq 0 }
>> +
>> +  # This is technically racy: if somehow we don't get here before the VM boots
>> +  # and connects to xdg-desktop-portal-spectrum-host, it won't be able to
>> +  # connect.  The VM rebooting will also break this, because the socket will be
>> +  # re-created with the wrong mode, but VM reboots are broken anyway at the time
>> +  # of writing:
>> +  #
>> +  # https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7547
>> +  #
>> +  # Ideally we'd be able to give a listening socket FD to Cloud Hypervisor for
>> +  # its VSOCK socket.
>> +  chown xdp-spectrum-${1} /run/vsock/${1}/vsock
>
> It's possible to avoid the race using extended ACLs.

Nice idea!

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]