Hi,

Here are a couple of changes to the ECDH/ECDSA code:

 - Fix a NULL-deref on loading of invalid ECC private keys

 - Support 224-bit ECDSA and ECDH and make it the default. This is
   strength-equivalent to our current default of 2048-bit RSA keys.
   Presently it is a bit faster than 256 bit ECC but it is going to get
   2 x faster for 64 bit systems in the next OpenSSL release where that
   specific curve has been hand-optimised.

 - Bake the ECC curve into private keys by name (actually ASN.1 OID) rather
   than explicitly. This makes for smaller key files, but more importantly
   simplifies the task of recovering the curve NID which we need in various
   places.

ok?

Index: authfile.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/authfile.c,v
retrieving revision 1.84
diff -u -p -r1.84 authfile.c
--- authfile.c  8 Sep 2010 03:54:36 -0000       1.84
+++ authfile.c  26 Oct 2010 05:16:23 -0000
@@ -511,13 +511,9 @@ key_load_private_pem(int fd, int type, c
                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__);
@@ -526,7 +522,7 @@ key_load_private_pem(int fd, int type, c
                }
                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
        } else {
Index: key.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/key.c,v
retrieving revision 1.93
diff -u -p -r1.93 key.c
--- key.c       9 Sep 2010 10:45:45 -0000       1.93
+++ key.c       26 Oct 2010 05:16:23 -0000
@@ -920,6 +920,8 @@ key_ssh_name_from_type_nid(int type, int
                return "ssh-dss-cert-...@openssh.com";
        case KEY_ECDSA:
                switch (nid) {
+               case NID_secp224r1:
+                       return "ecdsa-sha2-1.3.132.0.33";
                case NID_X9_62_prime256v1:
                        return "ecdsa-sha2-nistp256";
                case NID_secp384r1:
@@ -932,6 +934,8 @@ key_ssh_name_from_type_nid(int type, int
                break;
        case KEY_ECDSA_CERT:
                switch (nid) {
+               case NID_secp224r1:
+                       return "ecdsa-sha2-1.3.132.0.33-cert-...@openssh.com";
                case NID_X9_62_prime256v1:
                        return "ecdsa-sha2-nistp256-cert-...@openssh.com";
                case NID_secp384r1:
@@ -1008,6 +1012,8 @@ int
 key_ecdsa_bits_to_nid(int bits)
 {
        switch (bits) {
+       case 224:
+               return NID_secp224r1;
        case 256:
                return NID_X9_62_prime256v1;
        case 384:
@@ -1019,37 +1025,50 @@ key_ecdsa_bits_to_nid(int bits)
        }
 }
 
-/*
- * 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[] = {
+               NID_secp224r1,
                NID_X9_62_prime256v1,
                NID_secp384r1,
                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];
 }
 
@@ -1064,6 +1083,7 @@ ecdsa_generate_private_key(u_int bits, i
                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;
 }
 
@@ -1196,6 +1216,7 @@ key_type_from_name(char *name)
        } else if (strcmp(name, "ssh-dss") == 0) {
                return KEY_DSA;
        } else if (strcmp(name, "ecdsa") == 0 ||
+           strcmp(name, "ecdsa-sha2-1.3.132.0.33") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp256") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp384") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp521") == 0) {
@@ -1208,7 +1229,9 @@ key_type_from_name(char *name)
                return KEY_RSA_CERT;
        } else if (strcmp(name, "ssh-dss-cert-...@openssh.com") == 0) {
                return KEY_DSA_CERT;
-       } else if (strcmp(name, "ecdsa-sha2-nistp256-cert-...@openssh.com") == 
0 ||
+       } else if (strcmp(name,
+           "ecdsa-sha2-1.3.132.0.33-cert-...@openssh.com") == 0 ||
+           strcmp(name, "ecdsa-sha2-nistp256-cert-...@openssh.com") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp384-cert-...@openssh.com") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp521-cert-...@openssh.com") == 0)
                return KEY_ECDSA_CERT;
@@ -1220,6 +1243,9 @@ key_type_from_name(char *name)
 int
 key_ecdsa_nid_from_name(const char *name)
 {
+       if (strcmp(name, "ecdsa-sha2-1.3.132.0.33") == 0 ||
+           strcmp(name, "ecdsa-sha2-1.3.132.0.33-cert-...@openssh.com") == 0)
+               return NID_secp224r1;
        if (strcmp(name, "ecdsa-sha2-nistp256") == 0 ||
            strcmp(name, "ecdsa-sha2-nistp256-cert-...@openssh.com") == 0)
                return NID_X9_62_prime256v1;
@@ -1951,6 +1977,8 @@ key_cert_is_legacy(Key *k)
 int
 key_curve_name_to_nid(const char *name)
 {
+       if (strcmp(name, "1.3.132.0.33") == 0)
+               return NID_secp224r1;
        if (strcmp(name, "nistp256") == 0)
                return NID_X9_62_prime256v1;
        else if (strcmp(name, "nistp384") == 0)
@@ -1966,6 +1994,8 @@ u_int
 key_curve_nid_to_bits(int nid)
 {
        switch (nid) {
+       case NID_secp224r1:
+               return 224;
        case NID_X9_62_prime256v1:
                return 256;
        case NID_secp384r1:
@@ -1981,7 +2011,9 @@ key_curve_nid_to_bits(int nid)
 const char *
 key_curve_nid_to_name(int nid)
 {
-       if (nid == NID_X9_62_prime256v1)
+       if (nid == NID_secp224r1)
+               return "1.3.132.0.33";
+       else if (nid == NID_X9_62_prime256v1)
                return "nistp256";
        else if (nid == NID_secp384r1)
                return "nistp384";
Index: key.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/key.h,v
retrieving revision 1.32
diff -u -p -r1.32 key.h
--- key.h       9 Sep 2010 10:45:45 -0000       1.32
+++ key.h       26 Oct 2010 05:16:23 -0000
@@ -114,7 +114,7 @@ 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 *);
+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 *);
Index: myproposal.h
===================================================================
RCS file: /cvs/src/usr.bin/ssh/myproposal.h,v
retrieving revision 1.27
diff -u -p -r1.27 myproposal.h
--- myproposal.h        1 Sep 2010 22:42:13 -0000       1.27
+++ myproposal.h        26 Oct 2010 05:16:23 -0000
@@ -25,6 +25,7 @@
  */
 
 #define KEX_DEFAULT_KEX                \
+       "ecdh-sha2-1.3.132.0.33," \
        "ecdh-sha2-nistp256," \
        "ecdh-sha2-nistp384," \
        "ecdh-sha2-nistp521," \
Index: ssh-keygen.c
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssh-keygen.c,v
retrieving revision 1.203
diff -u -p -r1.203 ssh-keygen.c
--- ssh-keygen.c        2 Sep 2010 17:21:50 -0000       1.203
+++ ssh-keygen.c        26 Oct 2010 05:16:24 -0000
@@ -49,7 +49,7 @@
 /* Number of bits in the RSA/DSA key.  This value can be set on the command 
line. */
 #define DEFAULT_BITS           2048
 #define DEFAULT_BITS_DSA       1024
-#define DEFAULT_BITS_ECDSA     256
+#define DEFAULT_BITS_ECDSA     224
 u_int32_t bits = 0;
 
 /*
@@ -545,8 +545,7 @@ do_convert_from_pkcs8(Key **k, int *priv
                *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;
        default:
                fatal("%s: unsupported pubkey type %d", __func__,
@@ -1812,7 +1811,7 @@ main(int argc, char **argv)
            "O:C:r:g:R:T:G:M:S:s:a:V:W:z:")) != -1) {
                switch (opt) {
                case 'b':
-                       bits = (u_int32_t)strtonum(optarg, 256, 32768, &errstr);
+                       bits = (u_int32_t)strtonum(optarg, 224, 32768, &errstr);
                        if (errstr)
                                fatal("Bits has bad value %s (%s)",
                                        optarg, errstr);
@@ -2116,7 +2115,7 @@ main(int argc, char **argv)
                fatal("Key must at least be 768 bits");
        else if (type == KEY_ECDSA && key_ecdsa_bits_to_nid(bits) == -1)
                fatal("Invalid ECDSA key length - valid lengths are "
-                   "256, 384 or 521 bits");
+                   "224, 256, 384 or 521 bits");
        if (!quiet)
                printf("Generating public/private %s key pair.\n", 
key_type_name);
        private = key_generate(type, bits);
Index: ssh_config.5
===================================================================
RCS file: /cvs/src/usr.bin/ssh/ssh_config.5,v
retrieving revision 1.141
diff -u -p -r1.141 ssh_config.5
--- ssh_config.5        22 Sep 2010 08:30:08 -0000      1.141
+++ ssh_config.5        26 Oct 2010 05:16:24 -0000
@@ -651,7 +651,8 @@ Specifies the available KEX (Key Exchang
 Multiple algorithms must be comma-separated.
 The default is:
 .Bd -literal -offset indent
-ecdh-sha2-nistp256,ecdh-sha2-nistp384,ecdh-sha2-nistp521,
+ecdh-sha2-1.3.132.0.33,ecdh-sha2-nistp256,
+ecdh-sha2-nistp384,ecdh-sha2-nistp521,
 diffie-hellman-group-exchange-sha256,
 diffie-hellman-group-exchange-sha1,
 diffie-hellman-group14-sha1,
Index: sshd_config.5
===================================================================
RCS file: /cvs/src/usr.bin/ssh/sshd_config.5,v
retrieving revision 1.127
diff -u -p -r1.127 sshd_config.5
--- sshd_config.5       22 Sep 2010 05:01:30 -0000      1.127
+++ sshd_config.5       26 Oct 2010 05:16:25 -0000
@@ -543,6 +543,7 @@ The default is
 Specifies the available KEX (Key Exchange) algorithms.
 Multiple algorithms must be comma-separated.
 The default is
+.Dq ecdh-sha2-1.3.132.0.33 ,
 .Dq ecdh-sha2-nistp256 ,
 .Dq ecdh-sha2-nistp384 ,
 .Dq ecdh-sha2-nistp521 ,

Reply via email to