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, 209 insertions, 127 deletions

diff --git a/ssh-pkcs11-helper.c b/ssh-pkcs11-helper.c
index ceabc8ba..d73e8357 100644
--- a/ssh-pkcs11-helper.c
+++ b/ssh-pkcs11-helper.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-pkcs11-helper.c,v 1.10 2015/01/20 23:14:00 deraadt Exp $ */
+/* $OpenBSD: ssh-pkcs11-helper.c,v 1.23 2020/03/06 18:26:21 markus Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
  *
@@ -24,26 +24,33 @@
 
 #include "openbsd-compat/sys-queue.h"
 
+#include <stdlib.h>
+#include <errno.h>
+#ifdef HAVE_POLL_H
+#include <poll.h>
+#endif
 #include <stdarg.h>
 #include <string.h>
 #include <unistd.h>
-#include <errno.h>
 
 #include "xmalloc.h"
-#include "buffer.h"
+#include "sshbuf.h"
 #include "log.h"
 #include "misc.h"
-#include "key.h"
+#include "sshkey.h"
 #include "authfd.h"
 #include "ssh-pkcs11.h"
+#include "ssherr.h"
 
 #ifdef ENABLE_PKCS11
 
+#ifdef WITH_OPENSSL
+
 /* borrows code from sftp-server and ssh-agent */
 
 struct pkcs11_keyinfo {
-	Key		*key;
-	char		*providername;
+	struct sshkey	*key;
+	char		*providername, *label;
 	TAILQ_ENTRY(pkcs11_keyinfo) next;
 };
 
@@ -51,22 +58,19 @@ TAILQ_HEAD(, pkcs11_keyinfo) pkcs11_keylist;
 
 #define MAX_MSG_LENGTH		10240 /*XXX*/
 
-/* helper */
-#define get_int()			buffer_get_int(&iqueue);
-#define get_string(lenp)		buffer_get_string(&iqueue, lenp);
-
 /* input and output queue */
-Buffer iqueue;
-Buffer oqueue;
+struct sshbuf *iqueue;
+struct sshbuf *oqueue;
 
 static void
-add_key(Key *k, char *name)
+add_key(struct sshkey *k, char *name, char *label)
 {
 	struct pkcs11_keyinfo *ki;
 
 	ki = xcalloc(1, sizeof(*ki));
 	ki->providername = xstrdup(name);
 	ki->key = k;
+	ki->label = xstrdup(label);
 	TAILQ_INSERT_TAIL(&pkcs11_keylist, ki, next);
 }
 
@@ -80,131 +84,175 @@ del_keys_by_name(char *name)
 		if (!strcmp(ki->providername, name)) {
 			TAILQ_REMOVE(&pkcs11_keylist, ki, next);
 			free(ki->providername);
-			key_free(ki->key);
+			free(ki->label);
+			sshkey_free(ki->key);
 			free(ki);
 		}
 	}
 }
 
 /* lookup matching 'private' key */
-static Key *
-lookup_key(Key *k)
+static struct sshkey *
+lookup_key(struct sshkey *k)
 {
 	struct pkcs11_keyinfo *ki;
 
 	TAILQ_FOREACH(ki, &pkcs11_keylist, next) {
-		debug("check %p %s", ki, ki->providername);
-		if (key_equal(k, ki->key))
+		debug("check %p %s %s", ki, ki->providername, ki->label);
+		if (sshkey_equal(k, ki->key))
 			return (ki->key);
 	}
 	return (NULL);
 }
 
 static void
-send_msg(Buffer *m)
+send_msg(struct sshbuf *m)
 {
-	int mlen = buffer_len(m);
+	int r;
 
-	buffer_put_int(&oqueue, mlen);
-	buffer_append(&oqueue, buffer_ptr(m), mlen);
-	buffer_consume(m, mlen);
+	if ((r = sshbuf_put_stringb(oqueue, m)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 }
 
 static void
 process_add(void)
 {
 	char *name, *pin;
-	Key **keys;
-	int i, nkeys;
+	struct sshkey **keys = NULL;
+	int r, i, nkeys;
 	u_char *blob;
-	u_int blen;
-	Buffer msg;
-
-	buffer_init(&msg);
-	name = get_string(NULL);
-	pin = get_string(NULL);
-	if ((nkeys = pkcs11_add_provider(name, pin, &keys)) > 0) {
-		buffer_put_char(&msg, SSH2_AGENT_IDENTITIES_ANSWER);
-		buffer_put_int(&msg, nkeys);
+	size_t blen;
+	struct sshbuf *msg;
+	char **labels = NULL;
+
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_get_cstring(iqueue, &name, NULL)) != 0 ||
+	    (r = sshbuf_get_cstring(iqueue, &pin, NULL)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	if ((nkeys = pkcs11_add_provider(name, pin, &keys, &labels)) > 0) {
+		if ((r = sshbuf_put_u8(msg,
+		    SSH2_AGENT_IDENTITIES_ANSWER)) != 0 ||
+		    (r = sshbuf_put_u32(msg, nkeys)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 		for (i = 0; i < nkeys; i++) {
-			if (key_to_blob(keys[i], &blob, &blen) == 0)
+			if ((r = sshkey_to_blob(keys[i], &blob, &blen)) != 0) {
+				debug("%s: sshkey_to_blob: %s",
+				    __func__, ssh_err(r));
 				continue;
-			buffer_put_string(&msg, blob, blen);
-			buffer_put_cstring(&msg, name);
+			}
+			if ((r = sshbuf_put_string(msg, blob, blen)) != 0 ||
+			    (r = sshbuf_put_cstring(msg, labels[i])) != 0)
+				fatal("%s: buffer error: %s",
+				    __func__, ssh_err(r));
 			free(blob);
-			add_key(keys[i], name);
+			add_key(keys[i], name, labels[i]);
+			free(labels[i]);
 		}
-		free(keys);
 	} else {
-		buffer_put_char(&msg, SSH_AGENT_FAILURE);
+		if ((r = sshbuf_put_u8(msg, SSH_AGENT_FAILURE)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
+		if ((r = sshbuf_put_u32(msg, -nkeys)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	}
+	free(labels);
+	free(keys); /* keys themselves are transferred to pkcs11_keylist */
 	free(pin);
 	free(name);
-	send_msg(&msg);
-	buffer_free(&msg);
+	send_msg(msg);
+	sshbuf_free(msg);
 }
 
 static void
 process_del(void)
 {
 	char *name, *pin;
-	Buffer msg;
-
-	buffer_init(&msg);
-	name = get_string(NULL);
-	pin = get_string(NULL);
+	struct sshbuf *msg;
+	int r;
+
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_get_cstring(iqueue, &name, NULL)) != 0 ||
+	    (r = sshbuf_get_cstring(iqueue, &pin, NULL)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	del_keys_by_name(name);
-	if (pkcs11_del_provider(name) == 0)
-		 buffer_put_char(&msg, SSH_AGENT_SUCCESS);
-	else
-		 buffer_put_char(&msg, SSH_AGENT_FAILURE);
+	if ((r = sshbuf_put_u8(msg, pkcs11_del_provider(name) == 0 ?
+	    SSH_AGENT_SUCCESS : SSH_AGENT_FAILURE)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	free(pin);
 	free(name);
-	send_msg(&msg);
-	buffer_free(&msg);
+	send_msg(msg);
+	sshbuf_free(msg);
 }
 
 static void
 process_sign(void)
 {
 	u_char *blob, *data, *signature = NULL;
-	u_int blen, dlen, slen = 0;
-	int ok = -1;
-	Key *key, *found;
-	Buffer msg;
-
-	blob = get_string(&blen);
-	data = get_string(&dlen);
-	(void)get_int(); /* XXX ignore flags */
-
-	if ((key = key_from_blob(blob, blen)) != NULL) {
+	size_t blen, dlen, slen = 0;
+	int r, ok = -1;
+	struct sshkey *key, *found;
+	struct sshbuf *msg;
+
+	/* XXX support SHA2 signature flags */
+	if ((r = sshbuf_get_string(iqueue, &blob, &blen)) != 0 ||
+	    (r = sshbuf_get_string(iqueue, &data, &dlen)) != 0 ||
+	    (r = sshbuf_get_u32(iqueue, NULL)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+
+	if ((r = sshkey_from_blob(blob, blen, &key)) != 0)
+		error("%s: sshkey_from_blob: %s", __func__, ssh_err(r));
+	else {
 		if ((found = lookup_key(key)) != NULL) {
 #ifdef WITH_OPENSSL
 			int ret;
 
-			slen = RSA_size(key->rsa);
-			signature = xmalloc(slen);
-			if ((ret = RSA_private_encrypt(dlen, data, signature,
-			    found->rsa, RSA_PKCS1_PADDING)) != -1) {
-				slen = ret;
-				ok = 0;
-			}
+			if (key->type == KEY_RSA) {
+				slen = RSA_size(key->rsa);
+				signature = xmalloc(slen);
+				ret = RSA_private_encrypt(dlen, data, signature,
+				    found->rsa, RSA_PKCS1_PADDING);
+				if (ret != -1) {
+					slen = ret;
+					ok = 0;
+				}
+#ifdef OPENSSL_HAS_ECC
+			} else if (key->type == KEY_ECDSA) {
+				u_int xslen = ECDSA_size(key->ecdsa);
+
+				signature = xmalloc(xslen);
+				/* "The parameter type is ignored." */
+				ret = ECDSA_sign(-1, data, dlen, signature,
+				    &xslen, found->ecdsa);
+				if (ret != 0)
+					ok = 0;
+				else
+					error("%s: ECDSA_sign"
+					    " returns %d", __func__, ret);
+				slen = xslen;
+#endif /* OPENSSL_HAS_ECC */
+			} else
+				error("%s: don't know how to sign with key "
+				    "type %d", __func__, (int)key->type);
 #endif /* WITH_OPENSSL */
 		}
-		key_free(key);
+		sshkey_free(key);
 	}
-	buffer_init(&msg);
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
 	if (ok == 0) {
-		buffer_put_char(&msg, SSH2_AGENT_SIGN_RESPONSE);
-		buffer_put_string(&msg, signature, slen);
+		if ((r = sshbuf_put_u8(msg, SSH2_AGENT_SIGN_RESPONSE)) != 0 ||
+		    (r = sshbuf_put_string(msg, signature, slen)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	} else {
-		buffer_put_char(&msg, SSH_AGENT_FAILURE);
+		if ((r = sshbuf_put_u8(msg, SSH2_AGENT_FAILURE)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	}
 	free(data);
 	free(blob);
 	free(signature);
-	send_msg(&msg);
-	buffer_free(&msg);
+	send_msg(msg);
+	sshbuf_free(msg);
 }
 
 static void
@@ -213,13 +261,14 @@ process(void)
 	u_int msg_len;
 	u_int buf_len;
 	u_int consumed;
-	u_int type;
-	u_char *cp;
+	u_char type;
+	const u_char *cp;
+	int r;
 
-	buf_len = buffer_len(&iqueue);
+	buf_len = sshbuf_len(iqueue);
 	if (buf_len < 5)
 		return;		/* Incomplete message. */
-	cp = buffer_ptr(&iqueue);
+	cp = sshbuf_ptr(iqueue);
 	msg_len = get_u32(cp);
 	if (msg_len > MAX_MSG_LENGTH) {
 		error("bad message len %d", msg_len);
@@ -227,9 +276,10 @@ process(void)
 	}
 	if (buf_len < msg_len + 4)
 		return;
-	buffer_consume(&iqueue, 4);
+	if ((r = sshbuf_consume(iqueue, 4)) != 0 ||
+	    (r = sshbuf_get_u8(iqueue, &type)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	buf_len -= 4;
-	type = buffer_get_char(&iqueue);
 	switch (type) {
 	case SSH_AGENTC_ADD_SMARTCARD_KEY:
 		debug("process_add");
@@ -248,17 +298,19 @@ process(void)
 		break;
 	}
 	/* discard the remaining bytes from the current packet */
-	if (buf_len < buffer_len(&iqueue)) {
+	if (buf_len < sshbuf_len(iqueue)) {
 		error("iqueue grew unexpectedly");
 		cleanup_exit(255);
 	}
-	consumed = buf_len - buffer_len(&iqueue);
+	consumed = buf_len - sshbuf_len(iqueue);
 	if (msg_len < consumed) {
 		error("msg_len %d < consumed %d", msg_len, consumed);
 		cleanup_exit(255);
 	}
-	if (msg_len > consumed)
-		buffer_consume(&iqueue, msg_len - consumed);
+	if (msg_len > consumed) {
+		if ((r = sshbuf_consume(iqueue, msg_len - consumed)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	}
 }
 
 void
@@ -268,68 +320,78 @@ cleanup_exit(int i)
 	_exit(i);
 }
 
+
 int
 main(int argc, char **argv)
 {
-	fd_set *rset, *wset;
-	int in, out, max, log_stderr = 0;
-	ssize_t len, olen, set_size;
+	int r, ch, in, out, log_stderr = 0;
+	ssize_t len;
 	SyslogFacility log_facility = SYSLOG_FACILITY_AUTH;
 	LogLevel log_level = SYSLOG_LEVEL_ERROR;
 	char buf[4*4096];
-
 	extern char *__progname;
+	struct pollfd pfd[2];
 
+	__progname = ssh_get_progname(argv[0]);
+	seed_rng();
 	TAILQ_INIT(&pkcs11_keylist);
-	pkcs11_init(0);
 
-	seed_rng();
-	__progname = ssh_get_progname(argv[0]);
+	log_init(__progname, log_level, log_facility, log_stderr);
+
+	while ((ch = getopt(argc, argv, "v")) != -1) {
+		switch (ch) {
+		case 'v':
+			log_stderr = 1;
+			if (log_level == SYSLOG_LEVEL_ERROR)
+				log_level = SYSLOG_LEVEL_DEBUG1;
+			else if (log_level < SYSLOG_LEVEL_DEBUG3)
+				log_level++;
+			break;
+		default:
+			fprintf(stderr, "usage: %s [-v]\n", __progname);
+			exit(1);
+		}
+	}
 
 	log_init(__progname, log_level, log_facility, log_stderr);
 
+	pkcs11_init(0);
+
 	in = STDIN_FILENO;
 	out = STDOUT_FILENO;
 
-	max = 0;
-	if (in > max)
-		max = in;
-	if (out > max)
-		max = out;
+	if ((iqueue = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((oqueue = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
 
-	buffer_init(&iqueue);
-	buffer_init(&oqueue);
-
-	set_size = howmany(max + 1, NFDBITS) * sizeof(fd_mask);
-	rset = (fd_set *)xmalloc(set_size);
-	wset = (fd_set *)xmalloc(set_size);
-
-	for (;;) {
-		memset(rset, 0, set_size);
-		memset(wset, 0, set_size);
+	while (1) {
+		memset(pfd, 0, sizeof(pfd));
+		pfd[0].fd = in;
+		pfd[1].fd = out;
 
 		/*
 		 * Ensure that we can read a full buffer and handle
 		 * the worst-case length packet it can generate,
 		 * otherwise apply backpressure by stopping reads.
 		 */
-		if (buffer_check_alloc(&iqueue, sizeof(buf)) &&
-		    buffer_check_alloc(&oqueue, MAX_MSG_LENGTH))
-			FD_SET(in, rset);
+		if ((r = sshbuf_check_reserve(iqueue, sizeof(buf))) == 0 &&
+		    (r = sshbuf_check_reserve(oqueue, MAX_MSG_LENGTH)) == 0)
+			pfd[0].events = POLLIN;
+		else if (r != SSH_ERR_NO_BUFFER_SPACE)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 
-		olen = buffer_len(&oqueue);
-		if (olen > 0)
-			FD_SET(out, wset);
+		if (sshbuf_len(oqueue) > 0)
+			pfd[1].events = POLLOUT;
 
-		if (select(max+1, rset, wset, NULL, NULL) < 0) {
-			if (errno == EINTR)
+		if ((r = poll(pfd, 2, -1 /* INFTIM */)) <= 0) {
+			if (r == 0 || errno == EINTR)
 				continue;
-			error("select: %s", strerror(errno));
-			cleanup_exit(2);
+			fatal("poll: %s", strerror(errno));
 		}
 
 		/* copy stdin to iqueue */
-		if (FD_ISSET(in, rset)) {
+		if ((pfd[0].revents & (POLLIN|POLLERR)) != 0) {
 			len = read(in, buf, sizeof buf);
 			if (len == 0) {
 				debug("read eof");
@@ -337,18 +399,21 @@ main(int argc, char **argv)
 			} else if (len < 0) {
 				error("read: %s", strerror(errno));
 				cleanup_exit(1);
-			} else {
-				buffer_append(&iqueue, buf, len);
+			} else if ((r = sshbuf_put(iqueue, buf, len)) != 0) {
+				fatal("%s: buffer error: %s",
+				    __func__, ssh_err(r));
 			}
 		}
 		/* send oqueue to stdout */
-		if (FD_ISSET(out, wset)) {
-			len = write(out, buffer_ptr(&oqueue), olen);
+		if ((pfd[1].revents & (POLLOUT|POLLHUP)) != 0) {
+			len = write(out, sshbuf_ptr(oqueue),
+			    sshbuf_len(oqueue));
 			if (len < 0) {
 				error("write: %s", strerror(errno));
 				cleanup_exit(1);
-			} else {
-				buffer_consume(&oqueue, len);
+			} else if ((r = sshbuf_consume(oqueue, len)) != 0) {
+				fatal("%s: buffer error: %s",
+				    __func__, ssh_err(r));
 			}
 		}
 
@@ -357,10 +422,27 @@ main(int argc, char **argv)
 		 * into the output buffer, otherwise stop processing input
 		 * and let the output queue drain.
 		 */
-		if (buffer_check_alloc(&oqueue, MAX_MSG_LENGTH))
+		if ((r = sshbuf_check_reserve(oqueue, MAX_MSG_LENGTH)) == 0)
 			process();
+		else if (r != SSH_ERR_NO_BUFFER_SPACE)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 	}
 }
+
+#else /* WITH_OPENSSL */
+void
+cleanup_exit(int i)
+{
+	_exit(i);
+}
+
+int
+main(int argc, char **argv)
+{
+	fprintf(stderr, "PKCS#11 code is not enabled\n");
+	return 1;
+}
+#endif /* WITH_OPENSSL */
 #else /* ENABLE_PKCS11 */
 int
 main(int argc, char **argv)