[bug#47013] [PATCH] gnu: Harden filesystem links.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

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 (%base-services): Add a default
sysctl-configuration that enables fs.protected_hardlinks and
fs.protected_symlinks.
---
 gnu/services/base.scm | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index f6a490f712..edd2c8e355 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015, 2016, 2020 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
-;;; Copyright © 2016, 2017 Leo Famulari <leo@famulari.name>
+;;; Copyright © 2016, 2017, 2021 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Mathieu Othacehe <m.othacehe@gmail.com>
@@ -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)
@@ -2532,6 +2533,12 @@ to handle."
                  (udev-configuration
                    (rules (list lvm2 fuse alsa-utils crda))))
 
+        (service sysctl-service-type
+                 (sysctl-configuration
+                   (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"))))))
-- 
2.30.1

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 122 bytes --]

Here is an updated patch that can be composed with other
sysctl-service-types that the user may have added to config.scm.

[-- Attachment #1.2: 0001-system-Harden-filesystem-links.patch --]
[-- Type: text/plain, Size: 2209 bytes --]

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(+)

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))
+
 \f
 (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"))))))
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

[-- Attachment #1: Type: text/plain, Size: 706 bytes --]

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?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 318 bytes --]

On Fri, Mar 12, 2021 at 05:51:21PM -0500, Leo Famulari wrote:
> Does anyone know how we could make it possible for users to change these
> new defaults?

With assistance from roptat on #guix, I wrote these patches that work
well and meet all the requirements I had in mind.

Your thoughts? I'd like to push this soon.

[-- Attachment #1.2: harden-filesystem-links.patch --]
[-- Type: text/plain, Size: 4477 bytes --]

From 38f1aaf8b44739ccfb1f824c7fb85d4dc6b5d991 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Mon, 15 Mar 2021 14:51:52 -0400
Subject: [PATCH 1/2] services: sysctl: Add a service to set default kernel
 parameters.

* gnu/services/sysctl.scm (default-sysctl-settings-service-type): New public
variable.
* doc/guix.texi (Miscellaneous Services): Document it.

Co-authored-by: Julien Lepiller <julien@lepiller.eu>
---
 doc/guix.texi           |  4 ++++
 gnu/services/sysctl.scm | 13 ++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 3e7ffc81bc..d468c6f742 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -31419,6 +31419,10 @@ An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@defvr {Scheme Variable} default-sysctl-settings-service-type
+The service type used to set default kernel parameters.
+@end defvr
+
 @cindex pcscd
 @subsubheading PC/SC Smart Card Daemon Service
 
diff --git a/gnu/services/sysctl.scm b/gnu/services/sysctl.scm
index eb7a61b2a9..83704084c3 100644
--- a/gnu/services/sysctl.scm
+++ b/gnu/services/sysctl.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2017 Sou Bunnbu <iyzsong@member.fsf.org>
+;;; Copyright © 2021 Leo Famulari <leo@famulari.name>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -25,7 +26,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
   #:export (sysctl-configuration
-            sysctl-service-type))
+            sysctl-service-type
+            default-sysctl-settings-service-type))
 
 \f
 ;;;
@@ -74,3 +76,12 @@
               (settings (append (sysctl-configuration-settings config)
                                 settings)))))
    (default-value (sysctl-configuration))))
+
+(define default-sysctl-settings-service-type
+;  "Return a service that is used to set default kernel parameters for Guix
+;  System."
+  (service-type
+    (name 'default-sysctl-settings)
+    (extensions
+      (list (service-extension sysctl-service-type
+                               identity)))))
-- 
2.30.2


From 3040f0bb33439f041eed85e8c8e80bb52d6277cc Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Mon, 15 Mar 2021 14:31:48 -0400
Subject: [PATCH 2/2] system: Harden filesystem links.

These sysctl options are enabled on most GNU/Linux distros, including
Debian, Fedora, NixOS, and OpenSUSE.

I've tested this options on Guix System for several weeks, and they
don'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 (%base-services): Add
default-sysctl-settings-service-type.
---
 gnu/services/base.scm | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index f6a490f712..646ad800f4 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -3,7 +3,7 @@
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015, 2016, 2020 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
-;;; Copyright © 2016, 2017 Leo Famulari <leo@famulari.name>
+;;; Copyright © 2016, 2017, 2021 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
 ;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Mathieu Othacehe <m.othacehe@gmail.com>
@@ -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)
@@ -2532,6 +2533,10 @@ to handle."
                  (udev-configuration
                    (rules (list lvm2 fuse alsa-utils crda))))
 
