[Pam-developers] [PATCH] pam_loginuid: return PAM_IGNORE when /proc/self/loginuid does not exist
Stéphane Graber
stgraber at ubuntu.com
Thu Jan 9 02:15:22 UTC 2014
On Thu, Jan 09, 2014 at 05:41:02AM +0400, Dmitry V. Levin wrote:
> Hi Steve,
>
> On Wed, Jan 08, 2014 at 04:09:39PM -0800, Steve Langasek wrote:
> > On Wed, Jan 08, 2014 at 05:16:32AM +0400, Dmitry V. Levin wrote:
> [...]
> > > * modules/pam_loginuid/pam_loginuid.c (set_loginuid): Change return
> > > code semantics: return PAM_SUCCESS on success, PAM_IGNORE when loginuid
> > > does not exist, PAM_SESSION_ERR in case of any other error.
> > > (_pam_loginuid): Forward the PAM error code returned by set_loginuid.
> >
> > Thanks, looks good. The only thing I have a question on is whether it makes
> > sense to still call require_auditd() in any cases that setting the loginuid
> > failed. Maybe this should be implicitly ignored?
>
> It doesn't look reasonable to me either, but there might be some less
> obvious use case for require_auditd option I'm not aware of.
>
> > Anyway, the current behavior fails closed, so that should be fine for a
> > first pass - and if somebody is concerned about setting "require_auditd" in
> > a pam config that's being used in an unprivileged container, we can always
> > revisit later.
>
> Agreed.
>
> > So, +1 from me - and attached is the rebased for userns support.
>
> I'm not quite familiar with uid_map semantics yet;
> can we assume that host's uid_map will remain the same?
> If yes, then the change is OK, although I'd tweak it a bit:
Yes.
uid_map is basically:
- <start of range in host ns>
- <start of range in child ns>
- <number of entries>
On the host, there's no map, so both range start at 0 and the number of
entries is the maximum value of uid_t.
So as long as the definition of uid_t doesn't change and the kernel
stays backward compatible, the host uid_map will remain identical to
that I've put in the patch.
>
> --- a/modules/pam_loginuid/pam_loginuid.c
> +++ b/modules/pam_loginuid/pam_loginuid.c
> @@ -55,7 +55,8 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
> {
> int fd, count, rc = PAM_SESSION_ERR;
> char loginuid[24], buf[24];
> - char uid_map[256];
> + static const char host_uid_map[] = " 0 0 4294967295\n";
> + char uid_map[sizeof(host_uid_map)];
>
> count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
> fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
> @@ -66,15 +67,14 @@ static int set_loginuid(pam_handle_t *pamh, uid_t uid)
> 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)
> + if (strncmp(uid_map, host_uid_map, count) != 0)
> rc = PAM_IGNORE;
> close(fd);
> }
> if (rc != PAM_IGNORE)
> - pam_syslog(pamh, LOG_ERR,
> - "Cannot open /proc/self/loginuid: Permission denied");
> - } else {
> + errno = EACCES;
> + }
> + if (rc != PAM_IGNORE) {
> pam_syslog(pamh, LOG_ERR,
> "Cannot open /proc/self/loginuid: %m");
> }
>
> --
> ldv
--
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/20140108/4d6ebe11/attachment.sig>
More information about the Pam-developers
mailing list