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/include/openssl/ssl.h b/include/openssl/ssl.h
index 80988b0..641b94a 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1506,13 +1506,6 @@
/* SSL_SESSION_get_timeout returns the lifetime of |session| in seconds. */
OPENSSL_EXPORT long SSL_SESSION_get_timeout(const SSL_SESSION *session);
-/* SSL_SESSION_get_key_exchange_info returns a value that describes the
- * strength of the asymmetric operation that provides confidentiality to
- * |session|. Its interpretation depends on the operation used. See the
- * documentation for this value in the |SSL_SESSION| structure. */
-OPENSSL_EXPORT uint32_t SSL_SESSION_get_key_exchange_info(
- const SSL_SESSION *session);
-
/* SSL_SESSION_get0_peer return's the peer leaf certificate stored in
* |session|.
*
@@ -1837,35 +1830,50 @@
* are supported. ECDHE is always enabled, but the curve preferences may be
* configured with these functions.
*
- * A client may use |SSL_SESSION_get_key_exchange_info| to determine the curve
- * selected. */
+ * Note that TLS 1.3 renames these from curves to groups. For consistency, we
+ * currently use the TLS 1.2 name in the API. */
/* SSL_CTX_set1_curves sets the preferred curves for |ctx| to be |curves|. Each
* element of |curves| should be a curve nid. It returns one on success and
- * zero on failure. */
+ * zero on failure.
+ *
+ * Note that this API uses nid values from nid.h and not the |SSL_CURVE_*|
+ * values defined below. */
OPENSSL_EXPORT int SSL_CTX_set1_curves(SSL_CTX *ctx, const int *curves,
size_t curves_len);
/* SSL_set1_curves sets the preferred curves for |ssl| to be |curves|. Each
* element of |curves| should be a curve nid. It returns one on success and
- * zero on failure. */
+ * zero on failure.
+ *
+ * Note that this API uses nid values from nid.h and not the |SSL_CURVE_*|
+ * values defined below. */
OPENSSL_EXPORT int SSL_set1_curves(SSL *ssl, const int *curves,
size_t curves_len);
-/* SSL_get_curve_name returns a human-readable name for the group specified by
- * the given TLS group id, or NULL if the group is unknown. */
-OPENSSL_EXPORT const char *SSL_get_curve_name(uint16_t group_id);
+/* SSL_CURVE_* define TLS curve IDs. */
+#define SSL_CURVE_SECP256R1 23
+#define SSL_CURVE_SECP384R1 24
+#define SSL_CURVE_SECP521R1 25
+#define SSL_CURVE_X25519 29
+
+/* SSL_get_curve_id returns the ID of the curve used by |ssl|'s most recently
+ * completed handshake or 0 if not applicable.
+ *
+ * TODO(davidben): This API currently does not work correctly if there is a
+ * renegotiation in progress. Fix this. */
+OPENSSL_EXPORT uint16_t SSL_get_curve_id(const SSL *ssl);
+
+/* SSL_get_curve_name returns a human-readable name for the curve specified by
+ * the given TLS curve id, or NULL if the curve is unknown. */
+OPENSSL_EXPORT const char *SSL_get_curve_name(uint16_t curve_id);
/* Multiplicative Diffie-Hellman.
*
* Cipher suites using a DHE key exchange perform Diffie-Hellman over a
* multiplicative group selected by the server. These ciphers are disabled for a
- * server unless a group is chosen with one of these functions.
- *
- * A client may use |SSL_SESSION_get_key_exchange_info| to determine the size of
- * the selected group's prime, but note that servers may select degenerate
- * groups. */
+ * server unless a group is chosen with one of these functions. */
/* SSL_CTX_set_tmp_dh configures |ctx| to use the group from |dh| as the group
* for DHE. Only the group is used, so |dh| needn't have a keypair. It returns
@@ -1898,6 +1906,15 @@
DH *(*dh)(SSL *ssl, int is_export,
int keylength));
+/* SSL_get_dhe_group_size returns the number of bits in the most recently
+ * completed handshake's selected group's prime, or zero if not
+ * applicable. Note, however, that validating this value does not ensure the
+ * server selected a secure group.
+ *
+ * TODO(davidben): This API currently does not work correctly if there is a
+ * renegotiation in progress. Fix this. */
+OPENSSL_EXPORT unsigned SSL_get_dhe_group_size(const SSL *ssl);
+
/* Certificate verification.
*
@@ -3458,6 +3475,18 @@
OPENSSL_EXPORT int SSL_add_dir_cert_subjects_to_stack(STACK_OF(X509_NAME) *out,
const char *dir);
+/* SSL_SESSION_get_key_exchange_info returns a value that describes the
+ * strength of the asymmetric operation that provides confidentiality to
+ * |session|. Its interpretation depends on the operation used. See the
+ * documentation for this value in the |SSL_SESSION| structure.
+ *
+ * Use |SSL_get_curve_id| or |SSL_get_dhe_group_size| instead.
+ *
+ * TODO(davidben): Remove this API once Chromium has switched to the new
+ * APIs. */
+OPENSSL_EXPORT uint32_t SSL_SESSION_get_key_exchange_info(
+ const SSL_SESSION *session);
+
/* Private structures.
*
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;
};