2019-03-15 18:06:02

by Jakub Kicinski

[permalink] [raw]
Subject: mount.nfs: Protocol error after upgrade to linux/master

Hi,

I just upgraded from:

commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
Merge: c6873d18cb4a 24319258660a
Author: David S. Miller <[email protected]>
Date: Mon Mar 11 16:22:49 2019 -0700

to

commit 3b319ee220a8795406852a897299dbdfc1b09911
Merge: 9352ca585b2a b6e88119f1ed
Author: Linus Torvalds <[email protected]>
Date: Thu Mar 14 10:48:14 2019 -0700

and I'm seeing:

# mount /home/
mount.nfs: Protocol error

No errors in dmesg, please let me know if it's a known problem or what
other info could be of use.


2019-03-15 18:41:25

by Trond Myklebust

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, 2019-03-15 at 11:05 -0700, Jakub Kicinski wrote:
> Hi,
>
> I just upgraded from:
>
> commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> Merge: c6873d18cb4a 24319258660a
> Author: David S. Miller <[email protected]>
> Date: Mon Mar 11 16:22:49 2019 -0700
>
> to
>
> commit 3b319ee220a8795406852a897299dbdfc1b09911
> Merge: 9352ca585b2a b6e88119f1ed
> Author: Linus Torvalds <[email protected]>
> Date: Thu Mar 14 10:48:14 2019 -0700
>
> and I'm seeing:
>
> # mount /home/
> mount.nfs: Protocol error
>
> No errors in dmesg, please let me know if it's a known problem or
> what
> other info could be of use.

It sounds like the exact same problem that Marc Dionne reported seeing.
Can you also try the patch at
http://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=patch;h=513149607d19bc3821386fb5ac75f8b99fd4b115
?

Thanks
Trond
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2019-03-15 19:01:11

by Jakub Kicinski

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> Hi,
>
> I just upgraded from:
>
> commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> Merge: c6873d18cb4a 24319258660a
> Author: David S. Miller <[email protected]>
> Date: Mon Mar 11 16:22:49 2019 -0700
>
> to
>
> commit 3b319ee220a8795406852a897299dbdfc1b09911
> Merge: 9352ca585b2a b6e88119f1ed
> Author: Linus Torvalds <[email protected]>
> Date: Thu Mar 14 10:48:14 2019 -0700
>
> and I'm seeing:
>
> # mount /home/
> mount.nfs: Protocol error
>
> No errors in dmesg, please let me know if it's a known problem or what
> other info could be of use.

Hm.. I tried to bisect but reverting to that commit doesn't help.

Looks like the server responds with:

ICMP parameter problem - octet 22, length 80

pointing at some IP options (type 134)...

2019-03-15 19:08:21

by Jakub Kicinski

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, 15 Mar 2019 18:41:20 +0000, Trond Myklebust wrote:
> On Fri, 2019-03-15 at 11:05 -0700, Jakub Kicinski wrote:
> > Hi,
> >
> > I just upgraded from:
> >
> > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > Merge: c6873d18cb4a 24319258660a
> > Author: David S. Miller <[email protected]>
> > Date: Mon Mar 11 16:22:49 2019 -0700
> >
> > to
> >
> > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > Merge: 9352ca585b2a b6e88119f1ed
> > Author: Linus Torvalds <[email protected]>
> > Date: Thu Mar 14 10:48:14 2019 -0700
> >
> > and I'm seeing:
> >
> > # mount /home/
> > mount.nfs: Protocol error
> >
> > No errors in dmesg, please let me know if it's a known problem or
> > what
> > other info could be of use.
>
> It sounds like the exact same problem that Marc Dionne reported seeing.
> Can you also try the patch at
> http://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=patch;h=513149607d19bc3821386fb5ac75f8b99fd4b115
> ?

Sorry, I didn't see your reply, trying now..

2019-03-15 19:18:20

by Jakub Kicinski

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, 15 Mar 2019 12:08:15 -0700, Jakub Kicinski wrote:
> On Fri, 15 Mar 2019 18:41:20 +0000, Trond Myklebust wrote:
> > On Fri, 2019-03-15 at 11:05 -0700, Jakub Kicinski wrote:
> > > Hi,
> > >
> > > I just upgraded from:
> > >
> > > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > > Merge: c6873d18cb4a 24319258660a
> > > Author: David S. Miller <[email protected]>
> > > Date: Mon Mar 11 16:22:49 2019 -0700
> > >
> > > to
> > >
> > > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > > Merge: 9352ca585b2a b6e88119f1ed
> > > Author: Linus Torvalds <[email protected]>
> > > Date: Thu Mar 14 10:48:14 2019 -0700
> > >
> > > and I'm seeing:
> > >
> > > # mount /home/
> > > mount.nfs: Protocol error
> > >
> > > No errors in dmesg, please let me know if it's a known problem or
> > > what
> > > other info could be of use.
> >
> > It sounds like the exact same problem that Marc Dionne reported seeing.
> > Can you also try the patch at
> > http://git.linux-nfs.org/?p=trondmy/linux-nfs.git;a=patch;h=513149607d19bc3821386fb5ac75f8b99fd4b115
> > ?
>
> Sorry, I didn't see your reply, trying now..

Didn't seem to help:

$ git show
commit a79da625d196d00d19e8cbf8ec6eff3a323e3423 (HEAD -> work)
Author: Trond Myklebust <[email protected]>
Date: Fri Mar 15 12:55:59 2019 -0400

SUNRPC: Fix the minimal size for reply buffer allocation

We must at minimum allocate enough memory to be able to see any auth
errors in the reply from the server.

Fixes: 2c94b8eca1a26 ("SUNRPC: Use au_rslack when computing reply...")
Signed-off-by: Trond Myklebust <[email protected]>

diff --git a/net/sunrpc/clnt.c b/net/sunrpc/clnt.c
index 4216fe33204a..310873895578 100644
--- a/net/sunrpc/clnt.c
+++ b/net/sunrpc/clnt.c
@@ -1730,7 +1730,12 @@ call_allocate(struct rpc_task *task)
req->rq_callsize = RPC_CALLHDRSIZE + (auth->au_cslack << 1) +
proc->p_arglen;
req->rq_callsize <<= 2;
- req->rq_rcvsize = RPC_REPHDRSIZE + auth->au_rslack + proc->p_replen;
+ /*
+ * Note: the reply buffer must at minimum allocate enough space
+ * for the 'struct accepted_reply' from RFC5531.
+ */
+ req->rq_rcvsize = RPC_REPHDRSIZE + auth->au_rslack + \
+ max_t(size_t, proc->p_replen, 2);
req->rq_rcvsize <<= 2;

status = xprt->ops->buf_alloc(task);

# uname -r
5.0.0-11527-ga79da625d196
# mount /home/
mount.nfs: Protocol error

2019-03-15 23:54:47

by Jakub Kicinski

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, 15 Mar 2019 12:01:05 -0700, Jakub Kicinski wrote:
> On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> > Hi,
> >
> > I just upgraded from:
> >
> > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > Merge: c6873d18cb4a 24319258660a
> > Author: David S. Miller <[email protected]>
> > Date: Mon Mar 11 16:22:49 2019 -0700
> >
> > to
> >
> > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > Merge: 9352ca585b2a b6e88119f1ed
> > Author: Linus Torvalds <[email protected]>
> > Date: Thu Mar 14 10:48:14 2019 -0700
> >
> > and I'm seeing:
> >
> > # mount /home/
> > mount.nfs: Protocol error
> >
> > No errors in dmesg, please let me know if it's a known problem or what
> > other info could be of use.
>
> Hm.. I tried to bisect but reverting to that commit doesn't help.
>
> Looks like the server responds with:
>
> ICMP parameter problem - octet 22, length 80
>
> pointing at some IP options (type 134)...

Okay, figured it out, it's the commit 13e735c0e953 ("LSM: Introduce
CONFIG_LSM") and all the related changes in security/

I did olddefconfig and it changed my security module from apparmor to
smack silently. smack must be slapping those IP options on by default.

Pretty awful user experience, and a non-zero chance that users who
upgrade their kernels will miss this and end up with the wrong security
module...

2019-03-16 05:25:10

