Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
97 changes: 97 additions & 0 deletions tests/unit/s2n_client_key_share_extension_pq_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -976,6 +976,103 @@ int main()
}
}

/* Test that the hybrid's classical key is reused as the standalone classical
* key share when (and only when) the hybrid's curve matches ecc_curves[0].
*
* When the reuse happens, both key shares point at the same EVP_PKEY, so we
* can detect it by comparing the two evp_pkey pointers. */
if (s2n_pq_is_enabled() && s2n_kem_group_is_available(&s2n_x25519_mlkem_768)) {
const struct s2n_kem_group *test_kem_groups[] = {
&s2n_x25519_mlkem_768,
};

const struct s2n_kem_preferences test_kem_prefs = {
.kem_count = 0,
.kems = NULL,
.tls13_kem_group_count = s2n_array_len(test_kem_groups),
.tls13_kem_groups = test_kem_groups,
.tls13_pq_hybrid_draft_revision = 5,
};

/* The hybrid X25519MLKEM768 generates an X25519 key. ecc_curves[0] for
* s2n_ecc_preferences_20200310 is also x25519, so the reuse should fire. */
{
const struct s2n_security_policy reuse_policy = {
.minimum_protocol_version = S2N_SSLv3,
.cipher_preferences = &cipher_preferences_test_all_tls13,
.kem_preferences = &test_kem_prefs,
.signature_preferences = &s2n_signature_preferences_20200207,
.ecc_preferences = &s2n_ecc_preferences_20200310,
};

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
conn->security_policy_override = &reuse_policy;

const struct s2n_ecc_preferences *ecc_pref = NULL;
EXPECT_SUCCESS(s2n_connection_get_ecc_preferences(conn, &ecc_pref));
EXPECT_NOT_NULL(ecc_pref);

/* Precondition: the hybrid's curve is the same as ecc_curves[0] */
EXPECT_EQUAL(s2n_x25519_mlkem_768.curve, ecc_pref->ecc_curves[0]);

DEFER_CLEANUP(struct s2n_stuffer key_share_extension = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&key_share_extension, MEM_FOR_EXTENSION));
EXPECT_SUCCESS(s2n_client_key_share_extension.send(conn, &key_share_extension));

struct s2n_ecc_evp_params *hybrid_ecc = &conn->kex_params.client_kem_group_params.ecc_params;
struct s2n_ecc_evp_params *classical_ecc = &conn->kex_params.client_ecc_evp_params;

EXPECT_NOT_NULL(hybrid_ecc->evp_pkey);
EXPECT_NOT_NULL(classical_ecc->evp_pkey);

/* The classical share reused the hybrid's key: same curve, same key */
EXPECT_EQUAL(classical_ecc->negotiated_curve, hybrid_ecc->negotiated_curve);
EXPECT_EQUAL(classical_ecc->evp_pkey, hybrid_ecc->evp_pkey);
}

/* ecc_curves[0] for s2n_ecc_preferences_20230623 is secp256r1, which does
* not match the hybrid's X25519 curve, so the reuse should NOT fire and
* each key share must hold its own independently generated key. */
{
const struct s2n_security_policy no_reuse_policy = {
.minimum_protocol_version = S2N_SSLv3,
.cipher_preferences = &cipher_preferences_test_all_tls13,
.kem_preferences = &test_kem_prefs,
.signature_preferences = &s2n_signature_preferences_20200207,
.ecc_preferences = &s2n_ecc_preferences_20230623,
};

DEFER_CLEANUP(struct s2n_connection *conn = s2n_connection_new(S2N_CLIENT),
s2n_connection_ptr_free);
EXPECT_NOT_NULL(conn);
conn->security_policy_override = &no_reuse_policy;

const struct s2n_ecc_preferences *ecc_pref = NULL;
EXPECT_SUCCESS(s2n_connection_get_ecc_preferences(conn, &ecc_pref));
EXPECT_NOT_NULL(ecc_pref);

/* Precondition: the hybrid's curve differs from ecc_curves[0] */
EXPECT_NOT_EQUAL(s2n_x25519_mlkem_768.curve, ecc_pref->ecc_curves[0]);

DEFER_CLEANUP(struct s2n_stuffer key_share_extension = { 0 }, s2n_stuffer_free);
EXPECT_SUCCESS(s2n_stuffer_growable_alloc(&key_share_extension, MEM_FOR_EXTENSION));
EXPECT_SUCCESS(s2n_client_key_share_extension.send(conn, &key_share_extension));

struct s2n_ecc_evp_params *hybrid_ecc = &conn->kex_params.client_kem_group_params.ecc_params;
struct s2n_ecc_evp_params *classical_ecc = &conn->kex_params.client_ecc_evp_params;

EXPECT_NOT_NULL(hybrid_ecc->evp_pkey);
EXPECT_NOT_NULL(classical_ecc->evp_pkey);

/* Distinct curves, and therefore distinct keys */
EXPECT_EQUAL(classical_ecc->negotiated_curve, ecc_pref->ecc_curves[0]);
EXPECT_NOT_EQUAL(classical_ecc->negotiated_curve, hybrid_ecc->negotiated_curve);
EXPECT_NOT_EQUAL(classical_ecc->evp_pkey, hybrid_ecc->evp_pkey);
}
}

