Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
Hello again Petter and sorry for the delay, I think you've misinterpretted me completely. Probably I wasn't very clear. I'll try to comment below and hopefully you can go back and reread my previous mail and then understand it in a new way. Thanks alot for your comments on this by the way. On Mon, Aug 13, 2018 at 11:32:29PM +0200, Petter Reinholdtsen wrote: > [Andreas Henriksson] > > (Please note, I've only looked quickly but it seems like > > USERGROUPS_ENAB option is only used by useradd/userdel and not any > > other tool like su or login implementations in src:shadow. Given we > > tend to use adduser rather than the lower level useradd/userdel tools > > in debian, I'm not sure how relevant it is at all to mix up pam_umask > > usergroups with USERGROUPS_ENAB.) > > I do not understand how USERGROUPS_ENAB would be relevant for su or > login. Care to explain? It isn't, except Ubuntu patches makes it so. I don't think it should be related (so basically I don't agree with the ubuntu way). > The way I understand it, it would only be > relevant for the mechanism creating home directories and users, and the > mechanism setting umask during login (aka PAM). Not sure I understand what you're saying here, but ubuntu hooks up USERGROUPS_ENAB by patching pam_umask and pam_unix to that it gets 'usergroups' functionality (without using the argument, which they've deprecated) if USERGROUPS_ENAB is yes in /etc/login.defs (as it is by default). Without ubunt patches (eg. Debian or plain upstream), the USERGROUPS_ENAB is only relevant for some minor functionatily in useradd and userdel. It's a pre-pam and src:shadow-only setting. > > > Given a decade has passed without this being handled in Debian > > (despite our PAM usage for as long) and we're now moving away from > > src:shadow implementations, I don't think it makes sense to patch > > things to read USERGROUPS_ENAB option which isn't supported anywhere > > in eg. util-linux implementations which also reads > > /etc/login.defs. I'd suggest we instead deprecate the USERGROUPS_ENAB > > option in /etc/login.defs. > > I did not quite understand this rationale. The fact that the default > Debian setup has been less than useful for a decade is no reason not to > fix it now. :) Sorry for confusion. What I meant was that there's no point in making the solution more complex and by that delaying it. I think we should just use the pam usergroups argument as is and get this fixed ASAP. That the legacy USERGROUPS_ENAB has been broken for a decade to me means that noone cares about that legacy setting. We should just get rid of it, instead of trying to implement it in *everything* that's not src:shadow but also uses /etc/login.defs (eg. pam, util-linux, ...). > > > JFTR, If common-session gets this setting then su would also given it > > includes common-session. > > Good point. Wasn't sure this was even worth mentioning, but apparently it was. All my comments is in the mindset that it's pam (common-session) that needs fixing, not util-linux (su). > > > Setting the pam bug as a blocker for now, but likely this bug report > > should just be reassigned, (force)merged and set as affects util-linux, > > et.al. > > To me it seem more sensible to submit the patch to > https://github.com/linux-pam/linux-pam/ > and try to get it into > upstream as soon as possible. Fixing this via usergroups needs no patching. The alternative would be to go the ubuntu way, but since I don't think that's a good way personally then I'm not going to submit patches I don't even support myself. I can only provide arguments against those patches (and potentially a little bit of sympathy for legacy, but that doesn't go a long way). > > > Question remains though how we get some movement on the pam side, should > > we just NMU it? Do most people agree we should just use 'usergroups' > > rather than go the ubuntu way of USERGROUPS_ENAB setting? > > A useful usecase to consider is a site with a LDAP directory with > thousands of users, and home directories on a central server, using some > configuration management system to control a large set of computers. In > such setting, I suspect it will be easier to change the USERGROUPS_ENAB > setting in /etc/login.defs than to modify the content of > /usr/share/pam-configs/ by providing a replacement debian package to > override the default pam.d configuration. This make me suspect the > current ubuntu way is better than the 'usergroups' approach. I suggest > to ask Steve about his view on this, as he know PAM a lot better than > me. If you have experience of big LDAP/kerberos deployments I'd be very happy to hear about it in other util-linux bug reports, like #833256, where we discuss potentially switching to util-linux implementations of certain tools. The main reason to do so (except using the same implementation as everyone else and thus benefiting from their fixing and maintenance work) would seem to be better support for LDAP and kerberos
Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
[Andreas Henriksson] > (Please note, I've only looked quickly but it seems like > USERGROUPS_ENAB option is only used by useradd/userdel and not any > other tool like su or login implementations in src:shadow. Given we > tend to use adduser rather than the lower level useradd/userdel tools > in debian, I'm not sure how relevant it is at all to mix up pam_umask > usergroups with USERGROUPS_ENAB.) I do not understand how USERGROUPS_ENAB would be relevant for su or login. Care to explain? The way I understand it, it would only be relevant for the mechanism creating home directories and users, and the mechanism setting umask during login (aka PAM). > Given a decade has passed without this being handled in Debian > (despite our PAM usage for as long) and we're now moving away from > src:shadow implementations, I don't think it makes sense to patch > things to read USERGROUPS_ENAB option which isn't supported anywhere > in eg. util-linux implementations which also reads > /etc/login.defs. I'd suggest we instead deprecate the USERGROUPS_ENAB > option in /etc/login.defs. I did not quite understand this rationale. The fact that the default Debian setup has been less than useful for a decade is no reason not to fix it now. :) > JFTR, If common-session gets this setting then su would also given it > includes common-session. Good point. > Setting the pam bug as a blocker for now, but likely this bug report > should just be reassigned, (force)merged and set as affects util-linux, > et.al. To me it seem more sensible to submit the patch to https://github.com/linux-pam/linux-pam/ > and try to get it into upstream as soon as possible. > Question remains though how we get some movement on the pam side, should > we just NMU it? Do most people agree we should just use 'usergroups' > rather than go the ubuntu way of USERGROUPS_ENAB setting? A useful usecase to consider is a site with a LDAP directory with thousands of users, and home directories on a central server, using some configuration management system to control a large set of computers. In such setting, I suspect it will be easier to change the USERGROUPS_ENAB setting in /etc/login.defs than to modify the content of /usr/share/pam-configs/ by providing a replacement debian package to override the default pam.d configuration. This make me suspect the current ubuntu way is better than the 'usergroups' approach. I suggest to ask Steve about his view on this, as he know PAM a lot better than me. Cc to Steve and Martin, hoping they can provide useful input. -- Happy hacking Petter Reinholdtsen
Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
Control: block -1 by 583958 Hello Petter Reinholdtsen, Thanks for your input on this. On Mon, Aug 13, 2018 at 07:57:06PM +0200, Petter Reinholdtsen wrote: [...] > optionalpam_umask.so umask=002 > > Perhaps the default setup should have a similar line? I see from the > pam_umask manual page a new 'usergroups' option is now available. [...] I got inspired and looked around and found these interesting things related to pam_umask and usergroups: https://bugs.launchpad.net/ubuntu/+source/pam/+bug/253096 https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=583958 Apparently ubuntu patches things in pam to basically use "pam_umask.so usergroups" except they deprecate usergroups in favour of reading the pre-pam (src:shadow only?) USERGROUPS_ENAB option in /etc/login.defs (which ships as set to yes). The Ubuntu bits has ofcourse also never made it into Debian. (Please note, I've only looked quickly but it seems like USERGROUPS_ENAB option is only used by useradd/userdel and not any other tool like su or login implementations in src:shadow. Given we tend to use adduser rather than the lower level useradd/userdel tools in debian, I'm not sure how relevant it is at all to mix up pam_umask usergroups with USERGROUPS_ENAB.) Given a decade has passed without this being handled in Debian (despite our PAM usage for as long) and we're now moving away from src:shadow implementations, I don't think it makes sense to patch things to read USERGROUPS_ENAB option which isn't supported anywhere in eg. util-linux implementations which also reads /etc/login.defs. I'd suggest we instead deprecate the USERGROUPS_ENAB option in /etc/login.defs. JFTR, If common-session gets this setting then su would also given it includes common-session. Setting the pam bug as a blocker for now, but likely this bug report should just be reassigned, (force)merged and set as affects util-linux, et.al. Question remains though how we get some movement on the pam side, should we just NMU it? Do most people agree we should just use 'usergroups' rather than go the ubuntu way of USERGROUPS_ENAB setting? Regards, Andreas Henriksson
Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
I'm not a PAM expert, but can perhaps provide an interesting data point. In Debian Edu, we provide the following /usr/share/pam-configs/edu-umask to ensure the umask is set to 002: Name: umask set at login (Debian Edu version) Default: yes Priority: 0 Session-Type: Additional Session: optionalpam_umask.so umask=002 Perhaps the default setup should have a similar line? I see from the pam_umask manual page a new 'usergroups' option is now available. As far as I remember, it was not available when I created the edu-umask pam-config. It seem to provide the setup wanted by Debian Edu, so perhaps Debian Edu should switch to pam_umask.so usergroups? CC to the debian-edu@ list to make everyone there aware of the option. -- Happy hacking Petter Reinholdtsen
Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
Control: tags -1 + help On Wed, Jun 05, 2013 at 07:00:29AM +0200, Christian PERRIER wrote: > Quoting Piotr Engelking (inkerma...@gmail.com): [...] > > The 'su -' command, unlike login, doesn't set umask. This behavior > > disagrees with the man page, which says: > > > > The optional argument - may be used to provide an environment similar > > to what the user would expect had the user logged in directly. > > > > Operating with an unexpected umask value is dangerous, particularly so > > if running as root. > > > > Please change su - to set umask to the same value that login does. > > > Without checking, though, I suspect this to be a PAM issue. Should /etc/pam.d/su-l gain a line for pam_umask ? Possibly also pam_limits ? Maybe they should even be in /etc/pam.d/su (which is also included by /et/pam.d/su-l) Help from pam experts would be appreciated. (Maybe this is looking at it too narrowly though, and instead the entire /etc/pam.d/su file carried over from src:shadow/login days should be revamped/rewritten.) Regards, Andreas Henriksson
Bug#711104: [Pkg-shadow-devel] Bug#711104: login: su - doesn't set umask
Quoting Piotr Engelking (inkerma...@gmail.com): Package: login Version: 1:4.1.5.1-1 Severity: important The 'su -' command, unlike login, doesn't set umask. This behavior disagrees with the man page, which says: The optional argument - may be used to provide an environment similar to what the user would expect had the user logged in directly. Operating with an unexpected umask value is dangerous, particularly so if running as root. Please change su - to set umask to the same value that login does. Without checking, though, I suspect this to be a PAM issue. signature.asc Description: Digital signature
