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, 232 insertions, 82 deletions

diff --git a/ssh-pkcs11-client.c b/ssh-pkcs11-client.c
index 8c74864a..8a0ffef5 100644
--- a/ssh-pkcs11-client.c
+++ b/ssh-pkcs11-client.c
@@ -1,6 +1,7 @@
-/* $OpenBSD: ssh-pkcs11-client.c,v 1.5 2014/06/24 01:13:21 djm Exp $ */
+/* $OpenBSD: ssh-pkcs11-client.c,v 1.16 2020/01/25 00:03:36 djm Exp $ */
 /*
  * Copyright (c) 2010 Markus Friedl.  All rights reserved.
+ * Copyright (c) 2014 Pedro Martelletto. All rights reserved.
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -30,52 +31,59 @@
 #include <unistd.h>
 #include <errno.h>
 
+#include <openssl/ecdsa.h>
 #include <openssl/rsa.h>
 
+#include "openbsd-compat/openssl-compat.h"
+
 #include "pathnames.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 "atomicio.h"
 #include "ssh-pkcs11.h"
+#include "ssherr.h"
 
 /* borrows code from sftp-server and ssh-agent */
 
-int fd = -1;
-pid_t pid = -1;
+static int fd = -1;
+static pid_t pid = -1;
 
 static void
-send_msg(Buffer *m)
+send_msg(struct sshbuf *m)
 {
 	u_char buf[4];
-	int mlen = buffer_len(m);
+	size_t mlen = sshbuf_len(m);
+	int r;
 
-	put_u32(buf, mlen);
+	POKE_U32(buf, mlen);
 	if (atomicio(vwrite, fd, buf, 4) != 4 ||
-	    atomicio(vwrite, fd, buffer_ptr(m),
-	    buffer_len(m)) != buffer_len(m))
+	    atomicio(vwrite, fd, sshbuf_mutable_ptr(m),
+	    sshbuf_len(m)) != sshbuf_len(m))
 		error("write to helper failed");
