All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mimi Zohar <zohar@linux.vnet.ibm.com>
To: Linus Torvalds <torvalds@linux-foundation.org>,
	James Morris <jmorris@namei.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	LSM List <linux-security-module@vger.kernel.org>
Subject: Re: [GIT PULL] Security subsystem updates for 4.14
Date: Sun, 10 Sep 2017 02:46:24 -0400	[thread overview]
Message-ID: <1505025984.3224.35.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFw8wgA+jhBOhnY-TSdbPgiYcrFiipCV=rsS1=GQEN+JgQ@mail.gmail.com>

On Thu, 2017-09-07 at 11:19 -0700, Linus Torvalds wrote:
> On Mon, Sep 4, 2017 at 3:29 AM, James Morris <jmorris@namei.org> wrote:
> >
> > IMA:
> >   - A new integrity_read file operation method, avoids races when
> >     calculating file hashes
> 
> Honestly, this seems really odd.
> 
> It documents that it needs to be called with i_rwsem held exclusively,
> and even has a lockdep assert to that effect (well, not really: the
> code claims "exclusive", but the lockdep assert does not), but I'm not
> actually seeing anybody doing it.
> 
> Quite the reverse, I just see integrity_read_file() doing filp_open()
> on the pathname and passing it to integrity_kernel_read() with no
> locking.
> 
> It really looks like just pure garbage to me. I pulled, and I'm not
> unpulling the whole thing. I don't think it's been tested, and I don't
> think it can be right.
> 
> Tell me why I'm wrong, or tell me why that garbage made it in in the
> first place?

I'm really sorry for the long delay in responding.  I've been on
vacation the last week, mostly without cell phone and very limited
wifi access. 

True, there is a side case where integrity_read_file() is being called
without first taking the i_rwsem.  This side case permits signed x509
certificates to be loaded onto the trusted IMA/EVM keyrings, without
verifying the file signature stored as security.ima/security.evm
xattrs.  Basically, the xattr signatures can not be verified until the
keys are loaded.  The main use case is embedded systems which do not
have an initramfs, but have a specially crafted init script.  It
requires enabling CONFIG_IMA_LOAD_X509 or CONFIG_EVM_LOAD_X509.  The
new VFS integrity_read() file operation method would not be called.

The main use case for the new VFS integrity_read() file operation
method is to calculate the file hash, as Christoph described.

Mimi

WARNING: multiple messages have this Message-ID (diff)
From: zohar@linux.vnet.ibm.com (Mimi Zohar)
To: linux-security-module@vger.kernel.org
Subject: [GIT PULL] Security subsystem updates for 4.14
Date: Sun, 10 Sep 2017 02:46:24 -0400	[thread overview]
Message-ID: <1505025984.3224.35.camel@linux.vnet.ibm.com> (raw)
In-Reply-To: <CA+55aFw8wgA+jhBOhnY-TSdbPgiYcrFiipCV=rsS1=GQEN+JgQ@mail.gmail.com>

On Thu, 2017-09-07 at 11:19 -0700, Linus Torvalds wrote:
> On Mon, Sep 4, 2017 at 3:29 AM, James Morris <jmorris@namei.org> wrote:
> >
> > IMA:
> >   - A new integrity_read file operation method, avoids races when
> >     calculating file hashes
> 
> Honestly, this seems really odd.
> 
> It documents that it needs to be called with i_rwsem held exclusively,
> and even has a lockdep assert to that effect (well, not really: the
> code claims "exclusive", but the lockdep assert does not), but I'm not
> actually seeing anybody doing it.
> 
> Quite the reverse, I just see integrity_read_file() doing filp_open()
> on the pathname and passing it to integrity_kernel_read() with no
> locking.
> 
> It really looks like just pure garbage to me. I pulled, and I'm not
> unpulling the whole thing. I don't think it's been tested, and I don't
> think it can be right.
> 
> Tell me why I'm wrong, or tell me why that garbage made it in in the
> first place?

I'm really sorry for the long delay in responding. ?I've been on
vacation the last week, mostly without cell phone and very limited
wifi access.?

True, there is a side case where integrity_read_file() is being called
without first taking the i_rwsem. ?This side case permits signed x509
certificates to be loaded onto the trusted IMA/EVM keyrings, without
verifying the file signature stored as security.ima/security.evm
xattrs. ?Basically, the xattr signatures can not be verified until the
keys are loaded. ?The main use case is embedded systems which do not
have an initramfs, but have a specially crafted init script. ?It
requires enabling CONFIG_IMA_LOAD_X509 or CONFIG_EVM_LOAD_X509. ?The
new VFS integrity_read() file operation method would not be called.

The main use case for the new VFS integrity_read() file operation
method is to calculate the file hash, as Christoph described.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

  parent reply	other threads:[~2017-09-10  6:47 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-09-04 10:29 [GIT PULL] Security subsystem updates for 4.14 James Morris
2017-09-04 10:29 ` James Morris
2017-09-07 18:19 ` Linus Torvalds
2017-09-07 18:19   ` Linus Torvalds
2017-09-08  4:48   ` James Morris
2017-09-08  4:48     ` James Morris
2017-09-08  7:09     ` Christoph Hellwig
2017-09-08  7:09       ` Christoph Hellwig
2017-09-08 17:25       ` Linus Torvalds
2017-09-08 17:25         ` Linus Torvalds
2017-09-08 17:36         ` Paul Moore
2017-09-08 17:36           ` Paul Moore
2017-09-10  4:32           ` James Morris
2017-09-10  4:32             ` James Morris
2017-09-10  4:53             ` James Morris
2017-09-10  4:53               ` James Morris
2017-09-11 22:30             ` Paul Moore
2017-09-11 22:30               ` Paul Moore
2017-09-14 21:09             ` Kees Cook
2017-09-14 21:09               ` Kees Cook
2017-09-14 21:13               ` James Morris
2017-09-14 21:13                 ` James Morris
2017-09-14 21:25                 ` Kees Cook
2017-09-14 21:25                   ` Kees Cook
2017-09-08 19:57         ` James Morris
2017-09-08 19:57           ` James Morris
2017-09-17  7:36           ` Mimi Zohar
2017-09-17  7:36             ` Mimi Zohar
2017-09-10  8:10         ` Christoph Hellwig
2017-09-10  8:10           ` Christoph Hellwig
2017-09-10 14:02           ` Mimi Zohar
2017-09-10 14:02             ` Mimi Zohar
2017-09-11  6:38             ` Christoph Hellwig
2017-09-11  6:38               ` Christoph Hellwig
2017-09-11 21:34               ` Mimi Zohar
2017-09-11 21:34                 ` Mimi Zohar
2017-09-08 22:38     ` Theodore Ts'o
2017-09-08 22:38       ` Theodore Ts'o
2017-09-10  2:08       ` James Morris
2017-09-10  2:08         ` James Morris
2017-09-10  7:13       ` Mimi Zohar
2017-09-10  7:13         ` Mimi Zohar
2017-09-10 12:17         ` Theodore Ts'o
2017-09-10 12:17           ` Theodore Ts'o
2017-09-10  6:46   ` Mimi Zohar [this message]
2017-09-10  6:46     ` 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=1505025984.3224.35.camel@linux.vnet.ibm.com \
    --to=zohar@linux.vnet.ibm.com \
    --cc=jmorris@namei.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=torvalds@linux-foundation.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 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.