bug#63082: [PATCH v3 10/16] services: mpd: Let Shepherd effect the user/group change.

2023-07-26 Thread Maxim Cournoyer
Hello,

Bruno Victal  writes:

> On 2023-05-05 19:29, Maxim Cournoyer wrote:
>> Relates to .
>> 
>> Quoting a MPD developer, regarding MPD's feature to switch user itself:
>> "that's legacy for the dark ages when proper service managers did not exist"
>> :-).
>> 
>> * gnu/services/audio.scm (mpd-serialize-user-account)
>> (mpd-serialize-user-group): Delete procedures.
>> * gnu/services/audio.scm (mpd-configuration) [user]: Do not serialize.
>> [group]: Likewise.
>> (mpd-shepherd-service): Provide the #:user, #:group and 
>> #:supplementary-groups
>> arguments.
>> (mympd-shepherd-service): Likewise, and remove the '--user' argument.
>> * doc/guix.texi (Audio Services): Update doc.
>> (mympd-configuration) [port]: Change default value to 8080.
>> [ssl-port]: Change default value to 443.
>> * gnu/tests/audio.scm (run-mympd-test): Adjust accordingly.
>> ---
>>  doc/guix.texi  | 12 +-
>>  gnu/services/audio.scm | 52 +-
>>  gnu/tests/audio.scm|  4 ++--
>>  3 files changed, 39 insertions(+), 29 deletions(-)
>
> This contains a submarine change that isn't easily spotted from the
> commit message, that mympd is getting its default port changed and that
> it can no longer bind to privileged ports, since although mympd can
> start as root in order to bind to possibly privileged ports, it will
> explicitly refuse to continue running as root afterwards.
>
> I think we can have shepherd effect for mympd, but only if (and after)
> shepherd gets support for POSIX capabilities (CAP_NET_BIND_SERVICE) or
> a suitable way to specify that “yes, the program invoked by the service
> should have CAP_NET_BIND_SERVICE” is provided.

As mentioned before, I've let go of this commit for now (though that
means supplementary-groups on a user-account are not honored anymore)
and other commits touching the current group mechanism until we've
implemented support for POSIX capabilities as mentioned in
https://issues.guix.gnu.org/64862.

We can thus close this issue for now, keeping on mind that some bits
could be salvaged at a later time when 64862 is done.

-- 
Thanks,
Maxim





bug#63082: [PATCH v3 10/16] services: mpd: Let Shepherd effect the user/group change.

2023-05-24 Thread Bruno Victal
On 2023-05-05 19:29, Maxim Cournoyer wrote:
> Relates to .
> 
> Quoting a MPD developer, regarding MPD's feature to switch user itself:
> "that's legacy for the dark ages when proper service managers did not exist"
> :-).
> 
> * gnu/services/audio.scm (mpd-serialize-user-account)
> (mpd-serialize-user-group): Delete procedures.
> * gnu/services/audio.scm (mpd-configuration) [user]: Do not serialize.
> [group]: Likewise.
> (mpd-shepherd-service): Provide the #:user, #:group and #:supplementary-groups
> arguments.
> (mympd-shepherd-service): Likewise, and remove the '--user' argument.
> * doc/guix.texi (Audio Services): Update doc.
> (mympd-configuration) [port]: Change default value to 8080.
> [ssl-port]: Change default value to 443.
> * gnu/tests/audio.scm (run-mympd-test): Adjust accordingly.
> ---
>  doc/guix.texi  | 12 +-
>  gnu/services/audio.scm | 52 +-
>  gnu/tests/audio.scm|  4 ++--
>  3 files changed, 39 insertions(+), 29 deletions(-)

This contains a submarine change that isn't easily spotted from the
commit message, that mympd is getting its default port changed and that
it can no longer bind to privileged ports, since although mympd can
start as root in order to bind to possibly privileged ports, it will
explicitly refuse to continue running as root afterwards.

I think we can have shepherd effect for mympd, but only if (and after)
shepherd gets support for POSIX capabilities (CAP_NET_BIND_SERVICE) or
a suitable way to specify that “yes, the program invoked by the service
should have CAP_NET_BIND_SERVICE” is provided.


-- 
Furthermore, I consider that nonfree software must be eradicated.

Cheers,
Bruno.






bug#63082: [PATCH v3 10/16] services: mpd: Let Shepherd effect the user/group change.

2023-05-05 Thread Maxim Cournoyer
Relates to .

Quoting a MPD developer, regarding MPD's feature to switch user itself:
"that's legacy for the dark ages when proper service managers did not exist"
:-).

