From 39f361466bbcaf012518cd90de73ebf6fdc78545 Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Mon, 1 Feb 2021 14:23:30 +0100 Subject: [PATCH 1/4] avoid errorneous computation of RSA_PRV_DER_MAX_BYTES if MBEDTLS_MPI_MAX_SIZE is odd. if MBEDTLS_MPI_MAX_SIZE is odd then RSA_PRV_DER_MAX_BYTES will be two less than expected, since the macros are lacking parentheses. Signed-off-by: Daniel Otte --- library/pkwrite.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index 0da369818..f46d486c0 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -504,8 +504,8 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ * otherPrimeInfos OtherPrimeInfos OPTIONAL 0 (not supported) * } */ -#define MPI_MAX_SIZE_2 MBEDTLS_MPI_MAX_SIZE / 2 + \ - MBEDTLS_MPI_MAX_SIZE % 2 +#define MPI_MAX_SIZE_2 ( MBEDTLS_MPI_MAX_SIZE / 2 + \ + MBEDTLS_MPI_MAX_SIZE % 2 ) #define RSA_PRV_DER_MAX_BYTES 47 + 3 * MBEDTLS_MPI_MAX_SIZE \ + 5 * MPI_MAX_SIZE_2 From 79fb5da63219664a9e6f04ee8c4b246e31d1739c Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Mon, 1 Feb 2021 14:26:08 +0100 Subject: [PATCH 2/4] adding parentheses to macro definitions, to avoid confusion and possible mistakes in usage. Signed-off-by: Daniel Otte --- library/pkwrite.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/library/pkwrite.c b/library/pkwrite.c index f46d486c0..566153dd9 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -487,7 +487,7 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ * publicExponent INTEGER -- e 1 + 3 + MPI_MAX + 1 * } */ -#define RSA_PUB_DER_MAX_BYTES 38 + 2 * MBEDTLS_MPI_MAX_SIZE +#define RSA_PUB_DER_MAX_BYTES ( 38 + 2 * MBEDTLS_MPI_MAX_SIZE ) /* * RSA private keys: @@ -506,8 +506,8 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ */ #define MPI_MAX_SIZE_2 ( MBEDTLS_MPI_MAX_SIZE / 2 + \ MBEDTLS_MPI_MAX_SIZE % 2 ) -#define RSA_PRV_DER_MAX_BYTES 47 + 3 * MBEDTLS_MPI_MAX_SIZE \ - + 5 * MPI_MAX_SIZE_2 +#define RSA_PRV_DER_MAX_BYTES ( 47 + 3 * MBEDTLS_MPI_MAX_SIZE \ + + 5 * MPI_MAX_SIZE_2 ) #else /* MBEDTLS_RSA_C */ @@ -528,7 +528,7 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ * + 2 * ECP_MAX (coords) [1] * } */ -#define ECP_PUB_DER_MAX_BYTES 30 + 2 * MBEDTLS_ECP_MAX_BYTES +#define ECP_PUB_DER_MAX_BYTES ( 30 + 2 * MBEDTLS_ECP_MAX_BYTES ) /* * EC private keys: @@ -539,7 +539,7 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ * publicKey [1] BIT STRING OPTIONAL 1 + 2 + [1] above * } */ -#define ECP_PRV_DER_MAX_BYTES 29 + 3 * MBEDTLS_ECP_MAX_BYTES +#define ECP_PRV_DER_MAX_BYTES ( 29 + 3 * MBEDTLS_ECP_MAX_BYTES ) #else /* MBEDTLS_ECP_C */ @@ -548,10 +548,10 @@ int mbedtls_pk_write_key_der( mbedtls_pk_context *key, unsigned char *buf, size_ #endif /* MBEDTLS_ECP_C */ -#define PUB_DER_MAX_BYTES RSA_PUB_DER_MAX_BYTES > ECP_PUB_DER_MAX_BYTES ? \ - RSA_PUB_DER_MAX_BYTES : ECP_PUB_DER_MAX_BYTES -#define PRV_DER_MAX_BYTES RSA_PRV_DER_MAX_BYTES > ECP_PRV_DER_MAX_BYTES ? \ - RSA_PRV_DER_MAX_BYTES : ECP_PRV_DER_MAX_BYTES +#define PUB_DER_MAX_BYTES ( RSA_PUB_DER_MAX_BYTES > ECP_PUB_DER_MAX_BYTES ? \ + RSA_PUB_DER_MAX_BYTES : ECP_PUB_DER_MAX_BYTES ) +#define PRV_DER_MAX_BYTES ( RSA_PRV_DER_MAX_BYTES > ECP_PRV_DER_MAX_BYTES ? \ + RSA_PRV_DER_MAX_BYTES : ECP_PRV_DER_MAX_BYTES ) int mbedtls_pk_write_pubkey_pem( mbedtls_pk_context *key, unsigned char *buf, size_t size ) { From 21cbe4a4943acbe83a5ec3b3081a9ad6e81de479 Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Mon, 1 Feb 2021 18:44:24 +0100 Subject: [PATCH 3/4] adding entry file to ChangeLog.d for PR4094 Signed-off-by: Daniel Otte --- ChangeLog.d/issue4093.txt | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 ChangeLog.d/issue4093.txt diff --git a/ChangeLog.d/issue4093.txt b/ChangeLog.d/issue4093.txt new file mode 100644 index 000000000..8f6e8f713 --- /dev/null +++ b/ChangeLog.d/issue4093.txt @@ -0,0 +1,6 @@ +Bugfix + * Fix an errorneous estimation for an internal buffer in + mbedtls_pk_write_key_pem(). If MBEDTLS_MPI_MAX_SIZE is set to an odd + value the function might fail to write a private RSA keys of the largest + supported size. + Found by Daniel Otte in #4093 and fixed in #4094. \ No newline at end of file From da43d78017e74ef089a2d5f0cd0be497d0cc46ed Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Tue, 2 Feb 2021 12:38:26 +0100 Subject: [PATCH 4/4] adjusting Changelog entry for PR #4094 Signed-off-by: Daniel Otte --- ChangeLog.d/issue4093.txt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/ChangeLog.d/issue4093.txt b/ChangeLog.d/issue4093.txt index 8f6e8f713..f6985cfd4 100644 --- a/ChangeLog.d/issue4093.txt +++ b/ChangeLog.d/issue4093.txt @@ -1,6 +1,6 @@ -Bugfix - * Fix an errorneous estimation for an internal buffer in +Security + * Fix an errorneous estimation for an internal buffer in mbedtls_pk_write_key_pem(). If MBEDTLS_MPI_MAX_SIZE is set to an odd value the function might fail to write a private RSA keys of the largest supported size. - Found by Daniel Otte in #4093 and fixed in #4094. \ No newline at end of file + Found by Daniel Otte, reported in #4093 and fixed in #4094.