netdev - Re: [RFC 1/6] netlink: add nlmsg_validate_strict() & nla_validate

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] [day] [month] [year] [list]

Message-ID: <2a18960ea4b711c5611f0decb46f707ed25c27cd.camel@sipsolutions.net>
Date:   Fri, 22 Mar 2019 07:23:40 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     Florian Westphal <fw@...len.de>
Cc:     netdev@...r.kernel.org, David Ahern <dsa@...ulusnetworks.com>
Subject: Re: [RFC 1/6] netlink: add nlmsg_validate_strict() &
 nla_validate_strict()

On Thu, 2019-03-21 at 23:50 +0100, Florian Westphal wrote:
> Johannes Berg <johannes@...solutions.net> wrote:
> > + * nla_validate_strict - Strictly validate a stream of attributes
> > + * @head: head of attribute stream
> > + * @len: length of attribute stream
> > + * @maxtype: maximum attribute type to be expected
> > + * @policy: validation policy
> > + * @extack: extended ACK report struct
> > + *
> > + * Validates all attributes in the specified attribute stream against the
> > + * specified policy. Attributes with a type exceeding maxtype will be
> > + * ignored.
> 
>       ^^^^^^^^
> rejected?

Oops, right, I didn't pay attentino to the docs at all.

But anyway, I don't think I want to do this.

I'm tempted to do the following:

 * add an
   enum netlink_validation {
       NETLINK_VALIDATION_LIBERAL,    // old behaviour
       NETLINK_VALIDATION_STRICT_MSG, // current strict
       NETLINK_VALIDATION_STRICT,     // strict message & attribute
   };
 * add __*_parse()/__*_validate() that get a new argument from this enum
 * for all existing callers of *_parse()/*_validate() add a new inline
   *_parse_liberal()/*_validate_liberal() and replace all calls, using
   _LIBERAL
 * change all existing *_parse_strict() to a new *_parse_strict_msg()
   inline using _STRICT_MSG
 * re-introduce *_parse()/*_validate() as being fully _STRICT

Also, do this before the generic netlink changes, so generic netlink
never gets the intermediate "STRICT_MSG" level.

That addresses two things:

 1) my table from the cover letter would be - at least for genl - what
    I want it to be, for some rtnetlink commands we'd have "strict_msg"
    semantics

 2) Default of *_parse()/*_validate() becomes to be strict for new
    code, so we don't need to pay as much attention to it - it'll be
    easier to see if somebody adds a call explicitly calling the more
   liberal versions. I'm tempted to not even add inline wrappers for
   this reason but to just open-code the __*() versions with the enum
   value instead (it's spatch, after all).



PS: STRICT_MSG (currently _strict()) semantics are a bit strange because
an attribute type that's out of range is rejected, while one that's in
range but has no policy is accepted; yet the range is prone to change
all the time... The "strict_start_type" fixes that though, if applied.

johannes