* [RFC][PATCH] IP: Make ping sockets optional
@ 2014-04-23 15:20 Ben Hutchings
2014-04-23 15:30 ` Florian Westphal
` (2 more replies)
0 siblings, 3 replies; 13+ messages in thread
From: Ben Hutchings @ 2014-04-23 15:20 UTC (permalink / raw)
To: netdev; +Cc: Vasiliy Kulikov
[-- Attachment #1: Type: text/plain, Size: 7284 bytes --]
ICMP ping sockets currently require a new ping binary and are only
available after setting a sysctl. The code adds about 10K to the
(uncompressed) size of the kernel. Make this optional and disable
it by default.
This is compile-tested only; I'd just like to see whether people think
this is worth doing.
---
include/net/ping.h | 16 ++++++++++++++++
net/ipv4/Kconfig | 10 ++++++++++
net/ipv4/Makefile | 4 +++-
net/ipv4/af_inet.c | 6 ++++++
net/ipv4/sysctl_net_ipv4.c | 7 +++++++
net/ipv6/Makefile | 4 +++-
net/ipv6/af_inet6.c | 4 ++++
7 files changed, 49 insertions(+), 2 deletions(-)
diff --git a/include/net/ping.h b/include/net/ping.h
index 026479b..d980ea7 100644
--- a/include/net/ping.h
+++ b/include/net/ping.h
@@ -13,6 +13,8 @@
#ifndef _PING_H
#define _PING_H
+#ifdef CONFIG_INET_PING
+
#include <net/icmp.h>
#include <net/netns/hash.h>
@@ -108,4 +110,18 @@ void __init ping_init(void);
int __init pingv6_init(void);
void pingv6_exit(void);
+#else /* !CONFIG_INET_PING */
+
+static inline void ping_err(struct sk_buff *skb, int offset, u32 info) {}
+static inline void ping_rcv(struct sk_buff *skb) {}
+
+static inline int __init ping_proc_init(void) { return 0; }
+static inline void ping_proc_exit(void) {}
+
+static inline void __init ping_init(void) {}
+static inline int __init pingv6_init(void) { return 0; }
+static inline void pingv6_exit(void) {}
+
+#endif
+
#endif /* _PING_H */
diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
index 05c57f0..a3cb144 100644
--- a/net/ipv4/Kconfig
+++ b/net/ipv4/Kconfig
@@ -618,3 +618,13 @@ config TCP_MD5SIG
on the Internet.
If unsure, say N.
+
+config INET_PING
+ bool "ICMP: Ping sockets"
+ default n
+ ---help---
+ ICMP ping sockets allow 'ping' to be implemented without using a
+ raw socket, so it does not need to be installed setuid-root or
+ setcap. See <http://openwall.info/wiki/people/segoon/ping>.
+
+ If unsure, say N.
diff --git a/net/ipv4/Makefile b/net/ipv4/Makefile
index f032688..85f65fc 100644
--- a/net/ipv4/Makefile
+++ b/net/ipv4/Makefile
@@ -11,7 +11,7 @@ obj-y := route.o inetpeer.o protocol.o \
tcp_offload.o datagram.o raw.o udp.o udplite.o \
udp_offload.o arp.o icmp.o devinet.o af_inet.o igmp.o \
fib_frontend.o fib_semantics.o fib_trie.o \
- inet_fragment.o ping.o ip_tunnel_core.o gre_offload.o
+ inet_fragment.o ip_tunnel_core.o gre_offload.o
obj-$(CONFIG_NET_IP_TUNNEL) += ip_tunnel.o
obj-$(CONFIG_SYSCTL) += sysctl_net_ipv4.o
@@ -56,3 +56,5 @@ obj-$(CONFIG_NETLABEL) += cipso_ipv4.o
obj-$(CONFIG_XFRM) += xfrm4_policy.o xfrm4_state.o xfrm4_input.o \
xfrm4_output.o xfrm4_protocol.o
+
+obj-$(CONFIG_INET_PING) += ping.o
diff --git a/net/ipv4/af_inet.c b/net/ipv4/af_inet.c
index 8c54870..3819b1d 100644
--- a/net/ipv4/af_inet.c
+++ b/net/ipv4/af_inet.c
@@ -1016,6 +1016,7 @@ static struct inet_protosw inetsw_array[] =
.flags = INET_PROTOSW_PERMANENT,
},
+#ifdef CONFIG_INET_PING
{
.type = SOCK_DGRAM,
.protocol = IPPROTO_ICMP,
@@ -1024,6 +1025,7 @@ static struct inet_protosw inetsw_array[] =
.no_check = UDP_CSUM_DEFAULT,
.flags = INET_PROTOSW_REUSE,
},
+#endif
{
.type = SOCK_RAW,
@@ -1719,9 +1721,11 @@ static int __init inet_init(void)
if (rc)
goto out_unregister_udp_proto;
+#ifdef CONFIG_INET_PING
rc = proto_register(&ping_prot, 1);
if (rc)
goto out_unregister_raw_proto;
+#endif
/*
* Tell SOCKET that we are alive...
@@ -1810,8 +1814,10 @@ static int __init inet_init(void)
rc = 0;
out:
return rc;
+#ifdef CONFIG_INET_PING
out_unregister_raw_proto:
proto_unregister(&raw_prot);
+#endif
out_unregister_udp_proto:
proto_unregister(&udp_prot);
out_unregister_tcp_proto:
diff --git a/net/ipv4/sysctl_net_ipv4.c b/net/ipv4/sysctl_net_ipv4.c
index 44eba05..bf87094 100644
--- a/net/ipv4/sysctl_net_ipv4.c
+++ b/net/ipv4/sysctl_net_ipv4.c
@@ -39,8 +39,10 @@ static int ip_ttl_min = 1;
static int ip_ttl_max = 255;
static int tcp_syn_retries_min = 1;
static int tcp_syn_retries_max = MAX_TCP_SYNCNT;
+#ifdef CONFIG_INET_PING
static int ip_ping_group_range_min[] = { 0, 0 };
static int ip_ping_group_range_max[] = { GID_T_MAX, GID_T_MAX };
+#endif
/* Update system visible IP port range */
static void set_local_port_range(struct net *net, int range[2])
@@ -82,6 +84,7 @@ static int ipv4_local_port_range(struct ctl_table *table, int write,
return ret;
}
+#ifdef CONFIG_INET_PING
static void inet_get_ping_group_range_table(struct ctl_table *table, kgid_t *low, kgid_t *high)
{
@@ -145,6 +148,8 @@ static int ipv4_ping_group_range(struct ctl_table *table, int write,
return ret;
}
+#endif /* CONFIG_INET_PING */
+
static int proc_tcp_congestion_control(struct ctl_table *ctl, int write,
void __user *buffer, size_t *lenp, loff_t *ppos)
{
@@ -803,6 +808,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = proc_dointvec
},
+#ifdef CONFIG_INET_PING
{
.procname = "ping_group_range",
.data = &init_net.ipv4.sysctl_ping_group_range,
@@ -810,6 +816,7 @@ static struct ctl_table ipv4_net_table[] = {
.mode = 0644,
.proc_handler = ipv4_ping_group_range,
},
+#endif
{
.procname = "tcp_ecn",
.data = &init_net.ipv4.sysctl_tcp_ecn,
diff --git a/net/ipv6/Makefile b/net/ipv6/Makefile
index 2fe6836..b8e0488 100644
--- a/net/ipv6/Makefile
+++ b/net/ipv6/Makefile
@@ -7,7 +7,7 @@ obj-$(CONFIG_IPV6) += ipv6.o
ipv6-objs := af_inet6.o anycast.o ip6_output.o ip6_input.o addrconf.o \
addrlabel.o \
route.o ip6_fib.o ipv6_sockglue.o ndisc.o udp.o udplite.o \
- raw.o icmp.o mcast.o reassembly.o tcp_ipv6.o ping.o \
+ raw.o icmp.o mcast.o reassembly.o tcp_ipv6.o \
exthdrs.o datagram.o ip6_flowlabel.o inet6_connection_sock.o
ipv6-offload := ip6_offload.o tcpv6_offload.o udp_offload.o exthdrs_offload.o
@@ -45,3 +45,5 @@ obj-y += addrconf_core.o exthdrs_core.o ip6_checksum.o ip6_icmp.o
obj-$(CONFIG_INET) += output_core.o protocol.o $(ipv6-offload)
obj-$(subst m,y,$(CONFIG_IPV6)) += inet6_hashtables.o
+
+obj-$(CONFIG_INET_PING) += ping.o
diff --git a/net/ipv6/af_inet6.c b/net/ipv6/af_inet6.c
index d935889..7599ed8 100644
--- a/net/ipv6/af_inet6.c
+++ b/net/ipv6/af_inet6.c
@@ -861,9 +861,11 @@ static int __init inet6_init(void)
if (err)
goto out_unregister_udplite_proto;
+#ifdef CONFIG_INET_PING
err = proto_register(&pingv6_prot, 1);
if (err)
goto out_unregister_ping_proto;
+#endif
/* We MUST register RAW sockets before we create the ICMP6,
* IGMP6, or NDISC control sockets.
@@ -1021,8 +1023,10 @@ register_pernet_fail:
rtnl_unregister_all(PF_INET6);
out_sock_register_fail:
rawv6_exit();
+#ifdef CONFIG_INET_PING
out_unregister_ping_proto:
proto_unregister(&pingv6_prot);
+#endif
out_unregister_raw_proto:
proto_unregister(&rawv6_prot);
out_unregister_udplite_proto:
--
Ben Hutchings
Beware of programmers who carry screwdrivers. - Leonard Brandwein
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 15:20 [RFC][PATCH] IP: Make ping sockets optional Ben Hutchings
@ 2014-04-23 15:30 ` Florian Westphal
2014-04-23 15:55 ` Ben Hutchings
2014-04-23 16:05 ` David Laight
2014-04-24 15:59 ` Lorenzo Colitti
2 siblings, 1 reply; 13+ messages in thread
From: Florian Westphal @ 2014-04-23 15:30 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Vasiliy Kulikov
Ben Hutchings <ben@decadent.org.uk> wrote:
> ICMP ping sockets currently require a new ping binary and are only
> available after setting a sysctl. The code adds about 10K to the
> (uncompressed) size of the kernel. Make this optional and disable
> it by default.
>
> This is compile-tested only; I'd just like to see whether people think
> this is worth doing.
Either this feature needs to be ripped out now or it has to be kept.
I do not think the middle-ground you're proposing will work:
If userspace cannot safely assume facility is available, binaries
need to be setuid to implement fallback, and when you go down that
road you've gained nothing and might as well not support ping sockets
at all.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 15:30 ` Florian Westphal
@ 2014-04-23 15:55 ` Ben Hutchings
2014-04-23 16:27 ` Florian Westphal
0 siblings, 1 reply; 13+ messages in thread
From: Ben Hutchings @ 2014-04-23 15:55 UTC (permalink / raw)
To: Florian Westphal; +Cc: netdev, Vasiliy Kulikov
[-- Attachment #1: Type: text/plain, Size: 1093 bytes --]
On Wed, 2014-04-23 at 17:30 +0200, Florian Westphal wrote:
> Ben Hutchings <ben@decadent.org.uk> wrote:
> > ICMP ping sockets currently require a new ping binary and are only
> > available after setting a sysctl. The code adds about 10K to the
> > (uncompressed) size of the kernel. Make this optional and disable
> > it by default.
> >
> > This is compile-tested only; I'd just like to see whether people think
> > this is worth doing.
>
> Either this feature needs to be ripped out now or it has to be kept.
>
> I do not think the middle-ground you're proposing will work:
>
> If userspace cannot safely assume facility is available, binaries
> need to be setuid to implement fallback, and when you go down that
> road you've gained nothing and might as well not support ping sockets
> at all.
Userspace can't assume it now because access is controlled by a sysctl.
I think it is for distributions to choose whether to enable this feature
in ping and the kernel.
Ben.
--
Ben Hutchings
Beware of programmers who carry screwdrivers. - Leonard Brandwein
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* RE: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 15:20 [RFC][PATCH] IP: Make ping sockets optional Ben Hutchings
2014-04-23 15:30 ` Florian Westphal
@ 2014-04-23 16:05 ` David Laight
2014-04-24 15:59 ` Lorenzo Colitti
2 siblings, 0 replies; 13+ messages in thread
From: David Laight @ 2014-04-23 16:05 UTC (permalink / raw)
To: 'Ben Hutchings', netdev; +Cc: Vasiliy Kulikov
From: Ben Hutchings
> ICMP ping sockets currently require a new ping binary and are only
> available after setting a sysctl. The code adds about 10K to the
> (uncompressed) size of the kernel. Make this optional and disable
> it by default.
10k seems a lot of code just to allow non-root programs to send and
receive ICMP echo requests.
I'd have thought that a few carefully places checks would allow the
normal 'ping' program (or applications using the same calls) work
without being root.
David
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 15:55 ` Ben Hutchings
@ 2014-04-23 16:27 ` Florian Westphal
2014-04-24 15:17 ` Hannes Frederic Sowa
0 siblings, 1 reply; 13+ messages in thread
From: Florian Westphal @ 2014-04-23 16:27 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Florian Westphal, netdev, Vasiliy Kulikov
Ben Hutchings <ben@decadent.org.uk> wrote:
> Userspace can't assume it now because access is controlled by a sysctl.
>
> I think it is for distributions to choose whether to enable this feature
> in ping and the kernel.
I am not (yet) buying this argument.
Saying 'you need to change sysctl foo for this to work' in a program manpage
is a lot different than 'you need to recompile the kernel'.
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 16:27 ` Florian Westphal
@ 2014-04-24 15:17 ` Hannes Frederic Sowa
2014-04-24 16:02 ` Ben Hutchings
0 siblings, 1 reply; 13+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 15:17 UTC (permalink / raw)
To: Florian Westphal; +Cc: Ben Hutchings, netdev, Vasiliy Kulikov
On Wed, Apr 23, 2014 at 06:27:12PM +0200, Florian Westphal wrote:
> Ben Hutchings <ben@decadent.org.uk> wrote:
> > Userspace can't assume it now because access is controlled by a sysctl.
> >
> > I think it is for distributions to choose whether to enable this feature
> > in ping and the kernel.
>
> I am not (yet) buying this argument.
>
> Saying 'you need to change sysctl foo for this to work' in a program manpage
> is a lot different than 'you need to recompile the kernel'.
Maybe we can make the Kconfig option depend on CONFIG_EMBEDDED so that we can
be sure people don't have man-pages on the device. ;)
Seriously, I think doing authorization check based on gids in a sysctl is
wrong. Switching over to capabilities seems to make this interface much
more useable to me. But we would need to make sure, that we don't suddenly
allow people to use those sockets where it was restricted previously.
Greetings,
Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-23 15:20 [RFC][PATCH] IP: Make ping sockets optional Ben Hutchings
2014-04-23 15:30 ` Florian Westphal
2014-04-23 16:05 ` David Laight
@ 2014-04-24 15:59 ` Lorenzo Colitti
2 siblings, 0 replies; 13+ messages in thread
From: Lorenzo Colitti @ 2014-04-24 15:59 UTC (permalink / raw)
To: Ben Hutchings; +Cc: netdev, Vasiliy Kulikov, YOSHIFUJI Hideaki, David Miller
On Thu, Apr 24, 2014 at 12:20 AM, Ben Hutchings <ben@decadent.org.uk> wrote:
> This is compile-tested only; I'd just like to see whether people think
> this is worth doing.
Here's a link to the original discussion about this in 2011:
http://markmail.org/thread/vcs2ui27k6yqbnmc
When I added IPv6 support to this code, I sent Yoshifuji-san patches
to iputils to use it. Unfortunately iputils hasn't seen a release
since then. I guess that's why a new binary is still required.
FWIW, Android uses this code because it does not allow setuid binaries
and so it's the only way to run ping on Android. Other distributions
(like openwall) probably have similar requirements. Of course that
doesn't mean it can't be optional. It would be nice if it wasn't
ripped out though :-)
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-24 15:17 ` Hannes Frederic Sowa
@ 2014-04-24 16:02 ` Ben Hutchings
2014-04-24 16:37 ` Hannes Frederic Sowa
0 siblings, 1 reply; 13+ messages in thread
From: Ben Hutchings @ 2014-04-24 16:02 UTC (permalink / raw)
To: Hannes Frederic Sowa; +Cc: Florian Westphal, netdev, Vasiliy Kulikov
[-- Attachment #1: Type: text/plain, Size: 1327 bytes --]
On Thu, 2014-04-24 at 17:17 +0200, Hannes Frederic Sowa wrote:
> On Wed, Apr 23, 2014 at 06:27:12PM +0200, Florian Westphal wrote:
> > Ben Hutchings <ben@decadent.org.uk> wrote:
> > > Userspace can't assume it now because access is controlled by a sysctl.
> > >
> > > I think it is for distributions to choose whether to enable this feature
> > > in ping and the kernel.
> >
> > I am not (yet) buying this argument.
> >
> > Saying 'you need to change sysctl foo for this to work' in a program manpage
> > is a lot different than 'you need to recompile the kernel'.
>
> Maybe we can make the Kconfig option depend on CONFIG_EMBEDDED so that we can
> be sure people don't have man-pages on the device. ;)
>
> Seriously, I think doing authorization check based on gids in a sysctl is
> wrong.
It is quite weird but perhaps made sense in the context of some embedded
systems.
> Switching over to capabilities seems to make this interface much
> more useable to me. But we would need to make sure, that we don't suddenly
> allow people to use those sockets where it was restricted previously.
Standard ping could already be implemented as setcap (CAP_NET_RAW). You
want a capability just for ping?
Ben.
--
Ben Hutchings
Beware of programmers who carry screwdrivers. - Leonard Brandwein
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-24 16:02 ` Ben Hutchings
@ 2014-04-24 16:37 ` Hannes Frederic Sowa
2014-04-25 10:37 ` Lorenzo Colitti
0 siblings, 1 reply; 13+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-24 16:37 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Florian Westphal, netdev, Vasiliy Kulikov, lorenzo
On Thu, Apr 24, 2014 at 05:02:00PM +0100, Ben Hutchings wrote:
> On Thu, 2014-04-24 at 17:17 +0200, Hannes Frederic Sowa wrote:
> > On Wed, Apr 23, 2014 at 06:27:12PM +0200, Florian Westphal wrote:
> > > Ben Hutchings <ben@decadent.org.uk> wrote:
> > > > Userspace can't assume it now because access is controlled by a sysctl.
> > > >
> > > > I think it is for distributions to choose whether to enable this feature
> > > > in ping and the kernel.
> > >
> > > I am not (yet) buying this argument.
> > >
> > > Saying 'you need to change sysctl foo for this to work' in a program manpage
> > > is a lot different than 'you need to recompile the kernel'.
> >
> > Maybe we can make the Kconfig option depend on CONFIG_EMBEDDED so that we can
> > be sure people don't have man-pages on the device. ;)
> >
> > Seriously, I think doing authorization check based on gids in a sysctl is
> > wrong.
>
> It is quite weird but perhaps made sense in the context of some embedded
> systems.
The origins of this interface are in the openwall project. I assume
embedded devices were not that high up on their agenda.
> > Switching over to capabilities seems to make this interface much
> > more useable to me. But we would need to make sure, that we don't suddenly
> > allow people to use those sockets where it was restricted previously.
>
> Standard ping could already be implemented as setcap (CAP_NET_RAW). You
> want a capability just for ping?
That came to my mind at first, yes.
Hm, that's quite difficult:
I don't think we can stop respecting ping_group_range. So one possibility
is to just always allow icmp socket access if CAP_NET_RAW is in the
effective set *or* user is in a valid gid. But why should people switch
to icmp sockets and why should we add more code to iputils then if they
also have full CAP_NET_RAW?
We could be nitpicking and add a new capability, but I would be too
lazy to do that for the very little gain to give users only access to
ping/traceroute without giving access to the whole NET_RAW world.
We absolutely cannot abandon the interface as it already is in use by
android, as Lorenzo stated. Will android switch to file based capabilities
in some time? Is that possible?
I think I am in favour of the Kconfig option that it can be disabled or
compiled as a module (maybe only visible with CONFIG_EMBEDDED) and push that
on the deprecated list as file based capabilities made this socket type
unnecessary.
Any thoughts?
Bye,
Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-24 16:37 ` Hannes Frederic Sowa
@ 2014-04-25 10:37 ` Lorenzo Colitti
2014-04-25 21:18 ` Hannes Frederic Sowa
2014-04-25 21:43 ` Ben Hutchings
0 siblings, 2 replies; 13+ messages in thread
From: Lorenzo Colitti @ 2014-04-25 10:37 UTC (permalink / raw)
To: Ben Hutchings, Florian Westphal, netdev, Vasiliy Kulikov,
Lorenzo Colitti
On Fri, Apr 25, 2014 at 1:37 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
> The origins of this interface are in the openwall project. I assume
> embedded devices were not that high up on their agenda.
One of the original discussion threads I posted above has a link to a
lengthy discussion on why the original designers of this code thought
capabilities were not a good idea from a security standpoint.
> We absolutely cannot abandon the interface as it already is in use by
> android, as Lorenzo stated.
Well, the fact that it's in use by Android doesn't mean it can't be
made optional - Android can just turn the feature on in their kernels.
It would be unfortunate if it were to be removed entirely.
> Will android switch to file based capabilities
> in some time? Is that possible?
I think Android does support file capabilities. But this socket type
is not just for the ping binary. The fact that this socket type is
available to any binary allows any application developer to write an
app that can send ping packets. That seems like a useful capability
for a diagnostic app.
On the other hand, it seems to me that giving that same diagnostic app
CAP_NET_RAW would be unacceptable from a security point of view since
that app would now be able to sniff all traffic on the system, with
obvious privacy implications. There are also the usual security
concerns such as what if an exploit is discovered in the ping binary,
etc. etc.
What's the problem with this code? Is it just the 10KB in size?
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-25 10:37 ` Lorenzo Colitti
@ 2014-04-25 21:18 ` Hannes Frederic Sowa
2014-04-26 3:02 ` Lorenzo Colitti
2014-04-25 21:43 ` Ben Hutchings
1 sibling, 1 reply; 13+ messages in thread
From: Hannes Frederic Sowa @ 2014-04-25 21:18 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: Ben Hutchings, Florian Westphal, netdev, Vasiliy Kulikov
On Fri, Apr 25, 2014 at 07:37:02PM +0900, Lorenzo Colitti wrote:
> On Fri, Apr 25, 2014 at 1:37 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > The origins of this interface are in the openwall project. I assume
> > embedded devices were not that high up on their agenda.
>
> One of the original discussion threads I posted above has a link to a
> lengthy discussion on why the original designers of this code thought
> capabilities were not a good idea from a security standpoint.
Hmm, maybe I have overlooked it but I have not found any references to
capabilities.
> > We absolutely cannot abandon the interface as it already is in use by
> > android, as Lorenzo stated.
>
> Well, the fact that it's in use by Android doesn't mean it can't be
> made optional - Android can just turn the feature on in their kernels.
> It would be unfortunate if it were to be removed entirely.
>
> > Will android switch to file based capabilities
> > in some time? Is that possible?
>
> I think Android does support file capabilities. But this socket type
> is not just for the ping binary. The fact that this socket type is
> available to any binary allows any application developer to write an
> app that can send ping packets. That seems like a useful capability
> for a diagnostic app.
Ok, I see. There seem to be more users of this on Android. I guess ping
sockets are available to every application writer or will it be set
dynamically because of application permissions? Sorry, I am not that common
with android.
> On the other hand, it seems to me that giving that same diagnostic app
> CAP_NET_RAW would be unacceptable from a security point of view since
> that app would now be able to sniff all traffic on the system, with
> obvious privacy implications. There are also the usual security
> concerns such as what if an exploit is discovered in the ping binary,
> etc. etc.
Ack, that's why my first hunch was to introduce a new capability just for ping
sockets. I assume this wouldn't work for android?
> What's the problem with this code? Is it just the 10KB in size?
I thought it was mostly unused. But now I heard that android uses it,
this is actually not true any more.
Bye,
Hannes
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-25 10:37 ` Lorenzo Colitti
2014-04-25 21:18 ` Hannes Frederic Sowa
@ 2014-04-25 21:43 ` Ben Hutchings
1 sibling, 0 replies; 13+ messages in thread
From: Ben Hutchings @ 2014-04-25 21:43 UTC (permalink / raw)
To: Lorenzo Colitti; +Cc: Florian Westphal, netdev, Vasiliy Kulikov
[-- Attachment #1: Type: text/plain, Size: 2356 bytes --]
On Fri, 2014-04-25 at 19:37 +0900, Lorenzo Colitti wrote:
> On Fri, Apr 25, 2014 at 1:37 AM, Hannes Frederic Sowa
> <hannes@stressinduktion.org> wrote:
> > The origins of this interface are in the openwall project. I assume
> > embedded devices were not that high up on their agenda.
>
> One of the original discussion threads I posted above has a link to a
> lengthy discussion on why the original designers of this code thought
> capabilities were not a good idea from a security standpoint.
Well, the use of gids for access control to global resources fits into
Android's security model very nicely. I think it's quite unsuitable for
most other distributions.
> > We absolutely cannot abandon the interface as it already is in use by
> > android, as Lorenzo stated.
>
> Well, the fact that it's in use by Android doesn't mean it can't be
> made optional - Android can just turn the feature on in their kernels.
> It would be unfortunate if it were to be removed entirely.
Which I'm not proposing at all.
> > Will android switch to file based capabilities
> > in some time? Is that possible?
>
> I think Android does support file capabilities. But this socket type
> is not just for the ping binary. The fact that this socket type is
> available to any binary allows any application developer to write an
> app that can send ping packets. That seems like a useful capability
> for a diagnostic app.
>
> On the other hand, it seems to me that giving that same diagnostic app
> CAP_NET_RAW would be unacceptable from a security point of view since
> that app would now be able to sniff all traffic on the system, with
> obvious privacy implications. There are also the usual security
> concerns such as what if an exploit is discovered in the ping binary,
> etc. etc.
>
> What's the problem with this code? Is it just the 10KB in size?
It's 10K of code to do a very simple job... which is then disabled, so
that it's complete dead weight in most distributions. (And it turned
out to open a new security hole for those that did enable it.)
As it happens, though, I just had to drop Debian's ixp4xx kernel due to
size limitations, and disabling ping sockets would fix that at least in
the short term.
Ben.
--
Ben Hutchings
Beware of programmers who carry screwdrivers. - Leonard Brandwein
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 811 bytes --]
^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [RFC][PATCH] IP: Make ping sockets optional
2014-04-25 21:18 ` Hannes Frederic Sowa
@ 2014-04-26 3:02 ` Lorenzo Colitti
0 siblings, 0 replies; 13+ messages in thread
From: Lorenzo Colitti @ 2014-04-26 3:02 UTC (permalink / raw)
To: Lorenzo Colitti, Ben Hutchings, Florian Westphal, netdev,
Vasiliy Kulikov
On Sat, Apr 26, 2014 at 6:18 AM, Hannes Frederic Sowa
<hannes@stressinduktion.org> wrote:
>> One of the original discussion threads I posted above has a link to a
>> lengthy discussion on why the original designers of this code thought
>> capabilities were not a good idea from a security standpoint.
>
> Hmm, maybe I have overlooked it but I have not found any references to
> capabilities.
I thought I had linked to that, but perhaps I hadn't. This was
proposed for the first time in 2010:
http://thread.gmane.org/gmane.linux.kernel/1079723 . The discussion on
capabilities is linked a few messages down,
http://www.openwall.com/lists/oss-security/2010/11/08/3 .
> Ok, I see. There seem to be more users of this on Android. I guess ping
> sockets are available to every application writer or will it be set
> dynamically because of application permissions? Sorry, I am not that common
> with android.
Android sets ping_group_range to "allow everything". Access to general
network connectivity is restricted by Android application permissions,
so if an application has that, it can send pings.
> Ack, that's why my first hunch was to introduce a new capability just for ping
> sockets. I assume this wouldn't work for android?
Android userspace would need to be changed to use it (to give it to
everything that has network access permissions). I don't know if
there's a way to do that today.
Also, if you introduced a new capability for this, I think the 10KB
extra code would still be there. Because ping sockets don't allow you
to send/receive arbitrary ICMP messages, they only allow ping packets.
So "a new capability to allow ping" would have to be similarly
restricted, which means a lot of the code would still have to be
there.
^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2014-04-26 3:02 UTC | newest]
Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-04-23 15:20 [RFC][PATCH] IP: Make ping sockets optional Ben Hutchings
2014-04-23 15:30 ` Florian Westphal
2014-04-23 15:55 ` Ben Hutchings
2014-04-23 16:27 ` Florian Westphal
2014-04-24 15:17 ` Hannes Frederic Sowa
2014-04-24 16:02 ` Ben Hutchings
2014-04-24 16:37 ` Hannes Frederic Sowa
2014-04-25 10:37 ` Lorenzo Colitti
2014-04-25 21:18 ` Hannes Frederic Sowa
2014-04-26 3:02 ` Lorenzo Colitti
2014-04-25 21:43 ` Ben Hutchings
2014-04-23 16:05 ` David Laight
2014-04-24 15:59 ` Lorenzo Colitti
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).