netdev - Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto

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:   Tue, 26 Feb 2019 10:17:39 +0800
From:   Hangbin Liu <liuhangbin@...il.com>
To:     David Ahern <dsahern@...il.com>
Cc:     Sabrina Dubroca <sd@...asysnail.net>, netdev@...r.kernel.org,
        Roopa Prabhu <roopa@...ulusnetworks.com>,
        "David S . Miller" <davem@...emloft.net>
Subject: Re: [PATCH net] ipv4: Add ICMPv6 support when parse route ipproto

On Mon, Feb 25, 2019 at 11:46:51AM -0700, David Ahern wrote:
> On 2/25/19 4:14 AM, Sabrina Dubroca wrote:
> > 2019-02-25, 15:47:00 +0800, Hangbin Liu wrote:
> >> @@ -14,6 +15,7 @@ int rtm_getroute_parse_ip_proto(struct nlattr *attr, u8 *ip_proto,
> >>  	case IPPROTO_TCP:
> >>  	case IPPROTO_UDP:
> >>  	case IPPROTO_ICMP:
> >> +	case IPPROTO_ICMPV6:
> > 
> > Is IPPROTO_ICMPV6 supposed to be valid in the IPv4 code path calling
> > this function? 
> 
> no. I do not see how that makes sense.

I also thought about this issue. Currently we didn't check the ipproto in both
IPv4 and IPv6. You can set icmp in ip6 rules or icmpv6 in ipv4 rules.
This looks don't make any serious problem. It's just a user mis-configuration,
the kernel check the proto number and won't match normal IP/IPv6 headers.

But yes, we should make it more strict, do you think if I should add a new
rtm_getroute_parse_ip6_proto() function, or just add a family parameter
in previous function?

The #if IS_ENABLED(CONFIG_IPV6) guardian makes sense. I will fix it.

Thanks
Hangbin