From mboxrd@z Thu Jan 1 00:00:00 1970 From: Xin Long Subject: Re: [PATCH net 0/2] net: diag: fix a potential security issue Date: Sat, 21 Oct 2017 16:45:10 +0800 Message-ID: References: <20171021.022737.1906342496133825805.davem@davemloft.net> <1508571955.30291.21.camel@edumazet-glaptop3.roam.corp.google.com> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Cc: Eric Dumazet , David Miller , network dev , Marcelo Ricardo Leitner , Sabrina Dubroca To: Eric Dumazet Return-path: Received: from mail-qk0-f193.google.com ([209.85.220.193]:46424 "EHLO mail-qk0-f193.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751668AbdJUIpM (ORCPT ); Sat, 21 Oct 2017 04:45:12 -0400 Received: by mail-qk0-f193.google.com with SMTP id k123so16916303qke.3 for ; Sat, 21 Oct 2017 01:45:11 -0700 (PDT) In-Reply-To: <1508571955.30291.21.camel@edumazet-glaptop3.roam.corp.google.com> Sender: netdev-owner@vger.kernel.org List-ID: On Sat, Oct 21, 2017 at 3:45 PM, Eric Dumazet wrote: > On Sat, 2017-10-21 at 14:51 +0800, Xin Long wrote: >> On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet wrote: >> > On Fri, Oct 20, 2017 at 11:06 PM, Xin Long wrote: >> >> >> >> >> >> On Sat, Oct 21, 2017 at 9:27 AM, David Miller wrote: >> >>> >> >>> From: Xin Long >> >>> Date: Thu, 19 Oct 2017 15:32:23 +0800 >> >>> >> >>> > This patch is to void the potential security issue that the family >> >>> > or protocol modules are autoloaded when requesting _diag module by >> >>> > not requesting _diag module if the family or protocol is not added >> >>> > or registered in sock_diag and inet_diag. >> >>> > >> >>> > As the repost of the patch '[PATCH net] sock_diag: request _diag >> >>> > module only when the family or proto has been registered', this >> >>> > patchset fixes the compiling errors when INET is not set, and >> >>> > also split into two patches to make it clear to review. >> >>> >> >>> This makes no sense to me. >> >>> >> >>> Any user can just open a socket() in the appropriate protocol >> >>> family to cause the module to be loaded. >> >>> >> >>> If someone wants modules to not be loaded, block them using >> >>> traditional module loading infrastructure mechanisms. Or >> >>> don't load the module at all. >> >>> >> >>> Sorry I am not applying this. >> >> >> >> Hi David, >> >> >> >> I'm still thinking it's not good after 'ss', sctp, dccp, >> >> af_packet ... are just loaded, in which case, no one actually >> >> open any socket with these family or proto. >> >> >> >> I talked with Marcelo before, one scenario as he said: >> >> >> >> Imagine a customer generates a sosreport on their system, and >> >> with that, it loads sctp module. From then on, if their firewall >> >> doesn't block incoming packets for sctp, they may be prone to some >> >> remotely triggerable issue on sctp code, without even actually using >> >> sctp. >> > >> > For that reason, we have disabled autoloading of SCTP. >> > ( removing the >> > MODULE_ALIAS("net-pf-" __stringify(PF_INET) "-proto-132"); >> > MODULE_ALIAS("net-pf-" __stringify(PF_INET6) "-proto-132"); >> > ) >> > root must modprobe the module before it is accessible. >> > >> > However inet_diag is a way to have the module loaded anyway. >> > >> > This is why I like your patch Xin. >> > >> > David is only saying that your patch alone is not enough to prevent a >> > user to use socket() to autoload SCTP. >> Using socket() to autoload SCTP should be fine, cause users would >> use SCTP, no ? >> >> "ss" doesn't mean users intend to use SCTP, "ss" may make users >> not aware that SCTP module would be loaded, unlike socket(SCTP). > > Your changelog mentions a security issue. > > How have you prevented user using socket() to bypass your 'security' > feature ? I think the hook in __sock_create(): err = security_socket_create(family, type, protocol, kern); if (err) return err; could work for this, no ? like, users add security rules to not allow to create SCTP socket (namely, not allow sctp module to be loaded) But, 'ss' could bypass it to load SCTP module. In this way, can this patch be considered a security issue ? > > If you have not yet, this security claim is simply false.