From 3dc62a0a9b6776ca1f58724c5be01c77012edf94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Mon, 4 Jun 2018 12:18:19 +0200 Subject: [PATCH] chachapoly: force correct mode for integrated API Allowing DECRYPT with crypt_and_tag is a risk as people might fail to check the tag correctly (or at all). So force them to use auth_decrypt() instead. See also https://github.com/ARMmbed/mbedtls/pull/1668 --- include/mbedtls/chachapoly.h | 21 ++++----- library/chachapoly.c | 51 +++++++++++++-------- library/cipher.c | 3 +- programs/test/benchmark.c | 5 +- tests/suites/test_suite_chachapoly.function | 27 ++++------- 5 files changed, 53 insertions(+), 54 deletions(-) diff --git a/include/mbedtls/chachapoly.h b/include/mbedtls/chachapoly.h index be10cfd32..649749a01 100644 --- a/include/mbedtls/chachapoly.h +++ b/include/mbedtls/chachapoly.h @@ -269,7 +269,7 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, /** * \brief This function performs a complete ChaCha20-Poly1305 - * operation with the previously-set key. + * authenticated encryption with the previously-set key. * * \note Before using this function, you must set the key with * \c mbedtls_chachapoly_setkey(). @@ -280,8 +280,6 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, * and key. * * \param ctx The ChaCha20-Poly1305 context to use (holds the key). - * \param mode The operation to perform: #MBEDTLS_CHACHAPOLY_ENCRYPT or - * #MBEDTLS_CHACHAPOLY_DECRYPT. * \param length The length (in bytes) of the data to encrypt or decrypt. * \param nonce The 96-bit (12 bytes) nonce/IV to use. * \param aad The buffer containing the additional authenticated data (AAD). @@ -297,15 +295,14 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, * \return #MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA * if one or more of the required parameters are NULL. */ -int mbedtls_chachapoly_crypt_and_tag( mbedtls_chachapoly_context *ctx, - mbedtls_chachapoly_mode_t mode, - size_t length, - const unsigned char nonce[12], - const unsigned char *aad, - size_t aad_len, - const unsigned char *input, - unsigned char *output, - unsigned char tag[16] ); +int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, + size_t length, + const unsigned char nonce[12], + const unsigned char *aad, + size_t aad_len, + const unsigned char *input, + unsigned char *output, + unsigned char tag[16] ); /** * \brief This function performs a complete ChaCha20-Poly1305 diff --git a/library/chachapoly.c b/library/chachapoly.c index 8f785883b..80c1ebf8f 100644 --- a/library/chachapoly.c +++ b/library/chachapoly.c @@ -311,15 +311,15 @@ int mbedtls_chachapoly_finish( mbedtls_chachapoly_context *ctx, return( ret ); } -int mbedtls_chachapoly_crypt_and_tag( mbedtls_chachapoly_context *ctx, - mbedtls_chachapoly_mode_t mode, - size_t length, - const unsigned char nonce[12], - const unsigned char *aad, - size_t aad_len, - const unsigned char *input, - unsigned char *output, - unsigned char tag[16] ) +static int chachapoly_crypt_and_tag( mbedtls_chachapoly_context *ctx, + mbedtls_chachapoly_mode_t mode, + size_t length, + const unsigned char nonce[12], + const unsigned char *aad, + size_t aad_len, + const unsigned char *input, + unsigned char *output, + unsigned char tag[16] ) { int ret; @@ -341,6 +341,20 @@ cleanup: return( ret ); } +int mbedtls_chachapoly_encrypt_and_tag( mbedtls_chachapoly_context *ctx, + size_t length, + const unsigned char nonce[12], + const unsigned char *aad, + size_t aad_len, + const unsigned char *input, + unsigned char *output, + unsigned char tag[16] ) +{ + return( chachapoly_crypt_and_tag( ctx, MBEDTLS_CHACHAPOLY_ENCRYPT, + length, nonce, aad, aad_len, + input, output, tag ) ); +} + int mbedtls_chachapoly_auth_decrypt( mbedtls_chachapoly_context *ctx, size_t length, const unsigned char nonce[12], @@ -358,7 +372,7 @@ int mbedtls_chachapoly_auth_decrypt( mbedtls_chachapoly_context *ctx, if( tag == NULL ) return( MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - if( ( ret = mbedtls_chachapoly_crypt_and_tag( ctx, + if( ( ret = chachapoly_crypt_and_tag( ctx, MBEDTLS_CHACHAPOLY_DECRYPT, length, nonce, aad, aad_len, input, output, check_tag ) ) != 0 ) { @@ -499,15 +513,14 @@ int mbedtls_chachapoly_self_test( int verbose ) ret = mbedtls_chachapoly_setkey( &ctx, test_key[i] ); ASSERT( 0 == ret, ( "setkey() error code: %i\n", ret ) ); - ret = mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, - test_input_len[i], - test_nonce[i], - test_aad[i], - test_aad_len[i], - test_input[i], - output, - mac ); + ret = mbedtls_chachapoly_encrypt_and_tag( &ctx, + test_input_len[i], + test_nonce[i], + test_aad[i], + test_aad_len[i], + test_input[i], + output, + mac ); ASSERT( 0 == ret, ( "crypt_and_tag() error code: %i\n", ret ) ); diff --git a/library/cipher.c b/library/cipher.c index cf10094f6..5a96e2bc7 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -992,8 +992,7 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, } *olen = ilen; - return( mbedtls_chachapoly_crypt_and_tag( ctx->cipher_ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + return( mbedtls_chachapoly_encrypt_and_tag( ctx->cipher_ctx, ilen, iv, ad, ad_len, input, output, tag ) ); } #endif /* MBEDTLS_CHACHAPOLY_C */ diff --git a/programs/test/benchmark.c b/programs/test/benchmark.c index 3e9ab0a29..f266b82f4 100644 --- a/programs/test/benchmark.c +++ b/programs/test/benchmark.c @@ -491,9 +491,8 @@ int main( int argc, char *argv[] ) mbedtls_chachapoly_setkey( &chachapoly, tmp ); TIME_AND_TSC( title, - mbedtls_chachapoly_crypt_and_tag( &chachapoly, - MBEDTLS_CHACHAPOLY_ENCRYPT, BUFSIZE, tmp, - NULL, 0, buf, buf, tmp ) ); + mbedtls_chachapoly_encrypt_and_tag( &chachapoly, + BUFSIZE, tmp, NULL, 0, buf, buf, tmp ) ); mbedtls_chachapoly_free( &chachapoly ); } diff --git a/tests/suites/test_suite_chachapoly.function b/tests/suites/test_suite_chachapoly.function index 7baa22995..95dfd8a91 100644 --- a/tests/suites/test_suite_chachapoly.function +++ b/tests/suites/test_suite_chachapoly.function @@ -48,8 +48,7 @@ void mbedtls_chachapoly_enc( char *hex_key_string, char *hex_nonce_string, char TEST_ASSERT( mbedtls_chachapoly_setkey( &ctx, key_str ) == 0 ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce_str, aad_str, aad_len, input_str, output, mac ) == 0 ); @@ -149,38 +148,32 @@ void chachapoly_bad_params() TEST_ASSERT( mbedtls_chachapoly_setkey( &ctx, NULL ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( NULL, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( NULL, 0, nonce, aad, 0, input, output, mac ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, NULL, aad, 0, input, output, mac ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, nonce, NULL, aad_len, input, output, mac ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce, aad, 0, NULL, output, mac ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce, aad, 0, input, NULL, mac ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, nonce, aad, 0, input, output, NULL ) @@ -217,8 +210,7 @@ void chachapoly_bad_params() mac, input, NULL ) == MBEDTLS_ERR_POLY1305_BAD_INPUT_DATA ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, 0, nonce, aad, aad_len, NULL, NULL, mac ) @@ -229,8 +221,7 @@ void chachapoly_bad_params() mac, NULL, NULL ) == 0 ); - TEST_ASSERT( mbedtls_chachapoly_crypt_and_tag( &ctx, - MBEDTLS_CHACHAPOLY_ENCRYPT, + TEST_ASSERT( mbedtls_chachapoly_encrypt_and_tag( &ctx, input_len, nonce, NULL, 0, input, output, mac )