diff mbox series

[bug#47013] gnu: Harden filesystem links.

Message ID 7072c80a192f3c136cb70da4a0662d77ce508b56.1615236603.git.leo@famulari.name
State Accepted
Headers show
Series [bug#47013] gnu: Harden filesystem links. | expand

Checks

Context Check Description
cbaines/submitting builds success
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch success View Laminar job
cbaines/issue success View issue

Commit Message

Leo Famulari March 8, 2021, 8:50 p.m. UTC
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(-)

Comments

muradm March 24, 2021, 7:19 a.m. UTC | #1
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 March 24, 2021, 10:38 a.m. UTC | #2
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.
diff mbox series

Patch

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"))))))