GNU bug report logs - #71071
[PATCH] services: nix: Mount Nix store read only.

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 71071 in the body.
You can then email your comments to 71071 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Oleg Pykhalov <go.wigust <at> gmail.com>
Subject: [PATCH] services: nix: Mount Nix store read only.
Date: Sun, 19 May 2024 22:26:15 +0300

* 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
---
 gnu/services/nix.scm | 47 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 40 insertions(+), 7 deletions(-)

diff --git a/gnu/services/nix.scm b/gnu/services/nix.scm
index 82853253f6..343b42c13a 100644
--- a/gnu/services/nix.scm
+++ b/gnu/services/nix.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019, 2020, 2021 Oleg Pykhalov <go.wigust <at> gmail.com>
+;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust <at> gmail.com>
 ;;; Copyright © 2020 Peng Mei Yu <i <at> pengmeiyu.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -97,12 +97,9 @@ (define (nix-activation _)
   #~(begin
       (use-modules (guix build utils)
                    (srfi srfi-26))
-      (for-each (cut mkdir-p <>) '("/nix/store" "/nix/var/log"
+      (for-each (cut mkdir-p <>) '("/nix/var/log"
                                    "/nix/var/nix/gcroots/per-user"
                                    "/nix/var/nix/profiles/per-user"))
-      (chown "/nix/store"
-             (passwd:uid (getpw "root")) (group:gid (getpw "nixbld01")))
-      (chmod "/nix/store" #o775)
       (for-each (cut chmod <> #o777) '("/nix/var/nix/profiles"
                                        "/nix/var/nix/profiles/per-user"))))
 
