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

[Ricardo Wurmus <rekado@elephly.net>]

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