From 80a2c2a5f96f6b20805a7ecc48d31f4285f5839f 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 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 150626c14..f9e598c93 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -472,8 +472,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 9c6cb217f18fbb54f7e1c835d04053e0c21a7819 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. Avoid confusion and possible mistakes in usage of macros. 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 f9e598c93..a770dfb93 100644 --- a/library/pkwrite.c +++ b/library/pkwrite.c @@ -455,7 +455,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: @@ -474,8 +474,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 */ @@ -496,7 +496,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: @@ -507,7 +507,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 */ @@ -516,10 +516,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 80fa1b4d8f93e93457c1fe99e72caa5e1464d435 Mon Sep 17 00:00:00 2001 From: Daniel Otte Date: Tue, 2 Feb 2021 12:57:48 +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..b8d634b15 --- /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 #4100.