WARNING in apparmor_secid_to_secctx

[syzbot]

Hello,

syzbot found the following crash on:

HEAD commit: 817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
git tree: net-next
console output: https://syzkaller.appspot.com/x/log.txt?x=1536d296400000
kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
compiler: gcc (GCC) 8.0.1 20180413 (experimental)

Unfortunately, I don't have any reproducer for this crash yet.

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+210161...@syzkaller.appspotmail.com

------------[ cut here ]------------
AppArmor WARN apparmor_secid_to_secctx: ((!secdata)):
WARNING: CPU: 0 PID: 14826 at security/apparmor/secid.c:82
apparmor_secid_to_secctx+0x2b5/0x2f0 security/apparmor/secid.c:82
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 14826 Comm: syz-executor1 Not tainted 4.19.0-rc1+ #193
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x252/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993
RIP: 0010:apparmor_secid_to_secctx+0x2b5/0x2f0 security/apparmor/secid.c:82
Code: c7 c7 40 66 58 87 e8 6a 6d 0f fe 0f 0b e9 6c fe ff ff e8 3e aa 44 fe
48 c7 c6 80 67 58 87 48 c7 c7 a0 65 58 87 e8 4b 6d 0f fe <0f> 0b e9 3f fe
ff ff 48 89 df e8 fc a7 83 fe e9 ed fe ff ff bb f4
RSP: 0018:ffff8801ba1bed10 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8801ba1beed0 RCX: ffffc9000227e000
RDX: 0000000000018482 RSI: ffffffff8163ac01 RDI: 0000000000000001
RBP: ffff8801ba1bed30 R08: ffff8801b80ec080 R09: ffffed003b603eca
R10: ffffed003b603eca R11: ffff8801db01f657 R12: 0000000000000001
R13: 0000000000000000 R14: 0000000000000000 R15: ffff8801ba1beed0
security_secid_to_secctx+0x63/0xc0 security/security.c:1314
ctnetlink_secctx_size net/netfilter/nf_conntrack_netlink.c:621 [inline]
ctnetlink_nlmsg_size net/netfilter/nf_conntrack_netlink.c:659 [inline]
ctnetlink_conntrack_event+0x303/0x1470
net/netfilter/nf_conntrack_netlink.c:706
nf_conntrack_eventmask_report+0x55f/0x930
net/netfilter/nf_conntrack_ecache.c:151
nf_conntrack_event_report include/net/netfilter/nf_conntrack_ecache.h:112
[inline]
nf_ct_delete+0x33c/0x5d0 net/netfilter/nf_conntrack_core.c:601
nf_ct_iterate_cleanup+0x48c/0x5e0 net/netfilter/nf_conntrack_core.c:1892
nf_ct_iterate_cleanup_net+0x23c/0x2d0
net/netfilter/nf_conntrack_core.c:1974
ctnetlink_flush_conntrack net/netfilter/nf_conntrack_netlink.c:1226
[inline]
ctnetlink_del_conntrack+0x66c/0x850
net/netfilter/nf_conntrack_netlink.c:1258
nfnetlink_rcv_msg+0xd88/0x1070 net/netfilter/nfnetlink.c:228
netlink_rcv_skb+0x172/0x440 net/netlink/af_netlink.c:2454
nfnetlink_rcv+0x1c0/0x4d0 net/netfilter/nfnetlink.c:560
netlink_unicast_kernel net/netlink/af_netlink.c:1317 [inline]
netlink_unicast+0x5a0/0x760 net/netlink/af_netlink.c:1343
netlink_sendmsg+0xa18/0xfc0 net/netlink/af_netlink.c:1908
sock_sendmsg_nosec net/socket.c:621 [inline]
sock_sendmsg+0xd5/0x120 net/socket.c:631
___sys_sendmsg+0x7fd/0x930 net/socket.c:2114
__sys_sendmsg+0x11d/0x290 net/socket.c:2152
__do_sys_sendmsg net/socket.c:2161 [inline]
__se_sys_sendmsg net/socket.c:2159 [inline]
__x64_sys_sendmsg+0x78/0xb0 net/socket.c:2159
do_syscall_64+0x1b9/0x820 arch/x86/entry/common.c:290
entry_SYSCALL_64_after_hwframe+0x49/0xbe
RIP: 0033:0x457089
Code: fd b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00 00 66 90 48 89 f8 48 89 f7
48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff
ff 0f 83 cb b4 fb ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007f7bc6e03c78 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 00007f7bc6e046d4 RCX: 0000000000457089
RDX: 0000000000000000 RSI: 0000000020d65000 RDI: 0000000000000003
RBP: 00000000009300a0 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00000000ffffffff
R13: 00000000004d4588 R14: 00000000004c8d5c R15: 0000000000000000
Dumping ftrace buffer:
(ftrace buffer empty)
Kernel Offset: disabled
Rebooting in 86400 seconds..


---
This bug is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at syzk...@googlegroups.com.

syzbot will keep track of this bug report. See:
https://goo.gl/tpsmEJ#bug-status-tracking for how to communicate with
syzbot.

[Dmitry Vyukov]

On Wed, Aug 29, 2018 at 7:17 PM, syzbot
<syzbot+210161...@syzkaller.appspotmail.com> wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit: 817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
> git tree: net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1536d296400000
> kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
> dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+210161...@syzkaller.appspotmail.com

Hi John, Tyler,

I've switched syzbot from selinux to apparmor as we discussed on lss:
https://github.com/google/syzkaller/commit/2c6cb254ae6c06f61e3aba21bb89ffb05b5db946

As expedited fix for this as possible would be nice to get, because we
are currently getting 1 machine crash/minute on this bug:
https://syzkaller.appspot.com/bug?extid=ab1882df6ecbb06d59be

[syzbot]

syzbot has found a reproducer for the following crash on:

HEAD commit: 817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
git tree: net-next

console output: https://syzkaller.appspot.com/x/log.txt?x=16662cb6400000

kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
compiler: gcc (GCC) 8.0.1 20180413 (experimental)

syz repro: https://syzkaller.appspot.com/x/repro.syz?x=14f20a96400000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10efd7bc400000

IMPORTANT: if you fix the bug, please add the following tag to the commit:
Reported-by: syzbot+210161...@syzkaller.appspotmail.com

random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)
random: sshd: uninitialized urandom read (32 bytes read)

------------[ cut here ]------------
AppArmor WARN apparmor_secid_to_secctx: ((!secdata)):

WARNING: CPU: 0 PID: 4682 at security/apparmor/secid.c:82

apparmor_secid_to_secctx+0x2b5/0x2f0 security/apparmor/secid.c:82
Kernel panic - not syncing: panic_on_warn set ...

CPU: 0 PID: 4682 Comm: syz-executor028 Not tainted 4.19.0-rc1+ #193

Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS
Google 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:77 [inline]
dump_stack+0x1c9/0x2b4 lib/dump_stack.c:113
panic+0x238/0x4e7 kernel/panic.c:184
__warn.cold.8+0x163/0x1ba kernel/panic.c:536
report_bug+0x252/0x2d0 lib/bug.c:186
fixup_bug arch/x86/kernel/traps.c:178 [inline]
do_error_trap+0x1fc/0x4d0 arch/x86/kernel/traps.c:296
do_invalid_op+0x1b/0x20 arch/x86/kernel/traps.c:316
invalid_op+0x14/0x20 arch/x86/entry/entry_64.S:993
RIP: 0010:apparmor_secid_to_secctx+0x2b5/0x2f0 security/apparmor/secid.c:82
Code: c7 c7 40 66 58 87 e8 6a 6d 0f fe 0f 0b e9 6c fe ff ff e8 3e aa 44 fe
48 c7 c6 80 67 58 87 48 c7 c7 a0 65 58 87 e8 4b 6d 0f fe <0f> 0b e9 3f fe
ff ff 48 89 df e8 fc a7 83 fe e9 ed fe ff ff bb f4

RSP: 0018:ffff8801ba5a6d10 EFLAGS: 00010286
RAX: 0000000000000000 RBX: ffff8801ba5a6ed0 RCX: 0000000000000000
RDX: 0000000000000000 RSI: ffffffff8163ac01 RDI: 0000000000000001
RBP: ffff8801ba5a6d30 R08: ffff8801d9ba2580 R09: ffffed003b603eca

R10: ffffed003b603eca R11: ffff8801db01f657 R12: 0000000000000001

R13: 0000000000000000 R14: 0000000000000000 R15: ffff8801ba5a6ed0

RIP: 0033:0x441189
Code: e8 cc ab 02 00 48 83 c4 18 c3 0f 1f 80 00 00 00 00 48 89 f8 48 89 f7

48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff

ff 0f 83 bb 0a fc ff c3 66 2e 0f 1f 84 00 00 00 00
RSP: 002b:00007ffc388e2108 EFLAGS: 00000217 ORIG_RAX: 000000000000002e
RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 0000000000441189
RDX: 0000000000000000 RSI: 0000000020d65000 RDI: 0000000000000004
RBP: 0000000000000000 R08: 00000000004002c8 R09: 00000000004002c8
R10: 00000000004002c8 R11: 0000000000000217 R12: 00000000004020d0
R13: 0000000000402160 R14: 0000000000000000 R15: 0000000000000000

[Stephen Smalley]

