From 1a98acac1c6a31507f81164ba2b30e6357d6e44e Mon Sep 17 00:00:00 2001 From: Paul Elliott Date: Thu, 20 May 2021 18:24:07 +0100 Subject: [PATCH] Properly handle GCM's range of nonce sizes Add comment to the effect that we cannot really check nonce size as the GCM spec allows almost arbitrarily large nonces. As a result of this, change the operation nonce over to an allocated buffer to avoid overflow situations. Signed-off-by: Paul Elliott --- include/psa/crypto_builtin_primitives.h | 6 +++--- library/psa_crypto.c | 6 ++++++ library/psa_crypto_aead.c | 18 ++++++++++++++++-- library/psa_crypto_aead.h | 2 ++ 4 files changed, 27 insertions(+), 5 deletions(-) diff --git a/include/psa/crypto_builtin_primitives.h b/include/psa/crypto_builtin_primitives.h index b28e0d7e2e..b67b23ff12 100644 --- a/include/psa/crypto_builtin_primitives.h +++ b/include/psa/crypto_builtin_primitives.h @@ -135,7 +135,6 @@ typedef struct unsigned int body_started : 1; uint8_t tag_length; - uint8_t nonce_length; /* Buffers for AD/data - only required until CCM gets proper multipart support. */ @@ -149,7 +148,8 @@ typedef struct /* buffer to store Nonce - only required until CCM and GCM get proper multipart support. */ - uint8_t nonce[PSA_AEAD_NONCE_MAX_SIZE]; + uint8_t *nonce; + size_t nonce_length; union { @@ -168,7 +168,7 @@ typedef struct } mbedtls_psa_aead_operation_t; -#define MBEDTLS_PSA_AEAD_OPERATION_INIT {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, {0}, {0}} +#define MBEDTLS_PSA_AEAD_OPERATION_INIT {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, {0}} /* * BEYOND THIS POINT, TEST DRIVER DECLARATIONS ONLY. diff --git a/library/psa_crypto.c b/library/psa_crypto.c index c53020a2be..fcc22e167e 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -3429,6 +3429,12 @@ psa_status_t psa_aead_set_nonce( psa_aead_operation_t *operation, goto exit; } + /* Not checking nonce size here as GCM spec allows almost abitrarily large + * nonces. Please note that we do not generally recommend the usage of + * nonces of greater length than PSA_AEAD_NONCE_MAX_SIZE, as large nonces + * are hashed to a shorter size, which can then lead to collisions if you + encrypt a very large number of messages. */ + status = psa_driver_wrapper_aead_set_nonce( operation, nonce, nonce_length ); diff --git a/library/psa_crypto_aead.c b/library/psa_crypto_aead.c index bbfc9271ea..10849b2ad4 100644 --- a/library/psa_crypto_aead.c +++ b/library/psa_crypto_aead.c @@ -388,11 +388,16 @@ psa_status_t mbedtls_psa_aead_set_nonce( #if defined(MBEDTLS_PSA_BUILTIN_ALG_GCM) if( operation->alg == PSA_ALG_GCM ) { + operation->nonce = mbedtls_calloc( 1, nonce_length ); + + if( operation->nonce == NULL ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + /* GCM sets nonce once additional data has been supplied */ memcpy( operation->nonce, nonce, nonce_length ); /* We know that nonce size cannot exceed the uint8_t size */ - operation->nonce_length = ( uint8_t ) nonce_length; + operation->nonce_length = nonce_length; status = PSA_SUCCESS; } else @@ -400,12 +405,17 @@ psa_status_t mbedtls_psa_aead_set_nonce( #if defined(MBEDTLS_PSA_BUILTIN_ALG_CCM) if( operation->alg == PSA_ALG_CCM ) { + operation->nonce = mbedtls_calloc( 1, nonce_length ); + + if( operation->nonce == NULL ) + return( PSA_ERROR_INSUFFICIENT_MEMORY ); + /* Multipart CCM not supported as yet, so CCM is basically operating in oneshot mode. Store the nonce as we need this later */ memcpy( operation->nonce, nonce, nonce_length ); /* We know that nonce size cannot exceed the uint8_t size */ - operation->nonce_length = ( uint8_t ) nonce_length; + operation->nonce_length = nonce_length; status = PSA_SUCCESS; } else @@ -919,6 +929,10 @@ psa_status_t mbedtls_psa_aead_abort( mbedtls_free( operation->tag_buffer ); operation->tag_buffer = NULL; + mbedtls_free( operation->nonce ); + operation->nonce = NULL; + operation->nonce_length = 0; + return( PSA_SUCCESS ); } diff --git a/library/psa_crypto_aead.h b/library/psa_crypto_aead.h index fcac5cac18..ef4842e355 100644 --- a/library/psa_crypto_aead.h +++ b/library/psa_crypto_aead.h @@ -263,6 +263,8 @@ psa_status_t mbedtls_psa_aead_decrypt_setup( * \retval #PSA_ERROR_NOT_SUPPORTED * Algorithm previously set is not supported in this configuration of * the library. + * \retval #PSA_ERROR_INSUFFICIENT_MEMORY + * (GCM and CCM only) Unable to allocate buffer for nonce. */ psa_status_t mbedtls_psa_aead_set_nonce( mbedtls_psa_aead_operation_t *operation,