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@xxxxxxxxxxxx> > > 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? -- Kees Cook
