From mboxrd@z Thu Jan 1 00:00:00 1970 From: Eric Dumazet Subject: Re: [PATCH net 0/2] net: diag: fix a potential security issue Date: Sat, 21 Oct 2017 00:45:55 -0700 Message-ID: <1508571955.30291.21.camel@edumazet-glaptop3.roam.corp.google.com> References: <20171021.022737.1906342496133825805.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: 7bit Cc: Eric Dumazet , David Miller , network dev , Marcelo Ricardo Leitner , Sabrina Dubroca To: Xin Long Return-path: Received: from mail-pg0-f67.google.com ([74.125.83.67]:46723 "EHLO mail-pg0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751392AbdJUHp6 (ORCPT ); Sat, 21 Oct 2017 03:45:58 -0400 Received: by mail-pg0-f67.google.com with SMTP id k7so8275291pga.3 for ; Sat, 21 Oct 2017 00:45:58 -0700 (PDT) In-Reply-To: Sender: netdev-owner@vger.kernel.org List-ID: 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 ? If you have not yet, this security claim is simply false.