[PATCH] gnu: Harden filesystem links.

  • Done
  • quality assurance status badge
Details
4 participants
  • Julien Lepiller
  • Leo Famulari
  • Ludovic Courtès
  • muradm
Owner
unassigned
Submitted by
Leo Famulari
Severity
normal
L
L
Leo Famulari wrote on 8 Mar 2021 21:50
(address . guix-patches@gnu.org)
7072c80a192f3c136cb70da4a0662d77ce508b56.1615236603.git.leo@famulari.name
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:


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

Toggle diff (36 lines)
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
L
L
Leo Famulari wrote on 12 Mar 2021 23:05
(address . 47013@debbugs.gnu.org)
YEvlv7v2LawKE0rF@jasmine.lan
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:


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

Toggle diff (37 lines)
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"))))))
--
2.30.2
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmBL5bgACgkQJkb6MLrK
fwjZwhAA8dZuCUj25k2K8ou2qMrviaayhf1lqAM+M7jW9WGVxMy2p9+5TkWGJsT4
tUZhaiEZewgzKKkZ+GZS25yZxu6JZnnVrj6qaP/O82aOL+B566CxWBAyqMShxMQ2
H7STEtf9OOwrnIlVBnO2ayCieWWYqsqpcq9N2bzHnHnDVn13W8KyKBRvTz2KEETj
o4OzQcXiOVYpYgAfTb0f6e/H2cDUsVzAUgdUF2wLaKw1Kg1Nn3Pgo0aZbcKH1uZt
vKAnpS95MQeXNinaqcpUS8YYRha0zgGMJi6SsmvJyaJFkA5vwJHJF8wpSb6vMwGs
RvYdGQVSkfmB1e0F7OJahECbPLq5WCiXyjVAy48PlFO82GBz3L9fb6bWpferMB4j
2jDTNUSX2gK8F2RWXoQfORDsa5HIS0srqesek8EmxUNLPS/UCT/r22QwBgNcmqCc
ItuFN8oLXIpMqWF+zQlMPhl+PH3L2N2nS0pUedmMETauWiRSQpcqsz0dpfAGnBxK
XWpSnY0AwTdHL3GtP0adR1WcbsBKpIfVYO3rRyn58ce4XqwlQWBtGsfetgYgLovZ
ajhFgT6x/76yawssZbLkVbMYyVCId2s5IOQFBprHlcCNjMb1FJFe0GprymsDBfbK
gl5YTL4uGcOaKneeJGinK4F6OwL49rhdh6y4HeSaTtDYdbiGTMA=
=Qvab
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 12 Mar 2021 23:51
(address . 47013@debbugs.gnu.org)
YEvwaTcZTyzk8O/U@jasmine.lan
On Fri, Mar 12, 2021 at 05:05:51PM -0500, Leo Famulari wrote:
Toggle quote (3 lines)
> 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?
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmBL8GkACgkQJkb6MLrK
fwi5bRAA3jeOwnOXT2KjNxoyazHG+1nNHGV/4LhCUOu7j5zu3WyXm9RcCx70TXUy
S3CVCiga6oNragbFBqTymCDlRfJkmJuZu6DHhuv9zEOAopimvH08iOffhb3k4nou
kQAeetOU7viPeAtKpAJChTVdxSJLy9yisOK397HdebHHu9D3zIt0VPj9PRVOOJpG
yTEHVUwqNswjkMVbC9I8Am/fGqP1V6lG5XZmkTQHr7jC2s2C+e7HYYjJ0cBXmrNq
O0mnRP1zN0+6aKQFLBC11AdEt7mGNbwJDMgATI5T1KJa6JH79Es1Yr9NfRVYA1MM
0uBtrjelOXymrdQwmqE39Kr4+XUb+ahODzfDszCBhE3xZpqygMZNXYZd8COD7mXG
M33b6Sqd4M0WHbJyn3Kk6zRiu3xJ+t48DZ68ob6U3JUhiSLhdnvltgodFVP3lKQe
ZdyaSBerH3VrloxxENEGsn2Oxn0v58ZRdTnyrPrczomtMgoPl3ZDugEBJY/v93d/
py7ORHbs4CbSl7PgmzsQR9lN+Jk0PajkxV3QlMa+oAQr/MNXY0KTbSJCpHYodXhK
zAeLMTE5OvGtyq7G+n83uIEUhz9dX/vzLsh34WKd5UVEV1Bs4ry6P4YKMBOvFG1f
fz73Ps0fjX5jhWyJyw2ag7QKROBhpqT9TIwOA3KXAzqjomZ1g1E=
=mom5
-----END PGP SIGNATURE-----


