diff --git a/ChangeLog b/ChangeLog index 2e7f92c94..79419367e 100644 --- a/ChangeLog +++ b/ChangeLog @@ -3,6 +3,14 @@ - djm@cvs.openbsd.org 2010/09/22 12:26:05 [regress/Makefile regress/kextype.sh] regress test for each of the key exchange algorithms that we support + - djm@cvs.openbsd.org 2010/10/28 11:22:09 + [authfile.c key.c key.h ssh-keygen.c] + fix a possible NULL deref on loading a corrupt ECDH key + + store ECDH group information in private keys files as "named groups" + rather than as a set of explicit group parameters (by setting + the OPENSSL_EC_NAMED_CURVE flag). This makes for shorter key files and + retrieves the group's OpenSSL NID that we need for various things. 20101025 - (tim) [openbsd-compat/glob.h] Remove sys/cdefs.h include that came with diff --git a/authfile.c b/authfile.c index b1e3eda5c..7f98ab547 100644 --- a/authfile.c +++ b/authfile.c @@ -1,4 +1,4 @@ -/* $OpenBSD: authfile.c,v 1.84 2010/09/08 03:54:36 djm Exp $ */ +/* $OpenBSD: authfile.c,v 1.85 2010/10/28 11:22:09 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -523,13 +523,9 @@ key_load_private_pem(int fd, int type, const char *passphrase, prv = key_new(KEY_UNSPEC); prv->ecdsa = EVP_PKEY_get1_EC_KEY(pk); prv->type = KEY_ECDSA; - prv->ecdsa_nid = key_ecdsa_group_to_nid( - EC_KEY_get0_group(prv->ecdsa)); - if (key_curve_nid_to_name(prv->ecdsa_nid) == NULL) { - key_free(prv); - prv = NULL; - } - if (key_ec_validate_public(EC_KEY_get0_group(prv->ecdsa), + if ((prv->ecdsa_nid = key_ecdsa_key_to_nid(prv->ecdsa)) == -1 || + key_curve_nid_to_name(prv->ecdsa_nid) == NULL || + key_ec_validate_public(EC_KEY_get0_group(prv->ecdsa), EC_KEY_get0_public_key(prv->ecdsa)) != 0 || key_ec_validate_private(prv->ecdsa) != 0) { error("%s: bad ECDSA key", __func__); @@ -538,7 +534,7 @@ key_load_private_pem(int fd, int type, const char *passphrase, } name = "ecdsa w/o comment"; #ifdef DEBUG_PK - if (prv->ecdsa != NULL) + if (prv != NULL && prv->ecdsa != NULL) key_dump_ec_key(prv->ecdsa); #endif #endif /* OPENSSL_HAS_ECC */ diff --git a/key.c b/key.c index 196092de5..c71bf5b0a 100644 --- a/key.c +++ b/key.c @@ -1,4 +1,4 @@ -/* $OpenBSD: key.c,v 1.93 2010/09/09 10:45:45 djm Exp $ */ +/* $OpenBSD: key.c,v 1.94 2010/10/28 11:22:09 djm Exp $ */ /* * read_bignum(): * Copyright (c) 1995 Tatu Ylonen , Espoo, Finland @@ -1053,12 +1053,8 @@ key_ecdsa_bits_to_nid(int bits) } #ifdef OPENSSL_HAS_ECC -/* - * This is horrid, but OpenSSL's PEM_read_PrivateKey seems not to restore - * the EC_GROUP nid when loading a key... - */ int -key_ecdsa_group_to_nid(const EC_GROUP *g) +key_ecdsa_key_to_nid(EC_KEY *k) { EC_GROUP *eg; int nids[] = { @@ -1067,23 +1063,39 @@ key_ecdsa_group_to_nid(const EC_GROUP *g) NID_secp521r1, -1 }; + int nid; u_int i; BN_CTX *bnctx; + const EC_GROUP *g = EC_KEY_get0_group(k); + /* + * The group may be stored in a ASN.1 encoded private key in one of two + * ways: as a "named group", which is reconstituted by ASN.1 object ID + * or explicit group parameters encoded into the key blob. Only the + * "named group" case sets the group NID for us, but we can figure + * it out for the other case by comparing against all the groups that + * are supported. + */ + if ((nid = EC_GROUP_get_curve_name(g)) > 0) + return nid; if ((bnctx = BN_CTX_new()) == NULL) fatal("%s: BN_CTX_new() failed", __func__); for (i = 0; nids[i] != -1; i++) { if ((eg = EC_GROUP_new_by_curve_name(nids[i])) == NULL) fatal("%s: EC_GROUP_new_by_curve_name failed", __func__); - if (EC_GROUP_cmp(g, eg, bnctx) == 0) { - EC_GROUP_free(eg); + if (EC_GROUP_cmp(g, eg, bnctx) == 0) break; - } EC_GROUP_free(eg); } BN_CTX_free(bnctx); debug3("%s: nid = %d", __func__, nids[i]); + if (nids[i] != -1) { + /* Use the group with the NID attached */ + EC_GROUP_set_asn1_flag(eg, OPENSSL_EC_NAMED_CURVE); + if (EC_KEY_set_group(k, eg) != 1) + fatal("%s: EC_KEY_set_group", __func__); + } return nids[i]; } @@ -1098,6 +1110,7 @@ ecdsa_generate_private_key(u_int bits, int *nid) fatal("%s: EC_KEY_new_by_curve_name failed", __func__); if (EC_KEY_generate_key(private) != 1) fatal("%s: EC_KEY_generate_key failed", __func__); + EC_KEY_set_asn1_flag(private, OPENSSL_EC_NAMED_CURVE); return private; } #endif /* OPENSSL_HAS_ECC */ diff --git a/key.h b/key.h index 86a1d889c..ec5ac5eb8 100644 --- a/key.h +++ b/key.h @@ -1,4 +1,4 @@ -/* $OpenBSD: key.h,v 1.32 2010/09/09 10:45:45 djm Exp $ */ +/* $OpenBSD: key.h,v 1.33 2010/10/28 11:22:09 djm Exp $ */ /* * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. @@ -121,7 +121,7 @@ const char * key_curve_nid_to_name(int); u_int key_curve_nid_to_bits(int); int key_ecdsa_bits_to_nid(int); #ifdef OPENSSL_HAS_ECC -int key_ecdsa_group_to_nid(const EC_GROUP *); +int key_ecdsa_key_to_nid(EC_KEY *); 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/ssh-keygen.c b/ssh-keygen.c index bbd434b0b..560c4818a 100644 --- a/ssh-keygen.c +++ b/ssh-keygen.c @@ -1,4 +1,4 @@ -/* $OpenBSD: ssh-keygen.c,v 1.203 2010/09/02 17:21:50 naddy Exp $ */ +/* $OpenBSD: ssh-keygen.c,v 1.204 2010/10/28 11:22:09 djm Exp $ */ /* * Author: Tatu Ylonen * Copyright (c) 1994 Tatu Ylonen , Espoo, Finland @@ -556,8 +556,7 @@ do_convert_from_pkcs8(Key **k, int *private) *k = key_new(KEY_UNSPEC); (*k)->type = KEY_ECDSA; (*k)->ecdsa = EVP_PKEY_get1_EC_KEY(pubkey); - (*k)->ecdsa_nid = key_ecdsa_group_to_nid( - EC_KEY_get0_group((*k)->ecdsa)); + (*k)->ecdsa_nid = key_ecdsa_key_to_nid((*k)->ecdsa); break; #endif default: