Enforce RFC 5746 renegotiation_info in TLS 1.2 client by default#10984
Enforce RFC 5746 renegotiation_info in TLS 1.2 client by default#10984embhorn wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes TLS 1.2 client builds enforce RFC 5746 / RFC 9325 secure renegotiation signaling by default: the client advertises an empty renegotiation_info extension and aborts the initial handshake with a fatal handshake_failure if the server omits the extension, while providing compile-time and runtime opt-outs.
Changes:
- Enable the RFC 5746 renegotiation_info initial-handshake check by default (when
HAVE_SERVER_RENEGOTIATION_INFOis available), and send a fatalhandshake_failurewhen the ServerHello omits the extension. - Add context-level SCR check controls (
wolfSSL_CTX_get/set_scr_check_enabled) and wire context → object inheritance for the setting. - Add/adjust tests around default behavior, opt-outs, and “advertise-only” refusal of server-initiated renegotiation; add a
--enable-legacy-server-connectconfigure option.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
wolfssl/ssl.h |
Exposes new context-level SCR check enable/disable APIs and updates build guards for SCR check APIs. |
wolfssl/internal.h |
Adds advertiseOnly state to secure renegotiation tracking and adds ctx/object SCR enforcement bitfields. |
src/internal.c |
Implements advertise-only behavior, defaults/inheritance for SCR enforcement, and sends fatal alert on missing ServerHello extension. |
src/ssl_api_ext.c |
Enforces advertise-only refusal in _Rehandshake() and adds ctx-level SCR check accessors. |
src/ssl.c |
Ensures secure renegotiation advertising state is rebuilt appropriately when reusing an object after wolfSSL_clear(). |
src/tls13.c |
Re-establishes renegotiation_info advertising on TLS 1.3 ClientHello send when needed (for downgrade paths / reuse). |
tests/api.c |
Expands SCR enforcement tests (default-on, strip extension failure, ctx/object toggles) and adds alert-history assertions. |
tests/api/test_tls_ext.h |
Adds new test prototype for advertise-only HelloRequest refusal. |
tests/api/test_tls_ext.c |
Adds new test ensuring advertise-only clients refuse server HelloRequest renegotiation and continue application data. |
configure.ac |
Adds --enable-legacy-server-connect and rejects it when combined with --enable-harden-tls. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (test_scr_strip_serverhello && sz > 44 && | ||
| (byte)buf[0] == 0x16 && (byte)buf[5] == 0x02) { | ||
| /* 5 record hdr + 4 handshake hdr + 2 version + 32 random */ | ||
| off = 5 + 4 + 2 + 32; | ||
| sidLen = (byte)buf[off]; |
|
|
||
| WOLFSSL_LOCAL int SetupClientSecureRenegotiation(WOLFSSL* ssl); | ||
|
|
||
| #endif /* HAVE_SECURE_RENEGOTIATION */ |
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #10984
Scan targets checked: wolfcrypt-rs-bugs, wolfssl-bugs, wolfssl-src
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
| ctx_s = NULL; | ||
| test_scr_strip_serverhello = 0; | ||
|
|
||
| /* Phase 3: the enforcement setting can be toggled per object and per |
There was a problem hiding this comment.
🔵 [Low] Disable path of SCR check untested at connection level · Missing edge-case coverage on a function the PR also changed
Phase 3 only exercises the getter/setter API (wolfSSL_CTX_set_scr_check_enabled(ctx, 0) / wolfSSL_set_scr_check_enabled(ssl, 0)), verifying the flag is stored and inherited — but never performs a handshake with scr_check_enabled == 0 against a server that omits the renegotiation_info extension to confirm the opt-out path actually allows the connection.
Fix: Add a Phase 4 that disables the check on client and repeats the Phase 2 stripping scenario, asserting the handshake now succeeds.
|
Description
A default-build TLS 1.2 client previously neither advertised the renegotiation_info extension nor verified that the server echoed it. The RFC 5746 4.1 / RFC 9325 check already existed but was compiled only under WOLFSSL_HARDEN_TLS.
In this PR, we make the client advertise the empty renegotiation_info extension and abort with a fatal handshake_failure alert when the initial ServerHello omits it, in any build with HAVE_SERVER_RENEGOTIATION_INFO (on by default). Provide opt-outs for legacy peers: the compile-time WOLFSSL_ALLOW_LEGACY_SERVER_CONNECT (--enable-legacy-server-connect), the
existing runtime wolfSSL_set_scr_check_enabled(), and a new wolfSSL_CTX_set_scr_check_enabled(). Reject --enable-legacy-server-connect together with --enable-harden-tls, since that combination violates RFC 9325 3.5.
Fixes zd22162
This is a draft message for the changelog:
Testing
Add unit tests for the default advertise and enforcement path, the runtime and context opt-outs, and the advertise-only HelloRequest refusal.
Checklist