| Message ID | YEvlv7v2LawKE0rF@jasmine.lan |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [bug#47013] gnu: Harden filesystem links. | expand |
| Context | Check | Description |
|---|---|---|
| cbaines/comparison | success | View comparision |
| cbaines/git branch | success | View Git branch |
| cbaines/applying patch | fail | View Laminar job |
| cbaines/issue | success | View issue |
On Fri, Mar 12, 2021 at 05:05:51PM -0500, Leo Famulari wrote: > Here is an updated patch that can be composed with other > sysctl-service-types that the user may have added to config.scm. The only issue that I see with this revised patch is that it's not clear how users could disable these default settings if they wanted to. Re-setting these options to another value by adding a sysctl-service-type to the services field of config.scm does not override the default-settings. And trying to remove the default-sysctl-settings simple-service doesn't work (even when exporting the variable from (gnu services base)). Does anyone know how we could make it possible for users to change these new defaults?
Hi! Leo Famulari <leo@famulari.name> skribis: > On Fri, Mar 12, 2021 at 05:05:51PM -0500, Leo Famulari wrote: >> Here is an updated patch that can be composed with other >> sysctl-service-types that the user may have added to config.scm. > > The only issue that I see with this revised patch is that it's not clear > how users could disable these default settings if they wanted to. With your first patch, to change the default settings, one has to write: (modify-services %base-services (sysctl-service-type config => …)) With your first patch, someone who already had a ‘sysctl-service-type’ instance as part of their services would now get an error at reconfigure time. Your second patch nicely addresses that; the downside is that it actually makes it slightly harder to change the defaults because you wouldn’t know what to pass in your ‘modify-services’ form. All in all, I have a slight preference for the first patch. It could be accompanied with a news.scm entry to explain the incompatible change, maybe. Thoughts? Thanks, Ludo’.
On Tue, Mar 16, 2021 at 11:18:18PM +0100, Ludovic Courtès wrote:
> Thoughts?
We discussed this on IRC.
Basically, my goal is to make it easy for users to add their own
sysctl-service-type without accidentally removing the default sysctl
settings. My third patch achieves that.
However, you did not like that it required creating a new service type
just to set some defaults.
As a compromise, we could create a new variable %default-sysctl-settings
and add a sysctl-service-type in %base-services that uses that variable.
At least, that way, it would be a little more clear that there are some
defaults. The manual could show users how to append their own sysctl
parameters to %default-sysctl-settings.
While implementing that, I noticed the variable
%default-kernel-arguments in (gnu system).
All these years, I have been setting some custom kernel-arguments, and I
never noticed there was a default value that I was erasing. This
illustrates why I prefer the approach in my 3rd patch. Otherwise, it
will be very easy for users to implicitly and unexpectedly disable the
default parameters we are trying to set, if they try to add their own.
diff --git a/gnu/services/base.scm b/gnu/services/base.scm index f6a490f712..64aac36401 100644 --- a/gnu/services/base.scm +++ b/gnu/services/base.scm @@ -35,6 +35,7 @@ #:use-module (gnu services) #:use-module (gnu services admin) #:use-module (gnu services shepherd) + #:use-module (gnu services sysctl) #:use-module (gnu system pam) #:use-module (gnu system shadow) ; 'user-account', etc. #:use-module (gnu system uuid) @@ -2484,6 +2485,11 @@ to handle." (requirement requirement) (name-servers name-servers))))) +(define (default-sysctl-settings default-settings) + (simple-service 'base-sysctl-settings + sysctl-service-type + default-settings)) + (define %base-services ;; Convenience variable holding the basic services. @@ -2532,6 +2538,10 @@ to handle." (udev-configuration (rules (list lvm2 fuse alsa-utils crda)))) + (default-sysctl-settings + '(("fs.protected_hardlinks" . "1") + ("fs.protected_symlinks" . "1"))) + (service special-files-service-type `(("/bin/sh" ,(file-append bash "/bin/sh")) ("/usr/bin/env" ,(file-append coreutils "/bin/env"))))))
Here is an updated patch that can be composed with other sysctl-service-types that the user may have added to config.scm. From 1e3bd831899a4ec9dfa7199a381421adbfe0dcf7 Mon Sep 17 00:00:00 2001 From: Leo Famulari <leo@famulari.name> Date: Fri, 12 Mar 2021 17:03:26 -0500 Subject: [PATCH] system: Harden filesystem links. These sysctl options are enabled on most GNU/Linux distros, including Debian, Fedora, NixOS, and OpenSUSE. I've tested this patch on Guix System for several weeks, and it doesn't appear to break anything. Plus, we know that Guix works on other distros that enable these restrictions. References: https://sysctl-explorer.net/fs/protected_hardlinks/ https://sysctl-explorer.net/fs/protected_symlinks/ * gnu/services/base.scm (default-sysctl-settings): New variable. (%base-services): Add default-sysctl-settings. --- gnu/services/base.scm | 10 ++++++++++ 1 file changed, 10 insertions(+)