[PATCH] gnu: system: Privilege programs after creating accounts.

  • Done
  • quality assurance status badge
Details
2 participants
  • Dariqq
  • Ludovic Courtès
Owner
unassigned
Submitted by
Dariqq
Severity
normal
D
D
Dariqq wrote on 12 Oct 09:55 +0200
(address . guix-patches@gnu.org)(name . Dariqq)(address . dariqq@posteo.net)
aa111d331df102534aa8850b1be424b2391599ee.1728719758.git.dariqq@posteo.net
Ensure that users and groups are already created when the privileging script
runs. The order these scripts appear in the folded activation-service depends
on the order these services are instantiated in the operating-system.


* gnu/system.scm (operating-system-default-essential-services): Move
privileged-program-service above account-service.
(hurd-default-essential-services): Likewise.

Change-Id: I662fb1eff42e4088496fccb76e0efbf2b1da096e
---
Hi,
I tested that this fixes my problem of setting something suid to a new user. For the hurd change i only looked at the final value of activation-service type in hurd-barebones-os and confirmed that
'#<gexp gnu/system/shadow.scm:430:4>' is before #<gexp gnu/services.scm:922:6> (which is the privileging script).
I would prefer a solution that also models this dependency to not depend on input order but this might be tricky.


gnu/system.scm | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

Toggle diff (52 lines)
diff --git a/gnu/system.scm b/gnu/system.scm
index 44f93f91d1..c19730b331 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -809,6 +809,11 @@ (define (operating-system-default-essential-services os)
%shepherd-root-service
(pam-root-service (operating-system-pam-services os))
+ ;; Make sure that privileged-programs activation script
+ ;; runs after accounts are created
+ (service privileged-program-service-type
+ (append (operating-system-privileged-programs os)
+ (operating-system-setuid-programs os)))
(account-service (append (operating-system-accounts os)
(operating-system-groups os))
(operating-system-skeletons os))
@@ -826,9 +831,6 @@ (define (operating-system-default-essential-services os)
(operating-system-environment-variables os))
(service host-name-service-type host-name)
procs root-fs
- (service privileged-program-service-type
- (append (operating-system-privileged-programs os)
- (operating-system-setuid-programs os)))
(service profile-service-type
(operating-system-packages os))
boot-fs non-boot-fs
@@ -850,6 +852,11 @@ (define (hurd-default-essential-services os)
(service shepherd-root-service-type)
(service user-processes-service-type)
+ ;; Make sure that privileged-programs activation script
+ ;; runs after accounts are created
+ (service privileged-program-service-type
+ (append (operating-system-privileged-programs os)
+ (operating-system-setuid-programs os)))
(account-service (append (operating-system-accounts os)
(operating-system-groups os))
(operating-system-skeletons os))
@@ -866,9 +873,6 @@ (define (hurd-default-essential-services os)
(list `("hosts" ,hosts-file)))
(service hosts-service-type
(local-host-entries host-name)))
- (service privileged-program-service-type
- (append (operating-system-privileged-programs os)
- (operating-system-setuid-programs os)))
(service profile-service-type (operating-system-packages os)))))
(define* (operating-system-services os)

base-commit: b8fd792ea267cb920da0651074a533d8abf00488
--
2.46.0
D
D
Dariqq wrote on 18 Oct 15:21 +0200
[PATCH v2 1/2] gnu: system: Privilege programs after creating accounts.
(address . 73767@debbugs.gnu.org)(name . Dariqq)(address . dariqq@posteo.net)
99be45a2fa553c2174a7062056ac7fced444ad5e.1729257683.git.dariqq@posteo.net
Ensure that users and groups are already created when the privileging script
runs. The order these scripts appear in the folded activation-service depends
on the order these services are instantiated in the operating-system.


* gnu/system.scm (operating-system-default-essential-services): Move
privileged-program-service above account-service.
(hurd-default-essential-services): Likewise.

Change-Id: I662fb1eff42e4088496fccb76e0efbf2b1da096e
---
gnu/system.scm | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)

Toggle diff (52 lines)
diff --git a/gnu/system.scm b/gnu/system.scm
index 44f93f91d1..c19730b331 100644
--- a/gnu/system.scm
+++ b/gnu/system.scm
@@ -809,6 +809,11 @@ (define (operating-system-default-essential-services os)
%shepherd-root-service
(pam-root-service (operating-system-pam-services os))
+ ;; Make sure that privileged-programs activation script
+ ;; runs after accounts are created
+ (service privileged-program-service-type
+ (append (operating-system-privileged-programs os)
+ (operating-system-setuid-programs os)))
(account-service (append (operating-system-accounts os)
(operating-system-groups os))
(operating-system-skeletons os))
@@ -826,9 +831,6 @@ (define (operating-system-default-essential-services os)
(operating-system-environment-variables os))
(service host-name-service-type host-name)
procs root-fs
- (service privileged-program-service-type
- (append (operating-system-privileged-programs os)
- (operating-system-setuid-programs os)))
(service profile-service-type
(operating-system-packages os))
boot-fs non-boot-fs
@@ -850,6 +852,11 @@ (define (hurd-default-essential-services os)
(service shepherd-root-service-type)
(service user-processes-service-type)
+ ;; Make sure that privileged-programs activation script
+ ;; runs after accounts are created
+ (service privileged-program-service-type
+ (append (operating-system-privileged-programs os)
+ (operating-system-setuid-programs os)))
(account-service (append (operating-system-accounts os)
(operating-system-groups os))
(operating-system-skeletons os))
@@ -866,9 +873,6 @@ (define (hurd-default-essential-services os)
(list `("hosts" ,hosts-file)))
(service hosts-service-type
(local-host-entries host-name)))
- (service privileged-program-service-type
- (append (operating-system-privileged-programs os)
- (operating-system-setuid-programs os)))
(service profile-service-type (operating-system-packages os)))))
(define* (operating-system-services os)

base-commit: 061e0acd596262420facef7c2d1fc9cc4327d75a
--
2.46.0
D
D
Dariqq wrote on 18 Oct 15:21 +0200
[PATCH v2 2/2] tests: Add activation test.
(address . 73767@debbugs.gnu.org)(name . Dariqq)(address . dariqq@posteo.net)
9e16b82e73de5da03c2aa8763a970fbdd513a83d.1729257683.git.dariqq@posteo.net
Add a test to verify that accounts are available for activation scripts.

* gnu/tests/base.scm (%activation-os): New variable.
(run-activation-test): New procedure.
(%test-activation): New variable.

Change-Id: I59a191c5519475f256e81bdf2dc4cb01b96c31fe
---
gnu/tests/base.scm | 121 ++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 120 insertions(+), 1 deletion(-)

Toggle diff (152 lines)
diff --git a/gnu/tests/base.scm b/gnu/tests/base.scm
index e1a676ecd4..9430cbee12 100644
--- a/gnu/tests/base.scm
+++ b/gnu/tests/base.scm
@@ -3,6 +3,7 @@
;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
;;; Copyright © 2022 Maxim Cournoyer <maxim.cournoyer@gmail.com>
;;; Copyright © 2022 Marius Bakke <marius@gnu.org>
+;;; Copyright © 2024 Dariqq <dariqq@posteo.net>
;;;
;;; This file is part of GNU Guix.
;;;
@@ -24,6 +25,7 @@ (define-module (gnu tests base)
#:use-module (gnu image)
#:use-module (gnu system)
#:autoload (gnu system image) (system-image)
+ #:use-module (gnu system privilege)
#:use-module (gnu system shadow)
#:use-module (gnu system nss)
#:use-module (gnu system vm)
@@ -60,7 +62,8 @@ (define-module (gnu tests base)
%test-root-unmount
%test-cleanup
%test-mcron
- %test-nss-mdns))
+ %test-nss-mdns
+ %test-activation))
(define %simple-os
(simple-operating-system))
@@ -1105,3 +1108,119 @@ (define %test-nss-mdns
"Test Avahi's multicast-DNS implementation, and in particular, test its
glibc name service switch (NSS) module.")
(value (run-nss-mdns-test))))
+
+
+;;;
+;;; Activation: Order of activation scripts
+;;; Create accounts before running scripts using them
+
+(define %activation-os
+ ;; System with a new user/group, a setuid/setgid binary and an activation script
+ (let* ((%hello-accounts
+ (list (user-group (name "hello") (system? #t))
+ (user-account
+ (name "hello")
+ (group "hello")
+ (system? #t)
+ (comment "")
+ (home-directory "/var/empty"))))
+ (%hello-privileged
+ (list
+ (privileged-program
+ (program (file-append hello "/bin/hello"))
+ (setuid? #t)
+ (setgid? #t)
+ (user "hello")
+ (group "hello"))))
+ (%hello-activation
+ (with-imported-modules (source-module-closure
+ '((gnu build activation)))
+ #~(begin
+ (use-modules (gnu build activation))
+
+ (let ((user (getpwnam "hello")))
+ (mkdir-p/perms "/run/hello" user #o755)))))
+
+ (hello-service-type
+ (service-type
+ (name 'hello)
+ (extensions
+ (list (service-extension account-service-type
+ (const %hello-accounts))
+ (service-extension activation-service-type
+ (const %hello-activation))
+ (service-extension privileged-program-service-type
+ (const %hello-privileged))))
+ (default-value #f)
+ (description ""))))
+
+ (operating-system
+ (inherit %simple-os)
+ (services
+ (cons* (service hello-service-type)
+ (operating-system-user-services
+ %simple-os))))))
+
+(define (run-activation-test name)
+ (define os
+ (marionette-operating-system
+ %activation-os))
+
+ (define test
+ (with-imported-modules '((gnu build marionette))
+ #~(begin
+ (use-modules (gnu build marionette)
+ (srfi srfi-64))
+
+ (define marionette
+ (make-marionette (list #$(virtual-machine os))))
+
+ (test-runner-current (system-test-runner #$output))
+ (test-begin "activation")
+
+ (test-assert "directory exists"
+ (marionette-eval
+ '(file-exists? "/run/hello")
+ marionette))
+
+ (test-assert "directory correct permissions and owner"
+ (marionette-eval
+ '(let ((dir (stat "/run/hello"))
+ (user (getpwnam "hello")))
+ (and (eqv? (stat:uid dir)
+ (passwd:uid user))
+ (eqv? (stat:gid dir)
+ (passwd:gid user))
+ (= (stat:perms dir)
+ #o0755)))
+ marionette))
+
+ (test-assert "privileged-program exists"
+ (marionette-eval
+ '(file-exists? "/run/privileged/bin/hello")
+ marionette))
+
+ (test-assert "privileged-program correct permissions and owner"
+ (marionette-eval
+ '(let ((binary (stat "/run/privileged/bin/hello"))
+ (user (getpwnam "hello"))
+ (group (getgrnam "hello")))
+ (and (eqv? (stat:uid binary)
+ (passwd:uid user))
+ (eqv? (stat:gid binary)
+ (group:gid group))
+ (= (stat:perms binary)
+ (+ #o0555 ;; base
+ #o4000 ;; setuid
+ #o2000)))) ;; setgid
+ marionette))
+
+ (test-end))))
+
+ (gexp->derivation name test))
+
+(define %test-activation
+ (system-test
+ (name "activation")
+ (description "Test that activation scripts are run in the correct order")
+ (value (run-activation-test name))))
--
2.46.0
L
L
Ludovic Courtès wrote on 24 Oct 12:14 +0200
Re: [bug#73767] [PATCH] gnu: system: Privilege programs after creating accounts.
(name . Dariqq)(address . dariqq@posteo.net)
87bjz924f9.fsf@gnu.org
Hi Dariqq,

Dariqq <dariqq@posteo.net> skribis:

Toggle quote (12 lines)
> Ensure that users and groups are already created when the privileging script
> runs. The order these scripts appear in the folded activation-service depends
> on the order these services are instantiated in the operating-system.
>
> Fixes https://issues.guix.gnu.org/73680.
>
> * gnu/system.scm (operating-system-default-essential-services): Move
> privileged-program-service above account-service.
> (hurd-default-essential-services): Likewise.
>
> Change-Id: I662fb1eff42e4088496fccb76e0efbf2b1da096e

[...]

Toggle quote (2 lines)
> I would prefer a solution that also models this dependency to not depend on input order but this might be tricky.

Yes, that would be best.

I applied both patches and took the liberty to squash them: we usually
arrange to have the bug-fix and the test that exhibits the bug in the
same commit, for clarity.

Thanks for the investigation & fix!

Ludo’.
Closed
?
Your comment

This issue is archived.

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

To respond to this issue using the mumi CLI, first switch to it
mumi current 73767
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