Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

src: re-add 1024-bit SSL certs removed by f9456a2 #8904

Closed
wants to merge 1 commit into from

Conversation

chrisdickinson
Copy link

this fixes a problem where connecting to AWS services
would report an untrusted cert error.

This partially reverts f9456a2.

The ValiCert Class 1 VA is commented out because I was unable to verify it manually.

this fixes a problem where connecting to AWS services
would report an untrusted cert error.
@misterdjules
Copy link

I haven't looked in details to all the certs that f9456a2 removes from src/node_root_certs.j, but why don't we put back all of them instead of just the ones with a key length of 1024 bits? It seems that without re-adding all of them, we could run into other issues that haven't been reported yet.

Mozilla uses an interesting tests suite that goes through a lot of domains and tests for SSL errors. I wonder if we could use some variant of that before making such changes in the future.

@chrisdickinson
Copy link
Author

I added back the certs that were specifically mentioned by this blog post, because mozilla had clarified their intent behind removing them there & it was easy to verify. The only cert I didn't add back of that set was the SECOM ValiCert one because I couldn't verify the fingerprint.

Since I do not know what the intent was behind dropping the other certs, I did not feel safe adding them back in. It would be good to determine why those were dropped, but this PR was scoped to just replacing the 1024-bit certs that mozilla dropped as an fix for users connecting to AWS.

EDIT: that said, if we have time before cutting a release we should ensure that the other certs can be omitted.

For other readers, some more context behind this: this is a change to a generated file. The long term fix will almost certainly involve changing the perl script to also include grandfathered certs.

@misterdjules
Copy link

@chrisdickinson Ok, thanks for the clarification! @indutny @bnoordhuis, do you know why these other certificates were dropped?

@bnoordhuis
Copy link
Member

@misterdjules Various reasons, see here.

@misterdjules
Copy link

@bnoordhuis Thank you for the link!

@chrisdickinson Indeed, I think we should make sure that other certs can be omitted. Mozilla doesn't use the same SSL/TLS implementation, and what works for them doesn't necessarily always works for us as we've seen recently.

It's not clear though how we can test these changes thoroughly now. We could try to use Mozilla's tests suite, but it would certainly require some work that is difficult to estimate.

I would suggest reverting f9456a2 until we can test changes we make to the trusted certificates, or have a release candidate for the next stable release that could be tested in the wild before it gets actually released.

chrisdickinson added a commit that referenced this pull request Dec 20, 2014
this fixes a problem where connecting to AWS services
would report an untrusted cert error.

Fixes: #8894
PR-URL: #8904
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
@chrisdickinson
Copy link
Author

Merged in 1425ccd.

jacknagel pushed a commit to Homebrew/legacy-homebrew that referenced this pull request Dec 24, 2014
Version bump.

Deals primarily with the issue of Node refusing connections to AWS
services based on untrusted certs as of the last release.

See nodejs/node-v0.x-archive#8904 for detail.

Closes #35223.

Signed-off-by: Jack Nagel <jacknagel@gmail.com>
mscdex pushed a commit to mscdex/node that referenced this pull request Dec 25, 2014
this fixes a problem where connecting to AWS services
would report an untrusted cert error.

Fixes: nodejs#8894
PR-URL: nodejs#8904
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
jasnell pushed a commit to jasnell/node-joyent that referenced this pull request Jan 5, 2015
this fixes a problem where connecting to AWS services
would report an untrusted cert error.

Fixes: nodejs#8894
PR-URL: nodejs#8904
Reviewed-By: Timothy J Fontaine <tjfontaine@gmail.com>
Reviewed-By: Julien Gilli <julien.gilli@joyent.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants