From 331d80e162d5b8528405e5e4814ec60cf8ab8618 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 4 Oct 2018 18:32:29 +0200 Subject: [PATCH] Evolve choose_int_from_mask to if_int Make the function more robust by taking an arbitrary zero/nonzero argument instead of insisting on zero/all-bits-one. Update and fix its documentation. --- library/rsa.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 5859bbeb1..f2b2fb4ed 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -1381,9 +1381,9 @@ cleanup: /** Turn zero-or-nonzero into zero-or-all-bits-one, without branches. * * \param value The value to analyze. - * \return \c 0 if \p value is zero, otherwise \c 0xff. + * \return Zero if \p value is zero, otherwise all-bits-one. */ -static unsigned unsigned_all_or_nothing( unsigned value ) +static unsigned all_or_nothing_int( unsigned value ) { /* MSVC has a warning about unary minus on unsigned, but this is * well-defined and precisely what we want to do here */ @@ -1399,13 +1399,17 @@ static unsigned unsigned_all_or_nothing( unsigned value ) /** Choose between two integer values, without branches. * - * \param mask Either \c 0 or \c ~0. - * \param if0 Value to use if \p mask = \c 0. - * \param if1 Value to use if \p mask = \c ~0. - * \return \c if1 if \p value is zero, otherwise \c if0. + * This is equivalent to `cond ? if1 : if0`, but is likely to be compiled + * to code using bitwise operation rather than a branch. + * + * \param cond Condition to test. + * \param if1 Value to use if \p cond is nonzero. + * \param if0 Value to use if \p cond is zero. + * \return \c if1 if \p cond is nonzero, otherwise \c if0. */ -static unsigned choose_int_from_mask( unsigned mask, unsigned if1, unsigned if0 ) +static unsigned if_int( unsigned cond, unsigned if1, unsigned if0 ) { + unsigned mask = all_or_nothing_int( cond ); return( ( mask & if1 ) | (~mask & if0 ) ); } @@ -1489,7 +1493,7 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, /* Set bad to zero if the padding is valid and * all-bits-one otherwise. The whole calculation of bad * is done in such a way to avoid branches. */ - bad = unsigned_all_or_nothing( bad ); + bad = all_or_nothing_int( bad ); /* If the padding is valid, set plaintext_size to the number of * remaining bytes after stripping the padding. If the padding @@ -1498,9 +1502,9 @@ int mbedtls_rsa_rsaes_pkcs1_v15_decrypt( mbedtls_rsa_context *ctx, * buffer. Do it without branches to avoid leaking the padding * validity through timing. RSA keys are small enough that all the * size_t values involved fit in unsigned int. */ - plaintext_size = choose_int_from_mask( bad, - (unsigned) plaintext_max_size, - (unsigned) ( ilen - ( p - buf ) ) ); + plaintext_size = if_int( bad, + (unsigned) plaintext_max_size, + (unsigned) ( ilen - ( p - buf ) ) ); /* Check if the decrypted plaintext fits in the output buffer. * If the padding is bad, this will always be the case,