RT3066: rewrite RSA padding checks to be slightly more constant time.

[openssl.git] / crypto / rsa / rsa_pk1.c

diff --git a/crypto/rsa/rsa_pk1.c b/crypto/rsa/rsa_pk1.c
index 8560755f1d12d8745f8272f2cd7441c225556677..e01fe62addf67f764f719e6127bfcb584107c0c8 100644 (file)
--- a/crypto/rsa/rsa_pk1.c
+++ b/crypto/rsa/rsa_pk1.c
@@ -56,6 +56,10 @@
  * [including the GNU Public Licence.]
  */
 
+#define OPENSSL_FIPSAPI
+
+#include "../constant_time_locl.h"
+
 #include <stdio.h>
 #include "cryptlib.h"
 #include <openssl/bn.h>
@@ -181,44 +185,87 @@ int RSA_padding_add_PKCS1_type_2(unsigned char *to, int tlen,
 int RSA_padding_check_PKCS1_type_2(unsigned char *to, int tlen,
             const unsigned char *from, int flen, int num)
        {
-       int i,j;
-       const unsigned char *p;
+       int i;
+       /* |em| is the encoded message, zero-padded to exactly |num| bytes */
+       unsigned char *em = NULL;
+       unsigned int good, found_zero_byte;
+       int zero_index = 0, msg_index, mlen = -1;
 
-       p=from;
-       if ((num != (flen+1)) || (*(p++) != 02))
-               {
-               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BLOCK_TYPE_IS_NOT_02);
-               return(-1);
-               }
-#ifdef PKCS1_CHECK
-       return(num-11);
-#endif
+        if (tlen < 0 || flen < 0)
+               return -1;
 
-       /* scan over padding data */
-       j=flen-1; /* one for type. */
-       for (i=0; i<j; i++)
-               if (*(p++) == 0) break;
+       /* PKCS#1 v1.5 decryption. See "PKCS #1 v2.2: RSA Cryptography
+        * Standard", section 7.2.2. */
 
-       if (i == j)
+       if (flen > num)
+               goto err;
+
+       if (num < 11)
+               goto err;
+
+       em = OPENSSL_malloc(num);
+       if (em == NULL)
                {
-               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_NULL_BEFORE_BLOCK_MISSING);
-               return(-1);
+               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, ERR_R_MALLOC_FAILURE);
+               return -1;
                }
+       memset(em, 0, num);
+       /*
+        * Always do this zero-padding copy (even when num == flen) to avoid
+        * leaking that information. The copy still leaks some side-channel
+        * information, but it's impossible to have a fixed  memory access
+        * pattern since we can't read out of the bounds of |from|.
+        *
+        * TODO(emilia): Consider porting BN_bn2bin_padded from BoringSSL.
+        */
+       memcpy(em + num - flen, from, flen);
 
-       if (i < 8)
+       good = constant_time_is_zero(em[0]);
+       good &= constant_time_eq(em[1], 2);
+
+       found_zero_byte = 0;
+       for (i = 2; i < num; i++)
                {
-               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_BAD_PAD_BYTE_COUNT);
-               return(-1);
+               unsigned int equals0 = constant_time_is_zero(em[i]);
+               zero_index = constant_time_select_int(~found_zero_byte & equals0, i, zero_index);
+               found_zero_byte |= equals0;
                }
-       i++; /* Skip over the '\0' */
-       j-=i;
-       if (j > tlen)
+
+       /*
+        * PS must be at least 8 bytes long, and it starts two bytes into |em|.
+         * If we never found a 0-byte, then |zero_index| is 0 and the check
+        * also fails.
+        */
+       good &= constant_time_ge((unsigned int)(zero_index), 2 + 8);
+
+       /* Skip the zero byte. This is incorrect if we never found a zero-byte
+        * but in this case we also do not copy the message out. */
+       msg_index = zero_index + 1;
+       mlen = num - msg_index;
+
+       /* For good measure, do this check in constant time as well; it could
+        * leak something if |tlen| was assuming valid padding. */
+       good &= constant_time_ge((unsigned int)(tlen), (unsigned int)(mlen));
+
+       /*
+        * We can't continue in constant-time because we need to copy the result
+        * and we cannot fake its length. This unavoidably leaks timing
+        * information at the API boundary.
+        * TODO(emilia): this could be addressed at the call site,
+        * see BoringSSL commit 0aa0767340baf925bda4804882aab0cb974b2d26.
+        */
+       if (!good)
                {
-               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2,RSA_R_DATA_TOO_LARGE);
-               return(-1);
+               mlen = -1;
+               goto err;
                }
-       memcpy(to,p,(unsigned int)j);
 
-       return(j);
-       }
+       memcpy(to, em + msg_index, mlen);
 
+err:
+       if (em != NULL)
+               OPENSSL_free(em);
+       if (mlen == -1)
+               RSAerr(RSA_F_RSA_PADDING_CHECK_PKCS1_TYPE_2, RSA_R_PKCS_DECODING_ERROR);
+       return mlen;
+       }