From 33ca6af8a3bd770cb454793b9f6fc52e5d43bb21 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 1 Dec 2021 21:58:05 +0100 Subject: [PATCH 1/6] Return an error for IV lengths other than 12 with ChaCha20 The implementation was silently overwriting the IV length to 12 even though the caller passed a different value. Change the behavior to signal that a different length is not supported. Signed-off-by: Andrzej Kurek --- library/cipher.c | 6 +++ tests/suites/test_suite_cipher.chacha20.data | 20 ++++++++++ tests/suites/test_suite_cipher.function | 41 ++++++++++++++++++++ 3 files changed, 67 insertions(+) diff --git a/library/cipher.c b/library/cipher.c index 03e84c6c8..39c105cfb 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -386,6 +386,12 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_CHACHA20_C) if ( ctx->cipher_info->type == MBEDTLS_CIPHER_CHACHA20 ) { + /* Even though the actual_iv_size is overwritten with a correct value + * of 12 from the cipher info, return an error to indicate that + * the input iv_len is wrong. */ + if( iv_len != 12 ) + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); + if ( 0 != mbedtls_chacha20_starts( (mbedtls_chacha20_context*)ctx->cipher_ctx, iv, 0U ) ) /* Initial counter value */ diff --git a/tests/suites/test_suite_cipher.chacha20.data b/tests/suites/test_suite_cipher.chacha20.data index 11de1038a..117fce339 100644 --- a/tests/suites/test_suite_cipher.chacha20.data +++ b/tests/suites/test_suite_cipher.chacha20.data @@ -109,3 +109,23 @@ enc_dec_buf_multipart:MBEDTLS_CIPHER_CHACHA20:256:6:16:-1:6:16:6:16 ChaCha20 Encrypt and decrypt 32 bytes in multiple parts depends_on:MBEDTLS_CHACHA20_C enc_dec_buf_multipart:MBEDTLS_CIPHER_CHACHA20:256:16:16:-1:16:16:16:16 + +ChaCha20 IV Length 0 +depends_on:MBEDTLS_CHACHA20_C +check_iv:MBEDTLS_CIPHER_CHACHA20:"CHACHA20":0:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20 IV Length 11 +depends_on:MBEDTLS_CHACHA20_C +check_iv:MBEDTLS_CIPHER_CHACHA20:"CHACHA20":11:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20 IV Length 12 +depends_on:MBEDTLS_CHACHA20_C +check_iv:MBEDTLS_CIPHER_CHACHA20:"CHACHA20":12:0 + +ChaCha20 IV Length 13 +depends_on:MBEDTLS_CHACHA20_C +check_iv:MBEDTLS_CIPHER_CHACHA20:"CHACHA20":13:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20 IV Length 16 +depends_on:MBEDTLS_CHACHA20_C +check_iv:MBEDTLS_CIPHER_CHACHA20:"CHACHA20":16:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index fd2985b5e..c837a69bd 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -442,6 +442,8 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, if( NULL != strstr( cipher_info->name, "CCM*-NO-TAG") ) iv_len = 13; /* For CCM, IV length is expected to be between 7 and 13 bytes. * For CCM*-NO-TAG, IV length must be exactly 13 bytes long. */ + else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 ) + iv_len = 12; else iv_len = sizeof(iv); @@ -689,6 +691,8 @@ void enc_dec_buf_multipart( int cipher_id, int key_len, int first_length_val, if( NULL != strstr( cipher_info->name, "CCM*-NO-TAG") ) iv_len = 13; /* For CCM, IV length is expected to be between 7 and 13 bytes. * For CCM*-NO-TAG, IV length must be exactly 13 bytes long. */ + else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 ) + iv_len = 12; else iv_len = sizeof(iv); @@ -1130,3 +1134,40 @@ void check_padding( int pad_mode, data_t * input, int ret, int dlen_check TEST_ASSERT( dlen == (size_t) dlen_check ); } /* END_CASE */ + +/* BEGIN_CASE */ +void check_iv( int cipher_id, char * cipher_string, + int iv_len_val, int ret ) +{ + size_t iv_len = iv_len_val; + unsigned char iv[16]; + + const mbedtls_cipher_info_t *cipher_info; + mbedtls_cipher_context_t ctx_dec; + mbedtls_cipher_context_t ctx_enc; + + /* + * Prepare contexts + */ + mbedtls_cipher_init( &ctx_dec ); + mbedtls_cipher_init( &ctx_enc ); + + /* Check and get info structures */ + cipher_info = mbedtls_cipher_info_from_type( cipher_id ); + TEST_ASSERT( NULL != cipher_info ); + TEST_ASSERT( mbedtls_cipher_info_from_string( cipher_string ) == cipher_info ); + TEST_ASSERT( strcmp( mbedtls_cipher_info_get_name( cipher_info ), + cipher_string ) == 0 ); + + /* Initialise enc and dec contexts */ + TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_dec, cipher_info ) ); + TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_enc, cipher_info ) ); + + TEST_ASSERT( ret == mbedtls_cipher_set_iv( &ctx_dec, iv, iv_len ) ); + TEST_ASSERT( ret == mbedtls_cipher_set_iv( &ctx_enc, iv, iv_len ) ); + +exit: + mbedtls_cipher_free( &ctx_dec ); + mbedtls_cipher_free( &ctx_enc ); +} +/* END_CASE */ From 63439eda62e629a6d3e8f75087819ef5c8133015 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 1 Dec 2021 22:19:33 +0100 Subject: [PATCH 2/6] Return an error for IV lengths other than 12 with ChaCha20+Poly1305 The implementation was silently overwriting the IV length to 12 even though the caller passed a different value. Change the behavior to signal that a different length is not supported. Signed-off-by: Andrzej Kurek --- library/cipher.c | 5 +++++ .../suites/test_suite_cipher.chachapoly.data | 20 +++++++++++++++++++ tests/suites/test_suite_cipher.function | 14 ++++++++++--- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/library/cipher.c b/library/cipher.c index 39c105cfb..4c7ca3f57 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -399,6 +399,11 @@ int mbedtls_cipher_set_iv( mbedtls_cipher_context_t *ctx, return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); } } +#if defined(MBEDTLS_CHACHAPOLY_C) + if ( ctx->cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 && + iv_len != 12 ) + return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); +#endif #endif #if defined(MBEDTLS_GCM_C) diff --git a/tests/suites/test_suite_cipher.chachapoly.data b/tests/suites/test_suite_cipher.chachapoly.data index 8c246adb4..908951a18 100644 --- a/tests/suites/test_suite_cipher.chachapoly.data +++ b/tests/suites/test_suite_cipher.chachapoly.data @@ -121,3 +121,23 @@ auth_crypt_tv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"1c9240a5eb55d38af333888604f6b5f0 Chacha20+Poly1305 RFC 7539 Test Vector #1 (streaming) depends_on:MBEDTLS_CHACHAPOLY_C decrypt_test_vec:MBEDTLS_CIPHER_CHACHA20_POLY1305:-1:"1c9240a5eb55d38af333888604f6b5f0473917c1402b80099dca5cbc207075c0":"000000000102030405060708":"64a0861575861af460f062c79be643bd5e805cfd345cf389f108670ac76c8cb24c6cfc18755d43eea09ee94e382d26b0bdb7b73c321b0100d4f03b7f355894cf332f830e710b97ce98c8a84abd0b948114ad176e008d33bd60f982b1ff37c8559797a06ef4f0ef61c186324e2b3506383606907b6a7c02b0f9f6157b53c867e4b9166c767b804d46a59b5216cde7a4e99040c5a40433225ee282a1b0a06c523eaf4534d7f83fa1155b0047718cbc546a0d072b04b3564eea1b422273f548271a0bb2316053fa76991955ebd63159434ecebb4e466dae5a1073a6727627097a1049e617d91d361094fa68f0ff77987130305beaba2eda04df997b714d6c6f2c29a6ad5cb4022b02709b":"496e7465726e65742d4472616674732061726520647261667420646f63756d656e74732076616c696420666f722061206d6178696d756d206f6620736978206d6f6e74687320616e64206d617920626520757064617465642c207265706c616365642c206f72206f62736f6c65746564206279206f7468657220646f63756d656e747320617420616e792074696d652e20497420697320696e617070726f70726961746520746f2075736520496e7465726e65742d447261667473206173207265666572656e6365206d6174657269616c206f7220746f2063697465207468656d206f74686572207468616e206173202fe2809c776f726b20696e2070726f67726573732e2fe2809d":"f33388860000000000004e91":"eead9d67890cbb22392336fea1851f38":0:0 + +ChaCha20+Poly1305 IV Length 0 +depends_on:MBEDTLS_CHACHAPOLY_C +check_iv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"CHACHA20-POLY1305":0:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20+Poly1305 IV Length 11 +depends_on:MBEDTLS_CHACHAPOLY_C +check_iv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"CHACHA20-POLY1305":11:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20+Poly1305 IV Length 12 +depends_on:MBEDTLS_CHACHAPOLY_C +check_iv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"CHACHA20-POLY1305":12:0 + +ChaCha20+Poly1305 IV Length 13 +depends_on:MBEDTLS_CHACHAPOLY_C +check_iv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"CHACHA20-POLY1305":13:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA + +ChaCha20+Poly1305 IV Length 16 +depends_on:MBEDTLS_CHACHAPOLY_C +check_iv:MBEDTLS_CIPHER_CHACHA20_POLY1305:"CHACHA20-POLY1305":16:MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index c837a69bd..2aa7303ac 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -442,7 +442,8 @@ void enc_dec_buf( int cipher_id, char * cipher_string, int key_len, if( NULL != strstr( cipher_info->name, "CCM*-NO-TAG") ) iv_len = 13; /* For CCM, IV length is expected to be between 7 and 13 bytes. * For CCM*-NO-TAG, IV length must be exactly 13 bytes long. */ - else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 ) + else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 || + cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) iv_len = 12; else iv_len = sizeof(iv); @@ -571,6 +572,7 @@ void dec_empty_buf( int cipher, { unsigned char key[32]; unsigned char iv[16]; + size_t iv_len = sizeof(iv); mbedtls_cipher_context_t ctx_dec; const mbedtls_cipher_info_t *cipher_info; @@ -591,6 +593,11 @@ void dec_empty_buf( int cipher, /* Initialise context */ cipher_info = mbedtls_cipher_info_from_type( cipher ); TEST_ASSERT( NULL != cipher_info); + + if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 || + cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) + iv_len = 12; + TEST_ASSERT( sizeof(key) * 8 >= cipher_info->key_bitlen ); TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_dec, cipher_info ) ); @@ -599,7 +606,7 @@ void dec_empty_buf( int cipher, key, cipher_info->key_bitlen, MBEDTLS_DECRYPT ) ); - TEST_ASSERT( 0 == mbedtls_cipher_set_iv( &ctx_dec, iv, 16 ) ); + TEST_ASSERT( 0 == mbedtls_cipher_set_iv( &ctx_dec, iv, iv_len ) ); TEST_ASSERT( 0 == mbedtls_cipher_reset( &ctx_dec ) ); @@ -691,7 +698,8 @@ void enc_dec_buf_multipart( int cipher_id, int key_len, int first_length_val, if( NULL != strstr( cipher_info->name, "CCM*-NO-TAG") ) iv_len = 13; /* For CCM, IV length is expected to be between 7 and 13 bytes. * For CCM*-NO-TAG, IV length must be exactly 13 bytes long. */ - else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 ) + else if( cipher_info->type == MBEDTLS_CIPHER_CHACHA20 || + cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) iv_len = 12; else iv_len = sizeof(iv); From 8be8e4a524d99c7fc39d4afc80f4112deb9d2bec Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 1 Dec 2021 22:20:06 +0100 Subject: [PATCH 3/6] Add a missing test case to ChaCha20 tests - decrypt empty buffer Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_cipher.chacha20.data | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/suites/test_suite_cipher.chacha20.data b/tests/suites/test_suite_cipher.chacha20.data index 117fce339..dfa0e76ad 100644 --- a/tests/suites/test_suite_cipher.chacha20.data +++ b/tests/suites/test_suite_cipher.chacha20.data @@ -1,3 +1,7 @@ +Decrypt empty buffer +depends_on:MBEDTLS_CHACHAPOLY_C +dec_empty_buf:MBEDTLS_CIPHER_CHACHA20:0:0 + Chacha20 RFC 7539 Test Vector #1 depends_on:MBEDTLS_CHACHA20_C decrypt_test_vec:MBEDTLS_CIPHER_CHACHA20:-1:"0000000000000000000000000000000000000000000000000000000000000000":"000000000000000000000000":"76b8e0ada0f13d90405d6ae55386bd28bdd219b8a08ded1aa836efcc8b770dc7da41597c5157488d7724e03fb8d84a376a43b8f41518a11cc387b669b2ee6586":"00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000":"":"":0:0 From f2d4e275a88de50d69be23dc19db4f098a16ad70 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 1 Dec 2021 22:25:48 +0100 Subject: [PATCH 4/6] Add a changelog entry for the ChaCha20 default behavior change Signed-off-by: Andrzej Kurek --- ChangeLog.d/chacha20_invalid_iv_len_fix.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 ChangeLog.d/chacha20_invalid_iv_len_fix.txt diff --git a/ChangeLog.d/chacha20_invalid_iv_len_fix.txt b/ChangeLog.d/chacha20_invalid_iv_len_fix.txt new file mode 100644 index 000000000..af35e2a00 --- /dev/null +++ b/ChangeLog.d/chacha20_invalid_iv_len_fix.txt @@ -0,0 +1,4 @@ +Default behavior changes + * mbedtls_cipher_set_iv will now fail with ChaCha20 and ChaCha20+Poly1305 + for IV lengths other than 12. The library was silently overwriting this + length with 12, but did not inform the caller about it. Fixes #4301. From b9fbc11e2c60a7c0f748c60ffc7e96e7a6665457 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Fri, 14 Jan 2022 16:31:39 +0100 Subject: [PATCH 5/6] Dynamically allocate iv in dec_empty_buf tests Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_cipher.function | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 2aa7303ac..cd79ba47f 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -571,8 +571,9 @@ void dec_empty_buf( int cipher, int expected_finish_ret ) { unsigned char key[32]; - unsigned char iv[16]; - size_t iv_len = sizeof(iv); + + unsigned char *iv = NULL; + size_t iv_len = 16; mbedtls_cipher_context_t ctx_dec; const mbedtls_cipher_info_t *cipher_info; @@ -583,7 +584,6 @@ void dec_empty_buf( int cipher, size_t outlen = 0; memset( key, 0, 32 ); - memset( iv , 0, 16 ); mbedtls_cipher_init( &ctx_dec ); @@ -598,6 +598,9 @@ void dec_empty_buf( int cipher, cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 ) iv_len = 12; + ASSERT_ALLOC( iv, iv_len ); + memset( iv , 0, iv_len ); + TEST_ASSERT( sizeof(key) * 8 >= cipher_info->key_bitlen ); TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_dec, cipher_info ) ); @@ -636,6 +639,7 @@ void dec_empty_buf( int cipher, TEST_ASSERT( 0 == outlen ); exit: + mbedtls_free( iv ); mbedtls_cipher_free( &ctx_dec ); } /* END_CASE */ From ad2b8b5c3c10fd9cf4fee0e22c099afed8b4c306 Mon Sep 17 00:00:00 2001 From: Andrzej Kurek Date: Wed, 19 Jan 2022 07:35:27 -0500 Subject: [PATCH 6/6] Fix a dependence in chacha cipher test suite Signed-off-by: Andrzej Kurek --- tests/suites/test_suite_cipher.chacha20.data | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/suites/test_suite_cipher.chacha20.data b/tests/suites/test_suite_cipher.chacha20.data index dfa0e76ad..bcd0032ea 100644 --- a/tests/suites/test_suite_cipher.chacha20.data +++ b/tests/suites/test_suite_cipher.chacha20.data @@ -1,5 +1,5 @@ Decrypt empty buffer -depends_on:MBEDTLS_CHACHAPOLY_C +depends_on:MBEDTLS_CHACHA20_C dec_empty_buf:MBEDTLS_CIPHER_CHACHA20:0:0 Chacha20 RFC 7539 Test Vector #1