From f9111bbdd89966ff781cc6385de1f4e3534c423e Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Mon, 1 Feb 2021 14:23:30 +0100 Subject: [PATCH 1/3] 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 b02956519..6bb31bc3b 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -454,8 +454,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 4490fc6a380165c537fafafbe3f3ea3a70ff91aa Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Mon, 1 Feb 2021 14:26:08 +0100 Subject: [PATCH 2/3] 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 6bb31bc3b..d54c74a34 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -437,7 +437,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: @@ -456,8 +456,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 */ @@ -478,7 +478,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: @@ -489,7 +489,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 */ @@ -498,10 +498,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 b49815a88f882471eaafd043b2ed10cd186750fe Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Tue, 2 Feb 2021 12:44:07 +0100 Subject: [PATCH 3/3] adding changelog entry for issue #4093 Signed-off-by: Daniel Otte --- ChangeLog.d/issue4093.txt | 7 +++++++ 1 file changed, 7 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..fe4ce49f3 --- /dev/null +++ b/ChangeLog.d/issue4093.txt @@ -0,0 +1,7 @@ +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, reported in #4093 and fixed in #4094, + backported in #4099.