[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Kristofer Buffington]

[-- Attachment #1: opensmtpd-service-type to extend pam --]
[-- Type: text/x-patch, Size: 1716 bytes --]

From 4498a2c9e1453cb81b54231a5203bc55019e62db Mon Sep 17 00:00:00 2001
From: Kristofer Buffington <kristoferbuffington@gmail.com>
Date: Wed, 12 Jun 2019 16:34:30 -0400
Subject: [PATCH] * gnu/services/mail.scm added %opensmtpd-pam-services to the
 opensmtpd-service-type
To: guix-patches@gnu.org

---
 gnu/services/mail.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 216b2c80b0..e54d7eb3c1 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2017, 2018 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2019 Kristofer Buffington <kristoferbuffington@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1667,6 +1668,9 @@ accept from local for any relay
            (mkdir-p "/var/spool/smtpd")
            (chmod "/var/spool/smtpd" #o711))))))
 
+(define %opensmtpd-pam-services
+  (list (unix-pam-service "smtpd")))
+
 (define opensmtpd-service-type
   (service-type
    (name 'opensmtpd)
@@ -1675,6 +1679,8 @@ accept from local for any relay
                              (const %opensmtpd-accounts))
           (service-extension activation-service-type
                              opensmtpd-activation)
+          (service-extension pam-root-service-type
+                             (const %opensmtpd-pam-services))
           (service-extension profile-service-type
                              (compose list opensmtpd-configuration-package))
           (service-extension shepherd-root-service-type
-- 
2.18.0

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Ludovic Courtès]

Hi Kristofer,

Kristofer Buffington <kristofer@walletsworth.com> skribis:

> +(define %opensmtpd-pam-services
> +  (list (unix-pam-service "smtpd")))
> +
>  (define opensmtpd-service-type
>    (service-type
>     (name 'opensmtpd)
> @@ -1675,6 +1679,8 @@ accept from local for any relay
>                               (const %opensmtpd-accounts))
>            (service-extension activation-service-type
>                               opensmtpd-activation)
> +          (service-extension pam-root-service-type
> +                             (const %opensmtpd-pam-services))

Could you explain why the PAM service is needed and what went wrong
before that?

I haven’t use this service (yet) but I’m surprised it needs a PAM hook.

Thanks for your patch,
Ludo’.

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Kristofer Buffington]

The opensmtpd package includes --with-pam at build time, but the service
does not extend pam. As a result, an administrator would need to
maintain a seperate passwd or other authentication
mechanism instead of using PAM.

Kris

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Tobias Geerinckx-Rice]

[-- Attachment #1.1: Type: text/plain, Size: 521 bytes --]

Kristofer, Ludo',

Kristofer Buffington wrote:
> The opensmtpd package includes --with-pam at build time, but the 
> service
> does not extend pam. As a result, an administrator would need to
> maintain a seperate passwd or other authentication
> mechanism instead of using PAM.

Thanks for the patch!  Looks good to me, assuming you've 
battle-tested the thing.  I've Guixed up the commit message, but 
let's give Ludo' a chance to respond.

> walletsworth

I feel all, like, personally offended.

Kind regards,

T G-R


[-- Attachment #1.2: 0001-services-opensmtpd-Extend-the-PAM-service.patch --]
[-- Type: text/x-patch, Size: 1853 bytes --]

From 60f3fd35ac4925e86556781ccc2620f54dccacd9 Mon Sep 17 00:00:00 2001
From: Kristofer Buffington <kristoferbuffington@gmail.com>
Date: Wed, 12 Jun 2019 16:34:30 -0400
Subject: [PATCH] services: opensmtpd: Extend the PAM service.

* gnu/services/mail.scm (%opensmtpd-pam-services): New variable.
(opensmtpd-service-type)[extensions]: Add it, extending
PAM-ROOT-SERVICE-TYPE.

Signed-off-by: Tobias Geerinckx-Rice <me@tobias.gr>
---
 gnu/services/mail.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/services/mail.scm b/gnu/services/mail.scm
index 0e85ada3d0..3de0b4c2f3 100644
--- a/gnu/services/mail.scm
+++ b/gnu/services/mail.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2017, 2018 Clément Lassieur <clement@lassieur.org>
 ;;; Copyright © 2017 Carlo Zancanaro <carlo@zancanaro.id.au>
 ;;; Copyright © 2017 Tobias Geerinckx-Rice <me@tobias.gr>
+;;; Copyright © 2019 Kristofer Buffington <kristoferbuffington@gmail.com>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -1667,6 +1668,9 @@ accept from local for any relay
            (mkdir-p "/var/spool/smtpd")
            (chmod "/var/spool/smtpd" #o711))))))
 
+(define %opensmtpd-pam-services
+  (list (unix-pam-service "smtpd")))
+
 (define opensmtpd-service-type
   (service-type
    (name 'opensmtpd)
@@ -1675,6 +1679,8 @@ accept from local for any relay
                              (const %opensmtpd-accounts))
           (service-extension activation-service-type
                              opensmtpd-activation)
+          (service-extension pam-root-service-type
+                             (const %opensmtpd-pam-services))
           (service-extension profile-service-type
                              (compose list opensmtpd-configuration-package))
           (service-extension shepherd-root-service-type
-- 
2.22.0


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

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Tobias Geerinckx-Rice]

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

Ludo',

> +(define %opensmtpd-pam-services
> +  (list (unix-pam-service "smtpd")))

Hm.  This is consistent with the (inconsistent) use of % in (gnu 
services mail) even for unexported variables, but that doesn't 
match my understanding of %.

I'd like to remove it here and elsewhere in the file:

  $ grep define.*-activation gnu/services/mail.scm
  (define (%dovecot-activation config) ← local
  (define opensmtpd-activation
  (define exim-activation

Would that be noise?

Kind regards,

T G-R

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

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Ludovic Courtès]

Hello,

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

> From 60f3fd35ac4925e86556781ccc2620f54dccacd9 Mon Sep 17 00:00:00 2001
> From: Kristofer Buffington <kristoferbuffington@gmail.com>
> Date: Wed, 12 Jun 2019 16:34:30 -0400
> Subject: [PATCH] services: opensmtpd: Extend the PAM service.
>
> * gnu/services/mail.scm (%opensmtpd-pam-services): New variable.
> (opensmtpd-service-type)[extensions]: Add it, extending
> PAM-ROOT-SERVICE-TYPE.
>
> Signed-off-by: Tobias Geerinckx-Rice <me@tobias.gr>

LGTM, please push Tobias!

Thanks,
Ludo’.

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Ludovic Courtès]

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

>> +(define %opensmtpd-pam-services
>> +  (list (unix-pam-service "smtpd")))
>
> Hm.  This is consistent with the (inconsistent) use of % in (gnu
> services mail) even for unexported variables, but that doesn't match
> my understanding of %.

I think I’d use it here; that’s consistent with the rest of the file
actually, no?  To me everything looks OK as it is.

Thanks,
Ludo’.

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Tobias Geerinckx-Rice]

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

Ludovic Courtès wrote:
> I think I’d use it here; that’s consistent with the rest of the 
> file
> actually, no?  To me everything looks OK as it is.

Then I'm not sure I grok % (which is likely as I've never read a 
description, only induced its use).

What's the difference between:

 (define (%dovecot-activation config) …

and

 (define opensmtpd-activation …
 (define exim-activation …

?  I thought it had something to do with being exports/globals but 
here (%opensmtpd-accounts, etc.) it is used simply to say ‘this is 
a list’.

Will push,

T G-R

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

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Ludovic Courtès]

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

> Ludovic Courtès wrote:
>> I think I’d use it here; that’s consistent with the rest of the file
>> actually, no?  To me everything looks OK as it is.
>
> Then I'm not sure I grok % (which is likely as I've never read a
> description, only induced its use).
>
> What's the difference between:
>
> (define (%dovecot-activation config) …
>
> and
>
> (define opensmtpd-activation …
> (define exim-activation …
>
> ?  I thought it had something to do with being exports/globals but
> here (%opensmtpd-accounts, etc.) it is used simply to say ‘this is a
> list’.

It’s a long story.  :-)

Initially in Guile the ‘%’ prefix was used for “system” bindings, thing
you don’t normally use as a normal user.  Then I think I started using
it for things that are meant to be “internal” or variables meant to be
read-only (not procedures).  In Guix we sort of follow that but somewhat
extend the meaning.

In the example above, I’d rather write ‘dovecot-activation’ (without
prefix, because it’s a procedure), but I’d probably write
‘%exim-activation’, depending on the phase of the moon.

HTH!

Ludo’.

[bug#36239] [PATCH] opensmtpd-service-type to extend pam

[Tobias Geerinckx-Rice]

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

Ludo',

Ludovic Courtès wrote:
>> I thought [%] had something to do with being exports/globals 
>> but
>> here (%opensmtpd-accounts, etc.) it is used simply to say ‘this 
>> is a
>> list’.
>
> It’s a long story.  :-)

Thanks for taking the time to tell it.

> Initially in Guile the ‘%’ prefix was used for “system” 
> bindings, thing
> you don’t normally use as a normal user.  Then I think I started 
> using
> it for things that are meant to be “internal” or variables meant 
> to be
> read-only (not procedures).  In Guix we sort of follow that but 
> somewhat
> extend the meaning.

s/extend/invert/ if I were feeling sassy.  I'd be lying if I said 
I understood it clearly now, but it'll be a lot easier to placate 
my obsessive side with a tasty bag of hysterical raisins than to 
keep searching for a pattern that's just not there.

> In the example above, I’d rather write ‘dovecot-activation’ 
> (without
> prefix, because it’s a procedure), but I’d probably write
> ‘%exim-activation’, depending on the phase of the moon.

Now you're just trollin'.

Thanks,

T G-R

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

bug#36239: [PATCH] opensmtpd-service-type to extend pam

[Tobias Geerinckx-Rice]

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

Ludovic Courtès wrote:
> LGTM, please push Tobias!

Done with ace8e269a40b31a9c6f413c0674529880da6a48e.

Thanks, Kristofer!

Kind regards,

T G-R

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