Skip to content

Add separate client cookie enable config - #11101

Open
padelsbach wants to merge 2 commits into
wolfSSL:masterfrom
padelsbach:client-cookie-enable
Open

Add separate client cookie enable config#11101
padelsbach wants to merge 2 commits into
wolfSSL:masterfrom
padelsbach:client-cookie-enable

Conversation

@padelsbach

Copy link
Copy Markdown
Contributor

Description

Split the existing flag which enables TLS cookie support WOLFSSL_SEND_HRR_COOKIE into a smaller portion WOLFSSL_TLS13_COOKIE which enables replying with a cookie echo'ed from the HelloClientResponse (HRR). This is enabled by default for clients.

Fixes #11074

Testing

New unit test

Checklist

  • added tests
  • updated/added doxygen
  • updated appropriate READMEs
  • Updated manual and documentation

@padelsbach padelsbach changed the title Add separate client cookie enable Add separate client cookie enable config Aug 6, 2026

@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 #11101

Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src

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

This review was generated automatically by Fenrir. Findings are non-blocking.

Comment thread tests/api/test_tls13.c Outdated
Comment thread src/tls.c
Comment thread wolfssl/internal.h
Comment thread tests/api/test_tls13.c
@github-actions

github-actions Bot commented Aug 7, 2026

Copy link
Copy Markdown

MemBrowse Memory Report

gcc-arm-cortex-m3

  • FLASH: .text +44 B (+0.0%, 122,987 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4

  • FLASH: .text +64 B (+0.0%, 201,303 B / 262,144 B, total: 77% used)

gcc-arm-cortex-m4-dtls13

  • FLASH: .text +64 B (+0.0%, 181,988 B / 1,048,576 B, total: 17% used)

gcc-arm-cortex-m4-openssl-compat

  • FLASH: .rodata +256 B, .text +640 B (+0.1%, 774,380 B / 1,048,576 B, total: 74% used)

gcc-arm-cortex-m4-pq

  • FLASH: .text +384 B (+0.1%, 281,664 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

  • FLASH: .text +320 B (+0.1%, 327,712 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m4-tls13

  • FLASH: .text +384 B (+0.2%, 237,769 B / 262,144 B, total: 91% used)

gcc-arm-cortex-m7-pq

  • FLASH: .text +384 B (+0.1%, 282,240 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

  • FLASH: .text +384 B (+0.2%, 237,833 B / 262,144 B, total: 91% used)

linuxkm-standard

@dgarske

dgarske commented Aug 7, 2026

Copy link
Copy Markdown
Member

Jenkins retest this please

Comment thread src/tls.c Outdated
#ifdef WOLFSSL_SEND_HRR_COOKIE
/* client_hello - only a server that sends cookies has one to check the
* echoed cookie against. Otherwise ignore it. */
{

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.

Please avoid empty brace.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

removed

Comment thread src/tls.c

if (msgType == hello_retry_request) {
ssl->options.hrrSentCookie = 1;
return TLSX_Cookie_Use(ssl, input + idx, len, NULL, 0, 1,

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.

Should we add a len sanity check here?

Enabling the client path by default means every wolfSSL TLS 1.3 client now accepts a cookie of any length permitted by the extension encoding (up to 65535 bytes, opaque cookie<1..2^16-1>) and heap-allocates it in TLSX_Cookie_Use (src/tls.c:7676: XMALLOC(sizeof(Cookie) + len + macSz, ...)), then copies it verbatim into the second ClientHello. HelloRetryRequest is unauthenticated, so any on-path attacker can inject an HRR with a maximal cookie; previously (default build) the extension was compiled out and the blob was discarded. The resulting failure is graceful — TLSX_GetRequestSize rejects totals above WOLFSSL_MAX_16BIT - OPAQUE16_LEN with BUFFER_E (src/tls.c:7614-7617 region), and CheckAvailableSize fails if the output buffer cannot grow — so this is new attack surface and transient memory pressure rather than a memory-safety defect. It is still a change worth bounding: wolfSSL sanity-bounds most other peer-supplied lengths, and the server side already has MAX_COOKIE_LEN.

Recommendation: Consider a sanity bound on the accepted HRR cookie length now that this path is on by default, or state explicitly in a comment that unbounded cookies are accepted deliberately per RFC 8446 4.2.2.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Done, defaults to 4096

Comment thread src/tls.c
return SANITY_MSG_E;
}

/* Message contains length and Cookie which must be at least one byte

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.

Because WOLFSSL_TLS13_COOKIE is derived from !NO_WOLFSSL_CLIENT, a combined client+server library built WITHOUT --enable-hrrcookie (the default) now compiles TLSX_Cookie_Parse, so a SERVER runs the length validation for a cookie extension in an incoming ClientHello even though the body of the checking logic is #ifdef WOLFSSL_SEND_HRR_COOKIE-ed out. Previously CKE_PARSE(a,b,c,d) expanded to 0 and the extension was ignored wholesale. Now a ClientHello carrying an empty cookie (00 00, i.e. length == 2 < OPAQUE16_LEN + 1) or an inconsistent inner length aborts the handshake with BUFFER_E on a server that has no interest in cookies at all. This is arguably more correct (an empty cookie violates opaque cookie<1..2^16-1>), and it is probably intentional, but it is an unannounced behavior change for the default server configuration and there is no test for it. The added comment ("only a server that sends cookies has one to check the echoed cookie against. Otherwise ignore it.") says "ignore it", which does not match the fact that length errors are still fatal.

Recommendation: Confirm this is the intended default-server behavior, fix the comment to say the encoding is still validated, and add a server-side test (no WOLFSSL_SEND_HRR_COOKIE) that a ClientHello with a stray cookie extension is ignored while a malformed one is rejected.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

intended behavior, updated comment

@padelsbach padelsbach removed their assignment Aug 7, 2026
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.

TLS 1.3 cookie extension is absent in the audited default build and only available behind build/runtime gates

4 participants