Hi, On Thu, Nov 3, 2016 at 4:11 PM, Scott Arciszewski <sc...@paragonie.com> wrote:
> Hi, > > Can we change openssl_public_encrypt() and openssl_private_decrypt() from > defaulting to PKCS1v1.5 padding, in favor of defaulting to OAEP? > > I'll create an RFC for this later. It will just prevent a lot of issues. > > To wit: > > - https://framework.zend.com/security/advisory/ZF2015-10 > - > https://github.com/garyr/portunus/blob/89853c180c85c71baac7015cb77ff8 > ddae129942/src/Portunus/Crypt/RSA/PrivateKey.php#L20 > - > https://github.com/NorseBlue/Sikker/blob/c158bab1e676d751e5228cb17ecf95 > 93c6b94e95/src/Asymmetric/Keys/PrivateKey.php#L72 > > If we can't stop PHP developers who aren't cryptographers from writing > their own high-level RSA implementation, we can at least make it safer by > default. > > We can't change default in 7.x as it would be a BC break and in this case a very hard to catch BC break as the only way how we could let user know about that is a documentation. This is a very low level function and PKCSv1.5 is still used a lot so some apps might depend on. I think that the default is bad but I don't think that breaking both function without notice is a good solution as users might not be able to catch it especially if it's some third party protocol that is not very secure but you don't have choice because 3rd party won't change it (that was actually the only case where I used these functions in practice so it's not made up). And even worse if you don't have tests, then it breaks just production without a warning... What I would prefer instead is to deprecate calling openssl_(private|public)_(en|de)crypt without a padding parameter. The default would stay the same but user would get a deprecation message and the parameter would become required in PHP 8. We should add a note to the doc, to let the user know what the safest option is. It means OAEP for openssl_private_decrypt and openssl_public_encrypt. In case of openssl_public_decrypt and openssl_private_encrypt, we support just PKCS1v1.5 but if we add support for PSS (would require a little bit of tweaking as it works on EVP level only but might add it later), we could easily recommend it then. I have got actually very similar plan for openssl_seal and openssl_open that have default method rc4. Again very bad default but the best way how to let user know is to deprecate it rather than choose new default and break compatibility. Cheers Jakub
