Add SSL_get_curve_id and SSL_get_dhe_group_size.
This replaces the old key_exchange_info APIs and does not require the
caller be aware of the mess around SSL_SESSION management. They
currently have the same bugs around renegotiation as before, but later
work to fix up SSL_SESSION tracking will fix their internals.
For consistency with the existing functions, I've kept the public API at
'curve' rather than 'group' for now. I think it's probably better to
have only one name with a single explanation in the section header
rather than half and half. (I also wouldn't be surprised if the IETF
ends up renaming 'group' again to 'key exchange' at some point. We'll
see what happens.)
Change-Id: I8e90a503bc4045d12f30835c86de64ef9f2d07c8
Reviewed-on: https://boringssl-review.googlesource.com/8565
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/internal.h b/ssl/internal.h
index 41543eb..d0eca69 100644
--- a/ssl/internal.h
+++ b/ssl/internal.h
@@ -545,11 +545,6 @@
/* ECDH groups. */
-#define SSL_GROUP_SECP256R1 23
-#define SSL_GROUP_SECP384R1 24
-#define SSL_GROUP_SECP521R1 25
-#define SSL_GROUP_X25519 29
-
/* An SSL_ECDH_METHOD is an implementation of ECDH-like key exchanges for
* TLS. */
struct ssl_ecdh_method_st {
diff --git a/ssl/ssl_ecdh.c b/ssl/ssl_ecdh.c
index 4d7b63f..bc363e9 100644
--- a/ssl/ssl_ecdh.c
+++ b/ssl/ssl_ecdh.c
@@ -461,7 +461,7 @@
static const SSL_ECDH_METHOD kMethods[] = {
{
NID_X9_62_prime256v1,
- SSL_GROUP_SECP256R1,
+ SSL_CURVE_SECP256R1,
"P-256",
ssl_ec_point_cleanup,
ssl_ec_point_offer,
@@ -472,7 +472,7 @@
},
{
NID_secp384r1,
- SSL_GROUP_SECP384R1,
+ SSL_CURVE_SECP384R1,
"P-384",
ssl_ec_point_cleanup,
ssl_ec_point_offer,
@@ -483,7 +483,7 @@
},
{
NID_secp521r1,
- SSL_GROUP_SECP521R1,
+ SSL_CURVE_SECP521R1,
"P-521",
ssl_ec_point_cleanup,
ssl_ec_point_offer,
@@ -494,7 +494,7 @@
},
{
NID_X25519,
- SSL_GROUP_X25519,
+ SSL_CURVE_X25519,
"X25519",
ssl_x25519_cleanup,
ssl_x25519_offer,
diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 4f90c47..21e5207 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -1345,6 +1345,18 @@
curves_len);
}
+uint16_t SSL_get_curve_id(const SSL *ssl) {
+ /* TODO(davidben): This checks the wrong session if there is a renegotiation in
+ * progress. */
+ if (ssl->session == NULL ||
+ ssl->session->cipher == NULL ||
+ !SSL_CIPHER_is_ECDHE(ssl->session->cipher)) {
+ return 0;
+ }
+
+ return (uint16_t)ssl->session->key_exchange_info;
+}
+
int SSL_CTX_set_tmp_dh(SSL_CTX *ctx, const DH *dh) {
DH_free(ctx->cert->dh_tmp);
ctx->cert->dh_tmp = DHparams_dup(dh);
@@ -2267,6 +2279,18 @@
ssl->cert->dh_tmp_cb = callback;
}
+unsigned SSL_get_dhe_group_size(const SSL *ssl) {
+ /* TODO(davidben): This checks the wrong session if there is a renegotiation in
+ * progress. */
+ if (ssl->session == NULL ||
+ ssl->session->cipher == NULL ||
+ !SSL_CIPHER_is_DHE(ssl->session->cipher)) {
+ return 0;
+ }
+
+ return ssl->session->key_exchange_info;
+}
+
int SSL_CTX_use_psk_identity_hint(SSL_CTX *ctx, const char *identity_hint) {
if (identity_hint != NULL && strlen(identity_hint) > PSK_MAX_IDENTITY_LEN) {
OPENSSL_PUT_ERROR(SSL, SSL_R_DATA_LENGTH_TOO_LONG);
diff --git a/ssl/t1_lib.c b/ssl/t1_lib.c
index e733b48..9be9add 100644
--- a/ssl/t1_lib.c
+++ b/ssl/t1_lib.c
@@ -292,11 +292,11 @@
}
static const uint16_t kDefaultGroups[] = {
- SSL_GROUP_X25519,
- SSL_GROUP_SECP256R1,
- SSL_GROUP_SECP384R1,
+ SSL_CURVE_X25519,
+ SSL_CURVE_SECP256R1,
+ SSL_CURVE_SECP384R1,
#if defined(BORINGSSL_ANDROID_SYSTEM)
- SSL_GROUP_SECP521R1,
+ SSL_CURVE_SECP521R1,
#endif
};
diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index 99a5b93..8516977 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1137,11 +1137,21 @@
return false;
}
- if (config->expect_key_exchange_info != 0) {
- uint32_t info = SSL_SESSION_get_key_exchange_info(SSL_get_session(ssl));
- if (static_cast<uint32_t>(config->expect_key_exchange_info) != info) {
- fprintf(stderr, "key_exchange_info was %" PRIu32 ", wanted %" PRIu32 "\n",
- info, static_cast<uint32_t>(config->expect_key_exchange_info));
+ if (config->expect_curve_id != 0) {
+ uint16_t curve_id = SSL_get_curve_id(ssl);
+ if (static_cast<uint16_t>(config->expect_curve_id) != curve_id) {
+ fprintf(stderr, "curve_id was %04x, wanted %04x\n", curve_id,
+ static_cast<uint16_t>(config->expect_curve_id));
+ return false;
+ }
+ }
+
+ if (config->expect_dhe_group_size != 0) {
+ unsigned dhe_group_size = SSL_get_dhe_group_size(ssl);
+ if (static_cast<unsigned>(config->expect_dhe_group_size) !=
+ dhe_group_size) {
+ fprintf(stderr, "dhe_group_size was %u, wanted %d\n", dhe_group_size,
+ config->expect_dhe_group_size);
return false;
}
}
diff --git a/ssl/test/runner/runner.go b/ssl/test/runner/runner.go
index 32a6959..0ede7bb 100644
--- a/ssl/test/runner/runner.go
+++ b/ssl/test/runner/runner.go
@@ -5258,7 +5258,7 @@
DHGroupPrime: bigFromHex("0215C589A86BE450D1255A86D7A08877A70E124C11F0C75E476BA6A2186B1C830D4A132555973F2D5881D5F737BB800B7F417C01EC5960AEBF79478F8E0BBB6A021269BD10590C64C57F50AD8169D5488B56EE38DC5E02DA1A16ED3B5F41FEB2AD184B78A31F3A5B2BEC8441928343DA35DE3D4F89F0D4CEDE0034045084A0D1E6182E5EF7FCA325DD33CE81BE7FA87D43613E8FA7A1457099AB53"),
},
},
- flags: []string{"-expect-key-exchange-info", "1234"},
+ flags: []string{"-expect-dhe-group-size", "1234"},
})
testCases = append(testCases, testCase{
testType: serverTest,
@@ -5268,7 +5268,7 @@
CipherSuites: []uint16{TLS_DHE_RSA_WITH_AES_128_GCM_SHA256},
},
// bssl_shim as a server configures a 2048-bit DHE group.
- flags: []string{"-expect-key-exchange-info", "2048"},
+ flags: []string{"-expect-dhe-group-size", "2048"},
})
// TODO(davidben): Add TLS 1.3 versions of these tests once the
@@ -5281,7 +5281,7 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{CurveX25519},
},
- flags: []string{"-expect-key-exchange-info", "29", "-enable-all-curves"},
+ flags: []string{"-expect-curve-id", "29", "-enable-all-curves"},
})
testCases = append(testCases, testCase{
testType: serverTest,
@@ -5291,7 +5291,7 @@
CipherSuites: []uint16{TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256},
CurvePreferences: []CurveID{CurveX25519},
},
- flags: []string{"-expect-key-exchange-info", "29", "-enable-all-curves"},
+ flags: []string{"-expect-curve-id", "29", "-enable-all-curves"},
})
}
diff --git a/ssl/test/test_config.cc b/ssl/test/test_config.cc
index 536978b..cf3541d 100644
--- a/ssl/test/test_config.cc
+++ b/ssl/test/test_config.cc
@@ -147,8 +147,8 @@
{ "-expect-total-renegotiations", &TestConfig::expect_total_renegotiations },
{ "-expect-server-key-exchange-hash",
&TestConfig::expect_server_key_exchange_hash },
- { "-expect-key-exchange-info",
- &TestConfig::expect_key_exchange_info },
+ { "-expect-curve-id", &TestConfig::expect_curve_id },
+ { "-expect-dhe-group-size", &TestConfig::expect_dhe_group_size },
{ "-initial-timeout-duration-ms", &TestConfig::initial_timeout_duration_ms },
};
diff --git a/ssl/test/test_config.h b/ssl/test/test_config.h
index aff194e..b554f14 100644
--- a/ssl/test/test_config.h
+++ b/ssl/test/test_config.h
@@ -103,7 +103,8 @@
bool p384_only = false;
bool enable_all_curves = false;
bool use_sparse_dh_prime = false;
- int expect_key_exchange_info = 0;
+ int expect_curve_id = 0;
+ int expect_dhe_group_size = 0;
bool use_old_client_cert_callback = false;
int initial_timeout_duration_ms = 0;
};