+        (service default-sysctl-settings-service-type
+          '(("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"))))))
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Julien Lepiller]

[-- Attachment #1: Type: text/plain, Size: 626 bytes --]

Not tested but looks ok. Could you extend the documentation a bit? Maybe add the expected type of data for the service and an example on how to use it with modify-services? With lirks to relevant sections.

Le 15 mars 2021 14:56:06 GMT-04:00, Leo Famulari <leo@famulari.name> a écrit :
>On Fri, Mar 12, 2021 at 05:51:21PM -0500, Leo Famulari wrote:
>> Does anyone know how we could make it possible for users to change
>these
>> new defaults?
>
>With assistance from roptat on #guix, I wrote these patches that work
>well and meet all the requirements I had in mind.
>
>Your thoughts? I'd like to push this soon.

[-- Attachment #2: Type: text/html, Size: 1007 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

On Mon, Mar 15, 2021 at 04:23:24PM -0400, Julien Lepiller wrote:
> Not tested but looks ok. Could you extend the documentation a bit?
> Maybe add the expected type of data for the service and an example on
> how to use it with modify-services? With lirks to relevant sections.

We ended up pushing a slightly different patch from the one you've
replied to.

We did add documentation along the lines you requested, but let me know
if you see more room for improvment:

https://git.savannah.gnu.org/cgit/guix.git/diff/doc/guix.texi?id=898489f48e436e45e86e1ba0fcdb6df5cd5a051a

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Julien Lepiller]

[-- Attachment #1: Type: text/plain, Size: 789 bytes --]

Ah sorry! Looks like my email was delayed, probably an issue on my side. Documentation looks good, thanks!

Le 18 mars 2021 13:39:20 GMT-04:00, Leo Famulari <leo@famulari.name> a écrit :
>On Mon, Mar 15, 2021 at 04:23:24PM -0400, Julien Lepiller wrote:
>> Not tested but looks ok. Could you extend the documentation a bit?
>> Maybe add the expected type of data for the service and an example on
>> how to use it with modify-services? With lirks to relevant sections.
>
>We ended up pushing a slightly different patch from the one you've
>replied to.
>
>We did add documentation along the lines you requested, but let me know
>if you see more room for improvment:
>
>https://git.savannah.gnu.org/cgit/guix.git/diff/doc/guix.texi?id=898489f48e436e45e86e1ba0fcdb6df5cd5a051a

[-- Attachment #2: Type: text/html, Size: 1294 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Ludovic Courtès]

Hi!

Leo Famulari <leo@famulari.name> skribis:

> From 38f1aaf8b44739ccfb1f824c7fb85d4dc6b5d991 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Mon, 15 Mar 2021 14:51:52 -0400
> Subject: [PATCH 1/2] services: sysctl: Add a service to set default kernel
>  parameters.
>
> * gnu/services/sysctl.scm (default-sysctl-settings-service-type): New public
> variable.
> * doc/guix.texi (Miscellaneous Services): Document it.
>
> Co-authored-by: Julien Lepiller <julien@lepiller.eu>

[...]

> +(define default-sysctl-settings-service-type
> +;  "Return a service that is used to set default kernel parameters for Guix
> +;  System."
> +  (service-type
> +    (name 'default-sysctl-settings)
> +    (extensions
> +      (list (service-extension sysctl-service-type
> +                               identity)))))

[...]

> +        (service default-sysctl-settings-service-type
> +          '(("fs.protected_hardlinks" . "1")
> +            ("fs.protected_symlinks" . "1")))

Why not just use ‘sysctl-service-type’ here?
‘default-sysctl-settings-service-type’ looks very much like
‘sysctl-service-type’, but I’m not sure we need a second one?

Thanks!

Ludo’.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Ludovic Courtès]

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’.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

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.

[bug#47013] [PATCH v4] gnu: Harden filesystem links.

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 780 bytes --]

On Tue, Mar 16, 2021 at 08:54:52PM -0400, Leo Famulari wrote:
> 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.

Here is a v4 patch that implements this. I wasn't sure where to put
%default-sysctl-settings, so it's in (gnu services sysctl).

From my naive perspective, it seemed to me that it belongs in (gnu
system), but when I exported it from there, and imported (gnu system) in
(gnu services base), building Guix crashes like this:

------
[ 12%] LOAD     guix/scripts/system.scm
ice-9/eval.scm:293:34: error: %default-sysctl-settings: unbound variable
hint: Did you forget `(use-modules (gnu system))'?

make[2]: *** [Makefile:6304: make-go] Error 1
------

[-- Attachment #1.2: harden-filesystem-links.patch --]
[-- Type: text/plain, Size: 2620 bytes --]

From 7c95b94918c0f119a16a9859b250bdc65054f646 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Tue, 16 Mar 2021 21:36:36 -0400
Subject: [PATCH v4] system: Harden filesystem links.

These sysctl options are enabled on most GNU/Linux distros, including
Debian, Fedora, NixOS, and OpenSUSE.

I've tested this options on Guix System for several weeks, and they
don'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/sysctl.scm (%default-sysctl-settings): New public variable.
* gnu/services/base.scm (%base-services): Use %default-sysctl-settings.
---
 gnu/services/base.scm   | 5 +++++
 gnu/services/sysctl.scm | 8 +++++++-
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index f6a490f712..eaa86ffb68 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)
@@ -2532,6 +2533,10 @@ to handle."
                  (udev-configuration
                    (rules (list lvm2 fuse alsa-utils crda))))
 
+        (service sysctl-service-type
+                 (sysctl-configuration
+                   (settings %default-sysctl-settings)))
+
         (service special-files-service-type
                  `(("/bin/sh" ,(file-append bash "/bin/sh"))
                    ("/usr/bin/env" ,(file-append coreutils "/bin/env"))))))
diff --git a/gnu/services/sysctl.scm b/gnu/services/sysctl.scm
index eb7a61b2a9..dbf918eb3a 100644
--- a/gnu/services/sysctl.scm
+++ b/gnu/services/sysctl.scm
@@ -25,7 +25,8 @@
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
   #:export (sysctl-configuration
-            sysctl-service-type))
+            sysctl-service-type
+            %default-sysctl-settings))
 
 \f
 ;;;
@@ -74,3 +75,8 @@
               (settings (append (sysctl-configuration-settings config)
                                 settings)))))
    (default-value (sysctl-configuration))))
+
+(define %default-sysctl-settings
+  ;; Default kernel parameters enabled with sysctl.
+  '(("fs.protected_hardlinks" . "1")
+    ("fs.protected_symlinks" . "1")))
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Ludovic Courtès]

Hi,

Leo Famulari <leo@famulari.name> skribis:

> On Tue, Mar 16, 2021 at 08:54:52PM -0400, Leo Famulari wrote:
>> 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.
>
> Here is a v4 patch that implements this. I wasn't sure where to put
> %default-sysctl-settings, so it's in (gnu services sysctl).
>
> From my naive perspective, it seemed to me that it belongs in (gnu
> system), but when I exported it from there, and imported (gnu system) in
> (gnu services base), building Guix crashes like this:
>
> ------
> [ 12%] LOAD     guix/scripts/system.scm
> ice-9/eval.scm:293:34: error: %default-sysctl-settings: unbound variable
> hint: Did you forget `(use-modules (gnu system))'?

Yeah, some circular module dependency.

I propose this minor change:

> +++ 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)
> @@ -2532,6 +2533,10 @@ to handle."
>                   (udev-configuration
>                     (rules (list lvm2 fuse alsa-utils crda))))
>  
> +        (service sysctl-service-type
> +                 (sysctl-configuration
> +                   (settings %default-sysctl-settings)))

Write (service sysctl-service-type) here, and…

> +++ b/gnu/services/sysctl.scm
> @@ -25,7 +25,8 @@
>    #:use-module (srfi srfi-1)
>    #:use-module (ice-9 match)
>    #:export (sysctl-configuration
> -            sysctl-service-type))
> +            sysctl-service-type
> +            %default-sysctl-settings))
>  
>  \f
>  ;;;
> @@ -74,3 +75,8 @@
>                (settings (append (sysctl-configuration-settings config)
>                                  settings)))))
>     (default-value (sysctl-configuration))))
> +
> +(define %default-sysctl-settings
> +  ;; Default kernel parameters enabled with sysctl.
> +  '(("fs.protected_hardlinks" . "1")
> +    ("fs.protected_symlinks" . "1")))

… change the default value of the ‘settings’ field of
<sysctl-configuration> to be ‘%default-sysctl-settings’.

We should also add a @defvr and adjust guix.texi accordingly.

WDYT?

Thanks,
Ludo’.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

On Wed, Mar 17, 2021 at 09:49:04PM +0100, Ludovic Courtès wrote:
> [...]
> … change the default value of the ‘settings’ field of
> <sysctl-configuration> to be ‘%default-sysctl-settings’.
> 
> We should also add a @defvr and adjust guix.texi accordingly.
> 
> WDYT?

Sure, I'll implement your suggestions and send a v5 patch.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

[-- Attachment #1.1: Type: text/plain, Size: 151 bytes --]

On Wed, Mar 17, 2021 at 05:01:54PM -0400, Leo Famulari wrote:
> Sure, I'll implement your suggestions and send a v5 patch.

Here is the revised patch.

[-- Attachment #1.2: 0001-system-Harden-filesystem-links.patch --]
[-- Type: text/plain, Size: 4408 bytes --]

From 1817aec86076307f7b85cdc27b9ead572d0575e7 Mon Sep 17 00:00:00 2001
From: Leo Famulari <leo@famulari.name>
Date: Tue, 16 Mar 2021 21:36:36 -0400
Subject: [PATCH] system: Harden filesystem links.

References:

https://sysctl-explorer.net/fs/protected_hardlinks/
https://sysctl-explorer.net/fs/protected_symlinks/

* gnu/services/sysctl.scm (%default-sysctl-settings): New public variable.
(<sysctl-configuration>): Use %default-sysctl-settings as the default value.
* gnu/services/base.scm (%base-services): Add sysctl-service-type.
* doc/guix.texi (Miscellaneous Services): Document the new defaults.
---
 doc/guix.texi           | 22 +++++++++++++++++++++-
 gnu/services/base.scm   |  3 +++
 gnu/services/sysctl.scm | 10 ++++++++--
 3 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 0a70ac7f11..73757be887 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -31378,6 +31378,21 @@ instantiated as:
          (sysctl-configuration
            (settings '(("net.ipv4.ip_forward" . "1")))))
 @end lisp
+
+Since @code{sysctl-service-type} is used in the default lists of
+services, @code{%base-services} and @code{%desktop-services}, you can
+use @code{modify-services} to change its configuration and add the
+kernel parameters that you want (@pxref{Service Reference,
+@code{modify-services}}).
+
+@lisp
+(modify-services %base-services
+  (sysctl-service-type config =>
+                       (sysctl-configuration
+                         (settings (append '(("net.ipv4.ip_forward" . "1"))
+                                           %default-sysctl-settings)))))
+@end lisp
+
 @end defvr
 
 @deftp {Data Type} sysctl-configuration
@@ -31387,11 +31402,16 @@ The data type representing the configuration of @command{sysctl}.
 @item @code{sysctl} (default: @code{(file-append procps "/sbin/sysctl"})
 The @command{sysctl} executable to use.
 
-@item @code{settings} (default: @code{'()})
+@item @code{settings} (default: @code{%default-sysctl-settings})
 An association list specifies kernel parameters and their values.
 @end table
 @end deftp
 
+@defvr {Scheme Variable} %default-sysctl-settings
+An association list specifying the default @command{sysctl} parameters
+on Guix System.
+@end defvr
+
 @cindex pcscd
 @subsubheading PC/SC Smart Card Daemon Service
 
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index f6a490f712..f50bcfdcb4 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)
@@ -2532,6 +2533,8 @@ to handle."
                  (udev-configuration
                    (rules (list lvm2 fuse alsa-utils crda))))
 
+        (service sysctl-service-type)
+
         (service special-files-service-type
                  `(("/bin/sh" ,(file-append bash "/bin/sh"))
                    ("/usr/bin/env" ,(file-append coreutils "/bin/env"))))))
