From: Alan T. DeKok <aland@freeradius.org>
Date: Fri, 23 Sep 2016 13:55:33 +0000 (-0400)
Subject: clean up OCSP / verify routines
X-Git-Tag: release_3_0_12~32
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6e0112dcff5c42c379aa1f3a033e2de3bbb4435d;p=thirdparty%2Ffreeradius-server.git

clean up OCSP / verify routines
---

diff --git a/raddb/mods-available/eap b/raddb/mods-available/eap
index f77aa90f8ab..0e8d5caef5a 100644
--- a/raddb/mods-available/eap
+++ b/raddb/mods-available/eap
@@ -437,7 +437,10 @@ eap {
 		#  the correct paths below to enable it.
 		#
 		#  If OCSP checking is enabled, and the OCSP checks fail,
-		#  the verify section is skipped.
+		#  the verify section is not run.
+		#
+		#  If OCSP checking is disabled, the verify section is
+		#  run on successful certificate validation.
 		#
 		verify {
 			#  If the OCSP checks succeed, the verify section
diff --git a/src/main/tls.c b/src/main/tls.c
index 33ce6834490..b126f5f13e7 100644
--- a/src/main/tls.c
+++ b/src/main/tls.c
@@ -1910,6 +1910,7 @@ int cbtls_verify(int ok, X509_STORE_CTX *ctx)
 #ifdef HAVE_OPENSSL_OCSP_H
 	X509_STORE	*ocsp_store = NULL;
 	X509		*issuer_cert;
+	bool		do_verify = false;
 #endif
 	VALUE_PAIR	*vp;
 	TALLOC_CTX	*talloc_ctx;
@@ -2214,33 +2215,47 @@ int cbtls_verify(int ok, X509_STORE_CTX *ctx)
 		} /* check_cert_cn */
 
 #ifdef HAVE_OPENSSL_OCSP_H
-		if (my_ok && conf->ocsp_enable){
-			RDEBUG2("Starting OCSP Request");
-			if ((X509_STORE_CTX_get1_issuer(&issuer_cert, ctx, client_cert) != 1) ||
-			    !issuer_cert) {
-				RERROR("Couldn't get issuer_cert for %s", common_name);
+		if (my_ok) {
+			/*
+			 *	No OCSP, allow external verification.
+			 */
+			if (!conf->ocsp_enable) {
+				do_verify = true;
+
 			} else {
-				my_ok = ocsp_check(request, ocsp_store, issuer_cert, client_cert, conf);
+				RDEBUG2("Starting OCSP Request");
+				if ((X509_STORE_CTX_get1_issuer(&issuer_cert, ctx, client_cert) != 1) ||
+				    !issuer_cert) {
+					/*
+					 *	Allow for external verify.
+					 */
+					RERROR("Couldn't get issuer_cert for %s", common_name);
+					do_verify = true;
+
+				} else {
+					/*
+					 *	Do the full OCSP checks.
+					 *
+					 *	If they fail, don't run the external verify.  We don't want
+					 *	to allow admins to force authentication success for bad
+					 *	certificates.
+					 *
+					 *	If the OCSP checks succeed, check whether we still want to
+					 *	run the external verification routine.  If it's marked as
+					 *	"skip verify on OK", then we don't do verify.
+					 */
+					my_ok = ocsp_check(request, ocsp_store, issuer_cert, client_cert, conf);
+					if (my_ok != OCSP_STATUS_FAILED) {
+						do_verify = !conf->verify_skip_if_ocsp_ok;
+					}
+				}
 			}
 		}
 #endif
 
-		/*
-		 *	If OCSP returns fail (0), the certificate has expired.
-		 *	Don't run the verify routines/
-		 *
-		 *	If OCSP returns success (1), we MAY want to run the verify section.
-		 *	but only if verify_skip_if_ocsp_ok is false.
-		 *
-		 *	If OCSP returns skipped (2), we run the verify command, unless
-		 *	conf->verify_skip_if_ocsp_ok is true.
-		 */
 		if ((my_ok != OCSP_STATUS_FAILED)
 #ifdef HAVE_OPENSSL_OCSP_H
-		    && conf->ocsp_enable &&
-		    (((my_ok == OCSP_STATUS_OK) && !conf->verify_skip_if_ocsp_ok) ||
-		     ((my_ok == OCSP_STATUS_SKIPPED) && conf->verify_skip_if_ocsp_ok))
-
+		    && do_verify
 #endif
 			) while (conf->verify_client_cert_cmd) {
 			char filename[256];