From bd636f40911094a39c2920bf87d2ec340533c152 Mon Sep 17 00:00:00 2001 From: "djm@openbsd.org" Date: Sun, 7 May 2017 23:15:59 +0000 Subject: [PATCH] upstream commit Refuse RSA keys <1024 bits in length. Improve reporting for keys that do not meet this requirement. ok markus@ Upstream-ID: b385e2a7b13b1484792ee681daaf79e1e203df6c --- ssh-keygen.c | 24 ++++++++++++++++-------- ssh-rsa.c | 10 ++++++---- ssh.h | 5 +---- ssherr.c | 4 +++- ssherr.h | 3 ++- sshkey.c | 29 ++++++++++++++++++++++++----- sshkey.h | 4 ++-- 7 files changed, 54 insertions(+), 25 deletions(-) diff --git a/ssh-keygen.c b/ssh-keygen.c index 51c24bc55..7886582d7 100644 --- a/ssh-keygen.c +++ b/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 * Copyright (c) 1994 Tatu Ylonen , Espoo, Finland @@ -226,13 +226,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/ssh-rsa.c b/ssh-rsa.c index cde05df10..e8acc01fa 100644 --- a/ssh-rsa.c +++ b/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 * @@ -99,9 +99,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; @@ -172,9 +173,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/ssh.h b/ssh.h index 08d05ce29..12d800922 100644 --- a/ssh.h +++ b/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 @@ -98,8 +98,5 @@ #define SSH_PRIVSEP_USER "sshd" #endif -/* 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/ssherr.c b/ssherr.c index 680207063..4bd5f59cc 100644 --- a/ssherr.c +++ b/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/ssherr.h b/ssherr.h index 6f771b4b7..a30781620 100644 --- a/ssherr.h +++ b/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/sshkey.c b/sshkey.c index 0f6468197..6518c6f0b 100644 --- a/sshkey.c +++ b/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. @@ -1392,10 +1392,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; @@ -1423,8 +1424,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; @@ -1876,6 +1879,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 @@ -2643,6 +2650,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 || @@ -2653,6 +2664,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: @@ -3427,6 +3442,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/sshkey.h b/sshkey.h index fc1956605..b0b5b2749 100644 --- a/sshkey.h +++ b/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. @@ -46,7 +46,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;