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

…#5)

README.md

@@ -83,6 +83,7 @@ But also **revoked certs** like most browsers (not handled by `curl`)
 
 ## Changelog
 
+* 1.3.1 - 2020-04-25: Improved caching of failed OCSP responses (#5)
 * 1.3.0 - 2020-04-25: Added revoked cert detection using OCSP (#3)
 
 ## Contributing

lib/ssl-test.rb

@@ -4,7 +4,8 @@
 require "uri"
 
 module SSLTest
-  VERSION = "1.3.0".freeze
+  VERSION = "1.3.1".freeze
+  OCSP_REQUEST_ERROR_CACHE_DURATION = 5 * 60
 
   def self.test url, open_timeout: 5, read_timeout: 5, redirection_limit: 5
     uri = URI.parse(url)
@@ -50,11 +51,15 @@ def self.test url, open_timeout: 5, read_timeout: 5, redirection_limit: 5
   # Returns an array with [ocsp_check_failed, certificate_revoked, error_reason, revocation_date]
   def self.test_ocsp_revocation chain, open_timeout: 5, read_timeout: 5, redirection_limit: 5
     @ocsp_response_cache ||= {}
+    @ocsp_request_error_cache ||= {}
     chain[0..-2].each_with_index do |cert, i|
       # https://tools.ietf.org/html/rfc5280#section-4.1.2.2
       # The serial number [...] MUST be unique for each certificate issued by a given CA (i.e., the issuer name and serial number identify a unique certificate)
       unicity_key = "#{cert.issuer}/#{cert.serial}"
 
+      current_request_error_cache = @ocsp_request_error_cache[unicity_key]
+      return current_request_error_cache[:error] if current_request_error_cache && Time.now <= current_request_error_cache[:cache_until]
+
       if @ocsp_response_cache[unicity_key].nil? || @ocsp_response_cache[unicity_key][:next_update] <= Time.now
         issuer = chain[i + 1]
 
@@ -83,27 +88,27 @@ def self.test_ocsp_revocation chain, open_timeout: 5, read_timeout: 5, redirecti
         return ocsp_soft_fail_return("Missing OCSP URI in authorityInfoAccess extension") unless ocsp
 
         ocsp_uri = URI(ocsp[/URI:(.*)/, 1])
-        http_response = follow_ocsp_redirects(ocsp_uri, request.to_der, open_timeout: open_timeout, read_timeout: read_timeout, redirection_limit: redirection_limit)
-        return ocsp_soft_fail_return("OCSP response request failed") unless http_response
+        http_response, ocsp_request_error = follow_ocsp_redirects(ocsp_uri, request.to_der, open_timeout: open_timeout, read_timeout: read_timeout, redirection_limit: redirection_limit)
+        return ocsp_soft_fail_return("Request failed (URI: #{ocsp_uri}): #{ocsp_request_error}", unicity_key) unless http_response
 
         response = OpenSSL::OCSP::Response.new http_response.body
         # https://ruby-doc.org/stdlib-2.6.3/libdoc/openssl/rdoc/OpenSSL/OCSP.html#constants-list
-        return ocsp_soft_fail_return("OCSP response failed: #{ocsp_response_status_to_string(response.status)}") unless response.status == OpenSSL::OCSP::RESPONSE_STATUS_SUCCESSFUL
+        return ocsp_soft_fail_return("Unsuccessful response (URI: #{ocsp_uri}): #{ocsp_response_status_to_string(response.status)}", unicity_key) unless response.status == OpenSSL::OCSP::RESPONSE_STATUS_SUCCESSFUL
         basic_response = response.basic
 
         # Check the response signature
         store = OpenSSL::X509::Store.new
         store.set_default_paths
         # https://ruby-doc.org/stdlib-2.4.0/libdoc/openssl/rdoc/OpenSSL/OCSP/BasicResponse.html#method-i-verify
-        return ocsp_soft_fail_return("OCSP response signature verification failed") unless basic_response.verify(chain, store)
+        return ocsp_soft_fail_return("Signature verification failed (URI: #{ocsp_uri})", unicity_key) unless basic_response.verify(chain, store)
 
         # https://ruby-doc.org/stdlib-2.4.0/libdoc/openssl/rdoc/OpenSSL/OCSP/Request.html#method-i-check_nonce
-        return ocsp_soft_fail_return("OCSP response nonce check failed") unless request.check_nonce(basic_response) != 0
+        return ocsp_soft_fail_return("Nonce check failed (URI: #{ocsp_uri})", unicity_key) unless request.check_nonce(basic_response) != 0
 
         # https://ruby-doc.org/stdlib-2.3.0/libdoc/openssl/rdoc/OpenSSL/OCSP/BasicResponse.html#method-i-status
-        response_certificate_id, status, reason, revocation_time, this_update, next_update, _extensions = basic_response.status.first
+        response_certificate_id, status, reason, revocation_time, _this_update, next_update, _extensions = basic_response.status.first
 
-        return ocsp_soft_fail_return("OCSP response serial check failed") unless response_certificate_id.serial == certificate_id.serial
+        return ocsp_soft_fail_return("Serial check failed (URI: #{ocsp_uri})", unicity_key) unless response_certificate_id.serial == certificate_id.serial
 
         @ocsp_response_cache[unicity_key] = { status: status, reason: reason, revocation_time: revocation_time, next_update: next_update }
       end
@@ -140,8 +145,9 @@ def matching_domains domains, hostname
         .select {|domain| domain.match?(hostname) }
     end
 
+    # Returns an array with [response, error_message]
     def follow_ocsp_redirects(uri, data, open_timeout: 5, read_timeout: 5, redirection_limit: 5)
-      return nil if redirection_limit == 0
+      return [nil, "Too many redirections (> #{redirection_limit})"] if redirection_limit == 0
 
       path = uri.path == "" ? "/" : uri.path
       http = Net::HTTP.new(uri.hostname, uri.port)
@@ -151,11 +157,11 @@ def follow_ocsp_redirects(uri, data, open_timeout: 5, read_timeout: 5, redirecti
       http_response = http.post(path, data, "content-type" => "application/ocsp-request")
       case http_response
       when Net::HTTPSuccess
-        http_response
+        [http_response, nil]
       when Net::HTTPRedirection
-        follow_ocsp_redirects(URI(http_response["location"]), data, open_timeout: open_timeout, read_timeout: read_timeout, redirection_limit: redirection_limit -1)
+        follow_ocsp_redirects(URI(http_response["location"]), data, open_timeout: open_timeout, read_timeout: read_timeout, redirection_limit: redirection_limit - 1)
       else
-        nil
+        [nil, "Wrong response type (#{http_response.class})"]
       end
     end
 
@@ -177,8 +183,10 @@ def ocsp_response_status_to_string(response_status)
       end
     end
 
-    def ocsp_soft_fail_return(reason)
-       [false, false, reason, nil].freeze
+    def ocsp_soft_fail_return(reason, unicity_key = nil)
+       error = [false, false, reason, nil]
+       @ocsp_request_error_cache[unicity_key] = { error: error, cache_until: Time.now + OCSP_REQUEST_ERROR_CACHE_DURATION } if unicity_key
+       error
     end
 
     def revocation_reason_to_string(revocation_reason)

test/ssl-test_test.rb

@@ -6,107 +6,107 @@
   describe '.test' do
     it "returns no error on valid SNI website" do
       valid, error, cert = SSLTest.test("https://www.mycs.com")
-      error.must_be_nil
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_be_nil
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns no error on valid SAN" do
       valid, error, cert = SSLTest.test("https://1000-sans.badssl.com/")
-      error.must_be_nil
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_be_nil
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns no error when no CN" do
       valid, error, cert = SSLTest.test("https://no-common-name.badssl.com/")
-      error.must_be_nil
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_be_nil
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "works with websites blocking http requests" do
       valid, error, cert = SSLTest.test("https://obyava.ua")
-      error.must_be_nil
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_be_nil
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns error on self signed certificate" do
       valid, error, cert = SSLTest.test("https://self-signed.badssl.com/")
-      error.must_equal "error code 18: self signed certificate"
-      valid.must_equal false
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "error code 18: self signed certificate"
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns error on incomplete chain" do
       valid, error, cert = SSLTest.test("https://incomplete-chain.badssl.com/")
-      error.must_equal "error code 20: unable to get local issuer certificate"
-      valid.must_equal false
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "error code 20: unable to get local issuer certificate"
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns error on untrusted root" do
       valid, error, cert = SSLTest.test("https://untrusted-root.badssl.com/")
-      error.must_equal "error code 20: unable to get local issuer certificate"
-      valid.must_equal false
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "error code 19: self signed certificate in certificate chain"
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns error on invalid host" do
       valid, error, cert = SSLTest.test("https://wrong.host.badssl.com/")
-      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')
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns error on expired cert" do
       valid, error, cert = SSLTest.test("https://expired.badssl.com/")
-      error.must_equal "error code 10: certificate has expired"
-      valid.must_equal false
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "error code 10: certificate has expired"
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "returns undetermined state on unhandled error" do
       valid, error, cert = SSLTest.test("https://pijoinlrfgind.com")
-      error.must_equal "SSL certificate test failed: Failed to open TCP connection to pijoinlrfgind.com:443 (getaddrinfo: Name or service not known)"
-      valid.must_be_nil
-      cert.must_be_nil
+      _(error).must_equal "SSL certificate test failed: Failed to open TCP connection to pijoinlrfgind.com:443 (getaddrinfo: Name or service not known)"
+      _(valid).must_be_nil
+      _(cert).must_be_nil
     end
 
     it "stops on timeouts" do
       valid, error, cert = SSLTest.test("https://updown.io", open_timeout: 0)
-      error.must_equal "SSL certificate test failed: Net::OpenTimeout"
-      valid.must_be_nil
-      cert.must_be_nil
+      _(error).must_equal "SSL certificate test failed: Net::OpenTimeout"
+      _(valid).must_be_nil
+      _(cert).must_be_nil
     end
 
     it "returns error on revoked cert" do
       valid, error, cert = SSLTest.test("https://revoked.badssl.com/")
-      error.must_equal "SSL certificate revoked: The certificate was revoked for an unknown reason (revocation date: 2019-10-07 20:30:39 UTC)"
-      valid.must_equal false
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "SSL certificate revoked: The certificate was revoked for an unknown reason (revocation date: 2019-10-07 20:30:39 UTC)"
+      _(valid).must_equal false
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "stops following redirection after the limit for the revoked certs check" do
       valid, error, cert = SSLTest.test("https://github.com/", redirection_limit: 0)
-      error.must_equal "OCSP test couldn't be performed: OCSP response request failed"
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "OCSP test couldn't be performed: Request failed (URI: http://ocsp.digicert.com): Too many redirections (> 0)"
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "warns when the OCSP URI is missing" do
       valid, error, cert = SSLTest.test("https://www.demarches-simplifiees.fr")
-      error.must_equal "OCSP test couldn't be performed: Missing OCSP URI in authorityInfoAccess extension"
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "OCSP test couldn't be performed: Missing OCSP URI in authorityInfoAccess extension"
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
 
     it "warns when the authorityInfoAccess extension is missing" do
       valid, error, cert = SSLTest.test("https://www.anonymisation.gov.pf")
-      error.must_equal "OCSP test couldn't be performed: Missing authorityInfoAccess extension"
-      valid.must_equal true
-      cert.must_be_instance_of OpenSSL::X509::Certificate
+      _(error).must_equal "OCSP test couldn't be performed: Missing authorityInfoAccess extension"
+      _(valid).must_equal true
+      _(cert).must_be_instance_of OpenSSL::X509::Certificate
     end
   end
 end