sesh / ready

Are you production ready?
ISC License
26 stars 4 forks source link

Certificate revocation #15

Closed Seirdy closed 4 months ago

Seirdy commented 11 months ago

ready currently reports a pass if a certificate is trusted. It could benefit from giving revocation information as well by reporting on:

In-depth TLS analysis may be out of scope and better suited for a tool like testssl.sh or Internet.nl, but a simple revocation check could work. At the very least: it could warn if OCSP stapling isn't set up for certificates with normal lifetimes (>10 days), and perhaps encourage them to use must-staple.

sesh commented 11 months ago

A check for OCSP stapling makes total sense to me.

A couple of notes from my quick investigation this morning.

getpeercert() doesn't return whether must-staple is enabled by the looks of things. It does return an OCSP value but that appears to be from the information access section.

Running the der file (from: getperrcert(binary_form=True)) through openssl (openssl x509 -in cert.der -inform der -text) does output the "1.3.6.1.4.1.11129.2.4.2" block.

sesh commented 11 months ago

Here's a cpython issue about adding support for extensions to the getpeercert function: https://github.com/python/cpython/issues/64668

The suggestion there is to use pyasn1 or similar.

sesh commented 11 months ago

Okay, so the pyasn1 rabbit whole has ended up with a couple of other interesting libraries that we could use here.

asn1crypto lets you load DER files with the extensions. Then certvalidator wraps that with a bunch of (very appropriate) validation rules:

https://github.com/wbond/certvalidator/

If I implement something beyond simply checking for the "OCSP" value in the certificate than I expect that it would end up using this library.

Seirdy commented 11 months ago

Sounds good. How do you feel about checking for "must-staple"? Without OCSP Must-Staple, I don't think stapling is anything more than a privacy measure to reduce traffic leakage to CAs because failures are silently ignored.

sesh commented 4 months ago

Circling back around to this one.

Initial check for the existence of the OCSP value in the certificate, and that each item in that must start with http:// or https:// is done as "SSL certificate must provide OCSP URI"

--

Scott Helme has more details about the must-staple block: https://scotthelme.co.uk/ocsp-must-staple/

--

I've gone with the cryptography library since it's widely used. I can implement the check for TLS extensions on the certificate by checking for the 1.3.6.1.5.5.7.1.24 OID in the features list.

1.3.6.1.4.1.11129.2.4.2 == stapling 1.3.6.1.5.5.7.1.24 == must-staple

You can use https://oidref.com/ to look them up.

I'm going to have this as an optional dependency and skip the check if the library is not installed (similar to beautifulsoup and grabbing the headers from the HTML body).

The check is "SSL certificate should provide OCSP must-staple"

And the check will warn on fail. For reference: SSL Labs still gives you an A+ without must-staple.

(updating a test domain to have must-staple)

Looks to be working a treat! Will do a new release today that includes this additional check.