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 10/17] firmware_loader: Use security_post_load_data()
Date: Thu, 06 Aug 2020 18:07:35 -0400 [thread overview]
Message-ID: <6be709297b60260faf9fb8b741799d130a663d52.camel@linux.ibm.com> (raw)
In-Reply-To: <20200729175845.1745471-11-keescook@chromium.org>
On Wed, 2020-07-29 at 10:58 -0700, Kees Cook wrote:
> Now that security_post_load_data() is wired up, use it instead
> of the NULL file argument style of security_post_read_file(),
> and update the security_kernel_load_data() call to indicate that a
> security_kernel_post_load_data() call is expected.
>
> Wire up the IMA check to match earlier logic. Perhaps a generalized
> change to ima_post_load_data() might look something like this:
>
> return process_buffer_measurement(buf, size,
> kernel_load_data_id_str(load_id),
> read_idmap[load_id] ?: FILE_CHECK,
> 0, NULL);
>
> Signed-off-by: Kees Cook <keescook@chromium.org>
Other than one change and one question below, it looks good.
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
<snip>
> diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> index 85000dc8595c..1a7bc4c7437d 100644
> --- a/security/integrity/ima/ima_main.c
> +++ b/security/integrity/ima/ima_main.c
> @@ -706,7 +697,7 @@ int ima_load_data(enum kernel_load_data_id id, bool contents)
> }
> break;
> case LOADING_FIRMWARE:
> - if (ima_enforce && (ima_appraise & IMA_APPRAISE_FIRMWARE)) {
> + if (ima_enforce && (ima_appraise & IMA_APPRAISE_FIRMWARE) && !contents) {
> pr_err("Prevent firmware sysfs fallback loading.\n");
Appended signatures are limited to kernel modules and, more recently,
to the kexec kernel image, not firmware. Without a file descriptor,
file signatures stored as an xattrs are not applicable either. We
might as well fail earlier, rather than later. Adding "!contents" is
unnecessary.
> return -EACCES; /* INTEGRITY_UNKNOWN */
> }
> @@ -739,6 +730,15 @@ int ima_load_data(enum kernel_load_data_id id, bool contents)
> */
> int ima_post_load_data(char *buf, loff_t size, enum kernel_load_data_id load_id)
> {
> + if (load_id == LOADING_FIRMWARE) {
> + if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> + (ima_appraise & IMA_APPRAISE_ENFORCE)) {
> + pr_err("Prevent firmware loading_store.\n");
> + return -EACCES; /* INTEGRITY_UNKNOWN */
> + }
> + return 0;
> + }
Even with failing LOADING_FIRMWARE early in ima_load_data(), is this
still needed for fw_sysfs_loading()?
thanks,
Mimi
> +
> return 0;
> }
>
next prev parent reply other threads:[~2020-08-06 22:08 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-29 17:58 [PATCH v4 00/17] Introduce partial kernel_read_file() support Kees Cook
2020-07-29 17:58 ` [PATCH v4 01/17] test_firmware: Test platform fw loading on non-EFI systems Kees Cook
2020-07-29 17:58 ` [PATCH v4 02/17] fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum Kees Cook
2020-07-29 17:58 ` [PATCH v4 03/17] fs/kernel_read_file: Remove FIRMWARE_EFI_EMBEDDED enum Kees Cook
2020-07-29 17:58 ` [PATCH v4 04/17] fs/kernel_read_file: Split into separate include file Kees Cook
2020-07-30 2:22 ` James Morris
2020-07-29 17:58 ` [PATCH v4 05/17] fs/kernel_read_file: Split into separate source file Kees Cook
2020-07-29 17:58 ` [PATCH v4 06/17] fs/kernel_read_file: Remove redundant size argument Kees Cook
2020-07-30 2:25 ` James Morris
2020-07-29 17:58 ` [PATCH v4 07/17] fs/kernel_read_file: Switch buffer size arg to size_t Kees Cook
2020-07-30 2:25 ` James Morris
2020-07-29 17:58 ` [PATCH v4 08/17] fs/kernel_read_file: Add file_size output argument Kees Cook
2020-07-30 2:26 ` James Morris
2020-07-29 17:58 ` [PATCH v4 09/17] LSM: Introduce kernel_post_load_data() hook Kees Cook
2020-08-06 21:59 ` Mimi Zohar
2020-08-07 0:21 ` KP Singh
2020-07-29 17:58 ` [PATCH v4 10/17] firmware_loader: Use security_post_load_data() Kees Cook
2020-08-06 22:07 ` Mimi Zohar [this message]
2020-07-29 17:58 ` [PATCH v4 11/17] module: Call security_kernel_post_load_data() Kees Cook
2020-08-05 14:53 ` Jessica Yu
2020-08-07 0:22 ` KP Singh
2020-07-29 17:58 ` [PATCH v4 12/17] LSM: Add "contents" flag to kernel_read_file hook Kees Cook
2020-08-07 0:23 ` Mimi Zohar
2020-07-29 17:58 ` [PATCH v4 13/17] IMA: Add support for file reads without contents Kees Cook
2020-07-29 17:58 ` [PATCH v4 14/17] fs/kernel_file_read: Add "offset" arg for partial reads Kees Cook
2020-07-29 17:58 ` [PATCH v4 15/17] firmware: Store opt_flags in fw_priv Kees Cook
2020-07-29 17:58 ` [PATCH v4 16/17] firmware: Add request_partial_firmware_into_buf() Kees Cook
2020-07-29 17:58 ` [PATCH v4 17/17] test_firmware: Test partial read support Kees Cook
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=6be709297b60260faf9fb8b741799d130a663d52.camel@linux.ibm.com \
--to=zohar@linux.ibm.com \
--cc=gregkh@linuxfoundation.org \
--cc=jeyu@kernel.org \
--cc=keescook@chromium.org \
--cc=kpsingh@chromium.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-security-module@vger.kernel.org \
--cc=mcgrof@kernel.org \
--cc=scott.branden@broadcom.com \
--cc=selinux@vger.kernel.org \
--cc=sjpark@amazon.de \
--cc=tiwai@suse.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).