Repository navigation
Leave objects unchanged when version, group and sigalgs setters fail - #11938
Open
julek-wolfssl wants to merge 1 commit into
Open
julek-wolfssl wants to merge 1 commit into
julek-wolfssl wants to merge 1 commit into
Conversation
Protocol version setters: - wolfSSL_CTX_set_min_proto_version(), wolfSSL_CTX_set_max_proto_version(), wolfSSL_set_min_proto_version() and wolfSSL_set_max_proto_version() work on a copy of the version limits and store it only on success. A failing call could leave every version disabled, or enable again versions that the application had disabled with options. - The CTX setters record the NO_* options each of them sets and drop only those, so options set by the application are kept and a later call can lower the min or raise the max. The max setter no longer re-applies the min. wolfSSL_CTX_set_options(), wolfSSL_CTX_clear_options() and wolfSSL_CTX_SetMinVersion() update the records. - A call fails when it leaves no version from the min up, counting a min held only in minDowngrade: the default one, one set with wolfSSL_CTX_SetMinVersion() or wolfSSL_SetMinVersion(), or the crypto policy's. In old TLS builds without TLS 1.0, a TLS 1.0 min is TLS 1.1. - A DTLS min on a TLS CTX fails, and so does an SSL max of the other major. - The CTX max setter rebuilds ctx->suites when it changes the method version. - wolfSSL_CTX_SetMinVersion() clears the automatic min flag. - WOLFSSL_MIN_DOWNGRADE is TLS 1.1 in old TLS builds without WOLFSSL_ALLOW_TLSV10. - With WOLFSSL_SYS_CRYPTO_POLICY at level 0, including when no policy is loaded, a new TLS CTX keeps the default min instead of an SSLv3 min. Cipher suites: - The CTX PSK callback setters and wolfSSL_CTX_allow_anon_cipher() rebuild ctx->suites when it was already derived. The max setter no longer leaves the min's options in ctx->mask, so SSLs created after a late PSK callback shared stale CTX suites without PSK. A cipher list set by the application is kept. Groups: - wolfSSL_CTX_set_groups() and wolfSSL_set_groups() build the Supported Groups extension in a local list and replace the old one only on success. They move from tls.c to ssl_api_ext.c. - set_curves_list() sets the groups once, after parsing. - wolfSSL_UseSupportedCurve() and wolfSSL_CTX_UseSupportedCurve() set userCurves only when the group was added. Signature algorithms: - SetSuitesHashSigAlgo() parses into a local buffer and copies it only when the whole list parses. - wolfSSL_CTX_set1_sigalgs_list() on a CTX without suites frees the suites it allocated when the list is rejected, and adds the default cipher suites when it is accepted.
|
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
No active review issues are reported for the setter-state preservation and derived cipher-suite consistency changes.
No open issues 🎉
What changed in this PR
This PR makes wolfSSL protocol-version, group, and signature-algorithm setters preserve existing state when a call fails. It also keeps derived cipher suites consistent with later configuration changes.
Changes:
- Apply protocol limits only on success while preserving application-set options and default minimum versions.
- Validate group and signature-algorithm lists before replacing existing settings.
- Refresh derived cipher suites after relevant configuration changes and add regression tests for state preservation and handshake behavior.
| File | Description |
|---|---|
wolfssl/internal.h |
Track setter-owned protocol options and adjust the default minimum version. |
tests/api.c |
Add regression coverage for failed setters, option preservation, and cipher-suite updates. |
src/tls13.c |
Refresh derived CTX suites when TLS 1.3 PSK callbacks change. |
src/tls.c |
Remove group setters relocated to ssl_api_ext.c. |
src/ssl.c |
Stage protocol-limit changes, track option ownership, refresh suites, and defer group updates until parsing completes. |
src/ssl_api_ext.c |
Replace groups only on success and handle fresh signature-algorithm suite allocation. |
src/internal.c |
Preserve signature algorithms on parse failure and correct the crypto-policy default minimum. |
Review effort: Balanced
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
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.
Several wolfSSL setters could leave a CTX or SSL half-modified when they failed. This change makes them apply their result only on success, and fixes related cipher suite and option handling.
Protocol versions
NO_*options they set and drop only those, so application options are kept and a later call can lower the min or raise the max. The max setter no longer re-applies the min.wolfSSL_CTX_set_options(),wolfSSL_CTX_clear_options()andwolfSSL_CTX_SetMinVersion()update the records.minDowngrade(the default, one set withwolfSSL_CTX_SetMinVersion()orwolfSSL_SetMinVersion(), or the crypto policy's). In old TLS builds without TLS 1.0, a TLS 1.0 min is TLS 1.1.ctx->suiteswhen it changes the method version.wolfSSL_CTX_SetMinVersion()clears the automatic min flag.WOLFSSL_MIN_DOWNGRADEis TLS 1.1 in old TLS builds withoutWOLFSSL_ALLOW_TLSV10.WOLFSSL_SYS_CRYPTO_POLICYat level 0, including when no policy is loaded, a new TLS CTX keeps the default min instead of an SSLv3 min.Cipher suites
wolfSSL_CTX_allow_anon_cipher()rebuildctx->suiteswhen it was already derived. The max setter no longer leaves the min's options inctx->mask. Previously, SSLs created after a late PSK callback shared stale CTX suites without PSK. A cipher list set by the application is kept.Groups
wolfSSL_CTX_set_groups()andwolfSSL_set_groups()build the Supported Groups extension in a local list and replace the old one only on success. They move fromtls.ctossl_api_ext.c.set_curves_list()sets the groups once, after parsing.wolfSSL_UseSupportedCurve()andwolfSSL_CTX_UseSupportedCurve()setuserCurvesonly when the group was added.Signature algorithms
SetSuitesHashSigAlgo()parses into a local buffer and copies it only when the whole list parses.wolfSSL_CTX_set1_sigalgs_list()on a CTX without suites frees the suites it allocated when the list is rejected, and adds the default cipher suites when it is accepted.ZD22696