[Toybox] banging on ping

[enh]

On Mon, Jul 17, 2017 at 11:47 PM, Rob Landley <rob at landley.net> wrote:
> On 07/17/2017 09:55 AM, enh wrote:
>> On Mon, Jul 17, 2017 at 3:33 AM, Rob Landley <rob at landley.net> wrote:
>>> Over the weekend I started looking at ping.c again thinking "this seems
>>> really easy, why haven't I already done it". And I figured out why (I
>>> wanted the code to autodetect ipv4 or ipv6 without you having to
>>> specify, but you could go "ping -I lo 127.0.0.1" and it could see ::1 as
>>> the first address of lo so you have to defer the decision of which type
>>> to use while detecting, AND I still wanted -4 and -6 to work to force
>>> the decision meaning it fails if source or dest can't do that, except
>>> supplying source address is optional.)
>>>
>>> So I finally untangled all that crap, and then I started in on the next
>>> thing I wantedit to do, use the "unprivileged ping sockets" stuff Linux
>>> merged back in 2011:
>>>
>>>   https://lwn.net/Articles/422330/
>>>
>>> It's almost been 7 years, no need to support the old "needs root" stuff
>>> if this should be ubiquitously deployed.
>>>
>>> Yes that description's wrong, there's no such thing as PROT_ICMP, they
>>> mean IPPROTO_ICMP but good luck finding example code using that because
>>> nobody uses it. Why does nobody use it? Because the API is stupidly
>>> disabled for no apparent reason.
>>
>> Android uses it all over the place. i even made it available to Java.
>>
>> in particular, external/iputils' ping/ping6 uses it.
>
> Did you patch the stupid out of the kernel, or does your init script
> just "echo 0 65535 > /proc/sys/net/ipv4/ping_group_range"?

the latter. there's this line in the default init script:

    write /proc/sys/net/ipv4/ping_group_range "0 2147483647"

> (Honestly, what's the point or creating a new API to do the same thing
> without requiring root access, and then not even let ROOT use it by
> default? I thought busybox was using this, but they yanked it back OUT
> in 2014: https://git.busybox.net/busybox/commit/?id=f0058b1b1fe9
> because of this nonsense.)
>
> Right. The kernel patch to fix this is:
>
> --- a/net/ipv4/af_inet.c
> +++ b/net/ipv4/af_inet.c
> @@ -1712,12 +1712,8 @@ static __net_init int inet_init_net(struct net *net)
>         net->ipv4.ip_local_ports.range[1] =  60999;
>
>         seqlock_init(&net->ipv4.ping_group_range.lock);
> -       /*
> -        * Sane defaults - nobody may create ping sockets.
> -        * Boot scripts should set this to distro-specific group.
> -        */
> -       net->ipv4.ping_group_range.range[0] = make_kgid(&init_user_ns, 1);
> -       net->ipv4.ping_group_range.range[1] = make_kgid(&init_user_ns, 0);
> +       net->ipv4.ping_group_range.range[0] = make_kgid(&init_user_ns, 0);
> +       net->ipv4.ping_group_range.range[1] = make_kgid(&init_user_ns, 65535);
>
>         /* Default values for sysctl-controlled parameters.
>          * We set them here, in case sysctl is not compiled.
>
> And I think I'll just put that patch in the toybox FAQ rather than
> implementing two APIs to do the same darn thing. I've asked the kernel
> guys what they were thinking (with my usual "I'm tracking down a bug
> and I found PEOPLE at the end of it" diplomacy, ala
> http://lkml.iu.edu/hypermail/linux/kernel/1707.2/01797.html ) and
> I have no _idea_ what they'll say because this makes no sense to me.
>
> Rob



-- 
Elliott Hughes - http://who/enh - http://jessies.org/~enh/
Android native code/tools questions? Mail me/drop by/add me as a reviewer.