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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 6 commits into from
Apr 26, 2020
Merged

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

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9be93ec
Cache OCSP request fail for 5 minutes and display a better error (#2)
FabienChaynes Apr 25, 2020
2c690d0
Better variable name.
FabienChaynes Apr 25, 2020
1280748
Better error message.
FabienChaynes Apr 25, 2020
2e5cd14
Cache everything related to OCSP response.
FabienChaynes Apr 25, 2020
fccd25e
Update some error messages + Minitest deprecation warnings
jarthod Apr 25, 2020
daa3661
Updated version and changelog
jarthod Apr 25, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 21 additions & 13 deletions lib/ssl-test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

module SSLTest
VERSION = "1.3.0".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)
Expand Down Expand Up @@ -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]

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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

Expand All @@ -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)
Expand Down
90 changes: 45 additions & 45 deletions test/ssl-test_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
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

_(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