Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » OpenSSL - New Defaults »

OpenSSL - New Defaults

php.internals

Scott Arciszewski

9 years ago
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/89853c180c85c71baac7015cb77ff8ddae129942/src/Portunus/Crypt/RSA/PrivateKey.php#L20 - https://github.com/NorseBlue/Sikker/blob/c158bab1e676d751e5228cb17ecf9593c6b94e95/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. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises <https://paragonie.com>

Jakub Zelenka

9 years ago
Hi, On Thu, Nov 3, 2016 at 4:11 PM, Scott Arciszewski <scott@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

Niklas Keller

9 years ago
2016-11-06 20:19 GMT+01:00 Jakub Zelenka <bukka@php.net>:
> Hi, > > On Thu, Nov 3, 2016 at 4:11 PM, Scott Arciszewski <scott@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 >
Huge +1 for that plan! Regards, Niklas

Scott Arciszewski

9 years ago
On Sun, Nov 6, 2016 at 2:19 PM, Jakub Zelenka <bukka@php.net> wrote:
> Hi, > > On Thu, Nov 3, 2016 at 4:11 PM, Scott Arciszewski <scott@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/89853c180c85c71baac7015cb77ff8ddae129942/src/Portunus/Crypt/RSA/PrivateKey.php#L20 >> - >> >> https://github.com/NorseBlue/Sikker/blob/c158bab1e676d751e5228cb17ecf9593c6b94e95/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 > >
Hi Jakub,
> 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.
A sane proposal had emerged before I even received this email, via kemmeta on Reddit: https://www.reddit.com/r/PHP/comments/5axmn5/phpinternals_openssl_new_defaults_proposal/d9kx8sa/?context=1 A backwards-compatible interface would default openssl_public_encrypt() to OAEP, but openssl_private_decrypt() would default to a new constant: OPENSSL_PKCS1_UNSPECIFIED, which when passed would first try OAEP and, if a padding error occurred, emit an E_DEPRECATED and fallback to PKCS1v1.5 (a.k.a. "let's pretend Daniel Bleichenbacher's research doesn't exist" mode). The upside is that better security would be applied opportunistically in codebases where the PHP developer was not a cryptographer. The downside is a performance hit unless you specify one mode or another. A long term solution would be to not even use RSA anymore.
> 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.
If your plan is to get it more in line with libsodium's crypto_box_seal API, I'd love to see it. Scott Arciszewski Chief Development Officer Paragon Initiative Enterprises

Nikita

9 years ago
> On 7 Nov 2016, at 03:35, Scott Arciszewski <scott@paragonie.com> wrote: > >> On Sun, Nov 6, 2016 at 2:19 PM, Jakub Zelenka <bukka@php.net> wrote: >> Hi, >> >> On Thu, Nov 3, 2016 at 4:11 PM, Scott Arciszewski <scott@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/89853c180c85c71baac7015cb77ff8ddae129942/src/Portunus/Crypt/RSA/PrivateKey.php#L20 >>> - >>> >>> https://github.com/NorseBlue/Sikker/blob/c158bab1e676d751e5228cb17ecf9593c6b94e95/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 >> >> > > Hi Jakub, > >> 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. > > A sane proposal had emerged before I even received this email, via > kemmeta on Reddit: > > https://www.reddit.com/r/PHP/comments/5axmn5/phpinternals_openssl_new_defaults_proposal/d9kx8sa/?context=1 > > A backwards-compatible interface would default > openssl_public_encrypt() to OAEP, but openssl_private_decrypt() would > default to a new constant: OPENSSL_PKCS1_UNSPECIFIED, which when > passed would first try OAEP and, if a padding error occurred, emit an > E_DEPRECATED and fallback to PKCS1v1.5 (a.k.a. "let's pretend Daniel > Bleichenbacher's research doesn't exist" mode). > > The upside is that better security would be applied opportunistically > in codebases where the PHP developer was not a cryptographer. The > downside is a performance hit unless you specify one mode or another. > > A long term solution would be to not even use RSA anymore. > >> 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. > > If your plan is to get it more in line with libsodium's > crypto_box_seal API, I'd love to see it. > > Scott Arciszewski > Chief Development Officer > Paragon Initiative Enterprises > > -- > PHP Internals - PHP Runtime Development Mailing List > To unsubscribe, visit: http://www.php.net/unsub.php >
Hey, It might make even more sense to not provide a default here at all. As history shows that those methods that are considered secure today can become less-than-desirably secure in a couple of years. Which means the same cycle of deprecation/changing defaults in years to come.