diff --git a/gnu/services/sysctl.scm b/gnu/services/sysctl.scm
index eb7a61b2a9..aaea7cc30d 100644
--- a/gnu/services/sysctl.scm
+++ b/gnu/services/sysctl.scm
@@ -25,20 +25,26 @@
   #:use-module (srfi srfi-1)
   #:use-module (ice-9 match)
   #:export (sysctl-configuration
-            sysctl-service-type))
+            sysctl-service-type
+            %default-sysctl-settings))
 
 \f
 ;;;
 ;;; System Control Service.
 ;;;
 
+(define %default-sysctl-settings
+  ;; Default kernel parameters enabled with sysctl.
+  '(("fs.protected_hardlinks" . "1")
+    ("fs.protected_symlinks" . "1")))
+
 (define-record-type* <sysctl-configuration>
   sysctl-configuration make-sysctl-configuration
   sysctl-configuration?
   (sysctl   sysctl-configuration-sysctl    ; path of the 'sysctl' command
             (default (file-append procps "/sbin/sysctl")))
   (settings sysctl-configuration-settings  ; alist of string pairs
-            (default '())))
+            (default %default-sysctl-settings)))
 
 (define (sysctl-configuration-settings->sysctl.conf settings)
   "Return a file for @command{sysctl} to set kernel parameters as specified by
-- 
2.30.2


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Ludovic Courtès]

Hi Leo,

Leo Famulari <leo@famulari.name> skribis:

> From 1817aec86076307f7b85cdc27b9ead572d0575e7 Mon Sep 17 00:00:00 2001
> From: Leo Famulari <leo@famulari.name>
> Date: Tue, 16 Mar 2021 21:36:36 -0400
> Subject: [PATCH] system: Harden filesystem links.
>
> References:
>
> https://sysctl-explorer.net/fs/protected_hardlinks/
> https://sysctl-explorer.net/fs/protected_symlinks/
>
> * gnu/services/sysctl.scm (%default-sysctl-settings): New public variable.
> (<sysctl-configuration>): Use %default-sysctl-settings as the default value.
> * gnu/services/base.scm (%base-services): Add sysctl-service-type.
> * doc/guix.texi (Miscellaneous Services): Document the new defaults.

Looks perfect to me, thank you!

Ludo’.

[bug#47013] [PATCH] gnu: Harden filesystem links.

[Leo Famulari]

On Thu, Mar 18, 2021 at 10:36:38AM +0100, Ludovic Courtès wrote:
> Hi Leo,
> 
> Leo Famulari <leo@famulari.name> skribis:
> 
> > From 1817aec86076307f7b85cdc27b9ead572d0575e7 Mon Sep 17 00:00:00 2001
> > From: Leo Famulari <leo@famulari.name>
> > Date: Tue, 16 Mar 2021 21:36:36 -0400
> > Subject: [PATCH] system: Harden filesystem links.
> >
> > References:
> >
> > https://sysctl-explorer.net/fs/protected_hardlinks/
> > https://sysctl-explorer.net/fs/protected_symlinks/
> >
> > * gnu/services/sysctl.scm (%default-sysctl-settings): New public variable.
> > (<sysctl-configuration>): Use %default-sysctl-settings as the default value.
> > * gnu/services/base.scm (%base-services): Add sysctl-service-type.
> > * doc/guix.texi (Miscellaneous Services): Document the new defaults.
> 
> Looks perfect to me, thank you!

Great! This was pushed as 898489f48e436e45e86e1ba0fcdb6df5cd5a051a

[bug#47013] (no subject)

[muradm]

There is a need to have important sysctl settings 
fs.protected_hardlinks and fs.protected_symlinks for all 
installations of Guix in the world unless explicitly stated 
otherwise. Currently in Linux kernel they are unset by default. It 
is also stated that other distributions do the same.

In perfect world I would go for Solution 1 below, as it is most 
effectful, and clean.

Solution 1: From this statement, it seems that the first resort 
whould be Linux kernel it self. If it would be possible to 
configure them with Kconfig, that would be best place. As of my 
brief look at linux/fs, they are not configurable, but may be I 
miss somthing. Any way preferred solution would be just compile 
kernel with protected hardlinks and symlinks set to 1. Since other 
distributions do the same, it could be reasonable to expose these 
two settings via Kconfig, and solve it there.
- pros: great for the world
- cons: have to do enhancement in mainline Linux

Solution 2: If it is not possible to have these two settings in 
kernel as per Solution 1, Guix may maintain a patch to kernel that 
would do this.
- pros: no need to enhance mainline Linux
- cons: will impact users who do use Guix and compile Linux kernel 
  them selves

Solution 3: Handle in Guix configuration. Everything below related 
to solution 3.

Currently it is set as folowing:

;; gnu/services/sysctl.scm
(define-module ....
  #:export (....
                  %default-sysctl-settings)

(define %default-sysctl-settings
  ;; Default kernel parameters enabled with sysctl.
  '(("fs.protected_hardlinks" . "1")
    ("fs.protected_symlinks" . "1")))

(define-record-type* <sysctl-configuration>
  sysctl-configuration make-sysctl-configuration
  sysctl-configuration?
  (sysctl   sysctl-configuration-sysctl    ; path of the 'sysctl' 
  command
            (default (file-append procps "/sbin/sysctl")))
  (settings sysctl-configuration-settings  ; alist of string pairs
            (default %default-sysctl-settings)))

;; ends- gnu/services/sysctl.scm

And sysctl-service-type it self is added to the 
%base-services. Since sysctl-configuration-settings function to 
access settings field of sysctl-configuration instance is not 
exported, I have to do the following in my configuration:

(define nomad-gx1-os
  (operating-system
    (inherit my-base-nomad-os) ;; important line-#1
    ...
    (services
      (modify-services my-base-nomad-services
        (sysctl-service-type config =>
          (inherit config)
          (settings
            (append
              %default-sysctl-settings ;; from 
              gnu/services/sysctl.scm
              '(("fs.inotify.max_user_watches" . "524288")
                ("fs.inotify.max_user_instances" . "16384")
                ("fs.inotify.max_queued_events" . "65536")))))))))

This is fine, until I extend sysctl-service-type in 
my-base-nomad-os. Then I have to export 
my-base-nomad-sysctl-settings and join them with 
%default-sysctl-settings and extra settings for 
nomad-gx1-os. While it is bearable for one or two levels of 
inheritance, it becomes hard to keep track for more levels and/or 
many hosts.

If sysctl-configuration-settings would be exported as per #47323, 
then my configuration would become simplier:

(services
  (modify-services my-base-nomad-services
    (sysctl-service-type config =>
      (inherit config)
      (settings
        (append
           (sysctl-configuration-settings config) ;; now I can't 
           do this
           '(("fs.inotify.max_user_watches" . "524288")
             ("fs.inotify.max_user_instances" . "16384")
             ("fs.inotify.max_queued_events" . "65536")))))))))

