Expected behavior of empty service arguments

[Demi Marie Obenour]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

Should an empty service argument (qubes.Service+) always be the same as
no argument at all (qubes.Service)? Right now, they are the same when
coming from a VM, but not when coming from dom0: qubes.Service from dom0
will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
will.
- --
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmYPAycACgkQsoi1X/+c
IsEdPRAAzFX1QOAVVRNx3yeWdrBW58IQKb7Jjv8KDpFQyhRjz+yeSooJUrrLIGE5
g3AI22Rz4oS1f0YJRHNP9whOU31iLJXZ2lsDsBlfwVLG6qy/cGejxMWAUImT+mRE
iU16oOm+LpJfRGZDVC471RglIA3rUy3ymNqj7jg1O/0AXXlk51lHoNEuX3EJEmh6
gAcYhRVQTJ3GoVJjML1NSjZQVAMtwnLuFt99Z5WNeATv4QB1+FVrp9gN3VuWj1pv
mmDnusgyCjxs8LyHJc7OGZ4sJqAB+a6uPySKa1cBTJgfuEPJ9hA8FJix+EmIJWL9
mDCZ2x2mCatQcfzeZwl2/2N/QAm/1O0tc3r2kkmgrybyIXz7UpUAqfZ36gobyE48
1N4LLhyxqzADoOvYu15Aw8T26Lt2zLPBnsVTMEKZUChzin1bx70Rr2omXwxwUajS
CRCp/MLN2dlnfCKCYsiEkwuOX8QkVPsUagL6Dk+aru9E2eeDyvdB5n7dE/UeZ6Th
9XAn1rBP+bw0PXyfr3BoXZGWbg1EmMmXkcwPUreF4P5duAZkxjn0BfnNzkzUZ55n
jLXFTdLWDHJASzrAj5QnegdLWvIveffpRcP/wWXO3giPgoDOFeZLMEDpwFDkCjv+
GfRNeqNtBEzoHRdB2ESBvfmnwMcZ0Y16hXmw5jK3WwEVNr78bZQ=
=f5rG
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Thu, Apr 04, 2024 at 03:44:40PM -0400, Demi Marie Obenour wrote:
> Should an empty service argument (qubes.Service+) always be the same as
> no argument at all (qubes.Service)? Right now, they are the same when
> coming from a VM, but not when coming from dom0: qubes.Service from dom0
> will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
> will.

I'd say they should behave the same - the "qubes.Service" call should
search for /etc/qubes-rpc/qubes.Service+ first.

- --
Best Regards,
Marek Marczykowski-Górecki

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmYPEPUACgkQ24/THMrX
1ywpTgf/ShJpgfb+JaByYYeMur5PGNqaDqwvh79QrTjPTJCh0omrw0NvWcctcQGU
o7YlEZ0eymAzF5iHDD2vG4MCwI0+xovaJrjV5LiOsauBCe5Jc1rRYTLS2aaK/Ysl
b8gCUgst5VteLseGj7sNNu3LBPLTFljWc6pPg09c3sGB7wB1VH+dmPSfeV98tWX2
m3RrHP0jLUxDcmaJyEhcuDZUAOCO0g8/mVPaDe2qtBrvawpQFela0Cp/h9aP/+LH
mgc+1n+fZC/Bg8u2AbJsadGd+bJ7MOt6Jrgcghn8rAO3Tkzc7OsZ+Nv7TBboYYe/
MQSJ/RoVT7/RCcdvl3kWK8KESov2Lw==
=0j62
-----END PGP SIGNATURE-----

[Demi Marie Obenour]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Thu, Apr 04, 2024 at 10:43:33PM +0200, Marek Marczykowski-Górecki wrote:
> On Thu, Apr 04, 2024 at 03:44:40PM -0400, Demi Marie Obenour wrote:
> > Should an empty service argument (qubes.Service+) always be the same as
> > no argument at all (qubes.Service)? Right now, they are the same when
> > coming from a VM, but not when coming from dom0: qubes.Service from dom0
> > will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
> > will.
>
> I'd say they should behave the same - the "qubes.Service" call should
> search for /etc/qubes-rpc/qubes.Service+ first.