On 08/29/2018 10:21 PM, Dmitry Vyukov wrote:
> On Wed, Aug 29, 2018 at 7:17 PM, syzbot
> <syzbot+210161...@syzkaller.appspotmail.com> wrote:
>> Hello,
>>
>> syzbot found the following crash on:
>>
>> HEAD commit: 817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
>> git tree: net-next
>> console output: https://syzkaller.appspot.com/x/log.txt?x=1536d296400000
>> kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
>> dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>
>> Unfortunately, I don't have any reproducer for this crash yet.
>>
>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>> Reported-by: syzbot+210161...@syzkaller.appspotmail.com
>
> Hi John, Tyler,
>
> I've switched syzbot from selinux to apparmor as we discussed on lss:
> https://github.com/google/syzkaller/commit/2c6cb254ae6c06f61e3aba21bb89ffb05b5db946

Sorry, does this mean that you are no longer testing selinux via syzbot?
That seems unfortunate. SELinux is default-enabled and used in
Fedora, RHEL and all derivatives (e.g. CentOS), and mandatory in Android
(and seemingly getting some use in ChromeOS now as well, at least for
the Android container and possibly wider), so it seems unwise to drop it
from your testing altogether. I was under the impression that you were
just going to add apparmor to your testing matrix, not drop selinux
altogether.

[Paul Moore]

It is also important to note that testing with SELinux enabled but no
policy loaded is not going to be very helpful (last we talked that is
what syzbot is/was doing). While syzbot did uncover some issues
relating to the enabled-no-policy case, those are much less
interesting and less relevant than the loaded-policy case.

--
paul moore
www.paul-moore.com

[Stephen Smalley]

I had thought that they had switched over to at least loading a policy
but possibly left it in permissive mode because the base distribution
didn't properly support SELinux out of the box. But I may be mistaken.
Regardless, the right solution is to migrate to testing with a policy
loaded not to stop testing altogether.

Optimally, they'd test on at least one distribution/OS where SELinux is
in fact supported out of the box, e.g. CentOS, Android, and/or ChromeOS.

[Stephen Smalley]

Or Fedora, of course.

[Dmitry Vyukov]

Hi,

Yes, we switched from selinux to apparmor. The thing is that we
effectively did not test selinux lately, so it's more like just
enabling apparmor rather than disabling selinux.

The story goes as follows.
We enabled selinux, but did not have policy and selinux wasn't enabled.
Then Paul (I think) pointer that out. After some debugging I figured
out that our debian wheezy actually has a policy, it's just that
selinux utilities were not installed, so init could not load the
policy.
I installed the tools, and we started loading policy.
But then it turned out that wheezy policy does not allows mounting
cgroup2 fs and maybe some others even in non-enforcing mode. As far as
I understand that's because the policy does not have definition for
the fs, and so loading bails out with an error.
We need cgroup2 both for testing and for better sandboxing (no other
way to restrict e.g. memory consumption). Moreover, we did not test
any actual interesting interactions with selinux (there must be some?
but I don't know what are they).
So I had to uninstall the tool and policy is not loaded again.
I investigated building a newer debian image with debootstrap (which
should have newer policy I guess). But they don't work, some cryptic
errors in init. Other people reported the same.
So that's we are. I don't have any ideas left...

[John Johansen]

On 08/29/2018 07:17 PM, syzbot wrote:
> Hello,
>
> syzbot found the following crash on:
>
> HEAD commit:    817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
> git tree:       net-next
> console output: https://syzkaller.appspot.com/x/log.txt?x=1536d296400000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
> dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
> compiler:       gcc (GCC) 8.0.1 20180413 (experimental)
>
> Unfortunately, I don't have any reproducer for this crash yet.
>
> IMPORTANT: if you fix the bug, please add the following tag to the commit:
> Reported-by: syzbot+210161...@syzkaller.appspotmail.com
>

<< snip >>

Patch sent directly to syzbot for testing

[Dmitry Vyukov]

Hi John,

What do you mean? syzbot has not received any test requests for this,
and it would reply within half an hour or so. Where is that patch?

[John Johansen]

Hrmmm strange I followed the web instruction and attached the patch to the
reply. The patch is below, its also available at

git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor 4.18-syzbot-secid

---

From 22dad84baabf4174f11f5e9b34a05529084fa29c Mon Sep 17 00:00:00 2001
From: John Johansen <john.j...@canonical.com>
Date: Sat, 1 Sep 2018 01:57:52 -0700
Subject: [PATCH] apparmor: fix apparmor_secid_to_secctx incorrect debug
triggering WARN_ON

apparmor_secid_to_secctx() has a bad debug statement tripping on a
condition handle by the code. When kconfig SECURITY_APPARMOR_DEBUG is
enabled the debug WARN_ON will trip when **secdata is NULL resulting
in the following trace.

Fixes: c092921219d2 ("apparmor: add support for mapping secids and using secctxes")
Reported-by: syzbot+210161...@syzkaller.appspotmail.com
Signed-off-by: John Johansen <john.j...@canonical.com>
---
security/apparmor/secid.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/security/apparmor/secid.c b/security/apparmor/secid.c
index f2f22d00db18..4ccec1bcf6f5 100644
--- a/security/apparmor/secid.c
+++ b/security/apparmor/secid.c
@@ -79,7 +79,6 @@ int apparmor_secid_to_secctx(u32 secid, char **secdata, u32 *seclen)
struct aa_label *label = aa_secid_to_label(secid);
int len;

- AA_BUG(!secdata);
AA_BUG(!seclen);

if (!label)
--
2.17.1

[syzbot]

> On Sun, Sep 2, 2018 at 6:52 AM, John Johansen

> Humm.. Maybe you did not send it to syzbot? The command should be just:

> #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

"git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor" does not
look like a valid git repo address.

>> --
>> You received this message because you are subscribed to the Google
>> Groups "syzkaller-bugs" group.
>> To unsubscribe from this group and stop receiving emails from it, send
>> an email to syzkaller-bug...@googlegroups.com.
>> To view this discussion on the web visit
>> https://groups.google.com/d/msgid/syzkaller-bugs/09def4f1-7dd8-ba41-139a-0c6f3be2db78%40canonical.com.
>> For more options, visit https://groups.google.com/d/optout.

[Dmitry Vyukov]

On Sun, Sep 2, 2018 at 6:52 AM, John Johansen

<john.j...@canonical.com> wrote:
> On 09/01/2018 09:33 PM, Dmitry Vyukov wrote:
>> On Sat, Sep 1, 2018 at 11:18 AM, John Johansen
>> <john.j...@canonical.com> wrote:
>>> On 08/29/2018 07:17 PM, syzbot wrote:
>>>> Hello,
>>>>
>>>> syzbot found the following crash on:
>>>>
>>>> HEAD commit: 817e60a7a2bb Merge branch 'nfp-add-NFP5000-support'
>>>> git tree: net-next
>>>> console output: https://syzkaller.appspot.com/x/log.txt?x=1536d296400000
>>>> kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492
>>>> dashboard link: https://syzkaller.appspot.com/bug?extid=21016130b0580a9de3b5
>>>> compiler: gcc (GCC) 8.0.1 20180413 (experimental)
>>>>
>>>> Unfortunately, I don't have any reproducer for this crash yet.
>>>>
>>>> IMPORTANT: if you fix the bug, please add the following tag to the commit:
>>>> Reported-by: syzbot+210161...@syzkaller.appspotmail.com
>>>>
>>>
>>> << snip >>
>>>
>>> Patch sent directly to syzbot for testing
>>
>> Hi John,
>>
>> What do you mean? syzbot has not received any test requests for this,
>> and it would reply within half an hour or so. Where is that patch?
>>
>
> Hrmmm strange I followed the web instruction and attached the patch to the
> reply. The patch is below, its also available at
>
> git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor 4.18-syzbot-secid

Humm.. Maybe you did not send it to syzbot? The command should be just:

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor

[Dmitry Vyukov]

I guess the repo is:

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor.git

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+210161...@syzkaller.appspotmail.com

Tested on:

commit: 360bd62dc494 Merge tag 'linux-watchdog-4.19-rc2' of git://..
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=531a917630d2a492

compiler: gcc (GCC) 8.0.1 20180413 (experimental)

patch: https://syzkaller.appspot.com/x/patch.diff?x=10d437bc400000

Note: testing is done by a robot and is best-effort only.

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger
crash:

Reported-and-tested-by:
syzbot+210161...@syzkaller.appspotmail.com

Tested on:

commit: 22dad84baabf apparmor: fix apparmor_secid_to_secctx incorr..
git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/jj/linux-apparmor.git/4.18-syzbot-secid
kernel config: https://syzkaller.appspot.com/x/.config?x=a7c5a36688323465

compiler: gcc (GCC) 8.0.1 20180413 (experimental)

[Stephen Smalley]

So why not ask for help from the SELinux community? I've cc'd the
selinux list and a couple of folks involved in Debian selinux. I see a
couple of options but I don't know your constraints for syzbot:

1) Run an instance of syzbot on a distro that supports SELinux enabled
out of the box like Fedora. Then you don't have to fight with SELinux
and can just focus on syzbot, while still testing SELinux enabled and
enforcing.

2) Report the problems you are having with enabling SELinux on newer
Debian to the selinux list and/or the Debian selinux package maintainers
so that someone can help you resolve them.

3) Back-port the cgroup2 policy definitions to your wheezy policy,
rebuild it, and install that. We could help provide guidance on that.
I think you'll need to rebuild the base policy on wheezy; in
distributions with modern SELinux userspace, one could do it just by
adding a CIL module locally.

As for exercising SELinux, you'll exercise SELinux just by enabling it
and loading a policy, since it will perform permission checking on all
object accesses. But you can get more extensive coverage by running
the selinux-testsuite. We only test that on Fedora and RHEL however, so
getting it to work on Debian might take some effort.

[Russell Coker]

On Tuesday, 4 September 2018 10:57:15 PM AEST Stephen Smalley wrote:
> > I installed the tools, and we started loading policy.
> > But then it turned out that wheezy policy does not allows mounting
> > cgroup2 fs and maybe some others even in non-enforcing mode. As far as
> > I understand that's because the policy does not have definition for
> > the fs, and so loading bails out with an error.

The aim has always been with SE Linux in Debian that the policy will support
the kernel from the next release and from the previous release. Much of this
comes from upstream, but sometimes we have to go out of our way to get it.
Sometimes if you want to run unusual combinations of kernel and OS Debian
doesn't get a backport of the policy to support it in which case I often have
a special policy on my site to do it.

It seems strange that you wouldn't be able to mount a filesystem in permissive
mode. Which program was trying to mount it? Was it systemd? It might be a
systemd bug.

> > We need cgroup2 both for testing and for better sandboxing (no other
> > way to restrict e.g. memory consumption). Moreover, we did not test
> > any actual interesting interactions with selinux (there must be some?
> > but I don't know what are they).
> > So I had to uninstall the tool and policy is not loaded again.
> > I investigated building a newer debian image with debootstrap (which
> > should have newer policy I guess). But they don't work, some cryptic
> > errors in init. Other people reported the same.
> > So that's we are. I don't have any ideas left...

It would be nice to know what the errors are. Although we aren't really
interested in bug reports from Wheezy, Stretch is the current stable release.

> So why not ask for help from the SELinux community? I've cc'd the
> selinux list and a couple of folks involved in Debian selinux. I see a
> couple of options but I don't know your constraints for syzbot:
>
> 1) Run an instance of syzbot on a distro that supports SELinux enabled
> out of the box like Fedora. Then you don't have to fight with SELinux
> and can just focus on syzbot, while still testing SELinux enabled and
> enforcing.
>
> 2) Report the problems you are having with enabling SELinux on newer
> Debian to the selinux list and/or the Debian selinux package maintainers
> so that someone can help you resolve them.
>
> 3) Back-port the cgroup2 policy definitions to your wheezy policy,
> rebuild it, and install that. We could help provide guidance on that.
> I think you'll need to rebuild the base policy on wheezy; in
> distributions with modern SELinux userspace, one could do it just by
> adding a CIL module locally.

I could backport that myself and put the package on my apt repository. Tell
me what version of the kernel you are using and I'll have a look at it.

> As for exercising SELinux, you'll exercise SELinux just by enabling it
> and loading a policy, since it will perform permission checking on all
> object accesses. But you can get more extensive coverage by running
> the selinux-testsuite. We only test that on Fedora and RHEL however, so
> getting it to work on Debian might take some effort.

--
My Main Blog http://etbe.coker.com.au/
My Documents Blog http://doc.coker.com.au/

[Dmitry Vyukov]

On Tue, Sep 4, 2018 at 3:16 PM, Russell Coker <rus...@coker.com.au> wrote:
> On Tuesday, 4 September 2018 10:57:15 PM AEST Stephen Smalley wrote:
>> > I installed the tools, and we started loading policy.
>> > But then it turned out that wheezy policy does not allows mounting
>> > cgroup2 fs and maybe some others even in non-enforcing mode. As far as
>> > I understand that's because the policy does not have definition for
>> > the fs, and so loading bails out with an error.
>
> The aim has always been with SE Linux in Debian that the policy will support
> the kernel from the next release and from the previous release. Much of this
> comes from upstream, but sometimes we have to go out of our way to get it.
> Sometimes if you want to run unusual combinations of kernel and OS Debian
> doesn't get a backport of the policy to support it in which case I often have
> a special policy on my site to do it.
>
> It seems strange that you wouldn't be able to mount a filesystem in permissive
> mode. Which program was trying to mount it? Was it systemd? It might be a
> systemd bug.

The program was our binary, but also reproducible with just mount
utility. I don't think the program matters, it's just that mount
system call returns an error. It also looked weird to me that selinux
fails the syscall in permissive mode. Maybe it's just a bug? Fixing
that would resolve our problem, and it the easiest way to resolve it.

Here is the script we use to build images:
https://github.com/google/syzkaller/blob/master/tools/create-image.sh
If you run it in qemu and try to mount cgroup2, it should fail.

>> > We need cgroup2 both for testing and for better sandboxing (no other
>> > way to restrict e.g. memory consumption). Moreover, we did not test
>> > any actual interesting interactions with selinux (there must be some?
>> > but I don't know what are they).
>> > So I had to uninstall the tool and policy is not loaded again.
>> > I investigated building a newer debian image with debootstrap (which
>> > should have newer policy I guess). But they don't work, some cryptic
>> > errors in init. Other people reported the same.
>> > So that's we are. I don't have any ideas left...
>
> It would be nice to know what the errors are. Although we aren't really
> interested in bug reports from Wheezy, Stretch is the current stable release.

The errors were unrelated to selinux, the image just failed to bring
up network or something. So it was just unusable.
Other people reported the same:
https://groups.google.com/forum/#!searchin/syzkaller/wheezy|sort:date/syzkaller/DJdHAsTXWVw/XXr8KMfgBQAJ
https://groups.google.com/forum/#!searchin/syzkaller/wheezy|sort:date/syzkaller/bgkbvqbjnfA/tlH7SGwCDQAJ

Again, if you change wheezy to stretch here, it should reproduce the problem:
https://github.com/google/syzkaller/blob/master/tools/create-image.sh

>> So why not ask for help from the SELinux community? I've cc'd the
>> selinux list and a couple of folks involved in Debian selinux. I see a
>> couple of options but I don't know your constraints for syzbot:
>>
>> 1) Run an instance of syzbot on a distro that supports SELinux enabled
>> out of the box like Fedora. Then you don't have to fight with SELinux
>> and can just focus on syzbot, while still testing SELinux enabled and
>> enforcing.
>>
>> 2) Report the problems you are having with enabling SELinux on newer
>> Debian to the selinux list and/or the Debian selinux package maintainers
>> so that someone can help you resolve them.
>>
>> 3) Back-port the cgroup2 policy definitions to your wheezy policy,
>> rebuild it, and install that. We could help provide guidance on that.
>> I think you'll need to rebuild the base policy on wheezy; in
>> distributions with modern SELinux userspace, one could do it just by
>> adding a CIL module locally.
>
> I could backport that myself and put the package on my apt repository. Tell
> me what version of the kernel you are using and I'll have a look at it.

We are testing upstream master as well as 4.4, 4.9 and 4.14.
We need some modifications to:
https://github.com/google/syzkaller/blob/master/tools/create-image.sh
produce image with working policy.

[Dmitry Vyukov]

Thanks, Stephen!

I would like to understand first if failing mount(2) for unknown fs is
selinux bug or not. Because if it is and it is fixed, then it would
resolve the problem without actually doing anything (well, at least on
our side :)).

> As for exercising SELinux, you'll exercise SELinux just by enabling it and
> loading a policy, since it will perform permission checking on all object
> accesses. But you can get more extensive coverage by running the
> selinux-testsuite. We only test that on Fedora and RHEL however, so getting
> it to work on Debian might take some effort.

That's good.
I just thought that there is some potential in making the policy
interact more with what the fuzzer does. With respect to fs accesses,
it works within own temp directory, and I guess the policy is actually
all the same for everything it does in that directory. There also may
be something related to extended attributes, context changes, etc?

[Stephen Smalley]

Yes, I think that's a selinux kernel regression, previously reported here:
https://lkml.org/lkml/2017/10/6/658

Unfortunately I don't think it has been fixed upstream. Generally
people using SELinux with a newer kernel are also using a newer policy.
That said, I agree it is a regression and ought to be fixed.

>> As for exercising SELinux, you'll exercise SELinux just by enabling it and
>> loading a policy, since it will perform permission checking on all object
>> accesses. But you can get more extensive coverage by running the
>> selinux-testsuite. We only test that on Fedora and RHEL however, so getting
>> it to work on Debian might take some effort.
>
> That's good.
> I just thought that there is some potential in making the policy
> interact more with what the fuzzer does. With respect to fs accesses,
> it works within own temp directory, and I guess the policy is actually
> all the same for everything it does in that directory. There also may
> be something related to extended attributes, context changes, etc?

Yes, by default, your fuzzer is going to just run in a single security
context and all files it creates will have a single security context.
So the policy side of things won't be interesting and probably
everything will be allowed (if it runs in the unconfined context), but
you'll still exercise many code paths. The selinux-testsuite would
trigger many process context changes and create files under varying
contexts, so that would be more complete in its coverage.

[Dmitry Vyukov]

How hard is it to fix it? We are on upstream head, so once it's in we
are ready to go.
Using multiple images is somewhat problematic (besides the fact that I
don't know how to build a fedora image) because syzbot does not
capture what image was used, and in the docs we just provide the
single image, so people will start complaining that bugs don't
reproduce but they are just using a wrong image.

>>> As for exercising SELinux, you'll exercise SELinux just by enabling it
>>> and
>>> loading a policy, since it will perform permission checking on all object
>>> accesses. But you can get more extensive coverage by running the
>>> selinux-testsuite. We only test that on Fedora and RHEL however, so
>>> getting
>>> it to work on Debian might take some effort.
>>
>>
>> That's good.
>> I just thought that there is some potential in making the policy
>> interact more with what the fuzzer does. With respect to fs accesses,
>> it works within own temp directory, and I guess the policy is actually
>> all the same for everything it does in that directory. There also may
>> be something related to extended attributes, context changes, etc?
>
>
> Yes, by default, your fuzzer is going to just run in a single security
> context and all files it creates will have a single security context. So the
> policy side of things won't be interesting and probably everything will be
> allowed (if it runs in the unconfined context), but you'll still exercise
> many code paths. The selinux-testsuite would trigger many process context
> changes and create files under varying contexts, so that would be more
> complete in its coverage.
>

> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bug...@googlegroups.com.
> To view this discussion on the web visit

> https://groups.google.com/d/msgid/syzkaller-bugs/b2b38348-f3a4-6498-c9b8-1090532f6a23%40tycho.nsa.gov.

[Stephen Smalley]

I'll take a look and see if I can provide a trivial fix.

[Paul Moore]

As a FYI, Stephen provided a patch and it has been merged into the
selinux/next tree.

* git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git
* https://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/selinux.git

Author: Stephen Smalley <s...@tycho.nsa.gov>
Date: Tue Sep 4 16:51:36 2018 -0400

selinux: fix mounting of cgroup2 under older policies

commit 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
broke mounting of cgroup2 under older SELinux policies which lacked
a genfscon rule for cgroup2. This prevents mounting of cgroup2 even
when SELinux is permissive.

Change the handling when there is no genfscon rule in policy to
just mark the inode unlabeled and not return an error to the caller.
This permits mounting and access if allowed by policy, e.g. to
unconfined domains.

I also considered changing the behavior of security_genfs_sid() to
never return -ENOENT, but the current behavior is relied upon by
other callers to perform caller-specific handling.

Fixes: 901ef845fa2469c ("selinux: allow per-file labeling for cgroupfs")
CC: <sta...@vger.kernel.org>
Reported-by: Dmitry Vyukov <dvy...@google.com>
Reported-by: Waiman Long <lon...@redhat.com>
Signed-off-by: Stephen Smalley <s...@tycho.nsa.gov>
Tested-by: Waiman Long <lon...@redhat.com>
Signed-off-by: Paul Moore <pa...@paul-moore.com>

--
paul moore
www.paul-moore.com

[Dmitry Vyukov]

Thanks! I've re-enabled selinux on syzbot:
https://github.com/google/syzkaller/commit/196410e4f5665d4d2bf6c818d06f1c8d03cfa8cc
Now we will have instances with apparmor and with selinux.

[Kees Cook]

On Tue, Sep 4, 2018 at 7:53 AM, Dmitry Vyukov <dvy...@google.com> wrote:
> Again, if you change wheezy to stretch here, it should reproduce the problem:
> https://github.com/google/syzkaller/blob/master/tools/create-image.sh

FWIW, I sent an update for the image version here:
https://github.com/google/syzkaller/pull/708

-Kees

--
Kees Cook
Pixel Security

[Casey Schaufler]

On 9/5/2018 4:08 AM, Dmitry Vyukov wrote:
> Thanks! I've re-enabled selinux on syzbot:
> https://github.com/google/syzkaller/commit/196410e4f5665d4d2bf6c818d06f1c8d03cfa8cc
> Now we will have instances with apparmor and with selinux.

Any chance we could get a Smack instance as well?

[Dmitry Vyukov]

Hi Casey,

Sure!
Provided you want to fix bugs ;)
I've setup an instance with smack enabled:
https://github.com/google/syzkaller/commit/0bb7a7eb8e0958c6fbe2d69615b9fae4af88c8ee

[Dmitry Vyukov]

But just doing default things does not seem to find much. I guess
common paths through the hooks are well exercised already.
So perhaps if we do more non-trivial things, it can find more stuff.
But what are they? Adding/changing/removing xattr's? Which? What are
the values? Changing security contexts? How? What else?
selinux has own filesystem and we should touch some files there:
https://github.com/google/syzkaller/blob/master/sys/linux/selinux.txt
But we don't anything similar for other modules.

[Dmitry Vyukov]

First one that looks smack-specific:
https://syzkaller.appspot.com/bug?id=9eda6092f146cb23cb9109f675a2e2cb743ee48b

[Tetsuo Handa]

Dmitry, is it possible to update configs for linux-next.git , for
we want to test a big change in LSM which will go to Linux 5.1 ?

TOMOYO security module (CONFIG_SECURITY_TOMOYO=y) can now coexist with
SELinux/Smack/AppArmor security modules, and SafeSetID security module
(CONFIG_SECURITY_SAFESETID=y) was added. Testing with these modules also
enabled might find something...

[Dmitry Vyukov]

Hi,

syzbot configs/cmdline args are stored here:
https://github.com/google/syzkaller/tree/master/dashboard/config

I've tried to update to the latest kernel, the diff is below.
Few questions:
1. How are modules enabled now?
We pass security=selinux of security=smack on command line. What do we
need to pass now to enable several modules at the same time?

2. What exactly does SECURITY_SAFESETID do?
syzkaller executor uses setuid/gid, and the config says "to only those
approved by a system-wide whitelist". What/where is that whitelist? If
we just enable it, but not whitelist will it break syzkaller?



diff --git a/dashboard/config/upstream-kasan.config
b/dashboard/config/upstream-kasan.config
index 475d84a3..ed026cd8 100644
--- a/dashboard/config/upstream-kasan.config
+++ b/dashboard/config/upstream-kasan.config
@@ -1,6 +1,6 @@
#
# Automatically generated file; DO NOT EDIT.
-# Linux/x86 4.20.0 Kernel Configuration
+# Linux/x86 5.0.0-rc4 Kernel Configuration
#

# The following configs are added manually, preserve them.
@@ -14,11 +14,12 @@ CONFIG_DEBUG_MEMORY=y
CONFIG_DEBUG_AID_FOR_SYZBOT=y

#
-# Compiler: gcc (GCC) 9.0.0 20181231 (experimental)
+# Compiler:
#
CONFIG_CC_IS_GCC=y
CONFIG_GCC_VERSION=90000
CONFIG_CLANG_VERSION=0
+CONFIG_CC_HAS_ASM_GOTO=y
CONFIG_CONSTRUCTORS=y
CONFIG_IRQ_WORK=y
CONFIG_BUILDTIME_EXTABLE_SORT=y
@@ -295,7 +296,7 @@ CONFIG_X86_X2APIC=y
CONFIG_X86_MPPARSE=y
# CONFIG_GOLDFISH is not set
CONFIG_RETPOLINE=y
-# CONFIG_RESCTRL is not set
+# CONFIG_X86_RESCTRL is not set
CONFIG_X86_EXTENDED_PLATFORM=y
# CONFIG_X86_NUMACHIP is not set
# CONFIG_X86_VSMP is not set
@@ -571,6 +572,7 @@ CONFIG_X86_ACPI_CPUFREQ_CPB=y
CONFIG_CPU_IDLE=y
# CONFIG_CPU_IDLE_GOV_LADDER is not set
CONFIG_CPU_IDLE_GOV_MENU=y
+# CONFIG_CPU_IDLE_GOV_TEO is not set
CONFIG_INTEL_IDLE=y

#
@@ -745,8 +747,9 @@ CONFIG_HAVE_ARCH_PREL32_RELOCATIONS=y
#
# CONFIG_GCOV_KERNEL is not set
CONFIG_ARCH_HAS_GCOV_PROFILE_ALL=y
-CONFIG_PLUGIN_HOSTCC=""
+CONFIG_PLUGIN_HOSTCC="g++"
CONFIG_HAVE_GCC_PLUGINS=y
+# CONFIG_GCC_PLUGINS is not set
CONFIG_RT_MUTEXES=y
CONFIG_BASE_SMALL=0
CONFIG_MODULES=y
@@ -932,6 +935,7 @@ CONFIG_NET_KEY_MIGRATE=y
CONFIG_SMC=y
CONFIG_SMC_DIAG=y
CONFIG_XDP_SOCKETS=y
+CONFIG_XDP_SOCKETS_DIAG=y
CONFIG_INET=y
CONFIG_IP_MULTICAST=y
CONFIG_IP_ADVANCED_ROUTER=y
@@ -3246,6 +3250,7 @@ CONFIG_SPI_MASTER=y
CONFIG_SPI_BITBANG=y
# CONFIG_SPI_CADENCE is not set
# CONFIG_SPI_DESIGNWARE is not set
+# CONFIG_SPI_NXP_FLEXSPI is not set
CONFIG_SPI_PXA2XX=y
CONFIG_SPI_PXA2XX_PCI=y
# CONFIG_SPI_ROCKCHIP is not set
@@ -3896,6 +3901,10 @@ CONFIG_DRM_KMS_CMA_HELPER=y
# CONFIG_DRM_I2C_SIL164 is not set
# CONFIG_DRM_I2C_NXP_TDA998X is not set
# CONFIG_DRM_I2C_NXP_TDA9950 is not set
+
+#
+# ARM devices
+#
# CONFIG_DRM_RADEON is not set
# CONFIG_DRM_AMDGPU is not set

@@ -3951,6 +3960,7 @@ CONFIG_DRM_PANEL_BRIDGE=y
# Display Interface Bridges
#
# CONFIG_DRM_ANALOGIX_ANX78XX is not set
+# CONFIG_DRM_ETNAVIV is not set
# CONFIG_DRM_HISI_HIBMC is not set
CONFIG_DRM_TINYDRM=y
# CONFIG_TINYDRM_HX8357D is not set
@@ -4060,7 +4070,6 @@ CONFIG_FRAMEBUFFER_CONSOLE_DETECT_PRIMARY=y
# CONFIG_FRAMEBUFFER_CONSOLE_ROTATION is not set
# CONFIG_FRAMEBUFFER_CONSOLE_DEFERRED_TAKEOVER is not set
CONFIG_LOGO=y
-# CONFIG_FB_LOGO_CENTER is not set
# CONFIG_LOGO_LINUX_MONO is not set
# CONFIG_LOGO_LINUX_VGA16 is not set
CONFIG_LOGO_LINUX_CLUT224=y
@@ -4298,6 +4307,7 @@ CONFIG_LOGIRUMBLEPAD2_FF=y
CONFIG_LOGIG940_FF=y
CONFIG_LOGIWHEELS_FF=y
CONFIG_HID_MAGICMOUSE=y
+# CONFIG_HID_MALTRON is not set
# CONFIG_HID_MAYFLASH is not set
# CONFIG_HID_REDRAGON is not set
CONFIG_HID_MICROSOFT=y
@@ -4396,6 +4406,7 @@ CONFIG_USB_EHCI_HCD=y
CONFIG_USB_EHCI_ROOT_HUB_TT=y
CONFIG_USB_EHCI_TT_NEWSCHED=y
CONFIG_USB_EHCI_PCI=y
+# CONFIG_USB_EHCI_FSL is not set
# CONFIG_USB_EHCI_HCD_PLATFORM is not set
# CONFIG_USB_OXU210HP_HCD is not set
# CONFIG_USB_ISP116X_HCD is not set
@@ -4743,6 +4754,10 @@ CONFIG_MLX4_INFINIBAND=y
# CONFIG_INFINIBAND_OCRDMA is not set
# CONFIG_INFINIBAND_VMWARE_PVRDMA is not set
CONFIG_INFINIBAND_USNIC=y
+# CONFIG_INFINIBAND_BNXT_RE is not set
+# CONFIG_INFINIBAND_HFI1 is not set
+CONFIG_INFINIBAND_RDMAVT=y
+CONFIG_RDMA_RXE=y
CONFIG_INFINIBAND_IPOIB=y
CONFIG_INFINIBAND_IPOIB_CM=y
CONFIG_INFINIBAND_IPOIB_DEBUG=y
@@ -4750,10 +4765,6 @@ CONFIG_INFINIBAND_IPOIB_DEBUG=y
CONFIG_INFINIBAND_SRP=y
CONFIG_INFINIBAND_ISER=y
CONFIG_INFINIBAND_OPA_VNIC=y
-CONFIG_INFINIBAND_RDMAVT=y
-CONFIG_RDMA_RXE=y
-# CONFIG_INFINIBAND_HFI1 is not set
-# CONFIG_INFINIBAND_BNXT_RE is not set
CONFIG_EDAC_ATOMIC_SCRUB=y
CONFIG_EDAC_SUPPORT=y
CONFIG_EDAC=y
@@ -4820,6 +4831,7 @@ CONFIG_RTC_INTF_DEV=y
# CONFIG_RTC_DRV_RX8025 is not set
# CONFIG_RTC_DRV_EM3027 is not set
# CONFIG_RTC_DRV_RV8803 is not set
+# CONFIG_RTC_DRV_SD3078 is not set

#
# SPI RTC drivers
@@ -4978,7 +4990,6 @@ CONFIG_STAGING=y
# CONFIG_VT6655 is not set
# CONFIG_VT6656 is not set
# CONFIG_FB_SM750 is not set
-# CONFIG_FB_XGI is not set

#
# Speakup console speech
@@ -5102,6 +5113,7 @@ CONFIG_CLKBLD_I8253=y
CONFIG_MAILBOX=y
CONFIG_PCC=y
# CONFIG_ALTERA_MBOX is not set
+CONFIG_IOMMU_IOVA=y
CONFIG_IOMMU_API=y
CONFIG_IOMMU_SUPPORT=y

@@ -5110,7 +5122,6 @@ CONFIG_IOMMU_SUPPORT=y
#
# CONFIG_IOMMU_DEBUGFS is not set
# CONFIG_IOMMU_DEFAULT_PASSTHROUGH is not set
-CONFIG_IOMMU_IOVA=y
CONFIG_AMD_IOMMU=y
# CONFIG_AMD_IOMMU_V2 is not set
CONFIG_DMAR_TABLE=y
@@ -5288,7 +5299,6 @@ CONFIG_EXPORTFS_BLOCK_OPS=y
CONFIG_FILE_LOCKING=y
CONFIG_MANDATORY_FILE_LOCKING=y
CONFIG_FS_ENCRYPTION=y
-# CONFIG_FS_VERITY is not set
CONFIG_FSNOTIFY=y
CONFIG_DNOTIFY=y
CONFIG_INOTIFY_USER=y
@@ -5549,7 +5559,6 @@ CONFIG_FORTIFY_SOURCE=y
# CONFIG_STATIC_USERMODEHELPER is not set
CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SELINUX_BOOTPARAM=y
-CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE=1
CONFIG_SECURITY_SELINUX_DISABLE=y
CONFIG_SECURITY_SELINUX_DEVELOP=y
CONFIG_SECURITY_SELINUX_AVC_STATS=y
@@ -5558,9 +5567,11 @@ CONFIG_SECURITY_SMACK=y
# CONFIG_SECURITY_SMACK_BRINGUP is not set
CONFIG_SECURITY_SMACK_NETFILTER=y
# CONFIG_SECURITY_SMACK_APPEND_SIGNALS is not set
-# CONFIG_SECURITY_TOMOYO is not set
+CONFIG_SECURITY_TOMOYO=y
+CONFIG_SECURITY_TOMOYO_MAX_ACCEPT_ENTRY=2048
+CONFIG_SECURITY_TOMOYO_MAX_AUDIT_LOG=1024
+CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER=y
CONFIG_SECURITY_APPARMOR=y
-CONFIG_SECURITY_APPARMOR_BOOTPARAM_VALUE=1
CONFIG_SECURITY_APPARMOR_HASH=y
CONFIG_SECURITY_APPARMOR_HASH_DEFAULT=y
CONFIG_SECURITY_APPARMOR_DEBUG=y
@@ -5568,6 +5579,7 @@ CONFIG_SECURITY_APPARMOR_DEBUG_ASSERTS=y
# CONFIG_SECURITY_APPARMOR_DEBUG_MESSAGES is not set
# CONFIG_SECURITY_LOADPIN is not set
CONFIG_SECURITY_YAMA=y
+# CONFIG_SECURITY_SAFESETID is not set
CONFIG_INTEGRITY=y
CONFIG_INTEGRITY_SIGNATURE=y
CONFIG_INTEGRITY_ASYMMETRIC_KEYS=y
@@ -5598,11 +5610,7 @@ CONFIG_EVM_ATTR_FSUUID=y
CONFIG_EVM_EXTRA_SMACK_XATTRS=y
CONFIG_EVM_ADD_XATTRS=y
# CONFIG_EVM_LOAD_X509 is not set
-# CONFIG_DEFAULT_SECURITY_SELINUX is not set
-# CONFIG_DEFAULT_SECURITY_SMACK is not set
-CONFIG_DEFAULT_SECURITY_APPARMOR=y
-# CONFIG_DEFAULT_SECURITY_DAC is not set
-CONFIG_DEFAULT_SECURITY="apparmor"
+CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
CONFIG_XOR_BLOCKS=y
CONFIG_ASYNC_CORE=y
CONFIG_ASYNC_MEMCPY=y

[Micah Morton]

There is a description of this recently-added LSM in
Documentation/admin-guide/LSM/SafeSetID.rst (has been pulled into
next-general branch in the linux-security tree, should be in
linux-next tree by now as well). TL;DR: if no whitelist is configured
by writing to the safesetid directory in securityfs, there will be no
policies preventing certain users from changing to other users/groups.
So shouldn't break syzkaller if you "just" enable it.

[Tetsuo Handa]

On 2019/01/30 23:45, Dmitry Vyukov wrote:
>> Dmitry, is it possible to update configs for linux-next.git , for
>> we want to test a big change in LSM which will go to Linux 5.1 ?
>>
>> TOMOYO security module (CONFIG_SECURITY_TOMOYO=y) can now coexist with
>> SELinux/Smack/AppArmor security modules, and SafeSetID security module
>> (CONFIG_SECURITY_SAFESETID=y) was added. Testing with these modules also
>> enabled might find something...
>
> Hi,
>
> syzbot configs/cmdline args are stored here:
> https://github.com/google/syzkaller/tree/master/dashboard/config
>
> I've tried to update to the latest kernel, the diff is below.
> Few questions:
> 1. How are modules enabled now?
> We pass security=selinux of security=smack on command line. What do we
> need to pass now to enable several modules at the same time?

Removing security= parameter from kernel boot command line will do it.

security/apparmor/lsm.c: .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
security/selinux/hooks.c: .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
security/smack/smack_lsm.c: .flags = LSM_FLAG_LEGACY_MAJOR | LSM_FLAG_EXCLUSIVE,
security/tomoyo/tomoyo.c: .flags = LSM_FLAG_LEGACY_MAJOR,
security/security.c: if ((major->flags & LSM_FLAG_LEGACY_MAJOR) &&

But this means that, if same kernel config/cmdline are used between
linux-next.git and linux.git (etc.), syzbot will need to choose from

(a) stop sharing kernel cmdline between linux-next.git and linux.git (etc.)

or

(b) stop sharing kernel config between SELinux, Smack and AppArmor

or

(c) start testing after the LSM changes went to linux.git as Linux 5.1-rc1

. Is (a) or (b) possible? If this is a too much change, (c) will be OK.

[Dmitry Vyukov]

Thanks for the explanations.

Here is the change that I've come up with:
https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a

I've disabled CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER (it
actually looked like omitting a user-space loader that I don't have is
the right thing to do, but okay, it indeed does not with =y).

