netdev - Re: [PATCH net 0/2] net: diag: fix a potential security issue

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADvbK_cKxwVxVbJU1uTVN1Goz_Wju1q71a0qUDWXY-zLCYNhhw@mail.gmail.com>
Date:   Sat, 21 Oct 2017 16:45:10 +0800
From:   Xin Long <lucien.xin@...il.com>
To:     Eric Dumazet <eric.dumazet@...il.com>
Cc:     Eric Dumazet <edumazet@...gle.com>,
        David Miller <davem@...emloft.net>,
        network dev <netdev@...r.kernel.org>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        Sabrina Dubroca <sd@...asysnail.net>
Subject: Re: [PATCH net 0/2] net: diag: fix a potential security issue

On Sat, Oct 21, 2017 at 3:45 PM, Eric Dumazet <eric.dumazet@...il.com> wrote:
> On Sat, 2017-10-21 at 14:51 +0800, Xin Long wrote:
>> On Sat, Oct 21, 2017 at 2:18 PM, Eric Dumazet <edumazet@...gle.com> wrote:
>> > On Fri, Oct 20, 2017 at 11:06 PM, Xin Long <lucien.xin@...il.com> wrote:
>> >>
>> >>
>> >> On Sat, Oct 21, 2017 at 9:27 AM, David Miller <davem@...emloft.net> wrote:
>> >>>
>> >>> From: Xin Long <lucien.xin@...il.com>
>> >>> 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.