The current code does not do this. I have a test and will write a
patch. The fix adds extra complexity, so I would like to fix the code
in dom0 to always pass an empty argument instead of no argument. Then
the next backwards-incompatible release (R5.0?) can treat a missing
argument as an error and refuse to execute the call.

If no argument is passed, should QREXEC_SERVICE_FULL_NAME reveal this,
or should it include the "+" as if an empty argument was passed?

- --
Sincerely,
Demi Marie Obenour (she/her/hers)

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmYPJXIACgkQsoi1X/+c
IsEI4hAA1OlFRwU5DONG/Y3079Z1KlaxprsxZE47ecJ5B3gQ3ZoXPkce6JjemEAn
4etsKwqhZ5JAEnq/Mdal7c4EddQtZHKAQLNNyTQNYxwmLd3fu2/J53d15T2R35s5
ftaXG1y8KQUTdgTzC6mUpQl4Xvo8vzN5h3nbiNJZSMbb/+lseXk+tispfnpTGUCu
FqPrsnJIzIT8vRYyBHS36zW0PClSu+GfARkIlIb9vFmK1Cu1CSzv7OWNOQcSUS9r
mNasTd7MBoTjXkf33u3m+YqqXMclXRsiK59MzY69jMT5ISXX8IoIqF54ypj4MINT
uKNpXzWskr7NJCy4uP8ePATJasLiQYm6Qf7y5DbCtD1/x3eK3lD/EOkV+hwhSbD4
7jwapFz1iuBey+oljVP9WCYEz2gLSOmghUi42JWX7KnXYJmyxGYG/IwDfU77tCrG
NRR4mjQKVsOu9qlLrrpAlX60vQZeQnoYoiA5+tiXlsQTHYUX3hW8ELNOm6IaCiaj
dMx1tpEUhYwwVUUgd+SCX1ohZrFWuno/jPnqYkwnkdG2ZG62TRIw+Dx0DxVkF7Um
IP+0DPuqsjLTpBP0dgRUfTKdACjcjjMVkIXguQ13ggK1KYYKNbZ8JbI8ys6k353o
HvpHR+6HVTrwEHEJsjAZT2wESPOkiCPi4WRhwgBi8hKBFe8kDks=
=nxaB
-----END PGP SIGNATURE-----

[Demi Marie Obenour]

-----BEGIN PGP SIGNED MESSAGE-----

Hash: SHA512

On Thu, Apr 04, 2024 at 10:43:33PM +0200, Marek Marczykowski-Górecki wrote:

> On Thu, Apr 04, 2024 at 03:44:40PM -0400, Demi Marie Obenour wrote:
> > Should an empty service argument (qubes.Service+) always be the same as
> > no argument at all (qubes.Service)? Right now, they are the same when
> > coming from a VM, but not when coming from dom0: qubes.Service from dom0
> > will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
> > will.
>
> I'd say they should behave the same - the "qubes.Service" call should
> search for /etc/qubes-rpc/qubes.Service+ first.

Is the "qubes.Service" call actually valid to make? dom0 certainly
makes such calls right now, but I'm wondering if that is a bug. In that
case, dom0 should be fixed to always include the "+". That can be done
in either Python (easy, but requires changing multiple callers) or in
qrexec-client itself.

Also, should socket-based services receive what was actually sent
("qubes.Service") or what was used for lookup ("qubes.Service+")? The
same goes for executable services and the QREXEC_SERVICE_FULL_NAME
environment variable. Right now I fix up QREXEC_SERVICE_FULL_NAME but
not the metadata passed to socket-based services.

