all messages for Guix-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix,
@ 2021-12-12 18:28 Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 1/4] syscalls: Add 'lchown' Brice Waegeneire
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-12 18:28 UTC (permalink / raw)
  To: 52454

Hello Guix,

A number of times I got hit by newly configured service not starting because
of wrong ownership for files they ought to own.  This appear when
reconfiguring a operating system with a service that was unsed in the past,
but not present in previous generation.  For example, this time, cuirass and
postgresql service wouldn't start because that system had them running before,
but a few weeks ago I reconfigured the operating system without them and now
that I want to have these services running again they won't start because the
activation scripts were only changing the ownership of the runtime, data, log
and co. directories but not their content.  Concretely
/var/lib/postgresql/data/PG_VERSION (and others) wasn't owned by
postgresql:postgresql but by an other pair of UID/GID, however
/var/lib/postgresql had the correct ownership

This patch fix such UID/GID mismatch for the cuirass and postgresql service by
recusrivly changing the owner and group of the whole tree these services
need. And not just the root directories of theses trees. It is related to the issue
<https://issues.guix.gnu.org/45571> about stable UID/GID in Guix's containers.

Cheers,
- Brice

Brice Waegeneire (4):
  syscalls: Add 'lchown'.
  activation: Add 'lchown-recursive'.
  services: postgresql: Ensure correct ownership of directory trees.
  services: cuirass: Ensure correct ownership of directory trees.

 gnu/build/activation.scm   | 22 ++++++++++++++++++++--
 gnu/services/cuirass.scm   | 18 +++++++++++-------
 gnu/services/databases.scm | 14 +++++++++-----
 guix/build/syscalls.scm    | 16 ++++++++++++++++
 4 files changed, 56 insertions(+), 14 deletions(-)


base-commit: 604880ae22e1a7662acb1d3f282242470de0cd03
-- 
2.34.0






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

* [bug#52454] [PATCH 1/4] syscalls: Add 'lchown'.
  2021-12-12 18:28 [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Brice Waegeneire
@ 2021-12-12 18:36 ` Brice Waegeneire
  2021-12-18 21:34   ` [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Ludovic Courtès
  2021-12-12 18:36 ` [bug#52454] [PATCH 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-12 18:36 UTC (permalink / raw)
  To: 52454

* guix/build/syscalls.scm (lchown): New procedure.
---
 guix/build/syscalls.scm | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 63bd017d1d..1c432507c3 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2021 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -118,6 +119,7 @@ (define-module (guix build syscalls)
             scandir*
             getxattr
             setxattr
+            lchown
 
             fcntl-flock
             lock-file
@@ -1275,6 +1277,20 @@ (define* (scandir* name #:optional
       (lambda ()
         (closedir* directory)))))
 
+(define lchown
+  (let ((proc (syscall->procedure int "lchown" (list '* int int))))
+    (lambda (file owner group)
+      "As 'chown', change the ownership and group of the file referred to by
+FILE to the integer values OWNER and GROUP but doesn't dereference symbolic
+links.  Unlike 'chown' this doesn't support port or integer file descriptor
+via 'fchown'."
+      (let-values (((ret err)
+                    (proc (string->pointer file) owner group)))
+        (unless (zero? ret)
+          (throw 'system-error "lchown" "~S: ~A"
+                 (list file (strerror err))
+                 (list err)))))))
+
 \f
 ;;;
 ;;; Advisory file locking.
-- 
2.34.0





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

* [bug#52454] [PATCH 2/4] activation: Add 'lchown-recursive'.
  2021-12-12 18:28 [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 1/4] syscalls: Add 'lchown' Brice Waegeneire
@ 2021-12-12 18:36 ` Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 4/4] services: cuirass: " Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-12 18:36 UTC (permalink / raw)
  To: 52454

* gnu/build/activation.scm (lchown-recursive): New procedure.
---
 gnu/build/activation.scm | 22 ++++++++++++++++++++--
 1 file changed, 20 insertions(+), 2 deletions(-)

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 9f6126023c..79c835a045 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -30,7 +30,7 @@ (define-module (gnu build activation)
   #:use-module (gnu build accounts)
   #:use-module (gnu build linux-boot)
   #:use-module (guix build utils)
-  #:use-module ((guix build syscalls) #:select (with-file-lock))
+  #:use-module ((guix build syscalls) #:select (with-file-lock lchown))
   #:use-module (ice-9 ftw)
   #:use-module (ice-9 match)
   #:use-module (ice-9 vlist)
@@ -46,7 +46,8 @@ (define-module (gnu build activation)
             activate-firmware
             activate-ptrace-attach
             activate-current-system
-            mkdir-p/perms))
+            mkdir-p/perms
+            lchown-recursive))
 
 ;;; Commentary:
 ;;;