* gnu/services/audio.scm (mpd-serialize-user-account)
(mpd-serialize-user-group): Delete procedures.
* gnu/services/audio.scm (mpd-configuration) [user]: Do not serialize.
[group]: Likewise.
(mpd-shepherd-service): Provide the #:user, #:group and #:supplementary-groups
arguments.
(mympd-shepherd-service): Likewise, and remove the '--user' argument.
* doc/guix.texi (Audio Services): Update doc.
(mympd-configuration) [port]: Change default value to 8080.
[ssl-port]: Change default value to 443.
* gnu/tests/audio.scm (run-mympd-test): Adjust accordingly.
---
 doc/guix.texi  | 12 +-
 gnu/services/audio.scm | 52 +-
 gnu/tests/audio.scm|  4 ++--
 3 files changed, 39 insertions(+), 29 deletions(-)

diff --git a/doc/guix.texi b/doc/guix.texi
index 253b8f113b..cdc1f4dedc 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -33598,7 +33598,7 @@ Audio Services
 The MPD package.
 
 @item @code{user} (type: user-account)
-The user to run mpd as.
+The user to run @command{mpd} as.
 
 @item @code{group} (default: @code{#f}) (type: boolean)
 Obsolete.  Do not use.
@@ -33642,7 +33642,7 @@ Audio Services
 The location of the sticker database.
 
 @item @code{default-port} (default: @code{6600}) (type: maybe-port)
-The default port to run mpd on.
+The default port to run @command{mpd} on.
 
 @item @code{endpoints} (type: maybe-list-of-strings)
 The addresses that mpd will bind to.  A port different from
@@ -33827,13 +33827,13 @@ Audio Services
 @uref{https://jcorporation.github.io/myMPD/, myMPD} is a web server
 frontend for MPD that provides a mobile friendly web client for MPD.
 
-The following example shows a myMPD instance listening on port 80,
+The following example shows a myMPD instance listening on port 8080,
 with album cover caching disabled.
 
 @lisp
 (service mympd-service-type
  (mympd-configuration
-  (port 80)
+  (port 8080)
   (covercache-ttl 0)))
 @end lisp
 
@@ -33877,7 +33877,7 @@ Audio Services
 @item @code{host} (default: @code{"[::]"}) (type: string)
 Host name to listen on.
 
-@item @code{port} (default: @code{80}) (type: maybe-port)
+@item @code{port} (default: @code{8080}) (type: maybe-port)
 HTTP port to listen on.
 
 @item @code{log-level} (default: @code{5}) (type: integer)
@@ -33903,7 +33903,7 @@ Audio Services
 @item @code{ssl?} (default: @code{#f}) (type: boolean)
 SSL/TLS support.
 
-@item @code{ssl-port} (default: @code{443}) (type: maybe-port)
+@item @code{ssl-port} (default: @code{4443}) (type: maybe-port)
 Port to listen for HTTPS.
 
 @item @code{ssl-cert} (type: maybe-string)
diff --git a/gnu/services/audio.scm b/gnu/services/audio.scm
index 7fb4b8ccf7..f470ca20e0 100644
--- a/gnu/services/audio.scm
+++ b/gnu/services/audio.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2019 Ricardo Wurmus 
 ;;; Copyright © 2020 Ludovic Courtès 
 ;;; Copyright © 2022⁠–⁠2023 Bruno Victal 
+;;; Copyright © 2023 Maxim Cournoyer 
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -165,9 +166,6 @@ (define mpd-serialize-boolean mpd-serialize-field)
 (define (mpd-serialize-list-of-strings field-name value)
   #~(string-append #$@(map (cut mpd-serialize-string field-name <>) value)))
 
-(define (mpd-serialize-user-account field-name value)
-  (mpd-serialize-string field-name (user-account-name value)))
-
 (define-maybe string (prefix mpd-))
 (define-maybe list-of-strings (prefix mpd-))
 (define-maybe boolean (prefix mpd-))
@@ -390,10 +388,14 @@ (define-configuration mpd-configuration
"The MPD package."
empty-serializer)
 
+  ;; Note: The user and its group are not serialized, otherwise MPD would
+  ;; attempt to switch the user/group itself.  The task of switching the
+  ;; user/group is left to Shepherd instead.
   (user
(user-account %mpd-user)
-   "The user to run mpd as."
-   (sanitizer mpd-user-sanitizer))
+   "The user to run @command{mpd} as."
+   (sanitizer mpd-user-sanitizer)
+   empty-serializer)
 
   (group
(boolean #f)
@@ -458,7 +460,7 @@ (define-configuration mpd-configuration
 
   (default-port
(maybe-port 6600)
-   "The default port to run mpd on.")
+   "The default port to run @command{mpd} on.")
 
   (endpoints
maybe-list-of-strings
@@ -611,7 +613,11 @@ (define (mpd-shepherd-service config)
   (make-forkexec-constructor
(list #$(file-append package "/bin/mpd") "--no-daemon"
  #$config-file)
-   #:environment-variables '#$environment-variables
+   #:environment-variables '#$environment-variables
+   #:user #$username
+   #:group #$(user-account-group user)
+   #:supplementary-groups
+   '#$(user-account-supplementary-groups user)
(stop  #~(make-kill-destructor))