- --
Sincerely,
Demi Marie Obenour (she/her/hers)

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmYQQAAACgkQsoi1X/+c
IsEUAQ/+I/7EWXa+5CyP2jlsPYH51zUPksQRMdoa7Vli+Wt7CU+Nu0egfu+bMfU1
dugQIjUAvFom0RkM2Yp1eXngoTPzCbAoikg2NSaD6b4wP9xkCDZXogkPzTnjGuld
6IZV0wuCOSZSEnFbwjWhH2IGYmXUywepy4yNUAx/3RNqelZ3JlF5kK8E1v8nuvOs
QTtHrQ6yDRq5PynMfDkkRpZRSsPS9ySiqgKoVel+Pqe1PJm8woX/KhhpMdx5fsGv
fbCuk4SR3O6qZc8xtvUUQ/kZZ4vR9lLMFkOTdQI0wR+7JKdd4IQwJB1YYqAGhZO+
Y2pergpB7dBkeTht8KyuFUvBFSLjKD66PZV2jll+OMQkAZ7U+0sswL8WykXoiSGr
2BSZsG1o+L9V9u21Ncbawfr7dWUFH8ioO0xmJAQ3SxgrhxyerokfYICa7AR3XGMr
0Rwafl4HQsr7VvNGMYeeJ6CdeX90MxD3XVc2OxBd7b/Ak9nYk7CUoE9en3ez1Db8
fp9j81MYe8udbdgDyUiAuWxBmBeJVN7ykA+sOvBwuo6lw346E67mtBPV6d0BgEcC
lGHmA/FfGon7Ah/hHXqMstlcC8haw9gTZmBZbslwAyxSZTJ8zVKH8Lr4pGj5JbGp
+DK148QVIyOCe50ifxOttqw5GGxC5I7zmHYqlnThZNX7pLWAAPY=
=Qlec
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

On Fri, Apr 05, 2024 at 02:16:32PM -0400, Demi Marie Obenour wrote:
> On Thu, Apr 04, 2024 at 10:43:33PM +0200, Marek Marczykowski-Górecki wrote:
> > On Thu, Apr 04, 2024 at 03:44:40PM -0400, Demi Marie Obenour wrote:
> > > Should an empty service argument (qubes.Service+) always be the same as
> > > no argument at all (qubes.Service)? Right now, they are the same when
> > > coming from a VM, but not when coming from dom0: qubes.Service from dom0
> > > will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
> > > will.
> >
> > I'd say they should behave the same - the "qubes.Service" call should
> > search for /etc/qubes-rpc/qubes.Service+ first.
>
> Is the "qubes.Service" call actually valid to make? dom0 certainly
> makes such calls right now, but I'm wondering if that is a bug.

That's a very good question. The argument (including "+") used to be
completely optional, and we did distinguished "no argument" from "empty
argument" in some places. But it was confusing (especially when writing
policy for such cases) and was removed in R4.0 for VM-originating calls.
For dom0-originating calls, policy concern doesn't apply, as those
aren't going through it. But for consistency it might be a good idea to
always include "+" anyway.

> In that
> case, dom0 should be fixed to always include the "+". That can be done
> in either Python (easy, but requires changing multiple callers) or in
> qrexec-client itself.

I'd say it's safer to change in Python. qrexec-client normally doesn't
parse the command it sends, and changing the command there may lead to
some unintended side effects or bugs. But also changing there would make
it quite hard to make some exception if turned out necessary.
On the Python side, it isn't that many places - one in core-admin, one
in core-admin-client and one in core-qrexec. Plus a few in various
tests. There are also not that many manual qrexec-client calls, I see
them in app-linux-input-proxy and gui-daemon.

Anyway, technically this is an API change and as such I'd avoid doing it
as R4.2 update. Linux agent should have no issue with this, but other
implementations may. For example I see qubes-mirage-firewall is looking
for "QUBESRPC qubes.SetDateTime dom0" explicitly:
https://github.com/mirage/qubes-mirage-firewall/blob/main/command.ml#L25

> Also, should socket-based services receive what was actually sent
> ("qubes.Service") or what was used for lookup ("qubes.Service+")? The
> same goes for executable services and the QREXEC_SERVICE_FULL_NAME
> environment variable. Right now I fix up QREXEC_SERVICE_FULL_NAME but
> not the metadata passed to socket-based services.

I'd say socket-based services should receive what was actually sent.

