From f2ffbc43878c971235253f815f3d1ce61012c92c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Tue, 1 Dec 2020 09:57:55 +0100 Subject: [PATCH] Stop supporting NIST_KW in cipher_auth_xxcrypt() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Manuel Pégourié-Gonnard --- include/mbedtls/cipher.h | 30 ++++++++++++-------- library/cipher.c | 28 ++++++------------- tests/suites/test_suite_cipher.function | 37 ++++++++++++++++++------- 3 files changed, 54 insertions(+), 41 deletions(-) diff --git a/include/mbedtls/cipher.h b/include/mbedtls/cipher.h index 49b8b5e9ea..24524c5c31 100644 --- a/include/mbedtls/cipher.h +++ b/include/mbedtls/cipher.h @@ -858,10 +858,14 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_CIPHER_MODE_AEAD) /** - * \brief The generic autenticated encryption (AEAD) function. + * \brief The generic authenticated encryption (AEAD) function. + * + * \note This function only supports AEAD algorithms, not key + * wrapping algorithms such as NIST_KW; for this, see + * mbedtls_cipher_auth_encrypt_ext(). * * \param ctx The generic cipher context. This must be initialized and - * bound to a key. + * bound to a key associated with an AEAD algorithm. * \param iv The nonce to use. This must be a readable buffer of * at least \p iv_len Bytes and must not be \c NULL. * \param iv_len The length of the nonce. This must satisfy the @@ -885,7 +889,7 @@ int mbedtls_cipher_crypt( mbedtls_cipher_context_t *ctx, * below regarding restrictions with PSA-based contexts. * \param tag_len The desired length of the authentication tag. This * must match the constraints imposed by the AEAD cipher - * used, and in particuler must not be \c 0. + * used, and in particular must not be \c 0. * * \note If the context is based on PSA (that is, it was set up * with mbedtls_cipher_setup_psa()), then it is required @@ -905,14 +909,18 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, unsigned char *tag, size_t tag_len ); /** - * \brief The generic autenticated decryption (AEAD) function. + * \brief The generic authenticated decryption (AEAD) function. + * + * \note This function only supports AEAD algorithms, not key + * wrapping algorithms such as NIST_KW; for this, see + * mbedtls_cipher_auth_encrypt_ext(). * * \note If the data is not authentic, then the output buffer * is zeroed out to prevent the unauthentic plaintext being * used, making this interface safer. * * \param ctx The generic cipher context. This must be initialized and - * and bound to a key. + * bound to a key associated with an AEAD algorithm. * \param iv The nonce to use. This must be a readable buffer of * at least \p iv_len Bytes and must not be \c NULL. * \param iv_len The length of the nonce. This must satisfy the @@ -959,14 +967,14 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, #if defined(MBEDTLS_CIPHER_MODE_AEAD) || defined(MBEDTLS_NIST_KW_C) /** - * \brief The autenticated encryption (AEAD/NIST_KW) function. + * \brief The authenticated encryption (AEAD/NIST_KW) function. * * \note For AEAD modes, the tag will be appended to the * ciphertext, as recommended by RFC 5116. * (NIST_KW doesn't have a separate tag.) * * \param ctx The generic cipher context. This must be initialized and - * bound to a key. + * bound to a key, with an AEAD algorithm or NIST_KW. * \param iv The nonce to use. This must be a readable buffer of * at least \p iv_len Bytes and may be \c NULL if \p * iv_len is \c 0. @@ -994,7 +1002,7 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, * writable object of type \c size_t. * \param tag_len The desired length of the authentication tag. For AEAD * ciphers, this must match the constraints imposed by - * the cipher used, and in particuler must not be \c 0. + * the cipher used, and in particular must not be \c 0. * For NIST_KW, this must be \c 0. * * \return \c 0 on success. @@ -1010,7 +1018,7 @@ int mbedtls_cipher_auth_encrypt_ext( mbedtls_cipher_context_t *ctx, size_t *olen, size_t tag_len ); /** - * \brief The autenticated encryption (AEAD/NIST_KW) function. + * \brief The authenticated encryption (AEAD/NIST_KW) function. * * \note If the data is not authentic, then the output buffer * is zeroed out to prevent the unauthentic plaintext being @@ -1021,7 +1029,7 @@ int mbedtls_cipher_auth_encrypt_ext( mbedtls_cipher_context_t *ctx, * (NIST_KW doesn't have a separate tag.) * * \param ctx The generic cipher context. This must be initialized and - * and bound to a key. + * bound to a key, with an AEAD algorithm or NIST_KW. * \param iv The nonce to use. This must be a readable buffer of * at least \p iv_len Bytes and may be \c NULL if \p * iv_len is \c 0. @@ -1049,7 +1057,7 @@ int mbedtls_cipher_auth_encrypt_ext( mbedtls_cipher_context_t *ctx, * writable object of type \c size_t. * \param tag_len The actual length of the authentication tag. For AEAD * ciphers, this must match the constraints imposed by - * the cipher used, and in particuler must not be \c 0. + * the cipher used, and in particular must not be \c 0. * For NIST_KW, this must be \c 0. * * \return \c 0 on success. diff --git a/library/cipher.c b/library/cipher.c index 20e1e7b3a9..47dafa4aa2 100644 --- a/library/cipher.c +++ b/library/cipher.c @@ -1491,16 +1491,10 @@ int mbedtls_cipher_auth_encrypt( mbedtls_cipher_context_t *ctx, if( MBEDTLS_MODE_KW == ctx->cipher_info->mode || MBEDTLS_MODE_KWP == ctx->cipher_info->mode ) { - mbedtls_nist_kw_mode_t mode = ( MBEDTLS_MODE_KW == ctx->cipher_info->mode ) ? - MBEDTLS_KW_MODE_KW : MBEDTLS_KW_MODE_KWP; - - /* There is no iv, tag or ad associated with KW and KWP, these length should be 0 */ - if( iv_len != 0 || tag_len != 0 || ad_len != 0 ) - { - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - } - - return( mbedtls_nist_kw_wrap( ctx->cipher_ctx, mode, input, ilen, output, olen, SIZE_MAX ) ); + /* NIST_KW is not supported because we used to document the wrong size + * of the output buffer, so people should move to the _ext API, + * which has an explicit argument for buffer size. */ + return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); } #endif /* MBEDTLS_NIST_KW_C */ @@ -1531,16 +1525,10 @@ int mbedtls_cipher_auth_decrypt( mbedtls_cipher_context_t *ctx, if( MBEDTLS_MODE_KW == ctx->cipher_info->mode || MBEDTLS_MODE_KWP == ctx->cipher_info->mode ) { - mbedtls_nist_kw_mode_t mode = ( MBEDTLS_MODE_KW == ctx->cipher_info->mode ) ? - MBEDTLS_KW_MODE_KW : MBEDTLS_KW_MODE_KWP; - - /* There is no iv, tag or ad associated with KW and KWP, these length should be 0 */ - if( iv_len != 0 || tag_len != 0 || ad_len != 0 ) - { - return( MBEDTLS_ERR_CIPHER_BAD_INPUT_DATA ); - } - - return( mbedtls_nist_kw_unwrap( ctx->cipher_ctx, mode, input, ilen, output, olen, SIZE_MAX ) ); + /* NIST_KW is not supported because we used to document the wrong size + * of the output buffer, so people should move to the _ext API, + * which has an explicit argument for buffer size. */ + return( MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); } #endif /* MBEDTLS_NIST_KW_C */ diff --git a/tests/suites/test_suite_cipher.function b/tests/suites/test_suite_cipher.function index 7ea1a14a2a..543ccf6fdc 100644 --- a/tests/suites/test_suite_cipher.function +++ b/tests/suites/test_suite_cipher.function @@ -1222,22 +1222,31 @@ void auth_crypt_tv( int cipher_id, data_t * key, data_t * iv, tmp_cipher, cipher->len, output, &outlen, tmp_tag, tag->len ); - /* make sure the message is rejected if it should be */ - if( strcmp( result, "FAIL" ) == 0 ) + if( using_nist_kw ) { + /* NIST_KW with legacy API */ + TEST_ASSERT( ret == MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); + } + else if( strcmp( result, "FAIL" ) == 0 ) + { + /* unauthentic message */ TEST_ASSERT( ret == MBEDTLS_ERR_CIPHER_AUTH_FAILED ); } else { - /* otherwise, make sure it was decrypted properly */ + /* authentic message: is the plaintext correct? */ TEST_ASSERT( ret == 0 ); TEST_ASSERT( outlen == clear->len ); TEST_ASSERT( memcmp( output, clear->x, clear->len ) == 0 ); + } - /* - * Prepare context for encryption - */ + /* + * Encrypt back if test data was authentic + */ + if( strcmp( result, "FAIL" ) != 0 ) + { + /* prepare context for encryption */ cipher_reset_key( &ctx, cipher_id, use_psa, tag->len, key, MBEDTLS_ENCRYPT ); @@ -1254,11 +1263,19 @@ void auth_crypt_tv( int cipher_id, data_t * key, data_t * iv, ret = mbedtls_cipher_auth_encrypt( &ctx, iv->x, iv->len, ad->x, ad->len, clear->x, clear->len, output, &outlen, output_tag, tag->len ); - TEST_ASSERT( ret == 0 ); - TEST_ASSERT( outlen == cipher->len ); - TEST_ASSERT( memcmp( output, cipher->x, cipher->len ) == 0 ); - TEST_ASSERT( memcmp( output_tag, tag->x, tag->len ) == 0 ); + if( using_nist_kw ) + { + TEST_ASSERT( ret == MBEDTLS_ERR_CIPHER_FEATURE_UNAVAILABLE ); + } + else + { + TEST_ASSERT( ret == 0 ); + + TEST_ASSERT( outlen == cipher->len ); + TEST_ASSERT( memcmp( output, cipher->x, cipher->len ) == 0 ); + TEST_ASSERT( memcmp( output_tag, tag->x, tag->len ) == 0 ); + } } exit: