diff mbox

UBUNTU: SAUCE: [net] disable autoloading of rare protocols

Message ID 20110111225413.GI4979@outflux.net
State Rejected
Delegated to: Andy Whitcroft
Headers show

Commit Message

Kees Cook Jan. 11, 2011, 10:54 p.m. UTC
This disables the autoloading of several rare network protocols
in an effort to reduce exposure to potential future security
issues with them, as recently demonstrated with RDS and Econet.

Thanks to Ben Hutchings and Debian for the patches:

http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=990932981b989699a710e1ec9eb3dd25f08ac362
http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=189f09eb39228b11fe8a6b56a27ad09639150d37
http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=6fd8c90166edf1595c2c828f7cbe4ba7febc4af8
http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=8d92d7b141b4767f9877ffd1a2c7b0060d50628f
http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=cf875d498103ff888db76892ae98ddc5ed0d3a4c

Signed-off-by: Kees Cook <kees.cook@canonical.com>
---
 net/decnet/af_decnet.c         |    2 +-
 net/econet/af_econet.c         |    2 +-
 net/ieee802154/af_ieee802154.c |    2 +-
 net/rds/af_rds.c               |    2 +-
 net/x25/af_x25.c               |    2 +-
 5 files changed, 5 insertions(+), 5 deletions(-)

Comments

Tim Gardner Jan. 11, 2011, 11:22 p.m. UTC | #1
On 01/11/2011 04:54 PM, Kees Cook wrote:
> This disables the autoloading of several rare network protocols
> in an effort to reduce exposure to potential future security
> issues with them, as recently demonstrated with RDS and Econet.
>
> Thanks to Ben Hutchings and Debian for the patches:
>
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=990932981b989699a710e1ec9eb3dd25f08ac362
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=189f09eb39228b11fe8a6b56a27ad09639150d37
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=6fd8c90166edf1595c2c828f7cbe4ba7febc4af8
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=8d92d7b141b4767f9877ffd1a2c7b0060d50628f
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=cf875d498103ff888db76892ae98ddc5ed0d3a4c
>
> Signed-off-by: Kees Cook<kees.cook@canonical.com>
> ---
>   net/decnet/af_decnet.c         |    2 +-
>   net/econet/af_econet.c         |    2 +-
>   net/ieee802154/af_ieee802154.c |    2 +-
>   net/rds/af_rds.c               |    2 +-
>   net/x25/af_x25.c               |    2 +-
>   5 files changed, 5 insertions(+), 5 deletions(-)
>

I'm not entirely opposed (having followed the original discussion on 
netdev). Could you describe for this list under what circumstances a 
protocol module is loaded and what DOSs and vulnerabilities this will 
prevent? I assume there are both user space and network receive side issues.

rtg
Kees Cook Jan. 11, 2011, 11:39 p.m. UTC | #2
On Tue, Jan 11, 2011 at 05:22:21PM -0600, Tim Gardner wrote:
> On 01/11/2011 04:54 PM, Kees Cook wrote:
> >This disables the autoloading of several rare network protocols
> >in an effort to reduce exposure to potential future security
> >issues with them, as recently demonstrated with RDS and Econet.

It's been recommended to possibly add can, rose, ax25, netrom, and phonet
to this list too.

> I'm not entirely opposed (having followed the original discussion on
> netdev). Could you describe for this list under what circumstances a
> protocol module is loaded and what DOSs and vulnerabilities this
> will prevent? I assume there are both user space and network receive
> side issues.

AFAIU, it is strictly a local issue. A process running:

    socket(AF_$SOMETHING, ...)

will trigger the kernel to autoload "net-pf-NNN". For a complete list of these
aliases, see the output:

    egrep "net-pf-[0-9]+ " /lib/modules/$(uname -r)/modules.alias

