unofficial mirror of guix-patches@gnu.org 
 help / color / mirror / code / Atom feed
* [bug#71071] [PATCH] services: nix: Mount Nix store read only.
@ 2024-05-19 19:26 Oleg Pykhalov
  2024-05-22 15:45 ` Ludovic Courtès
  0 siblings, 1 reply; 5+ messages in thread
From: Oleg Pykhalov @ 2024-05-19 19:26 UTC (permalink / raw)
  To: 71071; +Cc: Oleg Pykhalov

* 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@gmail.com>
+;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2020 Peng Mei Yu <i@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





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [bug#71071] [PATCH] services: nix: Mount Nix store read only.
  2024-05-19 19:26 [bug#71071] [PATCH] services: nix: Mount Nix store read only Oleg Pykhalov
@ 2024-05-22 15:45 ` Ludovic Courtès
  2024-05-23  4:38   ` Oleg Pykhalov
  0 siblings, 1 reply; 5+ messages in thread
From: Ludovic Courtès @ 2024-05-22 15:45 UTC (permalink / raw)
  To: Oleg Pykhalov; +Cc: 71071

Hello,

Oleg Pykhalov <go.wigust@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’.




^ permalink raw reply	[flat|nested] 5+ messages in thread

* [bug#71071] [PATCH] services: nix: Mount Nix store read only.
  2024-05-22 15:45 ` Ludovic Courtès
@ 2024-05-23  4:38   ` Oleg Pykhalov
  2024-05-27  1:32     ` Maxim Cournoyer
  0 siblings, 1 reply; 5+ messages in thread
From: Oleg Pykhalov @ 2024-05-23  4:38 UTC (permalink / raw)
  To: 71071; +Cc: Oleg Pykhalov

* 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@gmail.com>
+;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust@gmail.com>
 ;;; Copyright © 2020 Peng Mei Yu <i@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





^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [bug#71071] [PATCH] services: nix: Mount Nix store read only.
  2024-05-23  4:38   ` Oleg Pykhalov
@ 2024-05-27  1:32     ` Maxim Cournoyer
  2024-05-29  3:32       ` bug#71071: " Oleg Pykhalov
  0 siblings, 1 reply; 5+ messages in thread
From: Maxim Cournoyer @ 2024-05-27  1:32 UTC (permalink / raw)
  To: Oleg Pykhalov; +Cc: Ludovic Courtès, 71071

Hi Oleg,

Oleg Pykhalov <go.wigust@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@gmail.com>
> +;;; Copyright © 2019, 2020, 2021, 2024 Oleg Pykhalov <go.wigust@gmail.com>
>  ;;; Copyright © 2020 Peng Mei Yu <i@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




^ permalink raw reply	[flat|nested] 5+ messages in thread

* bug#71071: [PATCH] services: nix: Mount Nix store read only.
  2024-05-27  1:32     ` Maxim Cournoyer
@ 2024-05-29  3:32       ` Oleg Pykhalov
  0 siblings, 0 replies; 5+ messages in thread
From: Oleg Pykhalov @ 2024-05-29  3:32 UTC (permalink / raw)
  To: 71071-done; +Cc: Ludovic Courtès, Maxim Cournoyer

[-- Attachment #1: Type: text/plain, Size: 828 bytes --]

Hello Maxim and Ludovic.

Maxim Cournoyer <maxim.cournoyer@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.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2024-05-29  3:34 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-19 19:26 [bug#71071] [PATCH] services: nix: Mount Nix store read only Oleg Pykhalov
2024-05-22 15:45 ` Ludovic Courtès
2024-05-23  4:38   ` Oleg Pykhalov
2024-05-27  1:32     ` Maxim Cournoyer
2024-05-29  3:32       ` bug#71071: " Oleg Pykhalov

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).