Skip to content

Fix brainpool group and version downgrade handling for TLS 1.3, DTLS 1.3 and ECH - #11857

Open
Frauschi wants to merge 13 commits into
wolfSSL:masterfrom
Frauschi:fix-brainpool-downgrade
Open

Frauschi wants to merge 13 commits into
wolfSSL:masterfrom
Frauschi:fix-brainpool-downgrade

Conversation

@Frauschi

@Frauschi Frauschi commented Oct 22, 2026 •

Copy link
Copy Markdown
Member

Description

A client with TLS 1.3 or DTLS 1.3 as its minimum version still offered the RFC 7027 brainpool codepoints (26/27/28). The minDowngrade check was always true, because DTLS minors count down.

  1. TLSX_PreTls13Reachable() offers the RFC 7027 codepoints only when supported_versions would also offer a version before 1.3, taking the minimum version and the version masks into account. A TLS 1.3 server no longer sends them in EncryptedExtensions.
  2. The DTLS version now follows the version mask, as in OpenSSL 4.1 (the first release with DTLS 1.3, where SSL_OP_NO_DTLSv1_3 is the SSL_OP_NO_TLSv1_3 bit). Before, WOLFSSL_OP_NO_TLSv1_3 dropped DTLS 1.3 from supported_versions but left the SSL on DTLS 1.3, with a TLS 1.3 key_share and the TLS 1.3-only groups.
    • wolfSSL_set_options(), wolfSSL_set_max_proto_version() and wolfSSL_set_session() move a DTLS SSL that may downgrade to the newest version the mask and minimum allow. wolfSSL_connect(), wolfSSL_accept() and their _TLSv13 variants check the accumulated mask and minimum again when the handshake starts, so the order of the calls does not matter. The _TLSv13 variants hand a DTLS SSL below 1.3 to the legacy handshake.
    • The min and max proto version setters handle DTLS: DTLS1_VERSION and DTLS1_2_VERSION cap a DTLS SSL, DTLS1_3_VERSION is accepted, a version of 0 no longer caps a DTLS CTX at DTLS 1.2 or stores a TLS-numbered minimum (which made a DTLS 1.3 client send an empty supported_versions), and a TLS minimum on a DTLS object maps to the DTLS version with the same mask bit.
    • A DTLS server picks only allowed versions from the client's supported_versions, and checks the version it steps down to for a ClientHello without one. A DTLS client refuses a server version its mask does not allow.
    • Methods that cannot downgrade keep their version, as OpenSSL's fixed-version methods ignore the mask.
  3. ECH: the inner ClientHello no longer pulls in the outer brainpool list. The outer hello keeps its own groups in the same extension position as a GREASE ECH hello, and a TLS 1.2 or older ServerHello switches back to them. After a HelloRetryRequest, both lists stay as they were in the first hello. A pre-1.3 ServerHello after a HelloRetryRequest that accepted ECH now fails with illegal_parameter (RFC 9849 section 6.1.5).

Behavior changes:

  • A downgrade-capable DTLS SSL with no allowed version left fails in wolfSSL_connect()/wolfSSL_accept() with VERSION_ERROR, or in wolfSSL_new() when the mask comes from the CTX. Before, it went on and negotiated a masked version or failed later.
  • A session whose DTLS version the mask no longer allows is not resumed.
  • New SSL_OP_NO_DTLSv1, SSL_OP_NO_DTLSv1_2 and SSL_OP_NO_DTLSv1_3 aliases, with OpenSSL's values. SSL_OP_NO_DTLSv1 is WOLFSSL_OP_NO_TLSv1_1, the bit wolfSSL masks DTLS 1.0 with, so on a TLS object it masks TLS 1.1 rather than TLS 1.0.
  • wolfSSL_set_max_proto_version() with a DTLS version on a DTLS SSL now caps it; before it did nothing. wolfSSL_CTX_get_min_proto_version() reports DTLS minimums instead of 0.
  • A DTLS version as the minimum of a TLS SSL or CTX (including wolfSSL_(CTX_)SetMinVersion(), which returns BAD_FUNC_ARG), or as the maximum of a TLS SSL, now fails, as it already did for the CTX maximum. A TLS version as the maximum of a DTLS CTX still fails, but no longer masks DTLS 1.3 on the way. DTLS1_3_VERSION as the maximum of a fixed-version DTLS 1.2 or 1.0 CTX fails instead of turning it into DTLS 1.3.

#11756 touches the same check in TLSX_DefaultGroupAllowed(). Whichever PR lands second has to carry it over.

Testing

  • test_TLSX_SupportedCurve_brainpool_downgrade: client offers across min versions, methods and masks (TLS and DTLS), the TLS 1.3 server reply, the ECH inner and outer lists, the outer extension order against GREASE ECH, and HelloRetryRequest with ECH accepted and rejected.
  • test_dtls13_no_tls13_option: the DTLS mask on the client and server, through the SSL and the CTX, with up to three separate option calls, cases with no version left, the cipher suites after a move, wolfSSL_negotiate(), a DTLS 1.2-only peer on either side, a supported_versions list with only DTLS 1.2, sessions set before or after the mask, early data, EMS after a dropped resumption, an SNI callback that sets the mask mid-ClientHello, a minimum set after the mask, the SSL and CTX min and max proto versions including 0, and the _TLSv13 entry points on an SSL already at DTLS 1.2.
  • test_tls13_ech_hrr_tls11_server_hello: a TLS 1.1 ServerHello after an ECH-accepting HelloRetryRequest, and a TLS 1.1 fallback without one.

@Frauschi Frauschi self-assigned this Oct 22, 2026
Copilot AI balanced review requested due to automatic review settings October 22, 2026 07:51
@github-actions

github-actions Bot commented Oct 22, 2026 •

Copy link
Copy Markdown

MemBrowse Memory Report · Explore what changed →

