openssh unix dev - Feb 2025 - [PATCH] auth-pam: Check the user didn't change during PAM transaction

From: Marco Trevisan (Trevi?o) <mail at 3v1n0.net>

PAM modules can change the user during their execution, in such case ssh
would still use the user that has been provided giving potentially
access to another user with the credentials of another one.

So prevent this to happen, by ensuring that the final PAM user is
matching the one that initiated the transaction.

See also: https://github.com/util-linux/util-linux/pull/3206
---
 auth-pam.c | 35 ++++++++++++++++++++++++++++++-----
 1 file changed, 30 insertions(+), 5 deletions(-)

diff --git a/auth-pam.c b/auth-pam.c
index 13c0a792e..f45e61675 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -467,6 +467,28 @@ sshpam_thread_conv(int n, sshpam_const struct pam_message
**msg,
 	return (PAM_CONV_ERR);
 }
 
+static int
+check_pam_user(Authctxt *authctxt)
+{
+	const char *pam_user;
+
+	if (authctxt == NULL || authctxt->user == NULL)
+		fatal("%s: PAM authctxt user not initialized", __func__);
+
+	sshpam_err = pam_get_item(sshpam_handle,
+				  PAM_USER, (sshpam_const void **) &pam_user);
+	if (sshpam_err != PAM_SUCCESS)
+		return sshpam_err;
+
+	if (pam_user == NULL || strcmp(authctxt->user, pam_user) != 0) {
+		debug("PAM: User '%s' does not match expected
'%s'",
+		      pam_user, authctxt->user);
+		return PAM_USER_UNKNOWN;
+	}
+
+	return PAM_SUCCESS;
+}
+
 /*
  * Authentication thread.
  */
@@ -521,6 +543,9 @@ sshpam_thread(void *ctxtp)
 		sshpam_set_maxtries_reached(1);
 	if (sshpam_err != PAM_SUCCESS)
 		goto auth_fail;
+	sshpam_err = check_pam_user(sshpam_authctxt);
+	if (sshpam_err != PAM_SUCCESS)
+		goto auth_fail;
 
 	if (!do_pam_account()) {
 		sshpam_err = PAM_ACCT_EXPIRED;
@@ -686,8 +711,7 @@ sshpam_cleanup(void)
 static int
 sshpam_init(struct ssh *ssh, Authctxt *authctxt)
 {
-	const char *pam_user, *user = authctxt->user;
-	const char **ptr_pam_user = &pam_user;
+	const char *user = authctxt->user;
 	int r;
 
 	if (options.pam_service_name == NULL)
@@ -706,9 +730,8 @@ sshpam_init(struct ssh *ssh, Authctxt *authctxt)
 	}
 	if (sshpam_handle != NULL) {
 		/* We already have a PAM context; check if the user matches */
-		sshpam_err = pam_get_item(sshpam_handle,
-		    PAM_USER, (sshpam_const void **)ptr_pam_user);
-		if (sshpam_err == PAM_SUCCESS && strcmp(user, pam_user) == 0)
+		sshpam_err = check_pam_user(authctxt);
+		if (sshpam_err == PAM_SUCCESS)
 			return (0);
 		pam_end(sshpam_handle, sshpam_err);
 		sshpam_handle = NULL;
@@ -1378,6 +1401,8 @@ sshpam_auth_passwd(Authctxt *authctxt, const char
*password)
 	sshpam_err = pam_authenticate(sshpam_handle, flags);
 	sshpam_password = NULL;
 	free(fake);
+	if (sshpam_err == PAM_SUCCESS)
+		sshpam_err = check_pam_user(authctxt);
 	if (sshpam_err == PAM_MAXTRIES)
 		sshpam_set_maxtries_reached(1);
 	if (sshpam_err == PAM_SUCCESS && authctxt->valid) {
-- 
2.34.1

On Thu, Feb 13, 2025 at 05:45:47PM +0100, Marco Trevisan
wrote:
> From: Marco Trevisan (Trevi?o) <mail at 3v1n0.net>
> 
> PAM modules can change the user during their execution, in such case ssh
> would still use the user that has been provided giving potentially
> access to another user with the credentials of another one.
> 
> So prevent this to happen, by ensuring that the final PAM user is
> matching the one that initiated the transaction.
> 
> See also: https://github.com/util-linux/util-linux/pull/3206
Note that linux-pam provides a module called pam_canonicalize_user with
the following description:

    This PAM module uses the name of the user obtained via pam_get_user(3)
    as a key to query the password database, and replaces PAM_USER with
    the pw_name value that has been returned.
>From this perspective, a blanket ban on the user name change would be
too restrictive.


-- 
ldv