[PATCH] services: nix: Mount Nix store read only.

  • Done
  • quality assurance status badge
Details
3 participants
  • Oleg Pykhalov
  • Ludovic Courtès
  • Maxim Cournoyer
Owner
unassigned
Submitted by
Oleg Pykhalov
Severity
normal
O
O
Oleg Pykhalov wrote on 19 May 21:26 +0200
(address . guix-patches@gnu.org)(name . Oleg Pykhalov)(address . go.wigust@gmail.com)
274716c3156aa3290666ee3d33a2f1101d02d572.1716146775.git.go.wigust@gmail.com
* 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(-)

Toggle diff (83 lines)
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
L
L
Ludovic Courtès wrote on 22 May 17:45 +0200
(name . Oleg Pykhalov)(address . go.wigust@gmail.com)(address . 71071@debbugs.gnu.org)
87ttipdf5n.fsf@gnu.org
Hello,

Oleg Pykhalov <go.wigust@gmail.com> skribis:

Toggle quote (7 lines)
> * 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:

Toggle quote (7 lines)
> +(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…

Toggle quote (8 lines)
> +(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.

Toggle quote (27 lines)
> (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’.
O
O
Oleg Pykhalov wrote on 23 May 06:38 +0200
[PATCH] services: nix: Mount Nix store read only.
(address . 71071@debbugs.gnu.org)(name . Oleg Pykhalov)(address . go.wigust@gmail.com)
13d78de1d27742605cf51fc0ed91b832cb5027c9.1716439103.git.go.wigust@gmail.com
* 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(-)

Toggle diff (64 lines)
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
M
M
Maxim Cournoyer wrote on 27 May 03:32 +0200
(name . Oleg Pykhalov)(address . go.wigust@gmail.com)
87jzjgghul.fsf@gmail.com
Hi Oleg,

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

Toggle quote (6 lines)
> * 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).

Toggle quote (64 lines)
> ---
> 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
O
O
Oleg Pykhalov wrote on 29 May 05:32 +0200
(address . 71071-done@debbugs.gnu.org)
87ed9l1eei.fsf@gmail.com
Hello Maxim and Ludovic.

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

Toggle quote (10 lines)
>> * 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.

[…]

Toggle quote (2 lines)
> 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.
-----BEGIN PGP SIGNATURE-----

iQJIBAEBCgAyFiEEcjhxI46s62NFSFhXFn+OpQAa+pwFAmZWodUUHGdvLndpZ3Vz
dEBnbWFpbC5jb20ACgkQFn+OpQAa+pw7qQ/9G0O/iZbAVln2G65vfH0DpcC9j7fm
7ctNcVXysPu8Hr5jSKFma5hQeGN62EdrU5cWNJI0nHGkwRGdEIKV0K5upywa5G2K
qClLKFMTZv0D/lrw6dRhb/BG6fahllLScqpnj5wwpFokN0XYNve+WbibsHOXzyw9
V6WT3rOBxzTFsVI9emQ3x6vBf3yTfoI6NgwowSZkVlxfN89QUo46Qr5tdoQf/enq
zyDtWxGaMBJOpTosgwjHjOyXh9arvBLGhXjcLtAFif7ofse2obqKL459CW9vfV71
d8slMOyhOfvfIMd9IP+YNO8+LIesEBNHmo0CUsR4r6P9ykbqOwt5nEJaS49U6PIl
HTe9Y2N5hKfO+1CBduFNa1eW/XO76ZknXdBN15dU9y76VKN4lXbYL9a27ROth+rD
fBkfNVYT0sihm4YyD5tSGxy7NrJfKyP+DSan3wtUwaivJEdw0IzkjGMyj5Tp8WI5
zs0aMk4r3oRYXykR9y5YlbhKoA+B/Uww5FzOPNj/RwB4gybgLvM0M/TuGudg41PF
I+xsbvf0HJpu9HWBsm+vCuo3aOAbVr5LWNkogE95UEZDHklk2WxVoD8y9lkJSgNN
I/e4YXIBpjI7RHOcc5I8zgnTnPv/b69zGlBzrVJq7A7RkFH8SVZHWr+W0hwkyHDG
UtgrGgu/8VNZ+uw=
=skQT
-----END PGP SIGNATURE-----

Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 71071@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 71071
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch