feat: Ability to set "strongly preferred" groups #5634

alexw91 · 2025-11-22T00:31:42Z

Goal

Resolves #5602

Adds a new strongly preferred groups field to s2n_security_policy. This preference list represents the set of SupportedGroups that s2n's TLS server is willing to perform a 2-RTT handshake for if that SupportedGroup was sent in the SupportedGroups extension, but not the KeyShare extension.

If an s2n security policy contains strongly preferred groups, then s2n's usual negotiation logic to prefer KeyShares that are negotiable in 1-RTT when possible is bypassed, and the first mutually supported strongly-preferred SupportedGroup will be selected regardless if it is negotiable in 1-RTT or 2-RTT. If no mutually supported strongly-preferred SupportedGroup exists, the s2n's default logic is skipped.

After this change, s2n's new negotiation logic will be:
1. Any Strongly-Preferred Group (using server preference order)
2. Any PQ SupportedGroup in 1-RTT (using server preference order)
3. Any PQ SupportedGroup in 2-RTT (using server preference order)
4. Any ECDHE Group in 1-RTT (using server preference order)
5. Any mutually supported group in 2-RTT (using server preference order)

Why

Needed to strongly prefer p384 for CNSA policies to force a 2-RTT handshake to select p384 even if p256 is negotiable in 1-RTT. With this logic, TLS Clients that do support p384 will be always be upgraded to p384, and clients that only support p256 will continue negotiating p256 without causing breakages. Then once this CNSA transition policy is deployed we can measure how many TLS connections do not support p384 and we can work on tracking down and fixing those clients.

How

Adds a new field to s2n_security_policy that is checked in s2n_extensions_server_key_share_select() that overrides s2n's default KeyShare negotiation logic as per the description in #5602.

Callouts

PR is based on top of #5615

Testing

Unit tests confirm new negotiation behavior for CNSA transition policies

     * without sending HRR. (The case of both being non-NULL should never occur, and
     * is an error.) */
-    POSIX_ENSURE((server_curve == NULL) != (server_kem_group == NULL), S2N_ERR_ECDHE_UNSUPPORTED_CURVE);
+    POSIX_ENSURE((server_curve == NULL) != (server_kem_group == NULL), S2N_ERR_INVALID_SUPPORTED_GROUP_STATE);


The PR description for this feature is really light. Can you explain how you've implemented "strongly preferred groups" there? How does this feature interact with our existing behavior of doing HRRs when clients support PQ but don't send a PQ keyshare? Is it the same mechanism used here or not?

Can you explain how you've implemented "strongly preferred groups" there? ç

Added more detailed description to the PR. Overall the mechanism is described in #5602.

Can you explain how you've implemented "strongly preferred groups" there? How does this feature interact with our existing behavior of doing HRRs when clients support PQ but don't send a PQ keyshare?

I added one new check for a mutually supported strongly-preferred group before the rest of s2n's existing SupportedGroup negotiation logic.

After this change, s2n's new negotiation logic will be:

Strongly-Preferred Groups (using server preference order)

Any PQ SupportedGroup in 1-RTT (using server preference order)

Any PQ SupportedGroup in 2-RTT (using server preference order)

Any ECDHE Group in 1-RTT (using server preference order)

Any mutually supported group in 2-RTT (using server preference order)

Is it the same mechanism used here or not?

Yes, it's a new check before all the same mechanisms are used. All of this KeyShare negotiation logic exists in the function int s2n_extensions_server_key_share_select(struct s2n_connection *conn). Each of these 5 cases is annotated with a code comment. s2n_extensions_server_key_share_select() is responsible for updating and setting exactly one of conn->kex_params.server_ecc_evp_params.negotiated_curve and conn->kex_params.server_kem_group_params.kem_group to the SupportedGroup that s2n would like to negotiate, and (optionally) calling s2n_set_hello_retry_required(conn) if selecting that SupportedGroup would require a 2-RTT handshake.

Couldn't you reuse the strongly-preferred group mechanism for the PQ groups? You would have to edit all existing PQ policies to include strongly-preferred groups(and probably include a unit test making sure future PQ policies add their PQ groups to the strongly-preferred groups mechanism), but it would be cleaner than having 5 distinct prioritizations.
Then I think it would look like:

