improve OAEP check

diff --git a/CHANGES b/CHANGES
index a1294c5ace79652c6c46720f0376d7dd61ae3fed..b7a3b12485e58076ae4de964fd1a00414c438c32 100644 (file)
--- a/CHANGES
+++ b/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
 
          *) 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.
   +) 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
 
   +) Alter all existing ENGINE implementations (except "openssl" and
      "openbsd") to dynamically instantiate their own error strings. This

diff --git a/crypto/rsa/rsa_oaep.c b/crypto/rsa/rsa_oaep.c
index 62d2ed7e9fdbdfb9afe812c37bd15cadcff588e4..fb7ce08cfa6489eb19f229861bee5b28b52dc1e8 100644 (file)
--- a/crypto/rsa/rsa_oaep.c
+++ b/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);
                {
                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);
                }
 
        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);
        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;
                }
 
        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)
        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",
 #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(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];
        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];
        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)
 
        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 = 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;
        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);
        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);
                }
 
        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());
                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
                {
                goto decoding_err;
        else
                {


@@ -134,6 +147,8 @@ int RSA_padding_check_PKCS1_OAEP(unsigned char *to, int tlen,
                        goto decoding_err;
                else
                        {
                        goto decoding_err;
                else
                        {

+                       /* everything looks OK */
+

                        mlen = dblen - i;
                        if (tlen < mlen)
                                {
                        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;
                }
        OPENSSL_free(db);
        return mlen;

-       
+

 decoding_err:
        /* to avoid chosen ciphertext attacks, the error message should not reveal
         * which kind of decoding error happened */
 decoding_err:
        /* to avoid chosen ciphertext attacks, the error message should not reveal
         * which kind of decoding error happened */