| Message ID | 1318889783-23183-1-git-send-email-zenczykowski@gmail.com |
|---|---|
| State | Rejected, archived |
| Delegated to: | David Miller |
| Headers | show |
I still think we need a more precise permission for this, but possibly not quite as specific as a separate capability just for transparent. Maybe RAW should be split into RAW_READ (eavesdropping) and RAW_WRITE (spoof). Either way, I'll leave that for another day.
From: Maciej Żenczykowski <zenczykowski@gmail.com> Date: Mon, 17 Oct 2011 15:16:23 -0700 > From: Maciej Żenczykowski <maze@google.com> > > Up till now the IP{,V6}_TRANSPARENT socket options (which actually set > the same bit in the socket struct) have required CAP_NET_ADMIN > privileges to set or clear the option. > > - we make clearing the bit not require any privileges. > - we deprecate using CAP_NET_ADMIN for this purpose. > - we allow CAP_NET_RAW to set this bit, because raw > sockets already effectively allow you to emulate socket > transparency. > - we print a warning (but allow it) if you try to set the socket > option with CAP_NET_ADMIN privs, but without CAP_NET_RAW. > > Signed-off-by: Maciej Żenczykowski <maze@google.com> Warnings for something that has worked ever since the feature was added, and in fact was the only way to make use of the feature, is terrible. You must support the status quo forever or else you risk breaking existing setups. So the warning is pointless, you'll never be able to remove CAP_NET_ADMIN from these code paths, so there is zero value in warning about it because we'll never change this. I'm disliking these changes more and more. I refuse to apply this patch. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Are you okay with the patch without any warnings or deprecation markings? Or are you against opening up CAP_NET_RAW to this in general? On Wed, Oct 19, 2011 at 4:34 PM, David Miller <davem@davemloft.net> wrote: > From: Maciej Żenczykowski <zenczykowski@gmail.com> > Date: Mon, 17 Oct 2011 15:16:23 -0700 > >> From: Maciej Żenczykowski <maze@google.com> >> >> Up till now the IP{,V6}_TRANSPARENT socket options (which actually set >> the same bit in the socket struct) have required CAP_NET_ADMIN >> privileges to set or clear the option. >> >> - we make clearing the bit not require any privileges. >> - we deprecate using CAP_NET_ADMIN for this purpose. >> - we allow CAP_NET_RAW to set this bit, because raw >> sockets already effectively allow you to emulate socket >> transparency. >> - we print a warning (but allow it) if you try to set the socket >> option with CAP_NET_ADMIN privs, but without CAP_NET_RAW. >> >> Signed-off-by: Maciej Żenczykowski <maze@google.com> > > Warnings for something that has worked ever since the feature was > added, and in fact was the only way to make use of the feature, is > terrible. > > You must support the status quo forever or else you risk breaking > existing setups. So the warning is pointless, you'll never be > able to remove CAP_NET_ADMIN from these code paths, so there is > zero value in warning about it because we'll never change this. > > I'm disliking these changes more and more. I refuse to apply this > patch. >
From: Maciej Żenczykowski <zenczykowski@gmail.com> Date: Wed, 19 Oct 2011 20:32:31 -0700 > Are you okay with the patch without any warnings or deprecation markings? > Or are you against opening up CAP_NET_RAW to this in general? I don't see any real benefit. If it has been decided that you can't create a new capability for tproxy, so that tasks can be segregated out of these more powerful networking capability levels, I simply don't see the point. A process with CAP_NET_RAW can spit whatever crap they want onto the network, and receive all packets with impunity. I can't see what this buys us at all, sorry. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> I don't see any real benefit. > > If it has been decided that you can't create a new capability for > tproxy, so that tasks can be segregated out of these more powerful > networking capability levels, I simply don't see the point. It indeed seems that transparent may be a little too specific a capability. Ultimately linux permissions are just not fine grained enough... unless you start using LSMs > A process with CAP_NET_RAW can spit whatever crap they want onto the > network, and receive all packets with impunity. Agreed. But it can do so via raw sockets, it cannot do so via normal udp/tcp/ip sockets. That's why I'd like to relax the permissions check on being able to switch a socket into transparent mode. A process with CAP_NET_RAW can already pretty much emulate that behaviour by using raw sockets - it just can't do that using the higher level, often more usable/useful socket/protocol apis. > I can't see what this buys us at all, sorry. See above. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Maciej Żenczykowski <zenczykowski@gmail.com> Date: Wed, 19 Oct 2011 21:31:06 -0700 >> A process with CAP_NET_RAW can spit whatever crap they want onto the >> network, and receive all packets with impunity. > > Agreed. But it can do so via raw sockets, it cannot do so via normal > udp/tcp/ip sockets. > That's why I'd like to relax the permissions check on being able to > switch a socket > into transparent mode. A process with CAP_NET_RAW can already pretty > much emulate > that behaviour by using raw sockets - it just can't do that using the > higher level, often more > usable/useful socket/protocol apis. > >> I can't see what this buys us at all, sorry. > > See above. Ok, I'm convinced, send me that patch without the warning messages. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/include/linux/capability.h b/include/linux/capability.h index c421123..ce34ae3 100644 --- a/include/linux/capability.h +++ b/include/linux/capability.h @@ -198,7 +198,7 @@ struct cpu_vfs_cap_data { /* Allow modification of routing tables */ /* Allow setting arbitrary process / process group ownership on sockets */ -/* Allow binding to any address for transparent proxying */ +/* Allow binding to any address for transparent proxying (deprecated) */ /* Allow setting TOS (type of service) */ /* Allow setting promiscuous mode */ /* Allow clearing driver statistics */ @@ -210,6 +210,7 @@ struct cpu_vfs_cap_data { /* Allow use of RAW sockets */ /* Allow use of PACKET sockets */ +/* Allow binding to any address for transparent proxying */ #define CAP_NET_RAW 13 diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index 8905e92..74f7d30 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -961,12 +961,24 @@ mc_msf_out: break; case IP_TRANSPARENT: - if (!capable(CAP_NET_ADMIN)) { - err = -EPERM; - break; - } if (optlen < 1) goto e_inval; + /* Always allow clearing the transparent proxy socket option. + * The pre-3.2 permission for setting this was CAP_NET_ADMIN, + * and this is still supported - but deprecated. As of Linux + * 3.2 the proper permission is CAP_NET_RAW. + */ + if (!!val && !capable(CAP_NET_RAW)) { + if (!capable(CAP_NET_ADMIN)) { + err = -EPERM; + break; + } + printk_once(KERN_WARNING "%s (%d): " + "deprecated: attempt to set socket option " + "IP_TRANSPARENT with CAP_NET_ADMIN but " + "without CAP_NET_RAW.\n", + current->comm, task_pid_nr(current)); + } inet->transparent = !!val; break; diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c index 2fbda5f..7c4f5ce 100644 --- a/net/ipv6/ipv6_sockglue.c +++ b/net/ipv6/ipv6_sockglue.c @@ -343,13 +343,26 @@ static int do_ipv6_setsockopt(struct sock *sk, int level, int optname, break; case IPV6_TRANSPARENT: - if (!capable(CAP_NET_ADMIN)) { - retv = -EPERM; - break; - } if (optlen < sizeof(int)) goto e_inval; - /* we don't have a separate transparent bit for IPV6 we use the one in the IPv4 socket */ + /* Always allow clearing the transparent proxy socket option. + * The pre-3.2 permission for setting this was CAP_NET_ADMIN, + * and this is still supported - but deprecated. As of Linux + * 3.2 the proper permission is CAP_NET_RAW. + */ + if (valbool && !capable(CAP_NET_RAW)) { + if (!capable(CAP_NET_ADMIN)) { + retv = -EPERM; + break; + } + printk_once(KERN_WARNING "%s (%d): " + "deprecated: attempt to set socket option " + "IPV6_TRANSPARENT with CAP_NET_ADMIN but " + "without CAP_NET_RAW.\n", + current->comm, task_pid_nr(current)); + } + /* we don't have a separate transparent bit for IPv6, + * so we just use the one in the IPv4 socket */ inet_sk(sk)->transparent = valbool; retv = 0; break;