Alice Wonder

9 years ago
On 11/07/2016 04:29 AM, Nikita Nefedov wrote: *snip*
>> > > > Hey, > > It might make even more sense to not provide a default here at all. As history shows that those methods that are considered secure today can become less-than-desirably secure in a couple of years. Which means the same cycle of deprecation/changing defaults in years to come. >
100% agree One of the problems that OpenSSL had is that it kept too much for compatability with software that hadn't updated to use more modern more secure protocols. Hence it supported SSLv3 for forever even long after it was no longer really needed, supported things like heartbeat enabled by default even though less than 1% of users had any need for the extension, etc. Keeping things secure means breaking software that doesn't keep up to date. That's just the way of things. I confess it bothers me that mcrypt is only being depricated in 7.1 and not removed, for example, and that there are plans to continue it as a PECL module. It's a dead project, dead security projects are not secure. Using defaults can be dangerous too, when a system administrator doesn't have to think about it because the defaults "work" they often don't, even though the way the defaults "work" is often less than ideal. All the dangerous WordPress plugins out there are ample evidence of that, where they default to executing shell commands so that they "work" in the event the server doesn't have the right PHP extension available - or all the php image processing modules using ImageMagick that by default worked with everything under the sun and thus were vulnerable to CVE-2016–3714 when they could have (should have) required the admin specify a whitelist of image types it accepted. Anyway that's my viewpoint - screw compatibility when it comes to security, and don't use defaults that can cause security issues when the admin chooses not to think about the security implications. Hope this isn't just seen as noise, if it is, I apologize.

Fleshgrinder

9 years ago
On 11/7/2016 3:41 PM, Alice Wonder wrote:
> On 11/07/2016 04:29 AM, Nikita Nefedov wrote: >> It might make even more sense to not provide a default here at all. As >> history shows that those methods that are considered secure today can >> become less-than-desirably secure in a couple of years. Which means >> the same cycle of deprecation/changing defaults in years to come. >> > > 100% agree > > One of the problems that OpenSSL had is that it kept too much for > compatability with software that hadn't updated to use more modern more > secure protocols. Hence it supported SSLv3 for forever even long after > it was no longer really needed, supported things like heartbeat enabled > by default even though less than 1% of users had any need for the > extension, etc. > > Keeping things secure means breaking software that doesn't keep up to > date. That's just the way of things. > > I confess it bothers me that mcrypt is only being depricated in 7.1 and > not removed, for example, and that there are plans to continue it as a > PECL module. It's a dead project, dead security projects are not secure. > > Using defaults can be dangerous too, when a system administrator doesn't > have to think about it because the defaults "work" they often don't, > even though the way the defaults "work" is often less than ideal. > > All the dangerous WordPress plugins out there are ample evidence of > that, where they default to executing shell commands so that they "work" > in the event the server doesn't have the right PHP extension available - > or all the php image processing modules using ImageMagick that by > default worked with everything under the sun and thus were vulnerable to > CVE-2016–3714 when they could have (should have) required the admin > specify a whitelist of image types it accepted. > > Anyway that's my viewpoint - screw compatibility when it comes to > security, and don't use defaults that can cause security issues when the > admin chooses not to think about the security implications. > > Hope this isn't just seen as noise, if it is, I apologize. >
I agree and disagree. Security related defaults should be changed without worrying too much about breaking changes if the old approach is really problematic. The automated approach of trying to recover, as proposed, seems like a great idea. Ignore the performance hit completely and remove this fallback in the next minor version where it simply does not work anymore. However, having defaults makes perfect sense. Nobody will invest more time into finding what is more secure just because there is no default. They will use whatever they find first. Additionally it's just shifting of responsibility from the language/library to the developer. If a developer relies on algo-insecure as default value or algo-insecure as self chosen strategy does not change the fact that algo-insecure is used instead of algo-secure. Algo-secure might be the default for the next release of said language/library, however, if that language/library is not updated it will never be used. The same is true for the hard coded algo-insecure from the developer. BUT and this is a huge BUT. It is more likely that the developer updates the language/library than the various usages of the hardcoded algo-insecure everywhere in the code. Code, that he or she has long forgotten!
-- Richard "Fleshgrinder" Fussenegger