openssh unix dev - May 2007 - [RFC][PATCH] Detect and handle PAM changing user name

I've implemented a patch to openssh which allows the PAM auth layer
to detect if the PAM stack has changed the user name and then adjusts
its internal data structures accordingly.  (imagine a PAM stack that
uses individual credentials to authenticate, but assigns the user to
a role account).

First, is the openssh community interested in this patch?
Second, if there is interest in the patch, how do I go about
submitting the patch for formal review?
Third, regardless of interest by the openssh community, is there
anyone willing to review this code for me?

PS I've tested the code path going through sshpam_auth_passwd(),
but do know how to test the code path that goes through sshpam_thread().

<patch against 4.6p1 (portable) attached>
-- 
James R. Leu
jleu at inoc.com
INOC -> http://inoc.com/
DELIVERING UPTIME
-------------- next part --------------
diff -uNr openssh-4.6p1/auth-pam.c openssh-4.6p1.jleu/auth-pam.c
--- openssh-4.6p1/auth-pam.c	2006-09-16 20:57:47.000000000 -0500
+++ openssh-4.6p1.jleu/auth-pam.c	2007-05-24 13:16:56.000000000 -0500
@@ -335,6 +335,39 @@
 }
 
 /*
+ * Detect and deal with the PAM stack changing the user name on us
+ */
+static int
+sshpam_handle_user_change(pam_handle_t *sshpam_handle, Authctxt *authctxt)
+{
+	const char *pam_user;
+	const char **ptr_pam_user = &pam_user;
+
+	if (pam_get_item(sshpam_handle, PAM_USER,
+	    (sshpam_const void **)ptr_pam_user) != PAM_SUCCESS)
+		return PAM_AUTH_ERR;
+
+	if (strcmp(authctxt->user, pam_user)) {
+		char *user = strdup(pam_user);
+		struct passwd *pw;
+
+		if (!user)
+			return PAM_AUTH_ERR;
+		
+		if (!(pw = getpwnamallow(user))) {
+			free(user);
+			return PAM_AUTH_ERR;
+		}
+
+		free(authctxt->pw);
+		authctxt->pw = pw;
+		free(authctxt->user);
+		authctxt->user = user;
+	}
+	return PAM_SUCCESS;
+}
+
+/*
  * Conversation function for authentication thread.
  */
 static int
@@ -469,6 +502,10 @@
 	if (sshpam_err != PAM_SUCCESS)
 		goto auth_fail;
 
+	sshpam_err = sshpam_handle_user_change(sshpam_handle, sshpam_authctxt);
+	if (sshpam_err != PAM_SUCCESS)
+		goto auth_fail;
+
 	if (compat20) {
 		if (!do_pam_account()) {
 			sshpam_err = PAM_ACCT_EXPIRED;
@@ -1206,7 +1243,8 @@
 
 	sshpam_err = pam_authenticate(sshpam_handle, flags);
 	sshpam_password = NULL;
-	if (sshpam_err == PAM_SUCCESS && authctxt->valid) {
+	if (sshpam_err == PAM_SUCCESS && authctxt->valid &&
+	    sshpam_handle_user_change(sshpam_handle, authctxt) == PAM_SUCCESS) {
 		debug("PAM: password authentication accepted for %.100s",
 		    authctxt->user);
 		return 1;
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
Url :
http://lists.mindrot.org/pipermail/openssh-unix-dev/attachments/20070524/48f14c83/attachment-0001.bin

On Thu, May 24, 2007 at 02:18:33PM -0500, James R. Leu wrote:
> +		free(authctxt->pw);
> +		authctxt->pw = pw;
I haven't looked at 4.6 specifically, but in the version I did
have a look at the pw * returned by pwcopy() has a bunch of
fields that are xstrdup'd, so you'd probably want to implement
a corresponding pwfree() function.

	-Ed

James R. Leu wrote:
> I've implemented a patch to openssh which allows the PAM auth layer
> to detect if the PAM stack has changed the user name and then adjusts
> its internal data structures accordingly.  (imagine a PAM stack that
> uses individual credentials to authenticate, but assigns the user to
> a role account).
> 
> First, is the openssh community interested in this patch?
Maybe.  I'm not convinced it's the right thing to do, though.
> Second, if there is interest in the patch, how do I go about
> submitting the patch for formal review?
Attach it to http://bugzilla.mindrot.org/show_bug.cgi?id=1215, but from
a brief look it appears your patch is a subset of the one already there
(which also handles the case where the user doesn't exist on the system,
normally this would get the login marked as invalid).
> Third, regardless of interest by the openssh community, is there
> anyone willing to review this code for me?
> 
> PS I've tested the code path going through sshpam_auth_passwd(),
> but do know how to test the code path that goes through sshpam_thread().
Use ChallengeResponseAuthentication.

-- 
Darren Tucker (dtucker at zip.com.au)
GPG key 8FF4FA69 / D9A3 86E9 7EEE AF4B B2D4  37C9 C982 80C7 8FF4 FA69
    Good judgement comes with experience. Unfortunately, the experience
usually comes from bad judgement.