Merge changes I934c73d4,I28cdc9a0,I9e734da9,I3c079d86

* changes:
  UPSTREAM: depend
  UPSTREAM: upstream: avoid possible NULL deref; from Pedro Martelletto
  Revert "upstream: fix compilation with DEBUG_KEXDH; bz#3160 ok dtucker@"
  Merge upstream-master into master

1 files changed, 71 insertions, 64 deletions

diff --git a/auth2-chall.c b/auth2-chall.c
index ddabe1a9..3acd0a83 100644
--- a/auth2-chall.c
+++ b/auth2-chall.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-chall.c,v 1.42 2015/01/19 20:07:45 markus Exp $ */
+/* $OpenBSD: auth2-chall.c,v 1.53 2020/02/26 13:40:09 jsg Exp $ */
 /*
  * Copyright (c) 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2001 Per Allansson.  All rights reserved.
@@ -28,18 +28,20 @@
 
 #include <sys/types.h>
 
-#include <stdarg.h>
+#include <stdlib.h>
 #include <stdio.h>
 #include <string.h>
+#include <stdarg.h>
 
 #include "xmalloc.h"
 #include "ssh2.h"
-#include "key.h"
+#include "sshkey.h"
 #include "hostfile.h"
 #include "auth.h"
-#include "buffer.h"
+#include "sshbuf.h"
 #include "packet.h"
 #include "dispatch.h"
+#include "ssherr.h"
 #include "log.h"
 #include "misc.h"
 #include "servconf.h"
@@ -47,9 +49,9 @@
 /* import */
 extern ServerOptions options;
 
-static int auth2_challenge_start(Authctxt *);
-static int send_userauth_info_request(Authctxt *);
-static int input_userauth_info_response(int, u_int32_t, void *);
+static int auth2_challenge_start(struct ssh *);
+static int send_userauth_info_request(struct ssh *);
+static int input_userauth_info_response(int, u_int32_t, struct ssh *);
 
 #ifdef BSD_AUTH
 extern KbdintDevice bsdauth_device;
@@ -57,9 +59,6 @@ extern KbdintDevice bsdauth_device;
 #ifdef USE_PAM
 extern KbdintDevice sshpam_device;
 #endif
-#ifdef SKEY
-extern KbdintDevice skey_device;
-#endif
 #endif
 
 KbdintDevice *devices[] = {
@@ -69,9 +68,6 @@ KbdintDevice *devices[] = {
 #ifdef USE_PAM
 	&sshpam_device,
 #endif
-#ifdef SKEY
-	&skey_device,
-#endif
 #endif
 	NULL
 };
@@ -83,6 +79,7 @@ struct KbdintAuthctxt
 	void *ctxt;
 	KbdintDevice *device;
 	u_int nreq;
+	u_int devices_done;
 };
 
 #ifdef USE_PAM
@@ -104,8 +101,8 @@ static KbdintAuthctxt *
 kbdint_alloc(const char *devs)
 {
 	KbdintAuthctxt *kbdintctxt;
-	Buffer b;
-	int i;
+	struct sshbuf *b;
+	int i, r;
 
 #ifdef USE_PAM
 	if (!options.use_pam)
@@ -114,16 +111,17 @@ kbdint_alloc(const char *devs)
 
 	kbdintctxt = xcalloc(1, sizeof(KbdintAuthctxt));
 	if (strcmp(devs, "") == 0) {
-		buffer_init(&b);
+		if ((b = sshbuf_new()) == NULL)
+			fatal("%s: sshbuf_new failed", __func__);
 		for (i = 0; devices[i]; i++) {
-			if (buffer_len(&b) > 0)
-				buffer_append(&b, ",", 1);
-			buffer_append(&b, devices[i]->name,
-			    strlen(devices[i]->name));
+			if ((r = sshbuf_putf(b, "%s%s",
+			    sshbuf_len(b) ? "," : "", devices[i]->name)) != 0)
+				fatal("%s: buffer error: %s",
+				    __func__, ssh_err(r));
 		}
-		buffer_append(&b, "\0", 1);
-		kbdintctxt->devices = xstrdup(buffer_ptr(&b));
-		buffer_free(&b);
+		if ((kbdintctxt->devices = sshbuf_dup_string(b)) == NULL)
+			fatal("%s: sshbuf_dup_string failed", __func__);
+		sshbuf_free(b);
 	} else {
 		kbdintctxt->devices = xstrdup(devs);
 	}
@@ -149,8 +147,7 @@ kbdint_free(KbdintAuthctxt *kbdintctxt)
 	if (kbdintctxt->device)
 		kbdint_reset_device(kbdintctxt);
 	free(kbdintctxt->devices);
-	explicit_bzero(kbdintctxt, sizeof(*kbdintctxt));
-	free(kbdintctxt);
+	freezero(kbdintctxt, sizeof(*kbdintctxt));
 }
 /* get next device */
 static int
