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

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

[Ricardo Wurmus]

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

Hi Guix,

attached is a draft patch to make “su”, “login”, and “slim” respect a
limits file via pam_limits.so.  This seems to work, but I found two
things that are a little odd:

* 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.

* 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”.

I’d be glad if someone could give me a hint as to what’s going on here.
It would also help if you cannot reproduce it; then it’s probably
something to do with the state of my working directory.

~~ Ricardo


[-- Attachment #2: 0001-WIP-support-for-pam_limits.so.patch --]
[-- Type: text/x-patch, Size: 2455 bytes --]

From cfe06b7c37035ab95e7b527fcde3a785f9e7de13 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Fri, 1 Apr 2016 08:50:50 +0200
Subject: [PATCH] WIP support for pam_limits.so

---
 gnu/services/base.scm | 38 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 38 insertions(+)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index a006c00..2c256ad 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
+;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; 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:
@@ -818,6 +822,40 @@ settings.
 information on the configuration file syntax."
   (service syslog-service-type config-file))
 
+(define pam-limits-service-type
+  ;; 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))
+
 \f
 ;;;
 ;;; Guix services.
-- 
2.7.3

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

[Ludovic Courtès]

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

Ricardo Wurmus <rekado@elephly.net> 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
ludo@pluto ~/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’.


[-- Attachment #2: Type: text/x-patch, Size: 2270 bytes --]

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 <alezost@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
+;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; 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))
+
 \f
 ;;;
 ;;; Guix services.

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

[Ricardo Wurmus]

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


Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> * 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:

Oh, yes, I must have sent another version.  Attached is the version I
used.

>
> --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
> ludo@pluto ~/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.

How odd.  It does not work for me:

--8<---------------cut here---------------start------------->8---
rekado in guix: ./pre-inst-env guix system build /etc/config.scm
substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
The following derivations will be built:
   /gnu/store/8pldcwijn1szpyc4cbm4z808sbdn18r0-system.drv
   /gnu/store/x2pigxf3mrf5crbmnjfxza8h3a70fvnv-activate-service.drv
   /gnu/store/nwm5gsfavv35g3hjgzi5xymhxhqf2g4l-activate.drv
   /gnu/store/38x73bdqj40wmy3lz8qnpfc20wb0dr6x-boot.drv
   /gnu/store/mlkssjhw4avm8a37yi0nfk4z4zd9bkj6-security.drv
   /gnu/store/1lxncnhdab3l732r144r5ww0y9rjq72f-etc.drv
/gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system
rekado in guix: grep limits /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/*
/gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
--8<---------------cut here---------------end--------------->8---

When my configuration looks like this:

      (services (list (pam-limits-service)))

Then there’s one file containing pam_limits, namely “/etc/pam.d/su”.
When my configuration looks like this, however:

      (services (cons* (pam-limits-service)
                       %desktop-services))

Then only “/etc/pam.d/login” and “/etc/pam.d/slim” contain pam_limits;
“/etc/pam.d/su” does not.

~~ Ricardo


[-- Attachment #2: 0001-services-Add-pam-entry-for-pam_limits.patch --]
[-- Type: text/x-patch, Size: 2734 bytes --]

From 5bb848b73a19b5ebaa301c7c6bdedbd4a3bab8b6 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 12 Oct 2015 07:11:51 +0200
Subject: [PATCH] services: Add pam entry for pam_limits.

* gnu/services/base.scm (pam-limits-service-type): WIP support for
  pam_limits.so
---
 gnu/services/base.scm | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index b168543..7e0fbd4 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -3,6 +3,7 @@
 ;;; Copyright © 2015, 2016 Alex Kost <alezost@gmail.com>
 ;;; Copyright © 2015 Mark H Weaver <mhw@netris.org>
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
+;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; 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,42 @@ settings.
 information on the configuration file syntax."
   (service syslog-service-type config-file))
 
+(define pam-limits-service-type
+  (let ((security-limits
+         ;; Create /etc/security containing the provided "limits.conf" file.
+         (lambda (limits-file)
+           `(("security"
+              ,(computed-file
+                "security"
+                #~(begin (mkdir #$output)
+                         (stat #$limits-file)
+                         (symlink #$limits-file
+                                  (string-append #$output "/limits.conf"))))))))
+        (pam-extension
+         (lambda (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))
+
 \f
 ;;;
 ;;; Guix services.
-- 
2.7.3

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

[Ludovic Courtès]

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

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> --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
>> ludo@pluto ~/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.

… in addition to ‘%base-services’ and a bunch of things.

> How odd.  It does not work for me:
>
> rekado in guix: ./pre-inst-env guix system build /etc/config.scm
> substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
> The following derivations will be built:
>    /gnu/store/8pldcwijn1szpyc4cbm4z808sbdn18r0-system.drv
>    /gnu/store/x2pigxf3mrf5crbmnjfxza8h3a70fvnv-activate-service.drv
>    /gnu/store/nwm5gsfavv35g3hjgzi5xymhxhqf2g4l-activate.drv
>    /gnu/store/38x73bdqj40wmy3lz8qnpfc20wb0dr6x-boot.drv
>    /gnu/store/mlkssjhw4avm8a37yi0nfk4z4zd9bkj6-security.drv
>    /gnu/store/1lxncnhdab3l732r144r5ww0y9rjq72f-etc.drv
> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system
> rekado in guix: grep limits /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/*
> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
>
> When my configuration looks like this:
>
>       (services (list (pam-limits-service)))
>
> Then there’s one file containing pam_limits, namely “/etc/pam.d/su”.

That’s expected: in this case, there’s no ‘login’ and no ‘slim’.

> When my configuration looks like this, however:
>
>       (services (cons* (pam-limits-service)
>                        %desktop-services))
>
> Then only “/etc/pam.d/login” and “/etc/pam.d/slim” contain pam_limits;
> “/etc/pam.d/su” does not.

Hmm, I can’t reproduce it.  For instance, with this:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 852 bytes --]

diff --git a/gnu/system/examples/lightweight-desktop.tmpl b/gnu/system/examples/lightweight-desktop.tmpl
index 7cb461f..f98b6cf 100644
--- a/gnu/system/examples/lightweight-desktop.tmpl
+++ b/gnu/system/examples/lightweight-desktop.tmpl
@@ -33,13 +33,13 @@
 
   ;; Add a bunch of window managers; we can choose one at
   ;; the log-in screen with F1.
-  (packages (cons* ratpoison i3-wm xmonad  ;window managers
+  (packages (cons* ratpoison
                    nss-certs               ;for HTTPS access
                    %base-packages))
 
   ;; Use the "desktop" services, which include the X11
   ;; log-in service, networking with Wicd, and more.
-  (services %desktop-services)
+  (services (cons (pam-limits-service) %desktop-services))
 
   ;; Allow resolution of '.local' host names with mDNS.
   (name-service-switch %mdns-host-lookup-nss))

[-- Attachment #3: Type: text/plain, Size: 968 bytes --]


I get:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
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%

[...]

/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
$ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/su:session required pam_limits.so conf=/etc/security/limits.conf
--8<---------------cut here---------------end--------------->8---

Could you try it?

Thanks,
Ludo’.

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

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> --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
>>> ludo@pluto ~/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.
>
> … in addition to ‘%base-services’ and a bunch of things.
>
>> How odd.  It does not work for me:
>>
>> rekado in guix: ./pre-inst-env guix system build /etc/config.scm
>> substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
>> The following derivations will be built:
>>    /gnu/store/8pldcwijn1szpyc4cbm4z808sbdn18r0-system.drv
>>    /gnu/store/x2pigxf3mrf5crbmnjfxza8h3a70fvnv-activate-service.drv
>>    /gnu/store/nwm5gsfavv35g3hjgzi5xymhxhqf2g4l-activate.drv
>>    /gnu/store/38x73bdqj40wmy3lz8qnpfc20wb0dr6x-boot.drv
>>    /gnu/store/mlkssjhw4avm8a37yi0nfk4z4zd9bkj6-security.drv
>>    /gnu/store/1lxncnhdab3l732r144r5ww0y9rjq72f-etc.drv
>> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system
>> rekado in guix: grep limits /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/*
>> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
>> /gnu/store/qwjbfjzqlbdcrsbnyyjylfkhm6kqna5d-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
>>
>> When my configuration looks like this:
>>
>>       (services (list (pam-limits-service)))
>>
>> Then there’s one file containing pam_limits, namely “/etc/pam.d/su”.
>
> That’s expected: in this case, there’s no ‘login’ and no ‘slim’.

Yes, I know.  I just wanted to show that the file for “su” *does*
contain pam_limits in this case, but not when %desktop-services is used
as well.

>> When my configuration looks like this, however:
>>
>>       (services (cons* (pam-limits-service)
>>                        %desktop-services))
>>
>> Then only “/etc/pam.d/login” and “/etc/pam.d/slim” contain pam_limits;
>> “/etc/pam.d/su” does not.
>
> Hmm, I can’t reproduce it.  For instance, with this:
>
>
> I get:
>
> --8<---------------cut here---------------start------------->8---
> $ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
> 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%
>
> [...]
>
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
> $ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/su:session required pam_limits.so conf=/etc/security/limits.conf
> --8<---------------cut here---------------end--------------->8---
>
> Could you try it?

I did and I don’t get the same as you do:

--8<---------------cut here---------------start------------->8---
$ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
The following derivations will be built:
   /gnu/store/l8r7k5ysw5vkdi67rcz9wx5gl9sxp892-system.drv
   /gnu/store/5q0rh32ns03y4ndsj1fmsim9zm04x182-activate-service.drv
   /gnu/store/rvgr25dfw70kf3dyr3mp8w9dmpqsqlll-activate.drv
   /gnu/store/56d9psa8xcv3i6wqfc01zb39i9sbd7v5-boot.drv
   /gnu/store/siny40wkak05sqlnmwwsmpxwh93rva1f-gtk-icon-themes.drv
   /gnu/store/fx5bkg9cz15w90yqximsd678g31blyzk-info-dir.drv
   /gnu/store/68ri6jqwbg1k15iiyvj3j9a065c22rd1-ca-certificate-bundle.drv
   /gnu/store/ja6pgayi1qcyf8ffq27s4jimzcq2nm54-profile.drv
   /gnu/store/50s165xprg605n58i81z49sv1f797vpz-etc.drv
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
$ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
/gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
--8<---------------cut here---------------end--------------->8---

I’m using Guix at commit a754eaf with additional commits to add packages
to gnu/packages and the patch I sent earlier to extend the pam files
with pam_limits.  The only uncommited change is the modification of
“gnu/system/examples/lightweight-desktop.tmpl”.

Very odd.  It’s possible that this is a problem with my setup here.  If
that’s so, would you be okay with the commit (if it had a proper commit
message)?

~~ Ricardo

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

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I get:
>>
>> --8<---------------cut here---------------start------------->8---
>> $ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
>> 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%
>>
>> [...]
>>
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
>> $ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/su:session required pam_limits.so conf=/etc/security/limits.conf
>> --8<---------------cut here---------------end--------------->8---
>>
>> Could you try it?
>
> I did and I don’t get the same as you do:
>
> $ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
> substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
> The following derivations will be built:
>    /gnu/store/l8r7k5ysw5vkdi67rcz9wx5gl9sxp892-system.drv
>    /gnu/store/5q0rh32ns03y4ndsj1fmsim9zm04x182-activate-service.drv
>    /gnu/store/rvgr25dfw70kf3dyr3mp8w9dmpqsqlll-activate.drv
>    /gnu/store/56d9psa8xcv3i6wqfc01zb39i9sbd7v5-boot.drv
>    /gnu/store/siny40wkak05sqlnmwwsmpxwh93rva1f-gtk-icon-themes.drv
>    /gnu/store/fx5bkg9cz15w90yqximsd678g31blyzk-info-dir.drv
>    /gnu/store/68ri6jqwbg1k15iiyvj3j9a065c22rd1-ca-certificate-bundle.drv
>    /gnu/store/ja6pgayi1qcyf8ffq27s4jimzcq2nm54-profile.drv
>    /gnu/store/50s165xprg605n58i81z49sv1f797vpz-etc.drv
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
> $ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf

Crazy stuff.

The ‘/etc-entry’ procedure in (gnu system pam) clearly calls the
transformation procedure for all the PAM services.  I don’t see what
could go wrong.

Could you add a bunch of ’pk’ in this procedure and in your
‘pam-extension’ procedure as well and report on that?

> I’m using Guix at commit a754eaf with additional commits to add packages
> to gnu/packages and the patch I sent earlier to extend the pam files
> with pam_limits.  The only uncommited change is the modification of
> “gnu/system/examples/lightweight-desktop.tmpl”.
>
> Very odd.  It’s possible that this is a problem with my setup here.  If
> that’s so, would you be okay with the commit (if it had a proper commit
> message)?

Yes (and doc :-)).

I haven’t checked the feasibility etc., but eventually, maybe it would
be best to have Scheme bindings for limits.conf.  That way, we could
write services that extend ‘limits-service-type’ with new limits or
something.

WDYT?

Thanks,
Ludo’.

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

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> Ludovic Courtès <ludo@gnu.org> writes:
>
> [...]
>
>>> I get:
>>>
>>> --8<---------------cut here---------------start------------->8---
>>> $ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
>>> 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%
>>>
>>> [...]
>>>
>>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
>>> $ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
>>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
>>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
>>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/su:session required pam_limits.so conf=/etc/security/limits.conf
>>> --8<---------------cut here---------------end--------------->8---
>>>
>>> Could you try it?
>>
>> I did and I don’t get the same as you do:
>>
>> $ ./pre-inst-env guix system build gnu/system/examples/lightweight-desktop.tmpl
>> substitute: updating list of substitutes from 'https://hydra.gnu.org'... 100.0%
>> The following derivations will be built:
>>    /gnu/store/l8r7k5ysw5vkdi67rcz9wx5gl9sxp892-system.drv
>>    /gnu/store/5q0rh32ns03y4ndsj1fmsim9zm04x182-activate-service.drv
>>    /gnu/store/rvgr25dfw70kf3dyr3mp8w9dmpqsqlll-activate.drv
>>    /gnu/store/56d9psa8xcv3i6wqfc01zb39i9sbd7v5-boot.drv
>>    /gnu/store/siny40wkak05sqlnmwwsmpxwh93rva1f-gtk-icon-themes.drv
>>    /gnu/store/fx5bkg9cz15w90yqximsd678g31blyzk-info-dir.drv
>>    /gnu/store/68ri6jqwbg1k15iiyvj3j9a065c22rd1-ca-certificate-bundle.drv
>>    /gnu/store/ja6pgayi1qcyf8ffq27s4jimzcq2nm54-profile.drv
>>    /gnu/store/50s165xprg605n58i81z49sv1f797vpz-etc.drv
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system
>> $ grep pam_limit /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/*
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/login:session required pam_limits.so conf=/etc/security/limits.conf
>> /gnu/store/rx31x0m8fk5aknwf754in9yxl7vcq8ls-system/etc/pam.d/slim:session required pam_limits.so conf=/etc/security/limits.conf
>
> Crazy stuff.
>
> The ‘/etc-entry’ procedure in (gnu system pam) clearly calls the
> transformation procedure for all the PAM services.  I don’t see what
> could go wrong.

I tried this patch on another machine and it works just fine.  Weird.

> Could you add a bunch of ’pk’ in this procedure and in your
> ‘pam-extension’ procedure as well and report on that?

I’ll try this later.  I did this before and saw that my extension
procedure is in fact called with “su” (and the correct branch is taken),
but it has no effect.  I’ll did deeper at some other point.

>> I’m using Guix at commit a754eaf with additional commits to add packages
>> to gnu/packages and the patch I sent earlier to extend the pam files
>> with pam_limits.  The only uncommited change is the modification of
>> “gnu/system/examples/lightweight-desktop.tmpl”.
>>
>> Very odd.  It’s possible that this is a problem with my setup here.  If
>> that’s so, would you be okay with the commit (if it had a proper commit
>> message)?
>
> Yes (and doc :-)).

Oh, right :)

> I haven’t checked the feasibility etc., but eventually, maybe it would
> be best to have Scheme bindings for limits.conf.  That way, we could
> write services that extend ‘limits-service-type’ with new limits or
> something.

I’m not very familiar with limits.conf (I only copied the realtime audio
settings from the JACK website), but it looks like the format is very
simple.  We could certainly have something like a “limits-entry” to
specify the limits and a matching service.

~~ Ricardo

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

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[...]

>> I haven’t checked the feasibility etc., but eventually, maybe it would
>> be best to have Scheme bindings for limits.conf.  That way, we could
>> write services that extend ‘limits-service-type’ with new limits or
>> something.
>
> I’m not very familiar with limits.conf (I only copied the realtime audio
> settings from the JACK website), but it looks like the format is very
> simple.  We could certainly have something like a “limits-entry” to
> specify the limits and a matching service.

Sounds good.

Ludo’.

[PATCH] Add pam-limits-service. (was: [RFC] Support for pam_limits.so: “su” is ignored.)

[Ricardo Wurmus]

Ricardo Wurmus <rekado@elephly.net> writes:

> Ludovic Courtès <ludo@gnu.org> writes:

>> I haven’t checked the feasibility etc., but eventually, maybe it would
>> be best to have Scheme bindings for limits.conf.  That way, we could
>> write services that extend ‘limits-service-type’ with new limits or
>> something.
>
> I’m not very familiar with limits.conf (I only copied the realtime audio
> settings from the JACK website), but it looks like the format is very
> simple.  We could certainly have something like a “limits-entry” to
> specify the limits and a matching service.

A couple of months later, here’s my patch for doing just that.

We now have a constructor “pam-limits-entry”, which validates given
settings (i.e. it throws an error when values are passed that don’t make
sense) and returns a value of type “<pam-limits-entry>”.

A list of these values can be passed to “pam-limits-service”, which
generates a working “/etc/security/limits.conf”.  I’m using it right now
with the exact same limits that are now documented in the manual.

This snippet:

  (pam-limits-service
   (list
    (pam-limits-entry "@realtime" 'both 'rtprio 99)
    (pam-limits-entry "@realtime" 'both 'memlock 'unlimited)))

generates a limits.conf file with the following contents:

  @realtime   -  rtprio     99
  @realtime   -  memlock    unlimited

One advantage of using “pam-limits-entry” instead of a plain string is
that values are validated according to the documentation in “man 5
limits.conf”.

(I also added my copyright line to the manual, which should have been
done in 2015 when I documented the CRAN importer.)

~~ Ricardo



From 3f5d7b405ac7faadd753719fe4100d8f6605d191 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 12 Oct 2015 07:11:51 +0200
Subject: [PATCH] services: Add pam-limits-service.

* gnu/system/pam.scm (<pam-limits-entry>): New record type.
(pam-limits-entry, pam-limits-entry->string): New procedures.
* gnu/services/base.scm (pam-limits-service-type): New variable.
(pam-limits-service): New procedure.
* doc/guix.texi (Base Services): Document it.
---
 doc/guix.texi         | 29 ++++++++++++++++++++++++++
 gnu/services/base.scm | 42 +++++++++++++++++++++++++++++++++++++
 gnu/system/pam.scm    | 58 +++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)

diff --git a/doc/guix.texi b/doc/guix.texi
index a2732de..815f6e8 100644
--- a/doc/guix.texi
+++ b/doc/guix.texi
@@ -17,6 +17,7 @@ Copyright @copyright{} 2015, 2016 Mathieu Lirzin@*
 Copyright @copyright{} 2014 Pierre-Antoine Rault@*
 Copyright @copyright{} 2015 Taylan Ulrich Bayırlı/Kammer@*
 Copyright @copyright{} 2015, 2016 Leo Famulari@*
+Copyright @copyright{} 2015, 2016 Ricardo Wurmus@*
 Copyright @copyright{} 2016 Ben Woodcroft@*
 Copyright @copyright{} 2016 Chris Marusich@*
 Copyright @copyright{} 2016 Efraim Flashner
@@ -7554,6 +7555,34 @@ to add @var{device} to the kernel's entropy pool.  The service will fail if
 @var{device} does not exist.
 @end deffn
 
+@anchor{pam-limits-service}
+@cindex session limits
+@cindex ulimit
+@cindex priority
+@deffn {Scheme Procedure} pam-limits-service [#:limits @var{limits}]
+
+Return a service that installs a configuration file for the
+@code{pam_limits} module.  The procedure optionally takes a list of
+@code{pam-limits-entry} values, which can be used to specify
+@code{ulimit} limits and nice priority limits to user sessions.
+
+The following limits definition sets two hard and soft limits for all
+login sessions of users in the @code{realtime} group:
+
+@example
+(pam-limits-service
+ (list
+  (pam-limits-entry "@@realtime" 'both 'rtprio 99)
+  (pam-limits-entry "@@realtime" 'both 'memlock 'unlimited)))
+@end example
+
+The first entry increases the maximum realtime priority for
+non-privileged processes; the second entry lifts any restriction of the
+maximum address space that can be locked in memory.  These settings are
+commonly used for real-time audio systems.
+@end deffn
+
+
 @node Scheduled Job Execution
 @subsubsection Scheduled Job Execution
 
diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index c9c2594..df58bbd 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -5,6 +5,7 @@
 ;;; Copyright © 2015 Sou Bunnbu <iyzsong@gmail.com>
 ;;; Copyright © 2016 Leo Famulari <leo@famulari.name>
 ;;; Copyright © 2016 David Craven <david@craven.ch>
+;;; Copyright © 2016 Ricardo Wurmus <rekado@elephly.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -100,6 +101,8 @@
             urandom-seed-service
             rngd-service-type
             rngd-service
+            pam-limits-service-type
+            pam-limits-service
 
             %base-services))
 
@@ -924,6 +927,45 @@ settings.
 information on the configuration file syntax."
   (service syslog-service-type config-file))
 
+(define pam-limits-service-type
+  (let ((security-limits
+         ;; Create /etc/security containing the provided "limits.conf" file.
+         (lambda (limits-file)
+           `(("security"
+              ,(computed-file
+                "security"
+                #~(begin (mkdir #$output)
+                         (stat #$limits-file)
+                         (symlink #$limits-file
+                                  (string-append #$output "/limits.conf"))))))))
+        (pam-extension
+         (lambda (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 '()))
+  "Return a service that makes selected programs respect the list of
+pam-limits-entry specified in LIMITS via pam_limits.so."
+  (service pam-limits-service-type
+           (plain-file "limits.conf"
+                       (string-join (map pam-limits-entry->string limits)
+                                    "\n"))))
+
 \f
 ;;;
 ;;; Guix services.
diff --git a/gnu/system/pam.scm b/gnu/system/pam.scm
index 743039d..81b9eec 100644
--- a/gnu/system/pam.scm
+++ b/gnu/system/pam.scm
@@ -23,6 +23,7 @@
   #:use-module (gnu services)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
+  #:use-module (srfi srfi-9)
   #:use-module (srfi srfi-11)
   #:use-module (srfi srfi-26)
   #:use-module ((guix utils) #:select (%current-system))
@@ -38,6 +39,13 @@
             pam-entry-module
             pam-entry-arguments
 
+            pam-limits-entry
+            pam-limits-entry-domain
+            pam-limits-entry-type
+            pam-limits-entry-item
+            pam-limits-entry-value
+            pam-limits-entry->string
+
             pam-services->directory
             unix-pam-service
             base-pam-services
@@ -76,6 +84,56 @@
   (arguments  pam-entry-arguments        ; list of string-valued g-expressions
               (default '())))
 
+(define-record-type <pam-limits-entry>
+  (make-pam-limits-entry domain type item value)
+  pam-limits-entry?
+  (domain     pam-limits-entry-domain)   ; string
+  (type       pam-limits-entry-type)     ; symbol
+  (item       pam-limits-entry-item)     ; symbol
+  (value      pam-limits-entry-value))   ; symbol or number
+
+(define (pam-limits-entry domain type item value)
+  "Construct a pam-limits-entry ensuring that the provided values are valid."
+  (define (valid? value)
+    (case item
+      ((priority) (number? value))
+      ((nice)     (and (number? value)
+                       (>= value -20)
+                       (<= value 19)))
+      (else       (or (and (number? value)
+                           (>= value -1))
+                      (member value '(unlimited infinity))))))
+  (define items
+    (list 'core      'data       'fsize
+          'memlock   'nofile     'rss
+          'stack     'cpu        'nproc
+          'as        'maxlogins  'maxsyslogins
+          'priority  'locks      'sigpending
+          'msgqueue  'nice       'rtprio))
+  (when (not (member type '(hard soft both)))
+    (error "invalid limit type" type))
+  (when (not (member item items))
+    (error "invalid limit item" item))
+  (when (not (valid? value))
+    (error "invalid limit value" value))
+  (make-pam-limits-entry domain type item value))
+
+(define (pam-limits-entry->string entry)
+  "Convert a pam-limits-entry record to a string."
+  (match entry
+    (($ <pam-limits-entry> domain type item value)
+     (string-join (list domain
+                        (if (eq? type 'both)
+                            "-"
+                            (symbol->string type))
+                        (symbol->string item)
+                        (cond
+                         ((symbol? value)
+                          (symbol->string value))
+                         (else
+                          (number->string value))))
+                  "	"))))
+
 (define (pam-service->configuration service)
   "Return the derivation building the configuration file for SERVICE, to be
 dumped in /etc/pam.d/NAME, where NAME is the name of SERVICE."
-- 
2.8.4

Re: [PATCH] Add pam-limits-service.

[Ludovic Courtès]

Guten Tag!

Ricardo Wurmus <rekado@elephly.net> skribis:

> We now have a constructor “pam-limits-entry”, which validates given
> settings (i.e. it throws an error when values are passed that don’t make
> sense) and returns a value of type “<pam-limits-entry>”.
>
> A list of these values can be passed to “pam-limits-service”, which
> generates a working “/etc/security/limits.conf”.  I’m using it right now
> with the exact same limits that are now documented in the manual.
>
> This snippet:
>
>   (pam-limits-service
>    (list
>     (pam-limits-entry "@realtime" 'both 'rtprio 99)
>     (pam-limits-entry "@realtime" 'both 'memlock 'unlimited)))
>
> generates a limits.conf file with the following contents:
>
>   @realtime   -  rtprio     99
>   @realtime   -  memlock    unlimited
>
> One advantage of using “pam-limits-entry” instead of a plain string is
> that values are validated according to the documentation in “man 5
> limits.conf”.

Nice!

Eventually, we should probably use a constructor in the spirit of (rnrs
enums) to provide expansion-time validation, as already done in (gnu
system nss) (info "(guile) rnrs enums").

> From 3f5d7b405ac7faadd753719fe4100d8f6605d191 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 12 Oct 2015 07:11:51 +0200
> Subject: [PATCH] services: Add pam-limits-service.
>
> * gnu/system/pam.scm (<pam-limits-entry>): New record type.
> (pam-limits-entry, pam-limits-entry->string): New procedures.
> * gnu/services/base.scm (pam-limits-service-type): New variable.
> (pam-limits-service): New procedure.
> * doc/guix.texi (Base Services): Document it.

[...]

> +@deffn {Scheme Procedure} pam-limits-service [#:limits @var{limits}]
> +
> +Return a service that installs a configuration file for the
> +@code{pam_limits} module.  The procedure optionally takes a list of
         ^^^^^^^^^^^^^^^^^^
It would be nice to add an @uref to the on-line manual of pam_limits, if
it exists.

> +(define pam-limits-service-type
> +  (let ((security-limits
> +         ;; Create /etc/security containing the provided "limits.conf" file.
> +         (lambda (limits-file)
> +           `(("security"
> +              ,(computed-file
> +                "security"
> +                #~(begin (mkdir #$output)
> +                         (stat #$limits-file)
> +                         (symlink #$limits-file
> +                                  (string-append #$output "/limits.conf"))))))))

Indentation, rather:

  (begin
    (mkdir #$output)
    …)

> +    (service-type
> +     (name 'limits)
> +     (extensions
> +      (list (service-extension etc-service-type security-limits)
> +            (service-extension pam-root-service-type
> +                               (lambda _ (list pam-extension))))))))

It may be useful to allow users to extend this service with additional
<pam-limits-entry> objects.  To do that we’d simply need something like:

  (service-type
    (name 'limits)
    ;; …
    (compose concatenate)   ;concatenate lists of <pam-limits-entry>
    (extend append))        ;append them

WDYT?

This shouldn’t block this patch, though.

> +(define-record-type <pam-limits-entry>
> +  (make-pam-limits-entry domain type item value)

Maybe just add a comment above with the URL of the reference manual.

Otherwise LGTM, thank you!

Ludo’.

Re: [PATCH] Add pam-limits-service.

[Ricardo Wurmus]

Ludovic Courtès <ludo@gnu.org> writes:

> Ricardo Wurmus <rekado@elephly.net> skribis:

[…]

>> One advantage of using “pam-limits-entry” instead of a plain string is
>> that values are validated according to the documentation in “man 5
>> limits.conf”.
>
> Nice!
>
> Eventually, we should probably use a constructor in the spirit of (rnrs
> enums) to provide expansion-time validation, as already done in (gnu
> system nss) (info "(guile) rnrs enums").

Oh, that’s new to me.  I was a little disappointed that records don’t
support what Haskell calls “smart constructors”.  I’ll check out enums
and see if I can improve this later.  (Added to my growing org-mode
list of Guix things.)

Thanks for the pointer!

>> From 3f5d7b405ac7faadd753719fe4100d8f6605d191 Mon Sep 17 00:00:00 2001
>> From: Ricardo Wurmus <rekado@elephly.net>
>> Date: Mon, 12 Oct 2015 07:11:51 +0200
>> Subject: [PATCH] services: Add pam-limits-service.
>>
>> * gnu/system/pam.scm (<pam-limits-entry>): New record type.
>> (pam-limits-entry, pam-limits-entry->string): New procedures.
>> * gnu/services/base.scm (pam-limits-service-type): New variable.
>> (pam-limits-service): New procedure.
>> * doc/guix.texi (Base Services): Document it.
>
> [...]
>
>> +@deffn {Scheme Procedure} pam-limits-service [#:limits @var{limits}]
>> +
>> +Return a service that installs a configuration file for the
>> +@code{pam_limits} module.  The procedure optionally takes a list of
>          ^^^^^^^^^^^^^^^^^^
> It would be nice to add an @uref to the on-line manual of pam_limits, if
> it exists.

Added a link.

>> +(define pam-limits-service-type
>> +  (let ((security-limits
>> +         ;; Create /etc/security containing the provided "limits.conf" file.
>> +         (lambda (limits-file)
>> +           `(("security"
>> +              ,(computed-file
>> +                "security"
>> +                #~(begin (mkdir #$output)
>> +                         (stat #$limits-file)
>> +                         (symlink #$limits-file
>> +                                  (string-append #$output "/limits.conf"))))))))
>
> Indentation, rather:
>
>   (begin
>     (mkdir #$output)
>     …)

Okay.

>> +    (service-type
>> +     (name 'limits)
>> +     (extensions
>> +      (list (service-extension etc-service-type security-limits)
>> +            (service-extension pam-root-service-type
>> +                               (lambda _ (list pam-extension))))))))
>
> It may be useful to allow users to extend this service with additional
> <pam-limits-entry> objects.  To do that we’d simply need something like:
>
>   (service-type
>     (name 'limits)
>     ;; …
>     (compose concatenate)   ;concatenate lists of <pam-limits-entry>
>     (extend append))        ;append them
>
> WDYT?
>
> This shouldn’t block this patch, though.

Currently the default limits are an empty list, so there doesn’t seem to
be any advantage over simply passing a list of <pam-limits-entry>
values.  Or is composition/extension here about e.g. enabling some
specialised sub-service to inherit from pam-limits-service and modify
the list of entries?

>> +(define-record-type <pam-limits-entry>
>> +  (make-pam-limits-entry domain type item value)
>
> Maybe just add a comment above with the URL of the reference manual.

Done.

> Otherwise LGTM, thank you!

Yay, pushing this to master now.  Thank you for the patient review!

~~ Ricardo