diff mbox series

tomoyo: Add a kernel config option for fuzzing testing.

Message ID 1551362770-8655-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp (mailing list archive)
State New, archived
Headers show
Series tomoyo: Add a kernel config option for fuzzing testing. | expand

Commit Message

Tetsuo Handa Feb. 28, 2019, 2:06 p.m. UTC
syzbot is reporting kernel panic triggered by memory allocation fault
injection before loading TOMOYO's policy [1]. To make the fuzzing tests
useful, we need to assign a profile other than "disabled" (no-op) mode.
Therefore, let's allow syzbot to load TOMOYO's built-in policy for
"learning" mode using a kernel config option. This option must not be
enabled for kernels built for production system, for this option also
disables domain/program checks when modifying policy configuration via
/sys/kernel/security/tomoyo/ interface.

[1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95

Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 security/tomoyo/Kconfig  | 10 ++++++++++
 security/tomoyo/common.c | 13 ++++++++++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

Comments

Tetsuo Handa March 4, 2019, 1:35 p.m. UTC | #1
James, please include this patch for 5.1-rc1, for failing to include
this patch will prevent various trees (SELinux/Smack/AppArmor) from
proper testing due to this problem because syzbot is enabling both
TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.

By including this patch and building kernels with this config option
enabled, syzbot will be able to continue proper testing.

On 2019/02/28 23:06, Tetsuo Handa wrote:
> syzbot is reporting kernel panic triggered by memory allocation fault
> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
> useful, we need to assign a profile other than "disabled" (no-op) mode.
> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
> "learning" mode using a kernel config option. This option must not be
> enabled for kernels built for production system, for this option also
> disables domain/program checks when modifying policy configuration via
> /sys/kernel/security/tomoyo/ interface.
> 
> [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
> 
> Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
> Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
> ---
>  security/tomoyo/Kconfig  | 10 ++++++++++
>  security/tomoyo/common.c | 13 ++++++++++++-
>  2 files changed, 22 insertions(+), 1 deletion(-)
>
Stephen Smalley March 4, 2019, 2:34 p.m. UTC | #2
On 3/4/19 8:35 AM, Tetsuo Handa wrote:
> James, please include this patch for 5.1-rc1, for failing to include
> this patch will prevent various trees (SELinux/Smack/AppArmor) from
> proper testing due to this problem because syzbot is enabling both
> TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.
> 
> By including this patch and building kernels with this config option
> enabled, syzbot will be able to continue proper testing.

Could you clarify the status of upstream TOMOYO?  Is its MAINTAINERS 
entry still accurate?  Is it still actively maintained?  Its existing 
documentation (in-tree and the tomoyo.osdn.jp site) seem to suggest that 
using the pre-LSM version and/or AKARI are preferred to using the 
upstream version. Is that still true, and do you envision it changing?

> 
> On 2019/02/28 23:06, Tetsuo Handa wrote:
>> syzbot is reporting kernel panic triggered by memory allocation fault
>> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
>> useful, we need to assign a profile other than "disabled" (no-op) mode.
>> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
>> "learning" mode using a kernel config option. This option must not be
>> enabled for kernels built for production system, for this option also
>> disables domain/program checks when modifying policy configuration via
>> /sys/kernel/security/tomoyo/ interface.
>>
>> [1] https://syzkaller.appspot.com/bug?extid=29569ed06425fcf67a95
>>
>> Reported-by: syzbot <syzbot+e1b8084e532b6ee7afab@syzkaller.appspotmail.com>
>> Reported-by: syzbot <syzbot+29569ed06425fcf67a95@syzkaller.appspotmail.com>
>> Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
>> ---
>>   security/tomoyo/Kconfig  | 10 ++++++++++
>>   security/tomoyo/common.c | 13 ++++++++++++-
>>   2 files changed, 22 insertions(+), 1 deletion(-)
>>
Tetsuo Handa March 4, 2019, 11:59 p.m. UTC | #3
Stephen Smalley wrote:
> On 3/4/19 8:35 AM, Tetsuo Handa wrote:
> > James, please include this patch for 5.1-rc1, for failing to include
> > this patch will prevent various trees (SELinux/Smack/AppArmor) from
> > proper testing due to this problem because syzbot is enabling both
> > TOMOYO and one of SELinux/Smack/AppArmor via lsm= boot parameter.
> > 
> > By including this patch and building kernels with this config option
> > enabled, syzbot will be able to continue proper testing.
> 
> Could you clarify the status of upstream TOMOYO?  Is its MAINTAINERS 
> entry still accurate?  Is it still actively maintained?

Mainly bugfixes and Q&A phase like
https://osdn.net/projects/tomoyo/lists/archive/users-en/2017-July/000685.html .

Now that TOMOYO can coexist with one of SELinux/Smack/AppArmor, TOMOYO users
can borrow ready-made rules from them and utilize TOMOYO's ability to generate
custom-made rules for things like
https://tomoyo.osdn.jp/1.8/ssh-protection-using-environment.html .

>                                                          Its existing 
> documentation (in-tree and the tomoyo.osdn.jp site) seem to suggest that 
> using the pre-LSM version and/or AKARI are preferred to using the 
> upstream version. Is that still true, and do you envision it changing?

I guess that majority of TOMOYO users are now using the upstream version. But
pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
officially supported, for e.g. Fedora/RHEL users will need to use AKARI because
TOMOYO is not available ( https://bugzilla.redhat.com/show_bug.cgi?id=542986 ).
James Morris March 5, 2019, 3:32 a.m. UTC | #4
On Tue, 5 Mar 2019, Tetsuo Handa wrote:

> I guess that majority of TOMOYO users are now using the upstream version. But
> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> officially supported

You mean dynamically loadable LSMs?

There are no plans to support this.
Tetsuo Handa March 11, 2019, 1:18 p.m. UTC | #5
On 2019/03/05 12:32, James Morris wrote:
> On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> 
>> I guess that majority of TOMOYO users are now using the upstream version. But
>> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
>> officially supported
> 
> You mean dynamically loadable LSMs?

Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.

> 
> There are no plans to support this.

Currently you don't have a plan. But I have.

It took 10+ years to be able to allow coexisting inode based access control
and name based access control. And there are people who still cannot afford
keeping upstream LSM modules enabled.

Anyway, your question is irrelevant to whether to allow syzbot to test
TOMOYO module. syzbot already bisected this problem to an innocent
commit 89a9684ea158dd7e ("LSM: Ignore "security=" when "lsm=" is specified")
at https://syzkaller.appspot.com/bug?id=32ab41bbdc0c28643c507dd0cf1eea1a9ce67837 .
Will you send this patch to linux.git so that syzbot can test TOMOYO module?
James Morris March 12, 2019, 5:19 p.m. UTC | #6
On Mon, 11 Mar 2019, Tetsuo Handa wrote:

> On 2019/03/05 12:32, James Morris wrote:
> > On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> > 
> >> I guess that majority of TOMOYO users are now using the upstream version. But
> >> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> >> officially supported
> > 
> > You mean dynamically loadable LSMs?
> 
> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.

What do you mean cannot afford ?
James Morris March 12, 2019, 6:21 p.m. UTC | #7
On Thu, 28 Feb 2019, Tetsuo Handa wrote:

> syzbot is reporting kernel panic triggered by memory allocation fault
> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
> useful, we need to assign a profile other than "disabled" (no-op) mode.
> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
> "learning" mode using a kernel config option. This option must not be
> enabled for kernels built for production system, for this option also
> disables domain/program checks when modifying policy configuration via
> /sys/kernel/security/tomoyo/ interface.

I don't understand the logic here. If the cause of this is no policy 
loaded combined with running out of memory, shouldn't the no-policy issue 
be dealt with earlier?
Tetsuo Handa March 12, 2019, 8:56 p.m. UTC | #8
On 2019/03/13 3:21, James Morris wrote:
> On Thu, 28 Feb 2019, Tetsuo Handa wrote:
> 
>> syzbot is reporting kernel panic triggered by memory allocation fault
>> injection before loading TOMOYO's policy [1]. To make the fuzzing tests
>> useful, we need to assign a profile other than "disabled" (no-op) mode.
>> Therefore, let's allow syzbot to load TOMOYO's built-in policy for
>> "learning" mode using a kernel config option. This option must not be
>> enabled for kernels built for production system, for this option also
>> disables domain/program checks when modifying policy configuration via
>> /sys/kernel/security/tomoyo/ interface.
> 
> I don't understand the logic here. If the cause of this is no policy 
> loaded combined with running out of memory, shouldn't the no-policy issue 
> be dealt with earlier?
> 

This patch is for automatically loading minimal policy at boot time
in order to address the no-policy issue. By applying this patch, syzbot
can test TOMOYO module without modifying userspace to load TOMOYO's policy
when /sbin/init starts.
Tetsuo Handa March 12, 2019, 9:15 p.m. UTC | #9
On 2019/03/13 2:19, James Morris wrote:
> On Mon, 11 Mar 2019, Tetsuo Handa wrote:
> 
>> On 2019/03/05 12:32, James Morris wrote:
>>> On Tue, 5 Mar 2019, Tetsuo Handa wrote:
>>>
>>>> I guess that majority of TOMOYO users are now using the upstream version. But
>>>> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
>>>> officially supported
>>>
>>> You mean dynamically loadable LSMs?
>>
>> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
>> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> 
> What do you mean cannot afford ?
> 

Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
the kernel command line.
James Morris March 12, 2019, 9:19 p.m. UTC | #10
On Wed, 13 Mar 2019, Tetsuo Handa wrote:

> >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> > 
> > What do you mean cannot afford ?
> > 
> 
> Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> the kernel command line.

Why do they have to do this?
James Morris March 12, 2019, 9:24 p.m. UTC | #11
On Wed, 13 Mar 2019, Tetsuo Handa wrote:

> > I don't understand the logic here. If the cause of this is no policy 
> > loaded combined with running out of memory, shouldn't the no-policy issue 
> > be dealt with earlier?
> > 
> 
> This patch is for automatically loading minimal policy at boot time
> in order to address the no-policy issue. By applying this patch, syzbot
> can test TOMOYO module without modifying userspace to load TOMOYO's policy
> when /sbin/init starts.

If syzbot is trying to test Tomoyo and this requires policy to be loaded, 
shouldn't it do that?

And again, I think the no-policy situation needs to be detected before 
you start trying to apply memory policies to running processes. Surely 
there is some much earlier point during initialization that you will 
detect that there is no policy?
Edwin Zimmerman March 12, 2019, 9:56 p.m. UTC | #12
On March 12, 2019 5:15, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote
> On 2019/03/13 2:19, James Morris wrote:
> > On Mon, 11 Mar 2019, Tetsuo Handa wrote:
> >
> >> On 2019/03/05 12:32, James Morris wrote:
> >>> On Tue, 5 Mar 2019, Tetsuo Handa wrote:
> >>>
> >>>> I guess that majority of TOMOYO users are now using the upstream version. But
> >>>> pre-LSM version and/or AKARI will remain there until LKM-based LSMs becomes
> >>>> officially supported
> >>>
> >>> You mean dynamically loadable LSMs?
> >>
> >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> >
> > What do you mean cannot afford ?
> >
> 
> Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> the kernel command line.

If you specifically don't want in-kernel LSMs, and you specifically do want an out-of-tree LSM,
there are other options. For example, you could just livepatch the security_* hooks you need, 
since you already would using an LKM-based LSM.  That would give you your
out-of-tree module and would also disable selinux on the hooks that got livepatched.
Tetsuo Handa March 13, 2019, 10:29 a.m. UTC | #13
On 2019/03/13 6:24, James Morris wrote:
> On Wed, 13 Mar 2019, Tetsuo Handa wrote:
> 
>>> I don't understand the logic here. If the cause of this is no policy 
>>> loaded combined with running out of memory, shouldn't the no-policy issue 
>>> be dealt with earlier?
>>>
>>
>> This patch is for automatically loading minimal policy at boot time
>> in order to address the no-policy issue. By applying this patch, syzbot
>> can test TOMOYO module without modifying userspace to load TOMOYO's policy
>> when /sbin/init starts.
> 
> If syzbot is trying to test Tomoyo and this requires policy to be loaded, 
> shouldn't it do that?

SELinux has disabled/permissive/enforcing modes.
And syzbot is testing SELinux in permissive mode, isn't it?

TOMOYO has disabled/learning/permissive/enforcing modes.
And syzbot will test TOMOYO in learning mode.

This patch is required for telling TOMOYO to run in learning mode, by
loading minimal policy, without asking userspace to run policy loader.
This patch is easier than asking syzbot users to update their filesystem
images in order to embed policy loader and minimal policy into their
filesystem images.

> 
> And again, I think the no-policy situation needs to be detected before 
> you start trying to apply memory policies to running processes. Surely 
> there is some much earlier point during initialization that you will 
> detect that there is no policy?

TOMOYO is already detecting no-policy situation. TOMOYO is calling panic()
due to "memory allocation failure before loading minimal policy".

This patch avoids panic() by automatically loading minimal policy which is
embedded into the kernel.
Paul Moore March 13, 2019, 1:17 p.m. UTC | #14
On Wed, Mar 13, 2019 at 6:29 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
> On 2019/03/13 6:24, James Morris wrote:
> > On Wed, 13 Mar 2019, Tetsuo Handa wrote:
> >
> >>> I don't understand the logic here. If the cause of this is no policy
> >>> loaded combined with running out of memory, shouldn't the no-policy issue
> >>> be dealt with earlier?
> >>>
> >>
> >> This patch is for automatically loading minimal policy at boot time
> >> in order to address the no-policy issue. By applying this patch, syzbot
> >> can test TOMOYO module without modifying userspace to load TOMOYO's policy
> >> when /sbin/init starts.
> >
> > If syzbot is trying to test Tomoyo and this requires policy to be loaded,
> > shouldn't it do that?
>
> SELinux has disabled/permissive/enforcing modes.
> And syzbot is testing SELinux in permissive mode, isn't it?

I've lost track of what syzbot currently does, but in the beginning it
ran with SELinux enabled (probably in permissive mode, but that isn't
important here) without a policy loaded and that caused a handful of
problems which we have since fixed.  While it is not recommended, you
should be able to safely run a SELinux enabled system without a policy
loaded.

> TOMOYO has disabled/learning/permissive/enforcing modes.
> And syzbot will test TOMOYO in learning mode.
>
> This patch is required for telling TOMOYO to run in learning mode, by
> loading minimal policy, without asking userspace to run policy loader.
> This patch is easier than asking syzbot users to update their filesystem
> images in order to embed policy loader and minimal policy into their
> filesystem images.
James Morris March 13, 2019, 8 p.m. UTC | #15
On Tue, 12 Mar 2019, Edwin Zimmerman wrote:

> On March 12, 2019 5:15, Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote
> > >> Yes. As long as upstream can't accept all LSM modules, and some people cannot afford
> > >> utilizing upstream LSM modules, LKM-based LSMs will be needed by such people.
> > >
> > > What do you mean cannot afford ?
> > >
> > 
> > Some people have to set SELINUX=disabled in /etc/selinux/config or pass security=none from
> > the kernel command line.
> 
> If you specifically don't want in-kernel LSMs, and you specifically do want an out-of-tree LSM,
> there are other options. For example, you could just livepatch the security_* hooks you need, 
> since you already would using an LKM-based LSM.  That would give you your
> out-of-tree module and would also disable selinux on the hooks that got livepatched.
> 

Ahh, ok, this is about out of tree LSMs.

This has been discussed many times over the years and the answer is always 
the same: we will not add infrastructure to the kernel to support out of 
tree code.  This is a long-standing tenet of the Linux kernel.
Tetsuo Handa March 25, 2019, 9:09 p.m. UTC | #16
James,

I think that nothing prevents this patch.
diff mbox series

Patch

diff --git a/security/tomoyo/Kconfig b/security/tomoyo/Kconfig
index 404dce6..a00ab7e 100644
--- a/security/tomoyo/Kconfig
+++ b/security/tomoyo/Kconfig
@@ -74,3 +74,13 @@  config SECURITY_TOMOYO_ACTIVATION_TRIGGER
 	  You can override this setting via TOMOYO_trigger= kernel command line
 	  option. For example, if you pass init=/bin/systemd option, you may
 	  want to also pass TOMOYO_trigger=/bin/systemd option.
+
+config SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
+	bool "Use insecure built-in settings for fuzzing tests."
+	default n
+	depends on SECURITY_TOMOYO
+	select SECURITY_TOMOYO_OMIT_USERSPACE_LOADER
+	help
+	  Enabling this option forces minimal built-in policy and disables
+	  domain/program checks for run-time policy modifications. Please enable
+	  this option only if this kernel is built for doing fuzzing tests.
diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 57988d9..dd3d594 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -940,7 +940,7 @@  static bool tomoyo_manager(void)
 	const char *exe;
 	const struct task_struct *task = current;
 	const struct tomoyo_path_info *domainname = tomoyo_domain()->domainname;
-	bool found = false;
+	bool found = IS_ENABLED(CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING);
 
 	if (!tomoyo_policy_loaded)
 		return true;
@@ -2810,6 +2810,16 @@  void tomoyo_check_profile(void)
  */
 void __init tomoyo_load_builtin_policy(void)
 {
+#ifdef CONFIG_SECURITY_TOMOYO_INSECURE_BUILTIN_SETTING
+	static char tomoyo_builtin_profile[] __initdata =
+		"PROFILE_VERSION=20150505\n"
+		"0-CONFIG={ mode=learning grant_log=no reject_log=yes }\n";
+	static char tomoyo_builtin_exception_policy[] __initdata =
+		"aggregator proc:/self/exe /proc/self/exe\n";
+	static char tomoyo_builtin_domain_policy[] __initdata = "";
+	static char tomoyo_builtin_manager[] __initdata = "";
+	static char tomoyo_builtin_stat[] __initdata = "";
+#else
 	/*
 	 * This include file is manually created and contains built-in policy
 	 * named "tomoyo_builtin_profile", "tomoyo_builtin_exception_policy",
@@ -2817,6 +2827,7 @@  void __init tomoyo_load_builtin_policy(void)
 	 * "tomoyo_builtin_stat" in the form of "static char [] __initdata".
 	 */
 #include "builtin-policy.h"
+#endif
 	u8 i;
 	const int idx = tomoyo_read_lock();