search

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

Prevent adding known roots to tls connect response. #86

Closed mbrand1 closed 8 years ago

mbrand1 commented 8 years ago

If 'ca' is omitted "well known" roots will be checked against (and added if found to be part of the chain). But we don't want this. We only want to see certs sent by the server itself.

konklone commented 8 years ago

So how confident do you feel that the underlying problem is solved? My concern wasn't just that the root was being appended, but that the intermediates might also be affected. I specifically don't want the client to path-build, or to farm out to the OS for path-building, and instead I want to evaluate the certificates provided in the server chain. (In other words, I don't want the site to pass on OS-specific path-building bugs, which have been especially bad in the Debian/Linux world, and I run the site on an Ubuntu server.)

I'm having trouble confirming how the tls library works in this regard from the official Node documentation. Anything you can find that helps confirm the behavior?

mbrand1 commented 8 years ago

Well, I can't find anything to confirm this. I looked again for 'intermediate' or 'chain', but I don't see any other references like I did for this ca option and root certificates. It is troubling that this behavior is on by default. What else could be on that they aren't telling us? I did notice the roots in there, but didn't think much of it until your feedback. And the servername option appears to take care of SNI. Other than that, I can't think of another way the certs could be manipulated. There is caching, but that would require additional setup...not on by default as far as I can see... I'd say I'm reasonably confident based on your initial feedback and some more poking around.

mbrand1 commented 8 years ago

Removed that recursion and parsing no longer dependent on circular reference check.

konklone commented 8 years ago

OK, sorry to leave this hanging -- @mbrand1, this work is so good, thank you again for doing it, and for updating the chain-walking code to remove the potential infinite loop.

I still have some concern about possibly causing the client to evaluate certificates/chains that the site doesn't directly serve, but I'm willing to send this into the wild and deal with issues as they arise.

mbrand1 commented 8 years ago

Great to hear! If something weird happens, let me know.