Re: [Toybox] [PATCH] netcat: make -l exit after handling a request

[Josh Gao]

Sorry for pestering, but have you gotten a chance to take a look at these
yet? We're starting to grow dependencies on some of these patches (tests
that want to be checked in Soon are being written against a checkout with
the patches manually applied)

On Fri, Jun 30, 2017 at 12:17 PM, Josh Gao  wrote:

> Updated the original patch to just jump to cleanup, instead of rearranging
> things.
>
> Also, polished up the proof of concept xgetaddrinfo (this needs a better
> name)
> patch, and used it to implement -4, -6 (plus another patch to do -u).
>
> On Thu, Jun 29, 2017 at 6:59 PM, enh  wrote:
>
>> ping? various networking folks are looking to add tests that use
>> netcat, and i'd rather start them off on toybox netcat rather than BSD
>> and then have to move them across to toybox later. (obviously there's
>> other missing stuff in toybox, but these patches are the only things
>> they actually need right now.)
>>
>> On Wed, Jun 28, 2017 at 3:47 PM, Josh Gao  wrote:
>> > On Sun, Jun 25, 2017 at 12:14 PM, Rob Landley  wrote:
>> >>
>> >> 1) switching it to use xconnect() which it predates, and which is hard
>> >> because the various users in tree all want slighty different things out
>> >> of the getaddrinfo() plumbing and I've made a couple attempts to
>> >> unify/genericize it but keep getting pulled alway by $DAYJOB crisis du
>> >> jour halfway through and forgetting what design problem details I was
>> >> halfway through solving and have to start over again...
>> >
>> >
>> > BTW, I took a quick look at this because we have users that want -4/-6
>> (and
>> > IPv6 support in general). `nc -s` makes it so that you can't use
>> xconnect
>> > because you don't know what to bind to until after you've resolved the
>> > target
>> > address. Something like xbind_and_connect might work, but there's also
>> > things
>> > that we might want to do in between socket and bind (e.g. setting
>> > SO_REUSEADDR).
>> >
>> > The thing that everyone really wants is a way to iterate over
>> getaddrinfo
>> > results;
>> > maybe that's what should be exposed? I have a rough proof of concept
>> patch
>> > attached that implements this and uses it in netcat.
>> >
>> > (There's also another edge case with -s: what happens if the host you
>> pass
>> > in
>> > resolves to multiple addresses? OpenBSD's netcat seems to just bind the
>> > first
>> > compatible address it resolves to, so we can maybe just ignore this.)
>> >
>> > -Josh
>> >
>> > ___
>> > Toybox mailing list
>> > Toybox@lists.landley.net
>> > http://lists.landley.net/listinfo.cgi/toybox-landley.net
>> >
>>
>>
>>
>> --
>> Elliott Hughes - http://who/enh - http://jessies.org/~enh/
>> Android native code/tools questions? Mail me/drop by/add me as a reviewer.
>>
>
>
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] banging on ping

[enh]

On Mon, Jul 17, 2017 at 11:47 PM, Rob Landley  wrote:
> On 07/17/2017 09:55 AM, enh wrote:
>> On Mon, Jul 17, 2017 at 3:33 AM, Rob Landley  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(>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(_user_ns, 1);
> -   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 0);
> +   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 0);
> +   net->ipv4.ping_group_range.range[1] = make_kgid(_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.
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net

Re: [Toybox] banging on ping

[Rob Landley]

On 07/17/2017 09:55 AM, enh wrote:
> On Mon, Jul 17, 2017 at 3:33 AM, Rob Landley  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"?

(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(>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(_user_ns, 1);
-   net->ipv4.ping_group_range.range[1] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[0] = make_kgid(_user_ns, 0);
+   net->ipv4.ping_group_range.range[1] = make_kgid(_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
___
Toybox mailing list
Toybox@lists.landley.net
http://lists.landley.net/listinfo.cgi/toybox-landley.net