-	buffer_consume(m, mlen);
+	if ((r = sshbuf_consume(m, mlen)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
 }
 
 static int
-recv_msg(Buffer *m)
+recv_msg(struct sshbuf *m)
 {
 	u_int l, len;
-	u_char buf[1024];
+	u_char c, buf[1024];
+	int r;
 
 	if ((len = atomicio(read, fd, buf, 4)) != 4) {
 		error("read from helper failed: %u", len);
 		return (0); /* XXX */
 	}
-	len = get_u32(buf);
+	len = PEEK_U32(buf);
 	if (len > 256 * 1024)
 		fatal("response too long: %u", len);
 	/* read len bytes into m */
-	buffer_clear(m);
+	sshbuf_reset(m);
 	while (len > 0) {
 		l = len;
 		if (l > sizeof(buf))
@@ -84,10 +92,13 @@ recv_msg(Buffer *m)
 			error("response from helper failed.");
 			return (0); /* XXX */
 		}
-		buffer_append(m, buf, l);
+		if ((r = sshbuf_put(m, buf, l)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
 		len -= l;
 	}
-	return (buffer_get_char(m));
+	if ((r = sshbuf_get_u8(m, &c)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	return c;
 }
 
 int
@@ -99,56 +110,161 @@ pkcs11_init(int interactive)
 void
 pkcs11_terminate(void)
 {
-	close(fd);
+	if (fd >= 0)
+		close(fd);
 }
 
 static int
-pkcs11_rsa_private_encrypt(int flen, const u_char *from, u_char *to, RSA *rsa,
-    int padding)
+rsa_encrypt(int flen, const u_char *from, u_char *to, RSA *rsa, int padding)
 {
-	Key key;
-	u_char *blob, *signature = NULL;
-	u_int blen, slen = 0;
-	int ret = -1;
-	Buffer msg;
+	struct sshkey *key = NULL;
+	struct sshbuf *msg = NULL;
+	u_char *blob = NULL, *signature = NULL;
+	size_t blen, slen = 0;
+	int r, ret = -1;
 
 	if (padding != RSA_PKCS1_PADDING)
-		return (-1);
-	key.type = KEY_RSA;
-	key.rsa = rsa;
-	if (key_to_blob(&key, &blob, &blen) == 0)
-		return -1;
-	buffer_init(&msg);
-	buffer_put_char(&msg, SSH2_AGENTC_SIGN_REQUEST);
-	buffer_put_string(&msg, blob, blen);
-	buffer_put_string(&msg, from, flen);
-	buffer_put_int(&msg, 0);
-	free(blob);
-	send_msg(&msg);
-	buffer_clear(&msg);
+		goto fail;
+	key = sshkey_new(KEY_UNSPEC);
+	if (key == NULL) {
+		error("%s: sshkey_new failed", __func__);
+		goto fail;
+	}
+	key->type = KEY_RSA;
+	RSA_up_ref(rsa);
+	key->rsa = rsa;
+	if ((r = sshkey_to_blob(key, &blob, &blen)) != 0) {
+		error("%s: sshkey_to_blob: %s", __func__, ssh_err(r));
+		goto fail;
+	}
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_put_u8(msg, SSH2_AGENTC_SIGN_REQUEST)) != 0 ||
+	    (r = sshbuf_put_string(msg, blob, blen)) != 0 ||
+	    (r = sshbuf_put_string(msg, from, flen)) != 0 ||
+	    (r = sshbuf_put_u32(msg, 0)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	send_msg(msg);
+	sshbuf_reset(msg);
 
-	if (recv_msg(&msg) == SSH2_AGENT_SIGN_RESPONSE) {
-		signature = buffer_get_string(&msg, &slen);
-		if (slen <= (u_int)RSA_size(rsa)) {
+	if (recv_msg(msg) == SSH2_AGENT_SIGN_RESPONSE) {
+		if ((r = sshbuf_get_string(msg, &signature, &slen)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
+		if (slen <= (size_t)RSA_size(rsa)) {
 			memcpy(to, signature, slen);
 			ret = slen;
 		}
 		free(signature);
 	}
-	buffer_free(&msg);
+ fail:
+	free(blob);
+	sshkey_free(key);
+	sshbuf_free(msg);
 	return (ret);
 }
 
-/* redirect the private key encrypt operation to the ssh-pkcs11-helper */
+#ifdef HAVE_EC_KEY_METHOD_NEW
+static ECDSA_SIG *
+ecdsa_do_sign(const unsigned char *dgst, int dgst_len, const BIGNUM *inv,
+    const BIGNUM *rp, EC_KEY *ec)
+{
+	struct sshkey *key = NULL;
+	struct sshbuf *msg = NULL;
+	ECDSA_SIG *ret = NULL;
+	const u_char *cp;
+	u_char *blob = NULL, *signature = NULL;
+	size_t blen, slen = 0;
+	int r, nid;
+
+	nid = sshkey_ecdsa_key_to_nid(ec);
+	if (nid < 0) {
+		error("%s: couldn't get curve nid", __func__);
+		goto fail;
+	}
+
+	key = sshkey_new(KEY_UNSPEC);
+	if (key == NULL) {
+		error("%s: sshkey_new failed", __func__);
+		goto fail;
+	}
+	key->ecdsa = ec;
+	key->ecdsa_nid = nid;
+	key->type = KEY_ECDSA;
+	EC_KEY_up_ref(ec);
+
+	if ((r = sshkey_to_blob(key, &blob, &blen)) != 0) {
+		error("%s: sshkey_to_blob: %s", __func__, ssh_err(r));
+		goto fail;
+	}
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_put_u8(msg, SSH2_AGENTC_SIGN_REQUEST)) != 0 ||
+	    (r = sshbuf_put_string(msg, blob, blen)) != 0 ||
+	    (r = sshbuf_put_string(msg, dgst, dgst_len)) != 0 ||
+	    (r = sshbuf_put_u32(msg, 0)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	send_msg(msg);
+	sshbuf_reset(msg);
+
+	if (recv_msg(msg) == SSH2_AGENT_SIGN_RESPONSE) {
+		if ((r = sshbuf_get_string(msg, &signature, &slen)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
+		cp = signature;
+		ret = d2i_ECDSA_SIG(NULL, &cp, slen);
+		free(signature);
+	}
+
+ fail:
+	free(blob);
+	sshkey_free(key);
+	sshbuf_free(msg);
+	return (ret);
+}
+#endif /* HAVE_EC_KEY_METHOD_NEW */
+
+static RSA_METHOD	*helper_rsa;
+#ifdef HAVE_EC_KEY_METHOD_NEW
+static EC_KEY_METHOD	*helper_ecdsa;
+#endif /* HAVE_EC_KEY_METHOD_NEW */
+
+/* redirect private key crypto operations to the ssh-pkcs11-helper */
+static void
+wrap_key(struct sshkey *k)
+{
+	if (k->type == KEY_RSA)
+		RSA_set_method(k->rsa, helper_rsa);
+#ifdef HAVE_EC_KEY_METHOD_NEW
+	else if (k->type == KEY_ECDSA)
+		EC_KEY_set_method(k->ecdsa, helper_ecdsa);
+#endif /* HAVE_EC_KEY_METHOD_NEW */
+	else
+		fatal("%s: unknown key type", __func__);
+}
+
 static int
-wrap_key(RSA *rsa)
+pkcs11_start_helper_methods(void)
 {
-	static RSA_METHOD helper_rsa;
+	if (helper_rsa != NULL)
+		return (0);
+
+#ifdef HAVE_EC_KEY_METHOD_NEW
+	int (*orig_sign)(int, const unsigned char *, int, unsigned char *,
+	    unsigned int *, const BIGNUM *, const BIGNUM *, EC_KEY *) = NULL;
+	if (helper_ecdsa != NULL)
+		return (0);
+	helper_ecdsa = EC_KEY_METHOD_new(EC_KEY_OpenSSL());
+	if (helper_ecdsa == NULL)
+		return (-1);
+	EC_KEY_METHOD_get_sign(helper_ecdsa, &orig_sign, NULL, NULL);
+	EC_KEY_METHOD_set_sign(helper_ecdsa, orig_sign, NULL, ecdsa_do_sign);
+#endif /* HAVE_EC_KEY_METHOD_NEW */
+
+	if ((helper_rsa = RSA_meth_dup(RSA_get_default_method())) == NULL)
+		fatal("%s: RSA_meth_dup failed", __func__);
+	if (!RSA_meth_set1_name(helper_rsa, "ssh-pkcs11-helper") ||
+	    !RSA_meth_set_priv_enc(helper_rsa, rsa_encrypt))
+		fatal("%s: failed to prepare method", __func__);
 
-	memcpy(&helper_rsa, RSA_get_default_method(), sizeof(helper_rsa));
-	helper_rsa.name = "ssh-pkcs11-helper";
-	helper_rsa.rsa_priv_enc = pkcs11_rsa_private_encrypt;
-	RSA_set_method(rsa, &helper_rsa);
 	return (0);
 }
 
@@ -156,6 +272,15 @@ static int
 pkcs11_start_helper(void)
 {
 	int pair[2];
+	char *helper, *verbosity = NULL;
+
+	if (log_level_get() >= SYSLOG_LEVEL_DEBUG1)
+		verbosity = "-vvv";
+
+	if (pkcs11_start_helper_methods() == -1) {
+		error("pkcs11_start_helper_methods failed");
+		return (-1);
+	}
 
 	if (socketpair(AF_UNIX, SOCK_STREAM, 0, pair) == -1) {
 		error("socketpair: %s", strerror(errno));
@@ -172,10 +297,13 @@ pkcs11_start_helper(void)
 		}
 		close(pair[0]);
 		close(pair[1]);
-		execlp(_PATH_SSH_PKCS11_HELPER, _PATH_SSH_PKCS11_HELPER,
-		    (char *) 0);
-		fprintf(stderr, "exec: %s: %s\n", _PATH_SSH_PKCS11_HELPER,
-		    strerror(errno));
+		helper = getenv("SSH_PKCS11_HELPER");
+		if (helper == NULL || strlen(helper) == 0)
+			helper = _PATH_SSH_PKCS11_HELPER;
+		debug("%s: starting %s %s", __func__, helper,
+		    verbosity == NULL ? "" : verbosity);
+		execlp(helper, helper, verbosity, (char *)NULL);
+		fprintf(stderr, "exec: %s: %s\n", helper, strerror(errno));
 		_exit(1);
 	}
 	close(pair[1]);
@@ -184,58 +312,80 @@ pkcs11_start_helper(void)
 }
 
 int
-pkcs11_add_provider(char *name, char *pin, Key ***keysp)
+pkcs11_add_provider(char *name, char *pin, struct sshkey ***keysp,
+    char ***labelsp)
 {
-	Key *k;
-	int i, nkeys;
+	struct sshkey *k;
+	int r, type;
 	u_char *blob;
-	u_int blen;
-	Buffer msg;
+	char *label;
+	size_t blen;
+	u_int nkeys, i;
+	struct sshbuf *msg;
 
 	if (fd < 0 && pkcs11_start_helper() < 0)
 		return (-1);
 
-	buffer_init(&msg);
-	buffer_put_char(&msg, SSH_AGENTC_ADD_SMARTCARD_KEY);
-	buffer_put_cstring(&msg, name);
-	buffer_put_cstring(&msg, pin);
-	send_msg(&msg);
-	buffer_clear(&msg);
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_put_u8(msg, SSH_AGENTC_ADD_SMARTCARD_KEY)) != 0 ||
+	    (r = sshbuf_put_cstring(msg, name)) != 0 ||
+	    (r = sshbuf_put_cstring(msg, pin)) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	send_msg(msg);
+	sshbuf_reset(msg);
 
-	if (recv_msg(&msg) == SSH2_AGENT_IDENTITIES_ANSWER) {
-		nkeys = buffer_get_int(&msg);
-		*keysp = xcalloc(nkeys, sizeof(Key *));
+	type = recv_msg(msg);
+	if (type == SSH2_AGENT_IDENTITIES_ANSWER) {
+		if ((r = sshbuf_get_u32(msg, &nkeys)) != 0)
+			fatal("%s: buffer error: %s", __func__, ssh_err(r));
+		*keysp = xcalloc(nkeys, sizeof(struct sshkey *));
+		if (labelsp)
+			*labelsp = xcalloc(nkeys, sizeof(char *));
 		for (i = 0; i < nkeys; i++) {
-			blob = buffer_get_string(&msg, &blen);
-			free(buffer_get_string(&msg, NULL));
-			k = key_from_blob(blob, blen);
-			wrap_key(k->rsa);
+			/* XXX clean up properly instead of fatal() */
+			if ((r = sshbuf_get_string(msg, &blob, &blen)) != 0 ||
+			    (r = sshbuf_get_cstring(msg, &label, NULL)) != 0)
+				fatal("%s: buffer error: %s",
+				    __func__, ssh_err(r));
+			if ((r = sshkey_from_blob(blob, blen, &k)) != 0)
+				fatal("%s: bad key: %s", __func__, ssh_err(r));
+			wrap_key(k);
 			(*keysp)[i] = k;
+			if (labelsp)
+				(*labelsp)[i] = label;
+			else
+				free(label);
 			free(blob);
 		}
+	} else if (type == SSH2_AGENT_FAILURE) {
+		if ((r = sshbuf_get_u32(msg, &nkeys)) != 0)
+			nkeys = -1;
 	} else {
 		nkeys = -1;
 	}
-	buffer_free(&msg);
+	sshbuf_free(msg);
 	return (nkeys);
 }
 
 int
 pkcs11_del_provider(char *name)
 {
-	int ret = -1;
-	Buffer msg;
+	int r, ret = -1;
+	struct sshbuf *msg;
 
-	buffer_init(&msg);
-	buffer_put_char(&msg, SSH_AGENTC_REMOVE_SMARTCARD_KEY);
-	buffer_put_cstring(&msg, name);
-	buffer_put_cstring(&msg, "");
-	send_msg(&msg);
-	buffer_clear(&msg);
+	if ((msg = sshbuf_new()) == NULL)
+		fatal("%s: sshbuf_new failed", __func__);
+	if ((r = sshbuf_put_u8(msg, SSH_AGENTC_REMOVE_SMARTCARD_KEY)) != 0 ||
+	    (r = sshbuf_put_cstring(msg, name)) != 0 ||
+	    (r = sshbuf_put_cstring(msg, "")) != 0)
+		fatal("%s: buffer error: %s", __func__, ssh_err(r));
+	send_msg(msg);
+	sshbuf_reset(msg);
 
-	if (recv_msg(&msg) == SSH_AGENT_SUCCESS)
+	if (recv_msg(msg) == SSH_AGENT_SUCCESS)
 		ret = 0;
-	buffer_free(&msg);
+	sshbuf_free(msg);
 	return (ret);
 }