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

[Ludovic Courtès]

Hi,

muradm <mail@muradm.net> skribis:

>>From 0928d70c1cd5a98efd7671c05b38757400941790 Mon Sep 17 00:00:00 2001
> From: muradm <mail@muradm.net>
> Date: Mon, 22 Mar 2021 19:09:48 +0300
> Subject: [PATCH] services: export sysctl-configuration record field accessors
>
> * gnu/services/sysctl.scm (sysctl-configuration-sysctl): new public function
> * gnu/services/sysctl.scm (sysctl-configuration-settings): new public function
>
> Signed-off-by: muradm <mail@muradm.net>

I tweaked the commit log and applied.

> As per discussion with Leo on IRC #guix in relation to #47013 and
> #47323.
>
> 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 and current issue #47323.
>
> 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,
> 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.

Indeed, this is a discussion Leo Famulari and I had while preparing the
patch for this security issue.  Like you write, there are different
tradeoffs, and this solution is one possibility that looked reasonable.

Thanks!

Ludo’.