All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.ibm.com>
To: Tyler Hicks <tyhicks@linux.microsoft.com>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
	Prakhar Srivastava <prsriva02@gmail.com>,
	linux-kernel@vger.kernel.org, linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: Re: [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule()
Date: Mon, 06 Jul 2020 23:18:45 -0400	[thread overview]
Message-ID: <1594091925.23056.36.camel@linux.ibm.com> (raw)
In-Reply-To: <20200706131845.GI4694@sequoia>

On Mon, 2020-07-06 at 08:18 -0500, Tyler Hicks wrote:
> On 2020-07-03 10:15:32, Mimi Zohar wrote:
> > On Thu, 2020-07-02 at 17:16 -0500, Tyler Hicks wrote:
> > > On 2020-06-30 19:07:29, Mimi Zohar wrote:
> > > > On Fri, 2020-06-26 at 17:38 -0500, Tyler Hicks wrote:
> > > > > Use ima_validate_rule() to ensure that the combination of a hook
> > > > > function and the keyrings conditional is valid and that the keyrings
> > > > > conditional is not specified without an explicit KEY_CHECK func
> > > > > conditional. This is a code cleanup and has no user-facing change.
> > > > > 
> > > > > Signed-off-by: Tyler Hicks <tyhicks@linux.microsoft.com>
> > > > > ---
> > > > > 
> > > > > * v2
> > > > >   - Allowed IMA_DIGSIG_REQUIRED, IMA_PERMIT_DIRECTIO,
> > > > >     IMA_MODSIG_ALLOWED, and IMA_CHECK_BLACKLIST conditionals to be
> > > > >     present in the rule entry flags for non-buffer hook functions.
> > > > > 
> > > > >  security/integrity/ima/ima_policy.c | 13 +++++++++++--
> > > > >  1 file changed, 11 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> > > > > index 8cdca2399d59..43d49ad958fb 100644
> > > > > --- a/security/integrity/ima/ima_policy.c
> > > > > +++ b/security/integrity/ima/ima_policy.c
> > > > > @@ -1000,6 +1000,15 @@ static bool ima_validate_rule(struct ima_rule_entry *entry)
> > > > >  		case KEXEC_KERNEL_CHECK:
> > > > >  		case KEXEC_INITRAMFS_CHECK:
> > > > >  		case POLICY_CHECK:
> > > > > +			if (entry->flags & ~(IMA_FUNC | IMA_MASK | IMA_FSMAGIC |
> > > > > +					     IMA_UID | IMA_FOWNER | IMA_FSUUID |
> > > > > +					     IMA_INMASK | IMA_EUID | IMA_PCR |
> > > > > +					     IMA_FSNAME | IMA_DIGSIG_REQUIRED |
> > > > > +					     IMA_PERMIT_DIRECTIO |
> > > > > +					     IMA_MODSIG_ALLOWED |
> > > > > +					     IMA_CHECK_BLACKLIST))
> > > > 
> > > > Other than KEYRINGS, this patch should continue to behave the same.
> > > >  However, this list gives the impressions that all of these flags are
> > > > permitted on all of the above flags, which isn't true.
> > > > 
> > > > For example, both IMA_MODSIG_ALLOWED & IMA_CHECK_BLACKLIST are limited
> > > > to appended signatures, meaning KERNEL_CHECK and KEXEC_KERNEL_CHECK.
> > > 
> > > Just to clarify, are both IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST
> > > limited to KEXEC_KERNEL_CHECK, KEXEC_INITRAMFS_CHECK, and MODULE_CHECK?
> > > That's what ima_hook_supports_modsig() suggests.
> > 
> > Theoretically that is true, but I have no idea how you would append a
> > signature to the kexec boot command line.  The only users of appended
> > signatures are currently kernel modules and the kexec'ed kernel image.
> 
> The discrepancy was with KEXEC_INITRAMFS_CHECK, not KEXEC_CMDLINE. I now
> see that there's no support for initramfs signature verification in the
> kexec code so I'll assume that ima_hook_supports_modsig() is wrong and
> limit IMA_MODSIG_ALLOWED and IMA_CHECK_BLACKLIST to the
> KEXEC_KERNEL_CHECK and MODULE_CHECK actions, as you originally
> suggested.

My mistake.  Yes, both the kexec kernel image and the initramfs read
the respective file into memory and can be signed either with an
imasig or modsig.  Refer to kernel_read_file_from_fd().

Mimi

  reply	other threads:[~2020-07-07  3:19 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-26 22:38 [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Tyler Hicks
2020-06-26 22:38 ` Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 01/11] ima: Have the LSM free its audit rule Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 02/11] ima: Free the entire rule when deleting a list of rules Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 03/11] ima: Free the entire rule if it fails to parse Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 04/11] ima: Fail rule parsing when buffer hook functions have an invalid action Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 05/11] ima: Fail rule parsing when the KEXEC_CMDLINE hook is combined with an invalid cond Tyler Hicks
2020-06-27 23:40   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 06/11] ima: Fail rule parsing when the KEY_CHECK " Tyler Hicks
2020-06-27 23:39   ` Lakshmi Ramasubramanian
2020-06-26 22:38 ` [PATCH v2 07/11] ima: Shallow copy the args_p member of ima_rule_entry.lsm elements Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 08/11] ima: Use correct type for " Tyler Hicks
2020-06-26 22:38 ` [PATCH v2 09/11] ima: Move validation of the keyrings conditional into ima_validate_rule() Tyler Hicks
2020-06-27 23:49   ` Lakshmi Ramasubramanian
2020-06-29 14:16     ` Tyler Hicks
2020-06-30 23:07   ` Mimi Zohar
2020-07-02 22:16     ` Tyler Hicks
2020-07-03 14:15       ` Mimi Zohar
2020-07-06 13:18         ` Tyler Hicks
2020-07-07  3:18           ` Mimi Zohar [this message]
2020-06-26 22:38 ` [PATCH v2 10/11] ima: Use the common function to detect LSM conditionals in a rule Tyler Hicks
2020-06-26 22:39 ` [PATCH v2 11/11] ima: Support additional conditionals in the KEXEC_CMDLINE hook function Tyler Hicks
2020-06-26 22:39   ` Tyler Hicks
2020-06-28  0:03   ` Lakshmi Ramasubramanian
2020-06-28  0:03     ` Lakshmi Ramasubramanian
2020-07-01  8:04   ` Dave Young
2020-07-01  8:04     ` Dave Young
2020-07-01 14:38     ` Tyler Hicks
2020-07-01 14:38       ` Tyler Hicks
2020-07-01  0:29 ` [PATCH v2 00/11] ima: Fix rule parsing bugs and extend KEXEC_CMDLINE rule support Mimi Zohar
2020-07-01  0:29   ` Mimi Zohar

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=1594091925.23056.36.camel@linux.ibm.com \
    --to=zohar@linux.ibm.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=jmorris@namei.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=nramas@linux.microsoft.com \
    --cc=prsriva02@gmail.com \
    --cc=serge@hallyn.com \
    --cc=tyhicks@linux.microsoft.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.