[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [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 
> <URL: 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 (because util-linux tools are built on top of libuser).
Replacing large parts of the 'login' package (src:shadow) with
util-linux implementations are next on the agenda. Investigations
has also started for the small overlap of 'passwd' package binaries
with util-linux which is limited to chsh and chfn. Given the hassle
and very partial replacement I'm considering ignoring these tools,
leaving it up to src:shadow to continue providing everything in the
'passwd' package (including chsh and chfn). Do you have any comments
about that? Would LDAP/kerberos support be useful in those tools or
how does it work today using src:shadow implementations of them?

> 
> Cc to Steve and Martin, hoping they can provide useful input.

FWIW, I happened to stumble across this which seemed to be a big hurdle:

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=583976#10

"pam_umask requires both username == primary group name and uid == gid"

Debian still has no guarantee that uid==gid for neither system nor
regular user accounts.

However, that pam's usergroups functionality requires uid == gid
doesn't seem to be true as far as I can tell after a quick look
at the source:

https://sources.debian.org/src/pam/1.1.8-3.8/modules/pam_umask/pam_umask.c/#L202

As I read it the conditions are:
- usergroups argument supplied to enable the feature
- not uid 0
- username == groupname

Thus there doesn't seem to be any big issue here like I first worried
about. Maybe this worked differently in the past when the comment to
the bug (linked above) was written. If so, that might have been the
reason you (debian-edu) didn't use usergroups at the time.

Regards,
Andreas Henriksson


Reply to: