netdev - [RFC 0/7] netlink/genetlink: stricter parsing

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

Message-Id: <20190321220504.7642-1-johannes@sipsolutions.net>
Date:   Thu, 21 Mar 2019 23:04:58 +0100
From:   Johannes Berg <johannes@...solutions.net>
To:     netdev@...r.kernel.org
Cc:     David Ahern <dsa@...ulusnetworks.com>
Subject: [RFC 0/7] netlink/genetlink: stricter parsing

It seems that everytime David and I meet, we have further ideas for
netlink ;-)

This time, we talked about attribute parsing and making policy parsing
(even) stricter.

I'm not entirely happy with this patchset, due to the mistake David made
when he added the strict parsing. At that time, the strict parsing *should*
have extended to all the attributes, in the way that I'm doing it here
with the strict_start_type (in the last patch).

The reason I'm still not happy with this is that it seems to me that
even for *existing* attributes, we should apply the strict checks for
*new* commands. So, in effect, I think we should have

                         strict parsing
command    attribute     attribute   message
 old        old          -            -
 old        new          X            -
 new        *            X            X

However, what we get with these patches, is only

                         strict parsing
command    attribute     attribute   message
 old        old          -            -
 old        new          X            -
 new        old          - (!)        X
 new        new          X            X

Note the marked difference.

I see two ways to fix this now:

1) Replace all existing usage of *_strict() calls again with the regular
   ones, and change the *_strict() calls to pass strict through to the
   validate_nla() function. This would be sort of a regression since all
   things that are actually strict now would no longer be. I think that
   would even be an API break.

2) Add another level of "very_strict" to use in new commands, which then
   passes the strict through to validate_nla(). This probably means we
   want to pass the "strictness" as an enum argument instead.

I'll take a shot at this later.

Further ideas we discussed:

 * Have ways to expose the policies to userspace, with generic netlink
   that's easy, with others perhaps not quite so easy; this would allow
   userspace to discover what the kernel supports.

 * Validate messages created by the kernel against the policy, for debug
   purposes. This is a bit harder because the policy is sometimes not even
   symmetric (input and output have different data), but that could be
   managed (e.g. have a list of override policy entries) and it'd help us
   validate things too.

johannes