@@ -169,11 +166,15 @@ kbdint_next_device(Authctxt *authctxt, KbdintAuthctxt *kbdintctxt)
 		if (len == 0)
 			break;
 		for (i = 0; devices[i]; i++) {
-			if (!auth2_method_allowed(authctxt,
+			if ((kbdintctxt->devices_done & (1 << i)) != 0 ||
+			    !auth2_method_allowed(authctxt,
 			    "keyboard-interactive", devices[i]->name))
 				continue;
-			if (strncmp(kbdintctxt->devices, devices[i]->name, len) == 0)
+			if (strncmp(kbdintctxt->devices, devices[i]->name,
+			    len) == 0) {
 				kbdintctxt->device = devices[i];
+				kbdintctxt->devices_done |= 1 << i;
+			}
 		}
 		t = kbdintctxt->devices;
 		kbdintctxt->devices = t[len] ? xstrdup(t+len+1) : NULL;
@@ -190,8 +191,9 @@ kbdint_next_device(Authctxt *authctxt, KbdintAuthctxt *kbdintctxt)
  * wait for the response.
  */
 int
-auth2_challenge(Authctxt *authctxt, char *devs)
+auth2_challenge(struct ssh *ssh, char *devs)
 {
+	Authctxt *authctxt = ssh->authctxt;
 	debug("auth2_challenge: user=%s devs=%s",
 	    authctxt->user ? authctxt->user : "<nouser>",
 	    devs ? devs : "<no devs>");
@@ -200,15 +202,16 @@ auth2_challenge(Authctxt *authctxt, char *devs)
 		return 0;
 	if (authctxt->kbdintctxt == NULL)
 		authctxt->kbdintctxt = kbdint_alloc(devs);
-	return auth2_challenge_start(authctxt);
+	return auth2_challenge_start(ssh);
 }
 
 /* unregister kbd-int callbacks and context */
 void
-auth2_challenge_stop(Authctxt *authctxt)
+auth2_challenge_stop(struct ssh *ssh)
 {
+	Authctxt *authctxt = ssh->authctxt;
 	/* unregister callback */
-	dispatch_set(SSH2_MSG_USERAUTH_INFO_RESPONSE, NULL);
+	ssh_dispatch_set(ssh, SSH2_MSG_USERAUTH_INFO_RESPONSE, NULL);
 	if (authctxt->kbdintctxt != NULL) {
 		kbdint_free(authctxt->kbdintctxt);
 		authctxt->kbdintctxt = NULL;
@@ -217,29 +220,30 @@ auth2_challenge_stop(Authctxt *authctxt)
 
 /* side effect: sets authctxt->postponed if a reply was sent*/
 static int
-auth2_challenge_start(Authctxt *authctxt)
+auth2_challenge_start(struct ssh *ssh)
 {
+	Authctxt *authctxt = ssh->authctxt;
 	KbdintAuthctxt *kbdintctxt = authctxt->kbdintctxt;
 
 	debug2("auth2_challenge_start: devices %s",
 	    kbdintctxt->devices ?  kbdintctxt->devices : "<empty>");
 
 	if (kbdint_next_device(authctxt, kbdintctxt) == 0) {
-		auth2_challenge_stop(authctxt);
+		auth2_challenge_stop(ssh);
 		return 0;
 	}
 	debug("auth2_challenge_start: trying authentication method '%s'",
 	    kbdintctxt->device->name);
 
 	if ((kbdintctxt->ctxt = kbdintctxt->device->init_ctx(authctxt)) == NULL) {
-		auth2_challenge_stop(authctxt);
+		auth2_challenge_stop(ssh);
 		return 0;
 	}
-	if (send_userauth_info_request(authctxt) == 0) {
-		auth2_challenge_stop(authctxt);
+	if (send_userauth_info_request(ssh) == 0) {
+		auth2_challenge_stop(ssh);
 		return 0;
 	}
-	dispatch_set(SSH2_MSG_USERAUTH_INFO_RESPONSE,
+	ssh_dispatch_set(ssh, SSH2_MSG_USERAUTH_INFO_RESPONSE,
 	    &input_userauth_info_response);
 
 	authctxt->postponed = 1;
@@ -247,28 +251,32 @@ auth2_challenge_start(Authctxt *authctxt)
 }
 
 static int
-send_userauth_info_request(Authctxt *authctxt)
+send_userauth_info_request(struct ssh *ssh)
 {
+	Authctxt *authctxt = ssh->authctxt;
 	KbdintAuthctxt *kbdintctxt;
 	char *name, *instr, **prompts;
-	u_int i, *echo_on;
+	u_int r, i, *echo_on;
 
 	kbdintctxt = authctxt->kbdintctxt;
 	if (kbdintctxt->device->query(kbdintctxt->ctxt,
 	    &name, &instr, &kbdintctxt->nreq, &prompts, &echo_on))
 		return 0;
 
-	packet_start(SSH2_MSG_USERAUTH_INFO_REQUEST);
-	packet_put_cstring(name);
-	packet_put_cstring(instr);
-	packet_put_cstring("");		/* language not used */
-	packet_put_int(kbdintctxt->nreq);
+	if ((r = sshpkt_start(ssh, SSH2_MSG_USERAUTH_INFO_REQUEST)) != 0 ||
+	    (r = sshpkt_put_cstring(ssh, name)) != 0 ||
+	    (r = sshpkt_put_cstring(ssh, instr)) != 0 ||
+	    (r = sshpkt_put_cstring(ssh, "")) != 0 ||	/* language not used */
+	    (r = sshpkt_put_u32(ssh, kbdintctxt->nreq)) != 0)
+		fatal("%s: %s", __func__, ssh_err(r));
 	for (i = 0; i < kbdintctxt->nreq; i++) {
-		packet_put_cstring(prompts[i]);
-		packet_put_char(echo_on[i]);
+		if ((r = sshpkt_put_cstring(ssh, prompts[i])) != 0 ||
+		    (r = sshpkt_put_u8(ssh, echo_on[i])) != 0)
+			fatal("%s: %s", __func__, ssh_err(r));
 	}
-	packet_send();
-	packet_write_wait();
+	if ((r = sshpkt_send(ssh)) != 0 ||
+	    (r = ssh_packet_write_wait(ssh)) != 0)
+		fatal("%s: %s", __func__, ssh_err(r));
 
 	for (i = 0; i < kbdintctxt->nreq; i++)
 		free(prompts[i]);
@@ -280,11 +288,12 @@ send_userauth_info_request(Authctxt *authctxt)
 }
 
 static int
-input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
+input_userauth_info_response(int type, u_int32_t seq, struct ssh *ssh)
 {
-	Authctxt *authctxt = ctxt;
+	Authctxt *authctxt = ssh->authctxt;
 	KbdintAuthctxt *kbdintctxt;
 	int authenticated = 0, res;
+	int r;
 	u_int i, nresp;
 	const char *devicename = NULL;
 	char **response = NULL;
@@ -298,7 +307,8 @@ input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
 		fatal("input_userauth_info_response: no device");
 
 	authctxt->postponed = 0;	/* reset */
-	nresp = packet_get_int();
+	if ((r = sshpkt_get_u32(ssh, &nresp)) != 0)
+		fatal("%s: %s", __func__, ssh_err(r));
 	if (nresp != kbdintctxt->nreq)
 		fatal("input_userauth_info_response: wrong number of replies");
 	if (nresp > 100)
@@ -306,9 +316,12 @@ input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
 	if (nresp > 0) {
 		response = xcalloc(nresp, sizeof(char *));
 		for (i = 0; i < nresp; i++)
-			response[i] = packet_get_string(NULL);
+			if ((r = sshpkt_get_cstring(ssh, &response[i],
+			    NULL)) != 0)
+				fatal("%s: %s", __func__, ssh_err(r));
 	}
-	packet_check_eom();
+	if ((r = sshpkt_get_end(ssh)) != 0)
+		fatal("%s: %s", __func__, ssh_err(r));
 
 	res = kbdintctxt->device->respond(kbdintctxt->ctxt, nresp, response);
 
@@ -325,7 +338,7 @@ input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
 		break;
 	case 1:
 		/* Authentication needs further interaction */
-		if (send_userauth_info_request(authctxt) == 1)
+		if (send_userauth_info_request(ssh) == 1)
 			authctxt->postponed = 1;
 		break;
 	default:
@@ -335,14 +348,14 @@ input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
 	devicename = kbdintctxt->device->name;
 	if (!authctxt->postponed) {
 		if (authenticated) {
-			auth2_challenge_stop(authctxt);
+			auth2_challenge_stop(ssh);
 		} else {
 			/* start next device */
 			/* may set authctxt->postponed */
-			auth2_challenge_start(authctxt);
+			auth2_challenge_start(ssh);
 		}
 	}
-	userauth_finish(authctxt, authenticated, "keyboard-interactive",
+	userauth_finish(ssh, authenticated, "keyboard-interactive",
 	    devicename);
 	return 0;
 }
@@ -350,7 +363,7 @@ input_userauth_info_response(int type, u_int32_t seq, void *ctxt)
 void
 privsep_challenge_enable(void)
 {
-#if defined(BSD_AUTH) || defined(USE_PAM) || defined(SKEY)
+#if defined(BSD_AUTH) || defined(USE_PAM)
 	int n = 0;
 #endif
 #ifdef BSD_AUTH
@@ -359,9 +372,6 @@ privsep_challenge_enable(void)
 #ifdef USE_PAM
 	extern KbdintDevice mm_sshpam_device;
 #endif
-#ifdef SKEY
-	extern KbdintDevice mm_skey_device;
-#endif
 
 #ifdef BSD_AUTH
 	devices[n++] = &mm_bsdauth_device;
@@ -369,8 +379,5 @@ privsep_challenge_enable(void)
 #ifdef USE_PAM
 	devices[n++] = &mm_sshpam_device;
 #endif
-#ifdef SKEY
-	devices[n++] = &mm_skey_device;
-#endif
 #endif
 }