For now I just enabled TOMOYO and SAFESETID.
I see the problem with making both linux-next and upstream work. If we
use a single config and lsm= cmdline argument, then on upstream all
kernels will use the same module (they won't understand lsm=). But if
we add security= then it will take precedence over lsm= on linux-next,
so we won't get stacked modules.

Let's go with (c) because I don't want an additional long-term maintenance cost.
If I understand it correctly later we will need to replace:
security=selinux
security=smack
security=apparmor

with:
lsm=yama,safesetid,integrity,selinux,tomoyo
lsm=yama,safesetid,integrity,smack,tomoyo
lsm=yama,safesetid,integrity,tomoyo,apparmor

[Dmitry Vyukov]

Filed https://github.com/google/syzkaller/issues/973 to not forget about it.

[Tetsuo Handa]

On 2019/02/01 19:09, Dmitry Vyukov wrote:
> Thanks for the explanations.
>
> Here is the change that I've come up with:
> https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a

You are not going to apply this updated config to upstream kernels now, are you?
Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels
will cause failing to enable AppArmor (unless security=apparmor is specified).

I guess you can apply this updated config to linux-next kernels given that
you replace

CONFIG_LSM="yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"

with

CONFIG_LSM="yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo"

so that AppArmor is enabled instead of SELinux.

>
> I've disabled CONFIG_SECURITY_TOMOYO_OMIT_USERSPACE_LOADER (it
> actually looked like omitting a user-space loader that I don't have is
> the right thing to do, but okay, it indeed does not with =y).
>
> For now I just enabled TOMOYO and SAFESETID.
> I see the problem with making both linux-next and upstream work. If we
> use a single config and lsm= cmdline argument, then on upstream all
> kernels will use the same module (they won't understand lsm=). But if
> we add security= then it will take precedence over lsm= on linux-next,
> so we won't get stacked modules.

Right.

>
> Let's go with (c) because I don't want an additional long-term maintenance cost.

OK.

> If I understand it correctly later we will need to replace:
> security=selinux
> security=smack
> security=apparmor
>
> with:
> lsm=yama,safesetid,integrity,selinux,tomoyo
> lsm=yama,safesetid,integrity,smack,tomoyo
> lsm=yama,safesetid,integrity,tomoyo,apparmor

Yes. Thank you.

[Dmitry Vyukov]

On Fri, Feb 1, 2019 at 11:44 AM Tetsuo Handa
<penguin...@i-love.sakura.ne.jp> wrote:
>
> On 2019/02/01 19:09, Dmitry Vyukov wrote:
> > Thanks for the explanations.
> >
> > Here is the change that I've come up with:
> > https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a
>
> You are not going to apply this updated config to upstream kernels now, are you?
> Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels
> will cause failing to enable AppArmor (unless security=apparmor is specified).

We do use security=apparmor, see:
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-apparmor.cmdline
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-selinux.cmdline
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline

[Tetsuo Handa]

On 2019/02/01 19:50, Dmitry Vyukov wrote:
> On Fri, Feb 1, 2019 at 11:44 AM Tetsuo Handa
> <penguin...@i-love.sakura.ne.jp> wrote:
>>
>> On 2019/02/01 19:09, Dmitry Vyukov wrote:
>>> Thanks for the explanations.
>>>
>>> Here is the change that I've come up with:
>>> https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a
>>
>> You are not going to apply this updated config to upstream kernels now, are you?
>> Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels
>> will cause failing to enable AppArmor (unless security=apparmor is specified).
>
>
> We do use security=apparmor, see:
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-apparmor.cmdline
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-selinux.cmdline
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline
>

Oh, security= parameter is explicitly specified on all targets?
Then, we can abuse CONFIG_DEBUG_AID_FOR_SYZBOT option. ;-)

LSM folks, may we use this patch for linux-next.git ?
CONFIG_DEBUG_AID_FOR_SYZBOT is a linux-next.git-only kernel config option used by syzbot.



From c7d21f9c1c0b610ddea4233b89edf7d3140b8baf Mon Sep 17 00:00:00 2001
From: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
Date: Fri, 1 Feb 2019 22:03:55 +0900
Subject: [PATCH linux-next] LSM: Allow syzbot to ignore security= parameter.

LSM is going to get infrastructure managed security blob support in Linux
5.1, and it becomes possible to run TOMOYO with SELinux/Smack/AppArmor.
But for compatibility reason, since security= parameter makes it
impossible to run TOMOYO with SELinux/Smack/AppArmor, syzbot can't
test that combination. Therefore, this patch allows syzbot to temporarily
ignore security= parameter. This patch is meant for linux-next.git only,
and will be removed after infrastructure managed security blob support
went to linux.git.

Signed-off-by: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
---
security/security.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/security/security.c b/security/security.c
index ef03643..0632feb 100644
--- a/security/security.c
+++ b/security/security.c
@@ -346,12 +346,14 @@ int __init security_init(void)
}

/* Save user chosen LSM */
+#ifndef CONFIG_DEBUG_AID_FOR_SYZBOT
static int __init choose_major_lsm(char *str)
{
chosen_major_lsm = str;
return 1;
}
__setup("security=", choose_major_lsm);
+#endif

/* Explicitly choose LSM initialization order. */
static int __init choose_lsm_order(char *str)
--
1.8.3.1

[Dmitry Vyukov]

On Fri, Feb 1, 2019 at 2:09 PM Tetsuo Handa

<penguin...@i-love.sakura.ne.jp> wrote:
>
> On 2019/02/01 19:50, Dmitry Vyukov wrote:
> > On Fri, Feb 1, 2019 at 11:44 AM Tetsuo Handa
> > <penguin...@i-love.sakura.ne.jp> wrote:
> >>
> >> On 2019/02/01 19:09, Dmitry Vyukov wrote:
> >>> Thanks for the explanations.
> >>>
> >>> Here is the change that I've come up with:
> >>> https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a
> >>
> >> You are not going to apply this updated config to upstream kernels now, are you?
> >> Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels
> >> will cause failing to enable AppArmor (unless security=apparmor is specified).
> >
> >
> > We do use security=apparmor, see:
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-apparmor.cmdline
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-selinux.cmdline
> > https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline
> >
>
> Oh, security= parameter is explicitly specified on all targets?
> Then, we can abuse CONFIG_DEBUG_AID_FOR_SYZBOT option. ;-)
>
> LSM folks, may we use this patch for linux-next.git ?
> CONFIG_DEBUG_AID_FOR_SYZBOT is a linux-next.git-only kernel config option used by syzbot.

Then we also need this on syzbot side, right? Otherwise it seems that
all instances will default to a single security module.
https://github.com/google/syzkaller/commit/ffec3d1894ffd05966b50efa49ca19af76c9ea81

[Tetsuo Handa]

On 2019/02/04 17:07, Dmitry Vyukov wrote:
> On Fri, Feb 1, 2019 at 2:09 PM Tetsuo Handa
> <penguin...@i-love.sakura.ne.jp> wrote:
>>
>> On 2019/02/01 19:50, Dmitry Vyukov wrote:
>>> On Fri, Feb 1, 2019 at 11:44 AM Tetsuo Handa
>>> <penguin...@i-love.sakura.ne.jp> wrote:
>>>>
>>>> On 2019/02/01 19:09, Dmitry Vyukov wrote:
>>>>> Thanks for the explanations.
>>>>>
>>>>> Here is the change that I've come up with:
>>>>> https://github.com/google/syzkaller/commit/aa53be276dc84aa8b3825b3416542447ff82b41a
>>>>
>>>> You are not going to apply this updated config to upstream kernels now, are you?
>>>> Removing CONFIG_DEFAULT_SECURITY="apparmor" from configs used by upstream kernels
>>>> will cause failing to enable AppArmor (unless security=apparmor is specified).
>>>
>>>
>>> We do use security=apparmor, see:
>>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-apparmor.cmdline
>>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-selinux.cmdline
>>> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-smack.cmdline
>>>
>>
>> Oh, security= parameter is explicitly specified on all targets?
>> Then, we can abuse CONFIG_DEBUG_AID_FOR_SYZBOT option. ;-)
>>
>> LSM folks, may we use this patch for linux-next.git ?
>> CONFIG_DEBUG_AID_FOR_SYZBOT is a linux-next.git-only kernel config option used by syzbot.
>
>
> Then we also need this on syzbot side, right? Otherwise it seems that
> all instances will default to a single security module.
> https://github.com/google/syzkaller/commit/ffec3d1894ffd05966b50efa49ca19af76c9ea81
>

Right.

But as I update the documentation ( https://tomoyo.osdn.jp/2.6/chapter-3.html.en#3.6 ),
I came to think that we should ignore security= parameter when lsm= parameter is specified.

Currently, it is possible to enable TOMOYO and only one of SELinux/Smack/AppArmor. Therefore,
it is possible to disable only TOMOYO by specifying security=selinux when we want to enable
only SELinux, by specifying security=smack when we want to enable only Smack, by specifying
security=apparmor when we want to enable only AppArmor. That is, we can use security= parameter
in order to specify the other LSM module which should not be disabled.

But when it becomes possible to enable TOMOYO and more than one of SELinux/Smack/AppArmor,
we will no longer be able to selectively disable one LSM module using security= parameter, for
security= parameter is intended for specifying only one LSM module which should be enabled.
That is, we will need to use lsm= parameter in order to selectively disable LSM modules.

Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
when lsm= parameter is specified. Furthermore, we could even avoid introducing lsm= parameter
by allowing security= parameter to specify multiple LSM modules. For example, security= parameter
is interpreted as a list of all LSM modules which should be enabled when it contains a comma,
and it is interpreted as one of LSM_FLAG_LEGACY_MAJOR modules which should be enabled otherwise.
Then, specifying security=selinux or security=smack or security=tomoyo or security=apparmor or
security=none will respectively enable SELinux, Smack, TOMOYO, AppArmor, none of
SELinux/Smack/TOMOYO/AppArmor. And specifying e.g. security=, will disable all LSM modules.

[Casey Schaufler]

Yes. That is correct. The existing behavior of security= is maintained.
The new behavior of lsm= is provided to allow general handling of a list
of security modules. It uses the same form of data as CONFIG_LSM.

> Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
> when lsm= parameter is specified.

That reduces flexibility somewhat. If I am debugging security modules
I may want to use lsm= to specify the order while using security= to
identify a specific exclusive module. I could do that using lsm= by
itself, but habits die hard.

> Furthermore, we could even avoid introducing lsm= parameter
> by allowing security= parameter to specify multiple LSM modules.

security=yama would work differently than it does today.
There would be no way to specify an exclusive module but
no minor modules.

If I have Yama and SELinux built in I could never disable Yama.

security=selinux - would not disable Yama

whereas

lsm=selinux - would disable Yama

> For example, security= parameter
> is interpreted as a list of all LSM modules which should be enabled when it contains a comma,
> and it is interpreted as one of LSM_FLAG_LEGACY_MAJOR modules which should be enabled otherwise.
> Then, specifying security=selinux or security=smack or security=tomoyo or security=apparmor or
> security=none will respectively enable SELinux, Smack, TOMOYO, AppArmor, none of
> SELinux/Smack/TOMOYO/AppArmor. And specifying e.g. security=, will disable all LSM modules.

We debated the possibility of making the comma an indication
that we had an explicit list. It comes down to the "trailing
comma" syntax, where "security=selinux" and "security=selinux,"
mean different things. Consensus was that this is too clever,
and everyone would hate it.

[Tetsuo Handa]

Casey Schaufler wrote:
> On 2/6/2019 2:23 AM, Tetsuo Handa wrote:
> > But as I update the documentation ( https://tomoyo.osdn.jp/2.6/chapter-3.html.en#3.6 ),
> > I came to think that we should ignore security= parameter when lsm= parameter is specified.
> >
> > Currently, it is possible to enable TOMOYO and only one of SELinux/Smack/AppArmor. Therefore,
> > it is possible to disable only TOMOYO by specifying security=selinux when we want to enable
> > only SELinux, by specifying security=smack when we want to enable only Smack, by specifying
> > security=apparmor when we want to enable only AppArmor. That is, we can use security= parameter
> > in order to specify the other LSM module which should not be disabled.
> >
> > But when it becomes possible to enable TOMOYO and more than one of SELinux/Smack/AppArmor,
> > we will no longer be able to selectively disable one LSM module using security= parameter, for
> > security= parameter is intended for specifying only one LSM module which should be enabled.
> > That is, we will need to use lsm= parameter in order to selectively disable LSM modules.
>
> Yes. That is correct. The existing behavior of security= is maintained.

But the existing behavior of CONFIG_DEFAULT_SECURITY is not maintained.
This might cause a problem like

commit e5a3b95f581da62e2054ef79d3be2d383e9ed664
Author: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
Date: Sat Feb 14 11:46:56 2009 +0900

TOMOYO: Don't create securityfs entries unless registered.

TOMOYO should not create /sys/kernel/security/tomoyo/ interface unless
TOMOYO is registered.

for Ubuntu users because Ubuntu kernels are built with

CONFIG_SECURITY_SELINUX=y
CONFIG_SECURITY_SMACK=y
CONFIG_SECURITY_TOMOYO=y
CONFIG_SECURITY_APPARMOR=y
CONFIG_SECURITY_YAMA=y
CONFIG_DEFAULT_SECURITY="apparmor"

. Due to CONFIG_DEFAULT_SECURITY="apparmor", majority of Ubuntu users are enabling
only AppArmor without explicitly specifying "security=apparmor".

Currently default CONFIG_LSM setting is

"yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"

but Ubuntu kernels would have to be built with non-default CONFIG_LSM setting like

"yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo"

in order to make sure that AppArmor is by default chosen for the LSM_FLAG_EXCLUSIVE module.

Now that TOMOYO becomes a !LSM_FLAG_EXCLUSIVE module, not specifying "security=apparmor" will
automatically enable TOMOYO. And majority of Ubuntu users will unexpectedly encounter TOMOYO
messages. But removing "tomoyo" from CONFIG_LSM setting in order to save majority of Ubuntu
users from unexpectedly encountering TOMOYO messages also has a problem; Ubuntu users who want
to enable only TOMOYO from LSM_FLAG_LEGACY_MAJOR modules can specify "security=tomoyo", but
Ubuntu users who want to enable TOMOYO and one of SELinux,Smack,AppArmor (including syzbot)
will have to explicitly specify "lsm=" because "security=" can't allow enabling multiple
LSM_FLAG_LEGACY_MAJOR modules.

> The new behavior of lsm= is provided to allow general handling of a list
> of security modules. It uses the same form of data as CONFIG_LSM.
>
> > Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
> > when lsm= parameter is specified.
>
> That reduces flexibility somewhat. If I am debugging security modules
> I may want to use lsm= to specify the order while using security= to
> identify a specific exclusive module. I could do that using lsm= by
> itself, but habits die hard.

"lsm=" can be used for identifying a specific exclusive module, and Ubuntu kernels would
have to use CONFIG_LSM (or "lsm=") for identifying the default exclusive module (in order
to allow enabling both TOMOYO and one of SELinux,Smack,AppArmor at the same time).

Since "security=" can't be used for selectively enable/disable more than one of
SELinux,Smack,TOMOYO,AppArmor, I think that recommending users to migrate to "lsm=" is the
better direction. And ignoring "security=" when "lsm=" is specified is easier to understand.

[Casey Schaufler]

On 2/6/2019 6:30 PM, Tetsuo Handa wrote:
> Casey Schaufler wrote:
>> On 2/6/2019 2:23 AM, Tetsuo Handa wrote:
>>> But as I update the documentation ( https://tomoyo.osdn.jp/2.6/chapter-3.html.en#3.6 ),
>>> I came to think that we should ignore security= parameter when lsm= parameter is specified.
>>>
>>> Currently, it is possible to enable TOMOYO and only one of SELinux/Smack/AppArmor. Therefore,
>>> it is possible to disable only TOMOYO by specifying security=selinux when we want to enable
>>> only SELinux, by specifying security=smack when we want to enable only Smack, by specifying
>>> security=apparmor when we want to enable only AppArmor. That is, we can use security= parameter
>>> in order to specify the other LSM module which should not be disabled.
>>>
>>> But when it becomes possible to enable TOMOYO and more than one of SELinux/Smack/AppArmor,
>>> we will no longer be able to selectively disable one LSM module using security= parameter, for
>>> security= parameter is intended for specifying only one LSM module which should be enabled.
>>> That is, we will need to use lsm= parameter in order to selectively disable LSM modules.
>> Yes. That is correct. The existing behavior of security= is maintained.
> But the existing behavior of CONFIG_DEFAULT_SECURITY is not maintained.

That's a developer interface, not a user interface. I realize
that may be splitting hairs, but it had to change.

> This might cause a problem like
>
> commit e5a3b95f581da62e2054ef79d3be2d383e9ed664
> Author: Tetsuo Handa <penguin...@I-love.SAKURA.ne.jp>
> Date: Sat Feb 14 11:46:56 2009 +0900
>
> TOMOYO: Don't create securityfs entries unless registered.
>
> TOMOYO should not create /sys/kernel/security/tomoyo/ interface unless
> TOMOYO is registered.
>
> for Ubuntu users because Ubuntu kernels are built with
>
> CONFIG_SECURITY_SELINUX=y
> CONFIG_SECURITY_SMACK=y
> CONFIG_SECURITY_TOMOYO=y
> CONFIG_SECURITY_APPARMOR=y
> CONFIG_SECURITY_YAMA=y
> CONFIG_DEFAULT_SECURITY="apparmor"
>
> . Due to CONFIG_DEFAULT_SECURITY="apparmor", majority of Ubuntu users are enabling
> only AppArmor without explicitly specifying "security=apparmor".
>
> Currently default CONFIG_LSM setting is
>
> "yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor"
>
> but Ubuntu kernels would have to be built with non-default CONFIG_LSM setting like
>
> "yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo"
>
> in order to make sure that AppArmor is by default chosen for the LSM_FLAG_EXCLUSIVE module.

Yes, and Yocto Project is likely to want Smack specified first.

> Now that TOMOYO becomes a !LSM_FLAG_EXCLUSIVE module, not specifying "security=apparmor" will
> automatically enable TOMOYO. And majority of Ubuntu users will unexpectedly encounter TOMOYO
> messages. But removing "tomoyo" from CONFIG_LSM setting in order to save majority of Ubuntu
> users from unexpectedly encountering TOMOYO messages also has a problem; Ubuntu users who want
> to enable only TOMOYO from LSM_FLAG_LEGACY_MAJOR modules can specify "security=tomoyo", but
> Ubuntu users who want to enable TOMOYO and one of SELinux,Smack,AppArmor (including syzbot)
> will have to explicitly specify "lsm=" because "security=" can't allow enabling multiple
> LSM_FLAG_LEGACY_MAJOR modules.

I believe we got general buy in from Ubuntu, and I understand
that the LSM list is awkward, but I don't see a rational alternate.
I know that I played with a half dozen, and nothing was closer to
maintaining the status quo.

>> The new behavior of lsm= is provided to allow general handling of a list
>> of security modules. It uses the same form of data as CONFIG_LSM.
>>
>>> Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
>>> when lsm= parameter is specified.
>> That reduces flexibility somewhat. If I am debugging security modules
>> I may want to use lsm= to specify the order while using security= to
>> identify a specific exclusive module. I could do that using lsm= by
>> itself, but habits die hard.
> "lsm=" can be used for identifying a specific exclusive module, and Ubuntu kernels would
> have to use CONFIG_LSM (or "lsm=") for identifying the default exclusive module (in order
> to allow enabling both TOMOYO and one of SELinux,Smack,AppArmor at the same time).
>
> Since "security=" can't be used for selectively enable/disable more than one of
> SELinux,Smack,TOMOYO,AppArmor, I think that recommending users to migrate to "lsm=" is the
> better direction. And ignoring "security=" when "lsm=" is specified is easier to understand.

I added Kees to the CC list. Kees, what to you think about
ignoring security= if lsm= is specified? I'm ambivalent.

[Tetsuo Handa]

On 2019/02/08 1:24, Casey Schaufler wrote:
>>>> Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
>>>> when lsm= parameter is specified.
>>> That reduces flexibility somewhat. If I am debugging security modules
>>> I may want to use lsm= to specify the order while using security= to
>>> identify a specific exclusive module. I could do that using lsm= by
>>> itself, but habits die hard.
>> "lsm=" can be used for identifying a specific exclusive module, and Ubuntu kernels would
>> have to use CONFIG_LSM (or "lsm=") for identifying the default exclusive module (in order
>> to allow enabling both TOMOYO and one of SELinux,Smack,AppArmor at the same time).
>>
>> Since "security=" can't be used for selectively enable/disable more than one of
>> SELinux,Smack,TOMOYO,AppArmor, I think that recommending users to migrate to "lsm=" is the
>> better direction. And ignoring "security=" when "lsm=" is specified is easier to understand.
>
> I added Kees to the CC list. Kees, what to you think about
> ignoring security= if lsm= is specified? I'm ambivalent.
>
>

To help administrators easily understand what LSM modules are possibly enabled by default (which
have to be fetched from e.g. /boot/config-`uname -r`) and specify lsm= parameter when they need,
I propose changes shown below.

diff --git a/security/security.c b/security/security.c
index 3147785e..051d708 100644
--- a/security/security.c
+++ b/security/security.c
@@ -51,8 +51,6 @@
static __initdata const char *chosen_lsm_order;
static __initdata const char *chosen_major_lsm;

-static __initconst const char * const builtin_lsm_order = CONFIG_LSM;
-
/* Ordered list of LSMs to initialize. */
static __initdata struct lsm_info **ordered_lsms;
static __initdata struct lsm_info *exclusive;
@@ -284,14 +282,22 @@ static void __init ordered_lsm_parse(const char *order, const char *origin)
static void __init ordered_lsm_init(void)
{
struct lsm_info **lsm;
+ const char *order = CONFIG_LSM;
+ const char *origin = "builtin";

ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
GFP_KERNEL);

- if (chosen_lsm_order)
- ordered_lsm_parse(chosen_lsm_order, "cmdline");
- else
- ordered_lsm_parse(builtin_lsm_order, "builtin");
+ if (chosen_lsm_order) {
+ if (chosen_major_lsm) {
+ pr_info("security= is ignored because of lsm=\n");
+ chosen_major_lsm = NULL;
+ }
+ order = chosen_lsm_order;
+ origin = "cmdline";
+ }
+ pr_info("Security Framework initializing: %s\n", order);
+ ordered_lsm_parse(order, origin);

for (lsm = ordered_lsms; *lsm; lsm++)
prepare_lsm(*lsm);
@@ -333,8 +339,6 @@ int __init security_init(void)
int i;
struct hlist_head *list = (struct hlist_head *) &security_hook_heads;

- pr_info("Security Framework initializing\n");
-
for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
i++)
INIT_HLIST_HEAD(&list[i]);

[Casey Schaufler]

On 2/8/2019 2:52 AM, Tetsuo Handa wrote:
> On 2019/02/08 1:24, Casey Schaufler wrote:
>>>>> Then, I think that it is straightforward (and easier to manage) to ignore security= parameter
>>>>> when lsm= parameter is specified.
>>>> That reduces flexibility somewhat. If I am debugging security modules
>>>> I may want to use lsm= to specify the order while using security= to
>>>> identify a specific exclusive module. I could do that using lsm= by
>>>> itself, but habits die hard.
>>> "lsm=" can be used for identifying a specific exclusive module, and Ubuntu kernels would
>>> have to use CONFIG_LSM (or "lsm=") for identifying the default exclusive module (in order
>>> to allow enabling both TOMOYO and one of SELinux,Smack,AppArmor at the same time).
>>>
>>> Since "security=" can't be used for selectively enable/disable more than one of
>>> SELinux,Smack,TOMOYO,AppArmor, I think that recommending users to migrate to "lsm=" is the
>>> better direction. And ignoring "security=" when "lsm=" is specified is easier to understand.
>> I added Kees to the CC list. Kees, what to you think about
>> ignoring security= if lsm= is specified? I'm ambivalent.
>>
>>
> To help administrators easily understand what LSM modules are possibly enabled by default (which
> have to be fetched from e.g. /boot/config-`uname -r`)

$ cat /sys/kernel/security/lsm

I'm not going to object to this, but I don't see it as important.

[Kees Cook]

On Thu, Feb 7, 2019 at 8:24 AM Casey Schaufler <ca...@schaufler-ca.com> wrote:
> I added Kees to the CC list. Kees, what to you think about
> ignoring security= if lsm= is specified? I'm ambivalent.

This was one of many earlier suggestions, and the consensus seemed to
be "don't mix security= and lsm=". Why would anyone use both?

--
Kees Cook

[Kees Cook]

This is intended to be the new default way to change the LSM
("lsm=..."), so I'd rather not have this appear every time. Also, it
must continue to interact with the builtin ordering, so if you wanted
this, I think better would be to do:

diff --git a/security/security.c b/security/security.c
index 3147785e20d7..e6153ed54361 100644
--- a/security/security.c
+++ b/security/security.c
@@ -288,9 +288,13 @@ static void __init ordered_lsm_init(void)

ordered_lsms = kcalloc(LSM_COUNT + 1, sizeof(*ordered_lsms),
GFP_KERNEL);

- if (chosen_lsm_order)

+ if (chosen_lsm_order) {
+ if (chosen_major_lsm) {
+ pr_info("security= is ignored because of lsm=\n");
+ chosen_major_lsm = NULL;
+ }

ordered_lsm_parse(chosen_lsm_order, "cmdline");
- else

+ } else
ordered_lsm_parse(builtin_lsm_order, "builtin");

for (lsm = ordered_lsms; *lsm; lsm++)

> + pr_info("Security Framework initializing: %s\n", order);
> + ordered_lsm_parse(order, origin);
>
> for (lsm = ordered_lsms; *lsm; lsm++)
> prepare_lsm(*lsm);
> @@ -333,8 +339,6 @@ int __init security_init(void)
> int i;
> struct hlist_head *list = (struct hlist_head *) &security_hook_heads;
>
> - pr_info("Security Framework initializing\n");
> -
> for (i = 0; i < sizeof(security_hook_heads) / sizeof(struct hlist_head);
> i++)
> INIT_HLIST_HEAD(&list[i]);

--
Kees Cook

[Tetsuo Handa]

On 2019/02/09 1:23, Casey Schaufler wrote:
> On 2/8/2019 2:52 AM, Tetsuo Handa wrote:
>> To help administrators easily understand what LSM modules are possibly enabled by default (which
>> have to be fetched from e.g. /boot/config-`uname -r`)
>
> $ cat /sys/kernel/security/lsm
>

/sys/kernel/security/lsm is list of "actually" enabled modules, isn't it?
What I want is "possibly" enabled modules. Ubuntu would chose from either

(a) explicitly add security=apparmor to kernel command line

or

(b) explicitly remove tomoyo from CONFIG_LSM at kernel config

in order not to enable TOMOYO for those who want to enable only one of
SELinux/Smack/AppArmor. And for those who want to enable TOMOYO, I think
that (b) (in other words, add

lsm="modules listed in CONFIG_LSM" + ",tomoyo"

) will retain compatibility when it becomes possible to enable more than
one of SELinux/Smack/AppArmor at the same time.

If we can know "possibly" enabled modules from dmesg, users don't need to
look at e.g. /boot/config-`uname -r`. It is not essential, but it's handy.

[Tetsuo Handa]

Well, thinking again, specifying

lsm="modules listed in /sys/kernel/security/lsm" + ",tomoyo"

makes sense, for there is no need to care about disabled modules when
enabling TOMOYO. Therefore,

+ pr_info("Security Framework initializing: %s\n", order);

- pr_info("Security Framework initializing\n");

won't be needed.

On 2019/02/09 6:33, Kees Cook wrote:
> On Thu, Feb 7, 2019 at 8:24 AM Casey Schaufler <ca...@schaufler-ca.com> wrote:

>> I added Kees to the CC list. Kees, what to you think about
>> ignoring security= if lsm= is specified? I'm ambivalent.
>

> This was one of many earlier suggestions, and the consensus seemed to
> be "don't mix security= and lsm=". Why would anyone use both?
>

Then, can we add this change?