-Kees
Tim Gardner Jan. 12, 2011, 9:41 p.m. UTC | #3
On 01/11/2011 05:39 PM, Kees Cook wrote:
> On Tue, Jan 11, 2011 at 05:22:21PM -0600, Tim Gardner wrote:
>> On 01/11/2011 04:54 PM, Kees Cook wrote:
>>> This disables the autoloading of several rare network protocols
>>> in an effort to reduce exposure to potential future security
>>> issues with them, as recently demonstrated with RDS and Econet.
>
> It's been recommended to possibly add can, rose, ax25, netrom, and phonet
> to this list too.
>
>> I'm not entirely opposed (having followed the original discussion on
>> netdev). Could you describe for this list under what circumstances a
>> protocol module is loaded and what DOSs and vulnerabilities this
>> will prevent? I assume there are both user space and network receive
>> side issues.
>
> AFAIU, it is strictly a local issue. A process running:
>
>      socket(AF_$SOMETHING, ...)
>
> will trigger the kernel to autoload "net-pf-NNN". For a complete list of these
> aliases, see the output:
>
>      egrep "net-pf-[0-9]+ " /lib/modules/$(uname -r)/modules.alias
>
> -Kees
>

Why don't we blacklist these modules instead of carrying more SAUCE patches?
Kees Cook Jan. 12, 2011, 11:06 p.m. UTC | #4
On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
> Why don't we blacklist these modules instead of carrying more SAUCE patches?

I think that shipping a blacklist file is more of a pain since it would end up as a
debian conffile in /etc, so local changes would cause install debconf questions, etc.

Another option would be to filter it during the modules.aliases file creation so the
list is all in one place.

-Kees
Tim Gardner Jan. 12, 2011, 11:34 p.m. UTC | #5
On 01/12/2011 05:06 PM, Kees Cook wrote:
> On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
>> Why don't we blacklist these modules instead of carrying more SAUCE patches?
>
> I think that shipping a blacklist file is more of a pain since it would end up as a
> debian conffile in /etc, so local changes would cause install debconf questions, etc.
>

I think the folks that would enable these modules are also capable of 
dealing with answering a debconf question. Furthermore, /etc/modprobe.d 
is a well known place for module loading control. Isn't /etc/modprobe.d 
where Jockey does its thing when switching between nvidia and nouveau ?

> Another option would be to filter it during the modules.aliases file creation so the
> list is all in one place.
>

I'm not sure I follow you here. Are you suggesting we add code in the 
post install hook for the kernel that elides the protocol module 
aliases? That doesn't seem like a very good idea to me wrt updates.

rtg
Kees Cook Jan. 12, 2011, 11:57 p.m. UTC | #6
On Wed, Jan 12, 2011 at 05:34:52PM -0600, Tim Gardner wrote:
> On 01/12/2011 05:06 PM, Kees Cook wrote:
> >On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
> >>Why don't we blacklist these modules instead of carrying more SAUCE patches?
> >
> >I think that shipping a blacklist file is more of a pain since it would end up as a
> >debian conffile in /etc, so local changes would cause install debconf questions, etc.
> >
> 
> I think the folks that would enable these modules are also capable
> of dealing with answering a debconf question. Furthermore,
> /etc/modprobe.d is a well known place for module loading control.
> Isn't /etc/modprobe.d where Jockey does its thing when switching
> between nvidia and nouveau ?

It's certainly an option. I just try to avoid adding conffiles at all
cost since they're so annoying to deal with in packaging if you want to
remove them, change them, etc.

> >Another option would be to filter it during the modules.aliases file creation so the
> >list is all in one place.
> >
> 
> I'm not sure I follow you here. Are you suggesting we add code in
> the post install hook for the kernel that elides the protocol module
> aliases? That doesn't seem like a very good idea to me wrt updates.

No, I mean patching the kernel's build process to add effectively a
grep -v when generating the modules.aliases file. Though the more I
think about this, the more that seems to really be a patch to depmod,
so I probably don't recommend it now.

Anyway, why not carry the kernel patch so we're at least in sync with
Debian?

