Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

systemd failed to set up mount namespacing for /var/lib/bluetooth #329

Closed
herbrechtsmeier opened this issue Apr 12, 2022 · 14 comments
Closed

Comments

@herbrechtsmeier
Copy link

The systemd bluetooth service failed to start because the /var/lib/bluetooth path of ReadWritePaths= is created by the bluetooth daemon itself.

bluetooth.service: Failed to set up mount namespacing: /run/systemd/unit-root/var/lib/bluetooth: No such file or directory

The commit systemd: Add more filesystem lockdown (442d211) add ReadWritePaths=/etc/bluetooth and ReadOnlyPaths=/var/lib/bluetooth options to the bluetooth systemd service. The existing ProtectSystem=full option mounts the /usr, the boot loader directories and /etc read-only. This means the two option are useless and could be removed.

Alternative the ReadWritePaths= path should be prefixed with "-" to ignore a not existing path.

@herbrechtsmeier
Copy link
Author

@hadess Why you add the ReadWritePaths= and ReadOnlyPaths=?

@Vudentz
Copy link
Contributor

Vudentz commented Apr 12, 2022

@herbrechtsmeier it looks like it is possible to have a ReadWritePaths that points to a subdir inside ReadOnlyPaths:

Nest ReadWritePaths= inside of ReadOnlyPaths= in order to provide writable subdirectories within read-only directories. Use ReadWritePaths= in order to allow-list specific paths for write access if ProtectSystem=strict is used.

I do wonder why we didn't use StateDirectory and ConfigurationDirectory though since it appears to be a more explicit way to tell systemd what these directories are meant for.

@herbrechtsmeier
Copy link
Author

@herbrechtsmeier it looks like it is possible to have a ReadWritePaths that points to a subdir inside ReadOnlyPaths:

Nest ReadWritePaths= inside of ReadOnlyPaths= in order to provide writable subdirectories within read-only directories. Use ReadWritePaths= in order to allow-list specific paths for write access if ProtectSystem=strict is used.

Therefore you need ProtectSystem=strict or a ReadOnlyPaths= path above the ReadWritePaths= path.

Does the bluetooth systemd service support a state and configuration directory which is different to the system values? This is the only case in which the ReadOnlyPaths= and ReadWritePaths= could makes sense.

Why the bluetooth service use ProtectSystem=full instead of ProtectSystem=strict?

I do wonder why we didn't use StateDirectory and ConfigurationDirectory though since it appears to be a more explicit way to tell systemd what these directories are meant for.

This was also my first idea but the bluetooth daemon already create the state directory and the StateDirectory= and ConfigurationDirectory= only supports subfolders of the system state and configuration directory.

I propose to add ConfigurationDirectory=bluetooth and StateDirectory=bluetooth. In addition I would remove ReadOnlyPaths=/etc/bluetooth because it is the default of ProtectSystem=full and would remove ReadWritePaths=/var/lib/bluetooth because it is the default or would change ProtectSystem=full to ProtectSystem=strict.

@Vudentz
Copy link
Contributor

Vudentz commented Apr 13, 2022

@herbrechtsmeier https://patchwork.kernel.org/project/bluetooth/patch/20220412201949.4011833-1-luiz.dentz@gmail.com/

Regarding ProtectSystem, having it set to strict seem to be even more restrictive than full:

If true, mounts the /usr/ and the boot loader directories (/boot and /efi) read-only for processes invoked by this unit. If set to "full", the /etc/ directory is mounted read-only, too. If set to "strict" the entire file system hierarchy is mounted read-only, except for the API file system subtrees /dev/, /proc/ and /sys/ (protect these directories using PrivateDevices=, ProtectKernelTunables=, ProtectControlGroups=).

So if we set to strict I assume it will affect even the likes of StateDirectory since it set the entire file system as read-only so we would need to use ReadWritePaths to enabling writing the states, perhaps that was the original intend so we are kinda running in circle here and we would be hitting the same problem or does systemd creates the directory set in StateDirectory? Perhaps we can look into some other daemons similar to bluetoothd to see how the use ProtectSystem.

@herbrechtsmeier
Copy link
Author

@herbrechtsmeier https://patchwork.kernel.org/project/bluetooth/patch/20220412201949.4011833-1-luiz.dentz@gmail.com/

You have to use a relative directory for ConfigurationDirectory= and StateDirectory=. Therefore you could remove the statedir and confdir from the sed in Makefile.am.

Regarding ProtectSystem, having it set to strict seem to be even more restrictive than full

Yes. It looks like this was the indention of the original commit (442d211):

We can only access the configuration file as read-only and read-write to the Bluetooth cache directory and sub-directories.

So if we set to strict I assume it will affect even the likes of StateDirectory since it set the entire file system as read-only so we would need to use ReadWritePaths to enabling writing the states, perhaps that was the original intend so we are kinda running in circle here and we would be hitting the same problem or does systemd creates the directory set in StateDirectory? Perhaps we can look into some other daemons similar to bluetoothd to see how the use ProtectSystem.

Systemd creates the StateDirectory= paths outside of the namespace and it looks like it automatically mount it read-write in the namespace. The following change fix the problem on my system:

-ProtectSystem=full
+ProtectSystem=strict
...
-ReadWritePaths=@statedir@
-ReadOnlyPaths=@confdir@
+ConfigurationDirectory=bluetooth
+StateDirectory=bluetooth

The systemd-timesyncd.service use ProtectSystem=strict and has a StateDirectory= but no ReadWritePaths=.

@Vudentz
Copy link
Contributor

Vudentz commented Apr 14, 2022

@herbrechtsmeier https://patchwork.kernel.org/project/bluetooth/patch/20220412201949.4011833-1-luiz.dentz@gmail.com/

You have to use a relative directory for ConfigurationDirectory= and StateDirectory=. Therefore you could remove the statedir and confdir from the sed in Makefile.am.

Weird I don't see anything on the documentation suggesting it would be a relative path, the problem is that the likes of --sysconfdir and --localstatedir are absolute path so one can configure these to be outside of the default path and in that case that wouldn't match what the bluetooth.service points to.

Regarding ProtectSystem, having it set to strict seem to be even more restrictive than full

Yes. It looks like this was the indention of the original commit (442d211):

We can only access the configuration file as read-only and read-write to the Bluetooth cache directory and sub-directories.

So if we set to strict I assume it will affect even the likes of StateDirectory since it set the entire file system as read-only so we would need to use ReadWritePaths to enabling writing the states, perhaps that was the original intend so we are kinda running in circle here and we would be hitting the same problem or does systemd creates the directory set in StateDirectory? Perhaps we can look into some other daemons similar to bluetoothd to see how the use ProtectSystem.

Systemd creates the StateDirectory= paths outside of the namespace and it looks like it automatically mount it read-write in the namespace. The following change fix the problem on my system:

-ProtectSystem=full
+ProtectSystem=strict
...
-ReadWritePaths=@statedir@
-ReadOnlyPaths=@confdir@
+ConfigurationDirectory=bluetooth
+StateDirectory=bluetooth

The systemd-timesyncd.service use ProtectSystem=strict and has a StateDirectory= but no ReadWritePaths=.

halstead pushed a commit to openembedded/openembedded-core that referenced this issue Apr 14, 2022
The systemd bluetooth service failed to start. Add a workaround for this whilst the
final fix is discussed upstream, bluez/bluez#329.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
@herbrechtsmeier
Copy link
Author

@herbrechtsmeier https://patchwork.kernel.org/project/bluetooth/patch/20220412201949.4011833-1-luiz.dentz@gmail.com/

You have to use a relative directory for ConfigurationDirectory= and StateDirectory=. Therefore you could remove the statedir and confdir from the sed in Makefile.am.

Weird I don't see anything on the documentation suggesting it would be a relative path,

The second sentence of the Documentation mention it:

The specified directory names must be relative, and may not include ".."

the problem is that the likes of --sysconfdir and --localstatedir are absolute path so one can configure these to be outside of the default path and in that case that wouldn't match what the bluetooth.service points to.

Systemd expects that a service use the system wide state and configuration directories. Otherwise most of the assumption and configuration doesn't work as expected (ex. ProtectSystem=full).

