From ff5292c6581255c7fc1e8fe719f2fe22dbb8c095 Mon Sep 17 00:00:00 2001 From: Colton Willey Date: Thu, 2 Apr 2026 20:00:29 -0700 Subject: [PATCH] Fix error in CT compare, add negative tests for TLS pad checks --- src/wp_internal.c | 2 +- test/test_cipher.c | 162 +++++++++++++++++++++++++++++++++ test/test_tls_cbc.c | 213 ++++++++++++++++++++++++++++++++++++++++++++ test/unit.c | 8 ++ test/unit.h | 9 ++ 5 files changed, 393 insertions(+), 1 deletion(-) diff --git a/src/wp_internal.c b/src/wp_internal.c index b5bfec7c..e006eff4 100644 --- a/src/wp_internal.c +++ b/src/wp_internal.c @@ -1172,7 +1172,7 @@ byte wp_ct_byte_mask_eq(byte a, byte b) */ byte wp_ct_byte_mask_ne(byte a, byte b) { - return (((int32_t)b - a) >> 31) & (((int32_t)a - b) >> 31); + return (((int32_t)b - a) >> 31) | (((int32_t)a - b) >> 31); } /** diff --git a/test/test_cipher.c b/test/test_cipher.c index 763eddf3..2a95155a 100644 --- a/test/test_cipher.c +++ b/test/test_cipher.c @@ -457,6 +457,86 @@ int test_des3_cbc_stream(void *data) return err; } +/* + * Negative PKCS#7 padding test for DES3-CBC. + * Encrypt block-aligned plaintext (produces a full padding block), corrupt a + * ciphertext byte so the padding block is garbled, verify DecryptFinal rejects. + */ +int test_des3_cbc_bad_pad(void *data) +{ + int err = 0; + EVP_CIPHER *cipher = NULL; + EVP_CIPHER_CTX *ctx = NULL; + unsigned char key[24]; + unsigned char iv[8]; + unsigned char pt[8]; /* block-aligned: PKCS#7 adds full 8-byte pad block */ + unsigned char ct[24]; /* 8 pt + 8 pad = 16, but EncryptFinal may need room */ + unsigned char dec[24]; + int outLen = 0, fLen = 0; + + (void)data; + + PRINT_MSG("DES3-CBC negative PKCS#7 padding"); + + memset(key, 0xAA, sizeof(key)); + memset(iv, 0xBB, sizeof(iv)); + memset(pt, 0x42, sizeof(pt)); + + cipher = EVP_CIPHER_fetch(wpLibCtx, "DES-EDE3-CBC", ""); + if (cipher == NULL) { + err = 1; + } + + /* Encrypt with padding. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + err = 1; + } + if (err == 0) { + err = EVP_EncryptInit_ex(ctx, cipher, NULL, key, iv) != 1; + } + if (err == 0) { + err = EVP_EncryptUpdate(ctx, ct, &outLen, pt, (int)sizeof(pt)) != 1; + } + if (err == 0) { + err = EVP_EncryptFinal_ex(ctx, ct + outLen, &fLen) != 1; + outLen += fLen; + } + EVP_CIPHER_CTX_free(ctx); + ctx = NULL; + + /* Corrupt first ciphertext block -- CBC garbles the padding block. */ + if (err == 0) { + ct[0] ^= 0x01; + } + + /* Decrypt -- DecryptFinal should fail due to garbled padding. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + err = 1; + } + if (err == 0) { + err = EVP_DecryptInit_ex(ctx, cipher, NULL, key, iv) != 1; + } + if (err == 0) { + fLen = 0; + err = EVP_DecryptUpdate(ctx, dec, &fLen, ct, outLen) != 1; + } + if (err == 0) { + int ret = EVP_DecryptFinal_ex(ctx, dec + fLen, &fLen); + if (ret == 1) { + PRINT_ERR_MSG("DES3-CBC bad-pad: DecryptFinal should have failed"); + err = 1; + } + } + + EVP_CIPHER_CTX_free(ctx); + EVP_CIPHER_free(cipher); + return err; +} + #endif /* WP_HAVE_DES3CBC */ /******************************************************************************/ @@ -1326,4 +1406,86 @@ int test_aes256_cbc_multiple(void *data) return err; } + +/* + * Negative PKCS#7 padding test for AES-256-CBC. + * Encrypt block-aligned plaintext (produces a full padding block), corrupt a + * ciphertext byte so the padding block is garbled, verify DecryptFinal rejects. + */ +int test_aes256_cbc_bad_pad(void *data) +{ + int err = 0; + EVP_CIPHER *cipher = NULL; + EVP_CIPHER_CTX *ctx = NULL; + unsigned char key[32]; + unsigned char iv[16]; + unsigned char pt[16]; /* block-aligned: PKCS#7 adds full 16-byte pad block */ + unsigned char ct[48]; + unsigned char dec[48]; + int outLen = 0, fLen = 0; + + (void)data; + + PRINT_MSG("AES-256-CBC negative PKCS#7 padding"); + + memset(key, 0xAA, sizeof(key)); + memset(iv, 0xBB, sizeof(iv)); + memset(pt, 0x42, sizeof(pt)); + + cipher = EVP_CIPHER_fetch(wpLibCtx, "AES-256-CBC", ""); + if (cipher == NULL) { + err = 1; + } + + /* Encrypt with padding. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + err = 1; + } + if (err == 0) { + err = EVP_EncryptInit_ex(ctx, cipher, NULL, key, iv) != 1; + } + if (err == 0) { + err = EVP_EncryptUpdate(ctx, ct, &outLen, pt, (int)sizeof(pt)) != 1; + } + if (err == 0) { + err = EVP_EncryptFinal_ex(ctx, ct + outLen, &fLen) != 1; + outLen += fLen; + } + EVP_CIPHER_CTX_free(ctx); + ctx = NULL; + + /* Corrupt first ciphertext block -- CBC garbles the padding block. */ + if (err == 0) { + ct[0] ^= 0x01; + } + + /* Decrypt -- DecryptFinal should fail due to garbled padding. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) + err = 1; + } + if (err == 0) { + err = EVP_DecryptInit_ex(ctx, cipher, NULL, key, iv) != 1; + } + if (err == 0) { + fLen = 0; + err = EVP_DecryptUpdate(ctx, dec, &fLen, ct, outLen) != 1; + } + if (err == 0) { + int ret = EVP_DecryptFinal_ex(ctx, dec + fLen, &fLen); + if (ret == 1) { + PRINT_ERR_MSG("AES-256-CBC bad-pad: DecryptFinal should have " + "failed"); + err = 1; + } + } + + EVP_CIPHER_CTX_free(ctx); + EVP_CIPHER_free(cipher); + return err; +} + #endif /* WP_HAVE_AESCBC */ diff --git a/test/test_tls_cbc.c b/test/test_tls_cbc.c index 4e514951..10da08d1 100644 --- a/test/test_tls_cbc.c +++ b/test/test_tls_cbc.c @@ -237,4 +237,217 @@ int test_tls12_cbc_ossl(void *data) return err; } +/* + * AES TLS CBC negative padding test. + * + * MtE TLS with macSize > 0 always returns success from decryption -- bad + * padding causes a random MAC substitution (preventing padding oracle). + * Verify the extracted MAC does NOT match the original. + */ +static int test_aes_tls_cbc_bad_pad_helper(OSSL_LIB_CTX *libCtx, + const char *cipherName, int keyLen, int macSize) +{ + int err = 0; + EVP_CIPHER *cipher = NULL; + EVP_CIPHER_CTX *ctx = NULL; + OSSL_PARAM params[3]; + OSSL_PARAM getParams[2]; + unsigned int tlsVer = TLS1_2_VERSION; + size_t macSz = (size_t)macSize; + unsigned char key[32]; + unsigned char iv[BS]; + unsigned char mac[48]; + unsigned char buf[BS + sizeof(testPlain) + 48 + BS]; + int encLen = 0; + int decLen = 0; + int ptLen = (int)sizeof(testPlain); + unsigned char *tlsMac = NULL; + + memset(key, 0xAA, keyLen); + memset(iv, 0xBB, BS); + memset(mac, 0xCC, macSize); + + cipher = EVP_CIPHER_fetch(libCtx, cipherName, ""); + if (cipher == NULL) { + err = 1; + } + + /* Encrypt a valid TLS record. */ + if (err == 0) { + err = test_tls_cbc_enc(cipher, key, iv, testPlain, ptLen, + mac, macSize, buf, &encLen); + } + + /* CBC bit-flip: corrupt second-to-last byte in second-to-last ciphertext + * block, flipping a padding byte in the last plaintext block. */ + if (err == 0) { + int corruptOffset = encLen - BS - 2; + buf[corruptOffset] ^= 0x01; + } + + /* Decrypt -- MtE TLS returns success but substitutes a random MAC. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) { + err = 1; + } + } + if (err == 0) { + err = EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, 0) != 1; + } + if (err == 0) { + params[0] = OSSL_PARAM_construct_uint(OSSL_CIPHER_PARAM_TLS_VERSION, + &tlsVer); + params[1] = OSSL_PARAM_construct_size_t(OSSL_CIPHER_PARAM_TLS_MAC_SIZE, + &macSz); + params[2] = OSSL_PARAM_construct_end(); + err = EVP_CIPHER_CTX_set_params(ctx, params) != 1; + } + if (err == 0) { + err = EVP_CipherUpdate(ctx, buf, &decLen, buf, encLen) != 1; + } + + /* Bad padding should have triggered random MAC substitution. */ + if (err == 0) { + getParams[0] = OSSL_PARAM_construct_octet_ptr( + OSSL_CIPHER_PARAM_TLS_MAC, (void **)&tlsMac, macSize); + getParams[1] = OSSL_PARAM_construct_end(); + err = EVP_CIPHER_CTX_get_params(ctx, getParams) != 1; + } + if (err == 0) { + if (tlsMac != NULL && memcmp(tlsMac, mac, macSize) == 0) { + PRINT_ERR_MSG("TLS CBC bad-pad: MAC should have been randomized " + "but matches original (%s)", cipherName); + err = 1; + } + } + + EVP_CIPHER_CTX_free(ctx); + EVP_CIPHER_free(cipher); + return err; +} + +int test_aes_tls_cbc_bad_pad(void *data) +{ + int err = 0; + + (void)data; + + PRINT_MSG("TLS 1.2 AES-256-CBC negative padding (wolfProvider)"); + err = test_aes_tls_cbc_bad_pad_helper(wpLibCtx, "AES-256-CBC", 32, 48); + if (err == 0) { + PRINT_MSG("TLS 1.2 AES-128-CBC negative padding (wolfProvider)"); + err = test_aes_tls_cbc_bad_pad_helper(wpLibCtx, "AES-128-CBC", 16, 32); + } + + return err; +} + #endif /* WP_HAVE_AESCBC && WP_HAVE_RSA && WP_HAVE_ECDH && WP_HAVE_SHA384 */ + +#ifdef WP_HAVE_DES3CBC +#if !defined(HAVE_FIPS) || defined(WP_ALLOW_NON_FIPS) + +#define DES3_BS 8 + +/* + * DES3 TLS CBC negative padding test. + * Exercises wp_ct_byte_mask_ne in the DES3 TLS constant-time padding path. + * DES3 TLS does not use TLS_MAC_SIZE/TLS_MAC -- it only validates padding. + */ +static int test_des3_tls_cbc_bad_pad_helper(OSSL_LIB_CTX *libCtx) +{ + int err = 0; + EVP_CIPHER *cipher = NULL; + EVP_CIPHER_CTX *ctx = NULL; + OSSL_PARAM params[2]; + unsigned int tlsVer = TLS1_2_VERSION; + unsigned char key[24]; + unsigned char iv[DES3_BS]; + /* 10 bytes of plaintext. Padding: off=10%8=2, pad=8-2-1=5, padded=16. */ + unsigned char pt[10]; + unsigned char buf[64]; + int encLen = 0; + int decLen = 0; + + memset(key, 0xAA, sizeof(key)); + memset(iv, 0xBB, sizeof(iv)); + memset(pt, 0x42, sizeof(pt)); + + cipher = EVP_CIPHER_fetch(libCtx, "DES-EDE3-CBC", ""); + if (cipher == NULL) { + err = 1; + } + + /* Encrypt in TLS mode. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) { + err = 1; + } + } + if (err == 0) { + err = EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, 1) != 1; + } + if (err == 0) { + params[0] = OSSL_PARAM_construct_uint(OSSL_CIPHER_PARAM_TLS_VERSION, + &tlsVer); + params[1] = OSSL_PARAM_construct_end(); + err = EVP_CIPHER_CTX_set_params(ctx, params) != 1; + } + if (err == 0) { + /* Copy plaintext into buf; the provider pads in-place in the output. */ + memcpy(buf, pt, sizeof(pt)); + err = EVP_CipherUpdate(ctx, buf, &encLen, buf, (int)sizeof(pt)) != 1; + } + EVP_CIPHER_CTX_free(ctx); + ctx = NULL; + + /* CBC bit-flip: corrupt a padding byte in the last plaintext block + * without touching the pad-length byte at the final position. */ + if (err == 0) { + buf[encLen - DES3_BS - 2] ^= 0x01; + } + + /* Decrypt -- should fail due to bad padding. */ + if (err == 0) { + ctx = EVP_CIPHER_CTX_new(); + if (ctx == NULL) { + err = 1; + } + } + if (err == 0) { + err = EVP_CipherInit_ex(ctx, cipher, NULL, key, iv, 0) != 1; + } + if (err == 0) { + params[0] = OSSL_PARAM_construct_uint(OSSL_CIPHER_PARAM_TLS_VERSION, + &tlsVer); + params[1] = OSSL_PARAM_construct_end(); + err = EVP_CIPHER_CTX_set_params(ctx, params) != 1; + } + if (err == 0) { + int ret = EVP_CipherUpdate(ctx, buf, &decLen, buf, encLen); + if (ret == 1) { + PRINT_ERR_MSG("DES3 TLS CBC bad-pad: decryption should have failed " + "but succeeded"); + err = 1; + } + } + + EVP_CIPHER_CTX_free(ctx); + EVP_CIPHER_free(cipher); + return err; +} + +int test_des3_tls_cbc_bad_pad(void *data) +{ + (void)data; + + PRINT_MSG("DES3 TLS 1.2 CBC negative padding (wolfProvider)"); + return test_des3_tls_cbc_bad_pad_helper(wpLibCtx); +} + +#undef DES3_BS + +#endif /* !HAVE_FIPS || WP_ALLOW_NON_FIPS */ +#endif /* WP_HAVE_DES3CBC */ diff --git a/test/unit.c b/test/unit.c index 68fb7cab..47be9f3d 100644 --- a/test/unit.c +++ b/test/unit.c @@ -227,6 +227,7 @@ TEST_CASE test_case[] = { #if !defined(HAVE_FIPS) || defined(WP_ALLOW_NON_FIPS) TEST_DECL(test_des3_cbc, NULL), TEST_DECL(test_des3_cbc_stream, NULL), + TEST_DECL(test_des3_cbc_bad_pad, NULL), #endif #endif #ifdef WP_HAVE_AESECB @@ -245,6 +246,7 @@ TEST_CASE test_case[] = { TEST_DECL(test_aes192_cbc_stream, NULL), TEST_DECL(test_aes256_cbc_stream, NULL), TEST_DECL(test_aes256_cbc_multiple, NULL), + TEST_DECL(test_aes256_cbc_bad_pad, NULL), #endif #ifdef WP_HAVE_AESCTR TEST_DECL(test_aes128_ctr_stream, NULL), @@ -446,6 +448,12 @@ TEST_CASE test_case[] = { defined(WP_HAVE_ECDH) && defined(WP_HAVE_SHA384) TEST_DECL(test_tls12_cbc_ossl, NULL), TEST_DECL(test_tls12_cbc, NULL), + TEST_DECL(test_aes_tls_cbc_bad_pad, NULL), +#endif +#ifdef WP_HAVE_DES3CBC + #if !defined(HAVE_FIPS) || defined(WP_ALLOW_NON_FIPS) + TEST_DECL(test_des3_tls_cbc_bad_pad, NULL), + #endif #endif }; #define TEST_CASE_CNT (int)(sizeof(test_case) / sizeof(*test_case)) diff --git a/test/unit.h b/test/unit.h index 25103463..a31554d4 100644 --- a/test/unit.h +++ b/test/unit.h @@ -147,6 +147,7 @@ int test_krb5kdf(void *data); #ifdef WP_HAVE_DES3CBC int test_des3_cbc(void *data); int test_des3_cbc_stream(void *data); +int test_des3_cbc_bad_pad(void *data); #endif #ifdef WP_HAVE_AESECB @@ -169,6 +170,7 @@ int test_aes128_cbc_stream(void *data); int test_aes192_cbc_stream(void *data); int test_aes256_cbc_stream(void *data); int test_aes256_cbc_multiple(void *data); +int test_aes256_cbc_bad_pad(void *data); #endif @@ -437,6 +439,13 @@ int test_x509_cert(void *data); defined(WP_HAVE_ECDH) && defined(WP_HAVE_SHA384) int test_tls12_cbc(void *data); int test_tls12_cbc_ossl(void *data); +int test_aes_tls_cbc_bad_pad(void *data); +#endif + +#ifdef WP_HAVE_DES3CBC +#if !defined(HAVE_FIPS) || defined(WP_ALLOW_NON_FIPS) +int test_des3_tls_cbc_bad_pad(void *data); +#endif #endif #endif /* UNIT_H */