L
L
Leo Famulari wrote on 15 Mar 2021 19:56
(address . 47013@debbugs.gnu.org)
YE+txs/5q6sfGsKn@jasmine.lan
On Fri, Mar 12, 2021 at 05:51:21PM -0500, Leo Famulari wrote:
Toggle quote (3 lines)
> 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.
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(-)

Toggle diff (110 lines)
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))
;;;
@@ -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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmBPrcIACgkQJkb6MLrK
fwj95RAA2lgRtUbuJJreBv3MXoaSdoqmJP/ZhWfMFbcEAeHwK3KqP6Csk57id38y
gFUWl5WYjSNE6BQJpyX6rSlNgXgOHhu51eBfjE1XRr0LHIHgBxYYs9XHVrIm4jvv
iIPYLrBjN0z5KdolP++BKVVBmkjgf54VjkYxzmawKY1LaFz+6u7hIcOuTBJZMEyt
oNSXv+/69PITRe1dP/FYmVME9XcCMrN1nPDcqVIqDQJT9u3i4XJ+gSE6TXcQRCLX
t3pjlrJMvUN0JFcSBnJwg5D3hX0/yOXbYoHuM+5LmWlS3SpoU/uQb35BH16aIdtI
BVnETWwUSYu3iBhQTjUMmFK4nICApPve/xUi1aCwr2z0IY9s44uKUoOCNQX/u2tp
uA6tCT7JtLUIN3QayCWBF43MIVYlNYxLTAXbzkd5PIEZdy1VH0QFvsxKEdhVJYlk
jeCL3ybMzLB+TABm34z76sAFQl+x6A1/DV3i7AvaDj5DZSe9awmOPVIp6i8Ulcpe
NcXfaT2GhY4lTxy+ZRdF4n7oSfej6iGDW2HA0jxwEyZmcS8oddIL+EebLC5xn4yq
kQYBPLgyJXo+l9KgD9mxqn7YZvLkm9cN2u/T8TAlMe1YrEKJtviMF/CLKRE+qufN
f+9FAk3ngNs+rDhgyIYUOLBAwlgnlVOo1t9vaxhMffytUfD5rq8=
=kjyl
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 16 Mar 2021 22:42
Re: bug#47013: [PATCH] gnu: Harden filesystem links.
(name . Leo Famulari)(address . leo@famulari.name)(address . 47013@debbugs.gnu.org)
87blbi7p33.fsf_-_@gnu.org
Hi!

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (12 lines)
> 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>

[...]

Toggle quote (9 lines)
> +(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)))))

[...]

Toggle quote (4 lines)
> + (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’.
L
L
Ludovic Courtès wrote on 16 Mar 2021 23:18
(name . Leo Famulari)(address . leo@famulari.name)(address . 47013@debbugs.gnu.org)
8735wu7nf9.fsf_-_@gnu.org
Hi!

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (7 lines)
> 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’.
L
L
Leo Famulari wrote on 17 Mar 2021 01:54
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47013@debbugs.gnu.org)
YFFTXPcse7S9w8Q4@jasmine.lan
On Tue, Mar 16, 2021 at 11:18:18PM +0100, Ludovic Court�s wrote:
Toggle quote (2 lines)
> 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.
L
L
Leo Famulari wrote on 17 Mar 2021 03:14
Re: bug#47013: [PATCH v4] gnu: Harden filesystem links.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47013@debbugs.gnu.org)
YFFl6C4hBQTLBXNO@jasmine.lan
On Tue, Mar 16, 2021 at 08:54:52PM -0400, Leo Famulari wrote:
Toggle quote (3 lines)
> 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
------
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:


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

Toggle diff (48 lines)
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))
;;;
@@ -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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmBRZegACgkQJkb6MLrK
fwiMCw/+OUkbbeQtfhKVp06G7Rl/VxsFziHcf4MQo295QEwRXDXdMyvS2thek/hk
mCV9nLgeBcQaqyYnfgSp1X5KW1tJAqy7+bZ6W9DC64FzyzD2ZmUbx4+9b7aIA7cH
y0XWbYoRnpKwCZ1eAPb/M0weqwogBDW97JJm0EcQVfgIwXWZJHPdQqesRO17mEoq
VvsapnfvHrAT3YY4/2RDjWC4jOCervJpx5Giavdnrb5rhCG8oojvJO+rnguAxIod
csodPrc7MK9eOdUcwtsU7hUh0XoeJjyYT0Dot32kB3FDLy3fFAgyqz5BQGoY0QsG
hjqlUT/XwbNC1Lqvy8qE0uIjhmbxp5esxCCTJzXkZt997RDtNG/rKFzyqZHWVuDy
6i0cd4xNkwKuwRxpRdKWryeuTPC7ja0LDVMSdzifoCXiAnPhqdl2zp+3BWhuvIdR
/g1Lr1kSzoAjclgvNhp6L0/C/7SbWJVAVH2VhgaPYYXpSSn312UJAfosQB1tT5e6
+yhU1u90iqQNLwmX2l2BhJWpVs1FMrxvbW01tNooZbXyeSsnmJqYBfFdPOOaik00
eJoa6NIrPE65Ia/zjY575fdkOzcS4RmiRMAmd5Nc/0F0Fqv9k1WtIKp/CSWp9cYa
Nh6E6LwwFbxR2Odu1g2b6qUyW11wJ1erxriU73H76llhqd/3oA0=
=tbWI
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 17 Mar 2021 21:49
Re: bug#47013: [PATCH] gnu: Harden filesystem links.
(name . Leo Famulari)(address . leo@famulari.name)(address . 47013@debbugs.gnu.org)
875z1pzetb.fsf_-_@gnu.org
Hi,

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (16 lines)
> 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:

Toggle quote (17 lines)
> +++ 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…

Toggle quote (21 lines)
> +++ 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))
>
>
> ;;;
> @@ -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’.
L
L
Leo Famulari wrote on 17 Mar 2021 22:01
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47013@debbugs.gnu.org)
YFJuQr58VrrCu+Rl@jasmine.lan
On Wed, Mar 17, 2021 at 09:49:04PM +0100, Ludovic Courtès wrote:
Toggle quote (8 lines)
> [...]
> … 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.
L
L
Leo Famulari wrote on 18 Mar 2021 08:27
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47013@debbugs.gnu.org)
YFMAxV8G62Vz2gPy@jasmine.lan
On Wed, Mar 17, 2021 at 05:01:54PM -0400, Leo Famulari wrote:
Toggle quote (2 lines)
> Sure, I'll implement your suggestions and send a v5 patch.

Here is the revised patch.
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:


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

Toggle diff (100 lines)
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))
;;;
;;; 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
-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEsFFZSPHn08G5gDigJkb6MLrKfwgFAmBTAMUACgkQJkb6MLrK
fwj1gw/9FoUVoLN8NsY04ZMeplPZUWwMaRfag5qLsmOzBOVbNoBEy12DvdlKHPI0
sVtPwENE/qCbT52oLyR/WKl9AfvfXdW1TXqdVK2K0yMbwOlqbCvoWTe5K2gr+ymq
Edq+FS4WR/0aOmz+JnnjuzSDxVJnvfpHhpqRCl7sjfNALjSGoUUIBc4I6zKl4coM
EJqBJFhH5vKS5j1phcZDdwd3QdSXeER2fSUGo5JSaPaCgNoKwdLF8olbNcU2Vli4
toou/45U2Z0j8iUaF5UJa0uxa3RstdHCGyvz/5k9nvTqx/4lkC3/bXZy8zGmtmY/
mlJ8W64twCnxbqQzelpEOjH3Q/IWBHTDo1bqPiOmC9RagSCFhKpbp+CgqLgJj4ck
AetejdDQNO22KG5i5VVf6XQtK85Fm/7RK4dwkgnWIIq6R8QrSQjaYtTONtbaC1+z
twZi5nqjQoHKQk2CMpjIEu2zx7jHLBYQjX6eEYuVHJUW9Da4XTbQEBLymz7yObcP
DP/UE3q2xWtwr1fexf0jahHZ4l0E7uUwleE3HhcJXpaZfSMQu+imAlhs1CdkySIY
TBQ8uIHitBKHCI/7D8G63maV2xuWA4wvFJDG2WL6mLubaB/o8PjdYjBSjfdomaI/
RYzubCKiwA5oju6DjVIWlvXM1ss2xlaJ7HnH1yayYeGcBaLnhy4=
=WAvb
-----END PGP SIGNATURE-----


L
L
Ludovic Courtès wrote on 18 Mar 2021 10:36
(name . Leo Famulari)(address . leo@famulari.name)(address . 47013@debbugs.gnu.org)
87o8fgvm55.fsf@gnu.org
Hi Leo,

Leo Famulari <leo@famulari.name> skribis:

Toggle quote (15 lines)
> 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’.
J
J
Julien Lepiller wrote on 15 Mar 2021 21:23
Re: [bug#47013] [PATCH] gnu: Harden filesystem links.
2F1A50F6-D801-4DA6-9513-97644293A990@lepiller.eu
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 :
Toggle quote (9 lines)
>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: file
L
L
Leo Famulari wrote on 18 Mar 2021 18:25
Re: bug#47013: [PATCH] gnu: Harden filesystem links.
(name . Ludovic Courtès)(address . ludo@gnu.org)(address . 47013@debbugs.gnu.org)
YFONBYWuwrY/bruH@jasmine.lan
On Thu, Mar 18, 2021 at 10:36:38AM +0100, Ludovic Court�s wrote:
Toggle quote (21 lines)
> 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
L
L
Leo Famulari wrote on 18 Mar 2021 18:25
(no subject)
(address . control@debbugs.gnu.org)
YFONEpeoWhcyaSRP@jasmine.lan
close 47013
L
L
Leo Famulari wrote on 18 Mar 2021 18:39
Re: [bug#47013] [PATCH] gnu: Harden filesystem links.
(name . Julien Lepiller)(address . julien@lepiller.eu)(address . 47013@debbugs.gnu.org)
YFOQSOJoRGCHvngh@jasmine.lan
On Mon, Mar 15, 2021 at 04:23:24PM -0400, Julien Lepiller wrote:
Toggle quote (4 lines)
> 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:

J
J
Julien Lepiller wrote on 18 Mar 2021 20:45
(name . Leo Famulari)(address . leo@famulari.name)(address . 47013@debbugs.gnu.org)
904B0B62-0F07-429B-910D-F66C90304E45@lepiller.eu
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 :
Toggle quote (12 lines)
>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: file
M
M
muradm wrote on 24 Mar 2021 08:19
(no subject)
(address . 47013@debbugs.gnu.org)
878s6dnhmm.fsf@muradm.net
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.
M
M
muradm wrote on 24 Mar 2021 11:38
services: export sysctl-configuration record field accessors
(address . 47013@debbugs.gnu.org)
20210324103842.bzd66b47qxyzeqha@muradm-aln1
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
?