END_TEST();

return 0;
Expand Down
5 changes: 3 additions & 2 deletions tests/unit/s2n_tls13_pq_handshake_test.c
Original file line number Diff line number Diff line change
Expand Up @@ -527,8 +527,9 @@ int main()
.len_prefix_expected = true,
},

/* Server does not support PQ; client sends a PQ key share, but no EC shares;
* server should negotiate EC and send HRR. */
/* Server doesn't support PQ. Client's ecc_curves[0] is unsupported so
* the server sends HRR. After retry the server negotiates its top
* mutually-supported curve from the client's supported_groups. */
{
.client_policy = &security_policy_test_tls13_retry_with_pq,
.server_policy = &security_policy_test_all_tls13,
Expand Down
40 changes: 39 additions & 1 deletion tls/extensions/s2n_client_key_share.c
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ static int s2n_generate_default_ecc_key_share(struct s2n_connection *conn, struc
}

/* We only ever send a single EC key share: either the share requested by the server
* during a retry, or the most preferred share according to local preferences.
* during a retry, or a share selected based on local preferences and available key material.
*/
struct s2n_ecc_evp_params *client_params = &conn->kex_params.client_ecc_evp_params;
if (s2n_is_hello_retry_handshake(conn)) {
Expand All @@ -98,7 +98,45 @@ static int s2n_generate_default_ecc_key_share(struct s2n_connection *conn, struc
**/
client_params->negotiated_curve = server_curve;
} else {
/* Performance optimization: if the PQ hybrid already generated a key for
* the same curve as ecc_curves[0], reuse it as the standalone classical
* key share instead of generating a separate one. This saves one EC keygen
* per handshake.
*
* Because the reuse only fires when the hybrid's curve IS ecc_curves[0],
* the wire is always unchanged. It's the same curve, same group ID, same bytes a
* server would have seen without this optimization.
*/
client_params->negotiated_curve = ecc_pref->ecc_curves[0];

/* EVP_PKEY_up_ref was added in OpenSSL 1.1.0. On libcryptos without
* EVP_APIS_SUPPORTED (e.g. OpenSSL 1.0.2), PQ hybrids are never generated
* (s2n_pq_is_enabled() returns false), so hybrid_ecc->evp_pkey is always NULL
* and this reuse path is unreachable. Compile it out to avoid referencing the
* missing symbol. */
#if EVP_APIS_SUPPORTED
struct s2n_ecc_evp_params *hybrid_ecc = &conn->kex_params.client_kem_group_params.ecc_params;
const struct s2n_ecc_named_curve *hybrid_curve = hybrid_ecc->negotiated_curve;

if (hybrid_curve && hybrid_ecc->evp_pkey != NULL
&& hybrid_curve == ecc_pref->ecc_curves[0]) {
Comment thread
MrMistic marked this conversation as resolved.
/* The hybrid's classical curve matches the default standalone key share
* curve, so we can reuse the already-generated key instead of creating
* a new one. The same EC public key is offered both inside the hybrid
* KeyShareEntry and as the standalone classical KeyShareEntry.
* These are distinct groups (different IANA ids) and the server
* selects exactly one, so the private key is used in at most one
* shared secret computation with no cross-group key reuse.
*
* Both client_ecc_evp_params and client_kem_group_params.ecc_params
* hold a reference. EVP_PKEY_free (called by s2n_ecc_evp_params_free
* on connection cleanup) decrements the refcount, so the key is
* freed exactly once when the last reference is released. */
EVP_PKEY_up_ref(hybrid_ecc->evp_pkey);
client_params->negotiated_curve = hybrid_curve;
client_params->evp_pkey = hybrid_ecc->evp_pkey;
}
#endif
}
POSIX_GUARD(s2n_ecdhe_parameters_send(client_params, out));

Expand Down
Loading