gcc-arm-cortex-m3

  • FLASH: .text +44 B (+0.0%, 130,861 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m4

  • FLASH: .text +64 B (+0.0%, 207,280 B / 262,144 B, total: 79% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +704 B (+0.4%, 195,032 B / 1,048,576 B, total: 19% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .text +64 B (+0.0%, 793,628 B / 1,048,576 B, total: 76% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +64 B (+0.0%, 309,136 B / 1,048,576 B, total: 29% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +64 B (+0.0%, 340,488 B / 1,048,576 B, total: 32% used)

gcc-arm-cortex-m4-tls12

  • FLASH: .text +64 B (+0.0%, 131,665 B / 262,144 B, total: 50% used)

gcc-arm-cortex-m7

  • FLASH: .text +64 B (+0.0%, 207,280 B / 262,144 B, total: 79% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +64 B (+0.0%, 310,032 B / 1,048,576 B, total: 30% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +64 B (+0.0%, 246,626 B / 262,144 B, total: 94% used)

zephyr-frdm_rw612

  • FLASH: log_const_area -8 B, rodata -8 B, text +160 B (+0.1%, 282,608 B / 67,108,864 B, total: 0% used)

No memory changes detected for:


Catch memory regressions in CI with MemBrowse. Free for open source.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

DTLS downgrade handling can bypass mandatory PSK enforcement, select a compiled-out protocol, and leave a disabled version negotiable after separate option calls.

3 open issues
What changed in this PR

This PR aligns brainpool group advertising with reachable TLS/DTLS versions and separates ECH inner and outer group lists.

Changes:

  • Filter legacy brainpool codepoints using minimum versions and protocol masks.
  • Add DTLS downgrade handling when TLS 1.3 is disabled.
  • Preserve ECH group lists through fallback and HelloRetryRequest, with regression tests.
File Description
wolfssl/​internal.h Declare the DTLS downgrade helper.
tests/​api/​test_tls_ext.h Register the brainpool regression test.
tests/​api/​test_tls_ext.c Test brainpool offers, ECH fallback, and retries.
tests/​api/​test_dtls.h Register the DTLS version-mask test.
tests/​api/​test_dtls.c Test SSL- and CTX-level DTLS downgrade behavior.
tests/​api.c Add brainpool coverage to the API suite.
src/​tls.c Filter brainpool groups and separate ECH lists.
src/​ssl.c Apply DTLS fallback when TLS 1.3 is masked.
src/​internal.c Select DTLS fallback versions and restore ECH outer extensions.

Review effort: Balanced

Comment thread src/ssl.c Outdated
Comment thread src/internal.c Outdated
Comment thread src/ssl.c Outdated
@Frauschi

Copy link
Copy Markdown
Member Author

@wolfSSL-Fenrir-bot review balanced

@Frauschi

Copy link
Copy Markdown
Member Author

@claude review

@claude claude Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reviewed this PR and didn't find any bugs. It changes TLS/DTLS version downgrade and ECH ClientHello construction in a crypto library, so a human look would still be worthwhile.

What was reviewed:

  • TLSX_PreTls13Reachable() replacing the always-true minDowngrade check: DTLS minors are compared with versionIsLessEqual, and the mask checks are guarded by OPENSSL_EXTRA/HAVE_WEBSERVER/WOLFSSL_WPAS_SMALL.
  • Dtls13DowngradeMinor() in the CTX path (SetSSL_CTX_CheckVersion, which returns VERSION_ERROR when no version is left) and in the wolfSSL_set_options path, plus its WOLFSSL_NO_TLS12/NO_OLD_TLS guards.
  • ECH: splitting supported_groups between the inner and outer hello, not compressing extensions that ech->extensions overrides, skipping the split after an HRR, and the new TLSX_EchReplaceExtensions call when a ServerHello picks TLS 1.2 or older.
Extended reasoning...

The change touches TLS/DTLS protocol-version negotiation (src/internal.c, src/ssl.c), how the default supported_groups list is built, and how the ECH inner and outer ClientHello extensions are built (src/tls.c), and it adds about 640 lines of tests. That is security-sensitive handshake code: version downgrade logic and ECH privacy behavior. The bug hunt ran until it stopped finding anything and reported no findings. Earlier Copilot inline comments on ssl.c and internal.c were resolved only by the PR author, after follow-up commits. Because of the protocol-level scope and the many feature-macro combinations, this should get a human review rather than an automatic approval.

@wolfSSL-Fenrir-bot wolfSSL-Fenrir-bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fenrir Automated Review — PR #11857

Scan targets checked: wolfssl-src, wolfssl-bugs
Not reviewed (no scan target covers them): tests/api/test_dtls.h, tests/api/test_tls_ext.h
Coverage: 3 of 7 in-scope changed file(s) opened by the reviewer; not opened: tests/api.c, tests/api/test_dtls.c, tests/api/test_tls_ext.c, wolfssl/internal.h

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Reported findings require changes before merge.

Review tier: Balanced

Comment thread src/ssl.c Outdated
Comment thread src/internal.c
Comment on lines +8121 to 8141
#ifdef WOLFSSL_DTLS13
if (ssl->version.major == DTLS_MAJOR &&
ssl->version.minor == DTLSv1_3_MINOR &&
(ssl->options.mask & WOLFSSL_OP_NO_TLSv1_3) ==
WOLFSSL_OP_NO_TLSv1_3) {
byte minor = 0;

if (ctx->method->downgrade)
minor = Dtls13DowngradeMinor(ssl, ctx->minDowngrade);
if (minor == 0) {
WOLFSSL_MSG("\tInconsistent protocol options. DTLS 1.3 set but "
"not allowed and no lower DTLS version allowed.");
WOLFSSL_ERROR_VERBOSE(VERSION_ERROR);
return VERSION_ERROR;
}
WOLFSSL_MSG("\tOption set to not allow DTLSv1.3, Downgrading");
ssl->version.minor = minor;
}
#endif
if (ssl->version.minor == TLSv1_2_MINOR &&
(ssl->options.mask & WOLFSSL_OP_NO_TLSv1_2) == WOLFSSL_OP_NO_TLSv1_2) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Openssl doesn't use SSL_OP_NO_TLSv1_3 for DTLS. Why are we allowing this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OpenSSL does now. 4.1, the first release with DTLS 1.3, defines SSL_OP_NO_DTLSv1_3 as SSL_OP_BIT(29), the same bit as SSL_OP_NO_TLSv1_3, and uses it as the mask of the DTLS 1.3 methods (ssl/methods.c). DTLS_method() picks its version in ssl_get_min_max_version() when the handshake starts, skipping masked versions, so SSL_OP_NO_DTLSv1_3 gives DTLS 1.2 there.

wolfSSL master already did half of this: TLSX_SupportedVersions_GetSize() drops DTLS 1.3 from supported_versions under WOLFSSL_OP_NO_TLSv1_3, but the SSL stayed on DTLS 1.3 and still sent a TLS 1.3 key_share and the TLS 1.3-only groups. The PR now follows OpenSSL. wolfSSL_set_options() and wolfSSL_set_session() move the version straight away. wolfSSL_connect()/wolfSSL_accept() and the _TLSv13 entry points check the accumulated mask again when the handshake starts (DtlsCheckVersionMask()), so the order of the calls doesn't matter. On the server, the supported_versions parse only picks versions the mask allows. I also added SSL_OP_NO_DTLSv1_2 and SSL_OP_NO_DTLSv1_3, which use the same bits as in OpenSSL. I left out SSL_OP_NO_DTLSv1: OpenSSL defines it as the SSL_OP_NO_TLSv1 bit, while wolfSSL masks DTLS 1.0 with WOLFSSL_OP_NO_TLSv1_1, so the alias would behave differently.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, the OpenSSL 4.1 behavior checks out.

On SSL_OP_NO_DTLSv1: WOLFSSL_OP_NO_TLSv1_1 is 0x04000000, bit 26. That’s the same value OpenSSL uses for SSL_OP_NO_DTLSv1. So #define SSL_OP_NO_DTLSv1 WOLFSSL_OP_NO_TLSv1_1 would match OpenSSL’s value and mask DTLS 1.0 the way wolfSSL does. The only difference is on a TLS context, where it would mask TLS 1.1 rather than TLS 1.0. Code that sets a DTLS option on a TLS context should be rare.

Without the alias, code ported from OpenSSL that uses SSL_OP_NO_DTLSv1 won’t compile. I’d add the alias and document the TLS-context difference. What do you think?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, I had the bits wrong. wolfSSL's WOLFSSL_OP_NO_TLSv1_1 is OpenSSL's SSL_OP_NO_TLSv1/SSL_OP_NO_DTLSv1 value, so the alias matches OpenSSL's value and wolfSSL's DTLS 1.0 masking. Added SSL_OP_NO_DTLSv1 as WOLFSSL_OP_NO_TLSv1_1, and the wolfSSL_CTX_set_options() doc table now lists the three DTLS options, with the TLS 1.1 effect on a TLS object called out. test_dtls13_no_tls13_option uses the new name.

Comment thread src/ssl.c Outdated
Comment on lines +7601 to +7612
#ifdef WOLFSSL_DTLS13
else if (ssl->version.major == DTLS_MAJOR && ssl->options.downgrade &&
(ssl->version.minor == DTLSv1_3_MINOR ||
(ssl->version.minor == DTLSv1_2_MINOR &&
(ssl->options.mask & WOLFSSL_OP_NO_TLSv1_2) ==
WOLFSSL_OP_NO_TLSv1_2))) {
byte minor = Dtls13DowngradeMinor(ssl,
ssl->options.minDowngrade);
if (minor != 0)
ssl->version.minor = minor;
}
#endif

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is now DtlsMaskVersion(), shared with the check at the start of the handshake. See the reply on internal.c for the OpenSSL 4.1 behavior it follows.

@sebastian-carpenter sebastian-carpenter left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  HIGH-1: ECH split moves supported_groups to the front of the outer ClientHello, so real ECH can be told apart from GREASE ECH [BLOCK] (bug)
  File:       src/tls.c:16804-16814
  Function:   TLSX_EchSplitSupportedGroups
  Confidence: High

  `TLSX_EchSplitSupportedGroups` runs at the end of `TLSX_PopulateExtensions`, after ECH, key_share, psk_kex_modes and the rest have been pushed. It calls `TLSX_Remove(&ssl->extensions, TLSX_SUPPORTED_GROUPS, ...)` and then `TLSX_PopulateSupportedGroups(ssl, &ssl->extensions)`. `TLSX_UseSupportedCurve` creates the new node with `TLSX_Push`, which puts it at the head of `ssl->extensions`.

`TLSX_Write` walks the list from head to tail. Before this change, supported_groups was pushed early (around line 16859) and was written near the end of the ClientHello. After the split it is the head node, so it is written first.

When the outer hello is written, `TLSX_EchSwapExtensions` swaps nodes by type in place. The outer supported_groups therefore takes over the moved position too. The result: a ClientHello with real ECH (echConfigs set, brainpool built in, TLS 1.2 still reachable) has a different extension order than a GREASE-ECH or non-ECH ClientHello from the same client. A passive observer can spot real ECH from the order alone, which defeats the point of sending GREASE ECH (RFC 9849 §6.2).

Before this diff the outer SNI was swapped in place and the order matched GREASE. The CH1/CH2 comparison in `test_brainpool_ech_hrr` does not catch this, because both hellos come after the split.

  Code:
        ech = (WOLFSSL_ECH*)echX->data;
        ret = TLSX_PopulateSupportedGroups(ssl, &ech->extensions);
        if (ret == WOLFSSL_SUCCESS) {
            TLSX_Remove(&ssl->extensions, TLSX_SUPPORTED_GROUPS, ssl->heap);
            downgrade = ssl->options.downgrade;
            ssl->options.downgrade = 0;
            ret = TLSX_PopulateSupportedGroups(ssl, &ssl->extensions);
            ssl->options.downgrade = downgrade;
        }

  Recommendation: Leave the existing SUPPORTED_GROUPS node where it is in `ssl->extensions` and swap only its `data` for the TLS 1.3-only list built into a temporary list. `TLSX_SupportedCurve_CheckPriority` already swaps `data` this way. Also add a test that compares the extension type order of a real-ECH outer ClientHello with a GREASE-ECH ClientHello from the same configuration; `test_bp_parse_ch` can extract the type lists.
  MEDIUM-2: TLS 1.2 ServerHello ECH hook passes echAccepted, though the comment says the outer hello is answered [SUGGEST] (bug)
  File:       src/internal.c:36437-36442
  Function:   DoServerHello
  Confidence: Medium

  The new hook's comment says "A server negotiating TLS 1.2 or older answers the outer hello", but it calls `TLSX_EchReplaceExtensions(ssl, ssl->options.echAccepted)`.

`echAccepted` can only be 1 here after a HelloRetryRequest that accepted ECH. `tls13.c:5722` deliberately keeps `ech->extensions` for CH2 in that case. If the server then answers with a TLS 1.2 or lower ServerHello (for example the `wantDowngrade` path at `tls13.c:5863`, which runs before the HRR cipher-suite check), the client takes the accepted branch. It frees the outer extensions and continues with the inner ones; certificate verification also skips the public_name check because `echAccepted` is set.

The TLS 1.3 path treats "HRR accepted, ServerHello did not" as fatal (`tls13.c:5699`, RFC 9849 §6.1.5). The new hook is the natural place to enforce the same rule for the downgrade path, and as written it encodes the opposite of its own comment.

  Code:
    #if defined(WOLFSSL_TLS13) && defined(HAVE_ECH)
            /* A server negotiating TLS 1.2 or older answers the outer hello. */
            ret = TLSX_EchReplaceExtensions(ssl, ssl->options.echAccepted);
            if (ret != 0)
                return ret;
    #endif

  Recommendation: Pass 0 explicitly so the code matches the comment. Also reject the connection when `echAccepted` is already set, matching the TLS 1.3 ServerHello handling.

@Frauschi
Frauschi force-pushed the fix-brainpool-downgrade branch from e40d13a to d3bf702 Compare October 22, 2026 20:39
@Frauschi

Copy link
Copy Markdown
Member Author

@sebastian-carpenter thanks, both fixed.

HIGH-1: confirmed, the re-pushed supported_groups ended up at the head of the list. TLSX_EchSplitSupportedGroups() now builds the inner list in a temporary list and swaps only the data into the existing node. The brainpool test now compares the outer ClientHello's extension order with a GREASE ECH one from the same config; that check failed before the fix.

MEDIUM-2: DoServerHello() now fails with illegal_parameter when a HelloRetryRequest in this handshake accepted ECH, and passes 0 otherwise. It keys on msgsReceived.got_hello_retry_request, so it covers both ways a pre-1.3 ServerHello gets there after an HRR: a TLS 1.1 or older one through the wantDowngrade path, and a TLS 1.2 one with extensions that repeats the HRR's cipher suite. Any other TLS 1.2 ServerHello already fails the HRR cipher suite check. test_tls13_ech_hrr_tls11_server_hello covers the first path.

Comment thread src/tls13.c Outdated
}
if (ssl->version.minor != minor) {
#ifdef WOLFSSL_EARLY_DATA
ssl->earlyData = no_early_data;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this still be reached after d3bf702? Now that SetSession() applies the mask, wolfSSL_new(), wolfSSL_set_options() and wolfSSL_set_session() all move the version straight away. So I’d expect wolfSSL_write_early_data() to see DTLS 1.2 and return BAD_FUNC_ARG before it sets earlyData. That’s also what test_dtls13_mask_early_data checks. Is there a path I’m missing where the version only changes here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, there was one: wolfSSL_set_max_proto_version(ssl, TLS1_2_VERSION) sets WOLFSSL_OP_NO_TLSv1_3 directly, without going through wolfSSL_set_options(), so the SSL stayed at DTLS 1.3. wolfSSL_write_early_data() then accepted the data, and this check moved the SSL to DTLS 1.2. Without the reset the handshake completed but the next wolfSSL_write() failed.

wolfSSL_set_max_proto_version() now moves the version straight away, and also handles DTLS1_VERSION, DTLS1_2_VERSION and DTLS1_3_VERSION; before, the DTLS versions left the mask alone, so capping a DTLS SSL had no effect. While testing that I hit two more version-0 problems on master and fixed them in the same commit: wolfSSL_(CTX_)set_min_proto_version(x, 0) stored a TLS-numbered minimum on a DTLS object, which made a DTLS 1.3 client send an empty supported_versions (the server answers decode_error), and wolfSSL_CTX_set_max_proto_version(ctx, 0) set a DTLS CTX's method to DTLS 1.2, because protoVerTbl stopped at DTLS1_2_VERSION. It now ends at DTLS1_3_VERSION.

The one path left is SetSSL_CTX() under WOLFSSL_WPAS_SMALL without OPENSSL_EXTRA: it copies the CTX mask but SetSSL_CTX_CheckVersion() is OPENSSL_EXTRA-only, so the version is not moved until the handshake starts. I kept the reset for that case. The hand-off now also runs for any DTLS SSL below 1.3 (see the Copilot thread below), and clearing earlyData there is right in either case.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

A minimum-version call-order bypass and incorrect direct TLS 1.3 client dispatch remain unresolved.

2 open issues
3 resolved since last review

Review effort: Balanced

Comment thread src/ssl.c Outdated
Comment thread src/tls13.c Outdated
@Frauschi
Frauschi force-pushed the fix-brainpool-downgrade branch 3 times, most recently from 26da4b8 to fc951ae Compare October 23, 2026 15:52

@julek-wolfssl julek-wolfssl left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1. A DTLS cap on an SSL can't be undone (src/ssl.c:7794)
DtlsMaskVersion() only moves the version down. After set_max_proto_version(ssl, DTLS1_2_VERSION), calling it again with DTLS1_3_VERSION or 0 returns success but leaves the SSL on DTLS 1.2. set_options(SSL_OP_NO_DTLSv1_3)
followed by clear_options() does the same. Starting the walk from ctx->method->version.minor, as the resume path already does, would fix it.

2. A DTLS 1.3-only CTX can't be restored (src/ssl.c:5410)
On wolfDTLSv1_3_client_method(), capping at DTLS1_2_VERSION works. Setting it back to DTLS1_3_VERSION then fails (the !downgrade && minor != DTLSv1_3_MINOR guard), and 0 leaves it at 1.2.

3. With async crypto, accept can switch versions mid-ClientHello (src/ssl.c:7801)
The setters skip the mask walk once got_client_hello is set, but the DtlsCheckVersionMask() calls at the start of wolfSSL_accept/wolfSSL_accept_TLSv13 only check acceptState == ACCEPT_BEGIN. Suppose the SNI callback sets
SSL_OP_NO_DTLSv1_3 and key-share generation then returns WC_PENDING_E. On re-entry the SSL moves to DTLS 1.2 and is handed to the legacy accept with a DTLS 1.3 ClientHello half-processed. Not reproduced. Checking got_client_hello
inside DtlsMaskVersion() would cover every caller.

4. SSL and CTX setters disagree (src/ssl.c:5615 vs :5307)
On a DTLS SSL, wolfSSL_set_max_proto_version() accepts TLS constants. wolfSSL_CTX_set_max_proto_version() rejects them on a DTLS CTX. Is that intended?

5. Weak test (tests/api/test_dtls.c:16107)
test_dtls13_ctx_min_tls13 only asserts the version isn't DTLS 1.2. It should assert the exact expected result.

…chable

A TLS 1.3 client also lists the RFC 7027 brainpool codepoints when it may
fall back to an earlier version. The check compared minDowngrade against both
TLSv1_2_MINOR and DTLSv1_2_MINOR with <=, which is true for every TLS minor
and for DTLS 1.3, whose minor (0xFC) is below DTLS 1.2's. A client with TLS 1.3
or DTLS 1.3 as its minimum version therefore still offered the pre-1.3
codepoints.

Add TLSX_PreTls13Reachable(), which answers whether supported_versions would
also offer a version before 1.3, the same way TLSX_SupportedVersions_GetSize()
decides it: downgrade allowed, and TLS 1.2, TLS 1.1 (DTLS 1.0) or, with
WOLFSSL_ALLOW_TLSV10, TLS 1.0 at or above the minimum version and not masked
off with WOLFSSL_OP_NO_TLSv1_2, _1_1 or NO_TLSv1. A build with neither
TLS 1.2 nor old TLS never offers them. DTLS minors count down.

TLSX_PopulateSupportedGroups() also builds the list a TLS 1.3 server sends in
EncryptedExtensions when the client lacks its first choice. The version is
settled by then, so the helper answers 0 on the server side.

Adds a test for the P256, P384 and P512 brainpool codepoints a TLS, TLS
1.2-only, TLS 1.3-only and DTLS client offers with a minimum version set on
the SSL or the CTX, with versions masked off down to TLS 1.0 and DTLS 1.0,
and in the list a TLS 1.3 server sends back.
wolfSSL_set_options() and SetSSL_CTX_CheckVersion() move an SSL from TLS 1.3
to TLS 1.2 when WOLFSSL_OP_NO_TLSv1_3 is set, but compare the version against
TLSv1_3_MINOR only. A DTLS 1.3 SSL (minor 0xFC) kept its version, so its
ClientHello offered only older DTLS versions in supported_versions while
still carrying a TLS 1.3 key_share and the TLS 1.3-only groups (ML-KEM
hybrids, RFC 8734 brainpool).

Add Dtls13DowngradeMinor(), which picks the newest older DTLS version that
the version mask and minimum still allow and that is built: DTLS 1.2, or
DTLS 1.0 when old TLS is built. The later TLS mask steps only match TLS
minors, so DTLS needs its own choice. wolfSSL_set_options() moves to it when
there is one and the SSL may downgrade, so wolfSSL_require_psk() and the
DTLS 1.3-only methods keep DTLS 1.3. It also steps down again when a later
call masks DTLS 1.2 on an SSL it already moved there, so separate calls end
where one combined call does. SetSSL_CTX_CheckVersion() returns
VERSION_ERROR when the method cannot downgrade or no DTLS version is left,
as it does for TLS.

Adds test_dtls13_no_tls13_option for the option on the client or server SSL
or CTX, with DTLS 1.2 also masked off in the same or a separate call or
below the minimum, with no DTLS version left, in builds without TLS 1.2, for
a DTLS 1.3-only method and after wolfSSL_require_psk(), and a DTLS case to
the brainpool supported_groups test.
The inner ClientHello offers only TLS 1.3, as downgrade is cleared while it is
written, but it referenced the outer supported_groups through
ech_outer_extensions and so still carried the pre-1.3 brainpool codepoints.

When ECH is in use and a pre-1.3 version is reachable, the client now gives
the outer ClientHello its own default groups list in ech->extensions, the way
the public-name SNI is handled, and builds the inner list without the pre-1.3
groups. An extension with an outer-only copy is no longer referenced from
ech_outer_extensions.

A server that negotiates TLS 1.2 or older answers the outer ClientHello, so
DoServerHello() now swaps the outer extensions back in, as the TLS 1.3
rejection paths already do, unless a HelloRetryRequest accepted ECH. Without
that the client kept the inner list and could not use a brainpool client
certificate in the TLS 1.2 handshake.

A second ClientHello after a HelloRetryRequest keeps the lists the first one
sent, as RFC 8446 requires: the split is skipped then, and the default groups
are no longer added again to an existing supported_groups extension, which
would have put the pre-1.3 groups back into the inner list.

Adds ECH cases to the brainpool test: the inner list as the server parsed it,
the client's list after a TLS 1.2 ServerHello, and, with ECH accepted and
rejected, that the ClientHello after a HelloRetryRequest repeats the first
one's supported_groups and its position. supported_versions is left out of
the order check, as it already moves in every second ClientHello.
wolfSSL_set_options(): when a later call masks DTLS 1.2 on an SSL that an
earlier WOLFSSL_OP_NO_TLSv1_3 moved there and no older DTLS version is left,
put the SSL back on DTLS 1.3 so the handshake fails, as it does after one
combined call. OpenSSL 4.1 checks the accumulated mask when the handshake
starts, so the order of the calls does not matter there either. The DTLS
branch now leaves the version alone once the handshake has started.

Add SSL_OP_NO_DTLSv1, SSL_OP_NO_DTLSv1_2 and SSL_OP_NO_DTLSv1_3. OpenSSL 4.1,
the first release with DTLS 1.3, defines SSL_OP_NO_DTLSv1_3 as the
SSL_OP_NO_TLSv1_3 bit. SSL_OP_NO_DTLSv1 maps to WOLFSSL_OP_NO_TLSv1_1, the
option wolfSSL masks DTLS 1.0 with.

TLSX_EchSplitSupportedGroups() removed the supported_groups extension and
pushed the inner list back at the head of ssl->extensions, so a real ECH
outer ClientHello wrote supported_groups first and could be told apart from
a GREASE ECH one by its extension order. It now builds the inner list in a
temporary list and swaps only the data into the existing extension.

DoServerHello(): a ServerHello for TLS 1.1 or older after a
HelloRetryRequest that accepted ECH now fails with illegal_parameter, as a
TLS 1.3 ServerHello that rejects ECH after an accepting HelloRetryRequest
does (RFC 9849 section 6.1.5). Otherwise the outer extensions are swapped
back in. A TLS 1.2 ServerHello there already fails the HelloRetryRequest
cipher suite check in DoTls13ServerHello().

Tests: separate set_options calls that leave no DTLS version, set_options
after the handshake, the SSL_OP_NO_DTLS names, the extension order of a real
ECH outer ClientHello against a GREASE ECH one, and
test_tls13_ech_hrr_tls11_server_hello.
wolfSSL_set_options() moved a DTLS SSL to an older version as each option
call came in, so the result depended on the order of the calls: three
separate calls left a masked DTLS 1.0, a server put back on DTLS 1.3 when
nothing was left negotiated it anyway because the server never checks its
own mask, and a call made during the handshake still changed the version.

Do it the way OpenSSL 4.1 does instead and look at the accumulated mask once,
when wolfSSL_connect() or wolfSSL_accept() starts a handshake.
DtlsCheckVersionMask() moves a DTLS SSL that may downgrade to the newest
built version that the mask and minimum allow, or fails with VERSION_ERROR
when none is left. wolfSSL_set_options() no longer changes the DTLS version.
Methods that cannot downgrade keep their version, as OpenSSL's fixed-version
methods ignore the mask too.

A DTLS server that steps down to the client's version in DoClientHello() now
checks that version against the mask as well, the way the TLS path already
walks it, and refuses with protocol_version when no allowed version is left.
Before, a DTLS server negotiated a masked DTLS 1.2 or 1.0 with a client that
offered only that version.

Dtls13DowngradeMinor() becomes DtlsAllowedMinor(), which also takes the
newest version to start from and is built with DTLS 1.2 alone.

The ECH check in DoServerHello() now applies only when this handshake had a
HelloRetryRequest. wolfSSL_clear() keeps echAccepted, so a reused SSL whose
last handshake accepted ECH failed a TLS 1.2 or older fallback. An empty
inner groups list no longer fails the ECH split. SSL_OP_NO_DTLSv1_3 is
defined only when DTLS 1.3 is built, like SSL_OP_NO_TLSv1_3.

test_dtls13_no_tls13_option now runs every mask case on the client and the
server, through SSL and CTX, with up to three separate calls, and checks the
version after the handshake. It adds a DTLS 1.2-only client against a server
that masks DTLS 1.2, and checks that a DTLS 1.3-only method and
wolfSSL_require_psk() keep DTLS 1.3 through wolfSSL_connect().
test_tls13_ech_hrr_tls11_server_hello adds a TLS 1.1 fallback with a stale
echAccepted.
DtlsCheckVersionMask() lowered the version without rebuilding the cipher
suites, so a client or server moved to DTLS 1.2 or 1.0 kept the list built
for DTLS 1.3 and could negotiate a TLS 1.3 or TLS 1.2-only suite at DTLS
1.0. The suite rebuild at the end of wolfSSL_set_options() moves into
ResetSuitesForVersion(), unchanged, and DtlsCheckVersionMask() calls it
after a move.

wolfSSL_negotiate(), which wolfSSL_read() and wolfSSL_write() use, and
callers of wolfSSL_connect_TLSv13() or wolfSSL_accept_TLSv13() went straight
to the TLS 1.3 state machines and skipped the check. Both now run it at the
start of the handshake and hand over to wolfSSL_connect() or
wolfSSL_accept() when the version drops below DTLS 1.3.

The check keeps the current version whenever the mask allows it and only
applies the minimum when it has to step down, so a DTLS context left with a
TLS-numbered minimum by wolfSSL_CTX_set_min_proto_version(ctx, 0) no longer
fails every handshake once any option bit is set. It skips SSLs that already
completed a handshake, so a renegotiation cannot change the version.

A DTLS client now also checks the version the server chose against its own
mask in CheckVersion(), as the TLS client does. Before, a client that masked
DTLS 1.2 still completed a DTLS 1.2 handshake with a server that only spoke
DTLS 1.2.

Tests: no TLS 1.3 suite is left after a move, a handshake driven by
wolfSSL_negotiate() on a masked server, a client masking DTLS 1.2 against a
DTLS 1.2-only server, and builds without TLS 1.2 but with old TLS expect
DTLS 1.0 instead of no version.
DtlsCheckVersionMask() lowered the version but left options.tls1_3 and a
resumption of a DTLS 1.3 session in place. After wolfSSL_set_session() with
such a session, wolfSSL_connect() still went into the TLS 1.3 state machine,
where SendTls13ClientHello() compares the session version in TLS order and
put the SSL back on DTLS 1.3 to build the ClientHello. A move now clears
tls1_3 and drops a resumption whose version is no longer the SSL's.

wolfSSL_set_options() calls DtlsCheckVersionMask() as well, so a masked SSL
reports and acts as the older version before the handshake, as it does when
the mask comes from the CTX, and calls such as wolfSSL_write_early_data()
that check the version before the handshake see the older one. The check at
the start of the handshake stays and catches a session or option set later.
Neither moves the version once the handshake has begun.

The DTLS server mask check in DoClientHello() ran only when the server
stepped down to the legacy ClientHello version. A client whose
supported_versions offered DTLS 1.2 and nothing newer was negotiated at the
masked DTLS 1.2. The check now runs for every DTLS ClientHello that may
downgrade and rebuilds the suites through ResetSuitesForVersion() when it
moves.

The client's check in CheckVersion() skips a renegotiation, as the
handshake-start check does, so a mask set after the handshake does not break
it. The ECH HelloRetryRequest rejection in DoServerHello() returns
SOCKET_ERROR_E when the alert cannot be sent, like its neighbors.

Tests: the version right after wolfSSL_set_options(), a DTLS 1.3 session set
before or after the mask, and a ClientHello whose supported_versions offers
only DTLS 1.2 against a server that allows only DTLS 1.3.
wolfSSL_set_session() lowers the SSL to the session's version, so with a
DTLS 1.2 session and DTLS 1.2 masked the walk started at DTLS 1.2 and a
client that allows DTLS 1.3 fell to DTLS 1.0 or failed with VERSION_ERROR.
A resumption now walks from the method's version, and the resumption is
dropped as before when its version is no longer the SSL's.

DtlsMaskVersion() only moves the version and DtlsCheckVersionMask() adds
the suite rebuild, so wolfSSL_set_options() rebuilds the suites once.
wolfSSL_connect() and wolfSSL_accept() check the mask only for the side
they serve, so a call on the wrong side fails with SIDE_ERROR without
changing the SSL first. The server check in DoClientHello() skips a
renegotiation, as the client's does.

Drop SSL_OP_NO_DTLSv1. OpenSSL defines it as the SSL_OP_NO_TLSv1 bit, while
wolfSSL masks DTLS 1.0 with WOLFSSL_OP_NO_TLSv1_1, so the alias would have
behaved differently from OpenSSL. SSL_OP_NO_DTLSv1_2 and SSL_OP_NO_DTLSv1_3
use the same bits as in OpenSSL and stay.

Adds a DTLS 1.2 session with DTLS 1.2 masked to the session cases.
The DTLS server stepped down from the version supported_versions picked to
the next one its mask allowed, so a server masking DTLS 1.2 answered a
client that listed only DTLS 1.2 with DTLS 1.0.
TLSX_SupportedVersions_Parse() now skips DTLS versions the mask does not
allow on an SSL that may downgrade, so it picks the newest listed version
that is allowed or refuses with protocol_version, as OpenSSL does. The
step-down in DoClientHello() remains for a ClientHello without
supported_versions, and now calls DtlsCheckVersionMask() instead of
repeating it.

The ECH rejection in DoServerHello() detects the HelloRetryRequest through
msgsReceived.got_hello_retry_request. DoTls13ServerHello() sets serverState
to SERVER_HELLO_COMPLETE before it calls DoServerHello() for a TLS 1.2
ServerHello that carries extensions, so the serverState test missed it.

wolfSSL_connect_TLSv13() resets earlyData when the mask moves the SSL below
DTLS 1.3 and it hands over to wolfSSL_connect(). wolfSSL_write_early_data()
had set it, and nothing on the DTLS 1.2 path cleared it, so every write
after the handshake failed.

When the mask drops a resumption, haveEMS goes back to the default
wolfSSL_clear() uses instead of keeping the session's, so the full
handshake offers extended_master_secret again.

Tests: a server masking DTLS 1.2 against a client listing only DTLS 1.2,
early data written with a DTLS 1.3 session after the mask, and a dropped
resumption of a session without EMS.
wolfSSL_set_session() puts the SSL on the session's version, so with the
mask already set, a DTLS 1.3 session took a masked SSL back to DTLS 1.3
until the handshake started. wolfSSL_write_early_data() then started a
DTLS 1.3 handshake, the connect path moved it to DTLS 1.2, and every
non-blocking retry returned BAD_FUNC_ARG while the error still read
WANT_READ. SetSession() now applies the mask too, so the version is right
before any early data is written.

wolfSSL_set_options() no longer moves the version once a ClientHello has
arrived. A server callback that sets options while the ClientHello is
parsed, such as an SNI callback re-applying per-host options, moved an SSL
that ClientHello had already settled on DTLS 1.3.

Only a client resumption changes the walk and resets haveEMS. A server
reused through wolfSSL_clear() keeps a stale resuming flag, and the reset
would have forced EMS on before the ClientHello was read.

DtlsAllowedMinor() reads the mask only under OPENSSL_EXTRA, HAVE_WEBSERVER
or WOLFSSL_WPAS_SMALL, the builds where supported_versions honours it, so
other builds do not offer a version and then refuse it.

Tests: early data after a DTLS 1.3 session set behind the mask is refused
up front with nothing left pending, and an SNI callback that masks DTLS 1.3
leaves the handshake on DTLS 1.3.
wolfSSL_set_max_proto_version() ignored DTLS1_VERSION and
DTLS1_2_VERSION on a DTLS SSL, so capping a DTLS 1.3 SSL at DTLS 1.2 did
nothing. A TLS version such as TLS1_2_VERSION set the mask bit but left
the SSL on DTLS 1.3 until the handshake started, so
wolfSSL_write_early_data() still accepted data. The DTLS versions now
set the matching mask bits, and the SSL moves to the newest allowed
version straight away, as wolfSSL_set_options() does. A call that fails
because no version is left restores the mask it found. DTLS1_VERSION
needs old TLS, as it already did for the minimum.

DTLS1_3_VERSION is accepted by the SSL and CTX min and max setters and
is in protoVerTbl, so a maximum of 0 walks to DTLS 1.3. Before, the walk
stopped at DTLS1_2_VERSION and capped a DTLS CTX at DTLS 1.2. On a
fixed-version DTLS method below DTLS 1.3, DTLS1_3_VERSION fails rather
than turning the method into DTLS 1.3, so a maximum of 0 walks on to
DTLS1_2_VERSION as before.

A TLS-numbered minimum on a DTLS SSL or CTX, from a min proto version of
0, a TLS version, or wolfSSL_SetMinVersion(), was compared as a DTLS
minor by supported_versions, so a DTLS 1.3 client sent an empty list and
the server answered decode_error. The min setters now store the DTLS
version that shares the mask bit. A DTLS version is refused as the
minimum of a TLS SSL or CTX, through the proto setters or
wolfSSL_SetMinVersion(), and as the maximum of a TLS SSL. The CTX max
setter refuses a version of the other kind before it sets any mask bit;
it used to set them first, so a failed TLS1_2_VERSION maximum on a DTLS
CTX still masked DTLS 1.3. GetMinProtoVersion() reports DTLS minimums,
so wolfSSL_CTX_set_max_proto_version() no longer drops one when it
restores the minimum, and a DTLS CTX minimum no longer clears the CTX's
version options, which compared TLS table positions with DTLS ones.

DtlsMaskVersion() kept the current version when the mask allowed it,
without checking the minimum, so WOLFSSL_OP_NO_TLSv1_3 followed by
wolfSSL_SetMinVersion(ssl, WOLFSSL_DTLSV1_3) ran a DTLS 1.2 handshake.
It now checks both and fails with VERSION_ERROR.

Add SSL_OP_NO_DTLSv1. It is WOLFSSL_OP_NO_TLSv1_1, which has OpenSSL's
SSL_OP_NO_DTLSv1 value and is the bit wolfSSL masks DTLS 1.0 with; on a
TLS object it masks TLS 1.1 rather than TLS 1.0. The
wolfSSL_CTX_set_options() table lists the three DTLS options.

Tests: a minimum set after the mask, through either setter, on the
client and the server; the SSL max proto version at DTLS 1.0, 1.2, 1.3,
a TLS version and 0; a CTX minimum and maximum of 0; a CTX maximum of 0
after a DTLS 1.2 maximum; a fixed DTLS 1.2 CTX under DTLS 1.0, DTLS 1.3
and 0 maximums; TLS 1.2, TLS 1.1 and TLS 1.3 minimums on a DTLS SSL and
CTX; a TLS maximum on a DTLS CTX; failed maximums leaving the SSL
unchanged; a TLS 1.1 maximum; the CTX DTLS 1.3 minimum surviving a later
maximum and keeping a masked DTLS 1.3 masked; and DTLS versions refused
on TLS objects.
wolfSSL_connect_TLSv13() and wolfSSL_accept_TLSv13() handed off to
wolfSSL_connect() and wolfSSL_accept() only when their own mask check
moved the version. wolfSSL_set_options(),
wolfSSL_set_max_proto_version() and CTX setup now move a DTLS SSL to
DTLS 1.2 before either entry point runs, so such an SSL went into the
TLS 1.3 state machine. With ECH built in, SendTls13ClientHello() failed,
since no ECH extension is added below TLS 1.3.

Both entry points now hand any DTLS SSL below 1.3 to the legacy
handshake, on every call. That also covers re-entry with non-blocking
I/O after the hand-off, which used to run the TLS 1.3 state machine on
the legacy connect or accept state. The hand-off clears tls1_3, since
wolfSSL_connect() and wolfSSL_accept() route on that flag: setting a
DTLS 1.3 session and then a DTLS 1.2 one left it set on a DTLS 1.2 SSL,
and the two entry points called each other until the stack overflowed.
The check only runs in builds with a pre-1.3 handshake.

Tests: both entry points, driven directly over memio, complete a DTLS
1.2 handshake after SSL-level and CTX-level masking, and a client given
a DTLS 1.3 session and then a DTLS 1.2 one completes a DTLS 1.2
handshake.
wolfSSL_set_max_proto_version() and wolfSSL_clear_options() only moved a
DTLS SSL down. After a maximum of DTLS 1.2, a maximum of DTLS 1.3 or 0
returned success and left the SSL on DTLS 1.2, and clearing
SSL_OP_NO_DTLSv1_3 did not bring DTLS 1.3 back. When a maximum loosens
the mask, or wolfSSL_clear_options() clears a DTLS version bit, the
version is now walked down from the CTX method version, so the SSL moves
back up. A lift never moves an allowed version down, and keeps an
allowed version when the walk finds none, so a CTX maximum lowered after
wolfSSL_new() does not reach the SSL that way. A resuming client keeps
its session's version while the mask allows it. wolfSSL_set_options()
and the checks at handshake start still only step down.

The DTLS maximum shares the WOLFSSL_OP_NO_TLSv1_2/_3 bits with
SSL_OP_NO_DTLSv1_2/_3. The new options field dtlsMaxMask records the
bits the maximum added itself, and a later maximum clears only those,
so a version the application or the CTX masked off stays off.
wolfSSL_set_options() and wolfSSL_clear_options() take a bit out of the
record.

The mask no longer moves the version while the first handshake is under
way. DtlsHandshakeStarted() reports that from handShakeDone, the connect
and accept states, got_client_hello, buffered output and, with async
crypto, an async state other than TLS_ASYNC_BEGIN or handler args kept
in ssl->async, since DoTls13ClientHello() clears got_client_hello when
it pends and can pend before it leaves TLS_ASYNC_BEGIN. Before, an async
accept that re-entered could switch to DTLS 1.2 partway through a DTLS
1.3 ClientHello, and a client could have its version raised between
WANT_WRITE and the retry that sent the ClientHello. In that window a
DTLS wolfSSL_set_max_proto_version() that would change the mask fails
without changing anything, since it could no longer take effect, and one
that changes nothing succeeds. A maximum of 0 on a DTLS SSL means the
newest DTLS version, so it fails there too instead of settling on the
current cap. The maximum also refuses a change after the handshake: a
lift there would make a later DTLS 1.2 renegotiation treat the server's
downgrade sentinel as an attack. wolfSSL_clear() lets it apply again,
and now resets the async handler state in builds without TLS 1.3 too,
where a stale one would leave the SSL looking started.
wolfSSL_clear_options() reports a failed suite rebuild but not
VERSION_ERROR, which the handshake start reports, as
wolfSSL_set_options() does.

wolfSSL_CTX_set_max_proto_version() raises a fixed-version DTLS method
to DTLS 1.3, as it does fixed TLS methods. A DTLS 1.3 method lowered to
DTLS 1.2 could not be restored before. A wolfDTLSv1_2_method() CTX given
a maximum of 0 now becomes DTLS 1.3-only, as a wolfTLSv1_2_method() CTX
becomes TLS 1.3-only.

wolfSSL_CTX_set_max_proto_version() with DTLS1_2_VERSION or
DTLS1_VERSION also sets WOLFSSL_OP_NO_TLSv1_3 and WOLFSSL_OP_NO_TLSv1_2,
as the TLS versions do (WOLFSSL_OP_NO_TLSv1_3 only when DTLS 1.3 is
built, at both levels, so a maximum of 0 adds no bit that means
nothing), so an SSL keeps the CTX maximum it was created with when the
CTX maximum is raised later. Before, such an SSL stayed on DTLS 1.2
while TLSv1_3_Capable() read DTLS 1.3 from the shared method, and it
rejected the server's downgrade sentinel.
wolfSSL_CTX_get_max_proto_version() returns DTLS versions for a DTLS
CTX, so its result can be passed back to the setter. It honors the CTX
minimum, and for a fixed-version DTLS method it returns that version, or
0 when the mask or the minimum excludes it. It shares the version walk
with DtlsAllowedMinor() through the new DtlsMaskAllowedMinor(), which
takes the mask directly.

wolfSSL_set_max_proto_version() refuses a TLS version on a DTLS SSL,
like the CTX setter and OpenSSL. On a fixed-version DTLS SSL it also
refuses a maximum below the SSL's version, judged by the bits the
maximum adds, so an option the application set does not make every
later maximum fail. Before, the bit was set and
the SSL kept its version, so the ClientHello went out with an empty
supported_versions list.

DtlsCheckVersionMask() takes a mode (handshake start, lift, or the
server's step-down inside DoClientHello()), so every caller goes through
the same checks, and the DTLS mask helpers move above the OPENSSL_EXTRA
block so the setters can use them. DtlsMaskVersion() no longer records
VERSION_ERROR itself, so the option setters that ignore it leave nothing
on the error queue; DoClientHello() records it where it sends the alert.

When DtlsCheckVersionMask() lifts a server SSL to DTLS 1.3, it runs
InitSSL_DtlsServer() again, since the DTLS 1.3 cookie secret and
ClientHello fragmentation are only set up for the version the SSL was
created with. Without them every DTLS 1.3 handshake of a server created
under SSL_OP_NO_DTLSv1_3 and lifted later fails with COOKIE_ERROR.
InitSSL_DtlsServer() only fills in what is missing.

Tests: a server created under a CTX SSL_OP_NO_DTLSv1_3 and lifted, with
a plain and a fragmented post-quantum ClientHello, a maximum of 0 after
a handshake and a lower one after wolfSSL_clear(), a DTLS 1.2 cap lifted
by a maximum, by a maximum of 0 and by clearing the option, a maximum
lifted by clearing the option, a DTLS 1.0 cap lifted to DTLS 1.2, a lift
under a CTX maximum lowered to DTLS 1.0 that keeps DTLS 1.2, the CTX
maximum of DTLS 1.0, fixed DTLS 1.3 and DTLS 1.2 CTXs raised to DTLS
1.3, a TLS 1.2 maximum refused on a DTLS SSL, CTX and user options that
survive later maximums, a maximum and option calls while the first
ClientHello waits on WANT_WRITE, a maximum after the ClientHello was
sent, maximums after the handshake and after wolfSSL_clear(), maximums
that repeat, lift or tighten from a server SNI callback, a maximum of 0
mid-handshake, the CTX maximum read back and set again, a lower maximum
refused and an equal one or 0 accepted on a fixed DTLS 1.3 SSL with
SSL_OP_NO_DTLSv1_3 set and on a fixed DTLS 1.2 SSL, the CTX maximum
under a DTLS 1.3 minimum with SSL_OP_NO_DTLSv1_3, the CTX maximum of a
fixed DTLS 1.3 method that masks its own version, a CTX maximum raised
after wolfSSL_new(), option and maximum calls after the CTX maximum was
lowered, the return value of wolfSSL_clear_options() when no version is
left, and the exact error for a DTLS 1.3-only client against a DTLS 1.2
server.
@Frauschi

Copy link
Copy Markdown
Member Author

Thanks, all five are addressed in a6a5205:

  1. Fixed. A higher wolfSSL_set_max_proto_version(), a maximum of 0, or a wolfSSL_clear_options() that clears a DTLS version bit now walks the mask down from ctx->method->version, so lifting a cap brings the SSL back up. The SSL keeps track of the bits its DTLS maximum added and only clears those, so an SSL_OP_NO_DTLSv1_2/_3 set by the application or the CTX stays in force. A resuming client keeps its session's version while the mask allows it.
  2. Fixed by matching TLS: DTLS1_3_VERSION and 0 now raise a fixed DTLS method to DTLS 1.3, the same way fixed TLS methods are raised. That also means a wolfDTLSv1_2_method() CTX given a maximum of 0 ends up DTLS 1.3-only, as wolfTLSv1_2_method() does with TLS 1.3. If fixed methods should keep their own version, I'd do that as a follow-up for both TLS and DTLS.
  3. Fixed. got_client_hello alone wasn't enough, since DoTls13ClientHello() clears it when it pends. DtlsMaskVersion() now skips while the first handshake is under way, which a new DtlsHandshakeStarted() works out from the connect/accept states, got_client_hello, buffered output and the async state (pending handler args included). A DTLS maximum that would change the mask fails in that window, since it could no longer take effect, and also after the handshake until wolfSSL_clear(). DoClientHello() gets its own mode, because it calls the check after got_client_hello is set to step down a ClientHello without supported_versions. I couldn't reproduce it without async. A cert setup callback that returns retry, the synchronous way to pause there, already hangs wolfSSL_accept() on master, which is a separate bug.
  4. Not intended. The SSL setter now refuses TLS versions on a DTLS SSL, like the CTX setter and OpenSSL.
  5. The test now checks that the client fails with VERSION_ERROR and stays on DTLS 1.3.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants