a, .
On Mon, Jun 8, 2020 at 8:02 PM Tobias Brunner <tob...@strongswan.org> wrote:
>
> Hi Steffen, Xin,
>
> This change could be problematic. Actually, it's not really this one
> but the original one that causes the issue:
> > Fixes: 7cb8a93968e3 ("xfrm: Allow inserting policies with matching mark and
> > different priorities")
>
> However, because the code in xfrm_policy_mark_match() treated policies
> with the same mark/mask equal without considering the priority before
> this change, it wasn't apparent. The problem is that the code can now
> lead to duplicate policies, which can not correctly be removed or queried.
It's a problem that can't be removed, but not for being queried.
>
> That's because the priority is sent only in xfrm_userpolicy_info, which
> XFRM_MSG_NEWPOLICY and XFRM_MSG_UPDPOLICY expect, but not in
> xfrm_userpolicy_id, which is used to query and delete policies with
> XFRM_MSG_GETPOLICY and XFRM_MSG_DELPOLICY, respectively (the mark is
> sent with a separate attribute, which can be supplied to all commands).
> So we can only query/delete the duplicate policy with the highest
> priority. Such duplicates can even be created inadvertently via
> XFRM_MSG_UPDPOLICY if the priority of an existing policy should be
> changed, which worked fine so far.
priority doesn't exist in xfrm_userpolicy_id, and we can add
XFRMA_PRIORITY like XFRMA_MARK to fix it.
since we also take priority to make a policy unique, we can't update
this attribute.
>
> The latter currently happens when strongSwan e.g. replaces a trap or
> block policy with one that has templates assigned (those we install with
> a higher priority, by default), which uses XFRM_MSG_UPDPOLICY that
> doesn't update the existing policy anymore but creates a duplicate
> instead. Since only one XFRM_MSG_DELPOLICY is sent later when the
> policy is deleted, a duplicate policy remains (and we couldn't even
> delete the exact policy we wanted - the trap might be removed by the
> user before the regular policy - due to the missing priority field in
> xfrm_userpolicy_id).
I see.
>
> I guess we could workaround this issue in strongSwan by installing
> policies that share the same mark and selector with the same priority,
> so only one instance is ever installed in the kernel. But the inability
> to address the exact policy when querying/deleting still looks like a
> problem to me in general.
>
For deleting, yes, but for querying, I think it makes sense not to pass
the priority, and always get the policy with the highest priority.
We can separate the deleting path from the querying path when
XFRMA_PRIORITY attribute is set.
Is that enough for your case to only fix for the policy deleting?
