[Pam-developers] [PATCH] pam_loginuid: Ignore failure in user namespaces
Stéphane Graber
stgraber at ubuntu.com
Tue Jan 7 23:45:54 UTC 2014
On Tue, Jan 07, 2014 at 02:12:57PM -0800, Steve Langasek wrote:
> Hi Stéphane,
>
> On Fri, Dec 06, 2013 at 07:47:34PM -0500, Stéphane Graber wrote:
> > When running pam_loginuid in a container using the user namespaces, even
> > uid 0 isn't allowed to set the loginuid property.
>
> > This change catches the EACCES from opening loginuid, checks if the user
> > is in the host namespace (by comparing the uid_map with the host's one)
> > and only if that's the case, sets rc to 1.
>
> > Should uid_map not exist to be unreadable for some reason, it'll be
> > assumed that the process is running on the host's namespace.
>
> > The initial reason behind this change was failure to ssh into an
> > unprivileged container (using a 3.13 kernel and current LXC) when using
> > a standard pam profile for sshd (which requires success from
> > pam_loginuid).
>
> > I believe this solution doesn't have any drawback and will allow people
> > to use unprivileged containers normally. An alternative would be to have
> > all distros set pam_loginuid as optional but that'd be bad for any of
> > the other potential failure case which people may care about.
>
> This change looks generally correct to me; since it detects the specific
> case where we're EACCES on /proc/self/loginuid due to userns, and treats
> this one case as a non-error, it's safer than making the admin change the
> PAM config to ignore all errors from the module.
>
> Upon review, however, I do have one concern with the patch, which is that
> the module will return PAM_SUCCESS for this case. I think it would be
> preferable to return PAM_IGNORE, so that we can distinguish between
> "loginuid set successfully" and "loginuid not set, but this is expected".
>
> In that case, the ENOENT case should also be handled as PAM_IGNORE rather
> than PAM_SUCCESS.
Fair enough, I'll send a v2 of the patch with this change.
>
> --
> Steve Langasek Give me a lever long enough and a Free OS
> Debian Developer to set it on, and I can move the world.
> Ubuntu Developer http://www.debian.org/
> slangasek at ubuntu.com vorlon at debian.org
>
> > There has also been some discussions to get some of the audit features
> > tied with the user namespaces but currently none of that has been merged
> > upstream and the currently proposed implementation doesn't cover
> > loginuid (nor is it clear how this should even work when loginuid is set
> > as immutable after initial write).
>
> > ---
> > modules/pam_loginuid/pam_loginuid.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
> > index a903845..977cbea 100644
> > --- a/modules/pam_loginuid/pam_loginuid.c
> > +++ b/modules/pam_loginuid/pam_loginuid.c
> > @@ -53,11 +53,26 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
> > {
> > int fd, count, rc = 0;
> > char loginuid[24], buf[24];
> > + char uid_map[256];
> >
> > count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
> > fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
> > if (fd < 0) {
> > - if (errno != ENOENT) {
> > + if (errno == EACCES) {
> > + rc = 1;
> > + fd = open("/proc/self/uid_map", O_RDONLY);
> > + if (fd >= 0) {
> > + count = pam_modutil_read(fd, uid_map, sizeof(uid_map));
> > + if (strncmp(uid_map, " 0 0 4294967295\n",
> > + count) != 0)
> > + rc = 0;
> > + close(fd);
> > + }
> > + if (rc == 1)
> > + pam_syslog(pamh, LOG_ERR,
> > + "Cannot open /proc/self/loginuid: Permission denied");
> > + }
> > + else if (errno != ENOENT) {
> > rc = 1;
> > pam_syslog(pamh, LOG_ERR,
> > "Cannot open /proc/self/loginuid: %m");
> > --
> > 1.8.5.1
> >
> > _______________________________________________
> > Pam-developers mailing list
> > Pam-developers at lists.fedorahosted.org
> > https://lists.fedorahosted.org/mailman/listinfo/pam-developers
--
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 836 bytes
Desc: Digital signature
URL: <https://lists.fedorahosted.org/pipermail/pam-developers/attachments/20140107/74da2633/attachment.sig>
More information about the Pam-developers
mailing list