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

Steve Langasek vorlon at debian.org
Thu Jan 9 00:09:39 UTC 2014


Hi Dmitry,

On Wed, Jan 08, 2014 at 05:16:32AM +0400, Dmitry V. Levin wrote:
> When /proc/self/loginuid does not exist, return PAM_IGNORE instead of
> PAM_SUCCESS, so that we can distinguish between "loginuid set
> successfully" and "loginuid not set, but this is expected".

> Suggested by Steve Langasek.

> * 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?

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.

So, +1 from me - and attached is the rebased for userns support.

Thanks,
-- 
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


> ---
>  modules/pam_loginuid/pam_loginuid.c | 43 ++++++++++++++++++++++++-------------
>  1 file changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/modules/pam_loginuid/pam_loginuid.c b/modules/pam_loginuid/pam_loginuid.c
> index a903845..96f8ffa 100644
> --- a/modules/pam_loginuid/pam_loginuid.c
> +++ b/modules/pam_loginuid/pam_loginuid.c
> @@ -47,29 +47,35 @@
>  
>  /*
>   * This function writes the loginuid to the /proc system. It returns
> - * 0 on success and 1 on failure.
> + * PAM_SUCCESS on success,
> + * PAM_IGNORE when /proc/self/loginuid does not exist,
> + * PAM_SESSION_ERR in case of any other error.
>   */
>  static int set_loginuid(pam_handle_t *pamh, uid_t uid)
>  {
> -	int fd, count, rc = 0;
> +	int fd, count, rc = PAM_SESSION_ERR;
>  	char loginuid[24], buf[24];
>  
>  	count = snprintf(loginuid, sizeof(loginuid), "%lu", (unsigned long)uid);
>  	fd = open("/proc/self/loginuid", O_NOFOLLOW|O_RDWR);
>  	if (fd < 0) {
> -		if (errno != ENOENT) {
> -			rc = 1;
> +		if (errno == ENOENT) {
> +			rc = PAM_IGNORE;
> +		} else {
>  			pam_syslog(pamh, LOG_ERR,
>  				   "Cannot open /proc/self/loginuid: %m");
>  		}
>  		return rc;
>  	}
> +
>  	if (pam_modutil_read(fd, buf, sizeof(buf)) == count &&
> -	    memcmp(buf, loginuid, count) == 0)
> +	    memcmp(buf, loginuid, count) == 0) {
> +		rc = PAM_SUCCESS;
>  		goto done;	/* already correct */
> -	if (lseek(fd, 0, SEEK_SET) == -1 || (ftruncate(fd, 0) == -1 ||
> -	    pam_modutil_write(fd, loginuid, count) != count))
> -		rc = 1;
> +	}
> +	if (lseek(fd, 0, SEEK_SET) == 0 && ftruncate(fd, 0) == 0 &&
> +	    pam_modutil_write(fd, loginuid, count) == count)
> +		rc = PAM_SUCCESS;
>   done:
>  	close(fd);
>  	return rc;
> @@ -170,6 +176,7 @@ _pam_loginuid(pam_handle_t *pamh, int flags UNUSED,
>  {
>          const char *user = NULL;
>  	struct passwd *pwd;
> +	int ret;
>  #ifdef HAVE_LIBAUDIT
>  	int require_auditd = 0;
>  #endif
> @@ -188,9 +195,14 @@ _pam_loginuid(pam_handle_t *pamh, int flags UNUSED,
>  		return PAM_SESSION_ERR;
>  	}
>  
> -	if (set_loginuid(pamh, pwd->pw_uid)) {
> -		pam_syslog(pamh, LOG_ERR, "set_loginuid failed\n");
> -		return PAM_SESSION_ERR;
> +	ret = set_loginuid(pamh, pwd->pw_uid);
> +	switch (ret) {
> +		case PAM_SUCCESS:
> +		case PAM_IGNORE:
> +			break;
> +		default:
> +			pam_syslog(pamh, LOG_ERR, "set_loginuid failed");
> +			return ret;
>  	}
>  
>  #ifdef HAVE_LIBAUDIT
> @@ -200,11 +212,12 @@ _pam_loginuid(pam_handle_t *pamh, int flags UNUSED,
>  		argv++;
>  	}
>  
> -	if (require_auditd)
> -		return check_auditd();
> -	else
> +	if (require_auditd) {
> +		int rc = check_auditd();
> +		return rc != PAM_SUCCESS ? rc : ret;
> +	} else
>  #endif
> -		return PAM_SUCCESS;
> +		return ret;
>  }
>  
>  /*
> 
> -- 
> ldv
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-pam_loginuid-Ignore-failure-in-user-namespaces.patch
Type: text/x-diff
Size: 2717 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/pam-developers/attachments/20140108/f0854e12/attachment.bin>
-------------- 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/f0854e12/attachment.sig>


More information about the Pam-developers mailing list