Re: TOMOYO's pull request for v6.12

[Kees Cook <kees@kernel.org>]

On Wed, Oct 02, 2024 at 04:12:50PM -0400, Paul Moore wrote:
> As I wrote in my original response to this pull request, this is not
> something I would accept in a new LSM submission and thus I feel
> compelled to speak out against this change and submit a revert to
> Linus.

The patch should be reverted regardless of the LSM specifics:

- The patch is (extremely) non-trivial and:

  - was never posted to any mailing list, which means there was no
    opportunity for discussion

  - did not appear in linux-next, which means there was no opportunity
    for any integration or similar testing

- The patch provides new symbol exports without any discussion nor buy-in
  from the symbol owners. (I do appreciate that it was written to be
  extremely narrowly scoped, but it doesn't matter: it's new exports.)

Given the social history of the code in question, I think the patch
should be reverted due to its _intent_. The above items do not look like
accidents nor oversights, but rather an explicit attempt to bypass the
normal consensus-based development practices of Linux itself.

It's also a red flag that the code was written 6 days before the merge
window -- even if there HAD been discussion, that is an extremely narrow
time frame to propose it for a pull request.

Speaking to the implementation, it's a clever use of static calls, but:

- it internalizes features that were designed to be commonly implemented
  and NOT used internally by an LSM.

- tomoyo_register_hooks() becomes an exploitation gadget that could be
  used to bypass the LSM as a whole.

The first issue is more of a code pattern and best practices issue. The
latter is the core problem I personally have with the implementation:
LSMs are one of the first targets during memory corruption attacks,
so we've tried to reduce their attack surface very aggressively. That
generally translates into making any function pointers (or associated
tables, manipulation routines, etc) read-only.

That this patch would result in TOMOYO getting enabled on RedHat seems
extraordinarily unlikely as RH went out of its way to remove from upstream
the very last part of SELinux that might provide this kind of read-only
bypass: CONFIG_SECURITY_WRITABLE_HOOKS. See f22f9aaf6c3d ("selinux:
remove the runtime disable functionality").

So, no, I think this endangers the other LSMs, has not been discussed
with any other parties, and has likely no path forward in RedHat.

It may be possible to change RH's mind about enabling TOMOYO, though, as
it was denied[1] almost 13 years ago and MUCH has changed since then. A
good place to start would be a new bug report, which has happened[2],
but has yet to be triaged and was only opened 2 months ago. I would
recommend reaching out to the RH kernel team directly...

-Kees

[1] https://bugzilla.redhat.com/show_bug.cgi?id=542986#c8
[2] https://bugzilla.redhat.com/show_bug.cgi?id=2303689

-- 
Kees Cook