Any Strongly-Preferred Group 1 RTT or 2 RTT regardless (using server preference order)

Any mutually supported group in 1-RTT (using server preference order)

Any mutually supported group in 2-RTT (using server preference order)

I haven't quite thought through the consequences of this design, but I'm curious as to why you chose to go with a separate prioritization method rather than merging with the PQ prioritization design.

Merging the "Strongly Preferred" and "PQ" groups would result in different behavior that we wouldn't want. If we added x25519MLKEM768 to the strongly preferred list, but a client sent p256MLKEM768 (or any other PQ) KeyShare, we'd throw away the p256MLKEM768 KeyShare and perform a 2-RTT handshake with x25519MLKEM768.

The "strongly preferred" mechanism is a heavy-handed hammer for forcing a specific small subset of SupportedGroups to be negotiated for compliance-related security policies (like CNSA/FIPS) that we are willing to always take the 2-RTT performance impact for, while still offering backwards compatibility as a last resort to avoid breaking clients.

Our default negotiation mechanism for everything else without a strongly preferred preference list is that we will accept any PQ KeyShare in 1-RTT, and will prioritize a 2-RTT PQ KeyShare over a 1-RTT ECDHE KeyShare.

Then I think it would look like:

Any Strongly-Preferred Group 1 RTT or 2 RTT regardless (using server preference order)

Any mutually supported group in 1-RTT (using server preference order)

Any mutually supported group in 2-RTT (using server preference order)

This logic has a problem in the following case:

Server Preference: - MLKEM1024, secp384r1, secp256r1 - strongly_preferred: MLKEM1024, secp384r1 Client Hello: key_share: p384 supported_group: MLKEM1024, p384, p256

We'd negotiate p384 in 1-RTT instead of MLKEM1024 in 2-RTT

jmayclin · 2025-12-03T00:32:24Z

tests/unit/s2n_strongly_preferred_groups_test.c

+    BEGIN_TEST();
+
+    /* Enforce minimum requirements on all security policies that have strongly preferred groups */
+    for (size_t policy_index = 0; security_policy_selection[policy_index].version != NULL; policy_index++) {


I think that (for now) we should enforce that the strongly_preferred_groups feature is only ever used on completely ECC policies.

Trying to reason through PQ preferences vs strongly preferred groups is a bit 😵

I agree with Madeleine that the path forward to have some unified way of enforcing the "compliance/alignment" preferences AND PQ preferences. I think this is pretty closely related (imo) of unifying ECC/KEM handling for key exchange. And I think this would also be the time to tackle generating multiple key shares for the client.

But I think that the approach in the current PR is sufficient to satisfying our goals for the ECC case.

I think that will also make some of the strongly preferred group logic a bit easier to handle, since we currently store PQ group and ECC groups in separate lists.

…o PQ support)

Co-authored-by: Jou Ho <[email protected]>

jmayclin

A few nits.

I think generally I'd prefer to remove the PQ-only stuff since we aren't allowing that rn. Was the thought to keep it in in case it's used in the future?

jmayclin · 2025-12-03T18:40:00Z

tests/unit/s2n_strongly_preferred_groups_test.c

+
+        POSIX_ENSURE_REF(security_policy->kem_preferences);
+
+        /* Temporarily require that policies either have PQ or strongly supported groups, but not both. */


Suggested change

/* Temporarily require that policies either have PQ or strongly supported groups, but not both. */

/* Temporarily require that policies don't allow both PQ and strongly supported groups. */

jmayclin · 2025-12-03T18:41:28Z

tests/unit/s2n_strongly_preferred_groups_test.c

+        for (size_t i = 0; i < security_policy->kem_preferences->tls13_kem_group_count; i++) {
+            const struct s2n_kem_group *kem_group = security_policy->kem_preferences->tls13_kem_groups[i];
+            POSIX_ENSURE_REF(kem_group);
+            ordered_supported_groups[ordered_supported_group_count++] = kem_group->iana_id;
+        }