Systemd pass the state and configuration directories via environment variable to the service (see Table 2. Automatic directory creation and environment variables).

splitice pushed a commit to HalleyAssist/poky that referenced this issue Apr 14, 2022
The systemd bluetooth service failed to start. Add a workaround for this whilst the
final fix is discussed upstream, bluez/bluez#329.

(From OE-Core rev: 84ce3a90ba27b377c4a5dfea279f42b61640e9c3)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
@Vudentz
Copy link
Contributor

Vudentz commented Apr 14, 2022

@herbrechtsmeier https://patchwork.kernel.org/project/bluetooth/patch/20220412201949.4011833-1-luiz.dentz@gmail.com/

You have to use a relative directory for ConfigurationDirectory= and StateDirectory=. Therefore you could remove the statedir and confdir from the sed in Makefile.am.

Weird I don't see anything on the documentation suggesting it would be a relative path,

The second sentence of the Documentation mention it:

The specified directory names must be relative, and may not include ".."

Yep, not sure how I missed it.

the problem is that the likes of --sysconfdir and --localstatedir are absolute path so one can configure these to be outside of the default path and in that case that wouldn't match what the bluetooth.service points to.

Systemd expects that a service use the system wide state and configuration directories. Otherwise most of the assumption and configuration doesn't work as expected (ex. ProtectSystem=full).

Systemd pass the state and configuration directories via environment variable to the service (see Table 2. Automatic directory creation and environment variables).

So if we don't change build system we can't really use StateDirectory and ConfigurationDirectory, at least not when it is configured outside of the supported paths. We could in theory detect if no prefix have been given than use relative paths otherwise we need to use ReadWritePaths, but the fact that one can use ReadWritePaths absolute paths that would point inside StateDirectory makes me wonder why the later couldn't just do the same if an absolute path is given, or ReadWritePaths don't really work with ProtectSystem=strict? From the documentation it seems to be allowed:

Use ReadWritePaths= in order to allow-list specific paths for write access if ProtectSystem=strict is used.

Regarding the use of environment variables, that wouldn't work when example the daemon is run from the source tree and not as a service under systemd control, we would probably need something like a shell script to set the environment variables or a runtime code that detects if the environment variables are set or not.

halstead pushed a commit to openembedded/openembedded-core that referenced this issue Apr 14, 2022
The systemd bluetooth service failed to start. Add a workaround for this whilst the
final fix is discussed upstream, bluez/bluez#329.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
halstead pushed a commit to openembedded/openembedded-core that referenced this issue Apr 14, 2022
The systemd bluetooth service failed to start. Add a workaround for this whilst the
final fix is discussed upstream, bluez/bluez#329.

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
github-actions bot pushed a commit to cyber-zoo/poky that referenced this issue Apr 14, 2022
The systemd bluetooth service failed to start. Add a workaround for this whilst the
final fix is discussed upstream, bluez/bluez#329.

(From OE-Core rev: 3e85ce436699a2b5b7751f671e4a6eabb4ca5404)

Signed-off-by: Richard Purdie <richard.purdie@linuxfoundation.org>
github-actions bot pushed a commit to tedd-an/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
github-actions bot pushed a commit to tedd-an/bluez that referenced this issue Apr 15, 2022
This fixes bluetooth.service failing to start if statedir has not been
created yet:

bluetooth.service: Failed to set up mount namespacing:
/run/systemd/unit-root/var/lib/bluetooth: No such file or directory

It also removes ReadOnlyPaths since ProtectSystem=full already mounts
the entire filesystem as read-only.

Fixes: bluez#329
github-actions bot pushed a commit to BluezTestBot/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
github-actions bot pushed a commit to BluezTestBot/bluez that referenced this issue Apr 15, 2022
This fixes bluetooth.service failing to start if statedir has not been
created yet:

bluetooth.service: Failed to set up mount namespacing:
/run/systemd/unit-root/var/lib/bluetooth: No such file or directory

It also removes ReadOnlyPaths since ProtectSystem=full already mounts
the entire filesystem as read-only.

Fixes: bluez#329
github-actions bot pushed a commit to BluezTestBot/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
github-actions bot pushed a commit to tedd-an/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
github-actions bot pushed a commit to BluezTestBot/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
github-actions bot pushed a commit to tedd-an/bluez that referenced this issue Apr 15, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
@Vudentz
Copy link
Contributor

Vudentz commented Apr 16, 2022

@herbrechtsmeier
Copy link
Author

@herbrechtsmeier check with the following set:

https://patchwork.kernel.org/project/bluetooth/patch/20220415223049.1155838-3-luiz.dentz@gmail.com/

At the moment I have no access to the hardware but I have already test the changes manual on my hardware before I suggest the change above and poky already use this change.

@hadess
Copy link
Contributor

hadess commented Apr 19, 2022

Your change is mostly correct, but it makes /etc/bluetooth writable. From systemd.exec:

       RuntimeDirectoryMode=, StateDirectoryMode=, CacheDirectoryMode=, LogsDirectoryMode=, ConfigurationDirectoryMode=
           Specifies the access mode of the directories specified in RuntimeDirectory=, StateDirectory=, CacheDirectory=, LogsDirectory=, or ConfigurationDirectory=, respectively, as an octal number. Defaults to 0755. See "Permissions" in path_resolution(7) for a discussion of the meaning of permission bits.

You should add ConfigurationDirectoryMode=0555 to the configuration.

@hadess
Copy link
Contributor

hadess commented Apr 19, 2022

There's also a regression for anyone that uses --localstatedir= to something other than /var/ or --sysconfdir= to something other than /etc. You should probably use full paths in those variables.

@hadess
Copy link
Contributor

hadess commented Apr 19, 2022

There's also a regression for anyone that uses --localstatedir= to something other than /var/ or --sysconfdir= to something other than /etc. You should probably use full paths in those variables.

This seems to be worked around by 5fb2741 and 385e8d6 although that would also require changes in doc/settings-storage.txtand mesh/README.

@Vudentz
Copy link
Contributor

Vudentz commented Apr 19, 2022

Your change is mostly correct, but it makes /etc/bluetooth writable. From systemd.exec:

       RuntimeDirectoryMode=, StateDirectoryMode=, CacheDirectoryMode=, LogsDirectoryMode=, ConfigurationDirectoryMode=
           Specifies the access mode of the directories specified in RuntimeDirectory=, StateDirectory=, CacheDirectory=, LogsDirectory=, or ConfigurationDirectory=, respectively, as an octal number. Defaults to 0755. See "Permissions" in path_resolution(7) for a discussion of the meaning of permission bits.

You should add ConfigurationDirectoryMode=0555 to the configuration.

I will fix it, we actually need to set the StateDirectoryMode as well since the defaults is 0755 but we used to have 0700 since it may contain sensitve information.

github-actions bot pushed a commit to tedd-an/bluez that referenced this issue Apr 19, 2022
This sets ConfigurationDirectoryMode to 0555 to really enforce the
ConfigurationDirectory to be read-only [1].

[1] bluez#329 (comment)
github-actions bot pushed a commit to BluezTestBot/bluez that referenced this issue Apr 19, 2022
This sets ConfigurationDirectoryMode to 0555 to really enforce the
ConfigurationDirectory to be read-only [1].

[1] bluez#329 (comment)
ammarfaizi2 pushed a commit to ammarfaizi2/linux-block that referenced this issue Apr 20, 2022
This sets ConfigurationDirectoryMode to 0555 to really enforce the
ConfigurationDirectory to be read-only [1].

[1] bluez/bluez#329 (comment)
hadess pushed a commit to hadess/bluez that referenced this issue Aug 26, 2022
This makes use of StateDirectory[1] and ConfigurationDirectory[1] to
inform systemd what those paths are used for instead of using
ReadWritePaths and ReadOnlyPaths which can lead to issues.

Fixes: bluez/bluez#329

[1] https://www.freedesktop.org/software/systemd/man/systemd.exec.html
hadess pushed a commit to hadess/bluez that referenced this issue Aug 26, 2022
This sets ConfigurationDirectoryMode to 0555 to really enforce the
ConfigurationDirectory to be read-only [1].

[1] bluez/bluez#329 (comment)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment