Refuse RSA keys <1024 bits in length. Improve reporting for keys that

do not meet this requirement. ok markus@

7 files changed, 54 insertions, 25 deletions

diff --git a/usr.bin/ssh/ssh-keygen.c b/usr.bin/ssh/ssh-keygen.c
index d77609c766e..c9b7afa6289 100644
--- a/usr.bin/ssh/ssh-keygen.c
+++ b/usr.bin/ssh/ssh-keygen.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-keygen.c,v 1.302 2017/04/30 23:18:44 djm Exp $ */
+/* $OpenBSD: ssh-keygen.c,v 1.303 2017/05/07 23:15:59 djm Exp $ */
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
  * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -220,13 +220,21 @@ type_bits_valid(int type, const char *name, u_int32_t *bitsp)
 	    OPENSSL_DSA_MAX_MODULUS_BITS : OPENSSL_RSA_MAX_MODULUS_BITS;
 	if (*bitsp > maxbits)
 		fatal("key bits exceeds maximum %d", maxbits);
-	if (type == KEY_DSA && *bitsp != 1024)
-		fatal("DSA keys must be 1024 bits");
-	else if (type != KEY_ECDSA && type != KEY_ED25519 && *bitsp < 1024)
-		fatal("Key must at least be 1024 bits");
-	else if (type == KEY_ECDSA && sshkey_ecdsa_bits_to_nid(*bitsp) == -1)
-		fatal("Invalid ECDSA key length - valid lengths are "
-		    "256, 384 or 521 bits");
+	switch (type) {
+	case KEY_DSA:
+		if (*bitsp != 1024)
+			fatal("Invalid DSA key length: must be 1024 bits");
+		break;
+	case KEY_RSA:
+		if (*bitsp < SSH_RSA_MINIMUM_MODULUS_SIZE)
+			fatal("Invalid RSA key length: minimum is %d bits",
+			    SSH_RSA_MINIMUM_MODULUS_SIZE);
+		break;
+	case KEY_ECDSA:
+		if (sshkey_ecdsa_bits_to_nid(*bitsp) == -1)
+			fatal("Invalid ECDSA key length: valid lengths are "
+			    "256, 384 or 521 bits");
+	}
 #endif
 }
 
diff --git a/usr.bin/ssh/ssh-rsa.c b/usr.bin/ssh/ssh-rsa.c
index ed881edaab0..30a0ec38575 100644
--- a/usr.bin/ssh/ssh-rsa.c
+++ b/usr.bin/ssh/ssh-rsa.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh-rsa.c,v 1.60 2016/09/12 23:39:34 djm Exp $ */
+/* $OpenBSD: ssh-rsa.c,v 1.61 2017/05/07 23:15:59 djm Exp $ */
 /*
  * Copyright (c) 2000, 2003 Markus Friedl <markus@openbsd.org>
  *
@@ -94,9 +94,10 @@ ssh_rsa_sign(const struct sshkey *key, u_char **sigp, size_t *lenp,
 	else
 		hash_alg = rsa_hash_alg_from_ident(alg_ident);
 	if (key == NULL || key->rsa == NULL || hash_alg == -1 ||
-	    sshkey_type_plain(key->type) != KEY_RSA ||
-	    BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE)
+	    sshkey_type_plain(key->type) != KEY_RSA)
 		return SSH_ERR_INVALID_ARGUMENT;
+	if (BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE)
+		return SSH_ERR_KEY_LENGTH;
 	slen = RSA_size(key->rsa);
 	if (slen <= 0 || slen > SSHBUF_MAX_BIGNUM)
 		return SSH_ERR_INVALID_ARGUMENT;
@@ -167,9 +168,10 @@ ssh_rsa_verify(const struct sshkey *key,
 
 	if (key == NULL || key->rsa == NULL ||
 	    sshkey_type_plain(key->type) != KEY_RSA ||
-	    BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE ||
 	    sig == NULL || siglen == 0)
 		return SSH_ERR_INVALID_ARGUMENT;
+	if (BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE)
+		return SSH_ERR_KEY_LENGTH;
 
 	if ((b = sshbuf_from(sig, siglen)) == NULL)
 		return SSH_ERR_ALLOC_FAIL;
diff --git a/usr.bin/ssh/ssh.h b/usr.bin/ssh/ssh.h
index d562405a0a2..d34454b3096 100644
--- a/usr.bin/ssh/ssh.h
+++ b/usr.bin/ssh/ssh.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: ssh.h,v 1.86 2017/05/03 21:08:09 naddy Exp $ */
+/* $OpenBSD: ssh.h,v 1.87 2017/05/07 23:15:59 djm Exp $ */
 
 /*
  * Author: Tatu Ylonen <ylo@cs.hut.fi>
@@ -96,8 +96,5 @@
  */
 #define SSH_PRIVSEP_USER		"sshd"
 
-/* Minimum modulus size (n) for RSA keys. */
-#define SSH_RSA_MINIMUM_MODULUS_SIZE	768
-
 /* Listen backlog for sshd, ssh-agent and forwarding sockets */
 #define SSH_LISTEN_BACKLOG		128
diff --git a/usr.bin/ssh/ssherr.c b/usr.bin/ssh/ssherr.c
index 68020706381..4bd5f59cc9c 100644
--- a/usr.bin/ssh/ssherr.c
+++ b/usr.bin/ssh/ssherr.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ssherr.c,v 1.5 2015/09/13 14:39:16 tim Exp $	*/
+/*	$OpenBSD: ssherr.c,v 1.6 2017/05/07 23:15:59 djm Exp $	*/
 /*
  * Copyright (c) 2011 Damien Miller
  *
@@ -135,6 +135,8 @@ ssh_err(int n)
 		return "Connection corrupted";
 	case SSH_ERR_PROTOCOL_ERROR:
 		return "Protocol error";
+	case SSH_ERR_KEY_LENGTH:
+		return "Invalid key length";
 	default:
 		return "unknown error";
 	}
diff --git a/usr.bin/ssh/ssherr.h b/usr.bin/ssh/ssherr.h
index 6f771b4b78b..a30781620fa 100644
--- a/usr.bin/ssh/ssherr.h
+++ b/usr.bin/ssh/ssherr.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: ssherr.h,v 1.3 2015/01/30 01:13:33 djm Exp $	*/
+/*	$OpenBSD: ssherr.h,v 1.4 2017/05/07 23:15:59 djm Exp $	*/
 /*
  * Copyright (c) 2011 Damien Miller
  *
@@ -77,6 +77,7 @@
 #define SSH_ERR_CONN_TIMEOUT			-53
 #define SSH_ERR_CONN_CORRUPT			-54
 #define SSH_ERR_PROTOCOL_ERROR			-55
+#define SSH_ERR_KEY_LENGTH			-56
 
 /* Translate a numeric error code to a human-readable error string */
 const char *ssh_err(int n);
diff --git a/usr.bin/ssh/sshkey.c b/usr.bin/ssh/sshkey.c
index c9f149cd765..7249229a0a1 100644
--- a/usr.bin/ssh/sshkey.c
+++ b/usr.bin/ssh/sshkey.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.c,v 1.48 2017/04/30 23:18:44 djm Exp $ */
+/* $OpenBSD: sshkey.c,v 1.49 2017/05/07 23:15:59 djm Exp $ */
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
  * Copyright (c) 2008 Alexander von Gernler.  All rights reserved.
@@ -1364,10 +1364,11 @@ rsa_generate_private_key(u_int bits, RSA **rsap)
 	BIGNUM *f4 = NULL;
 	int ret = SSH_ERR_INTERNAL_ERROR;
 
-	if (rsap == NULL ||
-	    bits < SSH_RSA_MINIMUM_MODULUS_SIZE ||
-	    bits > SSHBUF_MAX_BIGNUM * 8)
+	if (rsap == NULL)
 		return SSH_ERR_INVALID_ARGUMENT;
