Re: [PATCH v4 09/17] LSM: Introduce kernel_post_load_data() hook

From: Mimi Zohar <zohar@linux.ibm.com>
To: Kees Cook <keescook@chromium.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Scott Branden <scott.branden@broadcom.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Takashi Iwai <tiwai@suse.de>, Jessica Yu <jeyu@kernel.org>,
	SeongJae Park <sjpark@amazon.de>, KP Singh <kpsingh@chromium.org>,
	linux-efi@vger.kernel.org, linux-security-module@vger.kernel.org,
	linux-integrity@vger.kernel.org, selinux@vger.kernel.org,
	linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH v4 09/17] LSM: Introduce kernel_post_load_data() hook
Date: Thu, 06 Aug 2020 17:59:46 -0400	[thread overview]
Message-ID: <cd7e4fdabf563792d31bbcb8f6df6483570e7221.camel@linux.ibm.com> (raw)
In-Reply-To: <20200729175845.1745471-10-keescook@chromium.org>

On Wed, 2020-07-29 at 10:58 -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>

Thanks, Kees.   Other than a missing "name" field, it looks good.

The security_kernel_post_load_data hook may be used to verify appended
signatures and to measure the buffer data.  Passing the kernel module
(load_info.name) and firmware (fw_name) names is critical at least for
IMA-measurement.

thanks,

Mimi