Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cache OCSP request fail for 5 minutes and display a better error (#2) #5

Merged
merged 6 commits into from
Apr 26, 2020

Conversation

FabienChaynes
Copy link
Collaborator

@FabienChaynes FabienChaynes commented Apr 25, 2020

(#2)

This PR improves the SSL revocation test.

Sometimes, when we query the OCSP responder, we get a failure. Currently this failure is not cached so we will keep querying the OCSP responder (which might be temporary down).

This PR improves this by caching the error for 5 minutes, so that we won't put more load on the OCSP responder if he's not working properly at the moment.
In addition to that, the error message returned was improved in this situation to explain with it failed and to include the OCSP responder URI.

Here's what the error will look like now:

irb(main):001:0> SSLTest.test("https://cloudreports.xyz")
=> [true, "OCSP test couldn't be performed: OCSP response request failed (OCSP URI: http://isrg.trustid.ocsp.identrust.com, error: Wrong response type (Net::HTTPServiceUnavailable))", #<OpenSSL::X509::Certificate: subject=#<OpenSSL::X509::Name CN=cloudreports.xyz>, issuer=#<OpenSSL::X509::Name CN=Let's Encrypt Authority X3,O=Let's Encrypt,C=US>, serial=#<OpenSSL::BN:0x0000000006baf620>, not_before="2020-03-10T03:25:33.000Z", not_after="2020-06-08T03:25:33.000Z">]

test/ssl-test_test.rb Outdated Show resolved Hide resolved
lib/ssl-test.rb Outdated Show resolved Hide resolved
error.must_equal 'hostname "wrong.host.badssl.com" does not match the server certificate'
valid.must_equal false
cert.must_be_instance_of OpenSSL::X509::Certificate
assert error.include?('hostname "wrong.host.badssl.com" does not match the server certificate')
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have the version before you changed it on my machine so I changed the matcher so work with both

@jarthod jarthod merged commit 130e77c into master Apr 26, 2020
@jarthod jarthod deleted the 2-handle-failing-ocsp branch April 26, 2020 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants