Bug in pkey_rsa_encrypt() and _decrypt()

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Bug in pkey_rsa_encrypt() and _decrypt()

Blumenthal, Uri - 0553 - MITLL

Working on pkcs11 engine, I discovered a bug in crypto/rsa/rsa_pmeth.c in pkey_rsa_encrypt() and pkey_rsa_decrypt().

 

They cause a crash when called with out==NULL. Normally it should not happen – but when an engine is called, and it cannot process the padding – it reverts to the original OpenSSL-provided pkey_rsa_encrypt() or pkey_rsa_decrypt() (as appropriate). OpenSSL pkeyutl makes two calls when the key is not directly available (aka not presented in a disk file), and the first call with out==NULL crashes when RSA_private_decrypt() or RSA_public_encrypt() tries to copy the result to out.

 

The fix should be adding something like

 

  if (out == NULL) {

       int klen = RSA_size(ctx->pkey->pkey.rsa);

       *outlen = klen;

       return 1;

 }

 

right before the call to RSA_public_encrypt().

 

P.S. It’s more critical in pkey_rsa_encrypt(), because it’s more likely that the engine would handle the decryption operation completely by itself.

--

Regards,

Uri Blumenthal


--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in pkey_rsa_encrypt() and _decrypt()

Richard Levitte - VMS Whacker-2
I think there's some confusion here...

OpenSSL's pkeyutl does indeed call something with out==NULL, but it's
not calling RSA_private_decrypt() or RSA_public_encrypt() directly,
it's calling the EVP_PKEY functions.  In *those* functions, there is a
check to see if the output argument is NULL and to return the
appropriate size in that case.  There is no promise that the lower
level functions called by the EVP_PKEY interface does the same.

As a matter of fact, the manual page for RSA_private_decrypt and
RSA_public_encrypt says this:

       ...
       RSA_public_encrypt() encrypts the flen bytes at from (usually a session
       key) using the public key rsa and stores the ciphertext in to. to must
       point to RSA_size(rsa) bytes of memory.
       ...
       RSA_private_decrypt() decrypts the flen bytes at from using the private
       key rsa and stores the plaintext in to. to must point to a memory
       section large enough to hold the decrypted data (which is smaller than
       RSA_size(rsa)). padding is the padding mode that was used to encrypt
       the data.
       ...

That should make it clear that a NULL output buffer isn't acceptable
at that level.

Cheers,
Richard

In message <[hidden email]> on Tue, 26 Sep 2017 20:29:11 +0000, "Blumenthal, Uri - 0553 - MITLL" <[hidden email]> said:

uri> Working on pkcs11 engine, I discovered a bug in crypto/rsa/rsa_pmeth.c in pkey_rsa_encrypt() and
uri> pkey_rsa_decrypt().
uri>
uri> They cause a crash when called with out==NULL. Normally it should not happen – but when an
uri> engine is called, and it cannot process the padding – it reverts to the original OpenSSL-provided
uri> pkey_rsa_encrypt() or pkey_rsa_decrypt() (as appropriate). OpenSSL pkeyutl makes two calls when
uri> the key is not directly available (aka not presented in a disk file), and the first call with out==NULL
uri> crashes when RSA_private_decrypt() or RSA_public_encrypt() tries to copy the result to out.
uri>
uri> The fix should be adding something like
uri>
uri> if (out == NULL) {
uri>
uri> int klen = RSA_size(ctx->pkey->pkey.rsa);
uri>
uri> *outlen = klen;
uri>
uri> return 1;
uri>
uri> }
uri>
uri> right before the call to RSA_public_encrypt().
uri>
uri> P.S. It’s more critical in pkey_rsa_encrypt(), because it’s more likely that the engine would handle
uri> the decryption operation completely by itself.
uri>
uri> --
uri>
uri> Regards,
uri>
uri> Uri Blumenthal
uri>
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Bug in pkey_rsa_encrypt() and _decrypt()

Dr. Stephen Henson
In reply to this post by Blumenthal, Uri - 0553 - MITLL
On Tue, Sep 26, 2017, Blumenthal, Uri - 0553 - MITLL wrote:

> Working on pkcs11 engine, I discovered a bug in crypto/rsa/rsa_pmeth.c in pkey_rsa_encrypt() and pkey_rsa_decrypt().
>
> They cause a crash when called with out==NULL. Normally it should not happen ??? but when an engine is called, and it cannot process the padding ??? it reverts to the original OpenSSL-provided pkey_rsa_encrypt() or pkey_rsa_decrypt() (as appropriate). OpenSSL pkeyutl makes two calls when the key is not directly available (aka not presented in a disk file), and the first call with out==NULL crashes when RSA_private_decrypt() or RSA_public_encrypt() tries to copy the result to out.
>

The original RSA pkey method has the flag EVP_PKEY_FLAG_AUTOARGLEN set which
handles the NULL output automatically so it is not handled in pkey_rsa_*().

The ENGINE should either set this flag itself too or deal with NULL arguments
manually if that is not appropriate.

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
Reply | Threaded
Open this post in threaded view
|

Re: Bug in pkey_rsa_encrypt() and _decrypt()

Blumenthal, Uri - 0553 - MITLL
    > Working on pkcs11 engine, I discovered a bug in crypto/rsa/rsa_pmeth.c in pkey_rsa_encrypt() and pkey_rsa_decrypt().
    >
    > They cause a crash when called with out==NULL. Normally it should not happen
    > but when an engine is called, and it cannot process the padding it reverts to the
    > original OpenSSL-provided pkey_rsa_encrypt() or pkey_rsa_decrypt() (as appropriate).

    The original RSA pkey method has the flag EVP_PKEY_FLAG_AUTOARGLEN set which
    handles the NULL output automatically so it is not handled in pkey_rsa_*().
   
    The ENGINE should either set this flag itself too or deal with NULL arguments
    manually if that is not appropriate.
   
Since hardware tokens I’m dealing with do not perform any public key operations (the engine in this case is used to merely pull and provide the public key  to the requestor) I’m somewhat ambivalent about writing engine Encrypt function specifically for handling the NULL argument case. On the one hand, it’s the simplest solution, and it avoids going through OpenSSL modification process.;) On the other hand, it’s not as clean as I’d like.

Where would I set this flag ? And would it work when the public key is on the token, and needs to be retrieved via engine?

--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev

smime.p7s (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Bug in pkey_rsa_encrypt() and _decrypt()

Dr. Stephen Henson
On Wed, Sep 27, 2017, Blumenthal, Uri - 0553 - MITLL wrote:

>     > Working on pkcs11 engine, I discovered a bug in crypto/rsa/rsa_pmeth.c in pkey_rsa_encrypt() and pkey_rsa_decrypt().
>     >
>     > They cause a crash when called with out==NULL. Normally it should not happen
>     > but when an engine is called, and it cannot process the padding it reverts to the
>     > original OpenSSL-provided pkey_rsa_encrypt() or pkey_rsa_decrypt() (as appropriate).
>
>     The original RSA pkey method has the flag EVP_PKEY_FLAG_AUTOARGLEN set which
>     handles the NULL output automatically so it is not handled in pkey_rsa_*().
>    
>     The ENGINE should either set this flag itself too or deal with NULL arguments
>     manually if that is not appropriate.
>    
> Since hardware tokens I???m dealing with do not perform any public key operations (the engine in this case is used to merely pull and provide the public key  to the requestor) I???m somewhat ambivalent about writing engine Encrypt function specifically for handling the NULL argument case. On the one hand, it???s the simplest solution, and it avoids going through OpenSSL modification process.;) On the other hand, it???s not as clean as I???d like.
>
> Where would I set this flag ? And would it work when the public key is on the token, and needs to be retrieved via engine?

It's set when the method is created via EVP_PKEY_meth_new(). If you set it it
assumes the public key components are set in the EVP_PKEY and calls
EVP_PKEY_size() appropriately to handle the NULL argument and if the supplied
buffer is too small.

Checkout the M_check_autoarg macro in crypto/evp/pmeth_fn.c to see exactly
what it does.

Steve.
--
Dr Stephen N. Henson. OpenSSL project core developer.
Commercial tech support now available see: http://www.openssl.org
--
openssl-dev mailing list
To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev