Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Return an error from mbedtls_cipher_set_iv for an invalid IV length with ChaCha20 and ChaCha20+Poly #5253

Merged
merged 6 commits into from
Jan 21, 2022
Merged

Return an error from mbedtls_cipher_set_iv for an invalid IV length with ChaCha20 and ChaCha20+Poly #5253

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
33ca6af
Return an error for IV lengths other than 12 with ChaCha20
Dec 1, 2021
63439ed
Return an error for IV lengths other than 12 with ChaCha20+Poly1305
Dec 1, 2021
8be8e4a
Add a missing test case to ChaCha20 tests - decrypt empty buffer
Dec 1, 2021
f2d4e27
Add a changelog entry for the ChaCha20 default behavior change
Dec 1, 2021
b9fbc11
Dynamically allocate iv in dec_empty_buf tests
Jan 14, 2022
ad2b8b5
Fix a dependence in chacha cipher test suite
Jan 19, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions ChangeLog.d/chacha20_invalid_iv_len_fix.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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.
11 changes: 11 additions & 0 deletions library/cipher.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -386,13 +386,24 @@ 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 */
{
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)
Expand Down
24 changes: 24 additions & 0 deletions tests/suites/test_suite_cipher.chacha20.data
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
Decrypt empty buffer
depends_on:MBEDTLS_CHACHA20_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
Expand Down Expand Up @@ -109,3 +113,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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use macro also on success.


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
20 changes: 20 additions & 0 deletions tests/suites/test_suite_cipher.chachapoly.data
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
gabor-mezei-arm marked this conversation as resolved.
Show resolved Hide resolved

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
59 changes: 56 additions & 3 deletions tests/suites/test_suite_cipher.function
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,9 @@ 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 ||
cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 )
iv_len = 12;
else
iv_len = sizeof(iv);

Expand Down Expand Up @@ -568,7 +571,9 @@ void dec_empty_buf( int cipher,
int expected_finish_ret )
{
unsigned char key[32];
unsigned char iv[16];

unsigned char *iv = NULL;
size_t iv_len = 16;

mbedtls_cipher_context_t ctx_dec;
const mbedtls_cipher_info_t *cipher_info;
Expand All @@ -579,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 );

Expand All @@ -589,6 +593,14 @@ 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;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that 16 byes for iv is default (and max) length and for those two cipher types we are only limiting the length, but maybe we could first determine the iv length and then allocate memory?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done that :) It has an additional advantage as described in my comment below.


ASSERT_ALLOC( iv, iv_len );
gabor-mezei-arm marked this conversation as resolved.
Show resolved Hide resolved
memset( iv , 0, iv_len );

TEST_ASSERT( sizeof(key) * 8 >= cipher_info->key_bitlen );

TEST_ASSERT( 0 == mbedtls_cipher_setup( &ctx_dec, cipher_info ) );
Expand All @@ -597,7 +609,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 ) );

Expand Down Expand Up @@ -627,6 +639,7 @@ void dec_empty_buf( int cipher,
TEST_ASSERT( 0 == outlen );

exit:
mbedtls_free( iv );
mbedtls_cipher_free( &ctx_dec );
}
/* END_CASE */
Expand Down Expand Up @@ -689,6 +702,9 @@ 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 ||
cipher_info->type == MBEDTLS_CIPHER_CHACHA20_POLY1305 )
iv_len = 12;
else
iv_len = sizeof(iv);

Expand Down Expand Up @@ -1130,3 +1146,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 */