Always enable ecdh_auto.
This is a really dumb API wart. Now that we have a limited set of curves that
are all reasonable, the automatic logic should just always kick in. This makes
set_ecdh_auto a no-op and, instead of making it the first choice, uses it as
the fallback behavior should none of the older curve selection APIs be used.
Currently, by default, server sockets can only use the plain RSA key exchange.
BUG=481139
Change-Id: Iaabc82de766cd00968844a71aaac29bd59841cd4
Reviewed-on: https://boringssl-review.googlesource.com/4531
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h
index 32f7f51..c981394 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -454,13 +454,15 @@
DH *dh_tmp;
DH *(*dh_tmp_cb)(SSL *ssl, int is_export, int keysize);
+
/* ecdh_nid, if not |NID_undef|, is the NID of the curve to use for ephemeral
- * ECDH keys. */
+ * ECDH keys. If unset, |ecdh_tmp_cb| is consulted. */
int ecdh_nid;
- /* Callback for generating ephemeral ECDH keys */
+ /* ecdh_tmp_cb is a callback for selecting the curve to use for ephemeral ECDH
+ * keys. If NULL, a curve is selected automatically. See
+ * |SSL_CTX_set_tmp_ecdh_callback|. */
EC_KEY *(*ecdh_tmp_cb)(SSL *ssl, int is_export, int keysize);
- /* Select ECDH parameters automatically */
- int ecdh_tmp_auto;
+
/* Flags related to certificates */
unsigned int cert_flags;
CERT_PKEY pkeys[SSL_PKEY_NUM];
diff --git a/ssl/s3_lib.c b/ssl/s3_lib.c
index 9c295e9..3d80565 100644
--- a/ssl/s3_lib.c
+++ b/ssl/s3_lib.c
@@ -676,7 +676,7 @@
case SSL_CTRL_SET_TMP_ECDH: {
/* For historical reasons, this API expects an |EC_KEY|, but only the
* group is used. */
- EC_KEY *ec_key = (EC_KEY *)parg;
+ const EC_KEY *ec_key = (const EC_KEY *)parg;
if (ec_key == NULL || EC_KEY_get0_group(ec_key) == NULL) {
OPENSSL_PUT_ERROR(SSL, ssl3_ctrl, ERR_R_PASSED_NULL_PARAMETER);
return ret;
@@ -766,10 +766,6 @@
return tls1_set_curves(&s->tlsext_ellipticcurvelist,
&s->tlsext_ellipticcurvelist_length, parg, larg);
- case SSL_CTRL_SET_ECDH_AUTO:
- s->cert->ecdh_tmp_auto = larg;
- return 1;
-
case SSL_CTRL_SET_SIGALGS:
return tls1_set_sigalgs(s->cert, parg, larg, 0);
@@ -945,7 +941,7 @@
case SSL_CTRL_SET_TMP_ECDH: {
/* For historical reasons, this API expects an |EC_KEY|, but only the
* group is used. */
- EC_KEY *ec_key = (EC_KEY *)parg;
+ const EC_KEY *ec_key = (const EC_KEY *)parg;
if (ec_key == NULL || EC_KEY_get0_group(ec_key) == NULL) {
OPENSSL_PUT_ERROR(SSL, ssl3_ctx_ctrl, ERR_R_PASSED_NULL_PARAMETER);
return 0;
@@ -993,10 +989,6 @@
return tls1_set_curves(&ctx->tlsext_ellipticcurvelist,
&ctx->tlsext_ellipticcurvelist_length, parg, larg);
- case SSL_CTRL_SET_ECDH_AUTO:
- ctx->cert->ecdh_tmp_auto = larg;
- return 1;
-
case SSL_CTRL_SET_SIGALGS:
return tls1_set_sigalgs(ctx->cert, parg, larg, 0);
diff --git a/ssl/s3_srvr.c b/ssl/s3_srvr.c
index a25775f..8f5712e 100644
--- a/ssl/s3_srvr.c
+++ b/ssl/s3_srvr.c
@@ -1410,9 +1410,7 @@
} else if (alg_k & SSL_kECDHE) {
/* Determine the curve to use. */
int nid = NID_undef;
- if (cert->ecdh_tmp_auto) {
- nid = tls1_get_shared_curve(s);
- } else if (cert->ecdh_nid != NID_undef) {
+ if (cert->ecdh_nid != NID_undef) {
nid = cert->ecdh_nid;
} else if (cert->ecdh_tmp_cb != NULL) {
/* Note: |ecdh_tmp_cb| does NOT pass ownership of the result
@@ -1421,6 +1419,8 @@
if (template != NULL && EC_KEY_get0_group(template) != NULL) {
nid = EC_GROUP_get_curve_name(EC_KEY_get0_group(template));
}
+ } else {
+ nid = tls1_get_shared_curve(s);
}
if (nid == NID_undef) {
al = SSL_AD_HANDSHAKE_FAILURE;
diff --git a/ssl/ssl_cert.c b/ssl/ssl_cert.c
index 14c5e31..313562b 100644
--- a/ssl/ssl_cert.c
+++ b/ssl/ssl_cert.c
@@ -216,7 +216,6 @@
ret->ecdh_nid = cert->ecdh_nid;
ret->ecdh_tmp_cb = cert->ecdh_tmp_cb;
- ret->ecdh_tmp_auto = cert->ecdh_tmp_auto;
for (i = 0; i < SSL_PKEY_NUM; i++) {
CERT_PKEY *cpk = cert->pkeys + i;
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index c491b12..2244dbd 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2004,7 +2004,6 @@
int rsa_enc, rsa_sign, dh_tmp;
uint32_t mask_k, mask_a;
int have_ecc_cert, ecdsa_ok;
- int have_ecdh_tmp;
X509 *x;
if (c == NULL) {
@@ -2016,8 +2015,6 @@
dh_tmp = (c->dh_tmp != NULL || c->dh_tmp_cb != NULL);
- have_ecdh_tmp = (c->ecdh_nid != NID_undef || c->ecdh_tmp_cb != NULL ||
- c->ecdh_tmp_auto);
rsa_enc = ssl_has_key(s, SSL_PKEY_RSA_ENC);
rsa_sign = ssl_has_key(s, SSL_PKEY_RSA_SIGN);
have_ecc_cert = ssl_has_key(s, SSL_PKEY_ECC);
@@ -2053,7 +2050,7 @@
/* If we are considering an ECC cipher suite that uses an ephemeral EC
* key, check it. */
- if (have_ecdh_tmp && tls1_check_ec_tmp_key(s)) {
+ if (tls1_check_ec_tmp_key(s)) {
mask_k |= SSL_kECDHE;
}
@@ -2625,15 +2622,16 @@
}
void SSL_CTX_set_tmp_ecdh_callback(SSL_CTX *ctx,
- EC_KEY *(*ecdh)(SSL *ssl, int is_export,
- int keylength)) {
- SSL_CTX_callback_ctrl(ctx, SSL_CTRL_SET_TMP_ECDH_CB, (void (*)(void))ecdh);
+ EC_KEY *(*callback)(SSL *ssl, int is_export,
+ int keylength)) {
+ SSL_CTX_callback_ctrl(ctx, SSL_CTRL_SET_TMP_ECDH_CB,
+ (void (*)(void))callback);
}
void SSL_set_tmp_ecdh_callback(SSL *ssl,
- EC_KEY *(*ecdh)(SSL *ssl, int is_export,
- int keylength)) {
- SSL_callback_ctrl(ssl, SSL_CTRL_SET_TMP_ECDH_CB, (void (*)(void))ecdh);
+ EC_KEY *(*callback)(SSL *ssl, int is_export,
+ int keylength)) {
+ SSL_callback_ctrl(ssl, SSL_CTRL_SET_TMP_ECDH_CB, (void (*)(void))callback);
}
int SSL_CTX_use_psk_identity_hint(SSL_CTX *ctx, const char *identity_hint) {
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index fdbb340..1e495b3 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -633,22 +633,22 @@
}
int tls1_check_ec_tmp_key(SSL *s) {
- if (s->cert->ecdh_tmp_auto) {
- /* If using |ecdh_tmp_auto|, ECDH is acceptable if there is a shared
- * curve. */
- return tls1_get_shared_curve(s) != NID_undef;
- }
-
if (s->cert->ecdh_nid != NID_undef) {
- /* If the curve is preconfigured, ECDH is acceptable if the peer supports
+ /* If the curve is preconfigured, ECDH is acceptable iff the peer supports
* the curve. */
uint16_t curve_id;
return tls1_ec_nid2curve_id(&curve_id, s->cert->ecdh_nid) &&
tls1_check_curve_id(s, curve_id);
}
- /* Otherwise, assume the callback will provide an acceptable curve. */
- return s->cert->ecdh_tmp_cb != NULL;
+ if (s->cert->ecdh_tmp_cb != NULL) {
+ /* Assume the callback will provide an acceptable curve. */
+ return 1;
+ }
+
+ /* Otherwise, the curve gets selected automatically. ECDH is acceptable iff
+ * there is a shared curve. */
+ return tls1_get_shared_curve(s) != NID_undef;
}
/* List of supported signature algorithms and hashes. Should make this
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index fd4bd60..bf526b8 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -413,10 +413,6 @@
SSL_CTX_set_read_ahead(ssl_ctx.get(), 1);
}
- if (!SSL_CTX_set_ecdh_auto(ssl_ctx.get(), 1)) {
- return nullptr;
- }
-
if (!SSL_CTX_set_cipher_list(ssl_ctx.get(), "ALL")) {
return nullptr;
}
