Fix some problems with CRL revocation checking #1051
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dcooper16
force-pushed
the
fix_crl_checking
branch
2 times, most recently
from
55bb967
to
496fa61
Compare
dcooper16
force-pushed
the
fix_crl_checking
branch
from
496fa61
to
579c61c
Compare
drwetter
added a commit
that referenced
this pull request
May 18, 2018
dcooper16
force-pushed
the
fix_crl_checking
branch
4 times, most recently
from
a1568ba
to
d622c2b
Compare
dcooper16
force-pushed
the
fix_crl_checking
branch
from
d622c2b
to
828b9e3
Compare
There are some circumstances in which check_revocation_crl() will incorrectly indicate that a CRL lists the server's certificate as revoked. drwetter#1046 is one of them. Another is any case in which the server's certificate cannot be validated using any of the certificates in the trust store that OpenSSL uses (e.g., the server's certificate was issued by a local CA). In both of these cases, "openssl verify" fails, for some reason other than "certificate revoked", and check_revocation_crl() assumes that any failure of "openssl verify" is the result of certificate revocation. This PR addresses the problem in two ways. First, it adds the "-partial_chain" option to the "openssl verify" command line whenever $OPENSSL supports that option (it is not supported by LibreSSL or by versions of OpenSSL earlier than 1.0.2). This will fix most of the problems when a version of OpenSSL that supports "-partial_chain" is used. Even if the "-partial_chain" option is provided, OpenSSL needs to have at least one CA certificate so that it can get the public key needed to verify the signatures on the server's certificate and on the CRL. So, if the server doesn't send any CA certificates and the server's certificate was not issued by a CA in the trust store, then the verify command will fail even if the "-partial_chain" option is provided. So, as a fail-safe, this PR changes check_revocation_crl() to check the error message that the verify command provides when it fails so that testssl.sh only reports a certificate a revoked if the verify command fails with a reason of "certificate revoked". Note that this PR also fixes two other minor issues. It incorporates drwetter#1047, which corrects a typo, and it redirects $OPENSSL's output on line 1479 in order to suppress any error messages that $OPENSSL might print (e.g., "WARNING: can't open config file").
dcooper16
force-pushed
the
fix_crl_checking
branch
from
2429d45
to
89f8ddc
Compare
dcooper16
added a commit
to dcooper16/testssl.sh
that referenced
this pull request
May 29, 2018
This PR fixes problems with check_revocation_crl() sometimes reporting that a certificate is revoked even when it isn't, and with check_revocation_ocsp() sometimes reporting "error querying OCSP responder" even if the OCSP responder provided a good response. The most common reason for this to happen is that OpenSSL cannot validate the server's certificate (even without status checking). PR drwetter#1051 attempted to get status checking to work even in cases in which the server's certificate could not be validated. This PR instead addresses the problem by not checking status if determine_trust() was unable to validate the server's certificate. In some cases the server's certificate can be validated using some, but not all of the bundles of trusted certificates. For example, I have encountered some sites that can be validated using the Microsoft and Apple bundles, but not the Linux or Mozilla bundles. This PR introduces GOOD_CA_BUNDLE to store a bundle that could be used to successfully validate the server's certificate. If there is no such bundle, then neither check_revocation_crl() nor check_revocation_ocsp() is run. When check_revocation_crl() and check_revocation_ocsp() are called, the status checks within them closely match the validation check in determine_trust(), which helps to ensure that if the check fails it is because of the status information. As noted in drwetter#1057, at least one CA provides incorrect information when the CRL is downloaded, so validation could fail for a reason other than the certificate being revoked. So, this PR adds a check of the reason that validation failed and only reports "revoked" if the validation failed for that reason. As noted in drwetter#1056, it is not possible to perform an OCSP query without access to the certificate issuer's public key. So, with this PR check_revocation_ocsp() is only called if the server's provided at least one intermediate certificate (i.e., the issuer's certificate, which contains the issuer's public key).
|
Closing this PR as #1067 provides an alternative solution. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There are some circumstances in which
check_revocation_crl()will incorrectly indicate that a CRL lists the server's certificate as revoked. #1046 is one of them. Another is any case in which the server's certificate cannot be validated using any of the certificates in the trust store that OpenSSL uses (e.g., the server's certificate was issued by a local CA). In both of these cases, "openssl verify" fails, for some reason other than "certificate revoked", andcheck_revocation_crl()assumes that any failure of "openssl verify" is the result of certificate revocation.This PR addresses the problem in two ways. First, it adds the "-partial_chain" option to the "openssl verify" command line whenever
$OPENSSLsupports that option (it is not supported by LibreSSL or by versions of OpenSSL earlier than 1.0.2). This will fix most of the problems when a version of OpenSSL that supports "-partial_chain" is used.However, even if the "-partial_chain" option is provided, OpenSSL needs to have at least one CA certificate so that it can get the public key needed to verify the signatures on the server's certificate and on the CRL. So, if the server doesn't send any CA certificates and the server's certificate was not issued by a CA in the trust store, then the verify command will fail even if the "-partial_chain" option is provided.
So, as a fail-safe, this PR changes
check_revocation_crl()to check the error message that the verify command provides when it fails so that testssl.sh only reports a certificate a revoked if the verify command fails with a reason of "certificate revoked".Note that this PR also fixes two other minor issues. It incorporates #1047, which corrects a typo, and it redirects $OPENSSL's output on line 1479 in order to suppress any error messages that $OPENSSL might print (e.g., "WARNING: can't open config file").