konklone / shaaaaaaaaaaaaa

Check if a website has weak SHA-1 TLS certificates.
https://shaaaaaaaaaaaaa.com
BSD 3-Clause "New" or "Revised" License
207 stars 27 forks source link

Ignore server-provided root certificates #54

Closed mathiasbynens closed 9 years ago

mathiasbynens commented 9 years ago

Fixes #37.

konklone commented 9 years ago

I'm going to refactor slightly (to load certs into memory on process start), but this looks terrific. I think there are some minor differences between Mozilla's and Chrome's certificate bundle, but not so much so that it'll cause us any issues.

Also, I'll add mathiasbynens.be in as a test case, but I'd love to find another one so that we can let @mathiasbynens tighten up his cert chain already. =)

jonnybarnes commented 9 years ago

The code I've written should only do a file read if there's an intermediate certificate, its the first intermediate cert in the chain (or only intermediate cert in chain) and its signed with SHA-1.

jonnybarnes commented 9 years ago

Being class-2 verified with StartSSL I'm happy to set up a subdomain with dummy content for testing purposes, but I'm not sure if StartSSL have an SHA-1 signed root certificate.

konklone commented 9 years ago

StartCom has two roots in the trust store, a SHA-1 and a SHA-2 cert, as seen here:

https://www.ssllabs.com/ssltest/analyze.html?d=shaaaaaaaaaaaaa.com

They also have a third, just Ctrl+F-ing through the bundle, a G2 cert, for some other purpose.

jonnybarnes commented 9 years ago

@konklone for the first time, my certificate request with StartSSL has been marked for approval...

konklone commented 9 years ago

I think that https://www.startssl.com/certs/ca.pem is the SHA-1 root, you could add that to your chain.

konklone commented 9 years ago

@mathiasbynens' chain is also interesting because it's out of order - client, root, then intermediates. I think that's a reason not to factor order or intermediacy when checking certs for their root-ness.

Anyway, working on tests and stuff now, coming along well. I did not expect the answer would be as simple as a .crt file an .indexOf() call - great work @jonnybarnes, this is so simple.

jonnybarnes commented 9 years ago

So this should do for testing: https://shaaaaaaaaaaaaa.com/check/sha1.jonnybarnes.uk

jonnybarnes commented 9 years ago

It also highlights the issue you mentioned of not assuming the order of the intermediaries

konklone commented 9 years ago

So it took me a bit to figure out what was happening, but this actually isn't working as submitted -- because the file read is async, the callback happens offthread, and never actually executes before the script is done running.

mathiasbynens.be comes back with a good instead of an almost because the conditional that would have marked intergood as false never happens, and intergood is set to true by default. So it was working by accident, but would actually short-circuit the check on any SHA-1 cert placed first in the chain.

A second problem is that a simple compare of the string index won't work, because Mozilla's cert bundle uses 77-char columns, and the certs I've seen coming back from openssl use 65-char columns, so the strings aren't right by default. I think stripping out newlines from the bundle upon load, and from the cert upon compare, is the way to go here. More in a bit.

jonnybarnes commented 9 years ago

Also, got my cert set-up properly now, we can use https://sha1-root.jonnybarnes.uk/ for testing

jonnybarnes commented 9 years ago

@konklone so would we want fs.readFileSync instead? To possibly simplify things could we manually edit the ca-bundle.crt file to not have new lines in the certs?

jonnybarnes commented 9 years ago

The only other thing I can think of is to have a script that parses through the all the certs in the ca-bundle.crt and generates a list of fingerprints, and then see if the fingerprint of the SHA-1 intermediate cert is in said list? we were going to check fingerprints to point to SHA-2 intermediate certs anyway...

konklone commented 9 years ago

Yep, that's what I'm doing now. Using x.509 to parse each root cert, and store the fingerprint of each one. I also modified the ca-bundle.crt to be easier to split and parse in-code.

jonnybarnes commented 9 years ago

Excellent, should then be relatively trivial to use the fingerprints.json file I'm working on to check for available SHA-2 certs when we come across a SHA-1 intermediate.

konklone commented 9 years ago

OK, I've updated this branch -

konklone commented 9 years ago

Tests pass, so I'm going to merge and deploy. @jonnybarnes thanks for making this happen! Looking forward to integrating with your fingerprints.json. :)

mathiasbynens commented 9 years ago

:+1: