On Wed, Nov 18, 2015 at 03:24:51PM +0000, Özgan, Tolgahan Jonas via RT wrote: > Dear List, > I have found a BUG in the function > " RSA_padding_check_PKCS1_type_1 " [...] > > the pointer p is incremented after the check therefore p is always the first > octet of the padded string. In the Case of PKCS1 type 1 padding always p=0, > hence the error. > Notes: > Changing the check to > if ((num != (flen+1)) || (*(++p) != 01)) > results also in a failure since the next check of p expects p to be "0xff" .
So the problem is that at least RSA_padding_check_PKCS1_type_1() and RSA_padding_check_PKCS1_type_2() are called with the 0 byte skipped internally in OpenSSL. This is because it's the result of a BN calculation, that is turned back into a char array, but it's not padded to RSA size. So the expected 0 byte is missing as first character. And the current functions depend on that behaviour. When RSA_padding_check_PKCS1_type_2() was updated to be most constant time, it internally pads it again, and then also checks that it's starts with the 0 byte. That commit also suggested to use BN_bn2bin_padded from BoringSSL. So we could change it so it works as you expect and that the 0 byte would be included. This could potentionally break other applications that try to use those functions, so maybe the 1.1 release might be the right time to change this. Looking at all sources in Debian I only find libdigidoc that tried to use RSA_padding_check_PKCS1_type_1 but it's commented out. For RSA_padding_check_PKCS1_type_2 I found yubico-piv-tool that has a comment about needing +1 (and so fails to check the 0), and mixmaster where like in the openssl internal cases it's a result of BN_bn2bin(). Kurt _______________________________________________ openssl-dev mailing list To unsubscribe: https://mta.openssl.org/mailman/listinfo/openssl-dev
