lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Message-ID: <87r2zqj82a.fsf@xmission.com> Date: Mon, 15 May 2017 13:20:29 -0500 From: ebiederm@...ssion.com (Eric W. Biederman) To: Mahesh Bandewar (महेश बंडेवार) <maheshb@...gle.com> Cc: David Miller <davem@...emloft.net>, gregkh@...uxfoundation.org, mahesh@...dewar.net, mingo@...nel.org, linux-kernel@...r.kernel.org, linux-netdev <netdev@...r.kernel.org>, keescook@...omium.org, Eric Dumazet <edumazet@...gle.com> Subject: Re: [PATCH] kmod: don't load module unless req process has CAP_SYS_MODULE "Mahesh Bandewar (महेश बंडेवार)" <maheshb@...gle.com> writes: > On Mon, May 15, 2017 at 6:52 AM, David Miller <davem@...emloft.net> wrote: >> From: Greg Kroah-Hartman <gregkh@...uxfoundation.org> >> Date: Mon, 15 May 2017 08:10:59 +0200 >> >>> On Sun, May 14, 2017 at 08:57:34AM -0500, Eric W. Biederman wrote: >>>> Greg Kroah-Hartman <gregkh@...uxfoundation.org> writes: >>>> >>>> diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c >>>> index bcb0f610ee42..6b72528a4636 100644 >>>> --- a/net/core/rtnetlink.c >>>> +++ b/net/core/rtnetlink.c >>>> @@ -2595,7 +2595,7 @@ static int rtnl_newlink(struct sk_buff *skb, struct nlmsghdr *nlh, >>>> >>>> if (!ops) { >>>> #ifdef CONFIG_MODULES >>>> - if (kind[0]) { >>>> + if (kind[0] && capable(CAP_NET_ADMIN)) { >>>> __rtnl_unlock(); >>>> request_module("rtnl-link-%s", kind); >>>> rtnl_lock(); >>> >>> I don't object to this if the networking developers don't mind the >>> change in functionality. They can handle the fallout :) >> >> As I've said in another email, I am pretty sure this can break things. > > The current behavior is already breaking things. e.g. unprivileged > process can be root inside it's own user-ns. This will allow it to > create IPtable rules causing contracking module to be loaded in > default-ns affecting every flow on the server (not just the namespace > that user or an unprivileged process is attached to). Cases that I > mentioned above are just the tip of an iceberg. If loading the conntrack module changes the semantics of packet processing when nothing is configured that is a bug in the conntrack module. > In a non-namespace world this wouldn't happen as capability checks are > performed correctly but the moment an unprivileged user can create > it's own user-ns and becomes root inside, it could make use of these > things and perform privileged operations in default-ns. So to protect > "global namespace" from making such things happen, we have to protect > using global capability check. > > Alternatively we can preserve the existing behavior by adding this > check for non-default-user-ns only. e.g. I believe last time this was discussed the compromise was that a prefix would be prepended to request_module calls so that what each call allows to be loaded would be limited in scope to what is sensible in that location. I don't think anyone made any arguments about increasing the attack surface at that time. So there may be reason to go back and reexamine the decision on security grounds, but it needs to be a clearly made argument. Explaining to people the pros and cons of the reason to perform the work. > diff --git a/net/core/rtnetlink.c b/net/core/rtnetlink.c > index 6e67315ec368..263f0d175091 100644 > --- a/net/core/rtnetlink.c > +++ b/net/core/rtnetlink.c > @@ -2595,7 +2595,9 @@ static int rtnl_newlink(struct sk_buff *skb, > struct nlmsghdr *nlh, > > if (!ops) { > #ifdef CONFIG_MODULES > - if (kind[0]) { > + if (kind[0] && > + ((net->user_ns == &init_user_ns) || > + capable(CAP_SYS_MODULE))) { > __rtnl_unlock(); > request_module("rtnl-link-%s", kind); > rtnl_lock(); This patch is definitely wrong. CAP_NET_ADMIN had always guarded this request_module call. CAP_SYS_MODULE means you can request any module you like dropping does not mean you can't request modules. Adding a capable(CAP_NET_ADMIN) at this call site would be the least breaking solution available, as it would only break things for callers in non-initial network namespaces. Your change would definitely things for ordinary network administration tools with capabilities. > if we have to do this in net-subsystem then it's not just this call > site and there are lot more. But if this is an acceptable alternative, > I can think of better implementation for all those sites. Eric
Powered by blists - more mailing lists