diff mbox

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

Message ID 87eg6snitm.fsf@elephly.net
State New
Headers show

Commit Message

Ricardo Wurmus July 17, 2016, 7:52 p.m. UTC
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(+)

Comments

Ludovic Courtès July 18, 2016, 12:29 p.m. UTC | #1
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’.
Ricardo Wurmus July 20, 2016, 5:28 a.m. UTC | #2
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
diff mbox

Patch

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"))))
+
 
 ;;;
 ;;; 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."