netdev - Re: [PATCH v2 net] ipv6: Fix MLD Query message check

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]

Date:	Fri, 27 Jun 2014 09:56:43 +0800
From:	Hangbin Liu <liuhangbin@...il.com>
To:	David Miller <davem@...emloft.net>
Cc:	network dev <netdev@...r.kernel.org>
Subject: Re: [PATCH v2 net] ipv6: Fix MLD Query message check

2014-06-27 8:33 GMT+08:00 David Miller <davem@...emloft.net>:
> From: Hangbin Liu <liuhangbin@...il.com>
> Date: Wed, 25 Jun 2014 09:31:07 +0800
>
>> @@ -1301,8 +1301,18 @@ int igmp6_event_query(struct sk_buff *skb)
>>       len = ntohs(ipv6_hdr(skb)->payload_len) + sizeof(struct ipv6hdr);
>>       len -= skb_network_header_len(skb);
>>
>> -     /* Drop queries with not link local source */
>> -     if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL))
>> +     /* RFC3810 6.2
>> +      * Upon reception of an MLD message that contains a Query, the node
>> +      * checks if the source address of the message is a valid link-local
>> +      * address, if the Hop Limit is set to 1, and if the Router Alert
>> +      * option is present in the Hop-By-Hop Options header of the IPv6
>> +      * packet.  If any of these checks fails, the packet is dropped.
>> +      */
>> +     if (!(ipv6_addr_type(&ipv6_hdr(skb)->saddr) & IPV6_ADDR_LINKLOCAL) ||
>> +         ipv6_hdr(skb)->hop_limit != 1 ||
>> +         ipv6_hdr(skb)->nexthdr != NEXTHDR_HOP ||
>> +         !(IP6CB(skb)->flags & IP6SKB_ROUTERALERT) ||
>> +         IP6CB(skb)->ra != htons(IPV6_OPT_ROUTERALERT_MLD))
>>               return -EINVAL;
>>
>
> Does the NEXTHDR_HOP have the be exactly the next extension header
> present?  I think you need to allow for more flexibility here.
>
> In any event, the only way the IP6SKB_ROUTERALERT bit can be set
> is if there were hop-by-hop options present, so testing only the
> bit itself is sufficient.
>
> The forwarding path simplifies the check in this way too.
>
> So please adjust this to only check the bit and remove the NEXTHDR_HOP
> test.
>
Thanks for this advise, will send a v3 patch

Best regards
Hangbin Liu
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html