Uh oh!
There was an error while loading. Please reload this page.
Handle TLS CBC bad padding in DES3 without a padding oracle - #448
Handle TLS CBC bad padding in DES3 without a padding oracle#448yosuke-wolfssl wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR hardens DES-EDE3-CBC TLS 1.2 CBC record decryption in wolfProvider to avoid a Lucky13-style padding oracle by adding constant-time padding validation and (for MAC-then-Encrypt) constant-time MAC extraction with randomized-MAC substitution on bad padding, aligning behavior with the existing AES TLS-CBC implementation. It also adds DES3 TLS-CBC unit tests to cover MtE vs ETM/no-MAC semantics and split-record update paths.
Changes:
- Add constant-time TLS 1.2 CBC decrypt post-processing for DES3, including MAC extraction/randomization for MtE and strict padding failure for ETM/no-MAC.
- Plumb TLS cipher parameters for DES3 (
TLS_VERSION,TLS_MAC_SIZE,TLS_MAC) and ensure context duplication/freeing handles TLS MAC state safely. - Add multiple DES3 TLS-CBC tests (valid/invalid padding, MtE/ETM, split updates) and register them in the unit test suite.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/wp_des.c | Adds DES3 TLS-CBC constant-time padding/MAC handling and TLS param plumbing (TLS_MAC_SIZE/TLS_MAC). |
test/test_tls_cbc.c | Adds DES3 TLS-CBC test coverage for MtE/ETM modes, bad padding behavior, and split-update paths. |
test/unit.c | Registers the new DES3 TLS-CBC tests in the test suite. |
test/unit.h | Declares the new DES3 TLS-CBC test functions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #448
Scan targets checked:wolfprovider-bugs, wolfprovider-src
Findings: 2
2 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #448
Scan targets checked:wolfprovider-bugs, wolfprovider-src
Findings: 3
3 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Findings are non-blocking.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Handle TLS CBC bad padding in DES3 without a padding oracle
Summary
DES-EDE3-CBCin wolfProvider had no TLS 1.2 CBC record post-processing, sodecryption of a TLS record leaked whether padding was valid before the MAC was
checked — a Lucky13-style padding oracle. This adds constant-time padding
validation and MAC extraction to the DES3 TLS decrypt path, mirroring the
existing
wp_aes_blockimplementation, and wires up the TLS cipher parametersthe record layer relies on.
Addresses F-517.
Changes
src/wp_des.cwp_des3_block_tls_dec_record()— constant-time TLS 1.2 CBC decryptpost-processing over the record
[explicit_IV][payload][MAC][padding]:macSize > 0): constant-time padding validationand MAC extraction using the rotation pattern from OpenSSL's
tls1_cbc_remove_padding_and_mac()/ssl3_cbc_copy_mac(). On bad paddingit substitutes a random MAC (via constant-time select) rather than
failing early, so an attacker cannot distinguish bad-padding from bad-MAC —
no oracle.
macSize == 0): strips the explicit IV andvalidates/removes padding; bad padding is a hard failure (the record layer
already verified the MAC, so there is no oracle concern).
tlsmac,tlsmacsize,tlsmacAlloced), freed infreectxand deep-copied indupctxto avoid adouble-free between contexts.
OSSL_CIPHER_PARAM_TLS_VERSIONandOSSL_CIPHER_PARAM_TLS_MAC_SIZEon set,and expose
OSSL_CIPHER_PARAM_TLS_MACon get.test/test_tls_cbc.c,test/unit.c,test/unit.h— new DES3 coverage:test_des3_tls_cbc_bad_pad— MtE bad padding returns success with arandomized MAC (oracle defense).
test_des3_tls_cbc_mte— MtE valid record: recovered length, plaintext, andextracted MAC.
test_des3_tls_cbc_etm— ETM/no-MAC valid record: recovered length andcontent.
test_des3_tls_cbc_etm_bad_pad— ETM bad padding must fail (notsubstitute a MAC), covering the differing MtE/ETM semantics.
test_des3_tls_cbc_dec/test_des3_tls_cbc_mte_split— split-record decryptacross two
EVP_CipherUpdatecalls to exercise the buffered-block path; theMtE split test asserts
l1 == 0before the completing update so theexact-sized output buffer can't overrun.
Testing
test/unit.testsuite passes (TESTSUITE SUCCESS), including all newDES3 TLS-CBC cases.
/ simulated failure) to confirm it is non-vacuous.