@@ -105,6 +106,23 @@ (define (mkdir-p/perms directory owner bits)
   (chown directory (passwd:uid owner) (passwd:gid owner))
   (chmod directory bits))
 
+(define (lchown-recursive file owner group)
+  "As 'lchown' but recursively, change ownership of FILE to the integer values
+OWNER and GROUP without dereferencing symbolic links it encounter."
+  (nftw file
+        (lambda (filename statinfo flag base level)
+          (catch 'system-error
+            (lambda ()
+              (when (member flag '(regular directory symlink))
+                (lchown filename owner group)))
+            (lambda args
+              (format (current-error-port)
+                      "warning: failed to chown ~s: ~a~%"
+                      filename
+                      (strerror (system-error-errno args)))))
+          #t)
+        'physical))
+
 (define* (copy-account-skeletons home
                                  #:key
                                  (directory %skeleton-directory)
-- 
2.34.0





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

* [bug#52454] [PATCH 3/4] services: postgresql: Ensure correct ownership of directory trees.
  2021-12-12 18:28 [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 1/4] syscalls: Add 'lchown' Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
@ 2021-12-12 18:36 ` Brice Waegeneire
  2021-12-12 18:36 ` [bug#52454] [PATCH 4/4] services: cuirass: " Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-12 18:36 UTC (permalink / raw)
  To: 52454

* gnu/services/databases.scm (postgresql-activation): Replace 'chown'
  calls by 'lchown-recursive'.
---
 gnu/services/databases.scm | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 8e983ef0be..83f9fe5239 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2019 Robert Vollmert <rob@vllmrt.net>
 ;;; Copyright © 2020 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2021 David Larsson <david.larsson@selfhosted.xyz>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -214,8 +215,11 @@ (define postgresql-activation
     (($ <postgresql-configuration> postgresql port locale config-file
                                    log-directory data-directory
                                    extension-packages)
-     #~(begin
+     (with-imported-modules (source-module-closure
+                             '((gnu build activation)))
+       #~(begin
          (use-modules (guix build utils)
+                      (gnu build activation)
                       (ice-9 match))
 
          (let ((user (getpwnam "postgres"))
@@ -230,19 +234,19 @@ (define postgresql-activation
                      '()))))
            ;; Create db state directory.
            (mkdir-p #$data-directory)
-           (chown #$data-directory (passwd:uid user) (passwd:gid user))
+           (lchown-recursive #$data-directory (passwd:uid user) (passwd:gid user))
 
            ;; Create the socket directory.
            (let ((socket-directory
                   #$(postgresql-config-file-socket-directory config-file)))
              (when (string? socket-directory)
                (mkdir-p socket-directory)
-               (chown socket-directory (passwd:uid user) (passwd:gid user))))
+               (lchown-recursive socket-directory (passwd:uid user) (passwd:gid user))))
 
            ;; Create the log directory.
            (when (string? #$log-directory)
              (mkdir-p #$log-directory)
-             (chown #$log-directory (passwd:uid user) (passwd:gid user)))
+             (lchown-recursive #$log-directory (passwd:uid user) (passwd:gid user)))
 
            ;; Drop privileges and init state directory in a new
            ;; process.  Wait for it to finish before proceeding.
@@ -262,7 +266,7 @@ (define postgresql-activation
                           initdb-args)))
                 (lambda ()
                   (primitive-exit 1))))
-             (pid (waitpid pid))))))))
+             (pid (waitpid pid)))))))))
 
 (define postgresql-shepherd-service
   (match-lambda
-- 
2.34.0





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

* [bug#52454] [PATCH 4/4] services: cuirass: Ensure correct ownership of directory trees.
  2021-12-12 18:28 [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Brice Waegeneire
                   ` (2 preceding siblings ...)
  2021-12-12 18:36 ` [bug#52454] [PATCH 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
@ 2021-12-12 18:36 ` Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-12 18:36 UTC (permalink / raw)
  To: 52454

* gnu/services/cuirass.scm (cuirass-activation): Replace 'chown'
  calls by 'lchown-recursive'.
---
 gnu/services/cuirass.scm | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index a69c20adb8..116c12063e 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -24,6 +25,7 @@
 (define-module (gnu services cuirass)
   #:use-module (guix channels)
   #:use-module (guix gexp)
+  #:use-module (guix modules)
   #:use-module (guix records)
   #:use-module (guix store)
   #:use-module (guix utils)
@@ -278,9 +280,11 @@ (define (cuirass-activation config)
          (profile        (string-append "/var/guix/profiles/per-user/" user))
          (roots          (string-append profile "/cuirass"))
          (group          (cuirass-configuration-group config)))
-    (with-imported-modules '((guix build utils))
+    (with-imported-modules (source-module-closure
+                            '((gnu build activation)))
       #~(begin
-          (use-modules (guix build utils))
+          (use-modules (guix build utils)
+                       (gnu build activation))
 
           (mkdir-p #$cache)
           (mkdir-p #$log)
@@ -291,13 +295,13 @@ (define (cuirass-activation config)
 
           (let ((uid (passwd:uid (getpw #$user)))
                 (gid (group:gid (getgr #$group))))
-            (chown #$cache uid gid)
-            (chown #$log uid gid)
-            (chown #$roots uid gid)
-            (chown #$profile uid gid)
+            (lchown-recursive #$cache uid gid)
+            (lchown-recursive #$log uid gid)
+            (lchown-recursive #$profile uid gid)
+            (lchown-recursive (passwd:dir (getpw #$user)) uid gid)
 
             (when #$remote-cache
-              (chown #$remote-cache uid gid)))))))
+              (lchown-recursive #$remote-cache uid gid)))))))
 
 (define (cuirass-log-rotations config)
   "Return the list of log rotations that corresponds to CONFIG."
-- 
2.34.0





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

* [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix,
  2021-12-12 18:36 ` [bug#52454] [PATCH 1/4] syscalls: Add 'lchown' Brice Waegeneire
@ 2021-12-18 21:34   ` Ludovic Courtès
  2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
  0 siblings, 1 reply; 11+ messages in thread
From: Ludovic Courtès @ 2021-12-18 21:34 UTC (permalink / raw)
  To: Brice Waegeneire; +Cc: 52454

Hi!

Great patch series!

This has been discussed a few times: I wonder if we should simply chown
service home directories systematically?

Brice Waegeneire <brice@waegenei.re> skribis:

> * guix/build/syscalls.scm (lchown): New procedure.

Would be nice to add even trivial tests to tests/syscalls.scm.

Unfortunately, this doesn’t work for service activation because when
booting, activation snippets are run from the initrd’s Guile, which is
statically linked and lacks dlopen.

This leads to failures like:

--8<---------------cut here---------------start------------->8---
$ make check-system TESTS="postgresql" -j4

[...]

populating /etc from /gnu/store/bchxln4wkfmdbsxww9jaxafsyvlpdbmg-etc...
Please wait while gathering entropy to generate the key pair;
this may take time...
warning: failed to chown "/var/lib/postgresql/data": Function not implemented
warning: failed to chown "/var/run/postgresql": Function not implemented
warning: failed to chown "/var/log/postgresql": Function not implemented
The files belonging to this database system will be owned by user "postgres".
This user must also own the server process.

[...]

fixing permissions on existing directory /var/lib/postgresql/data ... initdb: could not change permissions of directory "/var/lib/postgresql/data": Operation not permitted
--8<---------------cut here---------------end--------------->8---

(The ENOSYS error above comes from the ‘lchown’ wrapper.)

For this strategy to work, you need to add ‘lchown’ in
‘guile-3.0-linux-syscalls.patch’ and to use ‘define-as-needed’ in (guix
build syscalls).

(I’m surprised we didn’t already have recursive chown.)

With this in place, we should be all set!

Thanks,
Ludo’.




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

* [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services
  2021-12-18 21:34   ` [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Ludovic Courtès
@ 2021-12-21 19:30     ` Brice Waegeneire
  2021-12-21 19:36       ` [bug#52715] [PATCH v2 1/4] syscalls: Add 'lchown' Brice Waegeneire
                         ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-21 19:30 UTC (permalink / raw)
  To: Ludovic Courtès; +Cc: 52454

Hello Ludo’,

Here is a second version of the patch set.

Ludovic Courtès <ludo@gnu.org> writes:

> [...]
>
> This has been discussed a few times: I wonder if we should simply chown
> service home directories systematically?

#45571¹ is one of such discussion. For services' home, I guess that's what we
 should do, but it probably won't be sufficient as log or chache directories
 usualy aren't in a home, but still need to chowned. The easiest and probably
 least controversion would be to just replace current `chown` calls on
 directories by `lchown-recursive`.

Seeing that we don't want static UID/GID mapping, like most other distros do, we
could try to implement something like systemd's dynamic users² approch.

> Brice Waegeneire <brice@waegenei.re> skribis:
>
>> * guix/build/syscalls.scm (lchown): New procedure.
>
> Would be nice to add even trivial tests to tests/syscalls.scm.

I wrote 4 tests, however the last two, the ones actually testing 'lchown' fail
bescause "/tmp" has it's sticky bit set, which prevent changing ownership of
files there.  I tried to workaround this but didn't managed to.

> Unfortunately, this doesn’t work for service activation because when
> booting, activation snippets are run from the initrd’s Guile, which is
> statically linked and lacks dlopen.
>
> [...]
>
> For this strategy to work, you need to add ‘lchown’ in
> ‘guile-3.0-linux-syscalls.patch’ and to use ‘define-as-needed’ in (guix
> build syscalls).

Done and it fixes the check system for postgresql service.

¹ <https://issues.guix.gnu.org/45571>
² <https://0pointer.net/blog/dynamic-users-with-systemd.html>

Cheers,
- Brice

Brice Waegeneire (4):
  syscalls: Add 'lchown'.
  activation: Add 'lchown-recursive'.
  services: postgresql: Ensure correct ownership of directory trees.
  services: cuirass: Ensure correct ownership of directory trees.

 gnu/build/activation.scm                      | 20 +++++-
 .../patches/guile-3.0-linux-syscalls.patch    | 33 ++++++++++
 gnu/services/cuirass.scm                      | 18 +++---
 gnu/services/databases.scm                    | 14 +++--
 guix/build/syscalls.scm                       | 16 +++++
 tests/syscalls.scm                            | 62 +++++++++++++++++++
 6 files changed, 150 insertions(+), 13 deletions(-)


base-commit: 87e5502d406bfb44b61f7577b241602e02a3498e
-- 
2.34.0




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

* [bug#52715] [PATCH v2 1/4] syscalls: Add 'lchown'.
  2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
@ 2021-12-21 19:36       ` Brice Waegeneire
  2021-12-21 19:36       ` [bug#52713] [PATCH v2 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
                         ` (2 subsequent siblings)
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-21 19:36 UTC (permalink / raw)
  To: 52715

* guix/build/syscalls.scm (lchown): New procedure.
* gnu/packages/patches/guile-3.0-linux-syscalls.patch: Add lchown.
* tests/syscalls.scm ("lchown, ENOENT", "lchown, no changes",
  "lchown, regular file", "lchown, symlink"): New tests.
---
 .../patches/guile-3.0-linux-syscalls.patch    | 33 ++++++++++
 guix/build/syscalls.scm                       | 16 +++++
 tests/syscalls.scm                            | 62 +++++++++++++++++++
 3 files changed, 111 insertions(+)

diff --git a/gnu/packages/patches/guile-3.0-linux-syscalls.patch b/gnu/packages/patches/guile-3.0-linux-syscalls.patch
index 0d27f77ee2..77edd9a993 100644
--- a/gnu/packages/patches/guile-3.0-linux-syscalls.patch
+++ b/gnu/packages/patches/guile-3.0-linux-syscalls.patch
@@ -3,7 +3,40 @@ This patch adds bindings to Linux syscalls for which glibc has symbols.
 Using the FFI would have been nice, but that's not an option when using
 a statically-linked Guile in an initrd that doesn't have libc.so around.
 
+diff --git a/libguile/filesys.c b/libguile/filesys.c
+index 4f7115397..2ade4cfca 100644
+--- a/libguile/filesys.c
++++ b/libguile/filesys.c
+@@ -192,6 +192,27 @@ SCM_DEFINE (scm_chown, "chown", 3, 0, 0,
+ #undef FUNC_NAME
+ #endif /* HAVE_CHOWN */
+ 
++SCM_DEFINE (scm_lchown, "lchown", 3, 0, 0,
++            (SCM object, SCM owner, SCM group),
++            "As 'chown', change the ownership and group of the file referred to by\n"
++            "@var{file} to the integer values @var{owner} and @var{group} but\n"
++            "doesn't dereference symbolic links. Unlike 'chown' this doesn't support\n"
++            "port or integer file descriptor via 'fchown'.")
++#define FUNC_NAME s_scm_lchown
++{
++  int rv;
++
++  object = SCM_COERCE_OUTPORT (object);
++
++  STRING_SYSCALL (object, c_object,
++                  rv = lchown (c_object,
++                               scm_to_int (owner), scm_to_int (group)));
++  if (rv == -1)
++    SCM_SYSERROR;
++  return SCM_UNSPECIFIED;
++}
++#undef FUNC_NAME
++
+ \f
+ 
+ SCM_DEFINE (scm_open_fdes, "open-fdes", 2, 1, 0, 
 diff --git a/libguile/posix.c b/libguile/posix.c
+index a1520abc4..61d57cdb9 100644
 --- a/libguile/posix.c
 +++ b/libguile/posix.c
 @@ -2375,6 +2375,336 @@ scm_init_popen (void)
diff --git a/guix/build/syscalls.scm b/guix/build/syscalls.scm
index 45f95c509d..dbb96997d6 100644
--- a/guix/build/syscalls.scm
+++ b/guix/build/syscalls.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2020 Jan (janneke) Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2021 Chris Marusich <cmmarusich@gmail.com>
 ;;; Copyright © 2021 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -118,6 +119,7 @@ (define-module (guix build syscalls)
             scandir*
             getxattr
             setxattr
+            lchown
 
             fcntl-flock
             lock-file
@@ -1277,6 +1279,20 @@ (define* (scandir* name #:optional
       (lambda ()
         (closedir* directory)))))
 
+(define-as-needed lchown
+  (let ((proc (syscall->procedure int "lchown" (list '* int int))))
+    (lambda (file owner group)
+      "As 'chown', change the ownership and group of the file referred to by
+FILE to the integer values OWNER and GROUP but doesn't dereference symbolic
+links.  Unlike 'chown' this doesn't support port or integer file descriptor
+via 'fchown'."
+      (let-values (((ret err)
+                    (proc (string->pointer file) owner group)))
+        (unless (zero? ret)
+          (throw 'system-error "lchown" "~S: ~A"
+                 (list file (strerror err))
+                 (list err)))))))
+
 \f
 ;;;
 ;;; Advisory file locking.
diff --git a/tests/syscalls.scm b/tests/syscalls.scm
index c9e011f453..24a8fd9726 100644
--- a/tests/syscalls.scm
+++ b/tests/syscalls.scm
@@ -287,6 +287,68 @@ (define perform-container-tests?
            (scandir* directory)
            (scandir directory (const #t) string<?))))
 
+(test-equal "lchown, ENOENT"
+  ENOENT
+  (catch 'system-error
+    (lambda ()
+      (lchown "/does/not/exist" 0 0))
+    (lambda args
+      (system-error-errno args))))
+
+(test-assert "lchown, no changes"
+  (call-with-temporary-directory
+   (lambda (directory)
+     (let* ((file (string-append directory "/file"))
+            (link (string-append directory "/link"))
+            (user (getpwnam (getlogin)))
+            (uid (passwd:uid user))
+            (gid (passwd:gid user)))
+       (call-with-output-file file
+         (const #t))
+       (symlink file link)
+       (lchown file -1 -1)
+       (let ((lstat (lstat link))
+             (stat (stat link)))
+         (and (eq? uid (stat:uid lstat))
+              (eq? uid (stat:uid stat))
+              (eq? gid (stat:gid lstat))
+              (eq? gid (stat:gid stat))))))))
+
+(test-assert "lchown, regular file"
+  (call-with-temporary-directory
+   (lambda (directory)
+     (let* ((file (string-append directory "/file"))
+            (nobody (getpwnam "nobody"))
+            (uid (passwd:uid nobody))
+            (gid (passwd:gid nobody)))
+       (call-with-output-file file
+         (const #t))
+       (lchown file uid gid)
+       (let ((stat (stat file)))
+         (and (eq? uid (stat:uid stat))
+              (eq? gid (stat:gid stat))))))))
+
+(test-assert "lchown, symlink"
+  (call-with-temporary-directory
+   (lambda (directory)
+     (let* ((file (string-append directory "/file"))
+            (link (string-append directory "/link"))
+            (current-user (getpwnam (getlogin)))
+            (nobody (getpwnam "nobody"))
+            (nobody-uid (passwd:uid nobody))
+            (nobody-gid (passwd:gid nobody)))
+       (call-with-output-file file
+         (const #t))
+       (symlink file link)
+       (lchown link nobody-uid nobody-gid)
+       (let ((lstat (lstat link))
+             (stat (stat link)))
+         (and (eq? nobody-uid (stat:uid lstat))
+              (eq? (passwd:uid current-user) (stat:uid stat))
+              (eq? nobody-gid (stat:gid lstat))
+              (eq? (passwd:gid current-user) (stat:gid stat))))))))
+
+
 (false-if-exception (delete-file temp-file))
 (test-assert "getxattr, setxattr"
   (let ((key "user.translator")
-- 
2.34.0





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

* [bug#52713] [PATCH v2 2/4] activation: Add 'lchown-recursive'.
  2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
  2021-12-21 19:36       ` [bug#52715] [PATCH v2 1/4] syscalls: Add 'lchown' Brice Waegeneire
@ 2021-12-21 19:36       ` Brice Waegeneire
  2021-12-21 19:36       ` [bug#52714] [PATCH v2 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
  2021-12-21 19:36       ` [bug#52712] [PATCH v2 4/4] services: cuirass: " Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-21 19:36 UTC (permalink / raw)
  To: 52713

* gnu/build/activation.scm (lchown-recursive): New procedure.
---
 gnu/build/activation.scm | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index 9f6126023c..fff2d61b13 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -46,7 +46,8 @@ (define-module (gnu build activation)
             activate-firmware
             activate-ptrace-attach
             activate-current-system
-            mkdir-p/perms))
+            mkdir-p/perms
+            lchown-recursive))
 
 ;;; Commentary:
 ;;;
@@ -105,6 +106,23 @@ (define (mkdir-p/perms directory owner bits)
   (chown directory (passwd:uid owner) (passwd:gid owner))
   (chmod directory bits))
 
+(define (lchown-recursive file owner group)
+  "As 'lchown' but recursively, change ownership of FILE to the integer values
+OWNER and GROUP without dereferencing symbolic links it encounter."
+  (nftw file
+        (lambda (filename statinfo flag base level)
+          (catch 'system-error
+            (lambda ()
+              (when (member flag '(regular directory symlink))
+                (lchown filename owner group)))
+            (lambda args
+              (format (current-error-port)
+                      "warning: failed to chown ~s: ~a~%"
+                      filename
+                      (strerror (system-error-errno args)))))
+          #t)
+        'physical))
+
 (define* (copy-account-skeletons home
                                  #:key
                                  (directory %skeleton-directory)
-- 
2.34.0





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

* [bug#52714] [PATCH v2 3/4] services: postgresql: Ensure correct ownership of directory trees.
  2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
  2021-12-21 19:36       ` [bug#52715] [PATCH v2 1/4] syscalls: Add 'lchown' Brice Waegeneire
  2021-12-21 19:36       ` [bug#52713] [PATCH v2 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
@ 2021-12-21 19:36       ` Brice Waegeneire
  2021-12-21 19:36       ` [bug#52712] [PATCH v2 4/4] services: cuirass: " Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-21 19:36 UTC (permalink / raw)
  To: 52714

* gnu/services/databases.scm (postgresql-activation): Replace 'chown'
  calls by 'lchown-recursive'.
---
 gnu/services/databases.scm | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/gnu/services/databases.scm b/gnu/services/databases.scm
index 39225a4bd6..58d93a8e35 100644
--- a/gnu/services/databases.scm
+++ b/gnu/services/databases.scm
@@ -8,6 +8,7 @@
 ;;; Copyright © 2019 Robert Vollmert <rob@vllmrt.net>
 ;;; Copyright © 2020 Marius Bakke <marius@gnu.org>
 ;;; Copyright © 2021 David Larsson <david.larsson@selfhosted.xyz>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -214,8 +215,11 @@ (define postgresql-activation
     (($ <postgresql-configuration> postgresql port locale config-file
                                    log-directory data-directory
                                    extension-packages)
-     #~(begin
+     (with-imported-modules (source-module-closure
+                             '((gnu build activation)))
+       #~(begin
          (use-modules (guix build utils)
+                      (gnu build activation)
                       (ice-9 match))
 
          (let ((user (getpwnam "postgres"))
@@ -230,19 +234,19 @@ (define postgresql-activation
                      '()))))
            ;; Create db state directory.
            (mkdir-p #$data-directory)
-           (chown #$data-directory (passwd:uid user) (passwd:gid user))
+           (lchown-recursive #$data-directory (passwd:uid user) (passwd:gid user))
 
            ;; Create the socket directory.
            (let ((socket-directory
                   #$(postgresql-config-file-socket-directory config-file)))
              (when (string? socket-directory)
                (mkdir-p socket-directory)
-               (chown socket-directory (passwd:uid user) (passwd:gid user))))
+               (lchown-recursive socket-directory (passwd:uid user) (passwd:gid user))))
 
            ;; Create the log directory.
            (when (string? #$log-directory)
              (mkdir-p #$log-directory)
-             (chown #$log-directory (passwd:uid user) (passwd:gid user)))
+             (lchown-recursive #$log-directory (passwd:uid user) (passwd:gid user)))
 
            ;; Drop privileges and init state directory in a new
            ;; process.  Wait for it to finish before proceeding.
@@ -262,7 +266,7 @@ (define postgresql-activation
                           initdb-args)))
                 (lambda ()
                   (primitive-exit 1))))
-             (pid (waitpid pid))))))))
+             (pid (waitpid pid)))))))))
 
 (define postgresql-shepherd-service
   (match-lambda
-- 
2.34.0





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

* [bug#52712] [PATCH v2 4/4] services: cuirass: Ensure correct ownership of directory trees.
  2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
                         ` (2 preceding siblings ...)
  2021-12-21 19:36       ` [bug#52714] [PATCH v2 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
@ 2021-12-21 19:36       ` Brice Waegeneire
  3 siblings, 0 replies; 11+ messages in thread
From: Brice Waegeneire @ 2021-12-21 19:36 UTC (permalink / raw)
  To: 52712

* gnu/services/cuirass.scm (cuirass-activation): Replace 'chown'
  calls by 'lchown-recursive'.
---
 gnu/services/cuirass.scm | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/gnu/services/cuirass.scm b/gnu/services/cuirass.scm
index 96f28a9670..41e45604dd 100644
--- a/gnu/services/cuirass.scm
+++ b/gnu/services/cuirass.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2017 Jan Nieuwenhuizen <janneke@gnu.org>
 ;;; Copyright © 2018, 2019 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2018 Clément Lassieur <clement@lassieur.org>
+;;; Copyright © 2021 Brice Waegeneire <brice@waegenei.re>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -24,6 +25,7 @@
 (define-module (gnu services cuirass)
   #:use-module (guix channels)
   #:use-module (guix gexp)
+  #:use-module (guix modules)
   #:use-module (guix records)
   #:use-module (guix store)
   #:use-module (guix utils)
@@ -278,9 +280,11 @@ (define (cuirass-activation config)
          (profile        (string-append "/var/guix/profiles/per-user/" user))
          (roots          (string-append profile "/cuirass"))
          (group          (cuirass-configuration-group config)))
-    (with-imported-modules '((guix build utils))
+    (with-imported-modules (source-module-closure
+                            '((gnu build activation)))
       #~(begin
-          (use-modules (guix build utils))
+          (use-modules (guix build utils)
+                       (gnu build activation))
 
           (mkdir-p #$cache)
           (mkdir-p #$log)
@@ -291,13 +295,13 @@ (define (cuirass-activation config)
 
           (let ((uid (passwd:uid (getpw #$user)))
                 (gid (group:gid (getgr #$group))))
-            (chown #$cache uid gid)
-            (chown #$log uid gid)
-            (chown #$roots uid gid)
-            (chown #$profile uid gid)
+            (lchown-recursive #$cache uid gid)
+            (lchown-recursive #$log uid gid)
+            (lchown-recursive #$profile uid gid)
+            (lchown-recursive (passwd:dir (getpw #$user)) uid gid)
 
             (when #$remote-cache
-              (chown #$remote-cache uid gid)))))))
+              (lchown-recursive #$remote-cache uid gid)))))))
 
 (define (cuirass-log-rotations config)
   "Return the list of log rotations that corresponds to CONFIG."
-- 
2.34.0





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

end of thread, other threads:[~2021-12-21 19:42 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-12 18:28 [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Brice Waegeneire
2021-12-12 18:36 ` [bug#52454] [PATCH 1/4] syscalls: Add 'lchown' Brice Waegeneire
2021-12-18 21:34   ` [bug#52454] [PATCH 0/4] Ensure correct ownership of directory trees in services.Hello Guix, Ludovic Courtès
2021-12-21 19:30     ` [bug#52454] [PATCH v2 0/4] Ensure correct ownership of directory trees in services Brice Waegeneire
2021-12-21 19:36       ` [bug#52715] [PATCH v2 1/4] syscalls: Add 'lchown' Brice Waegeneire
2021-12-21 19:36       ` [bug#52713] [PATCH v2 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
2021-12-21 19:36       ` [bug#52714] [PATCH v2 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
2021-12-21 19:36       ` [bug#52712] [PATCH v2 4/4] services: cuirass: " Brice Waegeneire
2021-12-12 18:36 ` [bug#52454] [PATCH 2/4] activation: Add 'lchown-recursive' Brice Waegeneire
2021-12-12 18:36 ` [bug#52454] [PATCH 3/4] services: postgresql: Ensure correct ownership of directory trees Brice Waegeneire
2021-12-12 18:36 ` [bug#52454] [PATCH 4/4] services: cuirass: " Brice Waegeneire

Code repositories for project(s) associated with this external index

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

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.