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>
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index 88f462b..b346d14 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1377,7 +1377,7 @@
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 @@
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 @@
* [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 */
diff --git a/ssl/ssl_locl.h b/ssl/ssl_locl.h
index 7d02a2d..a21d240 100644
--- a/ssl/ssl_locl.h
+++ b/ssl/ssl_locl.h
@@ -919,7 +919,7 @@
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
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index 224d636..339c765 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -354,16 +354,7 @@
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 @@
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 @@
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 @@
/* 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;
}
