mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: Vivek Goyal <vgoyal@redhat.com>
Cc: "Kasatkin, Dmitry" <dmitry.kasatkin@intel.com>,
	linux-security-module@vger.kernel.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 2/2] ima: Support appraise_type=imasig_optional
Date: Thu, 14 Feb 2013 14:49:16 -0500	[thread overview]
Message-ID: <1360871356.3524.667.camel@falcor1.watson.ibm.com> (raw)
In-Reply-To: <20130214161719.GC16671@redhat.com>

On Thu, 2013-02-14 at 11:17 -0500, Vivek Goyal wrote:
> On Thu, Feb 14, 2013 at 10:35:59AM -0500, Mimi Zohar wrote:
> > On Thu, 2013-02-14 at 10:23 -0500, Vivek Goyal wrote:
> > > On Thu, Feb 14, 2013 at 07:57:16AM -0500, Mimi Zohar wrote:
> > > 
> > > [..]
> > > > > Ok, I will cleanup the code to do above. Just wanted to clear up one
> > > > > point. 
> > > > > 
> > > > > Above option will not have any effect on evm behavior? This only impacts
> > > > > IMA appraisal behavior. For example, if security.ima is not present it
> > > > > is fine and file access is allowed. But if EVM is enabled and initialized
> > > > > and EVM does not find security.evm label (INTEGRITY_NOLABEL) or returns
> > > > > INTEGRITY_NOXATTRS, file access should still be denied?
> > > > 
> > > > Can't happen.  evm_verifyxattr() is called from
> > > > ima_appraise_measurement(), only if 'security.ima' exists.
> > > 
> > > Actually what I meant is following.
> > > 
> > > Currently in process_measurement(), I will allow access if
> > > ima_appraise_measurement() returns INTEGRITY_NOLABEL.
> > 
> > I think you're making this more complicated than it needs to be.  Allow
> > the execution unless the file failed signature verification.  The
> > additional capability is given only if the signature verification
> > succeeds.
> 
> I am just trying to bring it inline with module signature verification.
> There also module loading fails if signatures are present but kernel
> can't verify it.

A specific hook is defined for kernel module signature verification,
which is enabled/disabled in Kconfig.  When enabled, only signed modules
are loaded.  The kernel module hook does not verify the integrity of the
userspace application (eg. insmod, modprobe), but of the kernel module
being loaded.

Your original patches verified the integrity of the userspace
application kexec, not the image being loaded.  ima_bprm_check()
verifies the integrity of executables.  To permit both signed and
unsigned files to execute, we defined the 'optional' IMA policy flag,
with the intention of giving more capability to signed executables.

Unless we define a kexec specific hook for verifying kernel images, it's
not the same.

> Following behavior is strange.
> 
>                 rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
>                                              xattr_value->digest, rc - 1,
>                                              iint->ima_xattr.digest,
>                                              IMA_DIGEST_SIZE);
>                 if (rc == -EOPNOTSUPP) {
>                         status = INTEGRITY_UNKNOWN;
>                 } else if (rc) {
>                         cause = "invalid-signature";
>                         status = INTEGRITY_FAIL;
>                 } else {
>                         status = INTEGRITY_PASS;
>                 }
> 
> signature verification can fail for so many reasons.
> 
> - EINVAL
> - keyring is not present
> - key is not present -ENOKEY
> - ENOTSUPP
> - ENOMEM
> ..
> ..
> 
> And in all these cases we return INTEGRITY_FAIL. But only in case of
> -EOPNOTSUPP we return INTEGRITY_UNKNOWN. So why this discrepancy.

This exception is for filesystems that don't support extended
attributes, such as CPIO.  We assumed, correctly/incorrectly, that the
initramfs would already be measured and appraised.

> So to me it makes sense to return INTEGRITY_FAIL if rc == -EOPNOTSUPP.

Perhaps, but unless the initramfs supports xattrs, we wouldn't be able
to boot.

> This will bring it inline with other error codes.  And then in
> process_measurement() I can allow access in every case except
> INTEGRITY_FAIL. 

thanks,

Mimi


  parent reply	other threads:[~2013-02-14 20:25 UTC|newest]

Thread overview: 47+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-02-11 20:11 [RFC PATCH 0/2] ima: Support a mode to appraise signed files only Vivek Goyal
2013-02-11 20:11 ` [PATCH 1/2] ima: Do not try to fix hash if file system does not support security xattr Vivek Goyal
2013-02-12 11:45   ` Mimi Zohar
2013-02-12 14:27     ` Vivek Goyal
2013-02-11 20:11 ` [PATCH 2/2] ima: Support appraise_type=imasig_optional Vivek Goyal
2013-02-11 22:10   ` Mimi Zohar
2013-02-12 14:26     ` Vivek Goyal
2013-02-12 17:14       ` Mimi Zohar
2013-02-12 18:52         ` Vivek Goyal
2013-02-12 18:57           ` Vivek Goyal
2013-02-13 12:14             ` Kasatkin, Dmitry
2013-02-13 13:29               ` Vivek Goyal
2013-02-13 13:36                 ` Kasatkin, Dmitry
2013-02-13 13:49                   ` Vivek Goyal
2013-02-13 14:03                   ` Mimi Zohar
2013-02-13 14:38                   ` Vivek Goyal
2013-02-13 15:26                     ` Kasatkin, Dmitry
2013-02-13 15:29                       ` Kasatkin, Dmitry
2013-02-13 15:39                         ` Vivek Goyal
2013-02-13 15:30                       ` Vivek Goyal
2013-02-13 22:27                         ` Mimi Zohar
2013-02-14 15:03                           ` Vivek Goyal
2013-02-14 15:30                             ` Mimi Zohar
2013-02-18 18:21                               ` Vivek Goyal
2013-02-19 21:54                                 ` Mimi Zohar
2013-02-13 15:51                     ` Mimi Zohar
2013-02-12 20:05           ` Mimi Zohar
2013-02-13 12:31   ` Kasatkin, Dmitry
2013-02-13 12:56     ` Mimi Zohar
2013-02-13 13:13       ` Kasatkin, Dmitry
2013-02-13 13:44         ` Mimi Zohar
2013-02-13 16:59           ` Vivek Goyal
2013-02-14 12:57             ` Mimi Zohar
2013-02-14 15:23               ` Vivek Goyal
2013-02-14 15:35                 ` Mimi Zohar
2013-02-14 16:17                   ` Vivek Goyal
2013-02-14 16:31                     ` Vivek Goyal
2013-02-14 19:49                     ` Mimi Zohar [this message]
2013-02-14 20:54                       ` Vivek Goyal
2013-02-14 20:57                         ` Vivek Goyal
2013-02-14 21:54                           ` Mimi Zohar
2013-02-13 17:33           ` Kasatkin, Dmitry
2013-02-13 17:51             ` Vivek Goyal
2013-02-13 18:20               ` Kasatkin, Dmitry
2013-02-13 21:45             ` Mimi Zohar
2013-02-14 14:40               ` Vivek Goyal
2013-02-14 15:48                 ` 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=1360871356.3524.667.camel@falcor1.watson.ibm.com \
    --to=zohar@linux.vnet.ibm.com \
    --cc=dmitry.kasatkin@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=vgoyal@redhat.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®