Closed mbrand1 closed 8 years ago
Happy New Year!
Happy New Year! And thank you for the wonderful 2016 gift!
I'm reviewing this now, but it looks generally good to me. I've updated the test suite on master
to be current and consistent. I'd like to merge this simultaneously with merging a test for an IPv6 domain too.
It looks like Travis CI supports IPv6 only in legacy containers, so I may need to add sudo: true
to the .travis.yml
file to get that to work.
I'll update with more comments if I find anything else.
It looks like this implementation isn't using SNI with the given hostname. In the current implementation, the openssl
command includes a -servername
argument that asks openssl
to also tell the server it's looking for the cert for the hostname being evaluated.
I don't have a SNI-specific test, but in my new test suite, I'm using sha1-2017.badssl.com
as a hostname we can expect to always use SHA-1. It also seems to require SNI, as I don't get the SHA-1 cert using the new tls
-based implementation as I do with the openssl
implementation.
Also -- I've got a branch, here in the main repo, called refactor-tls
, that has your PR branch rebased against the new master
, with a couple additional small commits.
OK, the SNI fix was easy: https://github.com/konklone/shaaaaaaaaaaaaa/commit/ea01b6b50eee53bfed2dc3264e445ab6973aecbe
But now that I'm through that, the tests are failing because the responses for sites include the root certificate, even when the site does not serve the root certificate. See konklone.com
or sha1-2017.badssl.com
for examples:
Output for konklone.com
in the new code:
{
"domain": "konklone.com",
"cert": {
"algorithm": "sha256",
"raw": "sha256WithRSAEncryption",
"good": true,
"root": false,
"expires": "2016-02-14T04:06:00.000Z",
"name": "konklone.com"
},
"intermediates": [
{
"algorithm": "sha256",
"raw": "sha256WithRSAEncryption",
"good": true,
"root": false,
"expires": "2020-10-19T22:33:36.000Z",
"name": "Let's Encrypt Authority X1"
},
{
"algorithm": "sha1",
"raw": "sha1WithRSAEncryption",
"good": true,
"root": true,
"replacement": null,
"expires": "2021-09-30T14:01:15.000Z",
"name": "DST Root CA X3"
}
],
"diagnosis": "good"
}
Under the old code, the DST Root CA X3
intermediate was missing.
So to sum up:
refactor-tls
branch here has your work, plus a rebase plus some extra commits.refactor-tls
.One thing I'm concerned about, since it's returning a root cert that the server did not serve, is that the intermediates may also differ. There are OS-specific chain-building bugs that Chrome surfaced during its aggressive deprecation that can cause clients to form chains with SHA-1 intermediates the server did not actually publish.
I want the SHAAAAAAAAAAAAA site and API to evaluate the served chain, not the chain that some clients may accidentally construct despite the server's best intentions. Can you look into whether that's possible with the tls
approach?
(Also, this PR motivated me to go enable IPv6 on my Digital Ocean boxes, which was non-trivial but feels great to have done.)
Thanks for all the feedback. I'll go through this later today. And checkout the refactor-tls branch. So the output is including a cert that openssl s_client does not? Hmm...
Ok, so it looks like the ca: option to tls.connect is forcing the addition of the root. I had to stick something in there to make it stop. Null and empty string won't work. I'll push merge request to refactor-tls so you'll see what I mean. Seems to do the trick.
This code, via #86 and #87, is now up and deployed: https://shaaaaaaaaaaaaa.com/check/ipv6.google.com
Happy New Year!
Well, I've cracked it. I was able to remove the dependence on the direct call to OpenSSL. Instead, I'm using the tls module to make the network connection.
TLS returns DER-encoded versions of the certificates using tls.getPeerCertificate().
I base64 encode that to put it back into PEM format so x509 can read it and parse the signature algorithm.
I started this because I wanted IPv6 support in my Chrome extension and traced this back to OpenSSL. The command line tool does not support IPv6 addresses. There's a 10 year old thread and several people have proposed patches. None have been merged!
But nevermind that. Now we have IPv6 support! Enable it on your Digital Ocean image for some IPv6-goodness.
I have a development image running this code here: http://shaaaaaaaaaaaaa.devel.netsville.com/
Here it is successfully checking ipv6.google.com:
http://shaaaaaaaaaaaaa.devel.netsville.com/api/check/ipv6.google.com