by Kees Cook

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, Mar 15, 2019 at 4:54 PM Jakub Kicinski
<[email protected]> wrote:
>
> On Fri, 15 Mar 2019 12:01:05 -0700, Jakub Kicinski wrote:
> > On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> > > Hi,
> > >
> > > I just upgraded from:
> > >
> > > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > > Merge: c6873d18cb4a 24319258660a
> > > Author: David S. Miller <[email protected]>
> > > Date: Mon Mar 11 16:22:49 2019 -0700
> > >
> > > to
> > >
> > > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > > Merge: 9352ca585b2a b6e88119f1ed
> > > Author: Linus Torvalds <[email protected]>
> > > Date: Thu Mar 14 10:48:14 2019 -0700
> > >
> > > and I'm seeing:
> > >
> > > # mount /home/
> > > mount.nfs: Protocol error
> > >
> > > No errors in dmesg, please let me know if it's a known problem or what
> > > other info could be of use.
> >
> > Hm.. I tried to bisect but reverting to that commit doesn't help.
> >
> > Looks like the server responds with:
> >
> > ICMP parameter problem - octet 22, length 80
> >
> > pointing at some IP options (type 134)...
>
> Okay, figured it out, it's the commit 13e735c0e953 ("LSM: Introduce
> CONFIG_LSM") and all the related changes in security/
>
> I did olddefconfig and it changed my security module from apparmor to
> smack silently. smack must be slapping those IP options on by default.
>
> Pretty awful user experience, and a non-zero chance that users who
> upgrade their kernels will miss this and end up with the wrong security
> module...

I wonder if we can add some kind of logic to Kconfig to retain the old
CONFIG_DEFAULT_SECURITY and include it as the first legacy-major LSM
listed in CONFIG_LSM?

Like, but the old selector back in, but mark is as "soon to be
entirely replaced with CONFIG_LSM" and then make CONFIG_LSM's default
be "yama,loadpin,safesetid,integrity,$(CONFIG_DEFAULT_SECURITY),selinux,smack,tomoyo,apparmor"
? Duplicates are ignored...

--
Kees Cook

2019-03-16 05:39:10

by Kees Cook

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Fri, Mar 15, 2019 at 10:24 PM Kees Cook <[email protected]> wrote:
>
> On Fri, Mar 15, 2019 at 4:54 PM Jakub Kicinski
> <[email protected]> wrote:
> >
> > On Fri, 15 Mar 2019 12:01:05 -0700, Jakub Kicinski wrote:
> > > On Fri, 15 Mar 2019 11:05:55 -0700, Jakub Kicinski wrote:
> > > > Hi,
> > > >
> > > > I just upgraded from:
> > > >
> > > > commit a3b1933d34d5bb26d7503752e3528315a9e28339 (net)
> > > > Merge: c6873d18cb4a 24319258660a
> > > > Author: David S. Miller <[email protected]>
> > > > Date: Mon Mar 11 16:22:49 2019 -0700
> > > >
> > > > to
> > > >
> > > > commit 3b319ee220a8795406852a897299dbdfc1b09911
> > > > Merge: 9352ca585b2a b6e88119f1ed
> > > > Author: Linus Torvalds <[email protected]>
> > > > Date: Thu Mar 14 10:48:14 2019 -0700
> > > >
> > > > and I'm seeing:
> > > >
> > > > # mount /home/
> > > > mount.nfs: Protocol error
> > > >
> > > > No errors in dmesg, please let me know if it's a known problem or what
> > > > other info could be of use.
> > >
> > > Hm.. I tried to bisect but reverting to that commit doesn't help.
> > >
> > > Looks like the server responds with:
> > >
> > > ICMP parameter problem - octet 22, length 80
> > >
> > > pointing at some IP options (type 134)...
> >
> > Okay, figured it out, it's the commit 13e735c0e953 ("LSM: Introduce
> > CONFIG_LSM") and all the related changes in security/
> >
> > I did olddefconfig and it changed my security module from apparmor to
> > smack silently. smack must be slapping those IP options on by default.
> >
> > Pretty awful user experience, and a non-zero chance that users who
> > upgrade their kernels will miss this and end up with the wrong security
> > module...
>
> I wonder if we can add some kind of logic to Kconfig to retain the old
> CONFIG_DEFAULT_SECURITY and include it as the first legacy-major LSM
> listed in CONFIG_LSM?
>
> Like, but the old selector back in, but mark is as "soon to be
> entirely replaced with CONFIG_LSM" and then make CONFIG_LSM's default
> be "yama,loadpin,safesetid,integrity,$(CONFIG_DEFAULT_SECURITY),selinux,smack,tomoyo,apparmor"
> ? Duplicates are ignored...

This would initialize a default order from the earlier Kconfig items:

diff --git a/security/Kconfig b/security/Kconfig
index 1d6463fb1450..e3813b5c6824 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,8 +239,40 @@ source "security/safesetid/Kconfig"

source "security/integrity/Kconfig"

+choice
+ prompt "First legacy-major LSM to be initialized"
+ default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
+ default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
+ default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
+ default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
+ default DEFAULT_SECURITY_DAC
+
+ help
+ Select the legacy-major security module that will be initialize
+ first. Overridden by non-default CONFIG_LSM.
+
+ config DEFAULT_SECURITY_SELINUX
+ bool "SELinux" if SECURITY_SELINUX=y
+
+ config DEFAULT_SECURITY_SMACK
+ bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
+
+ config DEFAULT_SECURITY_TOMOYO
+ bool "TOMOYO" if SECURITY_TOMOYO=y
+
+ config DEFAULT_SECURITY_APPARMOR
+ bool "AppArmor" if SECURITY_APPARMOR=y
+
+ config DEFAULT_SECURITY_DAC
+ bool "Unix Discretionary Access Controls"
+
+endchoice
+
config LSM
string "Ordered list of enabled LSMs"
+ default
"yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if
DEFAULT_SECURITY_SMACK
+ default
"yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if
DEFAULT_SECURITY_TOMOYO
+ default
"yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if
DEFAULT_SECURITY_APPARMOR
default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
help
A comma-separated list of LSMs, in initialization order.

(I don't see a way to include an earlier config string in a new
default.) Thoughts?

--
Kees Cook

2019-03-16 08:08:23

by Tetsuo Handa

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On 2019/03/16 14:38, Kees Cook wrote:
> config LSM
> string "Ordered list of enabled LSMs"
> + default "yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if DEFAULT_SECURITY_SMACK
> + default "yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if DEFAULT_SECURITY_TOMOYO
> + default "yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if DEFAULT_SECURITY_APPARMOR
> default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> help
> A comma-separated list of LSMs, in initialization order.
>
> (I don't see a way to include an earlier config string in a new
> default.) Thoughts?
>

Hmm, DEFAULT_SECURITY_TOMOYO no longer works because TOMOYO will be
always enabled as long as CONFIG_SECURITY_TOMOYO=y. Maybe

config LSM
string "Ordered list of enabled LSMs"
- default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+ default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
+ default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
+ default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
+ default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
+ default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
help
A comma-separated list of LSMs, in initialization order.

(i.e. include only up to one major LSM as default choice, and allow manually including
multiple major LSMs at both kernel build time and kernel boot time) is better?


2019-03-17 01:02:37

by Casey Schaufler

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On 3/16/2019 1:08 AM, Tetsuo Handa wrote:
> On 2019/03/16 14:38, Kees Cook wrote:
>> config LSM
>> string "Ordered list of enabled LSMs"
>> + default "yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor" if DEFAULT_SECURITY_SMACK
>> + default "yama,loadpin,safesetid,integrity,tomoyo,selinux,smack,apparmor" if DEFAULT_SECURITY_TOMOYO
>> + default "yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo" if DEFAULT_SECURITY_APPARMOR
>> default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
>> help
>> A comma-separated list of LSMs, in initialization order.
>>
>> (I don't see a way to include an earlier config string in a new
>> default.) Thoughts?
>>
> Hmm, DEFAULT_SECURITY_TOMOYO no longer works because TOMOYO will be
> always enabled as long as CONFIG_SECURITY_TOMOYO=y. Maybe
>
> config LSM
> string "Ordered list of enabled LSMs"
> - default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> + default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> + default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> + default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> + default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> + default "yama,loadpin,safesetid,integrity" if DEFAULT_SECURITY_DAC
> help
> A comma-separated list of LSMs, in initialization order.
>
> (i.e. include only up to one major LSM as default choice, and allow manually including
> multiple major LSMs at both kernel build time and kernel boot time) is better?

I think this looks pretty good.


2019-03-19 10:56:28

by Tetsuo Handa

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

Since Kees Cook seems to be busy now, here is my version...

From 885553e4793d9af2d4e9e99c7d137b0ec7b5f8ad Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <[email protected]>
Date: Tue, 19 Mar 2019 19:52:31 +0900
Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"

Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
default value. That commit expected that existing users (upgrading from
Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
since users might forget to edit CONFIG_LSM value, this patch revives
the choice (only for providing the default value for CONFIG_LSM) in order
to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
old kernel configs.

Reported-by: Jakub Kicinski <[email protected]>
Signed-off-by: Kees Cook <[email protected]>
Signed-off-by: Tetsuo Handa <[email protected]>
---
security/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
1 file changed, 35 insertions(+), 1 deletion(-)

diff --git a/security/Kconfig b/security/Kconfig
index 1d6463f..743e594 100644
--- a/security/Kconfig
+++ b/security/Kconfig
@@ -239,9 +239,43 @@ source "security/safesetid/Kconfig"

source "security/integrity/Kconfig"

+choice
+ prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
+ default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
+ default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
+ default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
+ default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
+ default DEFAULT_SECURITY_DAC
+
+ help
+ This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
+ kernel config to CONFIG_LSM in new kernel config. Don't change this choice
+ unless you are creating a fresh kernel config, for this choice will be
+ ignored after CONFIG_LSM is once defined.
+
+ config DEFAULT_SECURITY_SELINUX
+ bool "SELinux" if SECURITY_SELINUX=y
+
+ config DEFAULT_SECURITY_SMACK
+ bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
+
+ config DEFAULT_SECURITY_TOMOYO
+ bool "TOMOYO" if SECURITY_TOMOYO=y
+
+ config DEFAULT_SECURITY_APPARMOR
+ bool "AppArmor" if SECURITY_APPARMOR=y
+ config DEFAULT_SECURITY_DAC
+ bool "Unix Discretionary Access Controls"
+
+endchoice
+
config LSM
string "Ordered list of enabled LSMs"
- default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
+ default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
+ default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
+ default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
+ default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
+ default "yama,loadpin,safesetid,integrity"
help
A comma-separated list of LSMs, in initialization order.
Any LSMs left off this list will be ignored. This can be
--
1.8.3.1



2019-03-19 15:03:10

by Casey Schaufler

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On 3/19/2019 3:56 AM, Tetsuo Handa wrote:
> Since Kees Cook seems to be busy now, here is my version...
>
> From 885553e4793d9af2d4e9e99c7d137b0ec7b5f8ad Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <[email protected]>
> Date: Tue, 19 Mar 2019 19:52:31 +0900
> Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"
>
> Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
> CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
> security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
> default value. That commit expected that existing users (upgrading from
> Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
> their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
> since users might forget to edit CONFIG_LSM value, this patch revives
> the choice (only for providing the default value for CONFIG_LSM) in order
> to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
> old kernel configs.
>
> Reported-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>

Acked-by: Casey Schaufler <[email protected]>


> ---
> security/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463f..743e594 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -239,9 +239,43 @@ source "security/safesetid/Kconfig"
>
> source "security/integrity/Kconfig"
>
> +choice
> + prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
> + default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
> + default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> + default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> + default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> + default DEFAULT_SECURITY_DAC
> +
> + help
> + This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
> + kernel config to CONFIG_LSM in new kernel config. Don't change this choice
> + unless you are creating a fresh kernel config, for this choice will be
> + ignored after CONFIG_LSM is once defined.
> +
> + config DEFAULT_SECURITY_SELINUX
> + bool "SELinux" if SECURITY_SELINUX=y
> +
> + config DEFAULT_SECURITY_SMACK
> + bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
> +
> + config DEFAULT_SECURITY_TOMOYO
> + bool "TOMOYO" if SECURITY_TOMOYO=y
> +
> + config DEFAULT_SECURITY_APPARMOR
> + bool "AppArmor" if SECURITY_APPARMOR=y
> + config DEFAULT_SECURITY_DAC
> + bool "Unix Discretionary Access Controls"
> +
> +endchoice
> +
> config LSM
> string "Ordered list of enabled LSMs"
> - default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> + default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> + default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> + default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> + default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> + default "yama,loadpin,safesetid,integrity"
> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list will be ignored. This can be

2019-03-21 16:38:37

by Kees Cook

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Tue, Mar 19, 2019 at 3:56 AM Tetsuo Handa
<[email protected]> wrote:
>
> Since Kees Cook seems to be busy now, here is my version...
>
> From 885553e4793d9af2d4e9e99c7d137b0ec7b5f8ad Mon Sep 17 00:00:00 2001
> From: Tetsuo Handa <[email protected]>
> Date: Tue, 19 Mar 2019 19:52:31 +0900
> Subject: [PATCH] LSM: Revive CONFIG_DEFAULT_SECURITY_* for "make oldconfig"
>
> Commit 70b62c25665f636c ("LoadPin: Initialize as ordered LSM") removed
> CONFIG_DEFAULT_SECURITY_{SELINUX,SMACK,TOMOYO,APPARMOR,DAC} from
> security/Kconfig and changed CONFIG_LSM to provide a fixed ordering as a
> default value. That commit expected that existing users (upgrading from
> Linux 5.0 and earlier) will edit CONFIG_LSM value in accordance with
> their CONFIG_DEFAULT_SECURITY_* choice in their old kernel configs. But
> since users might forget to edit CONFIG_LSM value, this patch revives
> the choice (only for providing the default value for CONFIG_LSM) in order
> to make sure that CONFIG_LSM reflects CONFIG_DEFAULT_SECURITY_* from their
> old kernel configs.
>
> Reported-by: Jakub Kicinski <[email protected]>
> Signed-off-by: Kees Cook <[email protected]>
> Signed-off-by: Tetsuo Handa <[email protected]>
> ---
> security/Kconfig | 36 +++++++++++++++++++++++++++++++++++-
> 1 file changed, 35 insertions(+), 1 deletion(-)
>
> diff --git a/security/Kconfig b/security/Kconfig
> index 1d6463f..743e594 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -239,9 +239,43 @@ source "security/safesetid/Kconfig"
>
> source "security/integrity/Kconfig"
>
> +choice
> + prompt "Default security module [superseded by 'Ordered list of enabled LSMs' below]"
> + default DEFAULT_SECURITY_SELINUX if SECURITY_SELINUX
> + default DEFAULT_SECURITY_SMACK if SECURITY_SMACK
> + default DEFAULT_SECURITY_TOMOYO if SECURITY_TOMOYO
> + default DEFAULT_SECURITY_APPARMOR if SECURITY_APPARMOR
> + default DEFAULT_SECURITY_DAC
> +
> + help
> + This choice is there only for converting CONFIG_DEFAULT_SECURITY in old
> + kernel config to CONFIG_LSM in new kernel config. Don't change this choice
> + unless you are creating a fresh kernel config, for this choice will be
> + ignored after CONFIG_LSM is once defined.
> +
> + config DEFAULT_SECURITY_SELINUX
> + bool "SELinux" if SECURITY_SELINUX=y
> +
> + config DEFAULT_SECURITY_SMACK
> + bool "Simplified Mandatory Access Control" if SECURITY_SMACK=y
> +
> + config DEFAULT_SECURITY_TOMOYO
> + bool "TOMOYO" if SECURITY_TOMOYO=y
> +
> + config DEFAULT_SECURITY_APPARMOR
> + bool "AppArmor" if SECURITY_APPARMOR=y
> + config DEFAULT_SECURITY_DAC
> + bool "Unix Discretionary Access Controls"
> +
> +endchoice
> +
> config LSM
> string "Ordered list of enabled LSMs"
> - default "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
> + default "yama,loadpin,safesetid,integrity,selinux" if DEFAULT_SECURITY_SELINUX
> + default "yama,loadpin,safesetid,integrity,smack" if DEFAULT_SECURITY_SMACK
> + default "yama,loadpin,safesetid,integrity,tomoyo" if DEFAULT_SECURITY_TOMOYO
> + default "yama,loadpin,safesetid,integrity,apparmor" if DEFAULT_SECURITY_APPARMOR
> + default "yama,loadpin,safesetid,integrity"
> help
> A comma-separated list of LSMs, in initialization order.
> Any LSMs left off this list will be ignored. This can be

This is mostly good. I'd like to keep the other LSMs listed though
(similar to what I had originally) so that if a legacy-major doesn't
initialize, later ones will be. I want to remove the concept of
"major" LSMs. The only thing that should matter is init order...

-Kees

> --
> 1.8.3.1
>
>


--
Kees Cook

2019-03-21 21:10:23

by Tetsuo Handa

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On 2019/03/22 1:38, Kees Cook wrote:
> This is mostly good. I'd like to keep the other LSMs listed though
> (similar to what I had originally) so that if a legacy-major doesn't
> initialize, later ones will be. I want to remove the concept of
> "major" LSMs. The only thing that should matter is init order...

Excuse me? Are you saying that

if a legacy-major (which is defined as the "Default security module")
doesn't initialize, later ones (any of selinux,smack,tomoyo,apparmor
except the one which is defined as "Default security module") will be
initialized

? That sounds strange to me. Any of selinux,smack,tomoyo,apparmor can be
initialized when specified by lsm= kernel command line option (or security=
kernel command line option if lsm= kernel command line option is not
specified), won't it?


2019-03-22 22:45:26

by Kees Cook

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On Thu, Mar 21, 2019 at 2:10 PM Tetsuo Handa
<[email protected]> wrote:
>
> On 2019/03/22 1:38, Kees Cook wrote:
> > This is mostly good. I'd like to keep the other LSMs listed though
> > (similar to what I had originally) so that if a legacy-major doesn't
> > initialize, later ones will be. I want to remove the concept of
> > "major" LSMs. The only thing that should matter is init order...
>
> Excuse me? Are you saying that
>
> if a legacy-major (which is defined as the "Default security module")
> doesn't initialize, later ones (any of selinux,smack,tomoyo,apparmor
> except the one which is defined as "Default security module") will be
> initialized
>
> ? That sounds strange to me. Any of selinux,smack,tomoyo,apparmor can be
> initialized when specified by lsm= kernel command line option (or security=
> kernel command line option if lsm= kernel command line option is not
> specified), won't it?