+	if (bits < SSH_RSA_MINIMUM_MODULUS_SIZE ||
+	    bits > SSHBUF_MAX_BIGNUM * 8)
+		return SSH_ERR_KEY_LENGTH;
 	*rsap = NULL;
 	if ((private = RSA_new()) == NULL || (f4 = BN_new()) == NULL) {
 		ret = SSH_ERR_ALLOC_FAIL;
@@ -1395,8 +1396,10 @@ dsa_generate_private_key(u_int bits, DSA **dsap)
 	DSA *private;
 	int ret = SSH_ERR_INTERNAL_ERROR;
 
-	if (dsap == NULL || bits != 1024)
+	if (dsap == NULL)
 		return SSH_ERR_INVALID_ARGUMENT;
+	if (bits != 1024)
+		return SSH_ERR_KEY_LENGTH;
 	if ((private = DSA_new()) == NULL) {
 		ret = SSH_ERR_ALLOC_FAIL;
 		goto out;
@@ -1840,6 +1843,10 @@ sshkey_from_blob_internal(struct sshbuf *b, struct sshkey **keyp,
 			ret = SSH_ERR_INVALID_FORMAT;
 			goto out;
 		}
+		if (BN_num_bits(key->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) {
+			ret = SSH_ERR_KEY_LENGTH;
+			goto out;
+		}
 #ifdef DEBUG_PK
 		RSA_print_fp(stderr, key->rsa, 8);
 #endif
@@ -2593,6 +2600,10 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
 		    (r = sshbuf_get_bignum2(buf, k->rsa->q)) != 0 ||
 		    (r = rsa_generate_additional_parameters(k->rsa)) != 0)
 			goto out;
+		if (BN_num_bits(k->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) {
+			r = SSH_ERR_KEY_LENGTH;
+			goto out;
+		}
 		break;
 	case KEY_RSA_CERT:
 		if ((r = sshkey_froms(buf, &k)) != 0 ||
@@ -2603,6 +2614,10 @@ sshkey_private_deserialize(struct sshbuf *buf, struct sshkey **kp)
 		    (r = sshbuf_get_bignum2(buf, k->rsa->q)) != 0 ||
 		    (r = rsa_generate_additional_parameters(k->rsa)) != 0)
 			goto out;
+		if (BN_num_bits(k->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) {
+			r = SSH_ERR_KEY_LENGTH;
+			goto out;
+		}
 		break;
 #endif /* WITH_OPENSSL */
 	case KEY_ED25519:
@@ -3373,6 +3388,10 @@ sshkey_parse_private_pem_fileblob(struct sshbuf *blob, int type,
 			r = SSH_ERR_LIBCRYPTO_ERROR;
 			goto out;
 		}
+		if (BN_num_bits(prv->rsa->n) < SSH_RSA_MINIMUM_MODULUS_SIZE) {
+			r = SSH_ERR_KEY_LENGTH;
+			goto out;
+		}
 	} else if (pk->type == EVP_PKEY_DSA &&
 	    (type == KEY_UNSPEC || type == KEY_DSA)) {
 		if ((prv = sshkey_new(KEY_UNSPEC)) == NULL) {
diff --git a/usr.bin/ssh/sshkey.h b/usr.bin/ssh/sshkey.h
index e143243a097..b760b9637d6 100644
--- a/usr.bin/ssh/sshkey.h
+++ b/usr.bin/ssh/sshkey.h
@@ -1,4 +1,4 @@
-/* $OpenBSD: sshkey.h,v 1.17 2017/05/03 21:08:09 naddy Exp $ */
+/* $OpenBSD: sshkey.h,v 1.18 2017/05/07 23:15:59 djm Exp $ */
 
 /*
  * Copyright (c) 2000, 2001 Markus Friedl.  All rights reserved.
@@ -40,7 +40,7 @@
 #define EC_POINT	void
 #endif /* WITH_OPENSSL */
 
-#define SSH_RSA_MINIMUM_MODULUS_SIZE	768
+#define SSH_RSA_MINIMUM_MODULUS_SIZE	1024
 #define SSH_KEY_MAX_SIGN_DATA_SIZE	(1 << 20)
 
 struct sshbuf;