improve OAEP check

CHANGES

@@ -12,11 +12,18 @@
          *) applies to 0.9.6a/0.9.6b/0.9.6c and 0.9.7
          +) applies to 0.9.7 only
 
+  *) Improve RSA_padding_check_PKCS1_OAEP() check again to avoid
+     'wristwatch attack' using huge encoding parameters (cf.
+     James H. Manger's CRYPTO 2001 paper).  Note that the
+     RSA_PKCS1_OAEP_PADDING case of RSA_private_decrypt() does not use
+     encoding paramters and hence was not vulnerable.
+     [Bodo Moeller]
+
   +) Add a "destroy" handler to ENGINEs that allows structural cleanup to
      be done prior to destruction. Use this to unload error strings from
      ENGINEs that load their own error strings. NB: This adds two new API
      functions to "get" and "set" this destroy handler in an ENGINE.
-     [Geoff]
+     [Geoff Thorpe]
 
   +) Alter all existing ENGINE implementations (except "openssl" and
      "openbsd") to dynamically instantiate their own error strings. This

crypto/rsa/rsa_oaep.c

@@ -43,20 +43,20 @@ int RSA_padding_add_PKCS1_OAEP(unsigned char *to, int tlen,
 		{
 		RSAerr(RSA_F_RSA_PADDING_ADD_PKCS1_OAEP,
 		   RSA_R_DATA_TOO_LARGE_FOR_KEY_SIZE);
-		return (0);
+		return 0;
 		}
 
 	if (emlen < 2 * SHA_DIGEST_LENGTH + 1)
 		{
 		RSAerr(RSA_F_RSA_PADDING_ADD_PKCS1_OAEP, RSA_R_KEY_SIZE_TOO_SMALL);
-		return (0);
+		return 0;
 		}
-	
+
 	dbmask = OPENSSL_malloc(emlen - SHA_DIGEST_LENGTH);
 	if (dbmask == NULL)
 		{
 		RSAerr(RSA_F_RSA_PADDING_ADD_PKCS1_OAEP, ERR_R_MALLOC_FAILURE);
-		return (0);
+		return 0;
 		}
 
 	to[0] = 0;
@@ -69,7 +69,7 @@ int RSA_padding_add_PKCS1_OAEP(unsigned char *to, int tlen,
 	db[emlen - flen - SHA_DIGEST_LENGTH - 1] = 0x01;
 	memcpy(db + emlen - flen - SHA_DIGEST_LENGTH, from, (unsigned int) flen);
 	if (RAND_bytes(seed, SHA_DIGEST_LENGTH) <= 0)
-		return (0);
+		return 0;
 #ifdef PKCS_TESTVECT
 	memcpy(seed,
 	   "\xaa\xfd\x12\xf6\x59\xca\xe6\x34\x89\xb4\x79\xe5\x07\x6d\xde\xc2\xf0\x6c\xb5\x8f",
@@ -79,7 +79,7 @@ int RSA_padding_add_PKCS1_OAEP(unsigned char *to, int tlen,
 	MGF1(dbmask, emlen - SHA_DIGEST_LENGTH, seed, SHA_DIGEST_LENGTH);
 	for (i = 0; i < emlen - SHA_DIGEST_LENGTH; i++)
 		db[i] ^= dbmask[i];
-	
+
 	MGF1(seedmask, SHA_DIGEST_LENGTH, db, emlen - SHA_DIGEST_LENGTH);
 	for (i = 0; i < SHA_DIGEST_LENGTH; i++)
 		seed[i] ^= seedmask[i];
@@ -96,21 +96,34 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
 	const unsigned char *maskeddb;
 	int lzero;
 	unsigned char *db = NULL, seed[SHA_DIGEST_LENGTH], phash[SHA_DIGEST_LENGTH];
+	int bad = 0;
 
 	if (--num < 2 * SHA_DIGEST_LENGTH + 1)
+		/* 'num' is the length of the modulus, i.e. does not depend on the
+		 * particular ciphertext. */
 		goto decoding_err;
 
 	lzero = num - flen;
 	if (lzero < 0)
-		goto decoding_err;
+		{
+		/* lzero == -1 */
+
+		/* signalling this error immediately after detection might allow
+		 * for side-channel attacks (e.g. timing if 'plen' is huge
+		 * -- cf. James H. Manger, "A Chosen Ciphertext Attack on RSA Optimal
+		 * Asymmetric Encryption Padding (OAEP) [...]", CRYPTO 2001),
+		 * so we use a 'bad' flag */
+		bad = 1;
+		lzero = 0;
+		}
 	maskeddb = from - lzero + SHA_DIGEST_LENGTH;
-	
+
 	dblen = num - SHA_DIGEST_LENGTH;
 	db = OPENSSL_malloc(dblen);
 	if (db == NULL)
 		{
 		RSAerr(RSA_F_RSA_PADDING_ADD_PKCS1_OAEP, ERR_R_MALLOC_FAILURE);
-		return (-1);
+		return -1;
 		}
 
 	MGF1(seed, SHA_DIGEST_LENGTH, maskeddb, dblen);
@@ -122,8 +135,8 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
 		db[i] ^= maskeddb[i];
 
 	EVP_Digest((void *)param, plen, phash, NULL, EVP_sha1());
-	
+
-	if (memcmp(db, phash, SHA_DIGEST_LENGTH) != 0)
+	if (memcmp(db, phash, SHA_DIGEST_LENGTH) != 0 || bad)
 		goto decoding_err;
 	else
 		{
@@ -134,6 +147,8 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
 			goto decoding_err;
 		else
 			{
+			/* everything looks OK */
+
 			mlen = dblen - i;
 			if (tlen < mlen)
 				{
@@ -146,7 +161,7 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
 		}
 	OPENSSL_free(db);
 	return mlen;
-	
+
 decoding_err:
 	/* to avoid chosen ciphertext attacks, the error message should not reveal
 	 * which kind of decoding error happened */