Skip to content

Fix tests that assume X25519 will be negotiated #2682

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix tests that assume X25519 will be negotiated #2682

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ssl/extensions.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -312,6 +312,10 @@ static const uint16_t kDefaultGroups[] = {
SSL_GROUP_SECP384R1,
};

Span<const uint16_t> tls1_get_default_grouplist(void) {
return Span<const uint16_t>(kDefaultGroups);
}

Span<const uint16_t> tls1_get_grouplist(const SSL_HANDSHAKE *hs) {
if (!hs->config->supported_group_list.empty()) {
return hs->config->supported_group_list;
Expand Down
3 changes: 3 additions & 0 deletions ssl/internal.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3638,6 +3638,9 @@ bool tls1_change_cipher_state(SSL_HANDSHAKE *hs,
int tls1_generate_master_secret(SSL_HANDSHAKE *hs, uint8_t *out,
Span<const uint8_t> premaster);

// tls1_get_default_grouplist returns the default group list
OPENSSL_EXPORT Span<const uint16_t> tls1_get_default_grouplist(void);

// tls1_get_grouplist returns the locally-configured group preference list.
Span<const uint16_t> tls1_get_grouplist(const SSL_HANDSHAKE *ssl);

Expand Down
1 change: 1 addition & 0 deletions ssl/ssl_encoding_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,7 @@ static size_t GetClientHelloLen(uint16_t max_version, uint16_t session_version,
bssl::UniquePtr<SSL> ssl(SSL_new(ctx.get()));
if (!ssl || !SSL_set_session(ssl.get(), session.get()) ||
!SSL_set_strict_cipher_list(ssl.get(), "ECDHE-RSA-AES128-GCM-SHA256") ||
!SSL_set1_curves_list(ssl.get(), "x25519:P-256:P-384") ||
!SSL_set_max_proto_version(ssl.get(), max_version)) {
return 0;
}
Expand Down
24 changes: 23 additions & 1 deletion ssl/ssl_version_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2328,7 +2328,29 @@ TEST_P(SSLVersionTest, PeerTmpKey) {
GTEST_SKIP();
}

// Default should be using X5519 as the key exchange.
ASSERT_TRUE(Connect());
for (SSL *ssl : {client_.get(), server_.get()}) {
SCOPED_TRACE(SSL_is_server(ssl) ? "server" : "client");
EVP_PKEY *key = nullptr;
uint16_t preferred_group = tls1_get_default_grouplist()[0];
if (getVersionParam().version == TLS1_3_VERSION && preferred_group == SSL_GROUP_X25519_MLKEM768) {
// TLS 1.3 default should be using X25519MLKEM768 as the key exchange.
// We expect SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE because there is no EVP_PKEY type
// for hybrid keys, only individual X25519 or MLKEM768 keys.
ERR_clear_error();
EXPECT_FALSE(SSL_get_peer_tmp_key(ssl, &key));
ErrorEquals(ERR_get_error(), ERR_LIB_SSL, SSL_R_UNKNOWN_KEY_EXCHANGE_TYPE);
} else {
// Otherwise x25519 should be used
EXPECT_TRUE(preferred_group == SSL_GROUP_X25519);
EXPECT_TRUE(SSL_get_peer_tmp_key(ssl, &key));
EXPECT_EQ(EVP_PKEY_id(key), EVP_PKEY_X25519);
bssl::UniquePtr<EVP_PKEY> pkey(key);
}
}

// Check that x25519 works.
ASSERT_TRUE(SSL_CTX_set1_groups_list(server_ctx_.get(), "x25519"));
ASSERT_TRUE(Connect());
for (SSL *ssl : {client_.get(), server_.get()}) {
SCOPED_TRACE(SSL_is_server(ssl) ? "server" : "client");
Expand Down
8 changes: 6 additions & 2 deletions ssl/test/runner/runner.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8697,8 +8697,12 @@ func addExtensionTests() {
},
// This hostname just needs to be long enough to push the
// ClientHello into F5's danger zone between 256 and 511 bytes
// long.
flags: []string{"-host-name", "01234567890123456789012345678901234567890123456789012345678901234567890123456789.com"},
// long. Also override curves to just x25519 to remove any PQ
// KeyShares that might push ClientHello above 512 bytes.
flags: []string{
"-host-name", "01234567890123456789012345678901234567890123456789012345678901234567890123456789.com",
"-curves", strconv.Itoa(int(CurveX25519)),
},
})

// Test that illegal extensions in TLS 1.3 are rejected by the client if
Expand Down
Loading