diff --git a/doc/dox_comments/header_files/arc4.h b/doc/dox_comments/header_files/arc4.h index 3bbcced77c..7058e09c6f 100644 --- a/doc/dox_comments/header_files/arc4.h +++ b/doc/dox_comments/header_files/arc4.h @@ -7,7 +7,10 @@ Before this method may be called, one must first initialize the ARC4 structure using wc_Arc4SetKey. - \return none + \return 0 Returned upon successfully processing the message. + \return BAD_FUNC_ARG If arc4, out, or in is NULL. + \return MISSING_KEY If no key has been set on the ARC4 structure with + wc_Arc4SetKey. \param arc4 pointer to the ARC4 structure used to process the message \param out pointer to the output buffer in which to store the diff --git a/tests/api/test_arc4.c b/tests/api/test_arc4.c index 11bd20d671..3fe4d57eae 100644 --- a/tests/api/test_arc4.c +++ b/tests/api/test_arc4.c @@ -104,6 +104,43 @@ int test_wc_Arc4Process(void) } /* END test_wc_Arc4Process */ +/* + * Regression test for issue 5378: wc_Arc4Process must refuse to run unless a + * key has been configured with wc_Arc4SetKey(), both before SetKey and after + * Free. Otherwise the keystream is all-zero and ARC4 silently copies the + * plaintext into the ciphertext. + */ +int test_wc_Arc4Process_no_key(void) +{ + EXPECT_DECLS; +#ifndef NO_RC4 + Arc4 arc4; + const char* key = "\x01\x23\x45\x67\x89\xab\xcd\xef"; + int keyLen = 8; + const char* input = "\x01\x23\x45\x67\x89\xab\xcd\xef"; + byte cipher[8]; + + XMEMSET(&arc4, 0, sizeof(arc4)); + XMEMSET(cipher, 0, sizeof(cipher)); + + /* Processing without a key after init must fail, not silently copy. */ + ExpectIntEQ(wc_Arc4Init(&arc4, NULL, INVALID_DEVID), 0); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), + WC_NO_ERR_TRACE(MISSING_KEY)); + + /* After a key is set, processing succeeds. */ + ExpectIntEQ(wc_Arc4SetKey(&arc4, (byte*)key, (word32)keyLen), 0); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), 0); + + /* After free, the keyed state is cleared and processing must fail again. */ + wc_Arc4Free(&arc4); + ExpectIntEQ(wc_Arc4Process(&arc4, cipher, (byte*)input, (word32)keyLen), + WC_NO_ERR_TRACE(MISSING_KEY)); +#endif + return EXPECT_RESULT(); + +} /* END test_wc_Arc4Process_no_key */ + #include diff --git a/tests/api/test_arc4.h b/tests/api/test_arc4.h index f79104d9c6..28075e3bc9 100644 --- a/tests/api/test_arc4.h +++ b/tests/api/test_arc4.h @@ -26,11 +26,13 @@ int test_wc_Arc4SetKey(void); int test_wc_Arc4Process(void); +int test_wc_Arc4Process_no_key(void); int test_wc_Arc4_MonteCarlo(void); #define TEST_ARC4_DECLS \ TEST_DECL_GROUP("arc4", test_wc_Arc4SetKey), \ TEST_DECL_GROUP("arc4", test_wc_Arc4Process), \ + TEST_DECL_GROUP("arc4", test_wc_Arc4Process_no_key), \ TEST_DECL_GROUP("arc4", test_wc_Arc4_MonteCarlo) #endif /* WOLFCRYPT_TEST_ARC4_H */ diff --git a/tests/api/test_ossl_cipher.c b/tests/api/test_ossl_cipher.c index f40b097546..c618d45c4d 100644 --- a/tests/api/test_ossl_cipher.c +++ b/tests/api/test_ossl_cipher.c @@ -965,7 +965,9 @@ int test_wolfSSL_RC4(void) wolfSSL_RC4(NULL, 0, data, enc); ExpectIntEQ(1, 1); - for (i = 0; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) { + /* Start at a key length of 1: a zero-length key is invalid and leaves the + * Arc4 object without a key set, so encrypt/decrypt is a no-op. */ + for (i = 1; EXPECT_SUCCESS() && (i <= sizeof(key)); i++) { for (j = 0; EXPECT_SUCCESS() && (j <= sizeof(data)); j++) { XMEMSET(enc, 0, sizeof(enc)); XMEMSET(dec, 0, sizeof(dec)); diff --git a/wolfcrypt/src/arc4.c b/wolfcrypt/src/arc4.c index 6d83ed2569..dc37014335 100644 --- a/wolfcrypt/src/arc4.c +++ b/wolfcrypt/src/arc4.c @@ -67,6 +67,8 @@ int wc_Arc4SetKey(Arc4* arc4, const byte* key, word32 length) keyIndex = 0; } + arc4->keySet = 1; + return ret; } @@ -102,6 +104,10 @@ int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in, word32 length) } #endif + if (!arc4->keySet) { + return MISSING_KEY; + } + x = arc4->x; y = arc4->y; @@ -123,6 +129,7 @@ int wc_Arc4Init(Arc4* arc4, void* heap, int devId) return BAD_FUNC_ARG; arc4->heap = heap; + arc4->keySet = 0; #if defined(WOLFSSL_ASYNC_CRYPT) && defined(WC_ASYNC_ENABLE_ARC4) ret = wolfAsync_DevCtxInit(&arc4->asyncDev, WOLFSSL_ASYNC_MARKER_ARC4, @@ -148,6 +155,7 @@ void wc_Arc4Free(Arc4* arc4) ForceZero(arc4->state, sizeof(arc4->state)); arc4->x = 0; arc4->y = 0; + arc4->keySet = 0; } #endif /* NO_RC4 */ diff --git a/wolfssl/openssl/rc4.h b/wolfssl/openssl/rc4.h index c53c47b6b6..53168c6f81 100644 --- a/wolfssl/openssl/rc4.h +++ b/wolfssl/openssl/rc4.h @@ -40,9 +40,9 @@ typedef struct WOLFSSL_RC4_KEY { /* big enough for Arc4 from wolfssl/wolfcrypt/arc4.h */ #ifdef WC_NO_PTR_INT_CAST - void* holder[(288 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; + void* holder[(296 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; #else - void* holder[(272 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; + void* holder[(280 + WC_ASYNC_DEV_SIZE) / sizeof(void*)]; #endif } WOLFSSL_RC4_KEY; diff --git a/wolfssl/wolfcrypt/arc4.h b/wolfssl/wolfcrypt/arc4.h index 041ee71213..059014d544 100644 --- a/wolfssl/wolfcrypt/arc4.h +++ b/wolfssl/wolfcrypt/arc4.h @@ -51,6 +51,7 @@ typedef struct Arc4 { WC_ASYNC_DEV asyncDev; #endif void* heap; + WC_BITFIELD keySet:1; /* set to 1 once a key has been configured */ } Arc4; WOLFSSL_API int wc_Arc4Process(Arc4* arc4, byte* out, const byte* in,