Fix for DTLS1.3 HRR group handling#9848
Merged
douzzer merged 1 commit intowolfSSL:masterfrom Mar 5, 2026
Merged
Conversation
rizlik
previously requested changes
Mar 2, 2026
julek-wolfssl
requested changes
Mar 2, 2026
Member
julek-wolfssl
left a comment
julek-wolfssl
left a comment
There was a problem hiding this comment.
With this PR, we don't need the ssl->hrr_keyshare_group variable anymore.
Contributor
Author
|
@julek-wolfssl @rizlik I addressed all your comments. |
julek-wolfssl
requested changes
Mar 3, 2026
julek-wolfssl
requested changes
Mar 4, 2026
Member
There was a problem hiding this comment.
So I still don't like this solution.
- It relies on the cookie appearing before the key share in the extension list as we process extensions in order in TLSX_Parse
- I don't like having multiple independent parsing logics.
So my suggestion is to restoressl->hrr_keyshare_groupinRestartHandshakeHashWithCookieand move thessl->hrr_keyshare_groupcheck toDoTls13ClientHelloafter the call toTLSX_Parse. Whenssl->hrr_keyshare_group != server_helloyou should find the key share extension, make sure there is only one entry, and make sure it corresponds to thessl->hrr_keyshare_group.
julek-wolfssl
requested changes
Mar 4, 2026
julek-wolfssl
approved these changes
Mar 4, 2026
When a server uses a HRR to negotiate the key exchange group to use, the selected group is advertised in the HRR key share extension. Furthermore, this group is also stored in the Cookie that is sent to the client. When the server receives the second CH, the group used in the key share extension MUST be the one of the HRR. For stateless DTLS servers, the handling of this check had a bug. The key share group of the HRR is stored in the ssl->hrr_keyshare_group variable and is checked against the received key share of the second CH. However, in the stateless server case, another CH message may be received inbetween the two CH message of the desired client, potentially overwriting the ssl->hrr_keyshare_group variable. This then causes handshake failures when the ssl->hrr_keyshare_group variable contains another group than the second CH message of the desired client. To fix this, the following changes are conducted: 1. Disable the ssl->hrr_keyshare_group check for stateless DTLS 1.3 servers. As long as the server is stateless, CHs from multiple clients may be received that individually cause HRRs with different groups. For each of these clients, the HRR group is properly stored in the cookie. 2. When a valid cookie is received from the client, the server becomes stateful. In this case, we now parse the cookie for a stored HRR group in the RestartHandshakeHashWithCookie() method. If present, we restore the ssl->hrr_keyshare_group variable to this group to ensure the error checks succeed. 3. Move the check of ssl->hrr_keyshare_group of the the KeyShare extension parsing logic into the general TLS1.3 ClientHello parsing after extension handling. This ensures that the order of the cookie and key share extensions does not matter. A new test is added to check for this behavior.
douzzer
approved these changes
Mar 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a server uses an HRR to negotiate the key exchange group to use, the selected group is advertised in the HRR key share extension. Furthermore, this group is also stored in the Cookie that is sent to the client. When the server receives the second CH, the group used in the key share extension MUST be the one of the HRR.
For stateless DTLS servers, the handling of this check had a bug. The key share group of the HRR is stored in the ssl->hrr_keyshare_group variable and is checked against the received key share of the second CH. However, in the stateless server case, another CH message may be received inbetween the two CH messages of the desired client, potentially overwriting the ssl->hrr_keyshare_group variable. This then causes handshake failures when the ssl->hrr_keyshare_group variable contains another group than the second CH message of the desired client.
To fix this, two changes are conducted:
A new test is added to check for this behavior.
Identified while working on #9732.