From 010ddc2b620b463dc50d1929e499f635f873baa4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?C=C3=A9dric=20Meuter?= Date: Sat, 25 Apr 2020 09:24:11 +0200 Subject: [PATCH] Integrated feedback of first code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fixed code style. - Clarified the documentation of what happens when saltlen is set to MBEDTLS_RSA_SALT_LEN_ANY. - Added range check on saltlen to reject out of range values. (Code review done by @gilles-peskine-arm) Signed-off-by: Cédric Meuter --- include/mbedtls/rsa.h | 19 +++++++++---------- library/rsa.c | 11 ++++++++--- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/include/mbedtls/rsa.h b/include/mbedtls/rsa.h index 41a00a9ed..5fc6ddb42 100644 --- a/include/mbedtls/rsa.h +++ b/include/mbedtls/rsa.h @@ -979,14 +979,11 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, * Specifications it is advised to keep both hashes the * same. * - * \note This function always uses the maximum possible salt size, - * up to the length of the payload hash. This choice of salt - * size complies with FIPS 186-4 §5.5 (e) and RFC 8017 (PKCS#1 - * v2.2) §9.1.1 step 3. Furthermore this function enforces a - * minimum salt size which is the hash size minus 2 bytes. If - * this minimum size is too large given the key size (the salt - * size, plus the hash size, plus 2 bytes must be no more than - * the key size in bytes), this function returns + * \note This function enforces that the provided salt length complies + * with FIPS 186-4 §5.5 (e) and RFC 8017 (PKCS#1 v2.2) §9.1.1 + * step 3. The constraint is that the hash length plus the salt + * length plus 2 bytes must be at most the key length. If this + * constraint is not met, this function returns * #MBEDTLS_ERR_RSA_BAD_INPUT_DATA. * * \deprecated It is deprecated and discouraged to call this function @@ -1014,8 +1011,10 @@ int mbedtls_rsa_rsassa_pkcs1_v15_sign( mbedtls_rsa_context *ctx, * #MBEDTLS_MD_NONE, it must be a readable buffer of length * the size of the hash corresponding to \p md_alg. * \param saltlen The length of the salt that should be used. - * If passed MBEDTLS_RSA_SALT_LEN_ANY, the function will use - * the largest possible salt length. + * If passed #MBEDTLS_RSA_SALT_LEN_ANY, the function will use + * the largest possible salt length up to the hash length, + * which is the largest permitted by some standards including + * FIPS 186-4 §5.5. * \param sig The buffer to hold the signature. This must be a writable * buffer of length \c ctx->len Bytes. For example, \c 256 Bytes * for an 2048-bit RSA modulus. A buffer length of diff --git a/library/rsa.c b/library/rsa.c index 7652f3d6b..62c092746 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1843,8 +1843,9 @@ int mbedtls_rsa_rsassa_pss_sign_ext( mbedtls_rsa_context *ctx, if (saltlen == MBEDTLS_RSA_SALT_LEN_ANY) { - /* Calculate the largest possible salt length. Normally this is the hash - * length, which is the maximum length the salt can have. If there is not + /* Calculate the largest possible salt length, up to the hash size. + * Normally this is the hash length, which is the maximum salt length + * according to FIPS 185-4 §5.5 (e) and common practice. If there is not * enough room, use the maximum salt length that fits. The constraint is * that the hash length plus the salt length plus 2 bytes must be at most * the key length. This complies with FIPS 186-4 §5.5 (e) and RFC 8017 @@ -1857,9 +1858,13 @@ int mbedtls_rsa_rsassa_pss_sign_ext( mbedtls_rsa_context *ctx, else slen = olen - hlen - 2; } + else if ( (saltlen < 0) || ((size_t) saltlen > olen - hlen - 2) ) + { + return( MBEDTLS_ERR_RSA_BAD_INPUT_DATA ); + } else { - slen = (size_t)saltlen; + slen = (size_t) saltlen; } memset( sig, 0, olen );