| Message ID | alpine.DEB.2.20.1605131051170.27372@east.gentwo.org (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 05/13/2016 11:52 AM, Christoph Lameter wrote: > Sorry slight mistake in the original patch. V2 follows > > > > Subject: [PATCH] IB/core: Do not require CAP_NET_ADMIN for sniffing V2 > > Having to enable CAP_NET_ADMIN for every app that uses sniffer mode is kind > of risky. We do not want people to have the ability to mess around with the > network configuration and routing. We just want the app to direct streams and > deal with inbound data streams in various ways. > > So lets drop the requirement for CAP_NET_ADMIN and keep just CAP_NET_RAW. > > V1->V2 > - Check for CAP_NET_ADMIN was conditional on IB_FLOW_ATTR_SNIFFER. We need > to remove this in the correct way. > - Update description > > > Signed-off-by: Christoph Lameter <cl@linux.com> > > Index: linux/drivers/infiniband/core/uverbs_cmd.c > =================================================================== > --- linux.orig/drivers/infiniband/core/uverbs_cmd.c 2016-03-24 09:16:27.782778586 -0500 > +++ linux/drivers/infiniband/core/uverbs_cmd.c 2016-05-13 10:49:28.953000945 -0500 > @@ -3088,8 +3088,7 @@ int ib_uverbs_ex_create_flow(struct ib_u > if (cmd.comp_mask) > return -EINVAL; > > - if ((cmd.flow_attr.type == IB_FLOW_ATTR_SNIFFER && > - !capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW)) > + if (!capable(CAP_NET_RAW)) > return -EPERM; > > if (cmd.flow_attr.flags >= IB_FLOW_ATTR_FLAGS_RESERVED) > I'm not at all convinced this is the right thing to do. Sniffing of packets is definitely a privileged operation. Tcpdump needs to be run as root to do this on regular devices. If not CAP_NET_ADMIN, then a root check seems appropriate. CAP_NET_RAW does not seem sufficient for sniffing other people's packets.
On Fri, 13 May 2016, Doug Ledford wrote: > > if (cmd.flow_attr.flags >= IB_FLOW_ATTR_FLAGS_RESERVED) > > > > I'm not at all convinced this is the right thing to do. Sniffing of > packets is definitely a privileged operation. Tcpdump needs to be run > as root to do this on regular devices. If not CAP_NET_ADMIN, then a > root check seems appropriate. CAP_NET_RAW does not seem sufficient for > sniffing other people's packets. CAP_NET_RAW is sufficient for tcpdump and this is not more than that. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/16/2016 09:53 AM, Christoph Lameter wrote: > On Fri, 13 May 2016, Doug Ledford wrote: > >>> if (cmd.flow_attr.flags >= IB_FLOW_ATTR_FLAGS_RESERVED) >>> >> >> I'm not at all convinced this is the right thing to do. Sniffing of >> packets is definitely a privileged operation. Tcpdump needs to be run >> as root to do this on regular devices. If not CAP_NET_ADMIN, then a >> root check seems appropriate. CAP_NET_RAW does not seem sufficient for >> sniffing other people's packets. > > CAP_NET_RAW is sufficient for tcpdump and this is not more than that. That's not true. In order to set promisc on an interface, tcpdump calls ioctl with SIOCSIFFLAGS as the ioctl and IFF_PROMISC set in the flags, and in net/core/dev_ioctl.c we see this check for SIOCSIFFLAGS: if (!ns_capable(net->user_ns, CAP_NET_ADMIN)) return -EPERM; /* fall through */
On Mon, 16 May 2016, Doug Ledford wrote: > > CAP_NET_RAW is sufficient for tcpdump and this is not more than that. > > > That's not true. In order to set promisc on an interface, tcpdump calls > ioctl with SIOCSIFFLAGS as the ioctl and IFF_PROMISC set in the flags, > and in net/core/dev_ioctl.c we see this check for SIOCSIFFLAGS: Tcpdump works fine without promiscuous mode. DONT_TRAP does not mean that the interface is switched into promiscuous mode. It just means that all traffic accepted by the hardware is also going to the QP specified. One can switch the interface into promiscuous mode in addition if one wants that. Then you need CAP_NET_ADMIN. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/16/2016 12:28 PM, Christoph Lameter wrote: > On Mon, 16 May 2016, Doug Ledford wrote: > >>> CAP_NET_RAW is sufficient for tcpdump and this is not more than that. >> >> >> That's not true. In order to set promisc on an interface, tcpdump calls >> ioctl with SIOCSIFFLAGS as the ioctl and IFF_PROMISC set in the flags, >> and in net/core/dev_ioctl.c we see this check for SIOCSIFFLAGS: > > Tcpdump works fine without promiscuous mode. DONT_TRAP does not > mean that the interface is switched into promiscuous mode. It just means > that all traffic accepted by the hardware is also going to the QP > specified. You're right, I was thinking about snooping as being more than just what it is here. > One can switch the interface into promiscuous mode in addition if one > wants that. Then you need CAP_NET_ADMIN. > > > >
On 05/16/2016 12:28 PM, Christoph Lameter wrote: > On Mon, 16 May 2016, Doug Ledford wrote: > >>> CAP_NET_RAW is sufficient for tcpdump and this is not more than that. >> >> >> That's not true. In order to set promisc on an interface, tcpdump calls >> ioctl with SIOCSIFFLAGS as the ioctl and IFF_PROMISC set in the flags, >> and in net/core/dev_ioctl.c we see this check for SIOCSIFFLAGS: > > Tcpdump works fine without promiscuous mode. DONT_TRAP does not > mean that the interface is switched into promiscuous mode. It just means > that all traffic accepted by the hardware is also going to the QP > specified. > > One can switch the interface into promiscuous mode in addition if one > wants that. Then you need CAP_NET_ADMIN. I've applied your patch. I completely reworded the commit message, you'll want to check that it meets your satisfaction before I issue my pull request.
On Wed, 18 May 2016, Doug Ledford wrote: > > One can switch the interface into promiscuous mode in addition if one > > wants that. Then you need CAP_NET_ADMIN. > > I've applied your patch. I completely reworded the commit message, > you'll want to check that it meets your satisfaction before I issue my > pull request. I wish I knew how to view the patch. Its not in the kernel.org git repo. Could you post the patches you edit or provide a link to them. See how Andrew Morton does it. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 05/18/2016 02:12 PM, Christoph Lameter wrote: > On Wed, 18 May 2016, Doug Ledford wrote: > >>> One can switch the interface into promiscuous mode in addition if one >>> wants that. Then you need CAP_NET_ADMIN. >> >> I've applied your patch. I completely reworded the commit message, >> you'll want to check that it meets your satisfaction before I issue my >> pull request. > > I wish I knew how to view the patch. Its not in the kernel.org git repo. > Could you post the patches you edit or provide a link to them. See how > Andrew Morton does it. > It's in *my* kernel.org repo. Where you trying to see it in Linus' repo?
On Wed, 18 May 2016, Doug Ledford wrote: > On 05/18/2016 02:12 PM, Christoph Lameter wrote: > > On Wed, 18 May 2016, Doug Ledford wrote: > > > >>> One can switch the interface into promiscuous mode in addition if one > >>> wants that. Then you need CAP_NET_ADMIN. > >> > >> I've applied your patch. I completely reworded the commit message, > >> you'll want to check that it meets your satisfaction before I issue my > >> pull request. > > > > I wish I knew how to view the patch. Its not in the kernel.org git repo. > > Could you post the patches you edit or provide a link to them. See how > > Andrew Morton does it. > > > > It's in *my* kernel.org repo. Where you trying to see it in Linus' repo? Why would I be looking at Linus'ses tree? Ahh.. just an update issue I guess. Changelog is ok. Just a note that there are no levels of capabilities. CAP_NET_ADMIN covers different things from CAP_NET_RAW. It is not a higher level. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Index: linux/drivers/infiniband/core/uverbs_cmd.c =================================================================== --- linux.orig/drivers/infiniband/core/uverbs_cmd.c 2016-03-24 09:16:27.782778586 -0500 +++ linux/drivers/infiniband/core/uverbs_cmd.c 2016-05-13 10:49:28.953000945 -0500 @@ -3088,8 +3088,7 @@ int ib_uverbs_ex_create_flow(struct ib_u if (cmd.comp_mask) return -EINVAL; - if ((cmd.flow_attr.type == IB_FLOW_ATTR_SNIFFER && - !capable(CAP_NET_ADMIN)) || !capable(CAP_NET_RAW)) + if (!capable(CAP_NET_RAW)) return -EPERM; if (cmd.flow_attr.flags >= IB_FLOW_ATTR_FLAGS_RESERVED)
Sorry slight mistake in the original patch. V2 follows Subject: [PATCH] IB/core: Do not require CAP_NET_ADMIN for sniffing V2 Having to enable CAP_NET_ADMIN for every app that uses sniffer mode is kind of risky. We do not want people to have the ability to mess around with the network configuration and routing. We just want the app to direct streams and deal with inbound data streams in various ways. So lets drop the requirement for CAP_NET_ADMIN and keep just CAP_NET_RAW. V1->V2 - Check for CAP_NET_ADMIN was conditional on IB_FLOW_ATTR_SNIFFER. We need to remove this in the correct way. - Update description Signed-off-by: Christoph Lameter <cl@linux.com> -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html