Skip to content
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

Fix flaky ssl BadKemKeyShare tests #1876

Merged
merged 4 commits into from
Sep 25, 2024
Merged

Fix flaky ssl BadKemKeyShare tests #1876

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4754546
Fix flaky ssl BadKemKeyShare tests
Sep 25, 2024
69a1065
comment fix
dkostic Sep 25, 2024
b90a85c
ugh
dkostic Sep 25, 2024
be888d5
Merge branch 'main' into fix-ssl-test-badkemkeyshare
dkostic Sep 25, 2024
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
42 changes: 20 additions & 22 deletions ssl/ssl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11966,30 +11966,29 @@ TEST_P(BadKemKeyShareAcceptTest, BadKemKeyShareAccept) {
{
bssl::UniquePtr<SSLKeyShare> server_key_share = bssl::SSLKeyShare::Create(t.group_id);
bssl::UniquePtr<SSLKeyShare> client_key_share = bssl::SSLKeyShare::Create(t.group_id);
bssl::UniquePtr<SSLKeyShare> random_key_share = bssl::SSLKeyShare::Create(t.group_id);
ASSERT_TRUE(server_key_share);
ASSERT_TRUE(client_key_share);
ASSERT_TRUE(random_key_share);
uint8_t server_alert = 0;
uint8_t client_alert = 0;
Array<uint8_t> server_secret;
Array<uint8_t> client_secret;
CBB server_out_public_key;
CBB client_out_public_key;
CBB random_out_public_key;

// Start by having the client Offer() its public key
EXPECT_TRUE(CBB_init(&client_out_public_key, t.offer_key_share_size));
EXPECT_TRUE(client_key_share->Offer(&client_out_public_key));

// Modify the public key making it incompatible with the secret key
uint8_t *modified_client_public_key_buf =
(uint8_t *)OPENSSL_malloc(t.offer_key_share_size);
ASSERT_TRUE(modified_client_public_key_buf);
const uint8_t *client_out_public_key_data = CBB_data(&client_out_public_key);
ASSERT_TRUE(client_out_public_key_data);
OPENSSL_memcpy(modified_client_public_key_buf, client_out_public_key_data,
t.offer_key_share_size);
modified_client_public_key_buf[0] ^= 1;
// Generate a radnom public key that is incompatible with client's secret key
dkostic marked this conversation as resolved.
Show resolved Hide resolved
EXPECT_TRUE(CBB_init(&random_out_public_key, t.offer_key_share_size));
EXPECT_TRUE(random_key_share->Offer(&random_out_public_key));
const uint8_t *random_out_public_key_data = CBB_data(&random_out_public_key);
ASSERT_TRUE(random_out_public_key_data);
Span<const uint8_t> client_public_key =
MakeConstSpan(modified_client_public_key_buf, t.offer_key_share_size);
MakeConstSpan(random_out_public_key_data, t.offer_key_share_size);

// When the server calls Accept() with the modified public key, it will
// return success
Expand All @@ -12015,9 +12014,9 @@ TEST_P(BadKemKeyShareAcceptTest, BadKemKeyShareAccept) {

EXPECT_EQ(server_alert, 0);
EXPECT_EQ(client_alert, 0);
OPENSSL_free(modified_client_public_key_buf);
CBB_cleanup(&server_out_public_key);
CBB_cleanup(&client_out_public_key);
CBB_cleanup(&random_out_public_key);
}
}

Expand Down Expand Up @@ -12060,10 +12059,13 @@ TEST_P(BadKemKeyShareFinishTest, BadKemKeyShareFinish) {
// Set up the client and server states for the remaining tests
bssl::UniquePtr<SSLKeyShare> server_key_share = bssl::SSLKeyShare::Create(t.group_id);
bssl::UniquePtr<SSLKeyShare> client_key_share = bssl::SSLKeyShare::Create(t.group_id);
bssl::UniquePtr<SSLKeyShare> random_key_share = bssl::SSLKeyShare::Create(t.group_id);
ASSERT_TRUE(server_key_share);
ASSERT_TRUE(client_key_share);
ASSERT_TRUE(random_key_share);
CBB client_out_public_key;
CBB server_out_public_key;
CBB random_out_public_key;
Array<uint8_t> server_secret;
Array<uint8_t> client_secret;
uint8_t client_alert = 0;
Expand All @@ -12073,6 +12075,7 @@ TEST_P(BadKemKeyShareFinishTest, BadKemKeyShareFinish) {

EXPECT_TRUE(CBB_init(&client_out_public_key, t.offer_key_share_size));
EXPECT_TRUE(CBB_init(&server_out_public_key, t.accept_key_share_size));
EXPECT_TRUE(CBB_init(&random_out_public_key, t.accept_key_share_size));
EXPECT_TRUE(client_key_share->Offer(&client_out_public_key));
const uint8_t *client_out_public_key_data = CBB_data(&client_out_public_key);
ASSERT_TRUE(client_out_public_key_data);
Expand Down Expand Up @@ -12116,16 +12119,13 @@ TEST_P(BadKemKeyShareFinishTest, BadKemKeyShareFinish) {
{
// The server's public key was already correctly generated previously in
// a call to Accept(). Here we modify it.
WillChilds-Klein marked this conversation as resolved.
Show resolved Hide resolved
uint8_t *invalid_server_public_key_buf = (uint8_t *) OPENSSL_malloc(t.accept_key_share_size);
ASSERT_TRUE(invalid_server_public_key_buf);
const uint8_t *server_out_public_key_data = CBB_data(&server_out_public_key);
ASSERT_TRUE(server_out_public_key_data);
OPENSSL_memcpy(invalid_server_public_key_buf, server_out_public_key_data, t.accept_key_share_size);
invalid_server_public_key_buf[0] ^= 1;
EXPECT_TRUE(random_key_share->Offer(&random_out_public_key));
const uint8_t *random_out_public_key_data = CBB_data(&random_out_public_key);
ASSERT_TRUE(random_out_public_key_data);
server_public_key =
MakeConstSpan(random_out_public_key_data, t.accept_key_share_size);

// The call to Finish() will return success
server_public_key =
MakeConstSpan(invalid_server_public_key_buf, t.accept_key_share_size);
EXPECT_TRUE(client_key_share->Finish(&client_secret, &client_alert, server_public_key));
EXPECT_EQ(client_alert, 0);

Expand All @@ -12135,13 +12135,11 @@ TEST_P(BadKemKeyShareFinishTest, BadKemKeyShareFinish) {

// ... but they are not equal
EXPECT_NE(Bytes(client_secret), Bytes(server_secret));


OPENSSL_free(invalid_server_public_key_buf);
}

CBB_cleanup(&server_out_public_key);
CBB_cleanup(&client_out_public_key);
CBB_cleanup(&random_out_public_key);
}

class HybridKeyShareTest : public testing::TestWithParam<HybridGroupTest> {};
Expand Down
Loading