# PHP Crypt functions - security audit - Thread: https://externals.io/message/69147 - Original archive: https://news-web.php.net/php.internals/69147 - Started by Daniel Lowrey on 2013-09-16T13:58:38+00:00 - Messages: 24 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #69147 — Daniel Lowrey — 2013-09-16T13:58:38+00:00 URL: https://externals.io/message/69147#69147 > PHP itself doesn't do much crypto stuff. We rely mostly on libs like > openssl etc. and provide hashing algorithms which follow the > specifications. If the specifications are bad this is a global non-PHP > issue. That's all true, of course. But there are still places where new patches to the underlying libs (like openssl) get lost in translation and it's PHP's responsibility to add support. For example, about a year ago the CRIME attack vector ( https://community.qualys.com/blogs/securitylabs/2012/09/14/crime-information-leakage-attack-against-ssltls) was publicized. Preventing CRIME attacks is trival: disable compression at the TLS layer. The OpenSSL folks quickly provided the SSL_OP_NO_COMPRESSION constant in response, but it wasn't until a couple of months ago that I supplied a patch (http://lxr.php.net/xref/PHP_5_5/ext/openssl/xp_ssl.c#398) allowing a stream context option to disable compression on TLS streams. This means that for ~8-9 months any PHP code using the native encryption context options with `stream_socket_server()` was vulnerable. Now, I'm aware that very few developers use PHP to develop heavy-duty encrypted servers. Indeed there's a misconception that CLI PHP isn't capable of functioning in long-running server environments (not true). In such scenarios it doesn't help that PHP delegates the actual encryption to other libs under the hood -- we still need to implement security fixes as they become available. Perhaps more importantly, it's incumbent upon PHP to keep documentation up-to-date on these kinds of issues. The disable_compression ssl:// stream context option used to prevent the CRIME attack (above) is still not reflected in the php.net documentation ... *** THIS IS MY FAULT *** I should have submitted a docs patch as well, but failed to do so. I'll make that happen this week. More generally, PHP's stream encryption aspects are quite poorly documented. For example, https:// streams disable peer verification by default. While I understand that this is necessary to provide the easiest possible user experience for things like `file_get_contents(" https://somesite.com")`, it's also horribly insecure. 99% of people using tools like this won't know anything about this "feature" and won't realize that their stream transfers are totally vulnerable to Man-in-the-Middle attacks by default. IMO education and documentation is an important step we can take on the security front right now that's decoupled from any of the implementation code. Security should be treated proactively and not reactively and this is an area where improvement is necessary. I can't think of a better way to generate bad press for PHP than to have PHP code publicly exploited. That said, documenting things like this take time and we're all busy. I'll make an effort myself to improve documentation (at least with regard to standard SSL/TLS use-cases), but I'm only one person and I'm not a security expert by any means. I think it's important for anyone familiar with this area to also spend some time to see where the official documentation can be improved and explained. It's not enough to provide secure functionality -- we need to make sure its possible for people to understand how to implement that functionality in a secure way. --- ## #69217 — keisial@gmail.com — 2013-09-19T00:33:48+00:00 URL: https://externals.io/message/69147#69217 · In reply to: #69147 > More generally, PHP's stream encryption aspects are quite poorly > documented. For example, https:// streams disable peer verification by > default. While I understand that this is necessary to provide the easiest > possible user experience for things like `file_get_contents(" > https://somesite.com")`, it's also horribly insecure. 99% of people using > tools like this won't know anything about this "feature" and won't realize > that their stream transfers are totally vulnerable to Man-in-the-Middle > attacks by default. Count me as one of those that didn't know https:// streams didn't verify certificates. :) *I consider this a bug* I understand that it's easier to code not verifying the peer, and the hostname may not be available when you are stacking ssl over a stream. But file_get_contents("https://...") is *precisely* the case that should work right out of the box. --- ## #69219 — Tjerk Anne Meesters — 2013-09-19T01:06:56+00:00 URL: https://externals.io/message/69147#69219 · In reply to: #69217 > > > > […] > Count me as one of those that didn't know https:// streams didn't verify > certificates. :) > *I consider this a bug* I understand that it's easier to code not > verifying the > peer, and the hostname may not be available when you are stacking ssl over > a stream. > But file_get_contents("https://...**") is *precisely* the case that > should work right > out of the box. To be practical, verifying certificates requires an up-to-date CA bundle to be shipped with PHP; perhaps this is a simple thing to do, I'm not sure. This is an oft seen scenario for cURL; the developer would see the certificate issue, search online and continue with `CURLOPT_VERIFY_PEER => 0`. That said, at least cURL is configured to check the certificate by default. > > > > > -- > > > > -- -- Tjerk --- ## #69222 — Daniel Lowrey — 2013-09-19T02:51:02+00:00 URL: https://externals.io/message/69147#69222 · In reply to: #69219 > While we're on the topic, it's actually worse than that. Even if you > turn peer validation and name checking on, PHP can't handle > subjectAltNames in certificates, which causes quite a few failures. This is incorrect. PHP has supported both the "SNI_enabled" and "SNI_server_name" SSL context options since 5.3. Anything older than 5.3 is not remotely worth worrying over. You can verify this for yourself using the following code: [ 'verify_peer' => TRUE, 'cafile' => '/path/to/cacert.pem', // <-- change to point to an actual CA file 'SNI_enabled' => `TRUE` ]]); $uri = https://sni.velox.ch/ // <--- A test site using SNI certs $result = file_get_contents($uri, FALSE, $ctx); P.S. Thank you to whomever updated http://php.net/manual/en/context.ssl.phpto reflect the "disable_compression" SSL stream context option (and subsidized my laziness) :) On Wed, Sep 18, 2013 at 9:06 PM, Tjerk Anne Meesters wrote: > > > > > > > […] > > > To be practical, verifying certificates requires an up-to-date CA bundle > to be shipped with PHP; perhaps this is a simple thing to do, I'm not sure. > This is an oft seen scenario for cURL; the developer would see the > certificate issue, search online and continue with `CURLOPT_VERIFY_PEER => > 0`. That said, at least cURL is configured to check the certificate by > default. > > > > > […] > > > -- > -- > Tjerk --- ## #69223 — Pierre Joye — 2013-09-19T06:02:56+00:00 URL: https://externals.io/message/69147#69223 · In reply to: #69219 > > > > > […] easiest > > […] using > > […] realize > > […] over > > […] > > > To be practical, verifying certificates requires an up-to-date CA bundle to > be shipped with PHP; perhaps this is a simple thing to do, I'm not sure. > This is an oft seen scenario for cURL; the developer would see the > certificate issue, search online and continue with `CURLOPT_VERIFY_PEER => > 0`. That said, at least cURL is configured to check the certificate by > default. > FYI, curl allows to give the path to a cert db, it can be set in php.ini too (if I remember correctly) --- ## #69224 — Ryan McCue — 2013-09-19T06:23:55+00:00 URL: https://externals.io/message/69147#69224 · In reply to: #69222 Daniel Lowrey wrote: > This is incorrect. PHP has supported both the "SNI_enabled" and > "SNI_server_name" > SSL context options since 5.3. Anything older than 5.3 is not remotely > worth worrying over. You can verify this for yourself using the following > code: To be clear, I *don't* mean SNI, I mean subjectAltName (SAN) validation. The common name field (CN) contains the domain name for normal certificates, but you can have more than one domain per certificate. If you do that, it's stored in the subjectAltName (SAN) field. PHP only parses the CN, not the SAN, so any domain that isn't the main domain in the CN will fail. As per RFC 2818 (http://tools.ietf.org/html/rfc2818#section-3.1): > If a subjectAltName extension of type dNSName is present, that MUST > be used as the identity. Otherwise, the (most specific) Common Name > field in the Subject field of the certificate MUST be used. Although > the use of the Common Name is existing practice, it is deprecated and > Certification Authorities are encouraged to use the dNSName instead. -- Ryan McCue To be clear, I *don't* mean SNI, I mean subjectAltName (SAN) validation. Ah, apologies for the SNI/SAN confusion. However, I still cannot reproduce this failure when verifying peers with certs utilizing the SAN extension. Could you supply a code snippet demonstrating this failure? > Daniel Lowrey wrote: > > […] > > To be clear, I *don't* mean SNI, I mean subjectAltName (SAN) validation. > > The common name field (CN) contains the domain name for normal > certificates, but you can have more than one domain per certificate. If > you do that, it's stored in the subjectAltName (SAN) field. PHP only > parses the CN, not the SAN, so any domain that isn't the main domain in > the CN will fail. > > As per RFC 2818 (http://tools.ietf.org/html/rfc2818#section-3.1): > > > […] > > > -- > Ryan McCue > wrote: > To be practical, verifying certificates requires an up-to-date CA bundle > to be shipped with PHP; perhaps this is a simple thing to do, I'm not > sure. Unfortunately it isn't. It's easily possible to ship a current CA bundle *at the point when PHP is built/installed* but this needs to be *kept* up to date in order to remain useful. In the real world, people don't update production servers with every new release and the CA bundle that was correct at the time of print (as it were) would soon become outdated - although arguably an outdated bundle is better than nothing. IMHO the only real solution to this issue is education - the docs need some big fat red boxes along the lines of "in order for this to be useful you need an up to date CA bundle, it's dead easy to set up, here's how you do it". A fairly decent root CA bundle can be obtained and kept current with a simple cron/wget for http://curl.haxx.se/ca/cacert.pem - this is not the best source in the world but it is probably the simplest to explain to the common man. I figure that a CA bundle that may contain a couple of less than trustworthy certs (see http://sourceforge.net/p/curl/bugs/1178/) is better than completely disabling peer verification. On the same note, I suppose that shipping this bundle with PHP is better than nothing. > PHP can't handle subjectAltNames in certificates, which causes quite a > few failures. +1, I've been bitten by this a couple of times, it definitely needs fixing. > P.S. Thank you to whomever updated > http://php.net/manual/en/context.ssl.php to reflect the > "disable_compression" SSL stream context option (and subsidized my > laziness) :) You're welcome :-) You've got your work cut out breaking your own undocumented API, no time for updating other people's docs :-P > FYI, curl allows to give the path to a cert db, it can be set in php.ini > too (if I remember correctly) You do. Were PHP to be shipped with a default CA bundle, it would be nice to see the default ini files preconfigured with this, *however* PHP requires that curl.cafile be an absolute path, which could cause issues here. I don't know how difficult it would be to accept something that could be unambiguous without being absolute? I would suggest that if we were to ship a default CA bundle, it be included in the "extras" directory in the Windows builds. Thanks, Chris --- ## #69227 — Tjerk Anne Meesters — 2013-09-19T09:01:01+00:00 URL: https://externals.io/message/69147#69227 · In reply to: #69223 > > > > […] > wrote: > > […] > by > > […] > easiest > > […] > using > > […] > realize > > […] > Man-in-the-Middle > > […] > verify > > […] > over > > […] > to > > […] > => > > […] > > FYI, curl allows to give the path to a cert db, it can be set in php.ini > too (if I remember correctly) > Yes, I know that. This can also be done with the ca_file / ca_path context options when you use streams. My point is that you need a reasonably up-to-date certs bundle to enable verification by default. It could be impractical to ship such a bundle with php itself, in which case one might consider updating the documentation to highlight where such cert bundles can be downloaded from. -- -- Tjerk --- ## #69229 — Alain Williams — 2013-09-19T09:17:19+00:00 URL: https://externals.io/message/69147#69229 · In reply to: #69226 > On Thu, Sep 19, 2013 at 2:07 AM, Tjerk Anne Meesters > wrote: > > […] > > Unfortunately it isn't. It's easily possible to ship a current CA bundle > *at the point when PHP is built/installed* but this needs to be *kept* up > to date in order to remain useful. In the real world, people don't update > production servers with every new release and the CA bundle that was > correct at the time of print (as it were) would soon become outdated - > although arguably an outdated bundle is better than nothing. Agreed, however few people take PHP from the sources & compile it themselves. Most people will use the PHP that comes with their operating system and will expect their vendor/distributor to keep it up to date (security patches, etc). If the PHP project includes a CA bundle that is kept up to date then the vendor/distributors can update their customers as & when. It is not realistic to expect someone who is running a small web server to put the time into monitoring if they have up to date CA bundles; even if they understand what it is all about they probably don't have the time. This is what people expect the operating system vendor/distributor to do for them; end users will generally install updates when they become available, they generally have a hazy idea what these updates to -- that is OK, they have other things to worry about. What I am saying is that the PHP project should include a CA bundle, probably as a separately installable component that can be updated separately. This will help the vendors/distributors to push out updates to their users. -- Alain Williams Linux/GNU Consultant - Mail systems, Web sites, Networking, Programmer, IT Lecturer. +44 (0) 787 668 0256 http://www.phcomp.co.uk/ Parliament Hill Computers Ltd. Registration Information: http://www.phcomp.co.uk/contact.php #include My point is that you need a reasonably up-to-date certs bundle to enable verification by default. Actually, you don't. There is no reason why certificate validation cannot be enabled by default without a CA bundle. Yes, verifications will fail by default but this is no different than the cases where someone has an oddball provider or self-signed certificates; they have to manually add the cert for verification to pass. Additionally, given the current certificate climate, I wouldn't trust anything signed by the global CAs. If you're concerned about security, you should be validating the certificate fingerprint and not trusting CAs. Bryan --- ## #69234 — Daniel Lowrey — 2013-09-19T17:52:27+00:00 URL: https://externals.io/message/69147#69234 · In reply to: #69232 > If a subjectAltName extension of type dNSName is present, that MUST > be used as the identity. Otherwise, the (most specific) Common Name > field in the Subject field of the certificate MUST be used. Although > the use of the Common Name is existing practice, it is deprecated and > Certification Authorities are encouraged to use the dNSName instead. I have a working patch locally that adds SAN verification pursuant to RFC 2818 as outlined here (mentioned by Ryan): http://tools.ietf.org/html/rfc2818#section-3.1 Usage looks like this: ``` $ctx = stream_context_create(['ssl' => [ 'verify_peer' => TRUE, 'cafile' => '/path/to/cacert.pem', 'SAN_required' => TRUE, // default 'SAN_match' => 'www.github.com' 'CN_match' => 'www.github.com' ]]); $uri = 'https://www.github.com/'; file_get_contents($uri, FALSE, $ctx); ``` The patch is strict. I.E. it follows the spec and *will* fail your SSL negotiation attempt (with a descriptive error message to explain why) if the remote peer provided a list of SAN names and you didn't specify an `SN_match` stream context option. If no SAN dNSNames are supplied by the peer the routine will fallback to the CN_match if specified. This behavior is emphasized with a *MUST* in the RFC (meaning PHP's current implementation is NOT compliant). I'm happy to bring the default behavior into line with the spec but we need to determine if people are okay with making things secure by default. This seems like a no-brainer to me as IMHO security should be prioritized over simplicity: > *I consider this a bug* I understand that it's easier to code not verifying the > peer, and the hostname may not be available when you are stacking ssl over a stream. > But file_get_contents("https://...") is *precisely* the case that should work right > out of the box. ^^ This. Before I can fully/cleanly implement these changes we need to decide if PHP wants to move to a secure-by-default model for streams utilizing the built in encryption wrappers. Thoughts? --- ## #69235 — Adam Harvey — 2013-09-19T21:41:18+00:00 URL: https://externals.io/message/69147#69235 · In reply to: #69234 > > […] > > ^^ This. > > Before I can fully/cleanly implement these changes we need to decide > if PHP wants to move to a secure-by-default model for streams > utilizing the built in encryption wrappers. Thoughts? I think we should do this in 5.6. cURL has behaved this way for literally years at this point (verify by default, provide a switch to disable verification) and users seem to do just fine there. The much improved security story outweighs the (admittedly present) BC issues for mine. As for the CA bundle side of things, I wonder if this is one of those rare times where an ini setting might make sense, as opposed to actual bundling — that would allow distros to point to their packaged bundles without needing to patch php-src, and we could provide from-source installation instructions easily enough to point to common distro locations and the cURL download for users on more exotic OSes (like Windows). Adam --- ## #69236 — Daniel Lowrey — 2013-09-19T22:10:09+00:00 URL: https://externals.io/message/69147#69236 · In reply to: #69235 > I think we should do this in 5.6. +1 ... a renewed "emphasis on security" makes a good selling point when answering the "why should I upgrade" questions. At the same time, targeting the next minor version gives people ample time to plan/test/document changes. Secure stream encryption settings by default is a good place to start. > I wonder if this is one of those rare times where an ini setting might make sense I'm generally anti-.ini but this sounds sensible. --- ## #69237 — keisial@gmail.com — 2013-09-20T00:22:55+00:00 URL: https://externals.io/message/69147#69237 · In reply to: #69235 > As for the CA bundle side of things, I wonder if this is one of those > rare times where an ini setting might make sense, as opposed to actual > bundling — that would allow distros to point to their packaged bundles > without needing to patch php-src, and we could provide from-source > installation instructions easily enough to point to common distro > locations and the cURL download for users on more exotic OSes (like > Windows). > > Adam +1 --- ## #69238 — Pierre Joye — 2013-09-20T00:31:11+00:00 URL: https://externals.io/message/69147#69238 · In reply to: #69235 > > > […] > > I think we should do this in 5.6. cURL has behaved this way for > literally years at this point (verify by default, provide a switch to > disable verification) and users seem to do just fine there. The much > improved security story outweighs the (admittedly present) BC issues > for mine. > > As for the CA bundle side of things, I wonder if this is one of those > rare times where an ini setting might make sense, as opposed to actual > bundling — that would allow distros to point to their packaged bundles > without needing to patch php-src, and we could provide from-source > installation instructions easily enough to point to common distro > locations and the cURL download for users on more exotic OSes (like > Windows). Windows supports that very well, with Curl for example. It can also uses the OS certificates database. For the record here, curl has this setting already: http://us2.php.net/manual/en/curl.configuration.php#ini.curl.cainfo which is around for quite some time already. It could be possible to share it with openssl, but back then I did not check it out as only curl was concerned. One thing I do not remember off hand is if we actually enable cert validation per default with php's curl. It should be as we discussed that already many times. Cheers, -- Pierre @pierrejoye | http://www.libgd.org --- ## #69239 — Adam Harvey — 2013-09-20T00:38:46+00:00 URL: https://externals.io/message/69147#69239 · In reply to: #69238 > > > […] > > Windows supports that very well, with Curl for example. It can also > uses the OS certificates database. > > For the record here, curl has this setting already: > > http://us2.php.net/manual/en/curl.configuration.php#ini.curl.cainfo > > which is around for quite some time already. > > It could be possible to share it with openssl, but back then I did not > check it out as only curl was concerned. Is that something cURL provides, or that we do? A (very) cursory Google suggests that OpenSSL doesn't have support for the Windows certificate store natively, so we'd presumably have to patch that up (with a sensible default php.ini setting, if we went that way — "ssl.ca_bundle = win32", or something similar). > One thing I do not remember off hand is if we actually enable cert > validation per default with php's curl. It should be as we discussed > that already many times. We do. I checked before the first e-mail. :) Adam --- ## #69240 — Pierre Joye — 2013-09-20T00:41:58+00:00 URL: https://externals.io/message/69147#69240 · In reply to: #69239 > > > […] > > Is that something cURL provides, or that we do? A (very) cursory > Google suggests that OpenSSL doesn't have support for the Windows > certificate store natively, so we'd presumably have to patch that up > (with a sensible default php.ini setting, if we went that way — > "ssl.ca_bundle = win32", or something similar). It does when you use curl's win32 SSL support. That makes my previous point wrong as we do not compile it with this option but openssl (for cross platform compatibility reasons). But as the curl's ca file works just fine, everything is good. Would it make sense to share that option for openssl itself? > > […] > > We do. I checked before the first e-mail. :) Thanks :) -- Pierre @pierrejoye | http://www.libgd.org --- ## #69241 — Adam Harvey — 2013-09-20T00:48:26+00:00 URL: https://externals.io/message/69147#69241 · In reply to: #69240 > It does when you use curl's win32 SSL support. That makes my previous > point wrong as we do not compile it with this option but openssl (for > cross platform compatibility reasons). But as the curl's ca file works > just fine, everything is good. > > Would it make sense to share that option for openssl itself? I think so, particularly if we did make peer validation the default. Most Windows users would be happy to just use the system certificate store, I would think, so that would be one less thing to configure post-install. Adam --- ## #69244 — Tjerk Anne Meesters — 2013-09-20T07:24:19+00:00 URL: https://externals.io/message/69147#69244 · In reply to: #69232 Hi Bryan, On Thu, Sep 19, 2013 at 9:29 PM, Bryan C. Geraghty wrote: > -----Original Message----- > From: tjerk.meesters@gmail.com [mailto:tjerk.meesters@gmail.com] On Behalf > Of Tjerk Anne Meesters > Sent: Thursday, September 19, 2013 4:01 AM > > > […] > verification by default. > > Actually, you don't. There is no reason why certificate validation cannot > be > enabled by default without a CA bundle. Yes, verifications will fail by > default but this is no different than the cases where someone has an > oddball > provider or self-signed certificates; they have to manually add the cert > for > verification to pass. > For the most part, API libraries I've seen handle this scenario by shipping a certificate bundle with their code. The verification setting obviously does not apply to them, because they're handling it themselves or they don't use streams at all. However, I can imagine the bulk of developers will want the `file_get_contents('https://example.org')` to *just work* and flipping that switch between versions will likely upset a horde of them. > > Additionally, given the current certificate climate, I wouldn't trust > anything signed by the global CAs. If you're concerned about security, you > should be validating the certificate fingerprint and not trusting CAs. > That feature is currently not very well supported in streams. However, I'm working on a proposal to introduce [openssl_x509_digest()] which would make this possible: $context = stream_context_create([ 'ssl' => [ 'capture_peer_cert' => true, ]]); $f = fopen($url, 'rt', false, $context); $options = stream_context_get_options($context); openssl_x509_digest($options['ssl']['peer_certificate'], $result); echo "SHA1 fingerprint = ", bin2hex($result), "\n"; https://github.com/datibbaw/php-src/commit/b8f9a20286c6d88c11e81967df42d118bdba2f02 > > Bryan > > -- -- Tjerk --- ## #69255 — Daniel Lowrey — 2013-09-21T20:18:51+00:00 URL: https://externals.io/message/69147#69255 · In reply to: #69244 Hello security-conscious internals people! I've got (what believe to be) a pretty good working solution for the problem of insecure-by-default stream encryption. I need to do some more thorough testing before pushing it upstream to a public fork but here's the quick and dirty: --- Summary --- - All encrypted streams have peer verification enabled by default. - Global CA path defaults may be specified via new "openssl.cafile" and "openssl.capath" php.ini directives. This has the advantage mentioned upthread of allowing distros to customize the .ini file to point to an existing CA file. - Global CA path defaults may be specified at runtime via two new functions: + bool openssl_set_default_cafile(string $cafile) + bool openssl_set_default_capath(string $capath) - Per-stream CA paths may still be specified at call time by passing the existing "cafile" or "capath" ssl options in the stream context (the ini directives are a fallback) - If none of the above methods have been used to specify the necessary CA info you get a painfully explicit `E_WARNING` explaining that peer verification is necessary to prevent Man-in-the-Middle attacks and that you need to specify a CA or disable peer verification (gasp). - The peer name to be verified is automagically detected from the URI (eliminating the need for *any* configuration by the user for secure-by-default peer verification) - A new ssl context option, "peer_name" is added to allow manual override of the automatically detected name (can be useful if you're connecting via an IP address instead of a name). - Peer certs using the subjectAltName (SAN) extension are now honored when verifying peer names. Currently we're violating the relevant spec and not doing this at all. With this change a common name (CN) match is only attempted if no SAN matches were found first. This is a BC improvement that I'll PR for inclusion in 5.4/5.5 soon regardless of what happens with peer verification changes. --- BC ramifications --- The new "peer_name" context option (coupled with automatic name detection) totally invalidates the need for "CN_match" as well as "SNI_server_name" ... IMO these values are implementation details that are only necessary when fixing the problem of not verifying peers by default. My changes solve this problem and as a result I think the use of these context options should be deprecated and result in `E_NOTICE` for 5.6 with removal scheduled for 5.7. In the meantime these values, if passed in the context, will replace the "peer_name" value. Also, while I'm on the subject of the existing context options: I don't really see the point in keeping the "SNI_enabled" context option going forward either. If the underlying openssl libs support SNI then we should just use it automatically. Is there some benefit to disabling it when it's available that I'm unaware of? IMO "SNI_enabled" should receive the same deprecation treatment as "CN_match" and "SNI_server_name" ... As for existing code ... with the addition of a single line in php.ini almost all preexisting code will "just work." // way more secure, zero user knowledge needed, just works file_get_contents('https://www.github.com'); If you have problems you can always pass a context with "verify_peer" => FALSE. The difference is that now instead of being horribly insecure by default PHP would say, "okay, I'll let you do this if you really want to but understand that it's not safe." --- Other considerations --- It may be overkill to have *2* ini directives and setter functions (one for cafile and one for capath). I'm on the fence with this and would be fine with killing the capath directive/setter in the interest of keeping things simple. Finally, on the subject bundling a CA file with the distribution ... I don't care if we do or we don't. It's not that difficult to have a manual page dedicated to explaining why you need a CA file to verify peers in your encrypted transfers and offer instructions on how to procure one. Even if we don't bundle a cafile with the distro the hit to BC is extremely minimal and easy to remedy. Security is always a good reason to break BC. Alas, this turned into the neverending email, so thanks if you made it this far. Please share any thoughts, comments or questions. Or if you just want to tell me I'm an idiot that's fine too. I'll get the code on github in the next couple of days, but I want to incorporate any suggestions people may have first. --- ## #69256 — Nikita Popov — 2013-09-21T20:27:53+00:00 URL: https://externals.io/message/69147#69256 · In reply to: #69255 > Hello security-conscious internals people! > > I've got (what believe to be) a pretty good working solution for the > problem of insecure-by-default stream encryption. I need to do some more > thorough testing before pushing it upstream to a public fork but here's the > quick and dirty: > Sounds really great! One minor nitpick: - Global CA path defaults may be specified via new "openssl.cafile" and > "openssl.capath" php.ini directives. This has the advantage mentioned > upthread of allowing distros to customize the .ini file to point to an > existing CA file. > > - Global CA path defaults may be specified at runtime via two new > functions: > + bool openssl_set_default_cafile(string $cafile) > + bool openssl_set_default_capath(string $capath) > Why do we need these functions? Can't you just specify it with ini_set('openssl.cafile', $file)? I don't immediately see why we need additional functions to set those ini options. Nikita --- ## #69257 — Daniel Lowrey — 2013-09-21T20:32:17+00:00 URL: https://externals.io/message/69147#69257 · In reply to: #69256 Honestly that didn't even occur to me. I'd *much* rather kill the additional functions like you suggest. As long as I am not forced to use a PHP.ini to run I'm happy. `ini_set()` works fine for this and unless someone has a compelling reason not to I'll nix the functions. > > On Sat, Sep 21, 2013 at 10:18 PM, Daniel Lowrey > > […] > > > […] > > Sounds really great! One minor nitpick: > > - Global CA path defaults may be specified via new "openssl.cafile" and > > […] > > Why do we need these functions? Can't you just specify it with > ini_set('openssl.cafile', $file)? I don't immediately see why we need > additional functions to set those ini options. > > Nikita --- ## #69375 — Daniel Lowrey — 2013-09-27T03:59:54+00:00 URL: https://externals.io/message/69147#69375 · In reply to: #69255 --- SSL/TLS security progress update --- All previously discussed aspects of secure-by-default stream encryption are now implemented and tested. There are some other significant improvements including but not limited to full TLSv1.1+TLSv1.2 support and simplified peer fingerprint verification. As planned all changes are implemented with the minimum possible BC impact and require zero user-knowledge for secure implementations in userland. A rather exhaustive RFC is in the works and I hope to float it before too much longer. Consider yourselves updated. > Hello security-conscious internals people! > > I've got (what believe to be) a pretty good working solution for the > problem of insecure-by-default stream encryption. I need to do some more > thorough testing before pushing it upstream to a public fork but here's the > quick and dirty: > > --- Summary --- > > - All encrypted streams have peer verification enabled by default. > > - Global CA path defaults may be specified via new "openssl.cafile" and > "openssl.capath" php.ini directives. This has the advantage mentioned > upthread of allowing distros to customize the .ini file to point to an > existing CA file. > > - Global CA path defaults may be specified at runtime via two new > functions: > + bool openssl_set_default_cafile(string $cafile) > + bool openssl_set_default_capath(string $capath) > > - Per-stream CA paths may still be specified at call time by passing the > existing "cafile" or "capath" ssl options in the stream context (the ini > directives are a fallback) > > - If none of the above methods have been used to specify the necessary CA > info you get a painfully explicit `E_WARNING` explaining that peer > verification is necessary to prevent Man-in-the-Middle attacks and that you > need to specify a CA or disable peer verification (gasp). > > - The peer name to be verified is automagically detected from the URI > (eliminating the need for *any* configuration by the user for > secure-by-default peer verification) > > - A new ssl context option, "peer_name" is added to allow manual override > of the automatically detected name (can be useful if you're connecting via > an IP address instead of a name). > > - Peer certs using the subjectAltName (SAN) extension are now honored when > verifying peer names. Currently we're violating the relevant spec and not > doing this at all. With this change a common name (CN) match is only > attempted if no SAN matches were found first. This is a BC improvement that > I'll PR for inclusion in 5.4/5.5 soon regardless of what happens with peer > verification changes. > > --- BC ramifications --- > > The new "peer_name" context option (coupled with automatic name detection) > totally invalidates the need for "CN_match" as well as "SNI_server_name" > ... IMO these values are implementation details that are only necessary > when fixing the problem of not verifying peers by default. My changes solve > this problem and as a result I think the use of these context options > should be deprecated and result in `E_NOTICE` for 5.6 with removal scheduled > for 5.7. In the meantime these values, if passed in the context, will > replace the "peer_name" value. Also, while I'm on the subject of the > existing context options: I don't really see the point in keeping the > "SNI_enabled" context option going forward either. If the underlying > openssl libs support SNI then we should just use it automatically. Is there > some benefit to disabling it when it's available that I'm unaware of? IMO > "SNI_enabled" should receive the same deprecation treatment as "CN_match" > and "SNI_server_name" ... > > As for existing code ... with the addition of a single line in php.ini > almost all preexisting code will "just work." > > // way more secure, zero user knowledge needed, just works > file_get_contents('https://www.github.com'); > > If you have problems you can always pass a context with "verify_peer" => > FALSE. The difference is that now instead of being horribly insecure by > default PHP would say, "okay, I'll let you do this if you really want to > but understand that it's not safe." > > --- Other considerations --- > > It may be overkill to have *2* ini directives and setter functions (one > for cafile and one for capath). I'm on the fence with this and would be > fine with killing the capath directive/setter in the interest of keeping > things simple. Finally, on the subject bundling a CA file with the > distribution ... I don't care if we do or we don't. It's not that difficult > to have a manual page dedicated to explaining why you need a CA file to > verify peers in your encrypted transfers and offer instructions on how to > procure one. Even if we don't bundle a cafile with the distro the hit to BC > is extremely minimal and easy to remedy. Security is always a good reason > to break BC. > > Alas, this turned into the neverending email, so thanks if you made it > this far. Please share any thoughts, comments or questions. Or if you just > want to tell me I'm an idiot that's fine too. I'll get the code on github > in the next couple of days, but I want to incorporate any suggestions > people may have first.