Re: [RFC] Support for pam_limits.so: “su” is ignored.

[Ludovic Courtès]

Ricardo Wurmus <address@hidden> skribis:

> * the pam-extension procedure appears to be called more than once on the
>   same pam-service.  I added a “(format #t ...)” statement to print the
>   name of the pam-service that was passed to the procedure and the same
>   name appeared multiple times.  The order of these multiple executions
>   appears random, so we can have three times “su”, followed by one time
>   “login”, then two times “su” again, etc.  I thought service folding
>   ensured that each service extension is evaluated/applied just once.

It’s purely functional, so it doesn’t matter how many times we call it.
;-)

That said, I agree that the fact that it’s called several times suggests
we’re not doing things efficiently (presumably that’s because
‘fold-services’ is called in several places where there’s no
memoization.)  Would be worth improving it in the future.

> * pam-services “su”, “sudo”, and possibly “passwd” are not actually
>   modified when I check their names (as I do in this patch).  If I
>   extend all pam-services without checking their names they do get
>   modified.  With the attached patch the pam file for “su” does not get
>   the pam entry for “pam_limits.so”.

But it works for me!  :-)

I’ve applied the attached patch (fixes a minor syntax issue in the patch
you sent), and then did:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix system build ~/src/configuration/pluto-configuration.scm
substitute: updating list of substitutes from 'https://mirror.hydra.gnu.org'... 
100.0%
substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
La jenaj derivoj estos konstruataj:
   /gnu/store/0fxwylcmdxy7bqmjpxwqf24v2x4yr53w-system.drv
   /gnu/store/q254gqx3rgwxn98nsaz16z8rl58nhfkw-shepherd.conf.drv
   /gnu/store/517cxw8xiniwsh9bfdp389qxlm9w5jj8-activate-service.drv
   /gnu/store/ky8p3bq0xpj3lf989fz1ba3j2xycyiz6-activate.drv
   /gnu/store/ljjl4zs722flxbpip8jdssqd549g7pay-boot.drv
   /gnu/store/vz8745sdk0ph0rykxldwzmfa3a5r7cm1-etc.drv
/gnu/store/s84hvl064k494174z692wl1vaqzrygln-system
address@hidden ~/src/guix$ grep pam_limit 
/gnu/store/s84hvl064k494174z692wl1vaqzrygln-system/etc/pam.d/*
/gnu/store/s84hvl064k494174z692wl1vaqzrygln-system/etc/pam.d/login:session 
required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/s84hvl064k494174z692wl1vaqzrygln-system/etc/pam.d/slim:session 
required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/s84hvl064k494174z692wl1vaqzrygln-system/etc/pam.d/su:session 
required pam_limits.so conf=/etc/security/limits.conf
--8<---------------cut here---------------end--------------->8---

… where the configuration in question has (pam-limits-service) in its
‘services’ field.

HTH,
Ludo’.

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index b168543..37618a3 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Alex Kost <address@hidden>
 ;;; Copyright © 2015 Mark H Weaver <address@hidden>
 ;;; Copyright © 2015 Sou Bunnbu <address@hidden>
+;;; Copyright © 2016 Ricardo Wurmus <address@hidden>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -93,6 +94,9 @@
             gpm-service-type
             gpm-service
 
+            pam-limits-service-type
+            pam-limits-service
+
             %base-services))
 
 ;;; Commentary:
@@ -811,6 +815,41 @@ settings.
 information on the configuration file syntax."
   (service syslog-service-type config-file))
 
+(define pam-limits-service-type
+  (let ()
+    ;; Create /etc/security containing the provided "limits.conf" file.
+    (define (security-limits limits-file)
+      `(("security"
+         ,(computed-file
+           "security"
+           #~(begin (mkdir #$output)
+                    (stat #$limits-file)
+                    (symlink #$limits-file
+                             (string-append #$output "/limits.conf")))))))
+    (define (pam-extension pam)
+      (let ((pam-limits (pam-entry
+                         (control "required")
+                         (module "pam_limits.so")
+                         (arguments '("conf=/etc/security/limits.conf")))))
+        (if (member (pam-service-name pam)
+                    '("login" "su" "slim"))
+            (pam-service
+             (inherit pam)
+             (session (cons pam-limits
+                            (pam-service-session pam))))
+            pam)))
+    (service-type
+     (name 'limits)
+     (extensions
+      (list (service-extension etc-service-type security-limits)
+            (service-extension pam-root-service-type
+                               (lambda _ (list pam-extension))))))))
+
+(define* (pam-limits-service #:optional (limits (plain-file "limits.conf" "")))
+  "Return a service that makes selected programs respect the limits specified
+in LIMITS via pam_limits.so."
+  (service pam-limits-service-type limits))
+

 ;;;
 ;;; Guix services.