summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDamien Miller <djm@mindrot.org>2010-11-05 10:19:49 +1100
committerDamien Miller <djm@mindrot.org>2010-11-05 10:19:49 +1100
commitb472a90d4ceca15620aa525099bf4b2d5ba8a59b (patch)
treef4b9c97b1c3d78f68da5159852b7593a280edcf5
parent3a0e9f6479d50a95b5ccd7d7668b0ff45571de9c (diff)
- 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.
-rw-r--r--ChangeLog8
-rw-r--r--authfile.c14
-rw-r--r--key.c31
-rw-r--r--key.h4
-rw-r--r--ssh-keygen.c5
5 files changed, 39 insertions, 23 deletions
diff --git a/ChangeLog b/ChangeLog
index 2e7f92c94..79419367e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -3,6 +3,14 @@
3 - djm@cvs.openbsd.org 2010/09/22 12:26:05 3 - djm@cvs.openbsd.org 2010/09/22 12:26:05
4 [regress/Makefile regress/kextype.sh] 4 [regress/Makefile regress/kextype.sh]
5 regress test for each of the key exchange algorithms that we support 5 regress test for each of the key exchange algorithms that we support
6 - djm@cvs.openbsd.org 2010/10/28 11:22:09
7 [authfile.c key.c key.h ssh-keygen.c]
8 fix a possible NULL deref on loading a corrupt ECDH key
9
10 store ECDH group information in private keys files as "named groups"
11 rather than as a set of explicit group parameters (by setting
12 the OPENSSL_EC_NAMED_CURVE flag). This makes for shorter key files and
13 retrieves the group's OpenSSL NID that we need for various things.
6 14
720101025 1520101025
8 - (tim) [openbsd-compat/glob.h] Remove sys/cdefs.h include that came with 16 - (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 @@
1/* $OpenBSD: authfile.c,v 1.84 2010/09/08 03:54:36 djm Exp $ */ 1/* $OpenBSD: authfile.c,v 1.85 2010/10/28 11:22:09 djm Exp $ */
2/* 2/*
3 * Author: Tatu Ylonen <ylo@cs.hut.fi> 3 * Author: Tatu Ylonen <ylo@cs.hut.fi>
4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * 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,
523 prv = key_new(KEY_UNSPEC); 523 prv = key_new(KEY_UNSPEC);
524 prv->ecdsa = EVP_PKEY_get1_EC_KEY(pk); 524 prv->ecdsa = EVP_PKEY_get1_EC_KEY(pk);
525 prv->type = KEY_ECDSA; 525 prv->type = KEY_ECDSA;
526 prv->ecdsa_nid = key_ecdsa_group_to_nid( 526 if ((prv->ecdsa_nid = key_ecdsa_key_to_nid(prv->ecdsa)) == -1 ||
527 EC_KEY_get0_group(prv->ecdsa)); 527 key_curve_nid_to_name(prv->ecdsa_nid) == NULL ||
528 if (key_curve_nid_to_name(prv->ecdsa_nid) == NULL) { 528 key_ec_validate_public(EC_KEY_get0_group(prv->ecdsa),
529 key_free(prv);
530 prv = NULL;
531 }
532 if (key_ec_validate_public(EC_KEY_get0_group(prv->ecdsa),
533 EC_KEY_get0_public_key(prv->ecdsa)) != 0 || 529 EC_KEY_get0_public_key(prv->ecdsa)) != 0 ||
534 key_ec_validate_private(prv->ecdsa) != 0) { 530 key_ec_validate_private(prv->ecdsa) != 0) {
535 error("%s: bad ECDSA key", __func__); 531 error("%s: bad ECDSA key", __func__);
@@ -538,7 +534,7 @@ key_load_private_pem(int fd, int type, const char *passphrase,
538 } 534 }
539 name = "ecdsa w/o comment"; 535 name = "ecdsa w/o comment";
540#ifdef DEBUG_PK 536#ifdef DEBUG_PK
541 if (prv->ecdsa != NULL) 537 if (prv != NULL && prv->ecdsa != NULL)
542 key_dump_ec_key(prv->ecdsa); 538 key_dump_ec_key(prv->ecdsa);
543#endif 539#endif
544#endif /* OPENSSL_HAS_ECC */ 540#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 @@
1/* $OpenBSD: key.c,v 1.93 2010/09/09 10:45:45 djm Exp $ */ 1/* $OpenBSD: key.c,v 1.94 2010/10/28 11:22:09 djm Exp $ */
2/* 2/*
3 * read_bignum(): 3 * read_bignum():
4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * Copyright (c) 1995 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -1053,12 +1053,8 @@ key_ecdsa_bits_to_nid(int bits)
1053} 1053}
1054 1054
1055#ifdef OPENSSL_HAS_ECC 1055#ifdef OPENSSL_HAS_ECC
1056/*
1057 * This is horrid, but OpenSSL's PEM_read_PrivateKey seems not to restore
1058 * the EC_GROUP nid when loading a key...
1059 */
1060int 1056int
1061key_ecdsa_group_to_nid(const EC_GROUP *g) 1057key_ecdsa_key_to_nid(EC_KEY *k)
1062{ 1058{
1063 EC_GROUP *eg; 1059 EC_GROUP *eg;
1064 int nids[] = { 1060 int nids[] = {
@@ -1067,23 +1063,39 @@ key_ecdsa_group_to_nid(const EC_GROUP *g)
1067 NID_secp521r1, 1063 NID_secp521r1,
1068 -1 1064 -1
1069 }; 1065 };
1066 int nid;
1070 u_int i; 1067 u_int i;
1071 BN_CTX *bnctx; 1068 BN_CTX *bnctx;
1069 const EC_GROUP *g = EC_KEY_get0_group(k);
1072 1070
1071 /*
1072 * The group may be stored in a ASN.1 encoded private key in one of two
1073 * ways: as a "named group", which is reconstituted by ASN.1 object ID
1074 * or explicit group parameters encoded into the key blob. Only the
1075 * "named group" case sets the group NID for us, but we can figure
1076 * it out for the other case by comparing against all the groups that
1077 * are supported.
1078 */
1079 if ((nid = EC_GROUP_get_curve_name(g)) > 0)
1080 return nid;
1073 if ((bnctx = BN_CTX_new()) == NULL) 1081 if ((bnctx = BN_CTX_new()) == NULL)
1074 fatal("%s: BN_CTX_new() failed", __func__); 1082 fatal("%s: BN_CTX_new() failed", __func__);
1075 for (i = 0; nids[i] != -1; i++) { 1083 for (i = 0; nids[i] != -1; i++) {
1076 if ((eg = EC_GROUP_new_by_curve_name(nids[i])) == NULL) 1084 if ((eg = EC_GROUP_new_by_curve_name(nids[i])) == NULL)
1077 fatal("%s: EC_GROUP_new_by_curve_name failed", 1085 fatal("%s: EC_GROUP_new_by_curve_name failed",
1078 __func__); 1086 __func__);
1079 if (EC_GROUP_cmp(g, eg, bnctx) == 0) { 1087 if (EC_GROUP_cmp(g, eg, bnctx) == 0)
1080 EC_GROUP_free(eg);
1081 break; 1088 break;
1082 }
1083 EC_GROUP_free(eg); 1089 EC_GROUP_free(eg);
1084 } 1090 }
1085 BN_CTX_free(bnctx); 1091 BN_CTX_free(bnctx);
1086 debug3("%s: nid = %d", __func__, nids[i]); 1092 debug3("%s: nid = %d", __func__, nids[i]);
1093 if (nids[i] != -1) {
1094 /* Use the group with the NID attached */
1095 EC_GROUP_set_asn1_flag(eg, OPENSSL_EC_NAMED_CURVE);
1096 if (EC_KEY_set_group(k, eg) != 1)
1097 fatal("%s: EC_KEY_set_group", __func__);
1098 }
1087 return nids[i]; 1099 return nids[i];
1088} 1100}
1089 1101
@@ -1098,6 +1110,7 @@ ecdsa_generate_private_key(u_int bits, int *nid)
1098 fatal("%s: EC_KEY_new_by_curve_name failed", __func__); 1110 fatal("%s: EC_KEY_new_by_curve_name failed", __func__);
1099 if (EC_KEY_generate_key(private) != 1) 1111 if (EC_KEY_generate_key(private) != 1)
1100 fatal("%s: EC_KEY_generate_key failed", __func__); 1112 fatal("%s: EC_KEY_generate_key failed", __func__);
1113 EC_KEY_set_asn1_flag(private, OPENSSL_EC_NAMED_CURVE);
1101 return private; 1114 return private;
1102} 1115}
1103#endif /* OPENSSL_HAS_ECC */ 1116#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 @@
1/* $OpenBSD: key.h,v 1.32 2010/09/09 10:45:45 djm Exp $ */ 1/* $OpenBSD: key.h,v 1.33 2010/10/28 11:22:09 djm Exp $ */
2 2
3/* 3/*
4 * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved. 4 * Copyright (c) 2000, 2001 Markus Friedl. All rights reserved.
@@ -121,7 +121,7 @@ const char * key_curve_nid_to_name(int);
121u_int key_curve_nid_to_bits(int); 121u_int key_curve_nid_to_bits(int);
122int key_ecdsa_bits_to_nid(int); 122int key_ecdsa_bits_to_nid(int);
123#ifdef OPENSSL_HAS_ECC 123#ifdef OPENSSL_HAS_ECC
124int key_ecdsa_group_to_nid(const EC_GROUP *); 124int key_ecdsa_key_to_nid(EC_KEY *);
125const EVP_MD * key_ec_nid_to_evpmd(int nid); 125const EVP_MD * key_ec_nid_to_evpmd(int nid);
126int key_ec_validate_public(const EC_GROUP *, const EC_POINT *); 126int key_ec_validate_public(const EC_GROUP *, const EC_POINT *);
127int key_ec_validate_private(const EC_KEY *); 127int 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 @@
1/* $OpenBSD: ssh-keygen.c,v 1.203 2010/09/02 17:21:50 naddy Exp $ */ 1/* $OpenBSD: ssh-keygen.c,v 1.204 2010/10/28 11:22:09 djm Exp $ */
2/* 2/*
3 * Author: Tatu Ylonen <ylo@cs.hut.fi> 3 * Author: Tatu Ylonen <ylo@cs.hut.fi>
4 * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland 4 * Copyright (c) 1994 Tatu Ylonen <ylo@cs.hut.fi>, Espoo, Finland
@@ -556,8 +556,7 @@ do_convert_from_pkcs8(Key **k, int *private)
556 *k = key_new(KEY_UNSPEC); 556 *k = key_new(KEY_UNSPEC);
557 (*k)->type = KEY_ECDSA; 557 (*k)->type = KEY_ECDSA;
558 (*k)->ecdsa = EVP_PKEY_get1_EC_KEY(pubkey); 558 (*k)->ecdsa = EVP_PKEY_get1_EC_KEY(pubkey);
559 (*k)->ecdsa_nid = key_ecdsa_group_to_nid( 559 (*k)->ecdsa_nid = key_ecdsa_key_to_nid((*k)->ecdsa);
560 EC_KEY_get0_group((*k)->ecdsa));
561 break; 560 break;
562#endif 561#endif
563 default: 562 default: