netdev - Re: [devel-ipsec] [PATCH ipsec-next v6] xfrm: Add Direction to the SA in or out

Open Source and information security mailing list archives

Date: Sat, 6 Apr 2024 08:36:09 -0400
From: Christian Hopps <chopps@...pps.org>
To: Sabrina Dubroca <sd@...asysnail.net>
Cc: Christian Hopps <chopps@...pps.org>,
 Antony Antony <antony.antony@...unet.com>,
 Herbert Xu <herbert@...dor.apana.org.au>,
 netdev@...r.kernel.org,
 devel@...ux-ipsec.org,
 Eric Dumazet <edumazet@...gle.com>,
 Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>,
 "David S. Miller" <davem@...emloft.net>
Subject: Re: [devel-ipsec] [PATCH ipsec-next v6] xfrm: Add Direction to the SA
 in or out

> On Apr 5, 2024, at 17:56, Sabrina Dubroca via Devel <devel@...ux-ipsec.org> wrote:
> 
> Hi Antony,
> 
> 2024-04-05, 14:40:07 +0200, Antony Antony wrote:
>> This patch introduces the 'dir' attribute, 'in' or 'out', to the
>> xfrm_state, SA, enhancing usability by delineating the scope of values
>> based on direction. An input SA will now exclusively encompass values
>> pertinent to input, effectively segregating them from output-related
>> values.
> 
> But this patch isn't doing that for existing properties (I'm thinking
> of replay window, not sure if any others are relevant [1]). Why not?
> 
> [1] that should include values passed via xfrm_usersa_info too,
>    not just XFRMA_* attributes
> 
> Adding these checks should be safe (wrt breakage of API): Old software
> would not be passing XFRMA_SA_DIR, so adding checks when it is provided
> would not break anything there. Only new software using the attribute
> would benefit from having directed SAs and restriction on which attributes
> can be used (and that's fine).
> 
> Right now the new attribute is 100% duplicate of the existing offload
> direction, so I don't see much point.

A subset of the ipsec has requested that IPTFS utilize this new directional attribute as most of IPTFS config is send-only. So IPTFS is the planned first user and is also now blocked on this patch.

Chris.

> 
>> This change aims to streamline the configuration process and
>> improve the overall clarity of SA attributes.
>> 
>> This feature sets the groundwork for future patches, including
>> the upcoming IP-TFS patch.
>> 
>> Currently, dir is only allowed when HW OFFLOAD is set.
>> 
>> ---
> 
> BTW, everything after this '---' will get cut, including your sign-off.
> 
>> v5->v6:
>> - XFRMA_SA_DIR only allowed with HW OFFLOAD
>> 
>> v4->v5:
>> - add details to commit message
>> 
>> v3->v4:
>> - improve HW OFFLOAD DIR check check other direction
>> 
>> v2->v3:
>> - delete redundant XFRM_SA_DIR_USE
>> - use u8 for "dir"
>> - fix HW OFFLOAD DIR check
>> 
>> v1->v2:
>> - use .strict_start_type in struct nla_policy xfrma_policy
>> - delete redundant XFRM_SA_DIR_MAX enum
>> ---
>> 
>> Signed-off-by: Antony Antony <antony.antony@...unet.com>
>> Reviewed-by: Leon Romanovsky <leonro@...dia.com>
> 
> nit: If I'm making non-trivial changes to the contents of the patch, I
> typically drop the review (and test) tags I got on previous versions,
> since they may no longer apply.
> 
> -- 
> Sabrina
> 
> -- 
> Devel mailing list
> Devel@...ux-ipsec.org
> https://linux-ipsec.org/mailman/listinfo/devel
> 

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives