diff --git a/ChangeLog b/ChangeLog index 7bf2a6744..9419df268 100644 --- a/ChangeLog +++ b/ChangeLog @@ -6,6 +6,7 @@ Changes * Speedup of ECP multiplication operation * Relaxed some SHA2 ciphersuite's version requirements * Dropped use of readdir_r() instead of readdir() with threading support + * More constant-time checks in the RSA module Bugfix * Fixed X.509 hostname comparison (with non-regular characters) diff --git a/library/rsa.c b/library/rsa.c index 210ea46e3..fdcfaef75 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -646,14 +646,17 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, size_t output_max_len ) { int ret; - size_t ilen; - unsigned char *p; + size_t ilen, i, pad_len; + unsigned char *p, bad, pad_done; unsigned char buf[POLARSSL_MPI_MAX_SIZE]; unsigned char lhash[POLARSSL_MD_MAX_SIZE]; unsigned int hlen; const md_info_t *md_info; md_context_t md_ctx; + /* + * Parameters sanity checks + */ if( ctx->padding != RSA_PKCS_V21 ) return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); @@ -662,6 +665,13 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, if( ilen < 16 || ilen > sizeof( buf ) ) return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); + md_info = md_info_from_type( ctx->hash_id ); + if( md_info == NULL ) + return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); + + /* + * RSA operation + */ ret = ( mode == RSA_PUBLIC ) ? rsa_public( ctx, input, buf ) : rsa_private( ctx, f_rng, p_rng, input, buf ); @@ -669,50 +679,60 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, if( ret != 0 ) return( ret ); - p = buf; - - if( *p++ != 0 ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); - - md_info = md_info_from_type( ctx->hash_id ); - if( md_info == NULL ) - return( POLARSSL_ERR_RSA_BAD_INPUT_DATA ); - + /* + * Unmask data and generate lHash + */ hlen = md_get_size( md_info ); md_init_ctx( &md_ctx, md_info ); - // Generate lHash - // + /* Generate lHash */ md( md_info, label, label_len, lhash ); - // seed: Apply seedMask to maskedSeed - // + /* seed: Apply seedMask to maskedSeed */ mgf_mask( buf + 1, hlen, buf + hlen + 1, ilen - hlen - 1, &md_ctx ); - // DB: Apply dbMask to maskedDB - // + /* DB: Apply dbMask to maskedDB */ mgf_mask( buf + hlen + 1, ilen - hlen - 1, buf + 1, hlen, &md_ctx ); - p += hlen; md_free_ctx( &md_ctx ); - // Check validity - // - if( memcmp( lhash, p, hlen ) != 0 ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + /* + * Check contents, in "constant-time" + */ + p = buf; + bad = 0; - p += hlen; + bad |= *p++; /* First byte must be 0 */ - while( *p == 0 && p < buf + ilen ) - p++; + p += hlen; /* Skip seed */ - if( p == buf + ilen ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + /* Check lHash */ + for( i = 0; i < hlen; i++ ) + bad |= lhash[i] ^ *p++; - if( *p++ != 0x01 ) + /* Get zero-padding len, but always read till end of buffer + * (minus one, for the 01 byte) */ + pad_len = 0; + pad_done = 0; + for( i = 0; i < ilen - 2 * hlen - 2; i++ ) + { + pad_done |= p[i]; + pad_len += ( pad_done == 0 ); + } + + p += pad_len; + bad |= *p++ ^ 0x01; + + /* + * The only information "leaked" is whether the padding was correct or not + * (eg, no data is copied if it was not correct). This meets the + * recommendations in PKCS#1 v2.2: an opponent cannot distinguish between + * the different error conditions. + */ + if( bad != 0 ) return( POLARSSL_ERR_RSA_INVALID_PADDING ); if (ilen - (p - buf) > output_max_len) @@ -737,10 +757,9 @@ int rsa_rsaes_pkcs1_v15_decrypt( rsa_context *ctx, unsigned char *output, size_t output_max_len) { - int ret, correct = 1; - size_t ilen, pad_count = 0; - unsigned char *p, *q; - unsigned char bt; + int ret; + size_t ilen, pad_count = 0, i; + unsigned char *p, bad, pad_done = 0; unsigned char buf[POLARSSL_MPI_MAX_SIZE]; if( ctx->padding != RSA_PKCS_V15 ) @@ -759,57 +778,46 @@ int rsa_rsaes_pkcs1_v15_decrypt( rsa_context *ctx, return( ret ); p = buf; + bad = 0; - if( *p++ != 0 ) - correct = 0; + /* + * Check and get padding len in "constant-time" + */ + bad |= *p++; /* First byte must be 0 */ - bt = *p++; - if( ( bt != RSA_CRYPT && mode == RSA_PRIVATE ) || - ( bt != RSA_SIGN && mode == RSA_PUBLIC ) ) + /* This test does not depend on secret data */ + if( mode == RSA_PRIVATE ) { - correct = 0; - } + bad |= *p++ ^ RSA_CRYPT; - if( bt == RSA_CRYPT ) - { - while( *p != 0 && p < buf + ilen - 1 ) - pad_count += ( *p++ != 0 ); + /* Get padding len, but always read till end of buffer + * (minus one, for the 00 byte) */ + for( i = 0; i < ilen - 3; i++ ) + { + pad_done |= ( p[i] == 0 ); + pad_count += ( pad_done == 0 ); + } - correct &= ( *p == 0 && p < buf + ilen - 1 ); - - q = p; - - // Also pass over all other bytes to reduce timing differences - // - while ( q < buf + ilen - 1 ) - pad_count += ( *q++ != 0 ); - - // Prevent compiler optimization of pad_count - // - correct |= pad_count & 0x100000; /* Always 0 unless 1M bit keys */ - p++; + p += pad_count; + bad |= *p++; /* Must be zero */ } else { - while( *p == 0xFF && p < buf + ilen - 1 ) - pad_count += ( *p++ == 0xFF ); + bad |= *p++ ^ RSA_SIGN; - correct &= ( *p == 0 && p < buf + ilen - 1 ); + /* Get padding len, but always read till end of buffer + * (minus one, for the 00 byte) */ + for( i = 0; i < ilen - 3; i++ ) + { + pad_done |= ( p[i] == 0xFF ); + pad_count += ( pad_done == 0 ); + } - q = p; - - // Also pass over all other bytes to reduce timing differences - // - while ( q < buf + ilen - 1 ) - pad_count += ( *q++ != 0 ); - - // Prevent compiler optimization of pad_count - // - correct |= pad_count & 0x100000; /* Always 0 unless 1M bit keys */ - p++; + p += pad_count; + bad |= *p++; /* Must be zero */ } - if( correct == 0 ) + if( bad ) return( POLARSSL_ERR_RSA_INVALID_PADDING ); if (ilen - (p - buf) > output_max_len)