Skip to content

Commit 335523a

Browse files
davidbenBoringssl LUCI CQ
davidben
authored and
Boringssl LUCI CQ
committed
Align remaining TLS ECDH APIs on "group" terminology
This adds "group" versions of our codepoint-based APIs. This is mostly because it looks weird to switch terminology randomly in the implementation. See I7a356793d36419fc668364c912ca7b4f5c6c23a2 for additional context. I've not bothered renaming the bssl_shim flags. That seems a waste of time. Change-Id: I566ad132f5a33d99efd8cb2bb8b76d9a6038c825 Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/60207 Reviewed-by: Adam Langley <[email protected]> Commit-Queue: David Benjamin <[email protected]>
1 parent 2da5ba9 commit 335523a

10 files changed

+98
-92
lines changed

‎include/openssl/ssl.h

+27-21
Original file line numberDiff line numberDiff line change
@@ -2353,15 +2353,15 @@ OPENSSL_EXPORT size_t SSL_CTX_get_num_tickets(const SSL_CTX *ctx);
23532353
// element of |groups| should be a |NID_*| constant from nid.h. It returns one
23542354
// on success and zero on failure.
23552355
//
2356-
// Note that this API does not use the |SSL_CURVE_*| values defined below.
2356+
// Note that this API does not use the |SSL_GROUP_*| values defined below.
23572357
OPENSSL_EXPORT int SSL_CTX_set1_groups(SSL_CTX *ctx, const int *groups,
23582358
size_t num_groups);
23592359

23602360
// SSL_set1_groups sets the preferred groups for |ssl| to be |groups|. Each
23612361
// element of |groups| should be a |NID_*| constant from nid.h. It returns one
23622362
// on success and zero on failure.
23632363
//
2364-
// Note that this API does not use the |SSL_CURVE_*| values defined below.
2364+
// Note that this API does not use the |SSL_GROUP_*| values defined below.
23652365
OPENSSL_EXPORT int SSL_set1_groups(SSL *ssl, const int *groups,
23662366
size_t num_groups);
23672367

@@ -2377,27 +2377,24 @@ OPENSSL_EXPORT int SSL_CTX_set1_groups_list(SSL_CTX *ctx, const char *groups);
23772377
// failure.
23782378
OPENSSL_EXPORT int SSL_set1_groups_list(SSL *ssl, const char *groups);
23792379

2380-
// SSL_CURVE_* define TLS curve IDs.
2381-
#define SSL_CURVE_SECP224R1 21
2382-
#define SSL_CURVE_SECP256R1 23
2383-
#define SSL_CURVE_SECP384R1 24
2384-
#define SSL_CURVE_SECP521R1 25
2385-
#define SSL_CURVE_X25519 29
2386-
#define SSL_CURVE_X25519_KYBER768_DRAFT00 0x6399
2380+
// SSL_GROUP_* define TLS group IDs.
2381+
#define SSL_GROUP_SECP224R1 21
2382+
#define SSL_GROUP_SECP256R1 23
2383+
#define SSL_GROUP_SECP384R1 24
2384+
#define SSL_GROUP_SECP521R1 25
2385+
#define SSL_GROUP_X25519 29
2386+
#define SSL_GROUP_X25519_KYBER768_DRAFT00 0x6399
23872387

2388-
// SSL_get_curve_id returns the ID of the curve used by |ssl|'s most recently
2389-
// completed handshake or 0 if not applicable.
2390-
//
2391-
// TODO(davidben): This API currently does not work correctly if there is a
2392-
// renegotiation in progress. Fix this.
2393-
OPENSSL_EXPORT uint16_t SSL_get_curve_id(const SSL *ssl);
2388+
// SSL_get_group_id returns the ID of the group used by |ssl|'s most recently
2389+
// completed handshake, or 0 if not applicable.
2390+
OPENSSL_EXPORT uint16_t SSL_get_group_id(const SSL *ssl);
23942391

2395-
// SSL_get_curve_name returns a human-readable name for the curve specified by
2396-
// the given TLS curve id, or NULL if the curve is unknown.
2397-
OPENSSL_EXPORT const char *SSL_get_curve_name(uint16_t curve_id);
2392+
// SSL_get_group_name returns a human-readable name for the group specified by
2393+
// the given TLS group ID, or NULL if the group is unknown.
2394+
OPENSSL_EXPORT const char *SSL_get_group_name(uint16_t group_id);
23982395

2399-
// SSL_get_all_curve_names outputs a list of possible strings
2400-
// |SSL_get_curve_name| may return in this version of BoringSSL. It writes at
2396+
// SSL_get_all_group_names outputs a list of possible strings
2397+
// |SSL_get_group_name| may return in this version of BoringSSL. It writes at
24012398
// most |max_out| entries to |out| and returns the total number it would have
24022399
// written, if |max_out| had been large enough. |max_out| may be initially set
24032400
// to zero to size the output.
@@ -2408,7 +2405,7 @@ OPENSSL_EXPORT const char *SSL_get_curve_name(uint16_t curve_id);
24082405
// placeholder, experimental, or deprecated values that do not apply to every
24092406
// caller. Future versions of BoringSSL may also return strings not in this
24102407
// list, so this does not apply if, say, sending strings across services.
2411-
OPENSSL_EXPORT size_t SSL_get_all_curve_names(const char **out, size_t max_out);
2408+
OPENSSL_EXPORT size_t SSL_get_all_group_names(const char **out, size_t max_out);
24122409

24132410

24142411
// Certificate verification.
@@ -5243,6 +5240,15 @@ OPENSSL_EXPORT int SSL_CTX_set_tlsext_status_arg(SSL_CTX *ctx, void *arg);
52435240
#define SSL_set1_curves SSL_set1_groups
52445241
#define SSL_CTX_set1_curves_list SSL_CTX_set1_groups_list
52455242
#define SSL_set1_curves_list SSL_set1_groups_list
5243+
#define SSL_get_curve_id SSL_get_group_id
5244+
#define SSL_get_curve_name SSL_get_group_name
5245+
#define SSL_get_all_curve_names SSL_get_all_group_names
5246+
#define SSL_CURVE_SECP224R1 SSL_GROUP_SECP224R1
5247+
#define SSL_CURVE_SECP256R1 SSL_GROUP_SECP256R1
5248+
#define SSL_CURVE_SECP384R1 SSL_GROUP_SECP384R1
5249+
#define SSL_CURVE_SECP521R1 SSL_GROUP_SECP521R1
5250+
#define SSL_CURVE_X25519 SSL_GROUP_X25519
5251+
#define SSL_CURVE_X25519_KYBER768_DRAFT00 SSL_GROUP_X25519_KYBER768_DRAFT00
52465252

52475253

52485254
// Compliance policy configurations

‎ssl/extensions.cc

+4-4
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ static bool tls1_check_duplicate_extensions(const CBS *cbs) {
206206

207207
static bool is_post_quantum_group(uint16_t id) {
208208
switch (id) {
209-
case SSL_CURVE_X25519_KYBER768_DRAFT00:
209+
case SSL_GROUP_X25519_KYBER768_DRAFT00:
210210
return true;
211211
default:
212212
return false;
@@ -307,9 +307,9 @@ bool ssl_client_hello_get_extension(const SSL_CLIENT_HELLO *client_hello,
307307
}
308308

309309
static const uint16_t kDefaultGroups[] = {
310-
SSL_CURVE_X25519,
311-
SSL_CURVE_SECP256R1,
312-
SSL_CURVE_SECP384R1,
310+
SSL_GROUP_X25519,
311+
SSL_GROUP_SECP256R1,
312+
SSL_GROUP_SECP384R1,
313313
};
314314

315315
Span<const uint16_t> tls1_get_grouplist(const SSL_HANDSHAKE *hs) {

‎ssl/handoff.cc

+20-20
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,12 @@ static bool serialize_features(CBB *out) {
5252
return false;
5353
}
5454
}
55-
CBB curves;
56-
if (!CBB_add_asn1(out, &curves, CBS_ASN1_OCTETSTRING)) {
55+
CBB groups;
56+
if (!CBB_add_asn1(out, &groups, CBS_ASN1_OCTETSTRING)) {
5757
return false;
5858
}
5959
for (const NamedGroup& g : NamedGroups()) {
60-
if (!CBB_add_u16(&curves, g.group_id)) {
60+
if (!CBB_add_u16(&groups, g.group_id)) {
6161
return false;
6262
}
6363
}
@@ -169,46 +169,46 @@ static bool apply_remote_features(SSL *ssl, CBS *in) {
169169
return false;
170170
}
171171

172-
CBS curves;
173-
if (!CBS_get_asn1(in, &curves, CBS_ASN1_OCTETSTRING)) {
172+
CBS groups;
173+
if (!CBS_get_asn1(in, &groups, CBS_ASN1_OCTETSTRING)) {
174174
return false;
175175
}
176-
Array<uint16_t> supported_curves;
177-
if (!supported_curves.Init(CBS_len(&curves) / 2)) {
176+
Array<uint16_t> supported_groups;
177+
if (!supported_groups.Init(CBS_len(&groups) / 2)) {
178178
return false;
179179
}
180180
size_t idx = 0;
181-
while (CBS_len(&curves)) {
182-
uint16_t curve;
183-
if (!CBS_get_u16(&curves, &curve)) {
181+
while (CBS_len(&groups)) {
182+
uint16_t group;
183+
if (!CBS_get_u16(&groups, &group)) {
184184
return false;
185185
}
186-
supported_curves[idx++] = curve;
186+
supported_groups[idx++] = group;
187187
}
188-
Span<const uint16_t> configured_curves =
188+
Span<const uint16_t> configured_groups =
189189
tls1_get_grouplist(ssl->s3->hs.get());
190-
Array<uint16_t> new_configured_curves;
191-
if (!new_configured_curves.Init(configured_curves.size())) {
190+
Array<uint16_t> new_configured_groups;
191+
if (!new_configured_groups.Init(configured_groups.size())) {
192192
return false;
193193
}
194194
idx = 0;
195-
for (uint16_t configured_curve : configured_curves) {
195+
for (uint16_t configured_group : configured_groups) {
196196
bool ok = false;
197-
for (uint16_t supported_curve : supported_curves) {
198-
if (supported_curve == configured_curve) {
197+
for (uint16_t supported_group : supported_groups) {
198+
if (supported_group == configured_group) {
199199
ok = true;
200200
break;
201201
}
202202
}
203203
if (ok) {
204-
new_configured_curves[idx++] = configured_curve;
204+
new_configured_groups[idx++] = configured_group;
205205
}
206206
}
207207
if (idx == 0) {
208208
return false;
209209
}
210-
new_configured_curves.Shrink(idx);
211-
ssl->config->supported_group_list = std::move(new_configured_curves);
210+
new_configured_groups.Shrink(idx);
211+
ssl->config->supported_group_list = std::move(new_configured_groups);
212212

213213
CBS alps;
214214
CBS_init(&alps, nullptr, 0);

‎ssl/handshake_server.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -483,7 +483,7 @@ static bool is_probably_jdk11_with_tls13(const SSL_CLIENT_HELLO *client_hello) {
483483
while (CBS_len(&supported_groups) > 0) {
484484
uint16_t group;
485485
if (!CBS_get_u16(&supported_groups, &group) ||
486-
group == SSL_CURVE_X25519) {
486+
group == SSL_GROUP_X25519) {
487487
return false;
488488
}
489489
}

‎ssl/ssl_key_share.cc

+20-20
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ class X25519KeyShare : public SSLKeyShare {
141141
public:
142142
X25519KeyShare() {}
143143

144-
uint16_t GroupID() const override { return SSL_CURVE_X25519; }
144+
uint16_t GroupID() const override { return SSL_GROUP_X25519; }
145145

146146
bool Generate(CBB *out) override {
147147
uint8_t public_key[32];
@@ -198,7 +198,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
198198
X25519Kyber768KeyShare() {}
199199

200200
uint16_t GroupID() const override {
201-
return SSL_CURVE_X25519_KYBER768_DRAFT00;
201+
return SSL_GROUP_X25519_KYBER768_DRAFT00;
202202
}
203203

204204
bool Generate(CBB *out) override {
@@ -285,12 +285,12 @@ class X25519Kyber768KeyShare : public SSLKeyShare {
285285
};
286286

287287
constexpr NamedGroup kNamedGroups[] = {
288-
{NID_secp224r1, SSL_CURVE_SECP224R1, "P-224", "secp224r1"},
289-
{NID_X9_62_prime256v1, SSL_CURVE_SECP256R1, "P-256", "prime256v1"},
290-
{NID_secp384r1, SSL_CURVE_SECP384R1, "P-384", "secp384r1"},
291-
{NID_secp521r1, SSL_CURVE_SECP521R1, "P-521", "secp521r1"},
292-
{NID_X25519, SSL_CURVE_X25519, "X25519", "x25519"},
293-
{NID_X25519Kyber768Draft00, SSL_CURVE_X25519_KYBER768_DRAFT00,
288+
{NID_secp224r1, SSL_GROUP_SECP224R1, "P-224", "secp224r1"},
289+
{NID_X9_62_prime256v1, SSL_GROUP_SECP256R1, "P-256", "prime256v1"},
290+
{NID_secp384r1, SSL_GROUP_SECP384R1, "P-384", "secp384r1"},
291+
{NID_secp521r1, SSL_GROUP_SECP521R1, "P-521", "secp521r1"},
292+
{NID_X25519, SSL_GROUP_X25519, "X25519", "x25519"},
293+
{NID_X25519Kyber768Draft00, SSL_GROUP_X25519_KYBER768_DRAFT00,
294294
"X25519Kyber768Draft00", ""},
295295
};
296296

@@ -302,17 +302,17 @@ Span<const NamedGroup> NamedGroups() {
302302

303303
UniquePtr<SSLKeyShare> SSLKeyShare::Create(uint16_t group_id) {
304304
switch (group_id) {
305-
case SSL_CURVE_SECP224R1:
306-
return MakeUnique<ECKeyShare>(NID_secp224r1, SSL_CURVE_SECP224R1);
307-
case SSL_CURVE_SECP256R1:
308-
return MakeUnique<ECKeyShare>(NID_X9_62_prime256v1, SSL_CURVE_SECP256R1);
309-
case SSL_CURVE_SECP384R1:
310-
return MakeUnique<ECKeyShare>(NID_secp384r1, SSL_CURVE_SECP384R1);
311-
case SSL_CURVE_SECP521R1:
312-
return MakeUnique<ECKeyShare>(NID_secp521r1, SSL_CURVE_SECP521R1);
313-
case SSL_CURVE_X25519:
305+
case SSL_GROUP_SECP224R1:
306+
return MakeUnique<ECKeyShare>(NID_secp224r1, SSL_GROUP_SECP224R1);
307+
case SSL_GROUP_SECP256R1:
308+
return MakeUnique<ECKeyShare>(NID_X9_62_prime256v1, SSL_GROUP_SECP256R1);
309+
case SSL_GROUP_SECP384R1:
310+
return MakeUnique<ECKeyShare>(NID_secp384r1, SSL_GROUP_SECP384R1);
311+
case SSL_GROUP_SECP521R1:
312+
return MakeUnique<ECKeyShare>(NID_secp521r1, SSL_GROUP_SECP521R1);
313+
case SSL_GROUP_X25519:
314314
return MakeUnique<X25519KeyShare>();
315-
case SSL_CURVE_X25519_KYBER768_DRAFT00:
315+
case SSL_GROUP_X25519_KYBER768_DRAFT00:
316316
return MakeUnique<X25519Kyber768KeyShare>();
317317
default:
318318
return nullptr;
@@ -349,7 +349,7 @@ BSSL_NAMESPACE_END
349349

350350
using namespace bssl;
351351

352-
const char* SSL_get_curve_name(uint16_t group_id) {
352+
const char* SSL_get_group_name(uint16_t group_id) {
353353
for (const auto &group : kNamedGroups) {
354354
if (group.group_id == group_id) {
355355
return group.name;
@@ -358,7 +358,7 @@ const char* SSL_get_curve_name(uint16_t group_id) {
358358
return nullptr;
359359
}
360360

361-
size_t SSL_get_all_curve_names(const char **out, size_t max_out) {
361+
size_t SSL_get_all_group_names(const char **out, size_t max_out) {
362362
return GetAllNames(out, max_out, Span<const char *>(), &NamedGroup::name,
363363
MakeConstSpan(kNamedGroups));
364364
}

‎ssl/ssl_lib.cc

+1-1
Original file line numberDiff line numberDiff line change
@@ -2016,7 +2016,7 @@ int SSL_set1_groups_list(SSL *ssl, const char *groups) {
20162016
return ssl_str_to_group_ids(&ssl->config->supported_group_list, groups);
20172017
}
20182018

2019-
uint16_t SSL_get_curve_id(const SSL *ssl) {
2019+
uint16_t SSL_get_group_id(const SSL *ssl) {
20202020
SSL_SESSION *session = SSL_get_session(ssl);
20212021
if (session == NULL) {
20222022
return 0;

‎ssl/ssl_test.cc

+13-13
Original file line numberDiff line numberDiff line change
@@ -478,29 +478,29 @@ static const char* kShouldIncludeCBCSHA256[] = {
478478
static const CurveTest kCurveTests[] = {
479479
{
480480
"P-256",
481-
{ SSL_CURVE_SECP256R1 },
481+
{ SSL_GROUP_SECP256R1 },
482482
},
483483
{
484484
"P-256:X25519Kyber768Draft00",
485-
{ SSL_CURVE_SECP256R1, SSL_CURVE_X25519_KYBER768_DRAFT00 },
485+
{ SSL_GROUP_SECP256R1, SSL_GROUP_X25519_KYBER768_DRAFT00 },
486486
},
487487

488488
{
489489
"P-256:P-384:P-521:X25519",
490490
{
491-
SSL_CURVE_SECP256R1,
492-
SSL_CURVE_SECP384R1,
493-
SSL_CURVE_SECP521R1,
494-
SSL_CURVE_X25519,
491+
SSL_GROUP_SECP256R1,
492+
SSL_GROUP_SECP384R1,
493+
SSL_GROUP_SECP521R1,
494+
SSL_GROUP_X25519,
495495
},
496496
},
497497
{
498498
"prime256v1:secp384r1:secp521r1:x25519",
499499
{
500-
SSL_CURVE_SECP256R1,
501-
SSL_CURVE_SECP384R1,
502-
SSL_CURVE_SECP521R1,
503-
SSL_CURVE_X25519,
500+
SSL_GROUP_SECP256R1,
501+
SSL_GROUP_SECP384R1,
502+
SSL_GROUP_SECP521R1,
503+
SSL_GROUP_X25519,
504504
},
505505
},
506506
};
@@ -5902,7 +5902,7 @@ TEST_P(SSLVersionTest, SessionPropertiesThreads) {
59025902
bssl::UniquePtr<X509> peer(SSL_get_peer_certificate(ssl));
59035903
EXPECT_TRUE(peer);
59045904
EXPECT_TRUE(SSL_get_current_cipher(ssl));
5905-
EXPECT_TRUE(SSL_get_curve_id(ssl));
5905+
EXPECT_TRUE(SSL_get_group_id(ssl));
59065906
};
59075907

59085908
std::vector<std::thread> threads;
@@ -7754,7 +7754,7 @@ TEST(SSLTest, ConnectionPropertiesDuringRenegotiate) {
77547754
ASSERT_TRUE(cipher);
77557755
EXPECT_EQ(SSL_CIPHER_get_id(cipher),
77567756
uint32_t{TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256});
7757-
EXPECT_EQ(SSL_get_curve_id(client.get()), SSL_CURVE_X25519);
7757+
EXPECT_EQ(SSL_get_group_id(client.get()), SSL_GROUP_X25519);
77587758
EXPECT_EQ(SSL_get_peer_signature_algorithm(client.get()),
77597759
SSL_SIGN_RSA_PKCS1_SHA256);
77607760
bssl::UniquePtr<X509> peer(SSL_get_peer_certificate(client.get()));
@@ -8726,7 +8726,7 @@ TEST(SSLTest, NameLists) {
87268726
{"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_AES_128_GCM_SHA256"}},
87278727
{SSL_get_all_cipher_names,
87288728
{"ECDHE-ECDSA-AES128-GCM-SHA256", "TLS_AES_128_GCM_SHA256", "(NONE)"}},
8729-
{SSL_get_all_curve_names, {"P-256", "X25519"}},
8729+
{SSL_get_all_group_names, {"P-256", "X25519"}},
87308730
{SSL_get_all_signature_algorithm_names,
87318731
{"rsa_pkcs1_sha256", "ecdsa_secp256r1_sha256", "ecdsa_sha256"}},
87328732
};

‎ssl/test/bssl_shim.cc

+3-3
Original file line numberDiff line numberDiff line change
@@ -692,9 +692,9 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume,
692692
SSL_CIPHER_standard_name(SSL_get_current_cipher(ssl))) ||
693693
!CheckListContains("OpenSSL cipher name", SSL_get_all_cipher_names,
694694
SSL_CIPHER_get_name(SSL_get_current_cipher(ssl))) ||
695-
(SSL_get_curve_id(ssl) != 0 &&
696-
!CheckListContains("curve", SSL_get_all_curve_names,
697-
SSL_get_curve_name(SSL_get_curve_id(ssl)))) ||
695+
(SSL_get_group_id(ssl) != 0 &&
696+
!CheckListContains("group", SSL_get_all_group_names,
697+
SSL_get_group_name(SSL_get_group_id(ssl)))) ||
698698
(SSL_get_peer_signature_algorithm(ssl) != 0 &&
699699
!CheckListContains(
700700
"sigalg", SSL_get_all_signature_algorithm_names,

0 commit comments

Comments
 (0)