search

QubesOS / qubes-issues

The Qubes OS Project issue tracker
https://www.qubes-os.org/doc/issue-tracking/
541 stars 48 forks source link

move bind-dirs to its own systemd unit file #5256

Open adrelanos opened 5 years ago

adrelanos commented 5 years ago 
After=qubes-sysinit.service dev-xvdb.device
DefaultDependencies=no
Before=local-fs.target rw.mount home.mount qubes-gui-agent.service

The issue with mount-dirs.sh is that it calls /usr/lib/qubes/init/bind-dirs.sh which could execute malicious code through /rw/config/qubes-bind-dirs.d which during a previous boot could have used to place a malicious malware hook. That can be fixed by moving bind-dirs into its own systemd unit file.

Reason: That would help to allow adding a systemd unit into the middle such as https://github.com/tasket/Qubes-VM-hardening or similar.

Related: https://github.com/tasket/Qubes-VM-hardening/issues/34

DemiMarie commented 2 months ago

I wonder if it would be best to put at least some of this into the initramfs. That would avoid some potentially nasty race conditions.

marmarek commented 2 months ago

I wonder if it would be best to put at least some of this into the initramfs. That would avoid some potentially nasty race conditions.

I don't think it's a good idea. In initramfs you don't have access to a some of the config files, root fs may be mounted still read only and most importantly, /rw isn't mounted yet, so you don't have what to bind mount. And no, moving /rw mount to initramfs is a bad idea (at the very least, would make the issue in this very ticket even harder to solve).

DemiMarie commented 2 months ago

And no, moving /rw mount to initramfs is a bad idea (at the very least, would make the issue in this very ticket even harder to solve).

Good catch!

adrelanos commented 2 months ago

I wonder if it would be best to put at least some of this into the initramfs. That would avoid some potentially nasty race conditions.

I like the idea.

Disadvantage: Getting initramfs-generator specific.

In practice, it would probably need to be implemented for both, initramfs-tools and dracut.

In initramfs you don't have access to a some of the config files

This shouldn't be an issue. All initramfs-generators support configuration files available from the root disk. This wasn't even hard to implement. For example, we implemented something remotely similar (configuration files on the root disk, but needed at initramfs time) in security-misc:

An initramfs hook sets the sysctl values in /etc/sysctl.conf and /etc/sysctl.d before init is executed so sysctl hardening is enabled as early as possible.

Implementation can be seen here: https://github.com/Kicksecure/security-misc/tree/master/etc/initramfs-tools

Reasonable implementation effort and always has been stable.

marmarek commented 2 months ago

This shouldn't be an issue. All initramfs-generators support configuration files available from the root disk.

That doesn't work for dom0-provided kernel+initramfs.

adrelanos commented 2 months ago

Indeed. So getting,

done would be a prerequisite. Then this should be doable.

marmarek commented 2 months ago

Still, I think think moving bind-dirs (and by extension, the whole /rw setup, including mkfs/fsck etc) to initramfs is a very bad idea. Initramfs should do the bare minimum necessary to start init from the root filesystem and then switch to the normal init. Doing too much in initramfs is asking for troubles. Just a couple of them:

There are likely more.