From: Kees Cook <keescook@chromium.org>
To: Mimi Zohar <zohar@kernel.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Scott Branden <scott.branden@broadcom.com>,
Luis Chamberlain <mcgrof@kernel.org>,
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 v3 15/19] IMA: Add support for file reads without contents
Date: Tue, 28 Jul 2020 12:44:50 -0700 [thread overview]
Message-ID: <202007281244.2F2681AE9@keescook> (raw)
In-Reply-To: <1595856214.4841.86.camel@kernel.org>
On Mon, Jul 27, 2020 at 09:23:34AM -0400, Mimi Zohar wrote:
> On Fri, 2020-07-24 at 14:36 -0700, Kees Cook wrote:
> > From: Scott Branden <scott.branden@broadcom.com>
> >
> > When the kernel_read_file LSM hook is called with contents=false, IMA
> > can appraise the file directly, without requiring a filled buffer. When
> > such a buffer is available, though, IMA can continue to use it instead
> > of forcing a double read here.
> >
> > Signed-off-by: Scott Branden <scott.branden@broadcom.com>
> > Link: https://lore.kernel.org/lkml/20200706232309.12010-10-scott.branden@broadcom.com/
> > Signed-off-by: Kees Cook <keescook@chromium.org>
>
> After adjusting the comment below.
>
> Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
Sure!
Greg, shall I send a v4 with added Reviews and the comment change or is
that minor enough that you're able to do it?
Thanks for the reviews Mimi!
-Kees
>
> > ---
> > security/integrity/ima/ima_main.c | 22 ++++++++++++++++------
> > 1 file changed, 16 insertions(+), 6 deletions(-)
> >
> > diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
> > index dc4f90660aa6..459e50526a12 100644
> > --- a/security/integrity/ima/ima_main.c
> > +++ b/security/integrity/ima/ima_main.c
> > @@ -613,11 +613,8 @@ void ima_post_path_mknod(struct dentry *dentry)
> > int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
> > bool contents)
> > {
> > - /* Reject all partial reads during appraisal. */
> > - if (!contents) {
> > - if (ima_appraise & IMA_APPRAISE_ENFORCE)
> > - return -EACCES;
> > - }
> > + enum ima_hooks func;
> > + u32 secid;
> >
> > /*
> > * Do devices using pre-allocated memory run the risk of the
> > @@ -626,7 +623,20 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id,
> > * buffers? It may be desirable to include the buffer address
> > * in this API and walk all the dma_map_single() mappings to check.
> > */
> > - return 0;
> > +
> > + /*
> > + * There will be a call made to ima_post_read_file() with
> > + * a filled buffer, so we don't need to perform an extra
> > + * read early here.
> > + */
> > + if (contents)
> > + return 0;
> > +
> > + /* Read entire file for all partial reads during appraisal. */
>
> In addition to verifying the file signature, the file might be
> included in the IMA measurement list or the file hash may be used to
> augment the audit record. Please remove "during appraisal" from the
> comment.
>
> > + func = read_idmap[read_id] ?: FILE_CHECK;
> > + security_task_getsecid(current, &secid);
> > + return process_measurement(file, current_cred(), secid, NULL,
> > + 0, MAY_READ, func);
> > }
> >
> > const int read_idmap[READING_MAX_ID] = {
>
--
Kees Cook
next prev parent reply other threads:[~2020-07-28 19:45 UTC|newest]
Thread overview: 51+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-24 21:36 [PATCH v3 00/19] Introduce partial kernel_read_file() support Kees Cook
2020-07-24 21:36 ` [PATCH v3 01/19] test_firmware: Test platform fw loading on non-EFI systems Kees Cook
2020-07-24 21:36 ` [PATCH v3 02/19] selftest/firmware: Add selftest timeout in settings Kees Cook
2020-07-24 21:36 ` [PATCH v3 03/19] firmware_loader: EFI firmware loader must handle pre-allocated buffer Kees Cook
2020-07-25 10:07 ` Greg Kroah-Hartman
2020-07-25 15:50 ` Kees Cook
2020-07-25 17:20 ` Greg Kroah-Hartman
2020-07-24 21:36 ` [PATCH v3 04/19] fs/kernel_read_file: Remove FIRMWARE_PREALLOC_BUFFER enum Kees Cook
2020-07-27 13:35 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 05/19] fs/kernel_read_file: Remove FIRMWARE_EFI_EMBEDDED enum Kees Cook
2020-07-24 21:36 ` [PATCH v3 06/19] fs/kernel_read_file: Split into separate include file Kees Cook
2020-07-27 14:41 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 07/19] fs/kernel_read_file: Split into separate source file Kees Cook
2020-07-27 14:53 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 08/19] fs/kernel_read_file: Remove redundant size argument Kees Cook
2020-07-27 16:29 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 09/19] fs/kernel_read_file: Switch buffer size arg to size_t Kees Cook
2020-07-27 16:29 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 10/19] fs/kernel_read_file: Add file_size output argument Kees Cook
2020-07-27 16:29 ` Mimi Zohar
2020-07-24 21:36 ` [PATCH v3 11/19] LSM: Introduce kernel_post_load_data() hook Kees Cook
2020-07-27 10:49 ` Mimi Zohar
2020-07-28 19:41 ` Kees Cook
2020-07-24 21:36 ` [PATCH v3 12/19] firmware_loader: Use security_post_load_data() Kees Cook
2020-07-27 10:57 ` Mimi Zohar
2020-07-28 19:43 ` Kees Cook
2020-07-29 16:29 ` Mimi Zohar
2020-07-29 18:10 ` Mimi Zohar
2020-07-29 19:13 ` Kees Cook
2020-07-24 21:36 ` [PATCH v3 13/19] module: Call security_kernel_post_load_data() Kees Cook
2020-07-24 21:36 ` [PATCH v3 14/19] LSM: Add "contents" flag to kernel_read_file hook Kees Cook
2020-07-24 21:36 ` [PATCH v3 15/19] IMA: Add support for file reads without contents Kees Cook
2020-07-27 13:23 ` Mimi Zohar
2020-07-28 19:44 ` Kees Cook [this message]
2020-07-28 19:56 ` Greg Kroah-Hartman
2020-07-28 20:12 ` Kees Cook
2020-07-24 21:36 ` [PATCH v3 16/19] fs/kernel_file_read: Add "offset" arg for partial reads Kees Cook
2020-07-24 21:36 ` [PATCH v3 17/19] firmware: Store opt_flags in fw_priv Kees Cook
2020-07-24 21:36 ` [PATCH v3 18/19] firmware: Add request_partial_firmware_into_buf() Kees Cook
2020-07-29 1:17 ` Luis Chamberlain
2020-07-29 6:22 ` Takashi Iwai
2020-07-29 17:43 ` Kees Cook
2020-07-24 21:36 ` [PATCH v3 19/19] test_firmware: Test partial read support Kees Cook
2020-07-25 5:14 ` [PATCH v3 00/19] Introduce partial kernel_read_file() support Scott Branden
2020-07-25 10:05 ` Greg Kroah-Hartman
2020-07-25 15:48 ` Kees Cook
2020-07-27 11:16 ` Mimi Zohar
2020-07-27 19:18 ` Scott Branden
2020-07-28 18:48 ` Mimi Zohar
2020-07-28 19:56 ` Scott Branden
2020-07-29 1:19 ` Luis Chamberlain
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=202007281244.2F2681AE9@keescook \
--to=keescook@chromium.org \
--cc=gregkh@linuxfoundation.org \
--cc=jeyu@kernel.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=zohar@kernel.org \
/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).