From: Martin Willi <martin@strongswan.org>
Date: Wed, 12 Jan 2022 10:00:20 +0000 (+0100)
Subject: addrblock: Allow limiting validation depth of issuer addrblock extensions
X-Git-Tag: 5.9.6rc1~31
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=e3d1766aff96324a48612fa64e6fb36475c71afc;p=thirdparty%2Fstrongswan.git

addrblock: Allow limiting validation depth of issuer addrblock extensions

RFC3779 requires to validate the addrblocks of issuer certificates strictly,
that is, they must contain the extension and the claimed addrblock, up to
the root CA.

When working with third party root CAs that do not have the extension,
this makes using the plugin impossible. So add a depth setting that limits
the number of issuer certificates to check bottom-up towards the root CA.
A depth value of 0 disables any issuer check, the default value of -1
checks all issuers in the chain, keeping the existing behavior.

Closes strongswan/strongswan#860
---

diff --git a/conf/plugins/addrblock.opt b/conf/plugins/addrblock.opt
index e35e4c5ad6..af5a63d90f 100644
--- a/conf/plugins/addrblock.opt
+++ b/conf/plugins/addrblock.opt
@@ -6,3 +6,17 @@ charon.plugins.addrblock.strict = yes
 	to no, subject certificates issued without the addrblock extension are
 	accepted without any traffic selector checks and no policy is enforced
 	by the plugin.
+
+charon.plugins.addrblock.depth = -1
+	How deep towards the root CA to validate issuer cert addrblock extensions.
+
+	RFC3779 requires that all addrblocks claimed by a certificate must be
+	contained in the addrblock extension of the issuer certificate, up to
+	the root CA. The default depth setting of -1 enforces this.
+
+	In practice, third party (root) CAs may not contain the extension, making
+	the addrblock extension unusable under such CAs. By limiting the validation
+	depth, only a certain level of issuer certificates are validated for proper
+	addrblock extensions: A depth of 0 does not check any issuer certificate
+	extensions, a depth of 1 only the direct issuer of the end entity
+	certificate is checkend, and so on.
diff --git a/src/libcharon/plugins/addrblock/addrblock_validator.c b/src/libcharon/plugins/addrblock/addrblock_validator.c
index c5b634b02e..1fb98d0f23 100644
--- a/src/libcharon/plugins/addrblock/addrblock_validator.c
+++ b/src/libcharon/plugins/addrblock/addrblock_validator.c
@@ -38,13 +38,18 @@ struct private_addrblock_validator_t {
 	 * Whether to reject subject certificates not having a addrBlock extension
 	 */
 	bool strict;
+
+	/**
+	 * How deep to validate issuer parent addrBlock validity, -1 for full
+	 */
+	int depth;
 };
 
 /**
  * Do the addrblock check for two x509 plugins
  */
 static bool check_addrblock(private_addrblock_validator_t *this,
-							x509_t *subject, x509_t *issuer)
+							x509_t *subject, x509_t *issuer, u_int pathlen)
 {
 	bool subject_const, issuer_const, contained = TRUE;
 	enumerator_t *subject_enumerator, *issuer_enumerator;
@@ -62,6 +67,15 @@ static bool check_addrblock(private_addrblock_validator_t *this,
 		DBG1(DBG_CFG, "subject certificate lacks ipAddrBlocks extension");
 		return !this->strict;
 	}
+	if (this->depth >= 0 && this->depth <= pathlen)
+	{
+		/* at pathlen 0: skip for depth configuration == 0,
+		 * at pathlen 1: skip for depth configurations 0..1,
+		 * ... */
+		DBG1(DBG_CFG, "  skipping issuer ipAddrBlocks validation "
+					  "at pathlen %u", pathlen);
+		return TRUE;
+	}
 	if (!issuer_const)
 	{
 		DBG1(DBG_CFG, "issuer certificate lacks ipAddrBlocks extension");
@@ -103,7 +117,7 @@ METHOD(cert_validator_t, validate, bool,
 	if (subject->get_type(subject) == CERT_X509 &&
 		issuer->get_type(issuer) == CERT_X509)
 	{
-		if (!check_addrblock(this, (x509_t*)subject, (x509_t*)issuer))
+		if (!check_addrblock(this, (x509_t*)subject, (x509_t*)issuer, pathlen))
 		{
 			lib->credmgr->call_hook(lib->credmgr, CRED_HOOK_POLICY_VIOLATION,
 									subject);
@@ -135,6 +149,8 @@ addrblock_validator_t *addrblock_validator_create()
 		},
 		.strict = lib->settings->get_bool(lib->settings,
 						"%s.plugins.addrblock.strict", TRUE, lib->ns),
+		.depth = lib->settings->get_int(lib->settings,
+						"%s.plugins.addrblock.depth", -1, lib->ns),
 	);
 
 	return &this->public;