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

[Jakub Kicinski <kuba@kernel.org>]

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.