Fix false "revoked" results for CRL and OCSP checking

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 #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 #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 #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).
This commit is contained in:
David Cooper 2018-05-29 16:39:46 -04:00 committed by GitHub
parent 626e0fc65a
commit 998c2aa1f8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -247,6 +247,7 @@ UNBRACKTD_IPV6=${UNBRACKTD_IPV6:-false} # some versions of OpenSSL (like Gentoo)
NO_ENGINE=${NO_ENGINE:-false} # if there are problems finding the (external) openssl engine set this to true NO_ENGINE=${NO_ENGINE:-false} # if there are problems finding the (external) openssl engine set this to true
declare -r CLIENT_MIN_PFS=5 # number of ciphers needed to run a test for PFS declare -r CLIENT_MIN_PFS=5 # number of ciphers needed to run a test for PFS
CAPATH="${CAPATH:-/etc/ssl/certs/}" # Does nothing yet (FC has only a CA bundle per default, ==> openssl version -d) CAPATH="${CAPATH:-/etc/ssl/certs/}" # Does nothing yet (FC has only a CA bundle per default, ==> openssl version -d)
GOOD_CA_BUNDLE="" # A bundle of CA certificates that can be used to validate the server's certificate
MEASURE_TIME_FILE=${MEASURE_TIME_FILE:-""} MEASURE_TIME_FILE=${MEASURE_TIME_FILE:-""}
if [[ -n "$MEASURE_TIME_FILE" ]] && [[ -z "$MEASURE_TIME" ]]; then if [[ -n "$MEASURE_TIME_FILE" ]] && [[ -z "$MEASURE_TIME" ]]; then
MEASURE_TIME=true MEASURE_TIME=true
@ -1451,7 +1452,7 @@ check_revocation_crl() {
local crl="$1" local crl="$1"
local jsonID="$2" local jsonID="$2"
local tmpfile="" local tmpfile=""
local scheme local scheme retcode
local -i success local -i success
"$PHONE_OUT" || return 0 "$PHONE_OUT" || return 0
@ -1474,22 +1475,38 @@ check_revocation_crl() {
return 1 return 1
fi fi
# -crl_download could be more elegant but is supported from 1.0.2 onwards only # -crl_download could be more elegant but is supported from 1.0.2 onwards only
$OPENSSL crl -inform DER -in "$tmpfile" -outform PEM -out "${tmpfile%%.crl}.pem" $OPENSSL crl -inform DER -in "$tmpfile" -outform PEM -out "${tmpfile%%.crl}.pem" &>$ERRFILE
if [[ $? -ne 0 ]]; then if [[ $? -ne 0 ]]; then
pr_warning "conversion of "$tmpfile" failed" pr_warning "conversion of "$tmpfile" failed"
fileout "$jsonID" "WARN" "conversion of CRL to PEM format failed" fileout "$jsonID" "WARN" "conversion of CRL to PEM format failed"
return 1 return 1
fi fi
cat $TEMPDIR/intermediatecerts.pem "${tmpfile%%.crl}.pem" >$TEMPDIR/${NODE}-${NODEIP}-CRL-chain.pem if grep -q "\-\-\-\-\-BEGIN CERTIFICATE\-\-\-\-\-" $TEMPDIR/intermediatecerts.pem; then
$OPENSSL verify -crl_check -CAfile $TEMPDIR/${NODE}-${NODEIP}-CRL-chain.pem $TEMPDIR/host_certificate.pem &>$ERRFILE $OPENSSL verify -crl_check -CAfile <(cat $ADDITIONAL_CA_FILES "$GOOD_CA_BUNDLE" "${tmpfile%%.crl}.pem") -untrusted $TEMPDIR/intermediatecerts.pem $HOSTCERT &> "${tmpfile%%.crl}.err"
else
$OPENSSL verify -crl_check -CAfile <(cat $ADDITIONAL_CA_FILES "$GOOD_CA_BUNDLE" "${tmpfile%%.crl}.pem") $HOSTCERT &> "${tmpfile%%.crl}.err"
fi
if [[ $? -eq 0 ]]; then if [[ $? -eq 0 ]]; then
out ", " out ", "
pr_svrty_good "not revoked" pr_svrty_good "not revoked"
fileout "$jsonID" "OK" "not revoked" fileout "$jsonID" "OK" "not revoked"
else else
out ", " retcode=$(awk '/error [1-9][0-9]? at [0-9]+ depth lookup:/ { if (!found) {print $2; found=1} }' "${tmpfile%%.crl}.err")
pr_svrty_critical "revoked" if [[ "$retcode" == "23" ]]; then # see verify_retcode_helper()
fileout "$jsonID" "CRITICAL" "revoked" out ", "
pr_svrty_critical "revoked"
fileout "$jsonID" "CRITICAL" "revoked"
else
retcode="$(verify_retcode_helper "$retcode")"
out " $retcode"
retcode="${retcode#(}"
retcode="${retcode%)}"
fileout "$jsonID" "WARN" "$retcode"
if [[ $DEBUG -ge 2 ]]; then
outln
cat "${tmpfile%%.crl}.err"
fi
fi
fi fi
return 0 return 0
} }
@ -1513,7 +1530,7 @@ check_revocation_ocsp() {
fi fi
$OPENSSL ocsp -no_nonce ${host_header} -url "$uri" \ $OPENSSL ocsp -no_nonce ${host_header} -url "$uri" \
-issuer $TEMPDIR/hostcert_issuer.pem -verify_other $TEMPDIR/intermediatecerts.pem \ -issuer $TEMPDIR/hostcert_issuer.pem -verify_other $TEMPDIR/intermediatecerts.pem \
-CAfile $TEMPDIR/intermediatecerts.pem -cert $HOSTCERT -text &> "$tmpfile" -CAfile <(cat $ADDITIONAL_CA_FILES "$GOOD_CA_BUNDLE") -cert $HOSTCERT -text &> "$tmpfile"
if [[ $? -eq 0 ]] && grep -Fq "Response verify OK" "$tmpfile"; then if [[ $? -eq 0 ]] && grep -Fq "Response verify OK" "$tmpfile"; then
response="$(grep -F "$HOSTCERT: " "$tmpfile")" response="$(grep -F "$HOSTCERT: " "$tmpfile")"
response="${response#$HOSTCERT: }" response="${response#$HOSTCERT: }"
@ -6218,6 +6235,7 @@ verify_retcode_helper() {
case $retcode in case $retcode in
# codes from ./doc/apps/verify.pod | verify(1ssl) # codes from ./doc/apps/verify.pod | verify(1ssl)
44) tm_out "(different CRL scope)" ;; # X509_V_ERR_DIFFERENT_CRL_SCOPE
26) tm_out "(unsupported certificate purpose)" ;; # X509_V_ERR_INVALID_PURPOSE 26) tm_out "(unsupported certificate purpose)" ;; # X509_V_ERR_INVALID_PURPOSE
24) tm_out "(certificate unreadable)" ;; # X509_V_ERR_INVALID_CA 24) tm_out "(certificate unreadable)" ;; # X509_V_ERR_INVALID_CA
23) tm_out "(certificate revoked)" ;; # X509_V_ERR_CERT_REVOKED 23) tm_out "(certificate revoked)" ;; # X509_V_ERR_CERT_REVOKED
@ -6292,6 +6310,7 @@ determine_trust() {
if [[ ${verify_retcode[i]} -eq 0 ]]; then if [[ ${verify_retcode[i]} -eq 0 ]]; then
trust[i]=true trust[i]=true
some_ok=true some_ok=true
[[ -z "$GOOD_CA_BUNDLE" ]] && GOOD_CA_BUNDLE="$bundle_fname"
debugme tm_svrty_good "Ok " debugme tm_svrty_good "Ok "
debugme tmln_out "${verify_retcode[i]}" debugme tmln_out "${verify_retcode[i]}"
else else
@ -6977,6 +6996,7 @@ certificate_info() {
spaces=" " spaces=" "
fi fi
GOOD_CA_BUNDLE=""
cert_sig_algo="$(awk -F':' '/Signature Algorithm/ { print $2; if (++Match >= 1) exit; }' <<< "$cert_txt")" cert_sig_algo="$(awk -F':' '/Signature Algorithm/ { print $2; if (++Match >= 1) exit; }' <<< "$cert_txt")"
cert_sig_algo="${cert_sig_algo// /}" cert_sig_algo="${cert_sig_algo// /}"
cert_key_algo="$(awk -F':' '/Public Key Algorithm:/ { print $2; if (++Match >= 1) exit; }' <<< "$cert_txt")" cert_key_algo="$(awk -F':' '/Public Key Algorithm:/ { print $2; if (++Match >= 1) exit; }' <<< "$cert_txt")"
@ -7577,7 +7597,7 @@ certificate_info() {
else else
if [[ $(count_lines "$crl") -eq 1 ]]; then if [[ $(count_lines "$crl") -eq 1 ]]; then
out "$crl" out "$crl"
if [[ "$expfinding" != "expired" ]]; then if [[ "$expfinding" != "expired" ]] && [[ -n "$GOOD_CA_BUNDLE" ]]; then
check_revocation_crl "$crl" "cert_crlRevoked${json_postfix}" check_revocation_crl "$crl" "cert_crlRevoked${json_postfix}"
ret=$((ret +$?)) ret=$((ret +$?))
fi fi
@ -7591,7 +7611,7 @@ certificate_info() {
out "$spaces" out "$spaces"
fi fi
out "$line" out "$line"
if [[ "$expfinding" != "expired" ]]; then if [[ "$expfinding" != "expired" ]] && [[ -n "$GOOD_CA_BUNDLE" ]]; then
check_revocation_crl "$line" "cert_crlRevoked${json_postfix}" check_revocation_crl "$line" "cert_crlRevoked${json_postfix}"
ret=$((ret +$?)) ret=$((ret +$?))
fi fi
@ -7610,7 +7630,8 @@ certificate_info() {
else else
if [[ $(count_lines "$ocsp_uri") -eq 1 ]]; then if [[ $(count_lines "$ocsp_uri") -eq 1 ]]; then
out "$ocsp_uri" out "$ocsp_uri"
if [[ "$expfinding" != "expired" ]]; then if [[ "$expfinding" != "expired" ]] && [[ -n "$GOOD_CA_BUNDLE" ]] && \
grep -q "\-\-\-\-\-BEGIN CERTIFICATE\-\-\-\-\-" $TEMPDIR/intermediatecerts.pem; then
check_revocation_ocsp "$ocsp_uri" "cert_ocspRevoked${json_postfix}" check_revocation_ocsp "$ocsp_uri" "cert_ocspRevoked${json_postfix}"
fi fi
ret=$((ret +$?)) ret=$((ret +$?))
@ -7624,7 +7645,8 @@ certificate_info() {
out "$spaces" out "$spaces"
fi fi
out "$line" out "$line"
if [[ "$expfinding" != "expired" ]]; then if [[ "$expfinding" != "expired" ]] && [[ -n "$GOOD_CA_BUNDLE" ]] && \
grep -q "\-\-\-\-\-BEGIN CERTIFICATE\-\-\-\-\-" $TEMPDIR/intermediatecerts.pem; then
check_revocation_ocsp "$line" "cert_ocspRevoked${json_postfix}" check_revocation_ocsp "$line" "cert_ocspRevoked${json_postfix}"
ret=$((ret +$?)) ret=$((ret +$?))
fi fi