On Mon, Jul 27, 2020 at 06:49:11AM -0400, Mimi Zohar wrote:
On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote:
There are a few places in the kernel where LSMs would like to have visibility into the contents of a kernel buffer that has been loaded or read. While security_kernel_post_read_file() (which includes the buffer) exists as a pairing for security_kernel_read_file(), no such hook exists to pair with security_kernel_load_data().
Earlier proposals for just using security_kernel_post_read_file() with a NULL file argument were rejected (i.e. "file" should always be valid for the security_..._file hooks, but it appears at least one case was left in the kernel during earlier refactoring. (This will be fixed in a subsequent patch.)
Since not all cases of security_kernel_load_data() can have a single contiguous buffer made available to the LSM hook (e.g. kexec image segments are separately loaded), there needs to be a way for the LSM to reason about its expectations of the hook coverage. In order to handle this, add a "contents" argument to the "kernel_load_data" hook that indicates if the newly added "kernel_post_load_data" hook will be called with the full contents once loaded. That way, LSMs requiring full contents can choose to unilaterally reject "kernel_load_data" with contents=false (which is effectively the existing hook coverage), but when contents=true they can allow it and later evaluate the "kernel_post_load_data" hook once the buffer is loaded.
With this change, LSMs can gain coverage over non-file-backed data loads (e.g. init_module(2) and firmware userspace helper), which will happen in subsequent patches.
Additionally prepare IMA to start processing these cases.
Signed-off-by: Kees Cook keescook@chromium.org
At least from an IMA perspective, the original security_kernel_load_data() hook was defined in order to prevent certain syscalls - init_module, kexec_load - and loading firmware via sysfs. The resulting error messages were generic. Unlike security_kernel_load_data(), security_kernel_post_load_data() is meant to be used, but without a file desciptor specific information, like the filename associated with the buffer, is missing. Having the filename isn't actually necessary for verifying the appended signature, but it is needed for auditing signature verification failures and including in the IMA measurement list.
Right -- I'm open to ideas on this, but as it stands, other LSMs (e.g. BPF LSM) can benefit from the security_kernel_post_load_data() to examine the contents, etc.
Is there anything that needs to change in this patch?