-Kees
Ben Hutchings Jan. 13, 2011, 4:48 a.m. UTC | #7
On Tue, 2011-01-11 at 14:54 -0800, Kees Cook wrote:
> This disables the autoloading of several rare network protocols
> in an effort to reduce exposure to potential future security
> issues with them, as recently demonstrated with RDS and Econet.
> 
> Thanks to Ben Hutchings and Debian for the patches:
> 
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=990932981b989699a710e1ec9eb3dd25f08ac362
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=189f09eb39228b11fe8a6b56a27ad09639150d37
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=6fd8c90166edf1595c2c828f7cbe4ba7febc4af8
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=8d92d7b141b4767f9877ffd1a2c7b0060d50628f
> http://git.debian.org/?p=kernel/linux-2.6.git;a=commitdiff;h=cf875d498103ff888db76892ae98ddc5ed0d3a4c
[...]

Those are not stable references.  That repository is an experimental
conversion of our patch series in svn, and is subject to rebasing.

You could refer to the patches using URLs under
<http://svn.debian.org/wsvn/kernel/releases/linux-2.6/2.6.32-28/debian/patches/debian/>.

Ben.
Tim Gardner Jan. 13, 2011, 4:03 p.m. UTC | #8
On 01/12/2011 05:57 PM, Kees Cook wrote:
> On Wed, Jan 12, 2011 at 05:34:52PM -0600, Tim Gardner wrote:
>> On 01/12/2011 05:06 PM, Kees Cook wrote:
>>> On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
>>>> Why don't we blacklist these modules instead of carrying more SAUCE patches?
>>>
>>> I think that shipping a blacklist file is more of a pain since it would end up as a
>>> debian conffile in /etc, so local changes would cause install debconf questions, etc.
>>>
>>
>> I think the folks that would enable these modules are also capable
>> of dealing with answering a debconf question. Furthermore,
>> /etc/modprobe.d is a well known place for module loading control.
>> Isn't /etc/modprobe.d where Jockey does its thing when switching
>> between nvidia and nouveau ?
>
> It's certainly an option. I just try to avoid adding conffiles at all
> cost since they're so annoying to deal with in packaging if you want to
> remove them, change them, etc.
>
>>> Another option would be to filter it during the modules.aliases file creation so the
>>> list is all in one place.
>>>
>>
>> I'm not sure I follow you here. Are you suggesting we add code in
>> the post install hook for the kernel that elides the protocol module
>> aliases? That doesn't seem like a very good idea to me wrt updates.
>
> No, I mean patching the kernel's build process to add effectively a
> grep -v when generating the modules.aliases file. Though the more I
> think about this, the more that seems to really be a patch to depmod,
> so I probably don't recommend it now.
>
> Anyway, why not carry the kernel patch so we're at least in sync with
> Debian?
>

Because we aren't in sync with Debian. We're in sync with Linus' upstream.

Andy has some thoughts about how we might mitigate debconf questions.

rtg
Andy Whitcroft Jan. 13, 2011, 4:09 p.m. UTC | #9
On Wed, Jan 12, 2011 at 03:06:04PM -0800, Kees Cook wrote:
> On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
> > Why don't we blacklist these modules instead of carrying more SAUCE patches?
> 
> I think that shipping a blacklist file is more of a pain since it would end up as a
> debian conffile in /etc, so local changes would cause install debconf questions, etc.
> 
> Another option would be to filter it during the modules.aliases file creation so the
> list is all in one place.

If we used one file they would be required to comment things out to
enable one protocol and indeed generate a debconf question.

How about if we made one file per protocol.  disable-x25.conf stylee,
then they could simply remove the file to fix it.  Would that avoid the
conflict.  Cirtainly we could use a dpkg redirect if the file is to go
en-toto?

Would that work better?