I think we can remove this now that we've enforcing no PQ.

Suggested change

for (size_t i = 0; i < security_policy->kem_preferences->tls13_kem_group_count; i++) {

const struct s2n_kem_group *kem_group = security_policy->kem_preferences->tls13_kem_groups[i];

POSIX_ENSURE_REF(kem_group);

ordered_supported_groups[ordered_supported_group_count++] = kem_group->iana_id;

}

jmayclin · 2025-12-03T18:42:20Z

tests/unit/s2n_strongly_preferred_groups_test.c

+    }
+
+    END_TEST();
+}


Suggested change

}

}

jmayclin · 2025-12-03T19:00:48Z

tls/s2n_supported_group_preferences.c

+const uint16_t cnsa_1_2_supported_group_iana_ids[] = {
+    TLS_PQ_KEM_GROUP_ID_MLKEM_1024,
+    TLS_EC_CURVE_SECP_384_R1
+};
+
+const uint16_t cnsa_2_supported_group_iana_ids[] = {
+    TLS_PQ_KEM_GROUP_ID_MLKEM_1024
+};


Since these can't be used for now, I'd vote to nuke em

Suggested change

const uint16_t cnsa_1_2_supported_group_iana_ids[] = {

TLS_PQ_KEM_GROUP_ID_MLKEM_1024,

TLS_EC_CURVE_SECP_384_R1

};

const uint16_t cnsa_2_supported_group_iana_ids[] = {

TLS_PQ_KEM_GROUP_ID_MLKEM_1024

};

jmayclin · 2025-12-03T19:01:19Z

tls/s2n_supported_group_preferences.c

+const struct s2n_supported_group_preferences cnsa_1_2_strong_preference = {
+    .count = s2n_array_len(cnsa_1_2_supported_group_iana_ids),
+    .iana_ids = cnsa_1_2_supported_group_iana_ids
+};
+
+const struct s2n_supported_group_preferences cnsa_2_strong_preference = {
+    .count = s2n_array_len(cnsa_2_supported_group_iana_ids),
+    .iana_ids = cnsa_2_supported_group_iana_ids
+};


Suggested change

const struct s2n_supported_group_preferences cnsa_1_2_strong_preference = {

.count = s2n_array_len(cnsa_1_2_supported_group_iana_ids),

.iana_ids = cnsa_1_2_supported_group_iana_ids

};

const struct s2n_supported_group_preferences cnsa_2_strong_preference = {

.count = s2n_array_len(cnsa_2_supported_group_iana_ids),

.iana_ids = cnsa_2_supported_group_iana_ids

};

jmayclin · 2025-12-03T19:02:16Z

tls/extensions/s2n_server_key_share.c

+        for (int j = 0; j < S2N_KEM_GROUPS_COUNT && !matched_strongly_preferred_iana; j++) {
+            const struct s2n_kem_group *mutually_supported_kem_group = conn->kex_params.mutually_supported_kem_groups[j];
+            if (mutually_supported_kem_group == NULL) {
+                break; /* Reached end of mutually supported KEM Groups */
+            }
+            if (strongly_preferred_iana == mutually_supported_kem_group->iana_id) {
+                matched_strongly_preferred_iana = true;
+                strongly_preferred_kem_group = mutually_supported_kem_group;
+
+                /* Check if we can negotiate our strongly preferred KEM Group in 1-RTT */
+                if (client_kem_group != NULL && (strongly_preferred_iana == client_kem_group->iana_id)) {
+                    need_hrr_for_strongly_preferred_group = false;
+                } else {
+                    need_hrr_for_strongly_preferred_group = true;
+                }
+            }
+        }


Suggested change

for (int j = 0; j < S2N_KEM_GROUPS_COUNT && !matched_strongly_preferred_iana; j++) {

const struct s2n_kem_group *mutually_supported_kem_group = conn->kex_params.mutually_supported_kem_groups[j];

if (mutually_supported_kem_group == NULL) {

break; /* Reached end of mutually supported KEM Groups */

}

if (strongly_preferred_iana == mutually_supported_kem_group->iana_id) {

matched_strongly_preferred_iana = true;

strongly_preferred_kem_group = mutually_supported_kem_group;

/* Check if we can negotiate our strongly preferred KEM Group in 1-RTT */

if (client_kem_group != NULL && (strongly_preferred_iana == client_kem_group->iana_id)) {

need_hrr_for_strongly_preferred_group = false;

} else {

need_hrr_for_strongly_preferred_group = true;

}

}

}

/* strongly preferred groups and not allowed on policies that support PQ, so we don't check KEMs */

alexw91 force-pushed the strongly_preferred branch 2 times, most recently from 1ff3582 to 906ffeb Compare November 24, 2025 18:42

alexw91 changed the title ~~[WIP] Ability to set "strongly preferred" groups~~ Ability to set "strongly preferred" groups Nov 24, 2025

alexw91 marked this pull request as ready for review November 24, 2025 18:44

alexw91 requested review from a team and jouho November 24, 2025 18:44

alexw91 force-pushed the strongly_preferred branch 4 times, most recently from c2b318a to 8746fc1 Compare November 25, 2025 23:24

jouho reviewed Nov 25, 2025

View reviewed changes

tls/extensions/s2n_server_key_share.c Outdated Show resolved Hide resolved

tls/extensions/s2n_server_key_share.c Outdated Show resolved Hide resolved

tls/s2n_security_policies.h Show resolved Hide resolved

tls/s2n_security_policies.h Show resolved Hide resolved

alexw91 changed the title ~~Ability to set "strongly preferred" groups~~ feat: Ability to set "strongly preferred" groups Nov 26, 2025

jouho approved these changes Nov 26, 2025

View reviewed changes

jouho requested a review from maddeleine November 26, 2025 20:39

alexw91 force-pushed the strongly_preferred branch from 170c634 to 52ba908 Compare November 26, 2025 20:45

maddeleine reviewed Nov 27, 2025

View reviewed changes

alexw91 force-pushed the strongly_preferred branch from 8d5d00a to f8670b5 Compare December 1, 2025 14:48

jmayclin reviewed Dec 3, 2025

View reviewed changes

alexw91 and others added 9 commits December 3, 2025 08:18

Add support for strongly preferred groups

ed9fe7a

Delete dead code

ff86c54

Fix 1-RTT vs 2-RTT detection logic in test

8b04bb7

Fix tests to work with Openssl 3.0 (TLS 1.3 and x25519 support, but n…

252d95d

…o PQ support)

Address Feedback: Code Comment Nit

b2e77df

Co-authored-by: Jou Ho <[email protected]>

Address Feedback

3ff9a13

Simplify Pull Request

43800e7

Refactor selection logic

bbff2ff

Rebase onto main

efd62de

alexw91 force-pushed the strongly_preferred branch from f8670b5 to efd62de Compare December 3, 2025 16:27

Disable PQ from policies iwth strongly preferred groups

0b967be

jmayclin approved these changes Dec 3, 2025

View reviewed changes

Remove PQ from Strongly Preferred Group negotiation logic

24c62d5

jmayclin added this pull request to the merge queue Dec 4, 2025

Merged via the queue into aws:main with commit f1b90fe Dec 4, 2025
53 of 54 checks passed

jmayclin mentioned this pull request Dec 5, 2025

tests(integration): cases for TLS 1.3 group selection #5652

Merged


		POSIX_ENSURE_REF(security_policy->kem_preferences);

		/* Temporarily require that policies either have PQ or strongly supported groups, but not both. */

	/* Temporarily require that policies either have PQ or strongly supported groups, but not both. */
	/* Temporarily require that policies don't allow both PQ and strongly supported groups. */

feat: Ability to set "strongly preferred" groups #5634

feat: Ability to set "strongly preferred" groups #5634

Uh oh!

Conversation

alexw91 commented Nov 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Goal

Why

How

Callouts

Testing

Related

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexw91 Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexw91 Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jmayclin left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

alexw91 commented Nov 22, 2025 •

edited

Loading

alexw91 Nov 27, 2025 •

edited

Loading

alexw91 Dec 2, 2025 •

edited

Loading