From 213aec86885befa99a2db3bdd2091438a9013706 Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Thu, 23 Mar 2017 14:37:37 +0100 Subject: [PATCH 1/2] RSA: wipe stack buffers The RSA private key functions rsa_rsaes_pkcs1_v15_decrypt and rsa_rsaes_oaep_decrypt put sensitive data (decryption results) on the stack. Wipe it before returning. Thanks to Laurent Simon for reporting this issue. --- ChangeLog | 7 +++++++ library/rsa.c | 42 ++++++++++++++++++++++++++++++++++-------- 2 files changed, 41 insertions(+), 8 deletions(-) diff --git a/ChangeLog b/ChangeLog index b46c72879..49b02f01a 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,12 @@ mbed TLS ChangeLog (Sorted per branch, date) += mbed TLS 1.3.x branch released xxxx-xx-xx + +Security + * Wipe stack buffers in RSA private key operations + (rsa_rsaes_pkcs1_v15_decrypt(), rsa_rsaes_oaep_decrypt). + Found by Laurent Simon. + = mbed TLS 1.3.19 branch released 2017-03-08 Security diff --git a/library/rsa.c b/library/rsa.c index 79726c1b5..09477cbc4 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -56,6 +56,11 @@ #define polarssl_free free #endif +/* Implementation that should never be optimized out by the compiler */ +static void polarssl_zeroize( void *v, size_t n ) { + volatile unsigned char *p = (unsigned char*)v; while( n-- ) *p++ = 0; +} + /* * Initialize an RSA context */ @@ -720,7 +725,7 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, : rsa_private( ctx, f_rng, p_rng, input, buf ); if( ret != 0 ) - return( ret ); + goto cleanup; /* * Unmask data and generate lHash @@ -785,15 +790,26 @@ int rsa_rsaes_oaep_decrypt( rsa_context *ctx, * the different error conditions. */ if( bad != 0 ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + { + ret = POLARSSL_ERR_RSA_INVALID_PADDING; + goto cleanup; + } if( ilen - ( p - buf ) > output_max_len ) - return( POLARSSL_ERR_RSA_OUTPUT_TOO_LARGE ); + { + ret = POLARSSL_ERR_RSA_OUTPUT_TOO_LARGE; + goto cleanup; + } *olen = ilen - (p - buf); memcpy( output, p, *olen ); + ret = 0; - return( 0 ); +cleanup: + polarssl_zeroize( buf, sizeof( buf ) ); + polarssl_zeroize( lhash, sizeof( lhash ) ); + + return( ret ); } #endif /* POLARSSL_PKCS1_V21 */ @@ -827,7 +843,7 @@ int rsa_rsaes_pkcs1_v15_decrypt( rsa_context *ctx, : rsa_private( ctx, f_rng, p_rng, input, buf ); if( ret != 0 ) - return( ret ); + goto cleanup; p = buf; bad = 0; @@ -872,15 +888,25 @@ int rsa_rsaes_pkcs1_v15_decrypt( rsa_context *ctx, bad |= ( pad_count < 8 ); if( bad ) - return( POLARSSL_ERR_RSA_INVALID_PADDING ); + { + ret = POLARSSL_ERR_RSA_INVALID_PADDING; + goto cleanup; + } if( ilen - ( p - buf ) > output_max_len ) - return( POLARSSL_ERR_RSA_OUTPUT_TOO_LARGE ); + { + ret = POLARSSL_ERR_RSA_OUTPUT_TOO_LARGE; + goto cleanup; + } *olen = ilen - (p - buf); memcpy( output, p, *olen ); + ret = 0; - return( 0 ); +cleanup: + polarssl_zeroize( buf, sizeof( buf ) ); + + return( ret ); } #endif /* POLARSSL_PKCS1_V15 */ From 73e7f4c0eec1108dd331224b65184bc3b61b10ff Mon Sep 17 00:00:00 2001 From: Gilles Peskine Date: Fri, 5 May 2017 19:24:06 +0200 Subject: [PATCH 2/2] RSA: wipe more stack buffers MGF mask and PSS salt are not highly sensitive, but wipe them anyway for good hygiene. --- library/rsa.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/library/rsa.c b/library/rsa.c index 09477cbc4..8d7e9e623 100644 --- a/library/rsa.c +++ b/library/rsa.c @@ -492,6 +492,8 @@ static void mgf_mask( unsigned char *dst, size_t dlen, unsigned char *src, dlen -= use_len; } + + polarssl_zeroize( mask, sizeof( mask ) ); } #endif /* POLARSSL_PKCS1_V21 */ @@ -1011,6 +1013,7 @@ int rsa_rsassa_pss_sign( rsa_context *ctx, if( ( ret = md_init_ctx( &md_ctx, md_info ) ) != 0 ) { md_free( &md_ctx ); + /* No need to zeroize salt: we didn't use it. */ return( ret ); } @@ -1021,6 +1024,7 @@ int rsa_rsassa_pss_sign( rsa_context *ctx, md_update( &md_ctx, hash, hashlen ); md_update( &md_ctx, salt, slen ); md_finish( &md_ctx, p ); + polarssl_zeroize( salt, sizeof( salt ) ); // Compensate for boundary condition when applying mask //