Subject: Re: [bug#71071] [PATCH] services: nix: Mount Nix store read only.

From: Ludovic Courtès <[email-masked]>
Date: Wed, 22 May 2024 17:45:08 +0200

Hello,

Oleg Pykhalov <[email-masked]> skribis:

> * gnu/services/nix.scm (nix-shepherd-service): Mount Nix store read only.
> (%nix-store-directory, %immutable-nix-store): New variables.
> (%nix-store-prefix): New parameter.
> (nix-activation): Move /nix/store provision to 'nix-shepherd-service'.
>
> Change-Id: I18a5d58c92c1f2b5b6dcecc3d5b439cc15bf4e49

That’s a good idea.  Some suggestions:

> +(define %nix-store-directory
> +  "/nix/store")
> +
> +(define %nix-store-prefix
> +  ;; Absolute path to the Nix store.
> +  (make-parameter %nix-store-directory))

I think you can omit this parameter and simply use
‘%nix-store-directory’ because…

> +(define %immutable-nix-store
> +  ;; Read-only store to avoid users or daemons accidentally modifying it.
> +  ;; 'nix-daemon' has provisions to remount it read-write in its own name
> +  ;; space.
> +  #~(file-system
> +      (device #$(%nix-store-prefix))
> +      (mount-point #$(%nix-store-prefix))

… the parameter is used at the top-level anyway, so changing its value
won’t have any effect.

>         (start #~(make-forkexec-constructor
> -                 (list (string-append #$package "/bin/nix-daemon")
> -                       #$@extra-options)
> +                 (list
> +                  #$(program-file
> +                     "nix-daemon-wrapper"
> +                     (with-imported-modules (source-module-closure '((gnu build file-systems)
> +                                                                     (gnu system file-systems)))
> +                       #~(begin
> +                           (use-modules (gnu build file-systems)
> +                                        (gnu system file-systems)
> +                                        (guix build syscalls)
> +                                        (guix build utils))
> +                           (unless (member #$(%nix-store-prefix) (mount-points))
> +                             (mkdir-p "/nix/store")
> +                             (chown "/nix/store"
> +                                    (passwd:uid (getpw "root"))
> +                                    (group:gid (getpw "nixbld01")))
> +                             (chmod "/nix/store" #o775)
> +                             (mount-file-system #$%immutable-nix-store
> +                                                #:root "/"))
> +                           (execl #$(file-append package "/bin/nix-daemon")
> +                                  "nix-daemon" #$@extra-options)))))
>                   #:environment-variables
>                   (list (string-append "TMPDIR=" #$build-directory)
>                         "PATH=/run/current-system/profile/bin")))

Instead of having this wrapper, what about extending
‘file-system-service-type’ with a read-only bind-mount <file-system>
similar to ‘%immutable-store’?

The Shepherd service that spawns nix-daemon would depend on that file
system:

  (requirement '(user-processes file-system-/nix/store))

Thanks,
Ludo’.