[Pam-developers] [PATCH] pam_loginuid: return PAM_IGNORE when /proc/self/loginuid does not exist

Dmitry V. Levin ldv at altlinux.org
Thu Jan 9 01:41:02 UTC 2014


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:

--- 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/pam-developers/attachments/20140109/24519119/attachment.sig>


More information about the Pam-developers mailing list