Ricardo Wurmus <[email protected]> skribis: > Ludovic Courtès <[email protected]> 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’.