@@ -129,6 +126,24 @@ (define nix-service-etc
                                     '#$build-sandbox-items))
                     (for-each (cut display <>) '#$extra-config)))))))))))
 
+(define %nix-store-directory
+  "/nix/store")
+
+(define %nix-store-prefix
+  ;; Absolute path to the Nix store.
+  (make-parameter %nix-store-directory))
+
+(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))
+      (type "none")
+      (check? #f)
+      (flags '(read-only bind-mount))))
+
 (define nix-shepherd-service
   ;; Return a <shepherd-service> for Nix.
   (match-lambda
@@ -139,8 +154,26 @@ (define nix-shepherd-service
        (documentation "Run nix-daemon.")
        (requirement '())
        (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")))

base-commit: dd03be186adb64bdb77265dfd0ad53fe50ec016e
-- 
2.41.0

Message #8 received at 71071 <at> debbugs.gnu.org (full text, mbox):

From: Ludovic Courtès <ludo <at> gnu.org>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: 71071 <at> debbugs.gnu.org
Subject: Re: [bug#71071] [PATCH] services: nix: Mount Nix store read only.
Date: Wed, 22 May 2024 17:45:08 +0200

Hello,

Oleg Pykhalov <go.wigust <at> gmail.com> 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’.

Message #11 received at 71071 <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: 71071 <at> debbugs.gnu.org
Cc: Oleg Pykhalov <go.wigust <at> gmail.com>
Subject: [PATCH] services: nix: Mount Nix store read only.
Date: Thu, 23 May 2024 07:38:23 +0300

* gnu/services/nix.scm (nix-shepherd-service): Add requirements.
(%nix-store-directory): New variable.
(nix-service-type): Add file-system-service-type extension.

Change-Id: I73c54ab8699a54be33fac6732d919c4844d1daa4
---
 gnu/services/nix.scm | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

diff --git a/gnu/services/nix.scm b/gnu/services/nix.scm
index 82853253f6..419e5968fe 100644
--- a/gnu/services/nix.scm
+++ b/gnu/services/nix.scm
@@ -1,5 +1,5 @@
 ;;; GNU Guix --- Functional package management for GNU
-;;; Copyright © 2019, 2020, 2021 Oleg Pykhalov <go.wigust <at> gmail.com>
+;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust <at> gmail.com>
 ;;; Copyright © 2020 Peng Mei Yu <i <at> pengmeiyu.com>
 ;;;
 ;;; This file is part of GNU Guix.
@@ -26,6 +26,7 @@ (define-module (gnu services nix)
   #:use-module (gnu services shepherd)
   #:use-module (gnu services web)
   #:use-module (gnu services)
+  #:use-module (gnu system file-systems)
   #:use-module (gnu system shadow)
   #:use-module (guix gexp)
   #:use-module (guix packages)
@@ -129,6 +130,20 @@ (define nix-service-etc
                                     '#$build-sandbox-items))
                     (for-each (cut display <>) '#$extra-config)))))))))))
 
+(define %nix-store-directory
+  "/nix/store")
+
+(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.
+  (list (file-system
+          (device %nix-store-directory)
+          (mount-point %nix-store-directory)
+          (type "none")
+          (check? #f)
+          (flags '(read-only bind-mount)))))
+
 (define nix-shepherd-service
   ;; Return a <shepherd-service> for Nix.
   (match-lambda
@@ -137,7 +152,7 @@ (define nix-shepherd-service
       (shepherd-service
        (provision '(nix-daemon))
        (documentation "Run nix-daemon.")
-       (requirement '())
+       (requirement '(user-processes file-system-/nix/store))
        (start #~(make-forkexec-constructor
                  (list (string-append #$package "/bin/nix-daemon")
                        #$@extra-options)
@@ -156,7 +171,9 @@ (define nix-service-type
           (service-extension activation-service-type nix-activation)
           (service-extension etc-service-type nix-service-etc)
           (service-extension profile-service-type
-                             (compose list nix-configuration-package))))
+                             (compose list nix-configuration-package))
+          (service-extension file-system-service-type
+                             (const %immutable-nix-store))))
    (description "Run the Nix daemon.")
    (default-value (nix-configuration))))
 

base-commit: dd03be186adb64bdb77265dfd0ad53fe50ec016e
-- 
2.41.0

Message #14 received at 71071 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Oleg Pykhalov <go.wigust <at> gmail.com>
Cc: Ludovic Courtès <ludo <at> gnu.org>, 71071 <at> debbugs.gnu.org
Subject: Re: [bug#71071] [PATCH] services: nix: Mount Nix store read only.
Date: Sun, 26 May 2024 21:32:18 -0400

Hi Oleg,

Oleg Pykhalov <go.wigust <at> gmail.com> writes:

> * gnu/services/nix.scm (nix-shepherd-service): Add requirements.
> (%nix-store-directory): New variable.
> (nix-service-type): Add file-system-service-type extension.
>
> Change-Id: I73c54ab8699a54be33fac6732d919c4844d1daa4

Nitpick: The Change-Id value shouldn't change between revisions of a
change (so it should eb the same as in v1, which was
I18a5d58c92c1f2b5b6dcecc3d5b439cc15bf4e49).

> ---
>  gnu/services/nix.scm | 23 ++++++++++++++++++++---
>  1 file changed, 20 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/services/nix.scm b/gnu/services/nix.scm
> index 82853253f6..419e5968fe 100644
> --- a/gnu/services/nix.scm
> +++ b/gnu/services/nix.scm
> @@ -1,5 +1,5 @@
>  ;;; GNU Guix --- Functional package management for GNU
> -;;; Copyright © 2019, 2020, 2021 Oleg Pykhalov <go.wigust <at> gmail.com>
> +;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust <at> gmail.com>
>  ;;; Copyright © 2020 Peng Mei Yu <i <at> pengmeiyu.com>
>  ;;;
>  ;;; This file is part of GNU Guix.
> @@ -26,6 +26,7 @@ (define-module (gnu services nix)
>    #:use-module (gnu services shepherd)
>    #:use-module (gnu services web)
>    #:use-module (gnu services)
> +  #:use-module (gnu system file-systems)
>    #:use-module (gnu system shadow)
>    #:use-module (guix gexp)
>    #:use-module (guix packages)
> @@ -129,6 +130,20 @@ (define nix-service-etc
>                                      '#$build-sandbox-items))
>                      (for-each (cut display <>) '#$extra-config)))))))))))
>  
> +(define %nix-store-directory
> +  "/nix/store")
> +
> +(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.
> +  (list (file-system
> +          (device %nix-store-directory)
> +          (mount-point %nix-store-directory)
> +          (type "none")
> +          (check? #f)
> +          (flags '(read-only bind-mount)))))
> +
>  (define nix-shepherd-service
>    ;; Return a <shepherd-service> for Nix.
>    (match-lambda
> @@ -137,7 +152,7 @@ (define nix-shepherd-service
>        (shepherd-service
>         (provision '(nix-daemon))
>         (documentation "Run nix-daemon.")
> -       (requirement '())
> +       (requirement '(user-processes file-system-/nix/store))
>         (start #~(make-forkexec-constructor
>                   (list (string-append #$package "/bin/nix-daemon")
>                         #$@extra-options)
> @@ -156,7 +171,9 @@ (define nix-service-type
>            (service-extension activation-service-type nix-activation)
>            (service-extension etc-service-type nix-service-etc)
>            (service-extension profile-service-type
> -                             (compose list nix-configuration-package))))
> +                             (compose list nix-configuration-package))
> +          (service-extension file-system-service-type
> +                             (const %immutable-nix-store))))
>     (description "Run the Nix daemon.")
>     (default-value (nix-configuration))))

This LGTM, thanks to Ludo for suggesting this nice improvement in v2.

-- 
Thanks,
Maxim

Message #19 received at 71071-done <at> debbugs.gnu.org (full text, mbox):

From: Oleg Pykhalov <go.wigust <at> gmail.com>
To: 71071-done <at> debbugs.gnu.org
Cc: Ludovic Courtès <ludo <at> gnu.org>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: [bug#71071] [PATCH] services: nix: Mount Nix store read only.
Date: Wed, 29 May 2024 06:32:37 +0300

[Message part 1 (text/plain, inline)]

Hello Maxim and Ludovic.

Maxim Cournoyer <maxim.cournoyer <at> gmail.com> writes:

>> * gnu/services/nix.scm (nix-shepherd-service): Add requirements.
>> (%nix-store-directory): New variable.
>> (nix-service-type): Add file-system-service-type extension.
>>
>> Change-Id: I73c54ab8699a54be33fac6732d919c4844d1daa4
>
> Nitpick: The Change-Id value shouldn't change between revisions of a
> change (so it should eb the same as in v1, which was
> I18a5d58c92c1f2b5b6dcecc3d5b439cc15bf4e49).

Oh, I wasn't aware of that. Thanks for pointing it out. I've updated the
Change-Id and pushed the commit as
797be0ea5c3703ad96acd32c98dca5f946cf5c95.

[…]

> This LGTM, thanks to Ludo for suggesting this nice improvement in v2.

Yes, thanks for the suggestions. All of them have been implemented.


Regards,
Oleg.

[signature.asc (application/pgp-signature, inline)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.