From 72ef69aeaee2d79608427714d214b2202e50d459 Mon Sep 17 00:00:00 2001 From: David Cooper Date: Thu, 28 Jun 2018 16:17:04 -0400 Subject: [PATCH] Handle incorrectly populated certificate_list According to Section 7.4.2 of RFC 5246, when a server sends its certificate it MUST send a list in which the first certificate is the sender's certificate and "Each following certificate MUST directly certify the one preceding it." testssl.sh currently assumes that the server has populated the list way and so places the second certificate in the list into $TEMPDIR/hostcert_issuer.pem. However, not all servers have been following this requirement, and so draft-ietf-tls-tls13 (soon to be RFC 8446) only says that servers SHOULD list the certificates in this way and says that clients "SHOULD be prepared to handle potentially extraneous certificates and arbitrary orderings from any TLS version, with the exception of the end-entity certificate which MUST be first." testssl.sh needs to place the correct certificate in $TEMPDIR/hostcert_issuer.pem, since otherwise any OCSP request it sends will be incorrect, and any attempt to verify and OCSP response will be incorrect as well. This PR changes extract_certificates() and parse_tls_serverhello() to populate $TEMPDIR/hostcert_issuer.pem with the first certificate in certificate_list that has a subject DN that matches the issuer DN in the server's certificate, rather than simply populating $TEMPDIR/hostcert_issuer.pem with the second certificate in the list. In testing a random sampling of U.S. government servers, of 57 servers tested 5 reported "unauthorized" for the OCSP URI using the current testssl.sh and all 5 of these reported "not revoked" with this PR. This PR also corrects the same issue in some servers on the Alexa Top 1000, but this was only a problem for 12 of those 1000 servers. --- testssl.sh | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/testssl.sh b/testssl.sh index 816f616..c944201 100755 --- a/testssl.sh +++ b/testssl.sh @@ -6555,7 +6555,8 @@ determine_tls_extensions() { extract_certificates() { local version="$1" local savedir - local -i success nrsaved=0 + local -i i success nrsaved=0 + local issuerDN CAsubjectDN # Place the server's certificate in $HOSTCERT and any intermediate # certificates that were provided in $TEMPDIR/intermediatecerts.pem @@ -6582,7 +6583,26 @@ extract_certificates() { echo "" > $TEMPDIR/intermediatecerts.pem else cat level?.crt > $TEMPDIR/intermediatecerts.pem - cp level1.crt $TEMPDIR/hostcert_issuer.pem + issuerDN="$($OPENSSL x509 -in $HOSTCERT -noout -issuer 2>/dev/null)" + issuerDN="${issuerDN:8}" + # The second certficate (level1.crt) SHOULD be issued to the CA + # that issued the server's certificate. But, according to RFC 8446 + # clients SHOULD be prepared to handle cases in which the server + # does not order the certificates correctly. + for (( i=1; i < nrsaved; i++ )); do + CAsubjectDN="$($OPENSSL x509 -in "level$i.crt" -noout -subject 2>/dev/null)" + if [[ "${CAsubjectDN:9}" == "$issuerDN" ]]; then + cp "level$i.crt" $TEMPDIR/hostcert_issuer.pem + break + fi + done + # This should never happen, but if more than one certificate was + # provided and none of them belong to the CA that issued the + # server's certificate, then the extra certificates should just + # be deleted. There is code elsewhere that assumes that if + # $TEMPDIR/intermediatecerts.pem is non-empty, then + # $TEMPDIR/hostcert_issuer.pem is also present. + [[ $i -eq $nrsaved ]] && echo "" > $TEMPDIR/intermediatecerts.pem rm level?.crt fi fi @@ -11097,7 +11117,7 @@ parse_tls_serverhello() { echo " i:${CAissuerDN:8}" >> $TMPFILE echo "$pem_certificate" >> $TMPFILE echo "$pem_certificate" >> "$TEMPDIR/intermediatecerts.pem" - if [[ -z "$hostcert_issuer" ]]; then + if [[ -z "$hostcert_issuer" ]] && [[ "${CAsubjectDN:9}" == "${issuerDN:8}" ]]; then # The issuer's certificate is needed if there is a stapled OCSP response, # and it may be needed if check_revocation_ocsp() will later be called # with the OCSP URI in the server's certificate.