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 a24e7920523f3c998b868473f1454c09b750e295..3cb54a00e2627f803ea4d17d62a2e91d9895aba9 100644 (file)
--- 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,