Discussion:
banging on ping
(too old to reply)
Rob Landley
2017-07-17 10:33:17 UTC
Permalink
Raw Message
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.
socket(2) is restricted to the group range specified in
"/proc/sys/net/ipv4/ping_group_range". It is "1 0" by default, meaning
that nobody (not even root) may create ping sockets. Setting it to "100
100" would grant permissions to the single group (to either make
/sbin/ping g+s and owned by this group or to grant permissions to the
"netadmins" group), "0 4294967295" would enable it for the world, "100
4294967295" would enable it for the users, but not daemons.
This is why I was getting permission denied trying to test my code. If I
"sudo /bin/bash" and then "echo 0 65535 >
/proc/sys/net/ipv4/ping_group_range" my test code suddenly works.

Question 1: WHY THE HELL DID THEY DISABLE THIS? Normal users have been
able to ping from Linux forever (and still can), it just requires an
suid binary to do it. Why does the API to _remove_ this restriction have
this pointless safety catch REQUIRING ROOT TO ENABLE THE NON-ROOT API?

Question 2: Why not make the default so root can use this, so ping
implementations could switch over to the new API even if they still
require the suid bit to work during the transition period?

Question 3: Groups? Does anybody use groups post-y2k? UIDs, sure, but we
haven't been sharing these machines since the minicomputer days, what's
the point of making this depend on GID?

Question 4: It's under ipv4 but not under ipv6, but it works for ipv6 too?

Sigh. Anybody want to talk to the kernel guys to point a flamethrower at
this nonsense? I'm kinda burnt out dealing with them after
http://lkml.iu.edu/hypermail/linux/kernel/1705.2/06366.html (which dates
back to http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05742.html and
so on; I made a follow-up patch to check for that specific situation and
printk("Triggering workaround for obvious Debian bug.") but haven't got
the energy for a fourth go at dealing with those guys just yet.)

Sigh. I can implement a ping requiring suid, but... ouch? They MADE an
api not to need this, and then cripped that API for no apparent reason.
(If you wanna ping flood somebody you can do it with UDP?)

Lemme check in what I've done, anyway... Ok, there.

Rob
enh
2017-07-17 14:55:47 UTC
Permalink
Raw Message
Post by Rob Landley
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
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.
Post by Rob Landley
socket(2) is restricted to the group range specified in
"/proc/sys/net/ipv4/ping_group_range". It is "1 0" by default, meaning
that nobody (not even root) may create ping sockets. Setting it to "100
100" would grant permissions to the single group (to either make
/sbin/ping g+s and owned by this group or to grant permissions to the
"netadmins" group), "0 4294967295" would enable it for the world, "100
4294967295" would enable it for the users, but not daemons.
This is why I was getting permission denied trying to test my code. If I
"sudo /bin/bash" and then "echo 0 65535 >
/proc/sys/net/ipv4/ping_group_range" my test code suddenly works.
Question 1: WHY THE HELL DID THEY DISABLE THIS? Normal users have been
able to ping from Linux forever (and still can), it just requires an
suid binary to do it. Why does the API to _remove_ this restriction have
this pointless safety catch REQUIRING ROOT TO ENABLE THE NON-ROOT API?
Question 2: Why not make the default so root can use this, so ping
implementations could switch over to the new API even if they still
require the suid bit to work during the transition period?
Question 3: Groups? Does anybody use groups post-y2k? UIDs, sure, but we
haven't been sharing these machines since the minicomputer days, what's
the point of making this depend on GID?
Question 4: It's under ipv4 but not under ipv6, but it works for ipv6 too?
Sigh. Anybody want to talk to the kernel guys to point a flamethrower at
this nonsense? I'm kinda burnt out dealing with them after
http://lkml.iu.edu/hypermail/linux/kernel/1705.2/06366.html (which dates
back to http://lkml.iu.edu/hypermail/linux/kernel/1606.2/05742.html and
so on; I made a follow-up patch to check for that specific situation and
printk("Triggering workaround for obvious Debian bug.") but haven't got
the energy for a fourth go at dealing with those guys just yet.)
Sigh. I can implement a ping requiring suid, but... ouch? They MADE an
api not to need this, and then cripped that API for no apparent reason.
(If you wanna ping flood somebody you can do it with UDP?)
Lemme check in what I've done, anyway... Ok, there.
Rob
_______________________________________________
Toybox mailing list
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.
Rob Landley
2017-07-18 06:47:39 UTC
Permalink
Raw Message
Post by enh
Post by Rob Landley
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
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(&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
enh
2017-07-18 14:50:09 UTC
Permalink
Raw Message
Post by Rob Landley
Post by enh
Post by Rob Landley
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
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"
Post by Rob Landley
(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.)
--- 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.
Loading...