[RFC]: Respect /etc/security/limits.conf

[RFC]: Respect /etc/security/limits.conf

[Ricardo Wurmus]

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

Hi Guix,

I noticed that we aren’t using pam_limits.so, so it is impossible to set
session-wide limits per user.  This is useful to explicitly grant the
“audio” group realtime priorities and lift memory allocation
restrictions.

The attached patch tries to add an entry for pam_limits.so, but I have
no idea if this actually works or if this is the way it should be done.
As far as I can tell we only need the pam_limits.so entry for
“/etc/pam.d/login”, but I could not find where this file is generated.

Also, I wonder how users are supposed to edit /etc/security/limits.conf
at all.  I suppose they are not to edit anything in /etc anyway.
pam_limits.so also reads *.conf files in “/etc/security/limits.d/” and
maybe it would make sense for packages to provide a
“$out/etc/security/limits.d/$name.conf” file with settings.  For
example, the “jack” packages could then provide
“$out/etc/security/limits.d/realtime.conf”, which contains the
following:

    @realtime   -  rtprio     99
    @realtime   -  memlock    unlimited

(See http://www.jackaudio.org/faq/linux_rt_config.html)

A user in the “realtime” group could then finally use JACK in realtime
mode.

What is the best way to make this work?  (I really want to run JACK in
realtime mode.)

~~ Ricardo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-WIP-respect-etc-security-limits.conf.patch --]
[-- Type: text/x-patch, Size: 1809 bytes --]

From 7a92f5a3f9bfa22749d0a635d5ac878560336611 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Sat, 19 Sep 2015 12:40:20 +0200
Subject: [PATCH] WIP: respect /etc/security/limits.conf

---
 gnu/system/linux.scm | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/gnu/system/linux.scm b/gnu/system/linux.scm
index 7461a4a..d94e4ca 100644
--- a/gnu/system/linux.scm
+++ b/gnu/system/linux.scm
@@ -132,7 +132,10 @@ dumped in /etc/pam.d/NAME, where NAME is the name of SERVICE."
 (define unix-pam-service
   (let ((unix (pam-entry
                (control "required")
-               (module "pam_unix.so"))))
+               (module "pam_unix.so")))
+        (limits (pam-entry
+                 (control "required")
+                 (module "pam_limits.so"))))
     (lambda* (name #:key allow-empty-passwords? motd)
       "Return a standard Unix-style PAM service for NAME.  When
 ALLOW-EMPTY-PASSWORDS? is true, allow empty passwords.  When MOTD is true, it
@@ -154,13 +157,13 @@ should be the name of a file used as the message-of-the-day."
                           ;; Store SHA-512 encrypted passwords in /etc/shadow.
                           (arguments '("sha512" "shadow")))))
          (session (if motd
-                      (list unix
+                      (list unix limits
                             (pam-entry
                              (control "optional")
                              (module "pam_motd.so")
                              (arguments
                               (list #~(string-append "motd=" #$motd)))))
-                      (list unix))))))))
+                      (list unix limits))))))))
 
 (define (rootok-pam-service command)
   "Return a PAM service for COMMAND such that 'root' does not need to
-- 
2.5.0

Re: [RFC]: Respect /etc/security/limits.conf

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> The attached patch tries to add an entry for pam_limits.so, but I have
> no idea if this actually works or if this is the way it should be done.
> As far as I can tell we only need the pam_limits.so entry for
> “/etc/pam.d/login”, but I could not find where this file is generated.

It is generated based on the ‘pam-services’ field of the service
returned by ‘mingetty-service’.

Maybe it would be best to adjust just that part?

> Also, I wonder how users are supposed to edit /etc/security/limits.conf
> at all.  I suppose they are not to edit anything in /etc anyway.
> pam_limits.so also reads *.conf files in “/etc/security/limits.d/” and
> maybe it would make sense for packages to provide a
> “$out/etc/security/limits.d/$name.conf” file with settings.  For
> example, the “jack” packages could then provide
> “$out/etc/security/limits.d/realtime.conf”, which contains the
> following:
>
>     @realtime   -  rtprio     99
>     @realtime   -  memlock    unlimited
>
> (See http://www.jackaudio.org/faq/linux_rt_config.html)

Is this PREFIX/etc/security/limits.d convention already used?  If not,
I’d rather avoid inventing it.  ;-)

What we could do is add a field in ‘operating-system’ to specify the
limits.conf file to install as /etc/security/limits.conf?

It would be even better to create Scheme data types that mirror the
settings of a limits.conf file (similar to what is done for PAM
settings), and have users fiddle with that rather than with a plain text
file.

> A user in the “realtime” group could then finally use JACK in realtime
> mode.
>
> What is the best way to make this work?  (I really want to run JACK in
> realtime mode.)

(In the meantime I think your patch plus manual fiddling of
/etc/security/limits.conf does the job.)

Ludo’.

Re: [RFC]: Respect /etc/security/limits.conf

[Ricardo Wurmus]

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


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

> Ricardo Wurmus <rekado@elephly.net> skribis:
>
>> The attached patch tries to add an entry for pam_limits.so, but I have
>> no idea if this actually works or if this is the way it should be done.
>> As far as I can tell we only need the pam_limits.so entry for
>> “/etc/pam.d/login”, but I could not find where this file is generated.
>
> It is generated based on the ‘pam-services’ field of the service
> returned by ‘mingetty-service’.
>
> Maybe it would be best to adjust just that part?

Oh, right.  Attached are two patches:

  * The first exports the pam-service-* getters, making it possible to
    extend a pam-service.

  * The second extends the “session” field of the mingetty-service to
    add “pam_limits.so” to the required modules.

Loading the module doesn’t yet do anything on GuixSD because we don’t
generate ‘/etc/security/limits.conf’ (or ‘/etc/security/limits.d/’), but
it should respect such file if it does exist.  (I have not yet tested
this, but I will some time this week.)

Does this look okay?

> Is this PREFIX/etc/security/limits.d convention already used?  If not,
> I’d rather avoid inventing it.  ;-)
>
> What we could do is add a field in ‘operating-system’ to specify the
> limits.conf file to install as /etc/security/limits.conf?

Yes, that’s a better idea.

> It would be even better to create Scheme data types that mirror the
> settings of a limits.conf file (similar to what is done for PAM
> settings), and have users fiddle with that rather than with a plain text
> file.

I’ll familiarise myself with how other (service) configuration files are
created in GuixSD and propose a patch later.

~~ Ricardo


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-system-Export-pam-service-accessors.patch --]
[-- Type: text/x-patch, Size: 954 bytes --]

From cdf974eb7595cfb8997111d09f6da2350c72afdd Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 12 Oct 2015 07:08:32 +0200
Subject: [PATCH 1/2] system: Export pam-service accessors.

* gnu/system/linux.scm (pam-service-name, pam-service-account,
  pam-service-auth, pam-service-password, pam-service-session): Export.
---
 gnu/system/linux.scm | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/gnu/system/linux.scm b/gnu/system/linux.scm
index cd14bc9..bfb1db6 100644
--- a/gnu/system/linux.scm
+++ b/gnu/system/linux.scm
@@ -26,6 +26,12 @@
   #:use-module (srfi srfi-26)
   #:use-module ((guix utils) #:select (%current-system))
   #:export (pam-service
+            pam-service-name
+            pam-service-account
+            pam-service-auth
+            pam-service-password
+            pam-service-session
+
             pam-entry
             pam-services->directory
             unix-pam-service
-- 
2.5.0


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: 0002-services-Add-entry-for-pam_limits-to-mingetty-pam-se.patch --]
[-- Type: text/x-patch, Size: 1705 bytes --]

From 0a1b5cad3d302d937a29dec95e805488a26b34e8 Mon Sep 17 00:00:00 2001
From: Ricardo Wurmus <rekado@elephly.net>
Date: Mon, 12 Oct 2015 07:11:51 +0200
Subject: [PATCH 2/2] services: Add entry for pam_limits to
 mingetty-pam-service.

* gnu/services/base.scm (mingetty-pam-service): Add pam-entry for
  PAM module "pam_limits.so" to session field.
---
 gnu/services/base.scm | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

diff --git a/gnu/services/base.scm b/gnu/services/base.scm
index adafe1b..4243327 100644
--- a/gnu/services/base.scm
+++ b/gnu/services/base.scm
@@ -438,11 +438,17 @@ stopped before 'kill' is called."
   ;; Let 'login' be known to PAM.  All the mingetty services will have that
   ;; PAM service, but that's fine because they're all identical and duplicates
   ;; are removed.
-  (list (unix-pam-service "login"
-                          #:allow-empty-passwords?
-                          (mingetty-configuration-allow-empty-passwords? conf)
-                          #:motd
-                          (mingetty-configuration-motd conf))))
+  (let ((login (unix-pam-service "login"
+                                 #:allow-empty-passwords?
+                                 (mingetty-configuration-allow-empty-passwords? conf)
+                                 #:motd
+                                 (mingetty-configuration-motd conf))))
+    (list (pam-service
+           (inherit login)
+           (session (cons (pam-entry
+                           (control "required")
+                           (module "pam_limits.so"))
+                          (pam-service-session login)))))))
 
 (define mingetty-dmd-service
   (match-lambda
-- 
2.5.0

Re: [RFC]: Respect /etc/security/limits.conf

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>> Ricardo Wurmus <rekado@elephly.net> skribis:
>>
>>> The attached patch tries to add an entry for pam_limits.so, but I have
>>> no idea if this actually works or if this is the way it should be done.
>>> As far as I can tell we only need the pam_limits.so entry for
>>> “/etc/pam.d/login”, but I could not find where this file is generated.
>>
>> It is generated based on the ‘pam-services’ field of the service
>> returned by ‘mingetty-service’.
>>
>> Maybe it would be best to adjust just that part?
>
> Oh, right.  Attached are two patches:
>
>   * The first exports the pam-service-* getters, making it possible to
>     extend a pam-service.
>
>   * The second extends the “session” field of the mingetty-service to
>     add “pam_limits.so” to the required modules.
>
> Loading the module doesn’t yet do anything on GuixSD because we don’t
> generate ‘/etc/security/limits.conf’ (or ‘/etc/security/limits.d/’), but
> it should respect such file if it does exist.  (I have not yet tested
> this, but I will some time this week.)
>
> Does this look okay?

As long as lack of /etc/security/limits.conf doesn’t create any problems
or annoying warnings, that’s fine!

>> Is this PREFIX/etc/security/limits.d convention already used?  If not,
>> I’d rather avoid inventing it.  ;-)
>>
>> What we could do is add a field in ‘operating-system’ to specify the
>> limits.conf file to install as /etc/security/limits.conf?
>
> Yes, that’s a better idea.

One way to do that within the new service framework would be to have a
“limits” service that extends ‘etc-service-type’.  Something like that.

> From cdf974eb7595cfb8997111d09f6da2350c72afdd Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 12 Oct 2015 07:08:32 +0200
> Subject: [PATCH 1/2] system: Export pam-service accessors.
>
> * gnu/system/linux.scm (pam-service-name, pam-service-account,
>   pam-service-auth, pam-service-password, pam-service-session): Export.

Sure!

> From 0a1b5cad3d302d937a29dec95e805488a26b34e8 Mon Sep 17 00:00:00 2001
> From: Ricardo Wurmus <rekado@elephly.net>
> Date: Mon, 12 Oct 2015 07:11:51 +0200
> Subject: [PATCH 2/2] services: Add entry for pam_limits to
>  mingetty-pam-service.
>
> * gnu/services/base.scm (mingetty-pam-service): Add pam-entry for
>   PAM module "pam_limits.so" to session field.

[...]

> +           (session (cons (pam-entry
> +                           (control "required")
> +                           (module "pam_limits.so"))

Please add a one-line comment saying what this is about.

OK as long as it doesn’t break anything in the absence of limits.conf
and doesn’t trigger warnings.

Thanks!

Ludo’.

Re: [RFC]: Respect /etc/security/limits.conf

[Ricardo Wurmus]

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

>> Loading the module doesn’t yet do anything on GuixSD because we don’t
>> generate ‘/etc/security/limits.conf’ (or ‘/etc/security/limits.d/’), but
>> it should respect such file if it does exist.  (I have not yet tested
>> this, but I will some time this week.)
>>
>> Does this look okay?
>
> As long as lack of /etc/security/limits.conf doesn’t create any problems
> or annoying warnings, that’s fine!

So, I did test this and found a couple of issues:

* my patches need modification as ‘pam_limits.so’ looks for
  ‘limits.conf’ in the output directory of the linux-pam package, not in
  ‘/etc/security/’.  This can be changed by passing
  “conf=/etc/security/limits.conf” as an argument for the pam-entry.

* when ‘pam_limits.so’ is loaded by “login” and configured to look for
  ‘/etc/security/limits.conf’, logins fail with “Error in service
  module” when the file does not exist.

* changing the pam service for “login” is not enough as it only affects
  console logins.  When a user logs in via slim (or switches user
  accounts with ‘su’), limits are not respected.

I’ll update my patches to address the first point.  For the second point
we need to make sure to install ‘/etc/security/limits.conf’ (even if
it’s just empty).  The linux-pam package provides ‘$out/etc/security/*’
but nothing is deployed to ‘/etc’ when configuring the system.

To address the third point we could enhance the pam-services for ‘slim’
and ‘su’ in addition to ‘login’.

>>> Is this PREFIX/etc/security/limits.d convention already used?  If not,
>>> I’d rather avoid inventing it.  ;-)
>>>
>>> What we could do is add a field in ‘operating-system’ to specify the
>>> limits.conf file to install as /etc/security/limits.conf?
>>
>> Yes, that’s a better idea.
>
> One way to do that within the new service framework would be to have a
> “limits” service that extends ‘etc-service-type’.  Something like that.

I’ll try that and prepare an updated patch set.

~~ Ricardo

Re: [RFC]: Respect /etc/security/limits.conf

[Ludovic Courtès]

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:
>
>>> Loading the module doesn’t yet do anything on GuixSD because we don’t
>>> generate ‘/etc/security/limits.conf’ (or ‘/etc/security/limits.d/’), but
>>> it should respect such file if it does exist.  (I have not yet tested
>>> this, but I will some time this week.)
>>>
>>> Does this look okay?
>>
>> As long as lack of /etc/security/limits.conf doesn’t create any problems
>> or annoying warnings, that’s fine!
>
> So, I did test this and found a couple of issues:
>
> * my patches need modification as ‘pam_limits.so’ looks for
>   ‘limits.conf’ in the output directory of the linux-pam package, not in
>   ‘/etc/security/’.  This can be changed by passing
>   “conf=/etc/security/limits.conf” as an argument for the pam-entry.

We don’t even have to add it to /etc then; we could do
“conf=/gnu/store/…-limits.conf”, which is preferable IMO (it’s like
avoiding a global variable.)

> * when ‘pam_limits.so’ is loaded by “login” and configured to look for
>   ‘/etc/security/limits.conf’, logins fail with “Error in service
>   module” when the file does not exist.

So I guess we could create an empty(?) limits.conf file by default?

> * changing the pam service for “login” is not enough as it only affects
>   console logins.  When a user logs in via slim (or switches user
>   accounts with ‘su’), limits are not respected.
>
> I’ll update my patches to address the first point.  For the second point
> we need to make sure to install ‘/etc/security/limits.conf’ (even if
> it’s just empty).  The linux-pam package provides ‘$out/etc/security/*’
> but nothing is deployed to ‘/etc’ when configuring the system.
>
> To address the third point we could enhance the pam-services for ‘slim’
> and ‘su’ in addition to ‘login’.

Sounds like a good plan.

I guess changing the default values of the PAM entries to include
pam_limits.so is reasonable.  A similar pattern occurs with elogind
though, as Andy wrote some time ago.

So, looking forward, there’s the question of whether we should provide a
more flexible way to extend ‘pam-service-type’.  For instance, there
could be a ‘limits-service’ that extends ‘pam-service-type’ such that
all the contributed PAM entries are augmented with ‘pam_limits.so’;
likewise, ‘elogind-service’ would add ‘pam_elogind.so.’

One way to do that would be to extend ‘pam-service-type’ with a
procedure instead of a ‘pam-entry’; that procedure would then be mapped
over all the contributed PAM entries.

Thoughts?

Ludo’.