upstream: Improve strictness and control over RSA-SHA2 signature

In ssh, when an agent fails to return a RSA-SHA2 signature when requested and falls back to RSA-SHA1 instead, retry the signature to ensure that the public key algorithm sent in the SSH_MSG_USERAUTH matches the one in the signature itself. In sshd, strictly enforce that the public key algorithm sent in the SSH_MSG_USERAUTH message matches what appears in the signature. Make the sshd_config PubkeyAcceptedKeyTypes and HostbasedAcceptedKeyTypes options control accepted signature algorithms (previously they selected supported key types). This allows these options to ban RSA-SHA1 in favour of RSA-SHA2. Add new signature algorithms "rsa-sha2-256-cert-v01@openssh.com" and "rsa-sha2-512-cert-v01@openssh.com" to force use of RSA-SHA2 signatures with certificate keys. feedback and ok markus@ OpenBSD-Commit-ID: c6e9f6d45eed8962ad502d315d7eaef32c419dde
openssh · Jul 3, 2018 · 4ba0d54 · 4ba0d54
1 parent 95344c2
commit 4ba0d54
Show file tree

Hide file tree

Showing 18 changed files with 469 additions and 256 deletions.
diff --git a/PROTOCOL.certkeys b/PROTOCOL.certkeys
@@ -25,6 +25,10 @@ raw user keys. The ssh client will support automatic verification of
 acceptance of certified host keys, by adding a similar ability to
 specify CA keys in ~/.ssh/known_hosts.
 
+All certificate types include certification information along with the
+public key that is used to sign challenges. In OpenSSH, ssh-keygen
+performs the CA signing operation.
+
 Certified keys are represented using new key types:
 
     ssh-rsa-cert-v01@openssh.com
@@ -33,9 +37,17 @@ Certified keys are represented using new key types:
     ecdsa-sha2-nistp384-cert-v01@openssh.com
     ecdsa-sha2-nistp521-cert-v01@openssh.com
 
-These include certification information along with the public key
-that is used to sign challenges. ssh-keygen performs the CA signing
-operation.
+Two additional types exist for RSA certificates to force use of
+SHA-2 signatures (SHA-256 and SHA-512 respectively):
+
+    rsa-sha2-256-cert-v01@openssh.com
+    rsa-sha2-512-cert-v01@openssh.com
+
+These RSA/SHA-2 types should not appear in keys at rest or transmitted
+on their wire, but do appear in a SSH_MSG_KEXINIT's host-key algorithms
+field or in the "public key algorithm name" field of a "publickey"
+SSH_USERAUTH_REQUEST to indicate that the signature will use the
+specified algorithm.
 
 Protocol extensions
 -------------------
@@ -291,4 +303,4 @@ permit-user-rc          empty         Flag indicating that execution of
                                       of this script will not be permitted if
                                       this option is not present.
 
-$OpenBSD: PROTOCOL.certkeys,v 1.14 2018/04/10 00:10:49 djm Exp $
+$OpenBSD: PROTOCOL.certkeys,v 1.15 2018/07/03 11:39:54 djm Exp $
diff --git a/auth2-hostbased.c b/auth2-hostbased.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-hostbased.c,v 1.33 2018/01/23 05:27:21 djm Exp $ */
+/* $OpenBSD: auth2-hostbased.c,v 1.34 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -111,8 +111,7 @@ userauth_hostbased(struct ssh *ssh)
 		    "signature format");
 		goto done;
 	}
-	if (match_pattern_list(sshkey_ssh_name(key),
-	    options.hostbased_key_types, 0) != 1) {
+	if (match_pattern_list(pkalg, options.hostbased_key_types, 0) != 1) {
 		logit("%s: key type %s not in HostbasedAcceptedKeyTypes",
 		    __func__, sshkey_type(key));
 		goto done;

diff --git a/auth2-pubkey.c b/auth2-pubkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: auth2-pubkey.c,v 1.79 2018/06/06 18:29:18 markus Exp $ */
+/* $OpenBSD: auth2-pubkey.c,v 1.80 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
  *
@@ -109,7 +109,7 @@ userauth_pubkey(struct ssh *ssh)
 	pktype = sshkey_type_from_name(pkalg);
 	if (pktype == KEY_UNSPEC) {
 		/* this is perfectly legal */
-		logit("%s: unsupported public key algorithm: %s",
+		verbose("%s: unsupported public key algorithm: %s",
 		    __func__, pkalg);
 		goto done;
 	}
@@ -136,8 +136,7 @@ userauth_pubkey(struct ssh *ssh)
 		logit("refusing previously-used %s key", sshkey_type(key));
 		goto done;
 	}
-	if (match_pattern_list(sshkey_ssh_name(key),
-	    options.pubkey_key_types, 0) != 1) {
+	if (match_pattern_list(pkalg, options.pubkey_key_types, 0) != 1) {
 		logit("%s: key type %s not in PubkeyAcceptedKeyTypes",
 		    __func__, sshkey_ssh_name(key));
 		goto done;
@@ -188,8 +187,10 @@ userauth_pubkey(struct ssh *ssh)
 		/* test for correct signature */
 		authenticated = 0;
 		if (PRIVSEP(user_key_allowed(ssh, pw, key, 1, &authopts)) &&
-		    PRIVSEP(sshkey_verify(key, sig, slen, sshbuf_ptr(b),
-		    sshbuf_len(b), NULL, ssh->compat)) == 0) {
+		    PRIVSEP(sshkey_verify(key, sig, slen,
+		    sshbuf_ptr(b), sshbuf_len(b),
+		    (ssh->compat & SSH_BUG_SIGTYPE) == 0 ? pkalg : NULL,
+		    ssh->compat)) == 0) {
 			authenticated = 1;
 		}
 		sshbuf_free(b);

diff --git a/authfd.c b/authfd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: authfd.c,v 1.109 2018/04/10 00:10:49 djm Exp $ */
+/* $OpenBSD: authfd.c,v 1.110 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -343,8 +343,8 @@ ssh_agent_sign(int sock, const struct sshkey *key,
     const u_char *data, size_t datalen, const char *alg, u_int compat)
 {
 	struct sshbuf *msg;
-	u_char *blob = NULL, type;
-	size_t blen = 0, len = 0;
+	u_char *sig = NULL, type = 0;
+	size_t len = 0;
 	u_int flags = 0;
 	int r = SSH_ERR_INTERNAL_ERROR;
 
@@ -355,11 +355,9 @@ ssh_agent_sign(int sock, const struct sshkey *key,
 		return SSH_ERR_INVALID_ARGUMENT;
 	if ((msg = sshbuf_new()) == NULL)
 		return SSH_ERR_ALLOC_FAIL;
-	if ((r = sshkey_to_blob(key, &blob, &blen)) != 0)
-		goto out;
 	flags |= agent_encode_alg(key, alg);
 	if ((r = sshbuf_put_u8(msg, SSH2_AGENTC_SIGN_REQUEST)) != 0 ||
-	    (r = sshbuf_put_string(msg, blob, blen)) != 0 ||
+	    (r = sshkey_puts(key, msg)) != 0 ||
 	    (r = sshbuf_put_string(msg, data, datalen)) != 0 ||
 	    (r = sshbuf_put_u32(msg, flags)) != 0)
 		goto out;
@@ -374,15 +372,19 @@ ssh_agent_sign(int sock, const struct sshkey *key,
 		r = SSH_ERR_INVALID_FORMAT;
 		goto out;
 	}
-	if ((r = sshbuf_get_string(msg, sigp, &len)) != 0)
+	if ((r = sshbuf_get_string(msg, &sig, &len)) != 0)
+		goto out;
+	/* Check what we actually got back from the agent. */
+	if ((r = sshkey_check_sigtype(sig, len, alg)) != 0)
 		goto out;
+	/* success */
+	*sigp = sig;
 	*lenp = len;
+	sig = NULL;
+	len = 0;
 	r = 0;
  out:
-	if (blob != NULL) {
-		explicit_bzero(blob, blen);
-		free(blob);
-	}
+	freezero(sig, len);
 	sshbuf_free(msg);
 	return r;
 }

diff --git a/compat.c b/compat.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.c,v 1.107 2018/04/16 22:50:44 djm Exp $ */
+/* $OpenBSD: compat.c,v 1.108 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Copyright (c) 1999, 2000, 2001, 2002 Markus Friedl.  All rights reserved.
  *
@@ -52,16 +52,27 @@ compat_datafellows(const char *version)
 	} check[] = {
 		{ "OpenSSH_2.*,"
 		  "OpenSSH_3.0*,"
-		  "OpenSSH_3.1*",	SSH_BUG_EXTEOF|SSH_OLD_FORWARD_ADDR},
-		{ "OpenSSH_3.*",	SSH_OLD_FORWARD_ADDR },
-		{ "Sun_SSH_1.0*",	SSH_BUG_NOREKEY|SSH_BUG_EXTEOF},
+		  "OpenSSH_3.1*",	SSH_BUG_EXTEOF|SSH_OLD_FORWARD_ADDR|
+					SSH_BUG_SIGTYPE},
+		{ "OpenSSH_3.*",	SSH_OLD_FORWARD_ADDR|SSH_BUG_SIGTYPE },
+		{ "Sun_SSH_1.0*",	SSH_BUG_NOREKEY|SSH_BUG_EXTEOF|
+					SSH_BUG_SIGTYPE},
 		{ "OpenSSH_2*,"
 		  "OpenSSH_3*,"
-		  "OpenSSH_4*",		0 },
-		{ "OpenSSH_5*",		SSH_NEW_OPENSSH|SSH_BUG_DYNAMIC_RPORT},
-		{ "OpenSSH_6.6.1*",	SSH_NEW_OPENSSH},
+		  "OpenSSH_4*",		SSH_BUG_SIGTYPE },
+		{ "OpenSSH_5*",		SSH_NEW_OPENSSH|SSH_BUG_DYNAMIC_RPORT|
+					SSH_BUG_SIGTYPE},
+		{ "OpenSSH_6.6.1*",	SSH_NEW_OPENSSH|SSH_BUG_SIGTYPE},
 		{ "OpenSSH_6.5*,"
-		  "OpenSSH_6.6*",	SSH_NEW_OPENSSH|SSH_BUG_CURVE25519PAD},
+		  "OpenSSH_6.6*",	SSH_NEW_OPENSSH|SSH_BUG_CURVE25519PAD|
+					SSH_BUG_SIGTYPE},
+		{ "OpenSSH_7.0*,"
+		  "OpenSSH_7.1*,"
+		  "OpenSSH_7.2*,"
+		  "OpenSSH_7.3*,"
+		  "OpenSSH_7.4*,"
+		  "OpenSSH_7.5*,"
+		  "OpenSSH_7.6*",	SSH_NEW_OPENSSH|SSH_BUG_SIGTYPE},
 		{ "OpenSSH*",		SSH_NEW_OPENSSH },
 		{ "*MindTerm*",		0 },
 		{ "3.0.*",		SSH_BUG_DEBUG },

diff --git a/compat.h b/compat.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: compat.h,v 1.51 2018/02/16 04:43:11 dtucker Exp $ */
+/* $OpenBSD: compat.h,v 1.52 2018/07/03 11:39:54 djm Exp $ */
 
 /*
  * Copyright (c) 1999, 2000, 2001 Markus Friedl.  All rights reserved.
@@ -33,7 +33,7 @@
 #define	SSH_PROTO_2		0x04
 
 #define SSH_BUG_UTF8TTYMODE	0x00000001
-/* #define unused		0x00000002 */
+#define SSH_BUG_SIGTYPE		0x00000002
 /* #define unused		0x00000004 */
 /* #define unused		0x00000008 */
 #define SSH_OLD_SESSIONID	0x00000010

diff --git a/kex.c b/kex.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.c,v 1.136 2018/02/07 02:06:50 jsing Exp $ */
+/* $OpenBSD: kex.c,v 1.137 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  *
@@ -342,6 +342,7 @@ kex_send_ext_info(struct ssh *ssh)
 
 	if ((algs = sshkey_alg_list(0, 1, 1, ',')) == NULL)
 		return SSH_ERR_ALLOC_FAIL;
+	/* XXX filter algs list by allowed pubkey/hostbased types */
 	if ((r = sshpkt_start(ssh, SSH2_MSG_EXT_INFO)) != 0 ||
 	    (r = sshpkt_put_u32(ssh, 1)) != 0 ||
 	    (r = sshpkt_put_cstring(ssh, "server-sig-algs")) != 0 ||
@@ -378,7 +379,7 @@ kex_input_ext_info(int type, u_int32_t seq, struct ssh *ssh)
 {
 	struct kex *kex = ssh->kex;
 	u_int32_t i, ninfo;
-	char *name, *found;
+	char *name;
 	u_char *val;
 	size_t vlen;
 	int r;
@@ -401,16 +402,8 @@ kex_input_ext_info(int type, u_int32_t seq, struct ssh *ssh)
 				return SSH_ERR_INVALID_FORMAT;
 			}
 			debug("%s: %s=<%s>", __func__, name, val);
-			found = match_list("rsa-sha2-256", val, NULL);
-			if (found) {
-				kex->rsa_sha2 = 256;
-				free(found);
-			}
-			found = match_list("rsa-sha2-512", val, NULL);
-			if (found) {
-				kex->rsa_sha2 = 512;
-				free(found);
-			}
+			kex->server_sig_algs = val;
+			val = NULL;
 		} else
 			debug("%s: %s (unrecognised)", __func__, name);
 		free(name);

diff --git a/kex.h b/kex.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: kex.h,v 1.83 2017/05/30 14:23:52 markus Exp $ */
+/* $OpenBSD: kex.h,v 1.84 2018/07/03 11:39:54 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -139,7 +139,7 @@ struct kex {
 	int	hostkey_type;
 	int	hostkey_nid;
 	u_int	kex_type;
-	int	rsa_sha2;
+	char	*server_sig_algs;
 	int	ext_info_c;
 	struct sshbuf *my;
 	struct sshbuf *peer;

diff --git a/myproposal.h b/myproposal.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: myproposal.h,v 1.55 2017/05/07 23:13:42 djm Exp $ */
+/* $OpenBSD: myproposal.h,v 1.56 2018/07/03 11:39:54 djm Exp $ */
 
 /*
  * Copyright (c) 2000 Markus Friedl.  All rights reserved.
@@ -107,6 +107,8 @@
 #define	KEX_DEFAULT_PK_ALG	\
 	HOSTKEY_ECDSA_CERT_METHODS \
 	"ssh-ed25519-cert-v01@openssh.com," \
+	"rsa-sha2-512-cert-v01@openssh.com," \
+	"rsa-sha2-256-cert-v01@openssh.com," \
 	"ssh-rsa-cert-v01@openssh.com," \
 	HOSTKEY_ECDSA_METHODS \
 	"ssh-ed25519," \

diff --git a/ssh-rsa.c b/ssh-rsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-rsa.c,v 1.66 2018/02/14 16:27:24 jsing Exp $ */
+/* $OpenBSD: ssh-rsa.c,v 1.67 2018/07/03 11:39:54 djm Exp $ */
 /*
  * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
  *
@@ -51,11 +51,14 @@ rsa_hash_alg_ident(int hash_alg)
 	return NULL;
 }
 
+/*
+ * Returns the hash algorithm ID for a given algorithm identifier as used
+ * inside the signature blob,
+ */
 static int
-rsa_hash_alg_from_ident(const char *ident)
+rsa_hash_id_from_ident(const char *ident)
 {
-	if (strcmp(ident, "ssh-rsa") == 0 ||
-	    strcmp(ident, "ssh-rsa-cert-v01@openssh.com") == 0)
+	if (strcmp(ident, "ssh-rsa") == 0)
 		return SSH_DIGEST_SHA1;
 	if (strcmp(ident, "rsa-sha2-256") == 0)
 		return SSH_DIGEST_SHA256;
@@ -64,6 +67,27 @@ rsa_hash_alg_from_ident(const char *ident)
 	return -1;
 }
 
+/*
+ * Return the hash algorithm ID for the specified key name. This includes
+ * all the cases of rsa_hash_id_from_ident() but also the certificate key
+ * types.
+ */
+static int
+rsa_hash_id_from_keyname(const char *alg)
+{
+	int r;
+
+	if ((r = rsa_hash_id_from_ident(alg)) != -1)
+		return r;
+	if (strcmp(alg, "ssh-rsa-cert-v01@openssh.com") == 0)
+		return SSH_DIGEST_SHA1;
+	if (strcmp(alg, "rsa-sha2-256-cert-v01@openssh.com") == 0)
+		return SSH_DIGEST_SHA256;
+	if (strcmp(alg, "rsa-sha2-512-cert-v01@openssh.com") == 0)
+		return SSH_DIGEST_SHA512;
+	return -1;
+}
+
 static int
 rsa_hash_alg_nid(int type)
 {
@@ -135,7 +159,7 @@ ssh_rsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
 	if (alg_ident == NULL || strlen(alg_ident) == 0)
 		hash_alg = SSH_DIGEST_SHA1;
 	else
-		hash_alg = rsa_hash_alg_from_ident(alg_ident);
+		hash_alg = rsa_hash_id_from_keyname(alg_ident);
 	if (key == NULL || key->rsa == NULL || hash_alg == -1 ||
 	    sshkey_type_plain(key->type) != KEY_RSA)
 		return SSH_ERR_INVALID_ARGUMENT;
@@ -202,7 +226,7 @@ ssh_rsa_verify(const struct sshkey *key,
     const char *alg)
 {
 	char *sigtype = NULL;
-	int hash_alg, ret = SSH_ERR_INTERNAL_ERROR;
+	int hash_alg, want_alg, ret = SSH_ERR_INTERNAL_ERROR;
 	size_t len = 0, diff, modlen, dlen;
 	struct sshbuf *b = NULL;
 	u_char digest[SSH_DIGEST_MAX_LENGTH], *osigblob, *sigblob = NULL;
@@ -220,18 +244,24 @@ ssh_rsa_verify(const struct sshkey *key,
 		ret = SSH_ERR_INVALID_FORMAT;
 		goto out;
 	}
-	/* XXX djm: need cert types that reliably yield SHA-2 signatures */
-	if (alg != NULL && strcmp(alg, sigtype) != 0 &&
-	    strcmp(alg, "ssh-rsa-cert-v01@openssh.com") != 0) {
-		error("%s: RSA signature type mismatch: "
-		    "expected %s received %s", __func__, alg, sigtype);
-		ret = SSH_ERR_SIGNATURE_INVALID;
-		goto out;
-	}
-	if ((hash_alg = rsa_hash_alg_from_ident(sigtype)) == -1) {
+	if ((hash_alg = rsa_hash_id_from_ident(sigtype)) == -1) {
 		ret = SSH_ERR_KEY_TYPE_MISMATCH;
 		goto out;
 	}
+	/*
+	 * Allow ssh-rsa-cert-v01 certs to generate SHA2 signatures for
+	 * legacy reasons, but otherwise the signature type should match.
+	 */
+	if (alg != NULL && strcmp(alg, "ssh-rsa-cert-v01@openssh.com") != 0) {
+		if ((want_alg = rsa_hash_id_from_keyname(alg)) == -1) {
+			ret = SSH_ERR_INVALID_ARGUMENT;
+			goto out;
+		}
+		if (hash_alg != want_alg) {
+			ret = SSH_ERR_SIGNATURE_INVALID;
+			goto out;
+		}
+	}
 	if (sshbuf_get_string(b, &sigblob, &len) != 0) {
 		ret = SSH_ERR_INVALID_FORMAT;
 		goto out;