From 041ab7c1e7d6514ed84a539a767f79ffb356e807 Mon Sep 17 00:00:00 2001 From: Damien Miller Date: Fri, 10 Sep 2010 11:23:34 +1000 Subject: [PATCH] - djm@cvs.openbsd.org 2010/09/09 10:45:45 [kex.c kex.h kexecdh.c key.c key.h monitor.c ssh-ecdsa.c] ECDH/ECDSA compliance fix: these methods vary the hash function they use (SHA256/384/512) depending on the length of the curve in use. The previous code incorrectly used SHA256 in all cases. This fix will cause authentication failure when using 384 or 521-bit curve keys if one peer hasn't been upgraded and the other has. (256-bit curve keys work ok). In particular you may need to specify HostkeyAlgorithms when connecting to a server that has not been upgraded from an upgraded client. ok naddy@ --- ChangeLog | 13 +++++++++++++ kex.c | 10 +++++----- kex.h | 5 +++-- kexecdh.c | 14 +++++++++++--- key.c | 47 +++++++++++++++++++++++++++++++++++------------ key.h | 4 +++- monitor.c | 8 ++++---- ssh-ecdsa.c | 10 ++++++---- 8 files changed, 80 insertions(+), 31 deletions(-) diff --git a/ChangeLog b/ChangeLog index 32f82369d..87fee3bf0 100644 --- a/ChangeLog +++ b/ChangeLog @@ -49,6 +49,19 @@ gcc, at least in earlier versions, but this does not forgive your current transgressions) seen between zlib and openssl ok djm + - djm@cvs.openbsd.org 2010/09/09 10:45:45 + [kex.c kex.h kexecdh.c key.c key.h monitor.c ssh-ecdsa.c] + ECDH/ECDSA compliance fix: these methods vary the hash function they use + (SHA256/384/512) depending on the length of the curve in use. The previous + code incorrectly used SHA256 in all cases. + + This fix will cause authentication failure when using 384 or 521-bit curve + keys if one peer hasn't been upgraded and the other has. (256-bit curve + keys work ok). In particular you may need to specify HostkeyAlgorithms + when connecting to a server that has not been upgraded from an upgraded + client. + + ok naddy@ 20100831 - OpenBSD CVS Sync diff --git a/kex.c b/kex.c index abe9b9f5d..7c8763191 100644 --- a/kex.c +++ b/kex.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kex.c,v 1.84 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: kex.c,v 1.85 2010/09/09 10:45:45 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. * @@ -325,10 +325,10 @@ choose_kex(Kex *k, char *client, char *server) } else if (strcmp(k->name, KEX_DHGEX_SHA256) == 0) { k->kex_type = KEX_DH_GEX_SHA256; k->evp_md = evp_ssh_sha256(); - } else if (strncmp(k->name, KEX_ECDH_SHA256, - sizeof(KEX_ECDH_SHA256) - 1) == 0) { - k->kex_type = KEX_ECDH_SHA2; - k->evp_md = evp_ssh_sha256(); + } else if (strncmp(k->name, KEX_ECDH_SHA2_STEM, + sizeof(KEX_ECDH_SHA2_STEM) - 1) == 0) { + k->kex_type = KEX_ECDH_SHA2; + k->evp_md = kex_ecdh_name_to_evpmd(k->name); #endif } else fatal("bad kex alg %s", k->name); diff --git a/kex.h b/kex.h index a183ffda2..f5dcc8791 100644 --- a/kex.h +++ b/kex.h @@ -1,4 +1,4 @@ -/* $OpenBSD: kex.h,v 1.50 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: kex.h,v 1.51 2010/09/09 10:45:45 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -39,7 +39,7 @@ #define KEX_DHGEX_SHA256 "diffie-hellman-group-exchange-sha256" #define KEX_RESUME "resume@appgate.com" /* The following represents the family of ECDH methods */ -#define KEX_ECDH_SHA256 "ecdh-sha2-" +#define KEX_ECDH_SHA2_STEM "ecdh-sha2-" #define COMP_NONE 0 #define COMP_ZLIB 1 @@ -165,6 +165,7 @@ kex_ecdh_hash(const EVP_MD *, const EC_GROUP *, char *, char *, char *, int, const BIGNUM *, u_char **, u_int *); int kex_ecdh_name_to_nid(const char *); +const EVP_MD *kex_ecdh_name_to_evpmd(const char *); void derive_ssh1_session_id(BIGNUM *, BIGNUM *, u_int8_t[8], u_int8_t[16]); diff --git a/kexecdh.c b/kexecdh.c index bd5718136..f59d7b903 100644 --- a/kexecdh.c +++ b/kexecdh.c @@ -1,4 +1,4 @@ -/* $OpenBSD: kexecdh.c,v 1.1 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: kexecdh.c,v 1.2 2010/09/09 10:45:45 djm Exp $ */ /* * Copyright (c) 2001 Markus Friedl. All rights reserved. * Copyright (c) 2010 Damien Miller. All rights reserved. @@ -48,15 +48,23 @@ kex_ecdh_name_to_nid(const char *kexname) { int ret; - if (strlen(kexname) < sizeof(KEX_ECDH_SHA256) - 1) + if (strlen(kexname) < sizeof(KEX_ECDH_SHA2_STEM) - 1) fatal("%s: kexname too short \"%s\"", __func__, kexname); - ret = key_curve_name_to_nid(kexname + sizeof(KEX_ECDH_SHA256) - 1); + ret = key_curve_name_to_nid(kexname + sizeof(KEX_ECDH_SHA2_STEM) - 1); if (ret == -1) fatal("%s: unsupported curve negotiated \"%s\"", __func__, kexname); return ret; } +const EVP_MD * +kex_ecdh_name_to_evpmd(const char *kexname) +{ + int nid = kex_ecdh_name_to_nid(kexname); + + return key_ec_nid_to_evpmd(nid); +} + void kex_ecdh_hash( const EVP_MD *evp_md, diff --git a/key.c b/key.c index 842280a9f..b9dc2355b 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.92 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: key.c,v 1.93 2010/09/09 10:45:45 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -978,17 +978,7 @@ key_size(const Key *k) return BN_num_bits(k->dsa->p); case KEY_ECDSA: case KEY_ECDSA_CERT: - switch (k->ecdsa_nid) { - case NID_X9_62_prime256v1: - return 256; - case NID_secp384r1: - return 384; - case NID_secp521r1: - return 521; - default: - break; - } - break; + return key_curve_nid_to_bits(k->ecdsa_nid); } return 0; } @@ -1961,6 +1951,7 @@ key_cert_is_legacy(Key *k) } } +/* XXX: these are really begging for a table-driven approach */ int key_curve_name_to_nid(const char *name) { @@ -1975,6 +1966,22 @@ key_curve_name_to_nid(const char *name) return -1; } +u_int +key_curve_nid_to_bits(int nid) +{ + switch (nid) { + case NID_X9_62_prime256v1: + return 256; + case NID_secp384r1: + return 384; + case NID_secp521r1: + return 521; + default: + error("%s: unsupported EC curve nid %d", __func__, nid); + return 0; + } +} + const char * key_curve_nid_to_name(int nid) { @@ -1989,6 +1996,22 @@ key_curve_nid_to_name(int nid) return NULL; } +const EVP_MD * +key_ec_nid_to_evpmd(int nid) +{ + int kbits = key_curve_nid_to_bits(nid); + + if (kbits == 0) + fatal("%s: invalid nid %d", __func__, nid); + /* RFC5656 section 6.2.1 */ + if (kbits <= 256) + return EVP_sha256(); + else if (kbits <= 384) + return EVP_sha384(); + else + return EVP_sha512(); +} + int key_ec_validate_public(const EC_GROUP *group, const EC_POINT *public) { diff --git a/key.h b/key.h index 2eb124364..ba1a20c07 100644 --- a/key.h +++ b/key.h @@ -1,4 +1,4 @@ -/* $OpenBSD: key.h,v 1.31 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: key.h,v 1.32 2010/09/09 10:45:45 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -112,8 +112,10 @@ int key_cert_is_legacy(Key *); int key_ecdsa_nid_from_name(const char *); int key_curve_name_to_nid(const char *); const char * key_curve_nid_to_name(int); +u_int key_curve_nid_to_bits(int); int key_ecdsa_bits_to_nid(int); int key_ecdsa_group_to_nid(const EC_GROUP *); +const EVP_MD * key_ec_nid_to_evpmd(int nid); int key_ec_validate_public(const EC_GROUP *, const EC_POINT *); int key_ec_validate_private(const EC_KEY *); diff --git a/monitor.c b/monitor.c index 32395ee44..29d987c70 100644 --- a/monitor.c +++ b/monitor.c @@ -1,4 +1,4 @@ -/* $OpenBSD: monitor.c,v 1.109 2010/08/31 11:54:45 djm Exp $ */ +/* $OpenBSD: monitor.c,v 1.110 2010/09/09 10:45:45 djm Exp $ */ /* * Copyright 2002 Niels Provos * Copyright 2002 Markus Friedl @@ -590,10 +590,10 @@ mm_answer_sign(int sock, Buffer *m) p = buffer_get_string(m, &datlen); /* - * Supported KEX types will only return SHA1 (20 byte) or - * SHA256 (32 byte) hashes + * Supported KEX types use SHA1 (20 bytes), SHA256 (32 bytes), + * SHA384 (48 bytes) and SHA512 (64 bytes). */ - if (datlen != 20 && datlen != 32) + if (datlen != 20 && datlen != 32 && datlen != 48 && datlen != 64) fatal("%s: data length incorrect: %u", __func__, datlen); /* save session id, it will be passed on the first call */ diff --git a/ssh-ecdsa.c b/ssh-ecdsa.c index 0627ee5c0..5c4ce2311 100644 --- a/ssh-ecdsa.c +++ b/ssh-ecdsa.c @@ -1,4 +1,4 @@ -/* $OpenBSD$ */ +/* $OpenBSD: ssh-ecdsa.c,v 1.4 2010/09/10 01:04:10 djm Exp $ */ /* * Copyright (c) 2000 Markus Friedl. All rights reserved. * Copyright (c) 2010 Damien Miller. All rights reserved. @@ -46,7 +46,7 @@ ssh_ecdsa_sign(const Key *key, u_char **sigp, u_int *lenp, const u_char *data, u_int datalen) { ECDSA_SIG *sig; - const EVP_MD *evp_md = EVP_sha256(); + const EVP_MD *evp_md; EVP_MD_CTX md; u_char digest[EVP_MAX_MD_SIZE]; u_int len, dlen; @@ -57,6 +57,7 @@ ssh_ecdsa_sign(const Key *key, u_char **sigp, u_int *lenp, error("%s: no ECDSA key", __func__); return -1; } + evp_md = key_ec_nid_to_evpmd(key->ecdsa_nid); EVP_DigestInit(&md, evp_md); EVP_DigestUpdate(&md, data, datalen); EVP_DigestFinal(&md, digest, &dlen); @@ -94,21 +95,22 @@ ssh_ecdsa_verify(const Key *key, const u_char *signature, u_int signaturelen, const u_char *data, u_int datalen) { ECDSA_SIG *sig; - const EVP_MD *evp_md = EVP_sha256(); + const EVP_MD *evp_md; EVP_MD_CTX md; u_char digest[EVP_MAX_MD_SIZE], *sigblob; u_int len, dlen; int rlen, ret; Buffer b, bb; + char *ktype; if (key == NULL || key->ecdsa == NULL || (key->type != KEY_ECDSA && key->type != KEY_ECDSA_CERT)) { error("%s: no ECDSA key", __func__); return -1; } + evp_md = key_ec_nid_to_evpmd(key->ecdsa_nid); /* fetch signature */ - char *ktype; buffer_init(&b); buffer_append(&b, signature, signaturelen); ktype = buffer_get_string(&b, NULL);