-apw
Tim Gardner Jan. 13, 2011, 8:37 p.m. UTC | #10
On 01/13/2011 10:09 AM, Andy Whitcroft wrote:
> On Wed, Jan 12, 2011 at 03:06:04PM -0800, Kees Cook wrote:
>> On Wed, Jan 12, 2011 at 03:41:21PM -0600, Tim Gardner wrote:
>>> Why don't we blacklist these modules instead of carrying more SAUCE patches?
>>
>> I think that shipping a blacklist file is more of a pain since it would end up as a
>> debian conffile in /etc, so local changes would cause install debconf questions, etc.
>>
>> Another option would be to filter it during the modules.aliases file creation so the
>> list is all in one place.
>
> If we used one file they would be required to comment things out to
> enable one protocol and indeed generate a debconf question.
>
> How about if we made one file per protocol.  disable-x25.conf stylee,
> then they could simply remove the file to fix it.  Would that avoid the
> conflict.  Cirtainly we could use a dpkg redirect if the file is to go
> en-toto?
>
> Would that work better?
>
> -apw

After a hallway conversation with Kees, we've agreed that he'll write 
the module-init-tools patch to blacklist the aforementioned protocols. 
This has the advantage of being applicable to all Natty user space 
installs, regardless of the kernel in use.

rtg
diff mbox

Patch

diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c
index 6f97268..8c226bc 100644
--- a/net/decnet/af_decnet.c
+++ b/net/decnet/af_decnet.c
@@ -2361,7 +2361,7 @@  void dn_unregister_sysctl(void);
 MODULE_DESCRIPTION("The Linux DECnet Network Protocol");
 MODULE_AUTHOR("Linux DECnet Project Team");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NETPROTO(PF_DECnet);
+/*MODULE_ALIAS_NETPROTO(PF_DECnet);*/
 
 static char banner[] __initdata = KERN_INFO "NET4: DECnet for Linux: V.2.5.68s (C) 1995-2003 Linux DECnet Project Team\n";
 
diff --git a/net/econet/af_econet.c b/net/econet/af_econet.c
index 15dcc1a..548ccbf 100644
--- a/net/econet/af_econet.c
+++ b/net/econet/af_econet.c
@@ -1179,4 +1179,4 @@  module_init(econet_proto_init);
 module_exit(econet_proto_exit);
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NETPROTO(PF_ECONET);
+/*MODULE_ALIAS_NETPROTO(PF_ECONET);*/
diff --git a/net/ieee802154/af_ieee802154.c b/net/ieee802154/af_ieee802154.c
index 93c91b6..46dc52c 100644
--- a/net/ieee802154/af_ieee802154.c
+++ b/net/ieee802154/af_ieee802154.c
@@ -370,4 +370,4 @@  module_init(af_ieee802154_init);
 module_exit(af_ieee802154_remove);
 
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NETPROTO(PF_IEEE802154);
+/*MODULE_ALIAS_NETPROTO(PF_IEEE802154);*/
diff --git a/net/rds/af_rds.c b/net/rds/af_rds.c
index bb6ad81..821c578 100644
--- a/net/rds/af_rds.c
+++ b/net/rds/af_rds.c
@@ -600,4 +600,4 @@  MODULE_DESCRIPTION("RDS: Reliable Datagram Sockets"
 		   " v" DRV_VERSION " (" DRV_RELDATE ")");
 MODULE_VERSION(DRV_VERSION);
 MODULE_LICENSE("Dual BSD/GPL");
-MODULE_ALIAS_NETPROTO(PF_RDS);
+/*MODULE_ALIAS_NETPROTO(PF_RDS);*/
diff --git a/net/x25/af_x25.c b/net/x25/af_x25.c
index f7af98d..f02d762 100644
--- a/net/x25/af_x25.c
+++ b/net/x25/af_x25.c
@@ -1828,4 +1828,4 @@  module_exit(x25_exit);
 MODULE_AUTHOR("Jonathan Naylor <g4klx@g4klx.demon.co.uk>");
 MODULE_DESCRIPTION("The X.25 Packet Layer network layer protocol");
 MODULE_LICENSE("GPL");
-MODULE_ALIAS_NETPROTO(PF_X25);
+/*MODULE_ALIAS_NETPROTO(PF_X25);*/