File 0001-auth-pam-Immediately-report-instructions-t... of Package openssh

Overview Repositories Revisions Requests Users Attributes Meta

File 0001-auth-pam-Immediately-report-instructions-to-clients-and-fix-handling-in-ssh-client.patch of Package openssh (Revision 5a12e2847c1bdb136b89b0385bc8f2bc)

Currently displaying revision 5a12e2847c1bdb136b89b0385bc8f2bc , Show latest

From 7c116ef927a8ef14d09065757f75560fa0ab79d0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 17 Oct 2023 04:04:13 +0200
Subject: [PATCH 1/6] auth: Add KbdintResult definition to define result values
 explicitly

kbdint result vfunc may return various values, so use an enum to make it
clearer what each result means without having to dig into the struct
documentation.
---
 auth-bsdauth.c |  2 +-
 auth-pam.c     | 10 +++++-----
 auth.h         |  5 +++++
 auth2-chall.c  |  4 ++--
 4 files changed, 13 insertions(+), 8 deletions(-)

diff --git a/auth-bsdauth.c b/auth-bsdauth.c
index d124e994e77..ca41735debb 100644
--- a/auth-bsdauth.c
+++ b/auth-bsdauth.c
@@ -111,7 +111,7 @@ bsdauth_respond(void *ctx, u_int numresponses, char **responses)
 	authctxt->as = NULL;
 	debug3("bsdauth_respond: <%s> = <%d>", responses[0], authok);
 
-	return (authok == 0) ? -1 : 0;
+	return (authok == 0) ? KbdintResultFailure : KbdintResultSuccess;
 }
 
 static void
diff --git a/auth-pam.c b/auth-pam.c
index b49d415e7c7..86137a1acdb 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -990,15 +990,15 @@ sshpam_respond(void *ctx, u_int num, char **resp)
 	switch (ctxt->pam_done) {
 	case 1:
 		sshpam_authenticated = 1;
-		return (0);
+		return KbdintResultSuccess;
 	case 0:
 		break;
 	default:
-		return (-1);
+		return KbdintResultFailure;
 	}
 	if (num != 1) {
 		error("PAM: expected one response, got %u", num);
-		return (-1);
+		return KbdintResultFailure;
 	}
 	if ((buffer = sshbuf_new()) == NULL)
 		fatal("%s: sshbuf_new failed", __func__);
@@ -1015,10 +1015,10 @@ sshpam_respond(void *ctx, u_int num, char **resp)
 	}
 	if (ssh_msg_send(ctxt->pam_psock, PAM_AUTHTOK, buffer) == -1) {
 		sshbuf_free(buffer);
-		return (-1);
+		return KbdintResultFailure;
 	}
 	sshbuf_free(buffer);
-	return (1);
+	return KbdintResultAgain;
 }
 
 static void
diff --git a/auth.h b/auth.h
index 6d2d3976234..aac1e92d9cd 100644
--- a/auth.h
+++ b/auth.h
@@ -51,6 +51,7 @@ struct sshauthopt;
 typedef struct Authctxt Authctxt;
 typedef struct Authmethod Authmethod;
 typedef struct KbdintDevice KbdintDevice;
+typedef int KbdintResult;
 
 struct Authctxt {
 	sig_atomic_t	 success;
@@ -111,6 +112,10 @@ struct Authmethod {
# 	int	*enabled;
 	int	(*userauth)(struct ssh *, const char *);
 };
 
+#define KbdintResultFailure -1
+#define KbdintResultSuccess 0
+#define KbdintResultAgain 1
+
 /*
  * Keyboard interactive device:
  * init_ctx	returns: non NULL upon success
diff --git a/auth2-chall.c b/auth2-chall.c
index 021df829173..047d4e83c33 100644
--- a/auth2-chall.c
+++ b/auth2-chall.c
@@ -331,11 +331,11 @@ input_userauth_info_response(int type, u_int32_t seq, struct ssh *ssh)
 	free(response);
 
 	switch (res) {
-	case 0:
+	case KbdintResultSuccess:
 		/* Success! */
 		authenticated = authctxt->valid ? 1 : 0;
 		break;
-	case 1:
+	case KbdintResultAgain:
 		/* Authentication needs further interaction */
 		if (send_userauth_info_request(ssh) == 1)
 			authctxt->postponed = 1;

From 91ef15e8ed01a7e16d96ba6cb9ed51965dca9641 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Mon, 16 Oct 2023 21:15:45 +0200
Subject: [PATCH 2/6] auth-pam: Add an enum to define the PAM done status

Makes things more readable and easier to extend
---
 auth-pam.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/auth-pam.c b/auth-pam.c
index 86137a1acdb..21291631011 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -136,11 +136,16 @@ typedef pid_t sp_pthread_t;
 #define pthread_join	fake_pthread_join
 #endif
 
+typedef int SshPamDone;
+#define SshPamError -1
+#define SshPamNone 0
+#define SshPamAuthenticated 1
+
 struct pam_ctxt {
 	sp_pthread_t	 pam_thread;
 	int		 pam_psock;
 	int		 pam_csock;
-	int		 pam_done;
+	SshPamDone	 pam_done;
 };
 
 static void sshpam_free_ctx(void *);
@@ -904,7 +909,7 @@ sshpam_query(void *ctx, char **name, char **info,
 				**prompts = NULL;
 				*num = 0;
 				**echo_on = 0;
-				ctxt->pam_done = -1;
+				ctxt->pam_done = SshPamError;
 				free(msg);
 				sshbuf_free(buffer);
 				return 0;
@@ -931,7 +936,7 @@ sshpam_query(void *ctx, char **name, char **info,
 				import_environments(buffer);
 				*num = 0;
 				**echo_on = 0;
-				ctxt->pam_done = 1;
+				ctxt->pam_done = SshPamAuthenticated;
 				free(msg);
 				sshbuf_free(buffer);
 				return (0);
@@ -944,7 +949,7 @@ sshpam_query(void *ctx, char **name, char **info,
 			*num = 0;
 			**echo_on = 0;
 			free(msg);
-			ctxt->pam_done = -1;
+			ctxt->pam_done = SshPamError;
 			sshbuf_free(buffer);
 			return (-1);
 		}
@@ -988,10 +993,10 @@ sshpam_respond(void *ctx, u_int num, char **resp)
 
 	debug2("PAM: %s entering, %u responses", __func__, num);
 	switch (ctxt->pam_done) {
-	case 1:
+	case SshPamAuthenticated:
 		sshpam_authenticated = 1;
 		return KbdintResultSuccess;
-	case 0:
+	case SshPamNone:
 		break;
 	default:
 		return KbdintResultFailure;

From 6fa8934d31cb9925c856f1b992fc5e04dd26da21 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 17 Oct 2023 04:35:17 +0200
Subject: [PATCH 3/6] auth-pam: Add debugging information when we receive PAM
 messages

---
 auth-pam.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/auth-pam.c b/auth-pam.c
index 21291631011..7a72e724adc 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -450,6 +450,9 @@ sshpam_thread_conv(int n, sshpam_const struct pam_message **msg,
 			break;
 		case PAM_ERROR_MSG:
 		case PAM_TEXT_INFO:
+			debug3("PAM: Got message of type %d: %s",
+			       PAM_MSG_MEMBER(msg, i, msg_style),
+			       PAM_MSG_MEMBER(msg, i, msg));
 			if ((r = sshbuf_put_cstring(buffer,
 			    PAM_MSG_MEMBER(msg, i, msg))) != 0)
 				fatal("%s: buffer error: %s",

From 598ee34312b541fa7b3988b4896641bf81996e27 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 17 Oct 2023 04:27:32 +0200
Subject: [PATCH 4/6] auth-pam: Immediately report interactive instructions to
 clients

SSH keyboard-interactive authentication method supports instructions but
sshd didn't show them until an user prompt was requested.

This is quite inconvenient for various PAM modules that need to notify
an user without requiring for their explicit input.

So, properly implement RFC4256 making instructions to be shown to users
when they are requested from PAM.

Closes: https://bugzilla.mindrot.org/show_bug.cgi?id=2876
---
 auth-pam.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/auth-pam.c b/auth-pam.c
index 7a72e724adc..b756f0e5221 100644
--- a/auth-pam.c
+++ b/auth-pam.c
@@ -140,6 +140,7 @@ typedef int SshPamDone;
 #define SshPamError -1
 #define SshPamNone 0
 #define SshPamAuthenticated 1
+#define SshPamAgain 2
 
 struct pam_ctxt {
 	sp_pthread_t	 pam_thread;
@@ -868,6 +869,8 @@ sshpam_query(void *ctx, char **name, char **info,
 	**prompts = NULL;
 	plen = 0;
 	*echo_on = xmalloc(sizeof(u_int));
+	ctxt->pam_done = SshPamNone;
+
 	while (ssh_msg_recv(ctxt->pam_psock, buffer) == 0) {
 		if (++nmesg > PAM_MAX_NUM_MSG)
 			fatal_f("too many query messages");
@@ -888,15 +891,13 @@ sshpam_query(void *ctx, char **name, char **info,
 			return (0);
 		case PAM_ERROR_MSG:
 		case PAM_TEXT_INFO:
-			/* accumulate messages */
-			len = plen + mlen + 2;
-			**prompts = xreallocarray(**prompts, 1, len);
-			strlcpy(**prompts + plen, msg, len - plen);
-			plen += mlen;
-			strlcat(**prompts + plen, "\n", len - plen);
-			plen++;
-			free(msg);
-			break;
+			*num = 0;
+			free(*info);
+			*info = msg; /* Steal the message */
+			msg = NULL;
+			ctxt->pam_done = SshPamAgain;
+			sshbuf_free(buffer);
+			return (0);
 		case PAM_ACCT_EXPIRED:
 		case PAM_MAXTRIES:
 			if (type == PAM_ACCT_EXPIRED)
@@ -1001,6 +1002,8 @@ sshpam_respond(void *ctx, u_int num, char **resp)
 		return KbdintResultSuccess;
 	case SshPamNone:
 		break;
+	case SshPamAgain:
+		return KbdintResultAgain;
 	default:
 		return KbdintResultFailure;
 	}

From cc14301ce0542cdbb825eff8041ce98a1da9ef08 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 17 Oct 2023 06:12:03 +0200
Subject: [PATCH 5/6] sshconnect2: Write kbd-interactive service, info and
 instructions as utf-8
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

As per the previous server change now the keyboard-interactive service
and instruction values could be reported as soon as they are available
and so they're not prompts anymore and not parsed like them.

While this was already supported by the SSH client, these messages were
not properly written as the escaped sequences they contained were not
correctly reported.

So for example a message containing "\" was represented as "\\" and
similarly for all the other C escape sequences.

This was leading to more problems when it come to utf-8 chars, as they
were only represented by their octal representation.

This was easily testable by adding a line like the one below to the
sshd PAM service:
  auth    requisite pam_echo.so Hello SSHD! Want some 🍕?

Which was causing this to be written instead:
  Hello SSHD! Want some \360\237\215\225?

To handle this, instead of simply using fmprintf, we're using the notifier
in a way can be exposed to users in the proper format and UI.
---
 sshconnect2.c | 33 ++++++++++++++++++++++++---------
 1 file changed, 24 insertions(+), 9 deletions(-)

diff --git a/sshconnect2.c b/sshconnect2.c
index 5831a00c6d1..543431218c1 100644
--- a/sshconnect2.c
+++ b/sshconnect2.c
@@ -1091,6 +1091,7 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh)
 	char *info = NULL, *lang = NULL, *password = NULL, *retype = NULL;
 	char prompt[256];
 	const char *host;
+	size_t info_len;
 	int r;
 
 	debug2("input_userauth_passwd_changereq");
@@ -1100,11 +1101,15 @@ input_userauth_passwd_changereq(int type, u_int32_t seqnr, struct ssh *ssh)
 		    "no authentication context");
 	host = options.host_key_alias ? options.host_key_alias : authctxt->host;
 
-	if ((r = sshpkt_get_cstring(ssh, &info, NULL)) != 0 ||
+	if ((r = sshpkt_get_cstring(ssh, &info, &info_len)) != 0 ||
 	    (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0)
 		goto out;
-	if (strlen(info) > 0)
-		logit("%s", info);
+	if (info_len > 0) {
+		struct notifier_ctx *notifier = NULL;
+		debug_f("input_userauth_passwd_changereq info: %s", info);
+		notifier = notify_start(0, "%s", info);
+		notify_complete(notifier, NULL);
+	}
 	if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_REQUEST)) != 0 ||
 	    (r = sshpkt_put_cstring(ssh, authctxt->server_user)) != 0 ||
 	    (r = sshpkt_put_cstring(ssh, authctxt->service)) != 0 ||
@@ -1938,8 +1943,10 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh)
 	Authctxt *authctxt = ssh->authctxt;
 	char *name = NULL, *inst = NULL, *lang = NULL, *prompt = NULL;
 	char *display_prompt = NULL, *response = NULL;
+	struct notifier_ctx *notifier = NULL;
 	u_char echo = 0;
 	u_int num_prompts, i;
+	size_t name_len, inst_len;
 	int r;
 
 	debug2_f("entering");
@@ -1949,14 +1956,22 @@ input_userauth_info_req(int type, u_int32_t seq, struct ssh *ssh)
 
 	authctxt->info_req_seen = 1;
 
-	if ((r = sshpkt_get_cstring(ssh, &name, NULL)) != 0 ||
-	    (r = sshpkt_get_cstring(ssh, &inst, NULL)) != 0 ||
+	if ((r = sshpkt_get_cstring(ssh, &name, &name_len)) != 0 ||
+	    (r = sshpkt_get_cstring(ssh, &inst, &inst_len)) != 0 ||
 	    (r = sshpkt_get_cstring(ssh, &lang, NULL)) != 0)
 		goto out;
-	if (strlen(name) > 0)
-		logit("%s", name);
-	if (strlen(inst) > 0)
-		logit("%s", inst);
+	if (name_len > 0) {
+		debug_f("kbd int name: %s", name);
+		notifier = notify_start(0, "%s", name);
+		notify_complete(notifier, NULL);
+		notifier = NULL;
+	}
+	if (inst_len > 0) {
+		debug_f("kbd int inst: %s", inst);
+		notifier = notify_start(0, "%s", inst);
+		notify_complete(notifier, NULL);
+		notifier = NULL;
+	}
 
 	if ((r = sshpkt_get_u32(ssh, &num_prompts)) != 0)
 		goto out;

From 99656caabc5cff24122e5b9a140e5a38ab418a5d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Marco=20Trevisan=20=28Trevi=C3=B1o=29?= <mail@3v1n0.net>
Date: Tue, 17 Oct 2023 06:05:59 +0200
Subject: [PATCH 6/6] auth2-chall: Fix selection of the keyboard-interactive
 device

We were only checking if the prefix of a device name was matching what
we had in the devices list, so if the device list contained "pam", then
also the device "pam-foo" was matching.
---
 auth2-chall.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/auth2-chall.c b/auth2-chall.c
index 047d4e83c33..db658c9b4a7 100644
--- a/auth2-chall.c
+++ b/auth2-chall.c
@@ -170,7 +170,7 @@ kbdint_next_device(Authctxt *authctxt, KbdintAuthctxt *kbdintctxt)
 			    "keyboard-interactive", devices[i]->name))
 				continue;
 			if (strncmp(kbdintctxt->devices, devices[i]->name,
-			    len) == 0) {
+			    len) == 0 && strlen(devices[i]->name) == len) {
 				kbdintctxt->device = devices[i];
 				kbdintctxt->devices_done |= 1 << i;
 			}

Places

File 0001-auth-pam-Immediately-report-instructions-to-clients-and-fix-handling-in-ssh-client.patch of Package openssh (Revision 5a12e2847c1bdb136b89b0385bc8f2bc)

Places