Merge branch 'ocsp-constraints'

Limits cached OCSP verification to responses signed by the CA, a directly
delegated signer or a pre-installed OCSP responder certificate. Disables
auth config merge for revocation trust-chain strength checkin, as it breaks
CA constraints in some scenarios.

src/libstrongswan/plugins/revocation/revocation_validator.c

@@ -93,40 +93,92 @@ static certificate_t *fetch_ocsp(char *url, certificate_t *subject,
 /**
  * check the signature of an OCSP response
  */
-static bool verify_ocsp(ocsp_response_t *response, auth_cfg_t *auth)
+static bool verify_ocsp(ocsp_response_t *response, certificate_t *ca)
 {
 	certificate_t *issuer, *subject;
 	identification_t *responder;
 	ocsp_response_wrapper_t *wrapper;
 	enumerator_t *enumerator;
-	auth_cfg_t *current;
+	x509_t *x509;
-	bool verified = FALSE;
+	bool verified = FALSE, found = FALSE;
 
 	wrapper = ocsp_response_wrapper_create((ocsp_response_t*)response);
 	lib->credmgr->add_local_set(lib->credmgr, &wrapper->set, FALSE);
 
 	subject = &response->certificate;
 	responder = subject->get_issuer(subject);
-	enumerator = lib->credmgr->create_trusted_enumerator(lib->credmgr,
+
+	/* check OCSP response using CA or directly delegated OCSP signer */
+	enumerator = lib->credmgr->create_cert_enumerator(lib->credmgr, CERT_X509,
 													KEY_ANY, responder, FALSE);
-	while (enumerator->enumerate(enumerator, &issuer, &current))
+	while (enumerator->enumerate(enumerator, &issuer))
 	{
+		x509 = (x509_t*)issuer;
+		if (!issuer->get_validity(issuer, NULL, NULL, NULL))
+		{	/* OCSP signer currently invalid */
+			continue;
+		}
+		found = TRUE;
+		if (!ca->equals(ca, issuer))
+		{	/* delegated OCSP signer? */
+			if (!lib->credmgr->issued_by(lib->credmgr, issuer, ca, NULL))
+			{	/* OCSP response not signed by CA, nor delegated OCSP signer */
+				continue;
+			}
+			if (!(x509->get_flags(x509) & X509_OCSP_SIGNER))
+			{	/* delegated OCSP signer does not have OCSP signer flag */
+				continue;
+			}
+		}
 		if (lib->credmgr->issued_by(lib->credmgr, subject, issuer, NULL))
 		{
 			DBG1(DBG_CFG, "  ocsp response correctly signed by \"%Y\"",
-							 issuer->get_subject(issuer));
+				 issuer->get_subject(issuer));
-			if (auth)
-			{
-				auth->merge(auth, current, FALSE);
-			}
 			verified = TRUE;
 			break;
 		}
+		DBG1(DBG_CFG, "ocsp response verification failed, "
+			 "invalid signature");
 	}
 	enumerator->destroy(enumerator);
 
+	if (!verified)
+	{
+		/* as fallback, use any locally installed OCSP signer certificate */
+		enumerator = lib->credmgr->create_cert_enumerator(lib->credmgr,
+										CERT_X509, KEY_ANY, responder, TRUE);
+		while (enumerator->enumerate(enumerator, &issuer))
+		{
+			x509 = (x509_t*)issuer;
+			/* while issued_by() accepts both OCSP signer or CA basic
+			 * constraint flags to verify OCSP responses, unrelated but trusted
+			 * OCSP signers must explicitly have the OCSP signer flag set. */
+			if ((x509->get_flags(x509) & X509_OCSP_SIGNER) &&
+				issuer->get_validity(issuer, NULL, NULL, NULL))
+			{
+				found = TRUE;
+				if (lib->credmgr->issued_by(lib->credmgr, subject, issuer, NULL))
+				{
+					DBG1(DBG_CFG, "  ocsp response correctly signed by \"%Y\"",
+						 issuer->get_subject(issuer));
+					verified = TRUE;
+					break;
+				}
+				DBG1(DBG_CFG, "ocsp response verification failed, "
+					 "invalid signature");
+			}
+		}
+		enumerator->destroy(enumerator);
+	}
+
 	lib->credmgr->remove_local_set(lib->credmgr, &wrapper->set);
 	wrapper->destroy(wrapper);
+
+	if (!found)
+	{
+		DBG1(DBG_CFG, "ocsp response verification failed, "
+			 "no signer certificate '%Y' found", responder);
+	}
 	return verified;
 }
 
@@ -134,8 +186,8 @@ static bool verify_ocsp(ocsp_response_t *response, auth_cfg_t *auth)
  * Get the better of two OCSP responses, and check for usable OCSP info
  */
 static certificate_t *get_better_ocsp(certificate_t *cand, certificate_t *best,
-					x509_t *subject, x509_t *issuer, cert_validation_t *valid,
+									  x509_t *subject, x509_t *issuer,
-					auth_cfg_t *auth, bool cache)
+									  cert_validation_t *valid, bool cache)
 {
 	ocsp_response_t *response;
 	time_t revocation, this_update, next_update, valid_until;
@@ -145,9 +197,8 @@ static certificate_t *get_better_ocsp(certificate_t *cand, certificate_t *best,
 	response = (ocsp_response_t*)cand;
 
 	/* check ocsp signature */
-	if (!verify_ocsp(response, auth))
+	if (!verify_ocsp(response, &issuer->interface))
 	{
-		DBG1(DBG_CFG, "ocsp response verification failed");
 		cand->destroy(cand);
 		return best;
 	}
@@ -226,8 +277,7 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
 	while (enumerator->enumerate(enumerator, &current))
 	{
 		current->get_ref(current);
-		best = get_better_ocsp(current, best, subject, issuer,
+		best = get_better_ocsp(current, best, subject, issuer, &valid, FALSE);
-							   &valid, auth, FALSE);
 		if (best && valid != VALIDATION_STALE)
 		{
 			DBG1(DBG_CFG, "  using cached ocsp response");
@@ -254,7 +304,7 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
 			if (current)
 			{
 				best = get_better_ocsp(current, best, subject, issuer,
-									   &valid, auth, TRUE);
+									   &valid, TRUE);
 				if (best && valid != VALIDATION_STALE)
 				{
 					break;
@@ -276,7 +326,7 @@ static cert_validation_t check_ocsp(x509_t *subject, x509_t *issuer,
 			if (current)
 			{
 				best = get_better_ocsp(current, best, subject, issuer,
-									   &valid, auth, TRUE);
+									   &valid, TRUE);
 				if (best && valid != VALIDATION_STALE)
 				{
 					break;
@@ -330,25 +380,20 @@ static certificate_t* fetch_crl(char *url)
 /**
  * check the signature of an CRL
  */
-static bool verify_crl(certificate_t *crl, auth_cfg_t *auth)
+static bool verify_crl(certificate_t *crl)
 {
 	certificate_t *issuer;
 	enumerator_t *enumerator;
 	bool verified = FALSE;
-	auth_cfg_t *current;
 
 	enumerator = lib->credmgr->create_trusted_enumerator(lib->credmgr,
 										KEY_ANY, crl->get_issuer(crl), FALSE);
-	while (enumerator->enumerate(enumerator, &issuer, &current))
+	while (enumerator->enumerate(enumerator, &issuer, NULL))
 	{
 		if (lib->credmgr->issued_by(lib->credmgr, crl, issuer, NULL))
 		{
 			DBG1(DBG_CFG, "  crl correctly signed by \"%Y\"",
 						   issuer->get_subject(issuer));
-			if (auth)
-			{
-				auth->merge(auth, current, FALSE);
-			}
 			verified = TRUE;
 			break;
 		}
@@ -362,7 +407,7 @@ static bool verify_crl(certificate_t *crl, auth_cfg_t *auth)
  * Get the better of two CRLs, and check for usable CRL info
  */
 static certificate_t *get_better_crl(certificate_t *cand, certificate_t *best,
-					x509_t *subject, cert_validation_t *valid, auth_cfg_t *auth,
+					x509_t *subject, cert_validation_t *valid,
 					bool cache, crl_t *base)
 {
 	enumerator_t *enumerator;
@@ -390,7 +435,7 @@ static certificate_t *get_better_crl(certificate_t *cand, certificate_t *best,
 	}
 
 	/* check CRL signature */
-	if (!verify_crl(cand, auth))
+	if (!verify_crl(cand))
 	{
 		DBG1(DBG_CFG, "crl response verification failed");
 		cand->destroy(cand);
@@ -452,8 +497,8 @@ static certificate_t *get_better_crl(certificate_t *cand, certificate_t *best,
  * Find or fetch a certificate for a given crlIssuer
  */
 static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
-								  auth_cfg_t *auth, crl_t *base,
+								  crl_t *base, certificate_t **best,
-								  certificate_t **best, bool *uri_found)
+								  bool *uri_found)
 {
 	cert_validation_t valid = VALIDATION_SKIPPED;
 	enumerator_t *enumerator;
@@ -466,8 +511,7 @@ static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
 	while (enumerator->enumerate(enumerator, &current))
 	{
 		current->get_ref(current);
-		*best = get_better_crl(current, *best, subject, &valid,
+		*best = get_better_crl(current, *best, subject, &valid, FALSE, base);
-							   auth, FALSE, base);
 		if (*best && valid != VALIDATION_STALE)
 		{
 			DBG1(DBG_CFG, "  using cached crl");
@@ -495,7 +539,7 @@ static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
 					continue;
 				}
 				*best = get_better_crl(current, *best, subject,
-									   &valid, auth, TRUE, base);
+									   &valid, TRUE, base);
 				if (*best && valid != VALIDATION_STALE)
 				{
 					break;
@@ -511,7 +555,7 @@ static cert_validation_t find_crl(x509_t *subject, identification_t *issuer,
  * Look for a delta CRL for a given base CRL
  */
 static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
-					crl_t *base, cert_validation_t base_valid, auth_cfg_t *auth)
+									crl_t *base, cert_validation_t base_valid)
 {
 	cert_validation_t valid = VALIDATION_SKIPPED;
 	certificate_t *best = NULL, *current;
@@ -526,7 +570,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
 	if (chunk.len)
 	{
 		id = identification_create_from_encoding(ID_KEY_ID, chunk);
-		valid = find_crl(subject, id, auth, base, &best, &uri);
+		valid = find_crl(subject, id, base, &best, &uri);
 		id->destroy(id);
 	}
 
@@ -537,7 +581,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
 	{
 		if (cdp->issuer)
 		{
-			valid = find_crl(subject, cdp->issuer, auth, base, &best, &uri);
+			valid = find_crl(subject, cdp->issuer, base, &best, &uri);
 		}
 	}
 	enumerator->destroy(enumerator);
@@ -558,8 +602,7 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
 				current->destroy(current);
 				continue;
 			}
-			best = get_better_crl(current, best, subject, &valid,
+			best = get_better_crl(current, best, subject, &valid, TRUE, base);
-								  auth, TRUE, base);
 			if (best && valid != VALIDATION_STALE)
 			{
 				break;
@@ -576,7 +619,6 @@ static cert_validation_t check_delta_crl(x509_t *subject, x509_t *issuer,
 	return base_valid;
 }
 
-
 /**
  * validate a x509 certificate using CRL
  */
@@ -597,7 +639,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
 	if (chunk.len)
 	{
 		id = identification_create_from_encoding(ID_KEY_ID, chunk);
-		valid = find_crl(subject, id, auth, NULL, &best, &uri_found);
+		valid = find_crl(subject, id, NULL, &best, &uri_found);
 		id->destroy(id);
 	}
 
@@ -608,8 +650,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
 	{
 		if (cdp->issuer)
 		{
-			valid = find_crl(subject, cdp->issuer, auth, NULL,
+			valid = find_crl(subject, cdp->issuer, NULL, &best, &uri_found);
-							 &best, &uri_found);
 		}
 	}
 	enumerator->destroy(enumerator);
@@ -633,7 +674,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
 					continue;
 				}
 				best = get_better_crl(current, best, subject, &valid,
-									  auth, TRUE, NULL);
+									  TRUE, NULL);
 				if (best && valid != VALIDATION_STALE)
 				{
 					break;
@@ -646,7 +687,7 @@ static cert_validation_t check_crl(x509_t *subject, x509_t *issuer,
 	/* look for delta CRLs */
 	if (best && (valid == VALIDATION_GOOD || valid == VALIDATION_STALE))
 	{
-		valid = check_delta_crl(subject, issuer, (crl_t*)best, valid, auth);
+		valid = check_delta_crl(subject, issuer, (crl_t*)best, valid);
 	}
 
 	/* an uri was found, but no result. switch validation state to failed */

testing/tests/ikev2/ocsp-no-signer-cert/evaltest.dat

@@ -1,5 +1,5 @@
 moon:: cat /var/log/daemon.log::requesting ocsp status from::YES
-moon:: cat /var/log/daemon.log::ocsp response verification failed::YES
+moon:: cat /var/log/daemon.log::ocsp response verification failed, no signer::YES
 moon:: cat /var/log/daemon.log::certificate status is not available::YES
 moon:: cat /var/log/daemon.log::constraint check failed: RULE_CRL_VALIDATION is FAILED, but requires at least GOOD::YES
 moon:: ipsec status 2> /dev/null::rw.*ESTABLISHED::NO

testing/tests/ikev2/ocsp-untrusted-cert/evaltest.dat

@@ -1,6 +1,5 @@
 moon:: cat /var/log/daemon.log::requesting ocsp status from::YES
-moon:: cat /var/log/daemon.log::self-signed certificate.*is not trusted::YES
+moon:: cat /var/log/daemon.log::ocsp response verification failed, no signer::YES
-moon:: cat /var/log/daemon.log::ocsp response verification failed::YES
 moon:: cat /var/log/daemon.log::certificate status is not available::YES
 moon:: cat /var/log/daemon.log::constraint check failed: RULE_CRL_VALIDATION is FAILED, but requires at least GOOD::YES
 moon:: ipsec status 2> /dev/null::rw.*ESTABLISHED::NO