bug#57493: <user-account> should allow for customizing home directory permission bits

bug#57493: <user-account> should allow for customizing home directory permission bits

[Thompson, David]

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

Hi Guix,

Issue 56444 (https://issues.guix.gnu.org/56444) was caused by the
activate-users+groups procedure in (gnu build activation) unconditionally
setting all user home directory permission bits to 700. The fix for that
bug was to set the bits for a particular user to 750 in a service
activation script.  The fix is quite imperfect, however, because during
system reconfiguration the bits are temporarily reset back to 700 by
activate-users+groups, breaking Guix's promise of atomicity.  The proper
fix would be to add something like a 'home-directory-permission-bits' field
to <user-account>, which defaults to 700, and have activate-users+groups
use that value.  This way, there will no longer be an unknown amount of
time where the bits are reset and potentially breaking some service during
that time.

It seems that there is already some support for implementing such a change
and I am happy to do the work, but I wanted to ask: Are there any gotchas
or issues I should be aware of?  It seems straightforward to me but I
haven't made modifications to the system code in years. I don't want to be
the reason 'guix system reconfigure' fails for someone. :)

Thanks,

- Dave

[-- Attachment #2: Type: text/html, Size: 1408 bytes --]

bug#57493: <user-account> should allow for customizing home directory permission bits

[Thompson, David]

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

On Tue, Aug 30, 2022 at 1:10 PM Thompson, David
<dthompson2@worcester.edu> wrote:
>
> Hi Guix,
>
> Issue 56444 (https://issues.guix.gnu.org/56444) was caused by the activate-users+groups procedure in (gnu build activation) unconditionally setting all user home directory permission bits to 700. The fix for that bug was to set the bits for a particular user to 750 in a service activation script.  The fix is quite imperfect, however, because during system reconfiguration the bits are temporarily reset back to 700 by activate-users+groups, breaking Guix's promise of atomicity.  The proper fix would be to add something like a 'home-directory-permission-bits' field to <user-account>, which defaults to 700, and have activate-users+groups use that value.  This way, there will no longer be an unknown amount of time where the bits are reset and potentially breaking some service during that time.

FInally got around to writing a patch for this!

- Dave

[-- Attachment #2: 0001-gnu-system-Add-home-directory-permissions-field-to-u.patch --]
[-- Type: text/x-patch, Size: 3354 bytes --]

From 013ad524971dc6ea810fe3b92042c039cecd2f8a Mon Sep 17 00:00:00 2001
From: David Thompson <dthompson2@worcester.edu>
Date: Sat, 14 Jan 2023 10:53:16 -0500
Subject: [PATCH 1/2] gnu: system: Add home-directory-permissions field to
 <user-account>.

* gnu/system/accounts.scm (<user-account>)[home-directory-permissions]: New
field.
(user-account-home-directory-permissions): New accessor.
* gnu/build/activation.scm (activate-users+groups): Use home directory
permission bits from the user account object.
* doc/guix.texi (User Accounts): Document new field.
---
 doc/guix.texi            | 4 ++++
 gnu/build/activation.scm | 6 +++---
 gnu/system/accounts.scm  | 3 +++
 3 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index c07ec89b2f..52548c3dfa 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -17337,6 +17337,10 @@ administrator's choice; reconfiguring does @emph{not} change their name.
 @item @code{home-directory}
 This is the name of the home directory for the account.
 
+@item @code{home-directory-permissions} (default: @code{#o700})
+The permission bits for the home directory.  By default, full access is
+granted to the user account and all other access is denied.
+
 @item @code{create-home-directory?} (default: @code{#t})
 Indicates whether the home directory of this account should be created
 if it does not exist yet.
diff --git a/gnu/build/activation.scm b/gnu/build/activation.scm
index eea2233563..fd043ca131 100644
--- a/gnu/build/activation.scm
+++ b/gnu/build/activation.scm
@@ -162,14 +162,14 @@ (define (activate-users+groups users groups)
 group records) are all available."
   (define (make-home-directory user)
     (let ((home (user-account-home-directory user))
+          (home-permissions (user-account-home-directory-permissions user))
           (pwd  (getpwnam (user-account-name user))))
       (mkdir-p home)
 
       ;; Always set ownership and permissions for home directories of system
-      ;; accounts.  If a service needs looser permissions on its home
-      ;; directories, it can always chmod it in an activation snippet.
+      ;; accounts.
       (chown home (passwd:uid pwd) (passwd:gid pwd))
-      (chmod home #o700)))
+      (chmod home home-permissions)))
 
   (define system-accounts
     (filter (lambda (user)
diff --git a/gnu/system/accounts.scm b/gnu/system/accounts.scm
index 586cff1842..dd6930c619 100644
--- a/gnu/system/accounts.scm
+++ b/gnu/system/accounts.scm
@@ -28,6 +28,7 @@ (define-module (gnu system accounts)
             user-account-supplementary-groups
             user-account-comment
             user-account-home-directory
+            user-account-home-directory-permissions
             user-account-create-home-directory?
             user-account-shell
             user-account-system?
@@ -69,6 +70,8 @@ (define-record-type* <user-account>
   (comment        user-account-comment (default ""))
   (home-directory user-account-home-directory (thunked)
                   (default (default-home-directory this-record)))
+  (home-directory-permissions user-account-home-directory-permissions
+                              (default #o700))
   (create-home-directory? user-account-create-home-directory? ;Boolean
                           (default #t))
   (shell          user-account-shell              ; gexp
-- 
2.38.1

bug#57493: <user-account> should allow for customizing home directory permission bits

[Liliana Marie Prikler]

* gnu/system/accounts.scm
> (<user-account>)[home-directory-permissions]: New
> field.
> (user-account-home-directory-permissions): New accessor.
> * gnu/build/activation.scm (activate-users+groups): Use home
> directory
> permission bits from the user account object.
> * doc/guix.texi (User Accounts): Document new field.
LGTM.

The header says this is part 1/2.  Is that correct or did you just
invoke git format-patch wrong?

Cheers

bug#57493: [EXT] Re: bug#57493: <user-account> should allow for customizing home directory permission bits

[Thompson, David]

Hi Liliana,

On Sun, Jan 15, 2023 at 7:25 AM Liliana Marie Prikler
<liliana.prikler@gmail.com> wrote:
>
> * gnu/system/accounts.scm
> > (<user-account>)[home-directory-permissions]: New
> > field.
> > (user-account-home-directory-permissions): New accessor.
> > * gnu/build/activation.scm (activate-users+groups): Use home
> > directory
> > permission bits from the user account object.
> > * doc/guix.texi (User Accounts): Document new field.
> LGTM.
>
> The header says this is part 1/2.  Is that correct or did you just
> invoke git format-patch wrong?

Oops, that's my bad! I forgot that the patch file header would say
that.  There's a second patch that changes the Gitolite service to use
this new field, which is the service that sparked the need for this
additional flexibility, but I was going to leave that out for now and
maybe just push directly as it's a 2 line change and the gitolite
system test passes. So, please disregard that 1/2 thing!

Thanks for checking!

- Dave

bug#57493: [EXT] Re: bug#57493: <user-account> should allow for customizing home directory permission bits

[Josselin Poiret via Bug reports for GNU Guix]

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

Hi Dave,

Pushed as e9a5eebc785cb843034b38c5c5a6dd10904bdf2a.

Thanks for your contribution!  Closing.

Best,
-- 
Josselin Poiret

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