Remove constraints on curve ID values.

The under 32 constraint is silly; it's to check for duplicate curves in
library-supplied configuration. That API is new as of 1.0.2. It doesn't seem
worth bothering; if the caller supplies a repeated value, may as well emit a
repeated one and so be it. (Probably no one will ever call that function
outside of maybe test code anyway.)

While I'm here, remove the 0 constraint too. It's not likely to change, but
removing the return value overload seems easier than keeping comments about it
comments about it.

Change-Id: I01d36dba1855873875bb5a0ec84b040199e0e9bc
Reviewed-on: https://boringssl-review.googlesource.com/2844
Reviewed-by: Adam Langley <agl@google.com>

ssl/s3_srvr.c

@@ -1377,7 +1377,7 @@ int ssl3_send_server_key_exchange(SSL *s) {
   EC_KEY *ecdh = NULL, *ecdhp;
   uint8_t *encodedPoint = NULL;
   int encodedlen = 0;
-  int curve_id = 0;
+  uint16_t curve_id = 0;
   BN_CTX *bn_ctx = NULL;
   const char *psk_identity_hint = NULL;
   size_t psk_identity_hint_len = 0;
@@ -1517,10 +1517,8 @@ int ssl3_send_server_key_exchange(SSL *s) {
         goto err;
       }
 
-      /* We only support ephemeral ECDH keys over named (not generic) curves.
-       * For supported named curves, curve_id is non-zero. */
-      curve_id = tls1_ec_nid2curve_id(EC_GROUP_get_curve_name(group));
-      if (curve_id == 0) {
+      /* We only support ephemeral ECDH keys over named (not generic) curves. */
+      if (!tls1_ec_nid2curve_id(&curve_id, EC_GROUP_get_curve_name(group))) {
         OPENSSL_PUT_ERROR(SSL, ssl3_send_server_key_exchange,
                           SSL_R_UNSUPPORTED_ELLIPTIC_CURVE);
         goto err;
@@ -1617,18 +1615,14 @@ int ssl3_send_server_key_exchange(SSL *s) {
        * [1 byte CurveType], [2 byte CurveName]
        * [1 byte length of encoded point], followed by
        * the actual encoded point itself. */
-      *p = NAMED_CURVE_TYPE;
-      p += 1;
-      *p = 0;
-      p += 1;
-      *p = curve_id;
-      p += 1;
-      *p = encodedlen;
-      p += 1;
-      memcpy((uint8_t *)p, (uint8_t *)encodedPoint, encodedlen);
+      *(p++) = NAMED_CURVE_TYPE;
+      *(p++) = (uint8_t)(curve_id >> 8);
+      *(p++) = (uint8_t)(curve_id & 0xff);
+      *(p++) = encodedlen;
+      memcpy(p, encodedPoint, encodedlen);
+      p += encodedlen;
       OPENSSL_free(encodedPoint);
       encodedPoint = NULL;
-      p += encodedlen;
     }
 
     /* not anonymous */

ssl/ssl_locl.h

@@ -919,7 +919,7 @@ int ssl_check_srvr_ecc_cert_and_alg(X509 *x, SSL *s);
 
 char ssl_early_callback_init(struct ssl_early_callback_ctx *ctx);
 int tls1_ec_curve_id2nid(uint16_t curve_id);
-uint16_t tls1_ec_nid2curve_id(int nid);
+int tls1_ec_nid2curve_id(uint16_t *out_curve_id, int nid);
 
 /* tls1_check_curve parses ECParameters out of |cbs|, modifying it. It
  * checks the curve is one of our preferences and writes the

ssl/t1_lib.c

@@ -354,16 +354,7 @@ struct tls_curve {
   int nid;
 };
 
-/* ECC curves from RFC4492.
- *
- * NOTE: tls1_ec_curve_id2nid and tls1_set_curves assume that
- *
- * (a) 0 is not a valid curve ID.
- *
- * (b) The largest curve ID is 31.
- *
- * Those implementations must be revised before adding support for curve IDs
- * that break these assumptions. */
+/* ECC curves from RFC4492. */
 static const struct tls_curve tls_curves[] = {
     {21, NID_secp224r1},
     {23, NID_X9_62_prime256v1},
@@ -391,16 +382,14 @@ int tls1_ec_curve_id2nid(uint16_t curve_id) {
   return NID_undef;
 }
 
-uint16_t tls1_ec_nid2curve_id(int nid) {
+int tls1_ec_nid2curve_id(uint16_t *out_curve_id, int nid) {
   size_t i;
   for (i = 0; i < sizeof(tls_curves) / sizeof(tls_curves[0]); i++) {
     if (nid == tls_curves[i].nid) {
-      return tls_curves[i].curve_id;
+      *out_curve_id = tls_curves[i].curve_id;
+      return 1;
     }
   }
-
-  /* Use 0 for a non-existent curve ID. Note: this assumes that curve
-   * ID 0 will never be allocated. */
   return 0;
 }
 
@@ -479,25 +468,16 @@ int tls1_set_curves(uint16_t **out_curve_ids, size_t *out_curve_ids_len,
   uint16_t *curve_ids;
   size_t i;
 
-  /* Bitmap of curves included to detect duplicates: only works
-   * while curve ids < 32. */
-  uint32_t dup_list = 0;
   curve_ids = (uint16_t *)OPENSSL_malloc(ncurves * sizeof(uint16_t));
   if (curve_ids == NULL) {
     return 0;
   }
 
   for (i = 0; i < ncurves; i++) {
-    uint32_t idmask;
-    uint16_t id;
-    id = tls1_ec_nid2curve_id(curves[i]);
-    idmask = ((uint32_t)1) << id;
-    if (!id || (dup_list & idmask)) {
+    if (!tls1_ec_nid2curve_id(&curve_ids[i], curves[i])) {
       OPENSSL_free(curve_ids);
       return 0;
     }
-    dup_list |= idmask;
-    curve_ids[i] = id;
   }
 
   if (*out_curve_ids) {
@@ -529,8 +509,7 @@ static int tls1_curve_params_from_ec_key(uint16_t *out_curve_id,
 
   /* Determine curve ID */
   nid = EC_GROUP_get_curve_name(grp);
-  id = tls1_ec_nid2curve_id(nid);
-  if (!id) {
+  if (!tls1_ec_nid2curve_id(&id, nid)) {
     return 0;
   }