christgau / wsdd

A Web Service Discovery host daemon.
MIT License
841 stars 99 forks source link

Systemd service file DynamicUser option with --chroot #109

Closed mrplumber closed 2 years ago

mrplumber commented 3 years ago

Since User and Group options in wsdd.service had been replaced with DynamicUser option, wouldn't it make sense to replace ;ExecStartPre=/usr/bin/install -d -o nobody -g nobody -m 0700 /run/wsdd/chroot with ;ExecStartPre=/usr/bin/install -d -o wsdd -g wsdd -m 0700 /run/wsdd/chroot aswell? That's what worked for me on my Debian stable machine.

christgau commented 3 years ago

Indeed, it would make sense, IMO. If I understand the systemd.exec man page correctly, then the username of a service or unit having the DynamicUser (see #107) setting enabled is derived from the unit name. Thus, in case someone chooses a different unit name, the the ExecStartPre command may fail due to the hardcoded user/group. Therefore, it might be even better to specify the user and group to be wsdd plus having the DynamicUser directive active. This would ensure that the user/group used for execution and the PreStart command match regardless of the unit name. If someone wants to change the user it's more obvious what needs to be changed.

WDYT?

mrplumber commented 3 years ago

I agree. I tested your suggestion (with renamed systemd unit) on my machine and it seems to work as you described. By the way, according to this article DynamicUser feature already enables process isolation which can be further fine-tuned with other relevant systemd options, so I'm not sure if it makes sense to run with DynamicUser and --chroot options both enabled.

christgau commented 3 years ago

I also tested DynamicUser and also read through the documentation. DynamicUser, or actually the implied ProtectSystem=strict, does not isolate the started process from system's root file system but makes most of it read-only. Opposite, chroot will make the process see only what is on the new root directory. In case of wsdd and the provided unit file, this is intentionally an empty directory, because nothing is required for most operation use-cases. If something really bad is happening, an attacker may see nothing in the filesystem as long as he not able to escape the chroot. Therefore, it makes sense to have both enabled. In case, an escape is successful the filesystem is read-only, at least - thanks to the DynamicUser directive.

Note: Using an empty directory as root will only work after the Python interpreter has been started and the imports are processed. Chroot'ing to an empty directory from the systemd unit file is more difficult if not possible at all.

mrplumber commented 3 years ago

I get an error when trying to start the wsdd service with those contents of wsdd.service:

[Unit]
Description=Web Services Dynamic Discovery host daemon
; Start after the network has been configured
After=network-online.target
Wants=network-online.target
; It makes sense to have Samba running when wsdd starts, but is not required
Wants=smb.service

[Service]
Type=simple
ExecStart=/usr/local/bin/wsdd --shortlog --interface br0 --chroot /run/wsdd/chroot
DynamicUser=yes
; Replace those with an unprivledged user/group that matches your environment,
; like nobody/nogroup or daemon:daemon or a dedicated user for wsdd
User=wsdd
Group=wsdd
; The following lines can be used for a chroot execution of wsdd.
; Also append '--chroot /run/wsdd/chroot' to ExecStart to enable chrooting
AmbientCapabilities=CAP_SYS_CHROOT
ExecStartPre=/usr/bin/install -d -o wsdd -g wsdd -m 0700 /run/wsdd/chroot
ExecStopPost=/usr/bin/rmdir /run/wsdd/chroot

[Install]
WantedBy=multi-user.target

Output:

root@host:/var/run/wsdd# systemctl start wsdd.service
Job for wsdd.service failed because the control process exited with error code.
See "systemctl status wsdd.service" and "journalctl -xe" for details.
root@host:/var/run/wsdd# systemctl status wsdd.service
● wsdd.service - Web Services Dynamic Discovery host daemon
   Loaded: loaded (/etc/systemd/system/wsdd.service; enabled; vendor preset: enabled)
   Active: failed (Result: exit-code) since Sat 2021-05-22 23:41:39 EEST; 4s ago
  Process: 13978 ExecStartPre=/usr/bin/install -d -o wsdd -g wsdd -m 0700 /run/wsdd/chroot (code=exited, status=1/FAILURE)
  Process: 13979 ExecStopPost=/usr/bin/rmdir /run/wsdd/chroot (code=exited, status=1/FAILURE)

May 22 23:41:39 host systemd[1]: Starting Web Services Dynamic Discovery host daemon...
May 22 23:41:39 host install[13978]: /usr/bin/install: cannot change owner and permissions of ‘/run/wsdd/chroot’: No such file or directory
May 22 23:41:39 host systemd[1]: wsdd.service: Control process exited, code=exited, status=1/FAILURE
May 22 23:41:39 host rmdir[13979]: /usr/bin/rmdir: failed to remove '/run/wsdd/chroot': Read-only file system
May 22 23:41:39 host systemd[1]: wsdd.service: Control process exited, code=exited, status=1/FAILURE
May 22 23:41:39 host systemd[1]: wsdd.service: Failed with result 'exit-code'.
May 22 23:41:39 host systemd[1]: Failed to start Web Services Dynamic Discovery host daemon.

I solved the problem by prefixing executable paths in ExecStartPre and ExecStopPost options with '+' like this:

ExecStartPre=+/usr/bin/install -d -o wsdd -g wsdd -m 0700 /run/wsdd/chroot
ExecStopPost=+/usr/bin/rmdir /run/wsdd/chroot
christgau commented 3 years ago

I encountered the same issue while playing around with the chroot.

I went into a different direction and used the RuntimeDirectory which is automatically created and removed by systemd with the user/group provided in the unit file. So no need for pre/post exec commands anymore. See PR #110 for a new version of the unit file. Comments are welcome!

christgau commented 2 years ago

110 Integrated into master with c2ce2504 and released with v0.7.0

koitsu commented 1 year ago

Use of a chroot has had some negative ramifications on Ubuntu. I do not believe anything needs to be changed in wsdd, however. Please see https://github.com/liske/needrestart/issues/258 for details.

christgau commented 1 year ago

Thanks for digging into the details of needrestart and providing the pointer. I would agree that nothing needs to be changed for wsdd. It appears to me that there is a general issue with needrestart and chroot'ing units so it might affect other services out there as well.

However, if there is an issue arises, feel free to open a new issue here.