Re: [PATCH net-next 1/2] net_sched: don't do precise pkt_len computation for untrusted packets

[Jason Wang <jasowang@redhat.com>]

On 03/19/2013 08:58 PM, Eric Dumazet wrote:
> On Tue, 2013-03-19 at 05:10 -0700, Eric Dumazet wrote:
>> On Tue, 2013-03-19 at 17:25 +0800, Jason Wang wrote:
>>
>>> I believe before doing header check for untrusted packets, the only
>>> thing we can trust is skb->len and that's we've used before
>>> 1def9238d4aa2. But after that, we're trying to use unchecked or
>>> meaningless value (e.g gso_segs were reset to zero in
>>> tun/macvtap/packet), and guest then can utilize this to result a very
>>> huge (-1U) pkt_len by filling evil value in the header. Can all kinds of
>>> packet scheduler survive this kinds of possible DOS?
>> I would use the flow dissector to fix the transport header from all
>> DODGY providers.
>>
>> Daniel Borkmann is working on a patch serie adding nhoff into flow_keys,
>> and adding __skb_get_poff(const struct sk_buff *skb), for a BPF
>> extension we talked about in Copenhagen / Netfilter Workshop.
>>
>> You could then set the transport header offset to the right value.
>>
>> (and drop evil packets before they go further in the stack)
>>
>> if (gso_packet(skb)) {
>> 	u32 poff = __skb_get_poff(skb);
>>
>> 	if (!poff) {
>> 		drop_evil_packet(skb);
>> 	} else {
>> 		skb_set_transport_header(skb, poff);
>> 		...
>> 	}
>> }
>
> Oh well, no need to use __skb_get_poff() but plain skb_flow_dissect()
> (once patched to include thoff in struct flow_keys)
>
> struct flow_keys keys;
>
> if (!skb_flow_dissect(skb, &keys))
> 	goto drop;
>
> if ((gso_type & (SKB_GSO_TCPV4|SKB_GSO_TCPV6)) &&
>     keys.ip_proto != IP_PROTO_TCP)
> 	goto drop;
>
> skb_set_transport_header(skb, keys.thoff);

I was consider a just skb_reset_transport_header() here. Consider the
transport header maybe checked and reset during header check for packets
of gso or partial checksum. And bypass precise pkt len computation.

Some problems with skb_flow_dissect():

- it can only recognizes a subset of all ethernet protocols. The may
blocks guest who may want to use other protocol such as IPX.
- almost no check in the validity of the L4 protocol header which may be
used by qdisc_pkt_len_init(), which may still give a chance to evil
guest to use
- gso_segs were untouched (still zero)

Another method is doing header check here which needs more work.
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html