qubes-core-vm-systemd upgrade disabled my sshd

[Matt McCutchen]

Today a script that I use to transfer data between AppVMs broke because
the sshd service in the AppVMs, which I had manually enabled, had been
disabled. It looks like this was done by the postinstall scriptlet of
qubes-core-vm-systemd when I upgraded that package.

Assuming enabling sshd is considered a legitimate configuration, there
should be a way to make it not break on package upgrades. Wrapping the
scriptlet in "if [ $1 == 1 ]" would fix my case but would mean that if a
new service is added to Fedora enabled by default and is added to the
qubes-core-vm-systemd DISABLE_SERVICES list, it would not get disabled
on upgrade.

How many other services are there in DISABLE_SERVICES that users can
reasonably manually enable? If there are few and we don't expect more
to be added, one compromise would be to put only those services inside
"if [ $1 == 1 ]".

Another idea is to have qubes-core-vm-systemd install a preset file
in /usr/lib/systemd/system-preset and then have the scriptlet use
"systemd preset" instead of "systemd disable". That is strictly better
than the current approach, and it at least makes it possible for users
like me to permanently enable a service by creating a higher priority
preset file rather than just running "systemctl enable". Of course,
users would need to be aware of the need to do that. (FWIW, I'd argue
that systemd should never have supported the "enable/disable" command in
its current form, in favor of preset files, precisely to avoid problems
like the one I saw today, but I don't see myself campaigning to do
anything about that now.)

Thoughts? Once an approach is approved by the maintainers, I will be
happy to provide a patch.

Separately, I may look into solving the original problem of opening a
bidirectional stream to a command running in another AppVM via Qubes RPC
instead of sshd. After reading
https://groups.google.com/d/topic/qubes-devel/U259lZZYDJI/discussion, I
think what I'd do is write an RPC service that starts "sshd -i" with
dummy authentication parameters and then rig /etc/ssh/ssh_config so that
a command like "ssh my-other-appvm.qubes git-receive-pack ..." results
in the correct ProxyCommand. This gives us all the features of ssh
without having to reimplement the ones we want one at a time; of course,
some of them are dangerous to the client, as the type of people who use
Qubes will likely know. Maintainers, would you be interested in
including this functionality in qubes-core-vm?

Matt

[Marek Marczykowski-Górecki]

The current script does not use preset file because it wasn't available
at the time of adding systemd support to qubes-core-vm package. But
since it is supported now, IMO it is good idea for using it. "systemctl
preset" should be called at both install and upgrade, as the list
changes over the time (see #909 for example). This will
greatly reduce that %post script, which (always?) good.

One thing to take care of - when Qubes provides overridden .service file
(NetworkManager, crond, cups etc), it is required to disable+enable the
service, to change the symlink from /usr/lib/systemd/system/... to
/etc/systemd/system/... This is currently implemented as
installOverridenServices macro called from triggerss. So this part needs
to be left intact.

> Separately, I may look into solving the original problem of opening a
> bidirectional stream to a command running in another AppVM via Qubes RPC
> instead of sshd. After reading
> https://groups.google.com/d/topic/qubes-devel/U259lZZYDJI/discussion, I
> think what I'd do is write an RPC service that starts "sshd -i" with
> dummy authentication parameters and then rig /etc/ssh/ssh_config so that
> a command like "ssh my-other-appvm.qubes git-receive-pack ..." results
> in the correct ProxyCommand. This gives us all the features of ssh
> without having to reimplement the ones we want one at a time; of course,
> some of them are dangerous to the client, as the type of people who use
> Qubes will likely know. Maintainers, would you be interested in
> including this functionality in qubes-core-vm?

I don't think sshd is a good idea here - sounds like an overkill.
Perhaps it would be enough to just change /usr/lib/qubes/qrun-in-vm, to
use socat instead of unidirectional cat. Something like:
socat stdio "fd:$SAVED_FD_0!!fd:$SAVED_FD_1"

Once it will work for qvm-run, you can add your own service, which will
call predefined command (instead of anything as in sshd or qvm-run),
then for the git case, you can use git-remote-ext(1) to call
qrexec-client-vm with proper arguments. I'm not sure about git
internals, but it looks like in the worst case, you'll need 3 services:
git-receive-pack, git-upload-pack and git-upload-archive.

Regarding qubes-core-vm - I don't think it is a good place for such
specific implementations (with exception for making qvm-run
bi-directional). But if you succeed with "git over Qubes RPC", it would
be nice to have some instructions how to achieve it. You can just sent
them here (to be copied/linked to our wiki).

--
Best Regards,
Marek Marczykowski-Górecki
Invisible Things Lab
A: Because it messes up the order in which people normally read text.
Q: Why is top-posting such a bad thing?

[Matt McCutchen]

On Tue, 2015-02-03 at 15:52 +0100, Marek Marczykowski-Górecki wrote:
> The current script does not use preset file because it wasn't available
> at the time of adding systemd support to qubes-core-vm package. But
> since it is supported now, IMO it is good idea for using it. "systemctl
> preset" should be called at both install and upgrade, as the list
> changes over the time (see #909 for example). This will
> greatly reduce that %post script, which (always?) good.

FWIW, not really: the only changes are to replace "disable" with
"preset" and remove the "forcibly disable" case (it's unnecessary
according to my test with systemd-208-29.fc20.x86_64, although this
reduction is arguably unrelated to the use of a present file).

> One thing to take care of - when Qubes provides overridden .service file
> (NetworkManager, crond, cups etc), it is required to disable+enable the
> service, to change the symlink from /usr/lib/systemd/system/... to
> /etc/systemd/system/... This is currently implemented as
> installOverridenServices macro called from triggerss. So this part needs
> to be left intact.

Sounds good, I'll probably get to this in a few days.

> > Separately, I may look into solving the original problem of opening a
> > bidirectional stream to a command running in another AppVM via Qubes RPC
> > instead of sshd. After reading
> > https://groups.google.com/d/topic/qubes-devel/U259lZZYDJI/discussion, I
> > think what I'd do is write an RPC service that starts "sshd -i" with
> > dummy authentication parameters and then rig /etc/ssh/ssh_config so that
> > a command like "ssh my-other-appvm.qubes git-receive-pack ..." results
> > in the correct ProxyCommand. This gives us all the features of ssh
> > without having to reimplement the ones we want one at a time; of course,
> > some of them are dangerous to the client, as the type of people who use
> > Qubes will likely know. Maintainers, would you be interested in
> > including this functionality in qubes-core-vm?
>
> I don't think sshd is a good idea here - sounds like an overkill.
> Perhaps it would be enough to just change /usr/lib/qubes/qrun-in-vm, to
> use socat instead of unidirectional cat. Something like:
> socat stdio "fd:$SAVED_FD_0!!fd:$SAVED_FD_1"

What specifically do you see as the downside of ssh? Attack surface on
the client (which I suppose would be consistent with the philosophy of
the rest of Qubes)? Performance? Ease of use?

Replacing the cat in /usr/lib/qubes/qrun-in-vm by socat is the easy
part. The less trivial part is delimiting the command from the standard
input. A newline might be adequate for reasonable users; a null byte
might be a little better. I'll try the null byte.

This is probably fine for my git use case. For more complex scripting,
users may very reasonably want separate stderr and an exit code. Dom0's
qvm-run does those somehow. It might be reasonable to replicate that
implementation; otherwise, I've used the ssh solution before and I think
it's quite convenient for users who are willing to have the ssh client
in their TCB. I suppose we can wait until someone asks.

> Once it will work for qvm-run, you can add your own service, which will
> call predefined command (instead of anything as in sshd or qvm-run),
> then for the git case, you can use git-remote-ext(1) to call
> qrexec-client-vm with proper arguments. I'm not sure about git
> internals, but it looks like in the worst case, you'll need 3 services:
> git-receive-pack, git-upload-pack and git-upload-archive.
>
> Regarding qubes-core-vm - I don't think it is a good place for such
> specific implementations (with exception for making qvm-run
> bi-directional). But if you succeed with "git over Qubes RPC", it would
> be nice to have some instructions how to achieve it. You can just sent
> them here (to be copied/linked to our wiki).

At this time, I'm willing to give the client AppVM the equivalent of
full VMShell access to the target AppVM and am not interested in setting
up services for individual commands. Either way, I'm of the opinion
that it's appropriate for the Qubes wiki to describe how to allow
execution of either arbitrary commands or a specific command, but
application-specific options to use an alternate remote shell are out of
scope.

Matt

[Marek Marczykowski-Górecki]

On Wed, Feb 04, 2015 at 09:25:47PM -0500, Matt McCutchen wrote:
> On Tue, 2015-02-03 at 15:52 +0100, Marek Marczykowski-Górecki wrote:
> > The current script does not use preset file because it wasn't available
> > at the time of adding systemd support to qubes-core-vm package. But
> > since it is supported now, IMO it is good idea for using it. "systemctl
> > preset" should be called at both install and upgrade, as the list
> > changes over the time (see #909 for example). This will
> > greatly reduce that %post script, which (always?) good.
>
> FWIW, not really: the only changes are to replace "disable" with
> "preset" and remove the "forcibly disable" case (it's unnecessary
> according to my test with systemd-208-29.fc20.x86_64, although this
> reduction is arguably unrelated to the use of a present file).

Are you sure about that "forcibly disable"? This is meant to disable
services "static" service (which can't be disabled by "disable"
command). For example fedora-autorelabel.service.

Yes, adding ssh to TCB. Also some configuration could be non-trivial
- especially when someone want to limit the usage to single command(s)
(in that case you have also sshd in your TCB).

> Replacing the cat in /usr/lib/qubes/qrun-in-vm by socat is the easy
> part. The less trivial part is delimiting the command from the standard
> input. A newline might be adequate for reasonable users; a null byte
> might be a little better. I'll try the null byte.

In case of git, I can't think of any cases requiring anything special.
But in general case, you're right. Also parameters separation/quoting
can be quite tricky (especially in shell)...

> This is probably fine for my git use case. For more complex scripting,
> users may very reasonably want separate stderr and an exit code. Dom0's
> qvm-run does those somehow. It might be reasonable to replicate that
> implementation;

In R2, qrexec (in VM-VM connection) pass only stdin/stdout. stderr is
logged to syslog. There is also no feedback on exit code. Those
information information are available only in dom0->VM connection.

In R3, qrexec protocol is made much more symmetric and all that
information are present.

> otherwise, I've used the ssh solution before and I think
> it's quite convenient for users who are willing to have the ssh client
> in their TCB. I suppose we can wait until someone asks.
>
> > Once it will work for qvm-run, you can add your own service, which will
> > call predefined command (instead of anything as in sshd or qvm-run),
> > then for the git case, you can use git-remote-ext(1) to call
> > qrexec-client-vm with proper arguments. I'm not sure about git
> > internals, but it looks like in the worst case, you'll need 3 services:
> > git-receive-pack, git-upload-pack and git-upload-archive.
> >
> > Regarding qubes-core-vm - I don't think it is a good place for such
> > specific implementations (with exception for making qvm-run
> > bi-directional). But if you succeed with "git over Qubes RPC", it would
> > be nice to have some instructions how to achieve it. You can just sent
> > them here (to be copied/linked to our wiki).
>
> At this time, I'm willing to give the client AppVM the equivalent of
> full VMShell access to the target AppVM and am not interested in setting
> up services for individual commands. Either way, I'm of the opinion
> that it's appropriate for the Qubes wiki to describe how to allow
> execution of either arbitrary commands or a specific command, but
> application-specific options to use an alternate remote shell are out of
> scope.
>
> Matt
>

[Matt McCutchen]

On Thu, 2015-02-05 at 04:06 +0100, Marek Marczykowski-Górecki wrote:
> On Wed, Feb 04, 2015 at 09:25:47PM -0500, Matt McCutchen wrote:
> > On Tue, 2015-02-03 at 15:52 +0100, Marek Marczykowski-Górecki wrote:
> > > The current script does not use preset file because it wasn't available
> > > at the time of adding systemd support to qubes-core-vm package. But
> > > since it is supported now, IMO it is good idea for using it. "systemctl
> > > preset" should be called at both install and upgrade, as the list
> > > changes over the time (see #909 for example). This will
> > > greatly reduce that %post script, which (always?) good.
> >
> > FWIW, not really: the only changes are to replace "disable" with
> > "preset" and remove the "forcibly disable" case (it's unnecessary
> > according to my test with systemd-208-29.fc20.x86_64, although this
> > reduction is arguably unrelated to the use of a present file).
>
> Are you sure about that "forcibly disable"? This is meant to disable
> services "static" service (which can't be disabled by "disable"
> command). For example fedora-autorelabel.service.

Indeed, I misread the scriptlet as only removing symlinks, which
"systemctl disable" will do even for static services. What the
scriptlet actually does is now known as "systemctl mask", so that's what
I'll change it to use. Thanks for catching this.

> > What specifically do you see as the downside of ssh? Attack surface on
> > the client (which I suppose would be consistent with the philosophy of
> > the rest of Qubes)? Performance? Ease of use?
>
> Yes, adding ssh to TCB. Also some configuration could be non-trivial
> - especially when someone want to limit the usage to single command(s)
> (in that case you have also sshd in your TCB).

FTR: I intended the ssh solution mainly for the case of unlimited
commands. It uses its own configuration file and in the best case does
not require any changes, though there are probably various non-default
things one could do to an AppVM that would break it in the absence of
configuration changes.

Matt

[Marek Marczykowski-Górecki]

On Thu, Feb 05, 2015 at 02:48:06AM -0500, Matt McCutchen wrote:
> On Thu, 2015-02-05 at 04:06 +0100, Marek Marczykowski-Górecki wrote:
> > On Wed, Feb 04, 2015 at 09:25:47PM -0500, Matt McCutchen wrote:
> > > On Tue, 2015-02-03 at 15:52 +0100, Marek Marczykowski-Górecki wrote:
> > > > The current script does not use preset file because it wasn't available
> > > > at the time of adding systemd support to qubes-core-vm package. But
> > > > since it is supported now, IMO it is good idea for using it. "systemctl
> > > > preset" should be called at both install and upgrade, as the list
> > > > changes over the time (see #909 for example). This will
> > > > greatly reduce that %post script, which (always?) good.
> > >
> > > FWIW, not really: the only changes are to replace "disable" with
> > > "preset" and remove the "forcibly disable" case (it's unnecessary
> > > according to my test with systemd-208-29.fc20.x86_64, although this
> > > reduction is arguably unrelated to the use of a present file).
> >
> > Are you sure about that "forcibly disable"? This is meant to disable
> > services "static" service (which can't be disabled by "disable"
> > command). For example fedora-autorelabel.service.
>
> Indeed, I misread the scriptlet as only removing symlinks, which
> "systemctl disable" will do even for static services. What the
> scriptlet actually does is now known as "systemctl mask", so that's what
> I'll change it to use. Thanks for catching this.

There was a reason why I didn't used "systemctl mask", but I don't
remember details...

[Matt McCutchen]

The patch to use a preset file for DISABLE_SERVICES is attached. The
scriptlet is doing many other things that I wonder if users might want
to override under some circumstances, but I don't want to be responsible
for deciding how override mechanisms for them should work, so for now
I'm only addressing the case that affected me.

Separately, I made qvm-run bidirectional, and that patch is attached. I
wanted to achieve the EOF behavior that people expect for remote command
execution, namely: EOF on local stdin is propagated but qvm-run waits to
receive any more remote output, while when the command exits (detected
as EOF on remote stdout), qvm-run exits without reading any more local
input. I couldn't find a clean way to achieve this with socat, or even
with multiple cat subprocesses in bash, so I switched to Python. Git
remote services appear to work regardless of the EOF behavior, and the
same may be true of many of the other types of remote commands users
would want to run, but I still wanted to try to get the EOF behavior
right if possible while I'm here.

If you're not comfortable with the reliance on bash's behavior of
reading one character at a time (undocumented but IMO unlikely to
change, precisely because people like us are relying on it) to delimit
the command from the input using qubes.VMShell, I can change qvm-run to
use a new Qubes RPC service instead.

Matt

[Marek Marczykowski-Górecki]

On Sun, Feb 08, 2015 at 09:49:40PM -0500, Matt McCutchen wrote:
> The patch to use a preset file for DISABLE_SERVICES is attached. The
> scriptlet is doing many other things that I wonder if users might want
> to override under some circumstances, but I don't want to be responsible
> for deciding how override mechanisms for them should work, so for now
> I'm only addressing the case that affected me.

Looks good with one exception: it should be applied only on Fedora (at
least in its current shape) - other distributions can have different
unit names (especially fedora-*). But currently install scripts for
Debian and Archlinux use exactly the same list, including
Fedora-specific names... As the other scripts needs some changes anyway,
your patch shouldn't break them (hopefully).

For now I'll apply the patch to R3 code (master branch) and then after
some testing will eventually backport it to R2 (it applies cleanly to both
branches).

> Separately, I made qvm-run bidirectional, and that patch is attached. I
> wanted to achieve the EOF behavior that people expect for remote command
> execution, namely: EOF on local stdin is propagated but qvm-run waits to
> receive any more remote output, while when the command exits (detected
> as EOF on remote stdout), qvm-run exits without reading any more local
> input. I couldn't find a clean way to achieve this with socat, or even
> with multiple cat subprocesses in bash, so I switched to Python. Git
> remote services appear to work regardless of the EOF behavior, and the
> same may be true of many of the other types of remote commands users
> would want to run, but I still wanted to try to get the EOF behavior
> right if possible while I'm here.

This is a limitation in qrexec protocol itself. Which most likely will
be fixed in R3. Some details are in commit message of test case for
this:
http://git.qubes-os.org/?p=marmarek/core-admin.git;a=commit;h=256e35a62c7bce467ceda5288c160041458e0fa6

> If you're not comfortable with the reliance on bash's behavior of
> reading one character at a time (undocumented but IMO unlikely to
> change, precisely because people like us are relying on it) to delimit
> the command from the input using qubes.VMShell, I can change qvm-run to
> use a new Qubes RPC service instead.

I think it is ok. Applied (same as the other path - to R3 for now).

[Matt McCutchen]

On Mon, 2015-02-09 at 06:40 +0100, Marek Marczykowski-Górecki wrote:
> Looks good with one exception: it should be applied only on Fedora (at
> least in its current shape) - other distributions can have different
> unit names (especially fedora-*). But currently install scripts for
> Debian and Archlinux use exactly the same list, including
> Fedora-specific names... As the other scripts needs some changes anyway,
> your patch shouldn't break them (hopefully).

Right, that problem is orthogonal to my change.

> For now I'll apply the patch to R3 code (master branch) and then after
> some testing will eventually backport it to R2 (it applies cleanly to both
> branches).

Thanks for getting this (and qvm-run) fixed for the future. I guess I
still need to do something to keep my system working until the fixes are
released to R2: probably continue to use the modified packages I built
for testing.

> > Separately, I made qvm-run bidirectional, and that patch is attached. I
> > wanted to achieve the EOF behavior that people expect for remote command
> > execution, namely: EOF on local stdin is propagated but qvm-run waits to
> > receive any more remote output, while when the command exits (detected
> > as EOF on remote stdout), qvm-run exits without reading any more local
> > input. I couldn't find a clean way to achieve this with socat, or even
> > with multiple cat subprocesses in bash, so I switched to Python. Git
> > remote services appear to work regardless of the EOF behavior, and the
> > same may be true of many of the other types of remote commands users
> > would want to run, but I still wanted to try to get the EOF behavior
> > right if possible while I'm here.
>
> This is a limitation in qrexec protocol itself. Which most likely will
> be fixed in R3. Some details are in commit message of test case for
> this:
> http://git.qubes-os.org/?p=marmarek/core-admin.git;a=commit;h=256e35a62c7bce467ceda5288c160041458e0fa6

I guess you're talking about the fact that for processes executed in a
VM by qrexec-agent, su holds a reference to the stdout pipe, preventing
EOF from being detected until the process exits? I'm not worried about
this violating user expectations because in my tests, ssh behaved the
same way. (I haven't investigated why ssh behaved that way.) The case
I cared about is the analogue of test_052_qrexec_vm_service_eof, with
the additional possibility of the remote process writing to stdout after
it receives EOF on stdin. I still couldn't treat the other case
symmetrically because if the local stdin is the terminal, then after the
remote process exits, qvm-run would continue to wait indefinitely to
send more input, which is not what users expect.

Matt

[Marek Marczykowski-Górecki]

Exactly.

> I'm not worried about
> this violating user expectations because in my tests, ssh behaved the
> same way. (I haven't investigated why ssh behaved that way.) The case
> I cared about is the analogue of test_052_qrexec_vm_service_eof, with
> the additional possibility of the remote process writing to stdout after
> it receives EOF on stdin. I still couldn't treat the other case
> symmetrically because if the local stdin is the terminal, then after the
> remote process exits, qvm-run would continue to wait indefinitely to
> send more input, which is not what users expect.

Ah, I see. This should just work on R3, as the new protocol sends exit
status to the source VM (in R2 it is only the case if connection
originates from dom0).