From f28261fc14da5f52b460c903e027aff187f9cf5f Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Thu, 30 Sep 2021 16:39:07 +0200 Subject: [PATCH 1/4] Remove output buffer limitation for PSA with GCM. The requirement of minimum 15 bytes for output buffer in psa_aead_finish() and psa_aead_verify() does not apply to the built-in implementation of the GCM. Alternative implementations are expected to verify the length of the provided output buffers and to return the MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL in case the buffer length is too small. Signed-off-by: Mateusz Starzyk --- ChangeLog.d/psa_gcm_buffer_limitation.txt | 11 +++++++++++ include/mbedtls/gcm.h | 2 ++ library/psa_crypto.c | 2 ++ library/psa_crypto_aead.c | 3 --- tests/suites/test_suite_psa_crypto.data | 2 +- 5 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 ChangeLog.d/psa_gcm_buffer_limitation.txt diff --git a/ChangeLog.d/psa_gcm_buffer_limitation.txt b/ChangeLog.d/psa_gcm_buffer_limitation.txt new file mode 100644 index 000000000..7259e5068 --- /dev/null +++ b/ChangeLog.d/psa_gcm_buffer_limitation.txt @@ -0,0 +1,11 @@ +Bugfix + * Remove PSA'a AEAD finish/verify output buffer limitation for GCM. + The requirement of minimum 15 bytes for output buffer in + psa_aead_finish() and psa_aead_verify() does not apply to the built-in + implementation of GCM. + +API changes + * New error code for GCM: MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL. + Alternative GCM implementations are expected to verify + the length of the provided output buffers and to return the + MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL in case the buffer length is too small. diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index 9d9155fc5..a4de9191d 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -45,6 +45,8 @@ #define MBEDTLS_ERR_GCM_AUTH_FAILED -0x0012 /** Bad input parameters to function. */ #define MBEDTLS_ERR_GCM_BAD_INPUT -0x0014 +/** An output buffer is too small. */ +#define MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL -0x0018 #ifdef __cplusplus extern "C" { diff --git a/library/psa_crypto.c b/library/psa_crypto.c index ece64b100..5978b6ac5 100644 --- a/library/psa_crypto.c +++ b/library/psa_crypto.c @@ -201,6 +201,8 @@ psa_status_t mbedtls_to_psa_error( int ret ) case MBEDTLS_ERR_GCM_AUTH_FAILED: return( PSA_ERROR_INVALID_SIGNATURE ); + case MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL: + return( PSA_ERROR_BUFFER_TOO_SMALL ); case MBEDTLS_ERR_GCM_BAD_INPUT: return( PSA_ERROR_INVALID_ARGUMENT ); diff --git a/library/psa_crypto_aead.c b/library/psa_crypto_aead.c index a72865c04..673cdf344 100644 --- a/library/psa_crypto_aead.c +++ b/library/psa_crypto_aead.c @@ -567,9 +567,6 @@ psa_status_t mbedtls_psa_aead_finish( #if defined(MBEDTLS_PSA_BUILTIN_ALG_GCM) if( operation->alg == PSA_ALG_GCM ) { - if( ciphertext_size < 15 ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - status = mbedtls_to_psa_error( mbedtls_gcm_finish( &operation->ctx.gcm, ciphertext, ciphertext_size, ciphertext_length, diff --git a/tests/suites/test_suite_psa_crypto.data b/tests/suites/test_suite_psa_crypto.data index 063629e59..3a3e67821 100644 --- a/tests/suites/test_suite_psa_crypto.data +++ b/tests/suites/test_suite_psa_crypto.data @@ -3348,7 +3348,7 @@ aead_multipart_update_buffer_test:PSA_KEY_TYPE_CHACHA20:"808182838485868788898a8 PSA AEAD finish buffer test: AES - GCM, BUF = 8, TAG = 16 depends_on:PSA_WANT_ALG_GCM:PSA_WANT_KEY_TYPE_AES -aead_multipart_finish_buffer_test:PSA_KEY_TYPE_AES:"fbc0b4c56a714c83217b2d1bcadd2ed2e9efb0dcac6cc19f":PSA_ALG_GCM:8:16:"5f4b43e811da9c470d6a9b01":"":"d2ae38c4375954835d75b8e4c2f9bbb4":PSA_ERROR_BUFFER_TOO_SMALL +aead_multipart_finish_buffer_test:PSA_KEY_TYPE_AES:"fbc0b4c56a714c83217b2d1bcadd2ed2e9efb0dcac6cc19f":PSA_ALG_GCM:8:16:"5f4b43e811da9c470d6a9b01":"":"d2ae38c4375954835d75b8e4c2f9bbb4":PSA_SUCCESS PSA AEAD finish buffer test: AES - GCM, BUF = 15, TAG = 20 depends_on:PSA_WANT_ALG_GCM:PSA_WANT_KEY_TYPE_AES From c48f43b44de534af4a89aae26cefebdba0a14aca Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Mon, 4 Oct 2021 13:46:38 +0200 Subject: [PATCH 2/4] Fix PSA AEAD GCM's update output buffer length verification. Move GCM's update output buffer length verification from PSA AEAD to the built-in implementation of the GCM. Signed-off-by: Mateusz Starzyk --- ChangeLog.d/psa_gcm_buffer_limitation.txt | 5 +++++ library/gcm.c | 2 +- library/psa_crypto_aead.c | 3 --- tests/suites/test_suite_gcm.aes128_de.data | 4 ++++ tests/suites/test_suite_gcm.aes128_en.data | 3 +++ tests/suites/test_suite_gcm.function | 24 ++++++++++++++++++++++ 6 files changed, 37 insertions(+), 4 deletions(-) diff --git a/ChangeLog.d/psa_gcm_buffer_limitation.txt b/ChangeLog.d/psa_gcm_buffer_limitation.txt index 7259e5068..0c07e2415 100644 --- a/ChangeLog.d/psa_gcm_buffer_limitation.txt +++ b/ChangeLog.d/psa_gcm_buffer_limitation.txt @@ -3,6 +3,11 @@ Bugfix The requirement of minimum 15 bytes for output buffer in psa_aead_finish() and psa_aead_verify() does not apply to the built-in implementation of GCM. + * Move GCM's update output buffer length verification from PSA AEAD to + the built-in implementation of the GCM. + The requirement for output buffer size to be equal or greater then + input buffer size is valid only for the built-in implementation of GCM. + Alternative GCM implementations can process whole blocks only. API changes * New error code for GCM: MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL. diff --git a/library/gcm.c b/library/gcm.c index 910646b28..6d625642e 100644 --- a/library/gcm.c +++ b/library/gcm.c @@ -431,7 +431,7 @@ int mbedtls_gcm_update( mbedtls_gcm_context *ctx, unsigned char ectr[16]; if( output_size < input_length ) - return( MBEDTLS_ERR_GCM_BAD_INPUT ); + return( MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL ); GCM_VALIDATE_RET( output_length != NULL ); *output_length = input_length; diff --git a/library/psa_crypto_aead.c b/library/psa_crypto_aead.c index 673cdf344..c7f7352fb 100644 --- a/library/psa_crypto_aead.c +++ b/library/psa_crypto_aead.c @@ -510,9 +510,6 @@ psa_status_t mbedtls_psa_aead_update( #if defined(MBEDTLS_PSA_BUILTIN_ALG_GCM) if( operation->alg == PSA_ALG_GCM ) { - if( output_size < input_length ) - return( PSA_ERROR_BUFFER_TOO_SMALL ); - status = mbedtls_to_psa_error( mbedtls_gcm_update( &operation->ctx.gcm, input, input_length, diff --git a/tests/suites/test_suite_gcm.aes128_de.data b/tests/suites/test_suite_gcm.aes128_de.data index 3df31e56b..ede6f243c 100644 --- a/tests/suites/test_suite_gcm.aes128_de.data +++ b/tests/suites/test_suite_gcm.aes128_de.data @@ -726,6 +726,10 @@ AES-GCM Bad IV (AES-128,128,0,0,32) #0 depends_on:MBEDTLS_AES_C gcm_bad_parameters:MBEDTLS_CIPHER_ID_AES:MBEDTLS_GCM_DECRYPT:"d0194b6ee68f0ed8adc4b22ed15dbf14":"":"":"":32:MBEDTLS_ERR_GCM_BAD_INPUT +AES-GCM, output buffer too small, NIST Validation (AES-128,128,1024,0,128) #0 +depends_on:MBEDTLS_AES_C +gcm_update_output_buffer_too_small:MBEDTLS_CIPHER_ID_AES:MBEDTLS_GCM_DECRYPT:"0dd358bc3f992f26e81e3a2f3aa2d517":"87cc4fd75788c9d5cc83bae5d764dd249d178ab23224049795d4288b5ed9ea3f317068a39a7574b300c8544226e87b08e008fbe241d094545c211d56ac44437d41491a438272738968c8d371aa7787b5f606c8549a9d868d8a71380e9657d3c0337979feb01de5991fc1470dfc59eb02511efbbff3fcb479a862ba3844a25aaa":"d8c750bb443ee1a169dfe97cfe4d855b" + AES-GCM Selftest depends_on:MBEDTLS_AES_C gcm_selftest: diff --git a/tests/suites/test_suite_gcm.aes128_en.data b/tests/suites/test_suite_gcm.aes128_en.data index d60c458bc..273642cbd 100644 --- a/tests/suites/test_suite_gcm.aes128_en.data +++ b/tests/suites/test_suite_gcm.aes128_en.data @@ -726,6 +726,9 @@ AES-GCM Bad IV (AES-128,128,0,0,32) #0 depends_on:MBEDTLS_AES_C gcm_bad_parameters:MBEDTLS_CIPHER_ID_AES:MBEDTLS_GCM_ENCRYPT:"d0194b6ee68f0ed8adc4b22ed15dbf14":"":"":"":32:MBEDTLS_ERR_GCM_BAD_INPUT +AES-GCM, output buffer too small, NIST Validation (AES-128,128,1024,0,128) #0 +gcm_update_output_buffer_too_small:MBEDTLS_CIPHER_ID_AES:MBEDTLS_GCM_ENCRYPT:"ce0f8cfe9d64c4f4c045d11b97c2d918":"dfff250d380f363880963b42d6913c1ba11e8edf7c4ab8b76d79ccbaac628f548ee542f48728a9a2620a0d69339c8291e8d398440d740e310908cdee7c273cc91275ce7271ba12f69237998b07b789b3993aaac8dc4ec1914432a30f5172f79ea0539bd1f70b36d437e5170bc63039a5280816c05e1e41760b58e35696cebd55":"ad4c3627a494fc628316dc03faf81db8" + AES-GCM Selftest depends_on:MBEDTLS_AES_C gcm_selftest: diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index c530e6b42..816ebc4ec 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -431,6 +431,30 @@ exit: } /* END_CASE */ +/* BEGIN_CASE */ +void gcm_update_output_buffer_too_small( int cipher_id, int mode, + data_t * key_str, const data_t *input, + const data_t *iv ) +{ + mbedtls_gcm_context ctx; + uint8_t *output = NULL; + size_t olen; + size_t output_len = input->len - 1; + + mbedtls_gcm_init( &ctx ); + TEST_EQUAL( mbedtls_gcm_setkey( &ctx, cipher_id, key_str->x, key_str->len * 8 ), 0 ); + TEST_EQUAL( 0, mbedtls_gcm_starts( &ctx, mode, iv->x, iv->len ) ); + + ASSERT_ALLOC( output, output_len ); + olen = 0xdeadbeef; + TEST_EQUAL( MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL, mbedtls_gcm_update( &ctx, input->x, input->len, output, output_len, &olen ) ); + +exit: + mbedtls_free( output ); + mbedtls_gcm_free( &ctx ); +} +/* END_CASE */ + /* BEGIN_CASE depends_on:MBEDTLS_SELF_TEST */ void gcm_selftest( ) { From 33d01ffe60fe7ce2ebb8e649b657e89a4a8318dd Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Thu, 21 Oct 2021 14:55:59 +0200 Subject: [PATCH 3/4] Remove redundant value assignemnt to olen. Signed-off-by: Mateusz Starzyk --- tests/suites/test_suite_gcm.function | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/suites/test_suite_gcm.function b/tests/suites/test_suite_gcm.function index 816ebc4ec..5696679ea 100644 --- a/tests/suites/test_suite_gcm.function +++ b/tests/suites/test_suite_gcm.function @@ -438,7 +438,7 @@ void gcm_update_output_buffer_too_small( int cipher_id, int mode, { mbedtls_gcm_context ctx; uint8_t *output = NULL; - size_t olen; + size_t olen = 0; size_t output_len = input->len - 1; mbedtls_gcm_init( &ctx ); @@ -446,7 +446,6 @@ void gcm_update_output_buffer_too_small( int cipher_id, int mode, TEST_EQUAL( 0, mbedtls_gcm_starts( &ctx, mode, iv->x, iv->len ) ); ASSERT_ALLOC( output, output_len ); - olen = 0xdeadbeef; TEST_EQUAL( MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL, mbedtls_gcm_update( &ctx, input->x, input->len, output, output_len, &olen ) ); exit: From 30bd7fa607f63f1a7fa013898817ecd8385b60e6 Mon Sep 17 00:00:00 2001 From: Mateusz Starzyk Date: Fri, 22 Oct 2021 10:33:25 +0200 Subject: [PATCH 4/4] Change error code for MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL. Signed-off-by: Mateusz Starzyk --- include/mbedtls/error.h | 2 +- include/mbedtls/gcm.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/include/mbedtls/error.h b/include/mbedtls/error.h index 27420ce4f..8b2b9ea58 100644 --- a/include/mbedtls/error.h +++ b/include/mbedtls/error.h @@ -56,7 +56,7 @@ * Module Nr Codes assigned * ERROR 2 0x006E 0x0001 * MPI 7 0x0002-0x0010 - * GCM 3 0x0012-0x0014 0x0013-0x0013 + * GCM 3 0x0012-0x0016 0x0013-0x0013 * THREADING 3 0x001A-0x001E * AES 5 0x0020-0x0022 0x0021-0x0025 * CAMELLIA 3 0x0024-0x0026 0x0027-0x0027 diff --git a/include/mbedtls/gcm.h b/include/mbedtls/gcm.h index a4de9191d..7dc9dfb8e 100644 --- a/include/mbedtls/gcm.h +++ b/include/mbedtls/gcm.h @@ -46,7 +46,7 @@ /** Bad input parameters to function. */ #define MBEDTLS_ERR_GCM_BAD_INPUT -0x0014 /** An output buffer is too small. */ -#define MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL -0x0018 +#define MBEDTLS_ERR_GCM_BUFFER_TOO_SMALL -0x0016 #ifdef __cplusplus extern "C" {