Merge ~3v1n0/ubuntu/+source/openssh:ubuntu/devel into ubuntu/+source/openssh:ubuntu/devel

[Marco Trevisan (Treviño)]

Cherry-pick upstream proposed and partially approved patches (one maintainer already did, but we need the final merge) [1] to fix usage of SSHd and ssh client with PAM modules that use interactive messages and requests.

[1] https://github.com/openssh/openssh-portable/pull/452

[Tobias Heider (tobhe)]

review: Approve

[Julian Andres Klode (juliank)]

This is a bit awkward because it has no changelog but I semi-sponsored it with my signoff line

review: Approve

[Marco Trevisan (Treviño) (3v1n0)]

Julian, I didn't add the changelog because I wasn't sure how the team manages it. For example in desktop, we generate it at upload time using gbp dch, so I was thinking the uploader would have done the same.

diff --git a/debian/patches/auth-Add-KbdintResult-definition-to-define-result-values-.patch b/debian/patches/auth-Add-KbdintResult-definition-to-define-result-values-.patch
new file mode 100644
index 0000000..90880ec
--- /dev/null
+++ b/debian/patches/auth-Add-KbdintResult-definition-to-define-result-values-.patch
@@ -0,0 +1,106 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+Date: Tue, 17 Oct 2023 04:04:13 +0200
+Subject: 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.
+
+Origin: https://github.com/openssh/openssh-portable/pull/452
+---
+ 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 d124e99..ca41735 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 b49d415..86137a1 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 d16dc66..0f1ae29 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;
+@@ -112,6 +113,10 @@ struct Authmethod {
+ 	int	*enabled;
+ };
+ 
++#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 021df82..047d4e8 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;
diff --git a/debian/patches/auth-pam-Add-an-enum-to-define-the-PAM-done-status.patch b/debian/patches/auth-pam-Add-an-enum-to-define-the-PAM-done-status.patch
new file mode 100644
index 0000000..8deffea
--- /dev/null
+++ b/debian/patches/auth-pam-Add-an-enum-to-define-the-PAM-done-status.patch
@@ -0,0 +1,73 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+Date: Mon, 16 Oct 2023 21:15:45 +0200
+Subject: auth-pam: Add an enum to define the PAM done status
+
+Makes things more readable and easier to extend
+
+Origin: https://github.com/openssh/openssh-portable/pull/452
+---
+ auth-pam.c | 17 +++++++++++------
+ 1 file changed, 11 insertions(+), 6 deletions(-)
+
+diff --git a/auth-pam.c b/auth-pam.c
+index 86137a1..2129163 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;
diff --git a/debian/patches/auth-pam-Add-debugging-information-when-we-receive-PAM-me.patch b/debian/patches/auth-pam-Add-debugging-information-when-we-receive-PAM-me.patch
new file mode 100644
index 0000000..ea7fe20
--- /dev/null
+++ b/debian/patches/auth-pam-Add-debugging-information-when-we-receive-PAM-me.patch
@@ -0,0 +1,23 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+Date: Tue, 17 Oct 2023 04:35:17 +0200
+Subject: auth-pam: Add debugging information when we receive PAM messages
+
+Origin: https://github.com/openssh/openssh-portable/pull/452
+---
+ auth-pam.c | 3 +++
+ 1 file changed, 3 insertions(+)
+
+diff --git a/auth-pam.c b/auth-pam.c
+index 2129163..7a72e72 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",
diff --git a/debian/patches/auth-pam-Immediately-report-interactive-instructions-to-c.patch b/debian/patches/auth-pam-Immediately-report-interactive-instructions-to-c.patch
new file mode 100644
index 0000000..f8f00bf
--- /dev/null
+++ b/debian/patches/auth-pam-Immediately-report-interactive-instructions-to-c.patch
@@ -0,0 +1,73 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+Date: Tue, 17 Oct 2023 04:27:32 +0200
+Subject: 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
+
+Origin: https://github.com/openssh/openssh-portable/pull/452
+---
+ auth-pam.c | 21 ++++++++++++---------
+ 1 file changed, 12 insertions(+), 9 deletions(-)
+
+diff --git a/auth-pam.c b/auth-pam.c
+index 7a72e72..b756f0e 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;
+ 	}
diff --git a/debian/patches/series b/debian/patches/series
index 42a873f..aba5430 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -28,3 +28,8 @@ systemd-socket-activation.patch
 broken-zero-call-used-regs.patch
 socket-activation-documentation.patch
 test-set-UsePAM-no-on-some-tests.patch
+auth-Add-KbdintResult-definition-to-define-result-values-.patch
+auth-pam-Add-an-enum-to-define-the-PAM-done-status.patch
+auth-pam-Add-debugging-information-when-we-receive-PAM-me.patch
+auth-pam-Immediately-report-interactive-instructions-to-c.patch
+sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
diff --git a/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch b/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
new file mode 100644
index 0000000..238b98a
--- /dev/null
+++ b/debian/patches/sshconnect2-Write-kbd-interactive-service-info-and-instru.patch
@@ -0,0 +1,108 @@
+From: =?utf-8?b?Ik1hcmNvIFRyZXZpc2FuIChUcmV2acOxbyki?= <[e-mail]>
+Date: Tue, 17 Oct 2023 06:12:03 +0200
+Subject: 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.
+
+Origin: https://github.com/openssh/openssh-portable/pull/452
+---
+ sshconnect2.c | 33 ++++++++++++++++++++++++---------
+ 1 file changed, 24 insertions(+), 9 deletions(-)
+
+diff --git a/sshconnect2.c b/sshconnect2.c
+index cb584ad..7b9959d 100644
+--- a/sshconnect2.c
++++ b/sshconnect2.c
+@@ -1231,6 +1231,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");
+@@ -1240,11 +1241,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 ||
+@@ -2098,8 +2103,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");
+@@ -2109,14 +2116,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;