diff --git a/src/fuzz/ssl_ctx_api.cc b/src/fuzz/ssl_ctx_api.cc index f7c1f7350..bbf7c71a6 100644 --- a/src/fuzz/ssl_ctx_api.cc +++ b/src/fuzz/ssl_ctx_api.cc @@ -416,6 +416,13 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t *buf, size_t len) { } SSL_CTX_set1_groups(ctx, groups.data(), groups.size()); }, + [](SSL_CTX *ctx, CBS *cbs) { + std::vector groups; + if (!GetVector(&groups, cbs)) { + return; + } + SSL_CTX_set1_group_ids(ctx, groups.data(), groups.size()); + }, [](SSL_CTX *ctx, CBS *cbs) { std::string groups; if (!GetString(&groups, cbs)) { diff --git a/src/include/openssl/base.h b/src/include/openssl/base.h index 3141676e1..aa5c63d08 100644 --- a/src/include/openssl/base.h +++ b/src/include/openssl/base.h @@ -197,7 +197,7 @@ extern "C" { // A consumer may use this symbol in the preprocessor to temporarily build // against multiple revisions of BoringSSL at the same time. It is not // recommended to do so for longer than is necessary. -#define BORINGSSL_API_VERSION 21 +#define BORINGSSL_API_VERSION 22 #if defined(BORINGSSL_SHARED_LIBRARY) diff --git a/src/include/openssl/ssl.h b/src/include/openssl/ssl.h index 31add5539..7974b27c6 100644 --- a/src/include/openssl/ssl.h +++ b/src/include/openssl/ssl.h @@ -2349,55 +2349,37 @@ OPENSSL_EXPORT size_t SSL_CTX_get_num_tickets(const SSL_CTX *ctx); // TLS 1.3 and later, ECDSA curves are part of the signature algorithm. See // |SSL_SIGN_*|. -// SSL_CTX_set1_groups sets the preferred groups for |ctx| to be |groups|. Each -// element of |groups| should be a |NID_*| constant from nid.h. It returns one -// on success and zero on failure. -// -// Note that this API does not use the |SSL_CURVE_*| values defined below. -OPENSSL_EXPORT int SSL_CTX_set1_groups(SSL_CTX *ctx, const int *groups, - size_t num_groups); +// SSL_GROUP_* define TLS group IDs. +#define SSL_GROUP_SECP224R1 21 +#define SSL_GROUP_SECP256R1 23 +#define SSL_GROUP_SECP384R1 24 +#define SSL_GROUP_SECP521R1 25 +#define SSL_GROUP_X25519 29 +#define SSL_GROUP_X25519_KYBER768_DRAFT00 0x6399 -// SSL_set1_groups sets the preferred groups for |ssl| to be |groups|. Each -// element of |groups| should be a |NID_*| constant from nid.h. It returns one -// on success and zero on failure. -// -// Note that this API does not use the |SSL_CURVE_*| values defined below. -OPENSSL_EXPORT int SSL_set1_groups(SSL *ssl, const int *groups, - size_t num_groups); +// SSL_CTX_set1_group_ids sets the preferred groups for |ctx| to |group_ids|. +// Each element of |group_ids| should be one of the |SSL_GROUP_*| constants. It +// returns one on success and zero on failure. +OPENSSL_EXPORT int SSL_CTX_set1_group_ids(SSL_CTX *ctx, + const uint16_t *group_ids, + size_t num_group_ids); -// SSL_CTX_set1_groups_list sets the preferred groups for |ctx| to be the -// colon-separated list |groups|. Each element of |groups| should be a curve -// name (e.g. P-256, X25519, ...). It returns one on success and zero on -// failure. -OPENSSL_EXPORT int SSL_CTX_set1_groups_list(SSL_CTX *ctx, const char *groups); +// SSL_set1_group_ids sets the preferred groups for |ssl| to |group_ids|. Each +// element of |group_ids| should be one of the |SSL_GROUP_*| constants. It +// returns one on success and zero on failure. +OPENSSL_EXPORT int SSL_set1_group_ids(SSL *ssl, const uint16_t *group_ids, + size_t num_group_ids); -// SSL_set1_groups_list sets the preferred groups for |ssl| to be the -// colon-separated list |groups|. Each element of |groups| should be a curve -// name (e.g. P-256, X25519, ...). It returns one on success and zero on -// failure. -OPENSSL_EXPORT int SSL_set1_groups_list(SSL *ssl, const char *groups); +// SSL_get_group_id returns the ID of the group used by |ssl|'s most recently +// completed handshake, or 0 if not applicable. +OPENSSL_EXPORT uint16_t SSL_get_group_id(const SSL *ssl); -// SSL_CURVE_* define TLS curve IDs. -#define SSL_CURVE_SECP224R1 21 -#define SSL_CURVE_SECP256R1 23 -#define SSL_CURVE_SECP384R1 24 -#define SSL_CURVE_SECP521R1 25 -#define SSL_CURVE_X25519 29 -#define SSL_CURVE_X25519_KYBER768_DRAFT00 0x6399 +// SSL_get_group_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_group_name(uint16_t group_id); -// 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); - -// SSL_get_all_curve_names outputs a list of possible strings -// |SSL_get_curve_name| may return in this version of BoringSSL. It writes at +// SSL_get_all_group_names outputs a list of possible strings +// |SSL_get_group_name| may return in this version of BoringSSL. It writes at // most |max_out| entries to |out| and returns the total number it would have // written, if |max_out| had been large enough. |max_out| may be initially set // to zero to size the output. @@ -2408,7 +2390,40 @@ OPENSSL_EXPORT const char *SSL_get_curve_name(uint16_t curve_id); // placeholder, experimental, or deprecated values that do not apply to every // caller. Future versions of BoringSSL may also return strings not in this // list, so this does not apply if, say, sending strings across services. -OPENSSL_EXPORT size_t SSL_get_all_curve_names(const char **out, size_t max_out); +OPENSSL_EXPORT size_t SSL_get_all_group_names(const char **out, size_t max_out); + +// The following APIs also configure Diffie-Hellman groups, but use |NID_*| +// constants instead of |SSL_GROUP_*| constants. These are provided for OpenSSL +// compatibility. Where NIDs are unstable constants specific to OpenSSL and +// BoringSSL, group IDs are defined by the TLS protocol. Prefer the group ID +// representation if storing persistently, or exporting to another process or +// library. + +// SSL_CTX_set1_groups sets the preferred groups for |ctx| to be |groups|. Each +// element of |groups| should be a |NID_*| constant from nid.h. It returns one +// on success and zero on failure. +OPENSSL_EXPORT int SSL_CTX_set1_groups(SSL_CTX *ctx, const int *groups, + size_t num_groups); + +// SSL_set1_groups sets the preferred groups for |ssl| to be |groups|. Each +// element of |groups| should be a |NID_*| constant from nid.h. It returns one +// on success and zero on failure. +OPENSSL_EXPORT int SSL_set1_groups(SSL *ssl, const int *groups, + size_t num_groups); + +// SSL_CTX_set1_groups_list decodes |groups| as a colon-separated list of group +// names (e.g. "X25519" or "P-256") and sets |ctx|'s preferred groups to the +// result. It returns one on success and zero on failure. +OPENSSL_EXPORT int SSL_CTX_set1_groups_list(SSL_CTX *ctx, const char *groups); + +// SSL_set1_groups_list decodes |groups| as a colon-separated list of group +// names (e.g. "X25519" or "P-256") and sets |ssl|'s preferred groups to the +// result. It returns one on success and zero on failure. +OPENSSL_EXPORT int SSL_set1_groups_list(SSL *ssl, const char *groups); + +// SSL_get_negotiated_group returns the NID of the group used by |ssl|'s most +// recently completed handshake, or |NID_undef| if not applicable. +OPENSSL_EXPORT int SSL_get_negotiated_group(const SSL *ssl); // Certificate verification. @@ -5243,6 +5258,15 @@ OPENSSL_EXPORT int SSL_CTX_set_tlsext_status_arg(SSL_CTX *ctx, void *arg); #define SSL_set1_curves SSL_set1_groups #define SSL_CTX_set1_curves_list SSL_CTX_set1_groups_list #define SSL_set1_curves_list SSL_set1_groups_list +#define SSL_get_curve_id SSL_get_group_id +#define SSL_get_curve_name SSL_get_group_name +#define SSL_get_all_curve_names SSL_get_all_group_names +#define SSL_CURVE_SECP224R1 SSL_GROUP_SECP224R1 +#define SSL_CURVE_SECP256R1 SSL_GROUP_SECP256R1 +#define SSL_CURVE_SECP384R1 SSL_GROUP_SECP384R1 +#define SSL_CURVE_SECP521R1 SSL_GROUP_SECP521R1 +#define SSL_CURVE_X25519 SSL_GROUP_X25519 +#define SSL_CURVE_X25519_KYBER768_DRAFT00 SSL_GROUP_X25519_KYBER768_DRAFT00 // Compliance policy configurations @@ -5344,6 +5368,7 @@ OPENSSL_EXPORT int SSL_set_compliance_policy( #define SSL_CTRL_GET_CLIENT_CERT_TYPES doesnt_exist #define SSL_CTRL_GET_EXTRA_CHAIN_CERTS doesnt_exist #define SSL_CTRL_GET_MAX_CERT_LIST doesnt_exist +#define SSL_CTRL_GET_NEGOTIATED_GROUP doesnt_exist #define SSL_CTRL_GET_NUM_RENEGOTIATIONS doesnt_exist #define SSL_CTRL_GET_READ_AHEAD doesnt_exist #define SSL_CTRL_GET_RI_SUPPORT doesnt_exist @@ -5434,6 +5459,7 @@ OPENSSL_EXPORT int SSL_set_compliance_policy( #define SSL_get0_chain_certs SSL_get0_chain_certs #define SSL_get_max_cert_list SSL_get_max_cert_list #define SSL_get_mode SSL_get_mode +#define SSL_get_negotiated_group SSL_get_negotiated_group #define SSL_get_options SSL_get_options #define SSL_get_secure_renegotiation_support \ SSL_get_secure_renegotiation_support diff --git a/src/ssl/extensions.cc b/src/ssl/extensions.cc index 974a36c38..c5b1ed143 100644 --- a/src/ssl/extensions.cc +++ b/src/ssl/extensions.cc @@ -206,7 +206,7 @@ static bool tls1_check_duplicate_extensions(const CBS *cbs) { static bool is_post_quantum_group(uint16_t id) { switch (id) { - case SSL_CURVE_X25519_KYBER768_DRAFT00: + case SSL_GROUP_X25519_KYBER768_DRAFT00: return true; default: return false; @@ -307,9 +307,9 @@ bool ssl_client_hello_get_extension(const SSL_CLIENT_HELLO *client_hello, } static const uint16_t kDefaultGroups[] = { - SSL_CURVE_X25519, - SSL_CURVE_SECP256R1, - SSL_CURVE_SECP384R1, + SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, + SSL_GROUP_SECP384R1, }; Span tls1_get_grouplist(const SSL_HANDSHAKE *hs) { diff --git a/src/ssl/handoff.cc b/src/ssl/handoff.cc index 6e5cc2da1..a4563c7eb 100644 --- a/src/ssl/handoff.cc +++ b/src/ssl/handoff.cc @@ -52,12 +52,12 @@ static bool serialize_features(CBB *out) { return false; } } - CBB curves; - if (!CBB_add_asn1(out, &curves, CBS_ASN1_OCTETSTRING)) { + CBB groups; + if (!CBB_add_asn1(out, &groups, CBS_ASN1_OCTETSTRING)) { return false; } for (const NamedGroup& g : NamedGroups()) { - if (!CBB_add_u16(&curves, g.group_id)) { + if (!CBB_add_u16(&groups, g.group_id)) { return false; } } @@ -169,46 +169,46 @@ static bool apply_remote_features(SSL *ssl, CBS *in) { return false; } - CBS curves; - if (!CBS_get_asn1(in, &curves, CBS_ASN1_OCTETSTRING)) { + CBS groups; + if (!CBS_get_asn1(in, &groups, CBS_ASN1_OCTETSTRING)) { return false; } - Array supported_curves; - if (!supported_curves.Init(CBS_len(&curves) / 2)) { + Array supported_groups; + if (!supported_groups.Init(CBS_len(&groups) / 2)) { return false; } size_t idx = 0; - while (CBS_len(&curves)) { - uint16_t curve; - if (!CBS_get_u16(&curves, &curve)) { + while (CBS_len(&groups)) { + uint16_t group; + if (!CBS_get_u16(&groups, &group)) { return false; } - supported_curves[idx++] = curve; + supported_groups[idx++] = group; } - Span configured_curves = + Span configured_groups = tls1_get_grouplist(ssl->s3->hs.get()); - Array new_configured_curves; - if (!new_configured_curves.Init(configured_curves.size())) { + Array new_configured_groups; + if (!new_configured_groups.Init(configured_groups.size())) { return false; } idx = 0; - for (uint16_t configured_curve : configured_curves) { + for (uint16_t configured_group : configured_groups) { bool ok = false; - for (uint16_t supported_curve : supported_curves) { - if (supported_curve == configured_curve) { + for (uint16_t supported_group : supported_groups) { + if (supported_group == configured_group) { ok = true; break; } } if (ok) { - new_configured_curves[idx++] = configured_curve; + new_configured_groups[idx++] = configured_group; } } if (idx == 0) { return false; } - new_configured_curves.Shrink(idx); - ssl->config->supported_group_list = std::move(new_configured_curves); + new_configured_groups.Shrink(idx); + ssl->config->supported_group_list = std::move(new_configured_groups); CBS alps; CBS_init(&alps, nullptr, 0); diff --git a/src/ssl/handshake_server.cc b/src/ssl/handshake_server.cc index e50a69021..cffa52d88 100644 --- a/src/ssl/handshake_server.cc +++ b/src/ssl/handshake_server.cc @@ -483,7 +483,7 @@ static bool is_probably_jdk11_with_tls13(const SSL_CLIENT_HELLO *client_hello) { while (CBS_len(&supported_groups) > 0) { uint16_t group; if (!CBS_get_u16(&supported_groups, &group) || - group == SSL_CURVE_X25519) { + group == SSL_GROUP_X25519) { return false; } } diff --git a/src/ssl/internal.h b/src/ssl/internal.h index c95b8fab7..fa35073fa 100644 --- a/src/ssl/internal.h +++ b/src/ssl/internal.h @@ -1148,6 +1148,10 @@ bool ssl_nid_to_group_id(uint16_t *out_group_id, int nid); // true. Otherwise, it returns false. bool ssl_name_to_group_id(uint16_t *out_group_id, const char *name, size_t len); +// ssl_group_id_to_nid returns the NID corresponding to |group_id| or +// |NID_undef| if unknown. +int ssl_group_id_to_nid(uint16_t group_id); + // Handshake messages. diff --git a/src/ssl/ssl_key_share.cc b/src/ssl/ssl_key_share.cc index 77f16b5bf..d932ef385 100644 --- a/src/ssl/ssl_key_share.cc +++ b/src/ssl/ssl_key_share.cc @@ -141,7 +141,7 @@ class X25519KeyShare : public SSLKeyShare { public: X25519KeyShare() {} - uint16_t GroupID() const override { return SSL_CURVE_X25519; } + uint16_t GroupID() const override { return SSL_GROUP_X25519; } bool Generate(CBB *out) override { uint8_t public_key[32]; @@ -198,7 +198,7 @@ class X25519Kyber768KeyShare : public SSLKeyShare { X25519Kyber768KeyShare() {} uint16_t GroupID() const override { - return SSL_CURVE_X25519_KYBER768_DRAFT00; + return SSL_GROUP_X25519_KYBER768_DRAFT00; } bool Generate(CBB *out) override { @@ -285,12 +285,12 @@ class X25519Kyber768KeyShare : public SSLKeyShare { }; constexpr NamedGroup kNamedGroups[] = { - {NID_secp224r1, SSL_CURVE_SECP224R1, "P-224", "secp224r1"}, - {NID_X9_62_prime256v1, SSL_CURVE_SECP256R1, "P-256", "prime256v1"}, - {NID_secp384r1, SSL_CURVE_SECP384R1, "P-384", "secp384r1"}, - {NID_secp521r1, SSL_CURVE_SECP521R1, "P-521", "secp521r1"}, - {NID_X25519, SSL_CURVE_X25519, "X25519", "x25519"}, - {NID_X25519Kyber768Draft00, SSL_CURVE_X25519_KYBER768_DRAFT00, + {NID_secp224r1, SSL_GROUP_SECP224R1, "P-224", "secp224r1"}, + {NID_X9_62_prime256v1, SSL_GROUP_SECP256R1, "P-256", "prime256v1"}, + {NID_secp384r1, SSL_GROUP_SECP384R1, "P-384", "secp384r1"}, + {NID_secp521r1, SSL_GROUP_SECP521R1, "P-521", "secp521r1"}, + {NID_X25519, SSL_GROUP_X25519, "X25519", "x25519"}, + {NID_X25519Kyber768Draft00, SSL_GROUP_X25519_KYBER768_DRAFT00, "X25519Kyber768Draft00", ""}, }; @@ -302,17 +302,17 @@ Span NamedGroups() { UniquePtr SSLKeyShare::Create(uint16_t group_id) { switch (group_id) { - case SSL_CURVE_SECP224R1: - return MakeUnique(NID_secp224r1, SSL_CURVE_SECP224R1); - case SSL_CURVE_SECP256R1: - return MakeUnique(NID_X9_62_prime256v1, SSL_CURVE_SECP256R1); - case SSL_CURVE_SECP384R1: - return MakeUnique(NID_secp384r1, SSL_CURVE_SECP384R1); - case SSL_CURVE_SECP521R1: - return MakeUnique(NID_secp521r1, SSL_CURVE_SECP521R1); - case SSL_CURVE_X25519: + case SSL_GROUP_SECP224R1: + return MakeUnique(NID_secp224r1, SSL_GROUP_SECP224R1); + case SSL_GROUP_SECP256R1: + return MakeUnique(NID_X9_62_prime256v1, SSL_GROUP_SECP256R1); + case SSL_GROUP_SECP384R1: + return MakeUnique(NID_secp384r1, SSL_GROUP_SECP384R1); + case SSL_GROUP_SECP521R1: + return MakeUnique(NID_secp521r1, SSL_GROUP_SECP521R1); + case SSL_GROUP_X25519: return MakeUnique(); - case SSL_CURVE_X25519_KYBER768_DRAFT00: + case SSL_GROUP_X25519_KYBER768_DRAFT00: return MakeUnique(); default: return nullptr; @@ -345,11 +345,20 @@ bool ssl_name_to_group_id(uint16_t *out_group_id, const char *name, size_t len) return false; } +int ssl_group_id_to_nid(uint16_t group_id) { + for (const auto &group : kNamedGroups) { + if (group.group_id == group_id) { + return group.nid; + } + } + return NID_undef; +} + BSSL_NAMESPACE_END using namespace bssl; -const char* SSL_get_curve_name(uint16_t group_id) { +const char* SSL_get_group_name(uint16_t group_id) { for (const auto &group : kNamedGroups) { if (group.group_id == group_id) { return group.name; @@ -358,7 +367,7 @@ const char* SSL_get_curve_name(uint16_t group_id) { return nullptr; } -size_t SSL_get_all_curve_names(const char **out, size_t max_out) { +size_t SSL_get_all_group_names(const char **out, size_t max_out) { return GetAllNames(out, max_out, Span(), &NamedGroup::name, MakeConstSpan(kNamedGroups)); } diff --git a/src/ssl/ssl_lib.cc b/src/ssl/ssl_lib.cc index 066fe69f7..d7b5be3dd 100644 --- a/src/ssl/ssl_lib.cc +++ b/src/ssl/ssl_lib.cc @@ -1939,6 +1939,32 @@ int SSL_CTX_set_tlsext_ticket_key_cb( return 1; } +static bool check_group_ids(Span group_ids) { + for (uint16_t group_id : group_ids) { + if (ssl_group_id_to_nid(group_id) == NID_undef) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); + return false; + } + } + return true; +} + +int SSL_CTX_set1_group_ids(SSL_CTX *ctx, const uint16_t *group_ids, + size_t num_group_ids) { + auto span = MakeConstSpan(group_ids, num_group_ids); + return check_group_ids(span) && ctx->supported_group_list.CopyFrom(span); +} + +int SSL_set1_group_ids(SSL *ssl, const uint16_t *group_ids, + size_t num_group_ids) { + if (!ssl->config) { + return 0; + } + auto span = MakeConstSpan(group_ids, num_group_ids); + return check_group_ids(span) && + ssl->config->supported_group_list.CopyFrom(span); +} + static bool ssl_nids_to_group_ids(Array *out_group_ids, Span nids) { Array group_ids; @@ -1948,6 +1974,7 @@ static bool ssl_nids_to_group_ids(Array *out_group_ids, for (size_t i = 0; i < nids.size(); i++) { if (!ssl_nid_to_group_id(&group_ids[i], nids[i])) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); return false; } } @@ -1993,6 +2020,7 @@ static bool ssl_str_to_group_ids(Array *out_group_ids, col = strchr(ptr, ':'); if (!ssl_name_to_group_id(&group_ids[i++], ptr, col ? (size_t)(col - ptr) : strlen(ptr))) { + OPENSSL_PUT_ERROR(SSL, SSL_R_UNSUPPORTED_ELLIPTIC_CURVE); return false; } if (col) { @@ -2016,7 +2044,7 @@ int SSL_set1_groups_list(SSL *ssl, const char *groups) { return ssl_str_to_group_ids(&ssl->config->supported_group_list, groups); } -uint16_t SSL_get_curve_id(const SSL *ssl) { +uint16_t SSL_get_group_id(const SSL *ssl) { SSL_SESSION *session = SSL_get_session(ssl); if (session == NULL) { return 0; @@ -2025,6 +2053,14 @@ uint16_t SSL_get_curve_id(const SSL *ssl) { return session->group_id; } +int SSL_get_negotiated_group(const SSL *ssl) { + uint16_t group_id = SSL_get_group_id(ssl); + if (group_id == 0) { + return NID_undef; + } + return ssl_group_id_to_nid(group_id); +} + int SSL_CTX_set_tmp_dh(SSL_CTX *ctx, const DH *dh) { return 1; } @@ -3188,7 +3224,7 @@ namespace fips202205 { // Section 3.3.1 // "The server shall be configured to only use cipher suites that are // composed entirely of NIST approved algorithms" -static const int kGroups[] = {NID_X9_62_prime256v1, NID_secp384r1}; +static const uint16_t kGroups[] = {SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1}; static const uint16_t kSigAlgs[] = { SSL_SIGN_RSA_PKCS1_SHA256, @@ -3225,7 +3261,7 @@ static int Configure(SSL_CTX *ctx) { // Encrypt-then-MAC extension is required for all CBC cipher suites and so // it's easier to drop them. SSL_CTX_set_strict_cipher_list(ctx, kTLS12Ciphers) && - SSL_CTX_set1_groups(ctx, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && + SSL_CTX_set1_group_ids(ctx, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && SSL_CTX_set_signing_algorithm_prefs(ctx, kSigAlgs, OPENSSL_ARRAY_SIZE(kSigAlgs)) && SSL_CTX_set_verify_algorithm_prefs(ctx, kSigAlgs, @@ -3239,7 +3275,7 @@ static int Configure(SSL *ssl) { return SSL_set_min_proto_version(ssl, TLS1_2_VERSION) && SSL_set_max_proto_version(ssl, TLS1_3_VERSION) && SSL_set_strict_cipher_list(ssl, kTLS12Ciphers) && - SSL_set1_groups(ssl, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && + SSL_set1_group_ids(ssl, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && SSL_set_signing_algorithm_prefs(ssl, kSigAlgs, OPENSSL_ARRAY_SIZE(kSigAlgs)) && SSL_set_verify_algorithm_prefs(ssl, kSigAlgs, @@ -3252,7 +3288,7 @@ namespace wpa202304 { // See WPA version 3.1, section 3.5. -static const int kGroups[] = {NID_secp384r1}; +static const uint16_t kGroups[] = {SSL_GROUP_SECP384R1}; static const uint16_t kSigAlgs[] = { SSL_SIGN_RSA_PKCS1_SHA384, // @@ -3272,7 +3308,7 @@ static int Configure(SSL_CTX *ctx) { return SSL_CTX_set_min_proto_version(ctx, TLS1_2_VERSION) && SSL_CTX_set_max_proto_version(ctx, TLS1_3_VERSION) && SSL_CTX_set_strict_cipher_list(ctx, kTLS12Ciphers) && - SSL_CTX_set1_groups(ctx, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && + SSL_CTX_set1_group_ids(ctx, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && SSL_CTX_set_signing_algorithm_prefs(ctx, kSigAlgs, OPENSSL_ARRAY_SIZE(kSigAlgs)) && SSL_CTX_set_verify_algorithm_prefs(ctx, kSigAlgs, @@ -3285,7 +3321,7 @@ static int Configure(SSL *ssl) { return SSL_set_min_proto_version(ssl, TLS1_2_VERSION) && SSL_set_max_proto_version(ssl, TLS1_3_VERSION) && SSL_set_strict_cipher_list(ssl, kTLS12Ciphers) && - SSL_set1_groups(ssl, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && + SSL_set1_group_ids(ssl, kGroups, OPENSSL_ARRAY_SIZE(kGroups)) && SSL_set_signing_algorithm_prefs(ssl, kSigAlgs, OPENSSL_ARRAY_SIZE(kSigAlgs)) && SSL_set_verify_algorithm_prefs(ssl, kSigAlgs, diff --git a/src/ssl/ssl_test.cc b/src/ssl/ssl_test.cc index bd58f1c58..455b996e7 100644 --- a/src/ssl/ssl_test.cc +++ b/src/ssl/ssl_test.cc @@ -478,29 +478,29 @@ static const char* kShouldIncludeCBCSHA256[] = { static const CurveTest kCurveTests[] = { { "P-256", - { SSL_CURVE_SECP256R1 }, + { SSL_GROUP_SECP256R1 }, }, { "P-256:X25519Kyber768Draft00", - { SSL_CURVE_SECP256R1, SSL_CURVE_X25519_KYBER768_DRAFT00 }, + { SSL_GROUP_SECP256R1, SSL_GROUP_X25519_KYBER768_DRAFT00 }, }, { "P-256:P-384:P-521:X25519", { - SSL_CURVE_SECP256R1, - SSL_CURVE_SECP384R1, - SSL_CURVE_SECP521R1, - SSL_CURVE_X25519, + SSL_GROUP_SECP256R1, + SSL_GROUP_SECP384R1, + SSL_GROUP_SECP521R1, + SSL_GROUP_X25519, }, }, { "prime256v1:secp384r1:secp521r1:x25519", { - SSL_CURVE_SECP256R1, - SSL_CURVE_SECP384R1, - SSL_CURVE_SECP521R1, - SSL_CURVE_X25519, + SSL_GROUP_SECP256R1, + SSL_GROUP_SECP384R1, + SSL_GROUP_SECP521R1, + SSL_GROUP_X25519, }, }, }; @@ -5902,7 +5902,7 @@ TEST_P(SSLVersionTest, SessionPropertiesThreads) { bssl::UniquePtr peer(SSL_get_peer_certificate(ssl)); EXPECT_TRUE(peer); EXPECT_TRUE(SSL_get_current_cipher(ssl)); - EXPECT_TRUE(SSL_get_curve_id(ssl)); + EXPECT_TRUE(SSL_get_group_id(ssl)); }; std::vector threads; @@ -7754,7 +7754,8 @@ TEST(SSLTest, ConnectionPropertiesDuringRenegotiate) { ASSERT_TRUE(cipher); EXPECT_EQ(SSL_CIPHER_get_id(cipher), uint32_t{TLS1_CK_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256}); - EXPECT_EQ(SSL_get_curve_id(client.get()), SSL_CURVE_X25519); + EXPECT_EQ(SSL_get_group_id(client.get()), SSL_GROUP_X25519); + EXPECT_EQ(SSL_get_negotiated_group(client.get()), NID_X25519); EXPECT_EQ(SSL_get_peer_signature_algorithm(client.get()), SSL_SIGN_RSA_PKCS1_SHA256); bssl::UniquePtr peer(SSL_get_peer_certificate(client.get())); @@ -8716,6 +8717,20 @@ TEST(SSLTest, InvalidSignatureAlgorithm) { ctx.get(), kDuplicatePrefs, OPENSSL_ARRAY_SIZE(kDuplicatePrefs))); } +TEST(SSLTest, InvalidGroups) { + bssl::UniquePtr ctx(SSL_CTX_new(TLS_method())); + ASSERT_TRUE(ctx); + + static const uint16_t kInvalidIDs[] = {1234}; + EXPECT_FALSE(SSL_CTX_set1_group_ids( + ctx.get(), kInvalidIDs, OPENSSL_ARRAY_SIZE(kInvalidIDs))); + + // This is a valid NID, but it is not a valid group. + static const int kInvalidNIDs[] = {NID_rsaEncryption}; + EXPECT_FALSE(SSL_CTX_set1_groups( + ctx.get(), kInvalidNIDs, OPENSSL_ARRAY_SIZE(kInvalidNIDs))); +} + TEST(SSLTest, NameLists) { struct { size_t (*func)(const char **, size_t); @@ -8726,7 +8741,7 @@ TEST(SSLTest, NameLists) { {"TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", "TLS_AES_128_GCM_SHA256"}}, {SSL_get_all_cipher_names, {"ECDHE-ECDSA-AES128-GCM-SHA256", "TLS_AES_128_GCM_SHA256", "(NONE)"}}, - {SSL_get_all_curve_names, {"P-256", "X25519"}}, + {SSL_get_all_group_names, {"P-256", "X25519"}}, {SSL_get_all_signature_algorithm_names, {"rsa_pkcs1_sha256", "ecdsa_secp256r1_sha256", "ecdsa_sha256"}}, }; diff --git a/src/ssl/test/bssl_shim.cc b/src/ssl/test/bssl_shim.cc index 2640de7a7..e2f79d300 100644 --- a/src/ssl/test/bssl_shim.cc +++ b/src/ssl/test/bssl_shim.cc @@ -692,9 +692,9 @@ static bool CheckHandshakeProperties(SSL *ssl, bool is_resume, SSL_CIPHER_standard_name(SSL_get_current_cipher(ssl))) || !CheckListContains("OpenSSL cipher name", SSL_get_all_cipher_names, SSL_CIPHER_get_name(SSL_get_current_cipher(ssl))) || - (SSL_get_curve_id(ssl) != 0 && - !CheckListContains("curve", SSL_get_all_curve_names, - SSL_get_curve_name(SSL_get_curve_id(ssl)))) || + (SSL_get_group_id(ssl) != 0 && + !CheckListContains("group", SSL_get_all_group_names, + SSL_get_group_name(SSL_get_group_id(ssl)))) || (SSL_get_peer_signature_algorithm(ssl) != 0 && !CheckListContains( "sigalg", SSL_get_all_signature_algorithm_names, diff --git a/src/ssl/test/fuzzer.h b/src/ssl/test/fuzzer.h index 504150129..e6d2d0219 100644 --- a/src/ssl/test/fuzzer.h +++ b/src/ssl/test/fuzzer.h @@ -418,11 +418,11 @@ class TLSFuzzer { return false; } - static const int kGroups[] = {NID_X25519Kyber768Draft00, NID_X25519, - NID_X9_62_prime256v1, NID_secp384r1, - NID_secp521r1}; - if (!SSL_CTX_set1_groups(ctx_.get(), kGroups, - OPENSSL_ARRAY_SIZE(kGroups))) { + static const uint16_t kGroups[] = { + SSL_GROUP_X25519_KYBER768_DRAFT00, SSL_GROUP_X25519, + SSL_GROUP_SECP256R1, SSL_GROUP_SECP384R1, SSL_GROUP_SECP521R1}; + if (!SSL_CTX_set1_group_ids(ctx_.get(), kGroups, + OPENSSL_ARRAY_SIZE(kGroups))) { return false; } diff --git a/src/ssl/test/test_config.cc b/src/ssl/test/test_config.cc index b20449178..485a560b2 100644 --- a/src/ssl/test/test_config.cc +++ b/src/ssl/test/test_config.cc @@ -1895,38 +1895,9 @@ bssl::UniquePtr TestConfig::NewSSL( if (!check_close_notify) { SSL_set_quiet_shutdown(ssl.get(), 1); } - if (!curves.empty()) { - std::vector nids; - for (auto curve : curves) { - switch (curve) { - case SSL_CURVE_SECP224R1: - nids.push_back(NID_secp224r1); - break; - - case SSL_CURVE_SECP256R1: - nids.push_back(NID_X9_62_prime256v1); - break; - - case SSL_CURVE_SECP384R1: - nids.push_back(NID_secp384r1); - break; - - case SSL_CURVE_SECP521R1: - nids.push_back(NID_secp521r1); - break; - - case SSL_CURVE_X25519: - nids.push_back(NID_X25519); - break; - - case SSL_CURVE_X25519_KYBER768_DRAFT00: - nids.push_back(NID_X25519Kyber768Draft00); - break; - } - if (!SSL_set1_curves(ssl.get(), &nids[0], nids.size())) { - return nullptr; - } - } + if (!curves.empty() && + !SSL_set1_group_ids(ssl.get(), curves.data(), curves.size())) { + return nullptr; } if (initial_timeout_duration_ms > 0) { DTLSv1_set_initial_timeout_duration(ssl.get(), initial_timeout_duration_ms); diff --git a/src/ssl/test/test_config.h b/src/ssl/test/test_config.h index e8c473a16..e5c605799 100644 --- a/src/ssl/test/test_config.h +++ b/src/ssl/test/test_config.h @@ -35,7 +35,7 @@ struct TestConfig { std::vector signing_prefs; std::vector verify_prefs; std::vector expect_peer_verify_prefs; - std::vector curves; + std::vector curves; std::string key_file; std::string cert_file; std::string expect_server_name; diff --git a/src/tool/transport_common.cc b/src/tool/transport_common.cc index e88968856..15358de5a 100644 --- a/src/tool/transport_common.cc +++ b/src/tool/transport_common.cc @@ -288,9 +288,9 @@ void PrintConnectionInfo(BIO *bio, const SSL *ssl) { BIO_printf(bio, " Resumed session: %s\n", SSL_session_reused(ssl) ? "yes" : "no"); BIO_printf(bio, " Cipher: %s\n", SSL_CIPHER_standard_name(cipher)); - uint16_t curve = SSL_get_curve_id(ssl); - if (curve != 0) { - BIO_printf(bio, " ECDHE curve: %s\n", SSL_get_curve_name(curve)); + uint16_t group = SSL_get_group_id(ssl); + if (group != 0) { + BIO_printf(bio, " ECDHE group: %s\n", SSL_get_group_name(group)); } uint16_t sigalg = SSL_get_peer_signature_algorithm(ssl); if (sigalg != 0) {