netdev - Re: [PATCH ipsec-next 1/7] xfrm: add extack support to verify_newsa

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 20 Sep 2022 10:11:11 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Sabrina Dubroca <sd@...asysnail.net>
Cc:     netdev@...r.kernel.org, steffen.klassert@...unet.com
Subject: Re: [PATCH ipsec-next 1/7] xfrm: add extack support to
 verify_newsa_info

On Tue, 20 Sep 2022 17:55:28 +0200 Sabrina Dubroca wrote:
> 2022-09-19, 17:00:38 -0700, Jakub Kicinski wrote:
> > On Wed, 14 Sep 2022 19:04:00 +0200 Sabrina Dubroca wrote:  
> > >  	case IPPROTO_COMP:
> > > +		if (!attrs[XFRMA_ALG_COMP]) {
> > > +			NL_SET_ERR_MSG(extack, "Missing required attribute for COMP: COMP");
> > > +			goto out;
> > > +		}  
> > 
> > Did NL_SET_ERR_ATTR_MISS() make it to the xfrm tree?  
> 
> No, it hasn't. Thanks for the note, I hadn't seen those patches.

I figured you may not have seen them. Your call if using the new
constructs makes sense.

> It would only solve part of the problem here, since some cases need
> one of two possible attributes (AH needs AUTH or AUTH_TRUNC, ESP needs
> AEAD or CRYPT).
> 
> In this particular case, it's also a bit confusing because which
> attribute is required (or not allowed) depends on other parts of the
> configuration, so there isn't a way to express most of it outside of
> strings -- short of having netlink policies or extacks that can
> describe logical formulas, I guess.

I was considering adding "required" as part of policy validation, 
it would work in a couple of the simpler GENL cases. But I couldn't
think of a clean way which wouldn't require at least one linear policy
scan per message. Maybe the scan would not be a big deal, IDK.