In this case, if Guix documentation will include 
sysctl-configuration-settings, then most likely people won't 
forget use %default-sysctl-settings, and it is still possible to 
override them if one desires not to use protected symlinks and 
hardlinks.

[bug#47013] services: export sysctl-configuration record field accessors

[muradm]

As per discussion with Leo on IRC #guix.

There is a need to have important sysctl settings
fs.protected_hardlinks and fs.protected_symlinks for all
installations of Guix in the world unless explicitly stated
otherwise. Currently in Linux kernel they are unset by default. It
is also stated that other distributions do the same.

In perfect world I would go for Solution 1 below, as it is most
effectful, and clean.

Solution 1: From this statement, it seems that the first resort
whould be Linux kernel it self. If it would be possible to
configure them with Kconfig, that would be best place. As of my
brief look at linux/fs, they are not configurable, but may be I
miss somthing. Any way preferred solution would be just compile
kernel with protected hardlinks and symlinks set to 1. Since other
distributions do the same, it could be reasonable to expose these
two settings via Kconfig, and solve it there.
- pros: great for the world
- cons: have to do enhancement in mainline Linux

Solution 2: If it is not possible to have these two settings in
kernel as per Solution 1, Guix may maintain a patch to kernel that
would do this.
- pros: no need to enhance mainline Linux
- cons: will impact users who do use Guix and compile Linux kernel
   them selves

Solution 3: Handle in Guix configuration. Everything below related
to solution 3.

Currently it is set as folowing:

;; gnu/services/sysctl.scm
(define-module ....
   #:export (....
                   %default-sysctl-settings)

(define %default-sysctl-settings
   ;; Default kernel parameters enabled with sysctl.
   '(("fs.protected_hardlinks" . "1")
     ("fs.protected_symlinks" . "1")))

(define-record-type* <sysctl-configuration>
   sysctl-configuration make-sysctl-configuration
   sysctl-configuration?
   (sysctl   sysctl-configuration-sysctl    ; path of the 'sysctl'
   command
             (default (file-append procps "/sbin/sysctl")))
   (settings sysctl-configuration-settings  ; alist of string pairs
             (default %default-sysctl-settings)))

;; ends- gnu/services/sysctl.scm

And sysctl-service-type it self is added to the
%base-services. Since sysctl-configuration-settings function to
access settings field of sysctl-configuration instance is not
exported, I have to do the following in my configuration:

(define nomad-gx1-os
   (operating-system
     (inherit my-base-nomad-os) ;; important line-#1
     ...
     (services
       (modify-services my-base-nomad-services
         (sysctl-service-type config =>
           (inherit config)
           (settings
             (append
               %default-sysctl-settings ;; from
               gnu/services/sysctl.scm
               '(("fs.inotify.max_user_watches" . "524288")
                 ("fs.inotify.max_user_instances" . "16384")
                 ("fs.inotify.max_queued_events" . "65536")))))))))

This is fine, until I extend sysctl-service-type in
my-base-nomad-os. Then I have to export
my-base-nomad-sysctl-settings and join them with
%default-sysctl-settings and extra settings for
nomad-gx1-os. While it is bearable for one or two levels of
inheritance, it becomes hard to keep track for more levels and/or
many hosts.

If sysctl-configuration-settings would be exported as per #47323,
then my configuration would become simplier:

(services
   (modify-services my-base-nomad-services
     (sysctl-service-type config =>
       (inherit config)
       (settings
         (append
            (sysctl-configuration-settings config) ;; now I can't
            do this
            '(("fs.inotify.max_user_watches" . "524288")
              ("fs.inotify.max_user_instances" . "16384")
              ("fs.inotify.max_queued_events" . "65536")))))))))

In this case, if Guix documentation will include
sysctl-configuration-settings, then most likely people won't
forget use %default-sysctl-settings, and it is still possible to
override them if one desires not to use protected symlinks and
hardlinks.

-- 
muradm