It breaks the backward-compat for the "security=" line. If a system is
booted with CONFIG_LSM="minors...,apparmor" and "security=selinux",
neither apparmor nor selinux will be initialized. The logic on
"security=..." depends on the other LSMs being present in the list.

-Kees

--
Kees Cook

2019-03-23 02:44:19

by Tetsuo Handa

[permalink] [raw]
Subject: Re: mount.nfs: Protocol error after upgrade to linux/master

On 2019/03/23 7:45, Kees Cook wrote:
> It breaks the backward-compat for the "security=" line. If a system is
> booted with CONFIG_LSM="minors...,apparmor" and "security=selinux",
> neither apparmor nor selinux will be initialized. The logic on
> "security=..." depends on the other LSMs being present in the list.

Really? The logic on "security=..." does not depend on LSM_FLAG_LEGACY_MAJOR
LSMs being present in the CONFIG_LSM= list, for ordered_lsm_parse() does

(Step 1) Enable LSM_ORDER_FIRST module (i.e. capability).

(Step 2) Disable LSM_FLAG_LEGACY_MAJOR modules which was not specified
by "security=" parameter when "security=" parameter was specified.

(Step 3) Enable modules specified by "lsm=" parameter (or CONFIG_LSM= settings
if "lsm=" parameter was not specified).

(Step 4) Enable up to one LSM_FLAG_LEGACY_MAJOR module which was specified
by "security=" parameter when "security=" parameter was specified.

(Step 5) Disable all unused modules.

and (Step 4) will compensate for lack of that module in (Step 3).