diff mbox series

[bug#41763] services: opensmtpd: Fix the setgid problem for the smtpctl utility.

Message ID 87eeqpih6q.fsf@hurd.i-did-not-set--mail-host-address--so-tickle-me
State Accepted
Headers show
Series [bug#41763] services: opensmtpd: Fix the setgid problem for the smtpctl utility. | expand

Checks

Context Check Description
cbaines/comparison success View comparision
cbaines/git branch success View Git branch
cbaines/applying patch fail View Laminar job

Commit Message

Maxim Cournoyer June 8, 2020, 5:46 p.m. UTC
Hello!

The following patches provide a mean to specify a user and group for a
setuid program, and uses that to fix a setgid permission issue in the
context of the opensmtpd service.

Christopher, you should be able to leverage this new facility to
configure the uid/gid of the sendmail program to that of the smtpq user,
like this:

--8<---------------cut here---------------start------------->8---
(operating-system)
  [...]
  (setuid-programs (cons (list (file-append sendmail "/usr/sbin/sendmail") "smtpq")
                           %setuid-programs))
--8<---------------cut here---------------end--------------->8---

The smtpq user is created as part of the OpenSMTPD service definition.

Thank you,
Maxim

Comments

Christopher Baines June 11, 2020, 7:20 p.m. UTC | #1
maxim.cournoyer@gmail.com writes:

> The following patches provide a mean to specify a user and group for a
> setuid program, and uses that to fix a setgid permission issue in the
> context of the opensmtpd service.
>
> Christopher, you should be able to leverage this new facility to
> configure the uid/gid of the sendmail program to that of the smtpq user,
> like this:
>
> --8<---------------cut here---------------start------------->8---
> (operating-system)
>   [...]
>   (setuid-programs (cons (list (file-append sendmail "/usr/sbin/sendmail") "smtpq")
>                            %setuid-programs))
> --8<---------------cut here---------------end--------------->8---
>
> The smtpq user is created as part of the OpenSMTPD service definition.
>
> Thank you,
>
>
> Maxim

Well, thank you for looking in to this Maxim. I've had a brief look
through the patches, although I don't know enough about this area to
comment properly on them.

I wonder if it's worth using a record type to make it possible to pass
the user and group values to the service. That would probably result in
more readable configuration than just using a list of varying length.

Specifically on the diff:

- (list #$@programs))))))
+ (quote (#$@programs)))))))

This change here will mean that you can't pass some values in, as they
won't be evaluated. #~(string-append sendmail "/usr/sbin/sendmail")
would no longer work for example.

Thanks again,

Chris
Brice Waegeneire June 15, 2020, 3:12 p.m. UTC | #2
Hello Maxim,

Thank you for the patchset!

maxim.cournoyer@gmail.com writes:

> The following patches provide a mean to specify a user and group for a
> setuid program, and uses that to fix a setgid permission issue in the
> context of the opensmtpd service.

I applied it to try to use wireshark as non-root[0]:

--8<---------------cut here---------------start------------->8---
(simple-service 'wireshark-group account-service-type
                   (list (user-group (name "wireshark") (system? #t))))
(simple-service 'wireshark-dumpcap setuid-program-service-type
                   (list (list (file-append wireshark "/bin/dumpcap")
                               "root" "wireshark")))
--8<---------------cut here---------------end--------------->8---

And unfortunately the first run of “guix reconfigure“ failed to make
“dumpcap“ as a setuid, but subsequent run succeeded:

--8<---------------cut here---------------start------------->8---
[…]
setting up setuid programs in '/run/setuid-programs'...
warning: failed to make '/gnu/store/vdlk9rli5k5svy8p7bhf90ln03ybnxgj-wireshark-3.2.4/bin/dumpcap' setuid (root:wireshark): Success
populating /etc from /gnu/store/hxjyvg80zjaxfynjyk3jgqsn9249azmx-etc...
[…]
--8<---------------cut here---------------end--------------->8---

I guess it's because at first there wasn't a wireshark group on my
system, adding the group and the setuid program was done in the same
run, but “setting up setuid programs” is done before “populating /etc”
(comprising /etc/passwd) which in effect ended up trying to setuid
“dumpcap“ before the “wireshark“ group exists. And subsequent runs
succeeded creating a setuid “dumpcap” because the new group was already
on the system, it was created during the first run.

Populating /etc before setting up /run/setuid-programs should fix that
issue but maybe there is reason behind the current order of execution.

> Christopher, you should be able to leverage this new facility to
> configure the uid/gid of the sendmail program to that of the smtpq user,
> like this:
>
> (operating-system)
>   [...]
>   (setuid-programs (cons (list (file-append sendmail "/usr/sbin/sendmail") "smtpq")
>                            %setuid-programs))
>

Aside from that I wonder if specifying user and group in a list is
future proof, maybe using a record would be more Guixy. In particular I
would like to be able to set capabilities (as with “setcap“) on binaries
since the store don't support it[1]; if that's even possible but it's an
other issue.

[0]: https://wiki.wireshark.org/CaptureSetup/CapturePrivileges#Most_UNIXes
[1]: https://lists.gnu.org/archive/html/help-guix/2016-11/msg00046.html

- Brice
Jonathan Brielmaier Jan. 3, 2021, 2:14 p.m. UTC | #3
It's http://issues.guix.gnu.org/41763.

What does us block from merging this? It hits me hard when using OpenSMTPD.
Tobias Geerinckx-Rice Jan. 3, 2021, 2:49 p.m. UTC | #4
Jonathan Brielmaier 写道:
> What does us block from merging this?

Reading [0], Chris & Brice bring up two good points that I don't 
see addressed: using a record instead of a list & not breaking 
gexps, although fixing one would probably moot the other.

Kind regards,

T G-R

[0]: http://issues.guix.gnu.org/41763
Maxim Cournoyer July 16, 2021, 4:24 a.m. UTC | #5
Hello,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

> Jonathan Brielmaier 写道:
>> What does us block from merging this?
>
> Reading [0], Chris & Brice bring up two good points that I don't see
> addressed: using a record instead of a list & not breaking gexps,
> although fixing one would probably moot the other.
>
> Kind regards,
>
> T G-R
>
> [0]: http://issues.guix.gnu.org/41763

Closing in favor of https://issues.guix.gnu.org/44700.

Thanks,

Maxim
Tobias Geerinckx-Rice July 16, 2021, 5:37 a.m. UTC | #6
> Closing in favor of https://issues.guix.gnu.org/44700.

Yes please.  Thanks.

T G-R

Sent from a Web browser. Excuse or enjoy my brevity.
diff mbox series

Patch

From 52a1a031e6a7c0196cf17d0bd32061d02b453df8 Mon Sep 17 00:00:00 2001
From: Maxim Cournoyer <maxim.cournoyer@gmail.com>
Date: Sun, 7 Jun 2020 23:52:00 -0400
Subject: [PATCH 3/3] services: opensmtpd: Fix the setgid problem for the
 smtpctl utility.

The utility was complaining that it wasn't setgid to the group ID of the
"smtpq" group.

* gnu/services/mail.scm (opensmtpd-service-type): Extend the
setuid-program-service-type with the smtpctl program.
---
 gnu/services/mail.scm | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 7c49d99e9f..96efbd951d 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -1662,6 +1662,11 @@  match from local for any action outbound
          (home-directory "/var/empty")
          (shell (file-append shadow "/sbin/nologin")))))
 
+(define (opensmtpd-setuid-programs opensmtpd-configuration)
+  (let ((smtpctl (file-append (opensmtpd-configuration-package
+                               opensmtpd-configuration) "/sbin/smtpctl")))
+    (list (list smtpctl "smtpq"))))
+
 (define opensmtpd-activation
   (match-lambda
     (($ <opensmtpd-configuration> package config-file)
@@ -1683,6 +1688,8 @@  match from local for any action outbound
    (extensions
     (list (service-extension account-service-type
                              (const %opensmtpd-accounts))
+          (service-extension setuid-program-service-type
+                             opensmtpd-setuid-programs)
           (service-extension activation-service-type
                              opensmtpd-activation)
           (service-extension pam-root-service-type
-- 
2.26.2