crypto: update root certificates #1261
Conversation
|
I tried this today but I found the tls access to I'm now investing it. |
|
Good catch, @shigeki. I think it has to do with one of the Equifax certificates that are now removed. |
|
Okay, so adding the Equifax Secure CA back in fixes it: diff --git a/src/node_root_certs.h b/src/node_root_certs.h
index e650ac3..7a13a02 100644
--- a/src/node_root_certs.h
+++ b/src/node_root_certs.h
@@ -120,6 +120,25 @@
"UPMxxY8BqHTr9Xgn2uf3ZkPznoM+IKrDNWCRzg==\n"
"-----END CERTIFICATE-----\n",
+/* Equifax Secure CA */
+"-----BEGIN CERTIFICATE-----\n"
+"MIIDIDCCAomgAwIBAgIENd70zzANBgkqhkiG9w0BAQUFADBOMQswCQYDVQQGEwJVUzEQMA4G\n"
+"A1UEChMHRXF1aWZheDEtMCsGA1UECxMkRXF1aWZheCBTZWN1cmUgQ2VydGlmaWNhdGUgQXV0\n"
+"aG9yaXR5MB4XDTk4MDgyMjE2NDE1MVoXDTE4MDgyMjE2NDE1MVowTjELMAkGA1UEBhMCVVMx\n"
+"EDAOBgNVBAoTB0VxdWlmYXgxLTArBgNVBAsTJEVxdWlmYXggU2VjdXJlIENlcnRpZmljYXRl\n"
+"IEF1dGhvcml0eTCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAwV2xWGcIYu6gmi0fCG2R\n"
+"FGiYCh7+2gRvE4RiIcPRfM6fBeC4AfBONOziipUEZKzxa1NfBbPLZ4C/QgKO/t0BCezhABRP\n"
+"/PvwDN1Dulsr4R+AcJkVV5MW8Q+XarfCaCMczE1ZMKxRHjuvK9buY0V7xdlfUNLjUA86iOe/\n"
+"FP3gx7kCAwEAAaOCAQkwggEFMHAGA1UdHwRpMGcwZaBjoGGkXzBdMQswCQYDVQQGEwJVUzEQ\n"
+"MA4GA1UEChMHRXF1aWZheDEtMCsGA1UECxMkRXF1aWZheCBTZWN1cmUgQ2VydGlmaWNhdGUg\n"
+"QXV0aG9yaXR5MQ0wCwYDVQQDEwRDUkwxMBoGA1UdEAQTMBGBDzIwMTgwODIyMTY0MTUxWjAL\n"
+"BgNVHQ8EBAMCAQYwHwYDVR0jBBgwFoAUSOZo+SvSspXXR9gjIBBPM5iQn9QwHQYDVR0OBBYE\n"
+"FEjmaPkr0rKV10fYIyAQTzOYkJ/UMAwGA1UdEwQFMAMBAf8wGgYJKoZIhvZ9B0EABA0wCxsF\n"
+"VjMuMGMDAgbAMA0GCSqGSIb3DQEBBQUAA4GBAFjOKer89961zgK5F7WF0bnj4JXMJTENAKaS\n"
+"bn+2kmOeUJXRmm/kEd5jhW6Y7qj/WsjTVbJmcVfewCHrPSqnI0kBBIZCe/zuf6IWUrVnZ9NA\n"
+"2zsmWLIodz2uFHdh1voqZiegDfqnc1zqcPGUIWVEX/r87yloqaKHee9570+sB3c4\n"
+"-----END CERTIFICATE-----\n",
+
/* Entrust.net Premium 2048 Secure Server CA */
"-----BEGIN CERTIFICATE-----\n"
"MIIEKjCCAxKgAwIBAgIEOGPe+DANBgkqhkiG9w0BAQUFADCBtDEUMBIGA1UEChMLRW50cnVz\n"I'll do some digging as to why it was removed. |
|
I suspect the cause may be more benign and we simply need to update the mk-ca-bundle.pl script. |
|
Yes, I think so. Some labels in certdata.txt are changed. It need to be updated from the latest script in the curl distribution. |
bnoordhuis
force-pushed
the
update-root-certs
branch
from
4c6c4d0 to
e2aebfa
Compare
|
I updated the mk-ca-bundle.pl script but we may be in a pickle. The reason the Equifax certificate is skipped is because of this (from its entry in certdata.txt): That is, it's trusted as a CA for EMAIL_PROTECTION but not SERVER_AUTH (which is the default filter.) When I run the script with Suggestions? |
|
|
||
| my $url = 'http://mxr.mozilla.org/mozilla/source/security/nss/lib/ckfw/builtins/certdata.txt?raw=1'; |
There was a problem hiding this comment.
There was some concern in nodejs/node-v0.x-archive#13860 that this wasn't https. Does this supersede that entirely?
There was a problem hiding this comment.
I don't know if 'supersede' is the right word because our version of the script doesn't download the file. It's up to the committer to download it to tools/certdata.txt.
|
I found that the issue caused by the alt cert chain problem in openssl. The certificate chain of www.google.com shows it has a cross root cert betwenn Equifax Secure Certificate Authority and GeoTrust Global CA at 2. Without alt chains, connecting to google.com are not verified. https://github.com/shigeki/io.js/tree/upgrade_openssl102_and_upgrade_rootCA is my branch update rootCA with openssl-1.0.2 with alt cert chain patch. I will work on openssl-1.0.2a tomorrow. I think it is better to update root CA after that. |
|
@shigeki Apropos openssl 1.0.2: I know we discussed bundling nasm or yasm but I looked at it this weekend and I think it may be possible to simply post-process the generated .s files to make them acceptable to GNU as. The only requirement is that gas / binutils is new enough to know about AVX-256. That probably means 2.23 or 2.24 as the minimum but that seems acceptable to me. |
|
@bnoordhuis nasm or yasm requirement is for building on Win32. IIRC, the current build requirements of iojs on Win32 are only python and Visual Studio. Are you planning to have an additional build requirement to install gas/binutils on Win32 or put them into |
|
@bnoordhuis @shigeki I suggest backporting: da084a5ec6 From OpenSSL master once we'll switch to 1.0.2, this should solve 1024bit CA problem once and for all. |
|
btw, LGTM! Good job! |
|
@indutny I backported the first and second one in shigeki@90368ee and shigeki@39128d6 but not the rest of two. Do you need the apps and doc patches too? |
|
oh @shigeki ! You are awesome. |
This commit: - fixes development branch (v1.x -> master) - updates stability index wording - use iojs binary instead of node PR-URL: nodejs#1466 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This reverts commit 06cfff9. Reverted because it introduced a regression where (because options were modified in the later functionality) options.host and options.port would be overridden with values provided in other, supported ways. PR-URL: nodejs#1467 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This commit adds a test to ensure all options are NOT modified after passing them to http.request. Specifically options.host and options.port are the most prominent that would previously error, but add the other options that have default values. options.host and options.port were overridden for the one-argument net.createConnection(options) call. PR-URL: nodejs#1467 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
This brings in the '%PYTHON%' revert, and restores the correct NODE_MODULE_VERSION. PR-URL: nodejs#1482 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
|
I planed to work this again in this week with the latest certdata.txt and deprecating 1024bit root certs. But mozilla has found an issue of Equifax 1024-bit roots as https://bugzilla.mozilla.org/show_bug.cgi?id=1155279 . Wait and see it for a moment. |
Update AUTHORS list using tools/update-authors.sh PR-URL: nodejs#1476 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#1503 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Evan Lucas <evanlucas@me.com>
as per nodejs#1502 PR-URL: nodejs#1507 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
tls.connect(options) with no options.host should accept a certificate with CN: 'localhost'. Fix Error: Hostname/IP doesn't match certificate's altnames: "Host: undefined. is not cert's CN: localhost" 'localhost' is not added directly to defaults because that is not always desired (for example, when using options.socket) PR-URL: nodejs#1493 Fixes: nodejs#1489 Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com> Reviewed-By: Roman Reiss <me@silverwind.io>
If `$NODE_PATH` contains trailing separators, `Module.globalPaths` will
contains empty strings. When `Module` try to resolve a module's path,
`path.resolve('', 'index.js')` will boil down to `$PWD/index.js`, which
makes sub modules can access global modules and get unexpected result.
PR-URL: nodejs#1488
Reviewed-By: Roman Reiss <me@silverwind.io>
Update the remaining markdown files to refer to the master branch. PR-URL: nodejs#1511 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
When buffer list less than 2, no need to calculate the length. The change's benchmark result is here: https://gist.github.com/JacksonTian/2c9e2bdec00018e010e6 It improve 15% ~ 25% speed when list only have one buffer, to other cases no effect. PR-URL: nodejs#1437 Reviewed-By: Trevor Norris <trev.norris@gmail.com> Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
Update AUTHORS list using tools/update-authors.sh PR-URL: nodejs#1776 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs#1763 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Every npm version bump requires a few patches to be floated on node-gyp for io.js compatibility. These patches are found in 03d1992, 5de334c, and da730c7. This commit squashes them into a single commit. PR-URL: nodejs#990 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
The delay-load hook allows node.exe/iojs.exe to be renamed. See efadffe for more background. This commit is a combined squash of the following previous patches: ba93c58, 3bda6cb, 0d6d3dd. PR-URL: nodejs#1763 Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs#1777 Notable Changes: * crypto: Diffie-Hellman key exchange (DHE) parameters ('dhparams') must now be 1024 bits or longer or an error will be thrown. A warning will also be printed to the console if you supply less than 2048 bits. See https://weakdh.org/ for further context on this security concern. (Shigeki Ohtsu) nodejs#1739. * node: A new --trace-sync-io command line flag will print a warning and a stack trace whenever a synchronous API is used. This can be used to track down synchronous calls that may be slowing down an application. (Trevor Norris) nodejs#1707. * node: To allow for chaining of methods, the setTimeout(), setKeepAlive(), setNoDelay(), ref() and unref() methods used in 'net', 'dgram', 'http', 'https' and 'tls' now return the current instance instead of undefined (Roman Reiss & Evan Lucas) nodejs#1699 nodejs#1768 nodejs#1779. * npm: Upgraded to v2.10.1, release notes can be found in https://github.com/npm/npm/releases/tag/v2.10.1 and https://github.com/npm/npm/releases/tag/v2.10.0. * util: A significant speed-up (in the order of 35%) for the common-case of a single string argument to util.format(), used by console.log() (Сковорода Никита Андреевич) nodejs#1749.
PR-URL: nodejs#1782 Reviewed-By: Roman Reiss <me@silverwind.io>
PR-URL: nodejs#1572 Reviewed-By: Chris Dickinson <christopher.s.dickinson@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Explicitly states the fact that no decoding is performed on the url path or pathname or the query string by default in the URL module. Fixes: nodejs#1538 PR-URL: nodejs#1731 Reviewed-By: Roman Reiss <me@silverwind.io>
This commit removes unnecessary nextTick() closures and adds some shared nextTick() callbacks for better re-use. PR-URL: nodejs#1612 Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com> Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com>
The non-isolate version of node::FatalException() is deprecated, switch to the version that takes an isolate as its first argument. PR-URL: nodejs#1793 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Commit 3e1b1dd ("Remove excessive copyright/license boilerplate") left in a few lines of boilerplate here and there. This commit removes them. PR-URL: nodejs#1793 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
The JS source files in test/addons/doc-*/ are scraped from the reference documentation in doc/api and need not conform to the style guide. PR-URL: nodejs#1793 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fixes: nodejs#1754 PR-URL: nodejs#1775 Reviewed-By: Brendan Ashworth <brendan.ashworth@me.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Defer evaluation of the script for a tick. This is a workaround for events not firing when evaluating scripts on the command line with -e. Fixes: nodejs#1600 PR-URL: nodejs#1793 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fixes: nodejs#999 PR-URL: nodejs#1014 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Replace calls to fs.statSync() with an internal variant that does not create Error or Stat objects that put strain on the garbage collector. A secondary benefit is that it improves start-up times in the debugger because it no longer emits thousands of exception debug events. PR-URL: nodejs#1801 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Replace calls to fs.readFileSync() with an internal variant that does not create Error objects on failure and is a bit speedier in general. A secondary benefit is that it improves start-up times in the debugger because it no longer emits thousands of exception debug events. On a medium-sized application[0], this commit and its predecessor reduce start-up times from about 1.5s to 0.5s and reduce the number of start-up exceptions from ~6100 to 32, half of them internal to the application. [0] https://github.com/strongloop/loopback-sample-app PR-URL: nodejs#1801 Reviewed-By: Trevor Norris <trev.norris@gmail.com>
Fixes: nodejs#1797 PR-URL: nodejs#1806 Reviewed-By: fishrock123@rocketmail.com
Remove unneeded functionality and tweak the generated output so we
can #include it in C++ source code.
This commit essentially reapplies the changes from commit e159073
("tools: customize mk-ca-bundle.pl") to the updated script.
This is the latest certdata.txt from [0], last updated on 2015-04-20. [0] https://hg.mozilla.org/mozilla-central/raw-file/1a2a200aeb40/security/nss/lib/ckfw/builtins/certdata.txt
Update the list of root certificates in src/node_root_certs.h with tools/mk-ca-bundle.pl.
bnoordhuis
force-pushed
the
update-root-certs
branch
from
e2aebfa to
acc2040
Compare
|
@nodejs/crypto I updated to the latest certdata.txt, PTAL. The target branch is wrong now because of the switch to master but it's the last four commits. @shigeki Do I understand correctly that bnoordhuis/io.js@a87651a ("crypto: add back deprecated 1024 bits root certs") is no longer necessary? I've dropped it for now but I can add it again if necessary. |
|
@bnoordhuis Is it better to open a new PR against current master? This PR is something strange that contains 173 commits. |
|
I wanted to keep the PR URL the same, to keep a backlink to the conversation from March. I guess it did become a little befuddled though. I'll file a new PR. |
R=@iojs/crypto
Question: when are we going to drop the remaining 1024 bits certificates?