From: Tobias Brunner <tobias@strongswan.org>
Date: Fri, 4 Feb 2022 08:37:37 +0000 (+0100)
Subject: kernel-netlink: Allow reqid updates for policies again
X-Git-Tag: 5.9.6rc1~3^2~17
X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=96ecc39cd00f90175209606ebeabb8f7185afb1c;p=thirdparty%2Fstrongswan.git

kernel-netlink: Allow reqid updates for policies again

This was originally added with 1551d8b13d14 ("kernel-netlink: reject
policy refcount if the reqid differs").  Since then we added code to
allocate constant reqids for the same TS, which pretty much avoids the
previous issues.

However, the reqid might have to be changed due to MOBIKE updates. And
because reqids are allocated for a complete set of traffic selectors and
not individual pairs, this can create a problem with drop policies as
those will use the old reqid (they are installed with the same priority,
reqid etc. to replace the actual IPsec policies), while unmodified
replacement policies will use the new one.  A similar issue exists for
CHILD_SAs with SELinux contexts as those all use duplicate policies (same
generic label) but can't all be updated concurrently.
---

diff --git a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
index a24e792052..3cb54a00e2 100644
--- a/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
+++ b/src/libcharon/plugins/kernel_netlink/kernel_netlink_ipsec.c
@@ -2916,19 +2916,7 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
 	this->mutex->lock(this->mutex);
 	current = this->policies->get(this->policies, policy);
 	if (current)
-	{
-		if (current->reqid && data->sa->reqid &&
-			current->reqid != data->sa->reqid)
-		{
-			DBG1(DBG_CFG, "unable to install policy %R === %R %N%s%s for reqid "
-				 "%u, the same policy for reqid %u exists",
-				 id->src_ts, id->dst_ts, policy_dir_names, id->dir, markstr,
-				 labelstr, data->sa->reqid, current->reqid);
-			policy_entry_destroy(this, policy);
-			this->mutex->unlock(this->mutex);
-			return INVALID_STATE;
-		}
-		/* use existing policy */
+	{	/* use existing policy */
 		DBG2(DBG_KNL, "policy %R === %R %N%s%s already exists, increasing "
 			 "refcount", id->src_ts, id->dst_ts, policy_dir_names, id->dir,
 			 markstr, labelstr);
@@ -3000,7 +2988,13 @@ METHOD(kernel_ipsec_t, add_policy, status_t,
 			 id->dir, markstr, labelstr, cur_priority, use_count);
 		return SUCCESS;
 	}
-	policy->reqid = assigned_sa->sa->cfg.reqid;
+	if (policy->reqid != assigned_sa->sa->cfg.reqid)
+	{
+		DBG1(DBG_CFG, "updating reqid for policy %R === %R %N%s%s from %u "
+			 "to %u", id->src_ts, id->dst_ts, policy_dir_names, id->dir,
+			 markstr, labelstr, policy->reqid, assigned_sa->sa->cfg.reqid);
+		policy->reqid = assigned_sa->sa->cfg.reqid;
+	}
 
 	if (this->policy_update)
 	{
@@ -3208,7 +3202,13 @@ METHOD(kernel_ipsec_t, del_policy, status_t,
 			return SUCCESS;
 		}
 		current->used_by->get_first(current->used_by, (void**)&mapping);
-		current->reqid = mapping->sa->cfg.reqid;
+		if (current->reqid != mapping->sa->cfg.reqid)
+		{
+			DBG1(DBG_CFG, "updating reqid for policy %R === %R %N%s%s from %u "
+				 "to %u", id->src_ts, id->dst_ts, policy_dir_names, id->dir,
+				 markstr, labelstr, current->reqid, mapping->sa->cfg.reqid);
+			current->reqid = mapping->sa->cfg.reqid;
+		}
 
 		DBG2(DBG_KNL, "updating policy %R === %R %N%s%s [priority %u, "
 			 "refcount %d]", id->src_ts, id->dst_ts, policy_dir_names, id->dir,