diff --git a/doc/dox_comments/header_files/des3.h b/doc/dox_comments/header_files/des3.h index 89d7b5cc31..35b64ab67c 100644 --- a/doc/dox_comments/header_files/des3.h +++ b/doc/dox_comments/header_files/des3.h @@ -171,8 +171,11 @@ int wc_Des_EcbEncrypt(Des* des, byte* out, directly whenever possible. \return 0 Returned upon successfully encrypting the given plaintext + \return BAD_FUNC_ARG If des, out, or in is NULL. + \return MISSING_KEY If no key has been set on the Des3 structure with + wc_Des3_SetKey. - \param des3 pointer to the Des3 structure to use for encryption + \param des pointer to the Des3 structure to use for encryption \param out pointer to the buffer in which to store the encrypted message \param in pointer to the input buffer containing the plaintext to encrypt \param sz length of the plaintext to encrypt @@ -381,6 +384,9 @@ int wc_Des_EcbDecrypt(Des* des, byte* out, const byte* in, word32 sz); avoid using ECB APIs directly whenever possible. \return 0 On successfully decrypting the given ciphertext + \return BAD_FUNC_ARG If des, out, or in is NULL. + \return MISSING_KEY If no key has been set on the Des3 structure with + wc_Des3_SetKey. \param des pointer to the Des3 structure to use for decryption \param out pointer to the buffer in which to store the decrypted diff --git a/tests/api/test_des3.c b/tests/api/test_des3.c index 81faff205e..c524854d20 100644 --- a/tests/api/test_des3.c +++ b/tests/api/test_des3.c @@ -182,10 +182,11 @@ int test_wc_Des3_CbcEncryptDecrypt(void) } /* END wc_Des3_CbcEncrypt */ /* - * Regression test for issue 5379: wc_Des3_CbcEncrypt/Decrypt must refuse to - * run unless a key has been configured with wc_Des3_SetKey(), both before - * SetKey and after Free. Otherwise the operation would silently run with - * uninitialized or zeroed key material and return success. + * Regression test for issue 5379: the wc_Des3_Cbc* and wc_Des3_Ecb* entry + * points must refuse to run unless a key has been configured with + * wc_Des3_SetKey(), both before SetKey and after Free. Otherwise the operation + * would silently run with uninitialized or zeroed key material and return + * success. * * FIPS builds use the FIPS-certified DES3 implementation which does not track * key state, so skip the test for FIPS. @@ -223,6 +224,18 @@ int test_wc_Des3_CbcEncryptDecrypt_no_key(void) WC_NO_ERR_TRACE(MISSING_KEY)); ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, vector, 24), WC_NO_ERR_TRACE(MISSING_KEY)); +#ifdef WOLFSSL_DES_ECB + /* wc_Des3_EcbDecrypt is a separate function only on FREESCALE_MMCAU; + * elsewhere des3.h aliases it to wc_Des3_EcbEncrypt. Both names are + * asserted so the alias and the split implementation are each covered. */ + ExpectIntEQ(wc_Des3_EcbEncrypt(&des, cipher, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + ExpectIntEQ(wc_Des3_EcbDecrypt(&des, plain, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + /* A zero-length request must be rejected too, not treated as a no-op. */ + ExpectIntEQ(wc_Des3_EcbEncrypt(&des, cipher, vector, 0), + WC_NO_ERR_TRACE(MISSING_KEY)); +#endif /* After a key is set, the operations succeed. */ ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_ENCRYPTION), 0); @@ -230,6 +243,17 @@ int test_wc_Des3_CbcEncryptDecrypt_no_key(void) ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_DECRYPTION), 0); ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, cipher, 24), 0); ExpectIntEQ(XMEMCMP(plain, vector, 24), 0); +#ifdef WOLFSSL_DES_ECB + /* ECB round trip on a keyed context. Done after the CBC comparison above + * so its ciphertext is not overwritten, and carried through to plaintext + * so a regression in the transform fails here too, not just one in the + * keySet gate. */ + ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_ENCRYPTION), 0); + ExpectIntEQ(wc_Des3_EcbEncrypt(&des, cipher, vector, 24), 0); + ExpectIntEQ(wc_Des3_SetKey(&des, key, iv, DES_DECRYPTION), 0); + ExpectIntEQ(wc_Des3_EcbDecrypt(&des, plain, cipher, 24), 0); + ExpectBufEQ(plain, vector, 24); +#endif /* After free, the keyed state is cleared and operations must fail again. */ wc_Des3Free(&des); @@ -237,6 +261,12 @@ int test_wc_Des3_CbcEncryptDecrypt_no_key(void) WC_NO_ERR_TRACE(MISSING_KEY)); ExpectIntEQ(wc_Des3_CbcDecrypt(&des, plain, vector, 24), WC_NO_ERR_TRACE(MISSING_KEY)); +#ifdef WOLFSSL_DES_ECB + ExpectIntEQ(wc_Des3_EcbEncrypt(&des, cipher, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); + ExpectIntEQ(wc_Des3_EcbDecrypt(&des, plain, vector, 24), + WC_NO_ERR_TRACE(MISSING_KEY)); +#endif #endif return EXPECT_RESULT(); diff --git a/tests/api/test_evp_cipher.c b/tests/api/test_evp_cipher.c index a6643e9ad1..d347fad263 100644 --- a/tests/api/test_evp_cipher.c +++ b/tests/api/test_evp_cipher.c @@ -2769,6 +2769,55 @@ int test_wolfSSL_EVP_mdc2(void) return EXPECT_RESULT(); } +/* + * The 3DES ECB entry points reject a context that has no key, and that is + * visible here: wolfSSL_EVP_CipherInit only calls wc_Des3_SetKey when a key is + * supplied, so initializing EVP_des_ede3_ecb() with a NULL key and then + * feeding it data used to run against the zeroed key schedule and report + * success. It must fail instead. The ordinary two-stage OpenSSL idiom -- + * install the cipher, then install the key -- does key the context and must + * keep working. + * + * FIPS builds use the FIPS-certified DES3 implementation, which does not track + * key state, so skip the test for FIPS. + */ +int test_wolfSSL_EVP_des_ede3_ecb_no_key(void) +{ + EXPECT_DECLS; +#if !defined(NO_DES3) && !defined(HAVE_FIPS) && defined(OPENSSL_EXTRA) && \ + defined(WOLFSSL_DES_ECB) + EVP_CIPHER_CTX* ctx = NULL; + byte out[32]; + int outl = 0; + const byte key[24] = { + 0x01,0x23,0x45,0x67,0x89,0xab,0xcd,0xef, + 0xfe,0xde,0xba,0x98,0x76,0x54,0x32,0x10, + 0x89,0xab,0xcd,0xef,0x01,0x23,0x45,0x67 + }; + const byte in[16] = { + 0x4e,0x6f,0x77,0x20,0x69,0x73,0x20,0x74, + 0x68,0x65,0x20,0x74,0x69,0x6d,0x65,0x20 + }; + + XMEMSET(out, 0, sizeof(out)); + + /* No key ever supplied: the update must not produce ciphertext. */ + ExpectNotNull(ctx = EVP_CIPHER_CTX_new()); + ExpectIntEQ(EVP_CipherInit(ctx, EVP_des_ede3_ecb(), NULL, NULL, 1), 1); + ExpectIntNE(EVP_CipherUpdate(ctx, out, &outl, in, (int)sizeof(in)), 1); + EVP_CIPHER_CTX_free(ctx); + ctx = NULL; + + /* Cipher first, key second: unaffected. */ + ExpectNotNull(ctx = EVP_CIPHER_CTX_new()); + ExpectIntEQ(EVP_CipherInit(ctx, EVP_des_ede3_ecb(), NULL, NULL, 1), 1); + ExpectIntEQ(EVP_CipherInit(ctx, NULL, key, NULL, 1), 1); + ExpectIntEQ(EVP_CipherUpdate(ctx, out, &outl, in, (int)sizeof(in)), 1); + EVP_CIPHER_CTX_free(ctx); +#endif + return EXPECT_RESULT(); +} + /* Test for integer overflow in EVP AEAD AAD accumulation. * * wolfSSL_EVP_CipherUpdate_GCM_AAD (and the CCM/ARIA variants) compute diff --git a/tests/api/test_evp_cipher.h b/tests/api/test_evp_cipher.h index c4dccf839b..659fa5abf5 100644 --- a/tests/api/test_evp_cipher.h +++ b/tests/api/test_evp_cipher.h @@ -63,6 +63,7 @@ int test_wolfSSL_EVP_rc4(void); int test_wolfSSL_EVP_enc_null(void); int test_wolfSSL_EVP_rc2_cbc(void); int test_wolfSSL_EVP_mdc2(void); +int test_wolfSSL_EVP_des_ede3_ecb_no_key(void); int test_evp_cipher_pkcs7_pad_zero(void); int test_evp_cipher_aead_aad_overflow(void); @@ -106,6 +107,7 @@ int test_evp_cipher_aead_aad_overflow(void); TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_enc_null), \ TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_rc2_cbc), \ TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_mdc2), \ + TEST_DECL_GROUP("evp_cipher", test_wolfSSL_EVP_des_ede3_ecb_no_key), \ TEST_DECL_GROUP("evp_cipher", test_evp_cipher_pkcs7_pad_zero), \ TEST_DECL_GROUP("evp_cipher", test_evp_cipher_aead_aad_overflow) diff --git a/wolfcrypt/src/des3.c b/wolfcrypt/src/des3.c index 013c060610..f816a5fe96 100644 --- a/wolfcrypt/src/des3.c +++ b/wolfcrypt/src/des3.c @@ -859,6 +859,8 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) for (i = 0; i < 8; i++) dkey3[i] = ((dkey3[i] & 0xFE) | parityLookup[dkey3[i] >> 1]); + des->keySet = 1; + return ret; } @@ -1139,6 +1141,13 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) byte temp_block[DES_BLOCK_SIZE]; + if (des == NULL || out == NULL || in == NULL) { + return BAD_FUNC_ARG; + } + + if (!des->keySet) { + return MISSING_KEY; + } #ifdef FREESCALE_MMCAU_CLASSIC if ((wc_ptr_t)out % WOLFSSL_MMCAU_ALIGNMENT) { @@ -1182,6 +1191,14 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) byte temp_block[DES_BLOCK_SIZE]; + if (des == NULL || out == NULL || in == NULL) { + return BAD_FUNC_ARG; + } + + if (!des->keySet) { + return MISSING_KEY; + } + #ifdef FREESCALE_MMCAU_CLASSIC if ((wc_ptr_t)out % WOLFSSL_MMCAU_ALIGNMENT) { WOLFSSL_MSG("Bad 3ede cau_des_decrypt alignment"); @@ -1241,6 +1258,8 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) XMEMCPY(des->key[0], key, DES3_KEYLEN); XMEMCPY(des->reg, iv, DES3_IVLEN); + des->keySet = 1; + return 0; } @@ -1312,6 +1331,9 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) if (des == NULL || out == NULL || in == NULL) return BAD_FUNC_ARG; + if (!des->keySet) + return MISSING_KEY; + return wc_Pic32DesCrypt(des->key[0], DES3_KEYLEN, des->reg, DES3_IVLEN, out, in, (blocks * DES_BLOCK_SIZE), PIC32_ENCRYPTION, PIC32_ALGO_TDES, PIC32_CRYPTOALGO_TECB); @@ -1947,6 +1969,10 @@ static WC_INLINE void wc_Stm32_CrypDesBlock(const byte* in, byte* out) return BAD_FUNC_ARG; } + if (!des->keySet) { + return MISSING_KEY; + } + while (blocks--) { Des3ProcessBlock(des, in, out);