Merged more constant-time checking in RSA

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)

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;
+    size_t ilen, i, pad_len;
-    unsigned char *p;
+    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;
+    /*
-
+     * Unmask data and generate lHash
-    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 );
-
     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
+    /*
-    //
+     * Check contents, in "constant-time"
-    if( memcmp( lhash, p, hlen ) != 0 )
+     */
-        return( POLARSSL_ERR_RSA_INVALID_PADDING );
+    p = buf;
+    bad = 0;
 
-    p += hlen;
+    bad |= *p++; /* First byte must be 0 */
 
-    while( *p == 0 && p < buf + ilen )
+    p += hlen; /* Skip seed */
-        p++;
 
-    if( p == buf + ilen )
+    /* Check lHash */
-        return( POLARSSL_ERR_RSA_INVALID_PADDING );
+    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;
+    int ret;
-    size_t ilen, pad_count = 0;
+    size_t ilen, pad_count = 0, i;
-    unsigned char *p, *q;
+    unsigned char *p, bad, pad_done = 0;
-    unsigned char bt;
     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++;
+    /* This test does not depend on secret data */
-    if( ( bt != RSA_CRYPT && mode == RSA_PRIVATE ) ||
+    if( mode == RSA_PRIVATE )
-        ( bt != RSA_SIGN && mode == RSA_PUBLIC ) )
     {
-        correct = 0;
+        bad |= *p++ ^ RSA_CRYPT;
-    }
 
-    if( bt == RSA_CRYPT )
+        /* Get padding len, but always read till end of buffer
-    {
+         * (minus one, for the 00 byte) */
-        while( *p != 0 && p < buf + ilen - 1 )
+        for( i = 0; i < ilen - 3; i++ )
-            pad_count += ( *p++ != 0 );
+        {
+            pad_done |= ( p[i] == 0 );
+            pad_count += ( pad_done == 0 );
+        }
 
-        correct &= ( *p == 0 && p < buf + ilen - 1 );
+        p += pad_count;
-
+        bad |= *p++; /* Must be zero */
-        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++;
     }
     else
     {
-        while( *p == 0xFF && p < buf + ilen - 1 )
+        bad |= *p++ ^ RSA_SIGN;
-            pad_count += ( *p++ == 0xFF );
 
-        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;
+        p += pad_count;
-
+        bad |= *p++; /* Must be zero */
-        // 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++;
     }
 
-    if( correct == 0 )
+    if( bad )
         return( POLARSSL_ERR_RSA_INVALID_PADDING );
 
     if (ilen - (p - buf) > output_max_len)