- 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.
This commit is contained in:
Damien Miller 2010-11-05 10:19:49 +11:00
parent 3a0e9f6479
commit b472a90d4c
5 changed files with 39 additions and 23 deletions

View File

@ -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

View File

@ -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 <ylo@cs.hut.fi>
* Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, 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 */

31
key.c
View File

@ -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 <ylo@cs.hut.fi>, 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 */

4
key.h
View File

@ -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 *);

View File

@ -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 <ylo@cs.hut.fi>
* Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, 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: