From a5cfc35db26c433a073002a55b924e2e687959a6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Thu, 28 Nov 2013 15:57:52 +0100 Subject: [PATCH 1/3] RSA-OAEP decrypt: reorganise code --- library/rsa.c | 61 +++++++++++++++++++++++++++++---------------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 210ea46e3..577a14fe5 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -654,6 +654,9 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, 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,38 +679,37 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, if( ret != 0 ) return( ret ); + /* + * Unmask data + */ + hlen = md_get_size( md_info ); + + md_init_ctx( &md_ctx, md_info ); + + /* Generate lHash */ + md( md_info, label, label_len, lhash ); + + /* seed: Apply seedMask to maskedSeed */ + mgf_mask( buf + 1, hlen, buf + hlen + 1, ilen - hlen - 1, + &md_ctx ); + + /* DB: Apply dbMask to maskedDB */ + mgf_mask( buf + hlen + 1, ilen - hlen - 1, buf + 1, hlen, + &md_ctx ); + + md_free_ctx( &md_ctx ); + + /* + * Check contents + */ 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 ); + p += hlen; /* Skip seed */ - hlen = md_get_size( md_info ); - - md_init_ctx( &md_ctx, md_info ); - - // Generate lHash - // - md( md_info, label, label_len, lhash ); - - // seed: Apply seedMask to maskedSeed - // - mgf_mask( buf + 1, hlen, buf + hlen + 1, ilen - hlen - 1, - &md_ctx ); - - // 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 - // + /* Check lHash */ if( memcmp( lhash, p, hlen ) != 0 ) return( POLARSSL_ERR_RSA_INVALID_PADDING ); From ab44d7ecc3238108a56a5bbd073277d3da052aef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Fri, 29 Nov 2013 12:49:44 +0100 Subject: [PATCH 2/3] Check OAEP padding in a more constant-time way --- library/rsa.c | 41 ++++++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 15 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 577a14fe5..8c1f6170d 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -646,8 +646,8 @@ 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; @@ -680,7 +680,7 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, return( ret ); /* - * Unmask data + * Unmask data and generate lHash */ hlen = md_get_size( md_info ); @@ -700,28 +700,39 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, md_free_ctx( &md_ctx ); /* - * Check contents + * Check contents, in "constant-time" */ p = buf; + bad = 0; - if( *p++ != 0 ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + bad |= *p++; /* First byte must be 0 */ p += hlen; /* Skip seed */ /* Check lHash */ - if( memcmp( lhash, p, hlen ) != 0 ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + for( i = 0; i < hlen; i++ ) + bad |= lhash[i] ^ *p++; - p += hlen; + /* 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 ); + } - while( *p == 0 && p < buf + ilen ) - p++; + p += pad_len; + bad |= *p++ ^ 0x01; - if( p == buf + ilen ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); - - if( *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) From 27290daf3b92b841159d7b23f71251fee5ea6c1e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Manuel=20P=C3=A9gouri=C3=A9-Gonnard?= Date: Sat, 30 Nov 2013 13:36:53 +0100 Subject: [PATCH 3/3] Check PKCS 1.5 padding in a more constant-time way (Avoid branches that depend on secret data.) --- library/rsa.c | 74 +++++++++++++++++++++------------------------------ 1 file changed, 31 insertions(+), 43 deletions(-) diff --git a/library/rsa.c b/library/rsa.c index 8c1f6170d..fdcfaef75 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -757,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 ) @@ -779,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)