- --
Best Regards,
Marek Marczykowski-Górecki

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmYQiUIACgkQ24/THMrX
1yynWQf/QpvzpZ3kCiC1nMidobCjAY/rhtrramEpAuCpceEB7JxHj9vAfnJlify8
xag7PgRiIIZ31+eKxZrYfoW/QBOPiX9tU3/bhtUocjXqS6l0UDiSIA9UMDDqAl0w
UJainDUIdzRtH/EPXWjcr48FM3jZmKQPWOAmXteNZfWcmYWH6KtwEaHkyxZ2lLM0
zLcDnWrEykS8DbZpTbClxQ8S4vq8BcYKH1ULMAurmNE3sToJC858Sf4bp8QPvfLN
GU2ndsSMUsiHB/1mc7cazjD4DPTqpmy+Od98Cndd08JMpoLPZ6dqqwLh5C8hTQ/W
FcDghMPqLIutq4p35O2x2cqE13kOGQ==
=MM1I
-----END PGP SIGNATURE-----

[Demi Marie Obenour]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

On Sat, Apr 06, 2024 at 01:29:06AM +0200, Marek Marczykowski-Górecki wrote:
> On Fri, Apr 05, 2024 at 02:16:32PM -0400, Demi Marie Obenour wrote:
> > On Thu, Apr 04, 2024 at 10:43:33PM +0200, Marek Marczykowski-Górecki wrote:
> > > On Thu, Apr 04, 2024 at 03:44:40PM -0400, Demi Marie Obenour wrote:
> > > > Should an empty service argument (qubes.Service+) always be the same as
> > > > no argument at all (qubes.Service)? Right now, they are the same when
> > > > coming from a VM, but not when coming from dom0: qubes.Service from dom0
> > > > will not search for /etc/qubes-rpc/qubes.Service+, but qubes.Service+
> > > > will.
> > >
> > > I'd say they should behave the same - the "qubes.Service" call should
> > > search for /etc/qubes-rpc/qubes.Service+ first.
> >
> > Is the "qubes.Service" call actually valid to make? dom0 certainly
> > makes such calls right now, but I'm wondering if that is a bug.
>
> That's a very good question. The argument (including "+") used to be
> completely optional, and we did distinguished "no argument" from "empty
> argument" in some places. But it was confusing (especially when writing
> policy for such cases) and was removed in R4.0 for VM-originating calls.
> For dom0-originating calls, policy concern doesn't apply, as those
> aren't going through it. But for consistency it might be a good idea to
> always include "+" anyway.

I'm inclined to agree.

> > In that
> > case, dom0 should be fixed to always include the "+". That can be done
> > in either Python (easy, but requires changing multiple callers) or in
> > qrexec-client itself.
>
> I'd say it's safer to change in Python. qrexec-client normally doesn't
> parse the command it sends, and changing the command there may lead to
> some unintended side effects or bugs. But also changing there would make
> it quite hard to make some exception if turned out necessary.
> On the Python side, it isn't that many places - one in core-admin, one
> in core-admin-client and one in core-qrexec. Plus a few in various
> tests. There are also not that many manual qrexec-client calls, I see
> them in app-linux-input-proxy and gui-daemon.

Changing it in qrexec-client turned out to be quite easy, but it also
raises concerns about the same command being parsed different ways (due
to e.g. version skew), and so I decided to not go that route. It also
indeed makes an exception impossible to add without some sort of
out-of-band signalling, such as a new command-line option.

> Anyway, technically this is an API change and as such I'd avoid doing it
> as R4.2 update. Linux agent should have no issue with this, but other
> implementations may. For example I see qubes-mirage-firewall is looking
> for "QUBESRPC qubes.SetDateTime dom0" explicitly:
> https://github.com/mirage/qubes-mirage-firewall/blob/main/command.ml#L25

By "this", do you mean changing the API calls made by dom0 to include
the "+", or changing the way the Linux agent interprets calls that do
not include the "+"?

> > Also, should socket-based services receive what was actually sent
> > ("qubes.Service") or what was used for lookup ("qubes.Service+")? The
> > same goes for executable services and the QREXEC_SERVICE_FULL_NAME
> > environment variable. Right now I fix up QREXEC_SERVICE_FULL_NAME but
> > not the metadata passed to socket-based services.
>
> I'd say socket-based services should receive what was actually sent.

That makes sense, and is actually simpler to implement. What about
QREXEC_SERVICE_FULL_NAME? That's the analog for executable services.
By analogy to socket-based services, it makes sense to pass what was
actually sent here too, but I currently add a trailing "+" if none is
present.

- --
Sincerely,
Demi Marie Obenour (she/her/hers)

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQIzBAEBCgAdFiEEdodNnxM2uiJZBxxxsoi1X/+cIsEFAmYQlpUACgkQsoi1X/+c
IsHoUg/7B2ZBotLyXRL6teNFabpvDYmprficx8aYURWS/vUIyN/xuCdHBipHFeXi
x9Dl5vsPrrL/+2FAaTGd0dfe8/wc4b7nsev04ynUYTUZ0w5wTnLgA5yABrpct9Hl
TBHoIx6AfWrEZ6RzdPQaCnkjDveuTTSrblpRJ2AKi5CA1BJaQg3HHNL3+75akg8R
KFk0xFyVAcHyeF54FzCokdUcXbecY7LqXvZ0c1LttcAcn1vnBFPuwcUxFc5stIJk
cUOOmOcOAj+NRnPQyYfnbkDNTLVxuvCMmkyrmPJbiVoruHp9NRXIObpG+lI21bHs
rj6eeYM+XakP7wYc38uJC3UmevlPkj2+ctyZc9h9cfLVs0wL4N2uImHPlnvQKTmR
Fz8jCu68D3EHkcXZhqtc8PcVoSq8iR8wgzr4SoMIAlsZTDX0ZgbdGhxuIBSSvtb0
rr0ji8DaT5hwVUERWLFZChX9fZdjtA2KSkT4tTQHlYdenI+9L+jc0LT4fcZVbj2i
gvBRV6NpiCTTQnuUOxsApNF8Qe7fjMH+Y0BQPfRxdthBlNcIMcgV6qyTwsExfeH1
8DsgqfuA0H++Cs9YeToV5yhPWvYvuQPiSzp9WLZLMXN4aO5gPhoun4JS0J06J0TC
8Fy6pRAcwktir6y+69g68sKLyZz8iaXKi8xhnF5qxcM02U63mRc=
=URTR
-----END PGP SIGNATURE-----

[Marek Marczykowski-Górecki]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

I mean changing how dom0 makes calls.

> > > Also, should socket-based services receive what was actually sent
> > > ("qubes.Service") or what was used for lookup ("qubes.Service+")? The
> > > same goes for executable services and the QREXEC_SERVICE_FULL_NAME
> > > environment variable. Right now I fix up QREXEC_SERVICE_FULL_NAME but
> > > not the metadata passed to socket-based services.
> >
> > I'd say socket-based services should receive what was actually sent.
>
> That makes sense, and is actually simpler to implement. What about
> QREXEC_SERVICE_FULL_NAME? That's the analog for executable services.
> By analogy to socket-based services, it makes sense to pass what was
> actually sent here too, but I currently add a trailing "+" if none is
> present.

This also should IMO include the name as was sent to the qrexec agent.

- --
Best Regards,
Marek Marczykowski-Górecki

Invisible Things Lab
-----BEGIN PGP SIGNATURE-----

iQEzBAEBCAAdFiEEhrpukzGPukRmQqkK24/THMrX1ywFAmYQmAgACgkQ24/THMrX
1yw9fAf/TttfORQja9JZvDE0HELVhWDNKf3ZBRIZ8Q8DJJT9p9AM5/A/2Ifp20k7
csQ43n5YbN03b7Y7wkhJFgHpL3WYiGH94gg7hqr7L8U4jba7Q3Rn1C7KhMdT/iou
fRQCwmGHgh6UJiwPjlhnTU+Jdjv8PYhd4SmPcu7G/zlHOS1Ukim6W2XtfYe8efgP
zMOwGB3pjCaZMyZKWj+QTP56IBGzaHSwsBJcmF3Ng27Z84iu2I3+b6zPucm52Ab+
oPPiW3w9ABDDZ00Wqydhf+TF3zHUMRqxCM9oYdPFGZbcEqMAOvG9xvq2myvHGVQY
xL6rFzIlwnFDSkn6ZxwNZ6j4rFXBtA==
=fDP4
-----END PGP SIGNATURE-----