TOMOYO's pull request for v6.12

[John Johansen]

On 10/4/24 17:17, Kees Cook wrote:
> On Sat, Oct 05, 2024 at 08:41:06AM +0900, Tetsuo Handa wrote:
>> On 2024/10/05 5:54, Kees Cook wrote:
>>> - tomoyo_register_hooks() becomes an exploitation gadget that could be
>>>    used to bypass the LSM as a whole.
>>
>> tomoyo_register_hooks() is enabled only if "CONFIG_SECURITY_TOMOYO_LKM is
>> included while building the kernel" && "security=tomoyo is specified or
>> tomoyo is included in the lsm= kernel command line options".
>>
>> Therefore, those who are building kernels with CONFIG_SECURITY_TOMOYO_LKM=n are
>> not affected.
> 
> Sure, but my point is that convincing RedHat that this is acceptable is
> likely to be an uphill battle considering their effort to gain full
> ro_after_init coverage for SELinux.
> 

I concur. From a disto pov modules can be worse than builtin. The size difference
here really isn't worth the hassle of building tomoyo as a module.

>> Even if kernels are built with CONFIG_SECURITY_TOMOYO_LKM=y, callbacks
>> registered by tomoyo_register_hooks() won't be called unless "security=tomoyo
>> is specified or tomoyo is included in the lsm= kernel command line options", for
>> the proxy callbacks that use static call tables are not registered.
> 
> This part I overlooked. I forgot that Tomoyo is still not fully stackable,
> so it isn't getting included in CONFIG_LSM= for the distros that do
> build it.
> 
>> Even if kernels are built with CONFIG_SECURITY_TOMOYO_LKM=y, and "security=tomoyo
>> is specified or tomoyo is included in the lsm= kernel command line options",
>> tomoyo_register_hooks() can be called only once.
> 
> An attacker with a read/write primitive would be able to locate and
> write to "registered" (since it is not read-only), allowing them to call
> tomoyo_register_hooks() multiple times.
> 
>> And tomoyo.ko is loaded by the
>> time /sbin/init (nowadays /usr/lib/systemd/systemd) starts. That is, by the time
>> an attacker can login from console or can start attacking via network,
>> tomoyo_register_hooks() is no longer callable.
> 
> See above -- calling tomoyo_register_hooks() after boot is entirely
> feasible given a read/write attack primitive.
> 
>> Therefore, the only problem with tomoyo.ko approach is that the static call tables
>> for tomoyo_register_hooks() are currently not marked as __ro_after_init. But it will
>> be possible to make the static call tables read-only if the static call tables
> 
> The tables actually don't matter as much -- an attacker could construct
> their own table anywhere in kernel memory and pass that as an argument
> for their call to tomoyo_register_hooks().
> 
> (This is actually one of the reasons I have pushed to have sensitive
> functions like that be able to check that their passed-in argument is
> contained in a read-only area, but that didn't get much traction[1].)
> 
yep, its a real shame

>> are aligned in a page boundary and an architecture-dependent kernel API that changes
>> given page's attribute to read-only is called. (This is why __ro_after_init can work,
>> isn't it?)
> 
> The __ro_after_init section is immediately neighboring the .rodata
> section, so when .rodata is marked read-only (after __init has
> finished), the kernel marks both sections read-only. (Except for, I
> think, s390, which does two passes: .rodata is read-only before __init,
> and then __ro_after_init is marked read-only after __init.)
> 
>> As a whole, I don't think tomoyo.ko approach is unacceptably dangerous.
> 
> I agree, this implementation is safer than I initial assessed (due to the
> LSM's view of the hooks being skipped due to lsm= not including tomoyo).
> I still think how this patch ended up in Linus's tree was a big mistake,
> though.
> 
> Regardless, my opinion is unchanged: I think it will be harder to convince
> RedHat to build in _this_ version of tomoyo compared to the stock version.
> 
> (Out of curiosity, does RedHat build in AppArmor?)
> 

No they don't