netdev - Re: [PATCH RFC net-next] packet: always ensure that we pass hard_header_len bytes in skb

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:   Fri, 27 Jan 2017 15:51:54 -0500
From:   Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:     Sowmini Varadhan <sowmini.varadhan@...cle.com>
Cc:     David Miller <davem@...emloft.net>,
        Network Development <netdev@...r.kernel.org>
Subject: Re: [PATCH RFC net-next] packet: always ensure that we pass
 hard_header_len bytes in skb_headlen() to the driver

On Fri, Jan 27, 2017 at 3:06 PM, Sowmini Varadhan
<sowmini.varadhan@...cle.com> wrote:
> On (01/27/17 14:29), Willem de Bruijn wrote:
>>
>> As your patch state, the contract is that any packet delivered to a
>> driver has the entire L2 in its linear section. Drivers are not required
>> to be robust against shorter packets, so there is no reason to test
>> those.
>>
>> One option is to limit your fix to known fixed-header protocols.
>> In these cases hard_header_len is the minimum, so anything
>> smaller must be dropped.
>
> yes, but how would you you know that this is a fixed-header protocol
> or a var-hdrlen protocol? AIUI the hard_header_len itself will not
> tell you this info: it will be 77 for ax25, 14 for ethernet,
> but that does not tell me that ax25 is the "robust-er" driver
> with a min requirement of 21 for the hdrlen.

Right. Identifying the outliers is the hard part.

> That's why I was thinking of a IFF_L2_VARHDRLEN in the priv_flags
> of the net_device.
>
>> For protocols with variable header length it is fine to send packets
>> shorter than hard_header_len, even with corrupted content (i.e.,
>> even if they would fail that protocol's validate callback), as long as
>> they exceed the minimum length. ax25 already has a min length
>> check through its protocol-specific validate callback.
>
> Another option that comes to mind.. the real thorn-in-the-flesh
> here is the CAP_SYS_RAWIO check. Would it be a better idea to ask
> the test-suites (since they seem to be the major consumer of
> that path) to use a special PF_PACKET socket option instead, that

Introducing a sysctl has the same effect. It is not possible to
identify all callers dependent on the current ABI.

I see these options
- make capable() check conditional on sysctl (or interface flag, ..)
- limit capable() check to drivers with with .validate callback
- hardcode a list of known fixed length protocols that must fail
- let privileged applications shoot themselves in the foot (change nothing).

The first will break tests. Though with a runtime fix: flip the flag.

The second will break variable length header protocols unless
you exhaustively search for all variable length protocols and add
validate callbacks first.