Fix: Do not build RC4 cipher suites in D/TLS 1.3 capable configurations - #11277
Fix: Do not build RC4 cipher suites in D/TLS 1.3 capable configurations#11277aidankeefe2022 wants to merge 1 commit into
Conversation
RFC 8446 Appendix D.5 requires that a TLS 1.3 capable implementation neither offer nor negotiate RC4 cipher suites in any protocol version. Building with --enable-arc4 (WOLFSSL_ALLOW_RC4) left the RC4 suites in the built-in suite list even when D/TLS 1.3 was enabled, so a TLS 1.3 capable client offered TLS_ECDHE_ECDSA_WITH_RC4_128_SHA and TLS_ECDHE_RSA_WITH_RC4_128_SHA in its ClientHello, and a TLS 1.3 capable server negotiated RC4 with a TLS 1.2 peer. Gate the RC4 suite build macros on a new internal WSSL_NO_RC4_SUITES, defined when WOLFSSL_TLS13 or WOLFSSL_DTLS13 is set. RC4 itself is untouched: NO_RC4 is not defined, so wolfCrypt keeps RC4 available for non-TLS uses such as PKCS#12 and Kerberos. Builds with D/TLS 1.3 disabled keep the RC4 suites and are unaffected. Add test_tls_no_rc4_suites to check that no RC4 suite is selectable by name and that none appears in the built-in suite list.
|
Can one of the admins verify this patch? |
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #11277
Scan targets checked: wolfcrypt-bugs, wolfcrypt-rs-bugs, wolfcrypt-src, wolfssl-bugs, wolfssl-src
Findings: 5
5 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
| #endif | ||
|
|
||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) | ||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) && \ |
There was a problem hiding this comment.
Sniffer fails to compile when RC4 is enabled with D/TLS 1.3 · Logic errors
BUILD_ARC4 is now suppressed while NO_RC4 stays undefined, so the Ciphers.arc4 member (internal.h:4827) is absent. src/sniffer.c:4921 dereferences ssl->decrypt.arc4 under #ifndef NO_RC4, so --enable-sniffer --enable-arc4 (also implied by --enable-openssh/--enable-wpas) no longer builds.
Fix: Change the #ifndef NO_RC4 guard around the wolfssl_rc4 case in DecryptDo() in src/sniffer.c to #ifdef BUILD_ARC4.
|
|
||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) | ||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) && \ | ||
| !defined(WSSL_NO_RC4_SUITES) |
There was a problem hiding this comment.
Sniffer fails to compile: BUILD_ARC4 now off while sniffer.c still uses ssl-decrypt.arc4 · Incorrect feature flag gating
Gating BUILD_ARC4 on WSSL_NO_RC4_SUITES removes the arc4 member from struct Ciphers (internal.h:4827), but src/sniffer.c:4921 dereferences ssl->decrypt.arc4 under #ifndef NO_RC4. Any --enable-arc4 --enable-sniffer build (TLS 1.3 on by default) no longer compiles.
Related known finding #7643 (similar but distinct): Both affect src/sniffer.c's DecryptDo, but the candidate dereferences a conditionally removed ARC4 struct member due to inconsistent feature gating, while #7643 reads an AEAD nonce before validating record length. Their root causes and faulting operations differ, and each requires a separate patch.
Fix: Change the guard at src/sniffer.c:4919 from #ifndef NO_RC4 to #ifdef BUILD_ARC4, matching src/keys.c and src/internal.c.
|
|
||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) | ||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) && \ | ||
| !defined(WSSL_NO_RC4_SUITES) |
There was a problem hiding this comment.
Undefining BUILD_ARC4 for TLS 1.3 builds breaks sniffer compilation · Preprocessor-conditional security bypass
BUILD_ARC4 now stays undefined whenever WOLFSSL_TLS13/WOLFSSL_DTLS13 is set, so Ciphers (internal.h:4827) loses its arc4 member, but src/sniffer.c:4921 still dereferences ssl->decrypt.arc4 under a #ifndef NO_RC4 guard. --enable-sniffer --enable-arc4 with TLS 1.3 on fails to compile.
Fix: Change the #ifndef NO_RC4 guard around the wolfssl_rc4 case in DecryptDo() in src/sniffer.c to #ifdef BUILD_ARC4.
| #endif | ||
|
|
||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) | ||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) && \ |
There was a problem hiding this comment.
Gating BUILD_ARC4 off breaks the sniffer build with --enable-arc4 · Conditional compilation / build break
BUILD_ARC4 is now undefined whenever D/TLS 1.3 is enabled, but Ciphers.arc4 (internal.h:4827) exists only under BUILD_ARC4 while src/sniffer.c:4921 still dereferences ssl->decrypt.arc4 under a plain #ifndef NO_RC4. --enable-sniffer --enable-arc4 no longer compiles.
Fix: Change the #ifndef NO_RC4 guard around the wolfssl_rc4 case in DecryptDo() in src/sniffer.c to #ifdef BUILD_ARC4.
|
|
||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) | ||
| #if !defined(NO_RC4) && !defined(WSSL_HARDEN_TLS) && \ | ||
| !defined(WSSL_NO_RC4_SUITES) |
There was a problem hiding this comment.
BUILD_ARC4 removal breaks sniffer compilation with --enable-arc4 · Preprocessor-conditional security bypass
WSSL_NO_RC4_SUITES now suppresses BUILD_ARC4, which gates the arc4 member of struct Ciphers (internal.h:4827). src/sniffer.c:4921 dereferences ssl->decrypt.arc4 under #ifndef NO_RC4, which is still false with WOLFSSL_ALLOW_RC4. --enable-sniffer --enable-arc4 (TLS 1.3 on by default) fails to compile.
Fix: Change the #ifndef NO_RC4 guard around the wolfssl_rc4 case in sniffer.c DecryptDo() to #ifdef BUILD_ARC4.
1 similar comment
3 participants
Description
Reported by #11081
RFC 8446 Appendix D.5 requires that a TLS 1.3 capable implementation neither offer nor negotiate RC4 cipher suites in any protocol version. Building with --enable-arc4 (WOLFSSL_ALLOW_RC4) left the RC4 suites in the built-in suite list even when D/TLS 1.3 was enabled, so a TLS 1.3 capable client offered TLS_ECDHE_ECDSA_WITH_RC4_128_SHA and TLS_ECDHE_RSA_WITH_RC4_128_SHA in its ClientHello, and a TLS 1.3 capable server negotiated RC4 with a TLS 1.2 peer.
Gate the RC4 suite build macros on a new internal WSSL_NO_RC4_SUITES, defined when WOLFSSL_TLS13 or WOLFSSL_DTLS13 is set. RC4 itself is untouched: NO_RC4 is not defined, so wolfCrypt keeps RC4 available for non-TLS uses such as PKCS#12 and Kerberos. Builds with D/TLS 1.3 disabled keep the RC4 suites and are unaffected.
Testing
Add test_tls_no_rc4_suites to check that no RC4 suite is selectable by name and that none appears in the built-in suite list.
Checklist