Linux Security Modules development
 help / color / mirror / Atom feed
* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2020-08-11  8:48 UTC (permalink / raw)
  To: Jann Horn, Kees Cook, Deven Bowers, Mimi Zohar
  Cc: Al Viro, Andrew Morton, kernel list, Aleksa Sarai,
	Alexei Starovoitov, Andy Lutomirski, Christian Brauner,
	Christian Heimes, Daniel Borkmann, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Florian Weimer, James Morris, Jan Kara,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Philippe Trébuchet,
	Scott Shell, Sean Christopherson, Shuah Khan, Steve Dower,
	Steve Grubb, Tetsuo Handa, Thibaut Sautereau, Vincent Strubel,
	Kernel Hardening, Linux API, linux-integrity,
	linux-security-module, linux-fsdevel
In-Reply-To: <CAG48ez0NAV5gPgmbDaSjo=zzE=FgnYz=-OHuXwu0Vts=B5gesA@mail.gmail.com>


On 11/08/2020 01:03, Jann Horn wrote:
> On Tue, Aug 11, 2020 at 12:43 AM Mickaël Salaün <mic@digikod.net> wrote:
>> On 10/08/2020 22:21, Al Viro wrote:
>>> On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
>>>> It seems that there is no more complains nor questions. Do you want me
>>>> to send another series to fix the order of the S-o-b in patch 7?
>>>
>>> There is a major question regarding the API design and the choice of
>>> hooking that stuff on open().  And I have not heard anything resembling
>>> a coherent answer.
>>
>> Hooking on open is a simple design that enables processes to check files
>> they intend to open, before they open them. From an API point of view,
>> this series extends openat2(2) with one simple flag: O_MAYEXEC. The
>> enforcement is then subject to the system policy (e.g. mount points,
>> file access rights, IMA, etc.).
>>
>> Checking on open enables to not open a file if it does not meet some
>> requirements, the same way as if the path doesn't exist or (for whatever
>> reasons, including execution permission) if access is denied.
> 
> You can do exactly the same thing if you do the check in a separate
> syscall though.
> 
> And it provides a greater degree of flexibility; for example, you can
> use it in combination with fopen() without having to modify the
> internals of fopen() or having to use fdopen().
> 
>> It is a
>> good practice to check as soon as possible such properties, and it may
>> enables to avoid (user space) time-of-check to time-of-use (TOCTOU)
>> attacks (i.e. misuse of already open resources).
> 
> The assumption that security checks should happen as early as possible
> can actually cause security problems. For example, because seccomp was
> designed to do its checks as early as possible, including before
> ptrace, we had an issue for a long time where the ptrace API could be
> abused to bypass seccomp filters.
> 
> Please don't decide that a check must be ordered first _just_ because
> it is a security check. While that can be good for limiting attack
> surface, it can also create issues when the idea is applied too
> broadly.

I'd be interested with such security issue examples.

I hope that delaying checks will not be an issue for mechanisms such as
IMA or IPE:
https://lore.kernel.org/lkml/1544699060.6703.11.camel@linux.ibm.com/

Any though Mimi, Deven, Chrome OS folks?

> 
> I don't see how TOCTOU issues are relevant in any way here. If someone
> can turn a script that is considered a trusted file into an untrusted
> file and then maliciously change its contents, you're going to have
> issues either way because the modifications could still happen after
> openat(); if this was possible, the whole thing would kind of fall
> apart. And if that isn't possible, I don't see any TOCTOU.

Sure, and if the scripts are not protected in some way there is no point
to check anything.

> 
>> It is important to keep
>> in mind that the use cases we are addressing consider that the (user
>> space) script interpreters (or linkers) are trusted and unaltered (i.e.
>> integrity/authenticity checked). These are similar sought defensive
>> properties as for SUID/SGID binaries: attackers can still launch them
>> with malicious inputs (e.g. file paths, file descriptors, environment
>> variables, etc.), but the binaries can then have a way to check if they
>> can extend their trust to some file paths.
>>
>> Checking file descriptors may help in some use cases, but not the ones
>> motivating this series.
> 
> It actually provides a superset of the functionality that your
> existing patches provide.

It also brings new issues with multiple file descriptor origins (e.g.
memfd_create).

> 
>> Checking (already) opened resources could be a
>> *complementary* way to check execute permission, but it is not in the
>> scope of this series.

^ permalink raw reply

* RE: [PATCH v7 0/7] Add support for O_MAYEXEC
From: David Laight @ 2020-08-11  8:09 UTC (permalink / raw)
  To: 'Mickaël Salaün', Al Viro
  Cc: Kees Cook, Andrew Morton, linux-kernel@vger.kernel.org,
	Aleksa Sarai, Alexei Starovoitov, Andy Lutomirski,
	Christian Brauner, Christian Heimes, Daniel Borkmann,
	Deven Bowers, Dmitry Vyukov, Eric Biggers, Eric Chiang,
	Florian Weimer, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
In-Reply-To: <2531a0e8-5122-867c-ba06-5d2e623a3834@digikod.net>

> On 11/08/2020 00:28, Al Viro wrote:
> > On Mon, Aug 10, 2020 at 10:09:09PM +0000, David Laight wrote:
> >>> On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> >>>> It seems that there is no more complains nor questions. Do you want me
> >>>> to send another series to fix the order of the S-o-b in patch 7?
> >>>
> >>> There is a major question regarding the API design and the choice of
> >>> hooking that stuff on open().  And I have not heard anything resembling
> >>> a coherent answer.
> >>
> >> To me O_MAYEXEC is just the wrong name.
> >> The bit would be (something like) O_INTERPRET to indicate
> >> what you want to do with the contents.
> 
> The properties is "execute permission". This can then be checked by
> interpreters or other applications, then the generic O_MAYEXEC name.

The english sense of MAYEXEC is just wrong for what you are trying
to check.

> > ... which does not answer the question - name of constant is the least of
> > the worries here.  Why the hell is "apply some unspecified checks to
> > file" combined with opening it, rather than being an independent primitive
> > you apply to an already opened file?  Just in case - "'cuz that's how we'd
> > done it" does not make a good answer...

Maybe an access_ok() that acts on an open fd would be more
appropriate.
Which might end up being an fcntrl() action.
That would give you a full 32bit mask of options.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: James Bottomley @ 2020-08-11  5:43 UTC (permalink / raw)
  To: Chuck Lever
  Cc: Mimi Zohar, James Morris, Deven Bowers, Pavel Machek, Sasha Levin,
	snitzer, dm-devel, tyhicks, agk, Paul Moore, Jonathan Corbet,
	nramas, serge, pasha.tatashin, Jann Horn, linux-block, Al Viro,
	Jens Axboe, mdsakib, open list, eparis, linux-security-module,
	linux-audit, linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <6E907A22-02CC-42DD-B3CD-11D304F3A1A8@gmail.com>

On Mon, 2020-08-10 at 19:36 -0400, Chuck Lever wrote:
> > On Aug 10, 2020, at 11:35 AM, James Bottomley
> > <James.Bottomley@HansenPartnership.com> wrote:
> > On Sun, 2020-08-09 at 13:16 -0400, Mimi Zohar wrote:
> > > On Sat, 2020-08-08 at 13:47 -0400, Chuck Lever wrote:
[...]
> > > > The first priority (for me, anyway) therefore is getting the
> > > > ability to move IMA metadata between NFS clients and servers
> > > > shoveled into the NFS protocol, but that's been blocked for
> > > > various legal reasons.
> > > 
> > > Up to now, verifying remote filesystem file integrity has been
> > > out of scope for IMA.   With fs-verity file signatures I can at
> > > least grasp how remote file integrity could possibly work.  I
> > > don't understand how remote file integrity with existing IMA
> > > formats could be supported. You might want to consider writing a
> > > whitepaper, which could later be used as the basis for a patch
> > > set cover letter.
> > 
> > I think, before this, we can help with the basics (and perhaps we
> > should sort them out before we start documenting what we'll do).
> 
> Thanks for the help! I just want to emphasize that documentation
> (eg, a specification) will be critical for remote filesystems.
> 
> If any of this is to be supported by a remote filesystem, then we
> need an unencumbered description of the new metadata format rather
> than code. GPL-encumbered formats cannot be contributed to the NFS
> standard, and are probably difficult for other filesystems that are
> not Linux-native, like SMB, as well.

I don't understand what you mean by GPL encumbered formats.  The GPL is
a code licence not a data or document licence.  The way the spec
process works in Linux is that we implement or evolve a data format
under a GPL implementaiton, but that implementation doesn't implicate
the later standardisation of the data format and people are free to
reimplement under any licence they choose.

> > The first basic is that a merkle tree allows unit at a time
> > verification. First of all we should agree on the unit.  Since we
> > always fault a page at a time, I think our merkle tree unit should
> > be a page not a block.
> 
> Remote filesystems will need to agree that the size of that unit is
> the same everywhere, or the unit size could be stored in the per-file
> metadata.
> 
> 
> > Next, we should agree where the check gates for the per page
> > accesses should be ... definitely somewhere in readpage, I suspect
> > and finally we should agree how the merkle tree is presented at the
> > gate.  I think there are three ways:
> > 
> >   1. Ahead of time transfer:  The merkle tree is transferred and
> > verified
> >      at some time before the accesses begin, so we already have a
> >      verified copy and can compare against the lower leaf.
> >   2. Async transfer:  We provide an async mechanism to transfer the
> >      necessary components, so when presented with a unit, we check
> > the
> >      log n components required to get to the root
> >   3. The protocol actually provides the capability of 2 (like the
> > SCSI
> >      DIF/DIX), so to IMA all the pieces get presented instead of
> > IMA
> >      having to manage the tree
> 
> A Merkle tree is potentially large enough that it cannot be stored in
> an extended attribute. In addition, an extended attribute is not a
> byte stream that you can seek into or read small parts of, it is
> retrieved in a single shot.

Well you wouldn't store the tree would you, just the head hash.  The
rest of the tree can be derived from the data.  You need to distinguish
between what you *must* have to verify integrity (the head hash,
possibly signed) and what is nice to have to speed up the verification
process.  The choice for the latter is cache or reconstruct depending
on the resources available.  If the tree gets cached on the server,
that would be a server implementation detail invisible to the client.

> For this reason, the idea was to save only the signature of the
> tree's root on durable storage. The client would retrieve that
> signature possibly at open time, and reconstruct the tree at that
> time.

Right that's the integrity data you must have.

> Or the tree could be partially constructed on-demand at the time each
> unit is to be checked (say, as part of 2. above).

Whether it's reconstructed or cached can be an implementation detail. 
You clearly have to reconstruct once, but whether you have to do it
again depends on the memory available for caching and all the other
resource calls in the system.

> The client would have to reconstruct that tree again if memory
> pressure caused some or all of the tree to be evicted, so perhaps an
> on-demand mechanism is preferable.

Right, but I think that's implementation detail.  Probably what we need
is a way to get the log(N) verification hashes from the server and it's
up to the client whether it caches them or not.

> > There are also a load of minor things like how we get the head
> > hash, which must be presented and verified ahead of time for each
> > of the above 3.
> 
> Also, changes to a file's content and its tree signature are not
> atomic. If a file is mutable, then there is the period between when
> the file content has changed and when the signature is updated.
> Some discussion of how a client is to behave in those situations will
> be necessary.

For IMA, if you write to a checked file, it gets rechecked the next
time the gate (open/exec/mmap) is triggered.  This means you must
complete the update and have the new integrity data in-place before
triggering the check.  I think this could apply equally to a merkel
tree based system.  It's a sort of Doctor, Doctor it hurts when I do
this situation.

James


^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Chuck Lever @ 2020-08-10 23:36 UTC (permalink / raw)
  To: James Bottomley
  Cc: Mimi Zohar, James Morris, Deven Bowers, Pavel Machek, Sasha Levin,
	snitzer, dm-devel, tyhicks, agk, Paul Moore, Jonathan Corbet,
	nramas, serge, pasha.tatashin, Jann Horn, linux-block, Al Viro,
	Jens Axboe, mdsakib, open list, eparis, linux-security-module,
	linux-audit, linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <1597073737.3966.12.camel@HansenPartnership.com>



> On Aug 10, 2020, at 11:35 AM, James Bottomley <James.Bottomley@HansenPartnership.com> wrote:
> 
> On Sun, 2020-08-09 at 13:16 -0400, Mimi Zohar wrote:
>> On Sat, 2020-08-08 at 13:47 -0400, Chuck Lever wrote:
>>>> On Aug 5, 2020, at 2:15 PM, Mimi Zohar <zohar@linux.ibm.com>
>>>> wrote:
>> 
>> <snip>
>> 
>>>> If block layer integrity was enough, there wouldn't have been a
>>>> need for fs-verity.   Even fs-verity is limited to read only
>>>> filesystems, which makes validating file integrity so much
>>>> easier.  From the beginning, we've said that fs-verity signatures
>>>> should be included in the measurement list.  (I thought someone
>>>> signed on to add that support to IMA, but have not yet seen
>>>> anything.)
>>> 
>>> Mimi, when you and I discussed this during LSS NA 2019, I didn't
>>> fully understand that you expected me to implement signed Merkle
>>> trees for all filesystems. At the time, it sounded to me like you
>>> wanted signed Merkle trees only for NFS files. Is that still the
>>> case?
>> 
>> I definitely do not expect you to support signed Merkle trees for all
>> filesystems.  My interested is from an IMA perspective of measuring
>> and verifying the fs-verity Merkle tree root (and header info)
>> signature. This is independent of which filesystems support it.
>> 
>>> 
>>> The first priority (for me, anyway) therefore is getting the
>>> ability to move IMA metadata between NFS clients and servers
>>> shoveled into the NFS protocol, but that's been blocked for various
>>> legal reasons.
>> 
>> Up to now, verifying remote filesystem file integrity has been out of
>> scope for IMA.   With fs-verity file signatures I can at least grasp
>> how remote file integrity could possibly work.  I don't understand
>> how remote file integrity with existing IMA formats could be
>> supported. You might want to consider writing a whitepaper, which
>> could later be used as the basis for a patch set cover letter.
> 
> I think, before this, we can help with the basics (and perhaps we
> should sort them out before we start documenting what we'll do).

Thanks for the help! I just want to emphasize that documentation
(eg, a specification) will be critical for remote filesystems.

If any of this is to be supported by a remote filesystem, then we
need an unencumbered description of the new metadata format rather
than code. GPL-encumbered formats cannot be contributed to the NFS
standard, and are probably difficult for other filesystems that are
not Linux-native, like SMB, as well.


> The
> first basic is that a merkle tree allows unit at a time verification. 
> First of all we should agree on the unit.  Since we always fault a page
> at a time, I think our merkle tree unit should be a page not a block.

Remote filesystems will need to agree that the size of that unit is
the same everywhere, or the unit size could be stored in the per-file
metadata.


> Next, we should agree where the check gates for the per page accesses
> should be ... definitely somewhere in readpage, I suspect and finally
> we should agree how the merkle tree is presented at the gate.  I think
> there are three ways:
> 
>   1. Ahead of time transfer:  The merkle tree is transferred and verified
>      at some time before the accesses begin, so we already have a
>      verified copy and can compare against the lower leaf.
>   2. Async transfer:  We provide an async mechanism to transfer the
>      necessary components, so when presented with a unit, we check the
>      log n components required to get to the root
>   3. The protocol actually provides the capability of 2 (like the SCSI
>      DIF/DIX), so to IMA all the pieces get presented instead of IMA
>      having to manage the tree

A Merkle tree is potentially large enough that it cannot be stored in
an extended attribute. In addition, an extended attribute is not a
byte stream that you can seek into or read small parts of, it is
retrieved in a single shot.

For this reason, the idea was to save only the signature of the tree's
root on durable storage. The client would retrieve that signature
possibly at open time, and reconstruct the tree at that time.

Or the tree could be partially constructed on-demand at the time each
unit is to be checked (say, as part of 2. above).

The client would have to reconstruct that tree again if memory pressure
caused some or all of the tree to be evicted, so perhaps an on-demand
mechanism is preferable.


> There are also a load of minor things like how we get the head hash,
> which must be presented and verified ahead of time for each of the
> above 3.

Also, changes to a file's content and its tree signature are not
atomic. If a file is mutable, then there is the period between when
the file content has changed and when the signature is updated.
Some discussion of how a client is to behave in those situations will
be necessary.


--
Chuck Lever
chucklever@gmail.com




^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Al Viro @ 2020-08-10 23:05 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Kees Cook, Andrew Morton, linux-kernel, Aleksa Sarai,
	Alexei Starovoitov, Andy Lutomirski, Christian Brauner,
	Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
	Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
	Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
	linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <917bb071-8b1a-3ba4-dc16-f8d7b4cc849f@digikod.net>

On Tue, Aug 11, 2020 at 12:43:52AM +0200, Mickaël Salaün wrote:

> Hooking on open is a simple design that enables processes to check files
> they intend to open, before they open them.

Which is a good thing, because...?

> From an API point of view,
> this series extends openat2(2) with one simple flag: O_MAYEXEC. The
> enforcement is then subject to the system policy (e.g. mount points,
> file access rights, IMA, etc.).

That's what "unspecified" means - as far as the kernel concerned, it's
"something completely opaque, will let these hooks to play, semantics is
entirely up to them".
 
> Checking on open enables to not open a file if it does not meet some
> requirements, the same way as if the path doesn't exist or (for whatever
> reasons, including execution permission) if access is denied. It is a
> good practice to check as soon as possible such properties, and it may
> enables to avoid (user space) time-of-check to time-of-use (TOCTOU)
> attacks (i.e. misuse of already open resources).

?????  You explicitly assume a cooperating caller.  If it can't be trusted
to issue the check between open and use, or can be manipulated (ptraced,
etc.) into not doing so, how can you rely upon the flag having been passed
in the first place?  And TOCTOU window is definitely not wider that way.

If you want to have it done immediately after open(), bloody well do it
immediately after open.  If attacker has subverted your control flow to the
extent that allows them to hit descriptor table in the interval between
these two syscalls, you have already lost - they'll simply prevent that
flag from being passed.

What's the point of burying it inside openat2()?  A convenient multiplexor
to hook into?  We already have one - it's called do_syscall_...

^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Jann Horn @ 2020-08-10 23:03 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Al Viro, Kees Cook, Andrew Morton, kernel list, Aleksa Sarai,
	Alexei Starovoitov, Andy Lutomirski, Christian Brauner,
	Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel, Kernel Hardening, Linux API,
	linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <917bb071-8b1a-3ba4-dc16-f8d7b4cc849f@digikod.net>

On Tue, Aug 11, 2020 at 12:43 AM Mickaël Salaün <mic@digikod.net> wrote:
> On 10/08/2020 22:21, Al Viro wrote:
> > On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> >> It seems that there is no more complains nor questions. Do you want me
> >> to send another series to fix the order of the S-o-b in patch 7?
> >
> > There is a major question regarding the API design and the choice of
> > hooking that stuff on open().  And I have not heard anything resembling
> > a coherent answer.
>
> Hooking on open is a simple design that enables processes to check files
> they intend to open, before they open them. From an API point of view,
> this series extends openat2(2) with one simple flag: O_MAYEXEC. The
> enforcement is then subject to the system policy (e.g. mount points,
> file access rights, IMA, etc.).
>
> Checking on open enables to not open a file if it does not meet some
> requirements, the same way as if the path doesn't exist or (for whatever
> reasons, including execution permission) if access is denied.

You can do exactly the same thing if you do the check in a separate
syscall though.

And it provides a greater degree of flexibility; for example, you can
use it in combination with fopen() without having to modify the
internals of fopen() or having to use fdopen().

> It is a
> good practice to check as soon as possible such properties, and it may
> enables to avoid (user space) time-of-check to time-of-use (TOCTOU)
> attacks (i.e. misuse of already open resources).

The assumption that security checks should happen as early as possible
can actually cause security problems. For example, because seccomp was
designed to do its checks as early as possible, including before
ptrace, we had an issue for a long time where the ptrace API could be
abused to bypass seccomp filters.

Please don't decide that a check must be ordered first _just_ because
it is a security check. While that can be good for limiting attack
surface, it can also create issues when the idea is applied too
broadly.

I don't see how TOCTOU issues are relevant in any way here. If someone
can turn a script that is considered a trusted file into an untrusted
file and then maliciously change its contents, you're going to have
issues either way because the modifications could still happen after
openat(); if this was possible, the whole thing would kind of fall
apart. And if that isn't possible, I don't see any TOCTOU.

> It is important to keep
> in mind that the use cases we are addressing consider that the (user
> space) script interpreters (or linkers) are trusted and unaltered (i.e.
> integrity/authenticity checked). These are similar sought defensive
> properties as for SUID/SGID binaries: attackers can still launch them
> with malicious inputs (e.g. file paths, file descriptors, environment
> variables, etc.), but the binaries can then have a way to check if they
> can extend their trust to some file paths.
>
> Checking file descriptors may help in some use cases, but not the ones
> motivating this series.

It actually provides a superset of the functionality that your
existing patches provide.

> Checking (already) opened resources could be a
> *complementary* way to check execute permission, but it is not in the
> scope of this series.

^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2020-08-10 22:47 UTC (permalink / raw)
  To: Al Viro, David Laight
  Cc: Kees Cook, Andrew Morton, linux-kernel@vger.kernel.org,
	Aleksa Sarai, Alexei Starovoitov, Andy Lutomirski,
	Christian Brauner, Christian Heimes, Daniel Borkmann,
	Deven Bowers, Dmitry Vyukov, Eric Biggers, Eric Chiang,
	Florian Weimer, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
In-Reply-To: <20200810222838.GF1236603@ZenIV.linux.org.uk>



On 11/08/2020 00:28, Al Viro wrote:
> On Mon, Aug 10, 2020 at 10:09:09PM +0000, David Laight wrote:
>>> On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
>>>> It seems that there is no more complains nor questions. Do you want me
>>>> to send another series to fix the order of the S-o-b in patch 7?
>>>
>>> There is a major question regarding the API design and the choice of
>>> hooking that stuff on open().  And I have not heard anything resembling
>>> a coherent answer.
>>
>> To me O_MAYEXEC is just the wrong name.
>> The bit would be (something like) O_INTERPRET to indicate
>> what you want to do with the contents.

The properties is "execute permission". This can then be checked by
interpreters or other applications, then the generic O_MAYEXEC name.

> 
> ... which does not answer the question - name of constant is the least of
> the worries here.  Why the hell is "apply some unspecified checks to
> file" combined with opening it, rather than being an independent primitive
> you apply to an already opened file?  Just in case - "'cuz that's how we'd
> done it" does not make a good answer...
> 

That is not the case, see
https://lore.kernel.org/lkml/917bb071-8b1a-3ba4-dc16-f8d7b4cc849f@digikod.net/

^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2020-08-10 22:43 UTC (permalink / raw)
  To: Al Viro
  Cc: Kees Cook, Andrew Morton, linux-kernel, Aleksa Sarai,
	Alexei Starovoitov, Andy Lutomirski, Christian Brauner,
	Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
	Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
	Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
	linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <20200810202123.GC1236603@ZenIV.linux.org.uk>


On 10/08/2020 22:21, Al Viro wrote:
> On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
>> It seems that there is no more complains nor questions. Do you want me
>> to send another series to fix the order of the S-o-b in patch 7?
> 
> There is a major question regarding the API design and the choice of
> hooking that stuff on open().  And I have not heard anything resembling
> a coherent answer.

Hooking on open is a simple design that enables processes to check files
they intend to open, before they open them. From an API point of view,
this series extends openat2(2) with one simple flag: O_MAYEXEC. The
enforcement is then subject to the system policy (e.g. mount points,
file access rights, IMA, etc.).

Checking on open enables to not open a file if it does not meet some
requirements, the same way as if the path doesn't exist or (for whatever
reasons, including execution permission) if access is denied. It is a
good practice to check as soon as possible such properties, and it may
enables to avoid (user space) time-of-check to time-of-use (TOCTOU)
attacks (i.e. misuse of already open resources). It is important to keep
in mind that the use cases we are addressing consider that the (user
space) script interpreters (or linkers) are trusted and unaltered (i.e.
integrity/authenticity checked). These are similar sought defensive
properties as for SUID/SGID binaries: attackers can still launch them
with malicious inputs (e.g. file paths, file descriptors, environment
variables, etc.), but the binaries can then have a way to check if they
can extend their trust to some file paths.

Checking file descriptors may help in some use cases, but not the ones
motivating this series. Checking (already) opened resources could be a
*complementary* way to check execute permission, but it is not in the
scope of this series.

^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Al Viro @ 2020-08-10 22:28 UTC (permalink / raw)
  To: David Laight
  Cc: Mickaël Salaün, Kees Cook, Andrew Morton,
	linux-kernel@vger.kernel.org, Aleksa Sarai, Alexei Starovoitov,
	Andy Lutomirski, Christian Brauner, Christian Heimes,
	Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
In-Reply-To: <30b8c003f49d4280be5215f634ca2c06@AcuMS.aculab.com>

On Mon, Aug 10, 2020 at 10:09:09PM +0000, David Laight wrote:
> > On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> > > It seems that there is no more complains nor questions. Do you want me
> > > to send another series to fix the order of the S-o-b in patch 7?
> > 
> > There is a major question regarding the API design and the choice of
> > hooking that stuff on open().  And I have not heard anything resembling
> > a coherent answer.
> 
> To me O_MAYEXEC is just the wrong name.
> The bit would be (something like) O_INTERPRET to indicate
> what you want to do with the contents.

... which does not answer the question - name of constant is the least of
the worries here.  Why the hell is "apply some unspecified checks to
file" combined with opening it, rather than being an independent primitive
you apply to an already opened file?  Just in case - "'cuz that's how we'd
done it" does not make a good answer...

^ permalink raw reply

* RE: [PATCH v7 0/7] Add support for O_MAYEXEC
From: David Laight @ 2020-08-10 22:09 UTC (permalink / raw)
  To: 'Al Viro', Mickaël Salaün
  Cc: Kees Cook, Andrew Morton, linux-kernel@vger.kernel.org,
	Aleksa Sarai, Alexei Starovoitov, Andy Lutomirski,
	Christian Brauner, Christian Heimes, Daniel Borkmann,
	Deven Bowers, Dmitry Vyukov, Eric Biggers, Eric Chiang,
	Florian Weimer, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel,
	kernel-hardening@lists.openwall.com, linux-api@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	linux-fsdevel@vger.kernel.org
In-Reply-To: <20200810202123.GC1236603@ZenIV.linux.org.uk>

> On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> > It seems that there is no more complains nor questions. Do you want me
> > to send another series to fix the order of the S-o-b in patch 7?
> 
> There is a major question regarding the API design and the choice of
> hooking that stuff on open().  And I have not heard anything resembling
> a coherent answer.

To me O_MAYEXEC is just the wrong name.
The bit would be (something like) O_INTERPRET to indicate
what you want to do with the contents.

The kernel 'policy' then decides whether that needs 'r-x'
access or whether 'r--' access in enough.

I think that is what you 100 line comment in 0/n means.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: James Morris @ 2020-08-10 20:29 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: James Bottomley, Deven Bowers, Pavel Machek, Sasha Levin, snitzer,
	dm-devel, tyhicks, agk, paul, corbet, nramas, serge,
	pasha.tatashin, jannh, linux-block, viro, axboe, mdsakib,
	linux-kernel, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <268edec96cbe7d2626c9158b806e8865b6b1b8ed.camel@linux.ibm.com>

On Fri, 7 Aug 2020, Mimi Zohar wrote:

> > > Are you planning to attend Plumbers? Perhaps we could propose a BoF 
> > > session on this topic.
> > 
> > That sounds like a good idea.
> 
> Other than it is already sold out.

Mimi advised me off-list that she is able to attend, so I've submitted a 
BoF proposal:

https://www.linuxplumbersconf.org/event/7/abstracts/732/


-- 
James Morris
<jmorris@namei.org>


^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Al Viro @ 2020-08-10 20:21 UTC (permalink / raw)
  To: Mickaël Salaün
  Cc: Kees Cook, Andrew Morton, linux-kernel, Aleksa Sarai,
	Alexei Starovoitov, Andy Lutomirski, Christian Brauner,
	Christian Heimes, Daniel Borkmann, Deven Bowers, Dmitry Vyukov,
	Eric Biggers, Eric Chiang, Florian Weimer, James Morris, Jan Kara,
	Jann Horn, Jonathan Corbet, Lakshmi Ramasubramanian,
	Matthew Garrett, Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
	linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <0733fbed-cc73-027b-13c7-c368c2d67fb3@digikod.net>

On Mon, Aug 10, 2020 at 10:11:53PM +0200, Mickaël Salaün wrote:
> It seems that there is no more complains nor questions. Do you want me
> to send another series to fix the order of the S-o-b in patch 7?

There is a major question regarding the API design and the choice of
hooking that stuff on open().  And I have not heard anything resembling
a coherent answer.

^ permalink raw reply

* Re: [PATCH v7 0/7] Add support for O_MAYEXEC
From: Mickaël Salaün @ 2020-08-10 20:11 UTC (permalink / raw)
  To: Kees Cook, Andrew Morton
  Cc: linux-kernel, Aleksa Sarai, Alexei Starovoitov, Al Viro,
	Andy Lutomirski, Christian Brauner, Christian Heimes,
	Daniel Borkmann, Deven Bowers, Dmitry Vyukov, Eric Biggers,
	Eric Chiang, Florian Weimer, James Morris, Jan Kara, Jann Horn,
	Jonathan Corbet, Lakshmi Ramasubramanian, Matthew Garrett,
	Matthew Wilcox, Michael Kerrisk, Mimi Zohar,
	Philippe Trébuchet, Scott Shell, Sean Christopherson,
	Shuah Khan, Steve Dower, Steve Grubb, Tetsuo Handa,
	Thibaut Sautereau, Vincent Strubel, kernel-hardening, linux-api,
	linux-integrity, linux-security-module, linux-fsdevel
In-Reply-To: <202007241205.751EBE7@keescook>

It seems that there is no more complains nor questions. Do you want me
to send another series to fix the order of the S-o-b in patch 7?


On 24/07/2020 21:06, Kees Cook wrote:
> I think this looks good now.
> 
> Andrew, since you're already carrying my exec clean-ups (repeated here
> in patch 1-3), can you pick the rest of this series too?
> 
> Thanks!
> 
> -Kees
> 
> On Thu, Jul 23, 2020 at 07:12:20PM +0200, Mickaël Salaün wrote:
>> Hi,
>>
>> This seventh patch series do not set __FMODE_EXEC for the sake of
>> simplicity.  A notification feature could be added later if needed.  The
>> handling of all file types is now well defined and tested: by default,
>> when opening a path, access to a directory is denied (with EISDIR),
>> access to a regular file depends on the sysctl policy, and access to
>> other file types (i.e. fifo, device, socket) is denied if there is any
>> enforced policy.  There is new tests covering all these cases (cf.
>> test_file_types() ).
>>
>> As requested by Mimi Zohar, I completed the series with one of her
>> patches for IMA.  I also picked Kees Cook's patches to consolidate exec
>> permission checking into do_filp_open()'s flow.
>>
>>
>> # Goal of O_MAYEXEC
>>
>> The goal of this patch series is to enable to control script execution
>> with interpreters help.  A new O_MAYEXEC flag, usable through
>> openat2(2), is added to enable userspace script interpreters to delegate
>> to the kernel (and thus the system security policy) the permission to
>> interpret/execute scripts or other files containing what can be seen as
>> commands.
>>
>> A simple system-wide security policy can be enforced by the system
>> administrator through a sysctl configuration consistent with the mount
>> points or the file access rights.  The documentation patch explains the
>> prerequisites.
>>
>> Furthermore, the security policy can also be delegated to an LSM, either
>> a MAC system or an integrity system.  For instance, the new kernel
>> MAY_OPENEXEC flag closes a major IMA measurement/appraisal interpreter
>> integrity gap by bringing the ability to check the use of scripts [1].
>> Other uses are expected, such as for magic-links [2], SGX integration
>> [3], bpffs [4] or IPE [5].
>>
>>
>> # Prerequisite of its use
>>
>> Userspace needs to adapt to take advantage of this new feature.  For
>> example, the PEP 578 [6] (Runtime Audit Hooks) enables Python 3.8 to be
>> extended with policy enforcement points related to code interpretation,
>> which can be used to align with the PowerShell audit features.
>> Additional Python security improvements (e.g. a limited interpreter
>> withou -c, stdin piping of code) are on their way [7].
>>
>>
>> # Examples
>>
>> The initial idea comes from CLIP OS 4 and the original implementation
>> has been used for more than 12 years:
>> https://github.com/clipos-archive/clipos4_doc
>> Chrome OS has a similar approach:
>> https://chromium.googlesource.com/chromiumos/docs/+/master/security/noexec_shell_scripts.md
>>
>> Userland patches can be found here:
>> https://github.com/clipos-archive/clipos4_portage-overlay/search?q=O_MAYEXEC
>> Actually, there is more than the O_MAYEXEC changes (which matches this search)
>> e.g., to prevent Python interactive execution. There are patches for
>> Bash, Wine, Java (Icedtea), Busybox's ash, Perl and Python. There are
>> also some related patches which do not directly rely on O_MAYEXEC but
>> which restrict the use of browser plugins and extensions, which may be
>> seen as scripts too:
>> https://github.com/clipos-archive/clipos4_portage-overlay/tree/master/www-client
>>
>> An introduction to O_MAYEXEC was given at the Linux Security Summit
>> Europe 2018 - Linux Kernel Security Contributions by ANSSI:
>> https://www.youtube.com/watch?v=chNjCRtPKQY&t=17m15s
>> The "write xor execute" principle was explained at Kernel Recipes 2018 -
>> CLIP OS: a defense-in-depth OS:
>> https://www.youtube.com/watch?v=PjRE0uBtkHU&t=11m14s
>> See also an overview article: https://lwn.net/Articles/820000/
>>
>>
>> This patch series can be applied on top of v5.8-rc5 .  This can be tested
>> with CONFIG_SYSCTL.  I would really appreciate constructive comments on
>> this patch series.
>>
>> Previous version:
>> https://lore.kernel.org/lkml/20200505153156.925111-1-mic@digikod.net/
>>
>>
>> [1] https://lore.kernel.org/lkml/1544647356.4028.105.camel@linux.ibm.com/
>> [2] https://lore.kernel.org/lkml/20190904201933.10736-6-cyphar@cyphar.com/
>> [3] https://lore.kernel.org/lkml/CALCETrVovr8XNZSroey7pHF46O=kj_c5D9K8h=z2T_cNrpvMig@mail.gmail.com/
>> [4] https://lore.kernel.org/lkml/CALCETrVeZ0eufFXwfhtaG_j+AdvbzEWE0M3wjXMWVEO7pj+xkw@mail.gmail.com/
>> [5] https://lore.kernel.org/lkml/20200406221439.1469862-12-deven.desai@linux.microsoft.com/
>> [6] https://www.python.org/dev/peps/pep-0578/
>> [7] https://lore.kernel.org/lkml/0c70debd-e79e-d514-06c6-4cd1e021fa8b@python.org/
>>
>> Regards,
>>
>> Kees Cook (3):
>>   exec: Change uselib(2) IS_SREG() failure to EACCES
>>   exec: Move S_ISREG() check earlier
>>   exec: Move path_noexec() check earlier
>>
>> Mickaël Salaün (3):
>>   fs: Introduce O_MAYEXEC flag for openat2(2)
>>   fs,doc: Enable to enforce noexec mounts or file exec through O_MAYEXEC
>>   selftest/openat2: Add tests for O_MAYEXEC enforcing
>>
>> Mimi Zohar (1):
>>   ima: add policy support for the new file open MAY_OPENEXEC flag
>>
>>  Documentation/ABI/testing/ima_policy          |   2 +-
>>  Documentation/admin-guide/sysctl/fs.rst       |  49 +++
>>  fs/exec.c                                     |  23 +-
>>  fs/fcntl.c                                    |   2 +-
>>  fs/namei.c                                    |  36 +-
>>  fs/open.c                                     |  12 +-
>>  include/linux/fcntl.h                         |   2 +-
>>  include/linux/fs.h                            |   3 +
>>  include/uapi/asm-generic/fcntl.h              |   7 +
>>  kernel/sysctl.c                               |  12 +-
>>  security/integrity/ima/ima_main.c             |   3 +-
>>  security/integrity/ima/ima_policy.c           |  15 +-
>>  tools/testing/selftests/kselftest_harness.h   |   3 +
>>  tools/testing/selftests/openat2/Makefile      |   3 +-
>>  tools/testing/selftests/openat2/config        |   1 +
>>  tools/testing/selftests/openat2/helpers.h     |   1 +
>>  .../testing/selftests/openat2/omayexec_test.c | 325 ++++++++++++++++++
>>  17 files changed, 470 insertions(+), 29 deletions(-)
>>  create mode 100644 tools/testing/selftests/openat2/config
>>  create mode 100644 tools/testing/selftests/openat2/omayexec_test.c
>>
>> -- 
>> 2.27.0
>>
> 

^ permalink raw reply

* [GIT PULL] Security subsystem updates for v5.9
From: James Morris @ 2020-08-10 18:55 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: linux-kernel, linux-security-module

A couple of minor documentation updates only for this release. Please 
pull.

---

The following changes since commit 3d77e6a8804abcc0504c904bd6e5cdf3a5cf8162:

  Linux 5.7 (2020-05-31 16:49:15 -0700)

are available in the Git repository at:

  git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git tags/for-v5.9

for you to fetch changes up to bb22d80b47d5dd641d09d31946c4be0f610f3f45:

  LSM: drop duplicated words in header file comments (2020-08-06 12:00:17 -0700)

----------------------------------------------------------------
Minor fixes for v5.9.

----------------------------------------------------------------
Alexander A. Klimov (1):
      Replace HTTP links with HTTPS ones: security

Randy Dunlap (1):
      LSM: drop duplicated words in header file comments

 include/linux/lsm_hook_defs.h                    | 2 +-
 include/linux/lsm_hooks.h                        | 2 +-
 security/Kconfig                                 | 2 +-
 security/apparmor/Kconfig                        | 2 +-
 security/integrity/ima/Kconfig                   | 2 +-
 security/integrity/ima/ima_template.c            | 2 +-
 security/integrity/ima/ima_template_lib.c        | 2 +-
 security/integrity/ima/ima_template_lib.h        | 2 +-
 security/keys/encrypted-keys/ecryptfs_format.c   | 2 +-
 security/keys/encrypted-keys/ecryptfs_format.h   | 2 +-
 security/keys/encrypted-keys/encrypted.c         | 2 +-
 security/keys/encrypted-keys/masterkey_trusted.c | 2 +-
 12 files changed, 12 insertions(+), 12 deletions(-)

^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Mimi Zohar @ 2020-08-10 17:57 UTC (permalink / raw)
  To: James Bottomley, Chuck Lever, James Morris
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <1597079586.3966.34.camel@HansenPartnership.com>

On Mon, 2020-08-10 at 10:13 -0700, James Bottomley wrote:
> On Mon, 2020-08-10 at 12:35 -0400, Mimi Zohar wrote:
> > On Mon, 2020-08-10 at 08:35 -0700, James Bottomley wrote:
> [...]
> > > > Up to now, verifying remote filesystem file integrity has been
> > > > out of scope for IMA.   With fs-verity file signatures I can at
> > > > least grasp how remote file integrity could possibly work.  I
> > > > don't understand how remote file integrity with existing IMA
> > > > formats could be supported. You might want to consider writing a
> > > > whitepaper, which could later be used as the basis for a patch
> > > > set cover letter.
> > > 
> > > I think, before this, we can help with the basics (and perhaps we
> > > should sort them out before we start documenting what we'll do).
> > 
> > I'm not opposed to doing that, but you're taking this discussion in a
> > totally different direction.  The current discussion is about NFSv4
> > supporting the existing IMA signatures, not only fs-verity
> > signatures. I'd like to understand how that is possible and for the
> > community to weigh in on whether it makes sense.
> 
> Well, I see the NFS problem as being chunk at a time, right, which is
> merkle tree, or is there a different chunk at a time mechanism we want
> to use?  IMA currently verifies signature on open/exec and then
> controls updates.  Since for NFS we only control the client, we can't
> do that on an NFS server, so we really do need verification at read
> time ... unless we're threading IMA back to the NFS server?

Yes.  I still don't see how we can support the existing IMA signatures,
which is based on the file data hash, unless the "chunk at a time
mechanism" is not a tree, but linear.

Mimi

> 
> > > The first basic is that a merkle tree allows unit at a time
> > > verification. First of all we should agree on the unit.  Since we
> > > always fault a page at a time, I think our merkle tree unit should
> > > be a page not a block. Next, we should agree where the check gates
> > > for the per page accesses should be ... definitely somewhere in
> > > readpage, I suspect and finally we should agree how the merkle tree
> > > is presented at the gate.  I think there are three ways:
> > > 
> > >    1. Ahead of time transfer:  The merkle tree is transferred and
> > > verified
> > >       at some time before the accesses begin, so we already have a
> > >       verified copy and can compare against the lower leaf.
> > >    2. Async transfer:  We provide an async mechanism to transfer
> > > the
> > >       necessary components, so when presented with a unit, we check
> > > the
> > >       log n components required to get to the root
> > >    3. The protocol actually provides the capability of 2 (like the
> > > SCSI
> > >       DIF/DIX), so to IMA all the pieces get presented instead of
> > > IMA
> > >       having to manage the tree
> > > 
> > > There are also a load of minor things like how we get the head
> > > hash, which must be presented and verified ahead of time for each
> > > of the above 3.
> > 
> >  
> > I was under the impression that IMA support for fs-verity signatures
> > would be limited to including the fs-verity signature in the
> > measurement list and verifying the fs-verity signature.   As fs-
> > verity is limited to immutable files, this could be done on file
> > open.  fs-verity would be responsible for enforcing the block/page
> > data integrity.   From a local filesystem perspective, I think that
> > is all that is necessary.
> 
> The fs-verity use case is a bit of a crippled one because it's
> immutable.  I think NFS represents more the general case where you
> can't rely on immutability and have to verify at chunk read time.  If
> we get chunk at a time working for NFS, it should work also for fs-
> verity and we wouldn't need to have two special paths.
> 
> I think, even for NFS we would only really need to log the open, so
> same as you imagine for fs-verity.  As long as the chunk read hashes
> match, we can be silent because everything is going OK, so we only need
> to determine what to do and log on mismatch (which isn't expected to
> happen for fs-verity).
> 
> > In terms of remote file systems,  the main issue is transporting and
> > storing the Merkle tree.  As fs-verity is limited to immutable files,
> > this could still be done on file open.
> 
> Right, I mentioned that in my options ... we need some "supply
> integrity" hook ... or possibly multiple hooks for a variety of
> possible methods.


^ permalink raw reply

* Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: Madhavan T. Venkataraman @ 2020-08-10 17:34 UTC (permalink / raw)
  To: Andy Lutomirski
  Cc: Kernel Hardening, Linux API, linux-arm-kernel, Linux FS Devel,
	linux-integrity, LKML, LSM List, Oleg Nesterov, X86 ML
In-Reply-To: <CALCETrUJ2hBmJujyCtEqx4=pknRvjvi1-Gj9wfRcMMzejjKQsQ@mail.gmail.com>

Resending because of mailer problems. Some of the recipients did not receive
my email. I apologize. Sigh.

Here is a redefinition of trampfd based on review comments.

I wanted to address dynamic code in 3 different ways:

    Remove the need for dynamic code where possible
    --------------------------------------------------------------------

    If the kernel itself can perform the work of some dynamic code, then
    the code can be replaced by the kernel.

    This is what I implemented in the patchset. But reviewers objected
    to the performance impact. One trip to the kernel was needed for each
    trampoline invocation. So, I have decided to defer this approach.

    Convert dynamic code to static code where possible
    ----------------------------------------------------------------------

    This is possible with help from the kernel. This has no performance
    impact and can be used in libffi, GCC nested functions, etc. I have
    described the approach below.

    Deal with code generation
    -----------------------------------

    For cases like generating JIT code from Java byte code, I wanted to
    establish a framework. However, reviewers felt that details are missing.

    Should the kernel generate code or should it use a user-level code generator?
    How do you make sure that a user level code generator can be trusted?
    How would the communication work? ABI details? Architecture support?
    Support for different types - JIT, DBT, etc?

    I have come to the conclusion that this is best done separately.

My main interest is to provide a way to convert dynamic code such as
trampolines to static code without any special architecture support.
This can be done with the kernel's help. Any code that gets written in
the future can conform to this as well.

So, in version 2 of the Trampfd RFC, I would like to simplify trampfd and
just address item 2. I will reimplement the support in libffi and present it.

Convert dynamic code to static code
------------------------------------------------

One problem with dynamic code is that it cannot be verified or authenticated
by the kernel. The kernel cannot tell the difference between genuine dynamic
code and an attacker's code. Where possible, dynamic code should be converted
to static code and placed in the text segment of a binary file. This allows
the kernel to verify the code by verifying the signature of the file.

The other problem is using user-level methods to load and execute dynamic code
can potentially be exploited by an attacker to inject his code and have it be
executed. To prevent this, a system may enforce W^X. If W^X is enforced
properly, genuine dynamic code will not be able to run. This is another
reason to convert dynamic code to static code.

The issue in converting dynamic code to static code is that the data is
dynamic. The code does not know before hand where the data is going to be
at runtime.

Some architectures support PC-relative data references. So, if you co-locate
code and data, then the code can find the data at runtime. But this is not
supported on all architectures. When supported, there may be limitations to
deal with. Plus you have to take the trouble to co-locate code and data.
And, to deal with W^X, code and data need to be in different pages.

All architectures must be supported without any limitations. Fortunately,
the kernel can solve this problem quite easily. I suggest the following:

Convert dynamic code to static code like this:

    - Decide which register should point to the data that the code needs.
      Call it register R.

    - Write the static code assuming that R already points to the data.

    - Use trampfd and pass the following to the kernel:

        - pointers to the code and data
        - the name of the register R

The kernel will write the following instructions in a trampoline page
mapped into the caller's address space with R-X.

    - Load the data address in register R
    - Jump to the static code

Basically, the kernel provides a trampoline to jump to the user's code
and returns the kernel-provided trampoline's address to the user.

It is trivial to implement a trampoline table in the trampoline page to
conserve memory.

Issues raised previously
-------------------------------

I believe that the following issues that were raised by reviewers is not
a problem in this scheme. Please rereview.

    - Florian mentioned the libffi trampoline table. Trampoline tables can be
      implemented in this scheme easily.

    - Florian mentioned stack unwinders. I am not an expert on unwinders.
      But I don't see an issue with unwinders.

    - Mark Rutland mentioned Intel's CET and CFI. Don't see a problem there.

    - Mark Rutland mentioned PAC+BTI on ARM64. Don't see a problem there.

If I have missed addressing any previously raised issue, I apologize.
Please let me know.

Thanks!

Madhavan



^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: James Bottomley @ 2020-08-10 17:13 UTC (permalink / raw)
  To: Mimi Zohar, Chuck Lever, James Morris
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <4664ab7dc3b324084df323bfa4670d5bfde76e66.camel@linux.ibm.com>

On Mon, 2020-08-10 at 12:35 -0400, Mimi Zohar wrote:
> On Mon, 2020-08-10 at 08:35 -0700, James Bottomley wrote:
[...]
> > > Up to now, verifying remote filesystem file integrity has been
> > > out of scope for IMA.   With fs-verity file signatures I can at
> > > least grasp how remote file integrity could possibly work.  I
> > > don't understand how remote file integrity with existing IMA
> > > formats could be supported. You might want to consider writing a
> > > whitepaper, which could later be used as the basis for a patch
> > > set cover letter.
> > 
> > I think, before this, we can help with the basics (and perhaps we
> > should sort them out before we start documenting what we'll do).
> 
> I'm not opposed to doing that, but you're taking this discussion in a
> totally different direction.  The current discussion is about NFSv4
> supporting the existing IMA signatures, not only fs-verity
> signatures. I'd like to understand how that is possible and for the
> community to weigh in on whether it makes sense.

Well, I see the NFS problem as being chunk at a time, right, which is
merkle tree, or is there a different chunk at a time mechanism we want
to use?  IMA currently verifies signature on open/exec and then
controls updates.  Since for NFS we only control the client, we can't
do that on an NFS server, so we really do need verification at read
time ... unless we're threading IMA back to the NFS server?

> > The first basic is that a merkle tree allows unit at a time
> > verification. First of all we should agree on the unit.  Since we
> > always fault a page at a time, I think our merkle tree unit should
> > be a page not a block. Next, we should agree where the check gates
> > for the per page accesses should be ... definitely somewhere in
> > readpage, I suspect and finally we should agree how the merkle tree
> > is presented at the gate.  I think there are three ways:
> > 
> >    1. Ahead of time transfer:  The merkle tree is transferred and
> > verified
> >       at some time before the accesses begin, so we already have a
> >       verified copy and can compare against the lower leaf.
> >    2. Async transfer:  We provide an async mechanism to transfer
> > the
> >       necessary components, so when presented with a unit, we check
> > the
> >       log n components required to get to the root
> >    3. The protocol actually provides the capability of 2 (like the
> > SCSI
> >       DIF/DIX), so to IMA all the pieces get presented instead of
> > IMA
> >       having to manage the tree
> > 
> > There are also a load of minor things like how we get the head
> > hash, which must be presented and verified ahead of time for each
> > of the above 3.
> 
>  
> I was under the impression that IMA support for fs-verity signatures
> would be limited to including the fs-verity signature in the
> measurement list and verifying the fs-verity signature.   As fs-
> verity is limited to immutable files, this could be done on file
> open.  fs-verity would be responsible for enforcing the block/page
> data integrity.   From a local filesystem perspective, I think that
> is all that is necessary.

The fs-verity use case is a bit of a crippled one because it's
immutable.  I think NFS represents more the general case where you
can't rely on immutability and have to verify at chunk read time.  If
we get chunk at a time working for NFS, it should work also for fs-
verity and we wouldn't need to have two special paths.

I think, even for NFS we would only really need to log the open, so
same as you imagine for fs-verity.  As long as the chunk read hashes
match, we can be silent because everything is going OK, so we only need
to determine what to do and log on mismatch (which isn't expected to
happen for fs-verity).

> In terms of remote file systems,  the main issue is transporting and
> storing the Merkle tree.  As fs-verity is limited to immutable files,
> this could still be done on file open.

Right, I mentioned that in my options ... we need some "supply
integrity" hook ... or possibly multiple hooks for a variety of
possible methods.

James


^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Mimi Zohar @ 2020-08-10 16:35 UTC (permalink / raw)
  To: James Bottomley, Chuck Lever, James Morris
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <1597073737.3966.12.camel@HansenPartnership.com>

On Mon, 2020-08-10 at 08:35 -0700, James Bottomley wrote:
> On Sun, 2020-08-09 at 13:16 -0400, Mimi Zohar wrote:
> > On Sat, 2020-08-08 at 13:47 -0400, Chuck Lever wrote:
> > > > On Aug 5, 2020, at 2:15 PM, Mimi Zohar <zohar@linux.ibm.com>
> > > > wrote:
> > 
> > <snip>
> > 
> > > > If block layer integrity was enough, there wouldn't have been a
> > > > need for fs-verity.   Even fs-verity is limited to read only
> > > > filesystems, which makes validating file integrity so much
> > > > easier.  From the beginning, we've said that fs-verity signatures
> > > > should be included in the measurement list.  (I thought someone
> > > > signed on to add that support to IMA, but have not yet seen
> > > > anything.)
> > > 
> > > Mimi, when you and I discussed this during LSS NA 2019, I didn't
> > > fully understand that you expected me to implement signed Merkle
> > > trees for all filesystems. At the time, it sounded to me like you
> > > wanted signed Merkle trees only for NFS files. Is that still the
> > > case?
> > 
> > I definitely do not expect you to support signed Merkle trees for all
> > filesystems.  My interested is from an IMA perspective of measuring
> > and verifying the fs-verity Merkle tree root (and header info)
> > signature. This is independent of which filesystems support it.
> > 
> > > The first priority (for me, anyway) therefore is getting the
> > > ability to move IMA metadata between NFS clients and servers
> > > shoveled into the NFS protocol, but that's been blocked for various
> > > legal reasons.
> > 
> > Up to now, verifying remote filesystem file integrity has been out of
> > scope for IMA.   With fs-verity file signatures I can at least grasp
> > how remote file integrity could possibly work.  I don't understand
> > how remote file integrity with existing IMA formats could be
> > supported. You might want to consider writing a whitepaper, which
> > could later be used as the basis for a patch set cover letter.
> 
> I think, before this, we can help with the basics (and perhaps we
> should sort them out before we start documenting what we'll do).

I'm not opposed to doing that, but you're taking this discussion in a
totally different direction.  The current discussion is about NFSv4
supporting the existing IMA signatures, not only fs-verity signatures. 
I'd like to understand how that is possible and for the community to
weigh in on whether it makes sense.

> The
> first basic is that a merkle tree allows unit at a time verification.
> First of all we should agree on the unit.  Since we always fault a page
> at a time, I think our merkle tree unit should be a page not a block. 
> Next, we should agree where the check gates for the per page accesses
> should be ... definitely somewhere in readpage, I suspect and finally
> we should agree how the merkle tree is presented at the gate.  I think
> there are three ways:
> 
>    1. Ahead of time transfer:  The merkle tree is transferred and verified
>       at some time before the accesses begin, so we already have a
>       verified copy and can compare against the lower leaf.
>    2. Async transfer:  We provide an async mechanism to transfer the
>       necessary components, so when presented with a unit, we check the
>       log n components required to get to the root
>    3. The protocol actually provides the capability of 2 (like the SCSI
>       DIF/DIX), so to IMA all the pieces get presented instead of IMA
>       having to manage the tree
> 
> There are also a load of minor things like how we get the head hash,
> which must be presented and verified ahead of time for each of the
> above 3.
 
I was under the impression that IMA support for fs-verity signatures
would be limited to including the fs-verity signature in the
measurement list and verifying the fs-verity signature.   As fs-verity
is limited to immutable files, this could be done on file open.  fs-
verity would be responsible for enforcing the block/page data
integrity.   From a local filesystem perspective, I think that is all
that is necessary.

In terms of remote file systems,  the main issue is transporting and
storing the Merkle tree.  As fs-verity is limited to immutable files,
this could still be done on file open.

Mimi


^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: James Bottomley @ 2020-08-10 15:35 UTC (permalink / raw)
  To: Mimi Zohar, Chuck Lever, James Morris
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <da6f54d0438ee3d3903b2c75fcfbeb0afdf92dc2.camel@linux.ibm.com>

On Sun, 2020-08-09 at 13:16 -0400, Mimi Zohar wrote:
> On Sat, 2020-08-08 at 13:47 -0400, Chuck Lever wrote:
> > > On Aug 5, 2020, at 2:15 PM, Mimi Zohar <zohar@linux.ibm.com>
> > > wrote:
> 
> <snip>
> 
> > > If block layer integrity was enough, there wouldn't have been a
> > > need for fs-verity.   Even fs-verity is limited to read only
> > > filesystems, which makes validating file integrity so much
> > > easier.  From the beginning, we've said that fs-verity signatures
> > > should be included in the measurement list.  (I thought someone
> > > signed on to add that support to IMA, but have not yet seen
> > > anything.)
> > 
> > Mimi, when you and I discussed this during LSS NA 2019, I didn't
> > fully understand that you expected me to implement signed Merkle
> > trees for all filesystems. At the time, it sounded to me like you
> > wanted signed Merkle trees only for NFS files. Is that still the
> > case?
> 
> I definitely do not expect you to support signed Merkle trees for all
> filesystems.  My interested is from an IMA perspective of measuring
> and verifying the fs-verity Merkle tree root (and header info)
> signature. This is independent of which filesystems support it.
> 
> > 
> > The first priority (for me, anyway) therefore is getting the
> > ability to move IMA metadata between NFS clients and servers
> > shoveled into the NFS protocol, but that's been blocked for various
> > legal reasons.
> 
> Up to now, verifying remote filesystem file integrity has been out of
> scope for IMA.   With fs-verity file signatures I can at least grasp
> how remote file integrity could possibly work.  I don't understand
> how remote file integrity with existing IMA formats could be
> supported. You might want to consider writing a whitepaper, which
> could later be used as the basis for a patch set cover letter.

I think, before this, we can help with the basics (and perhaps we
should sort them out before we start documenting what we'll do).  The
first basic is that a merkle tree allows unit at a time verification. 
First of all we should agree on the unit.  Since we always fault a page
at a time, I think our merkle tree unit should be a page not a block. 
Next, we should agree where the check gates for the per page accesses
should be ... definitely somewhere in readpage, I suspect and finally
we should agree how the merkle tree is presented at the gate.  I think
there are three ways:

   1. Ahead of time transfer:  The merkle tree is transferred and verified
      at some time before the accesses begin, so we already have a
      verified copy and can compare against the lower leaf.
   2. Async transfer:  We provide an async mechanism to transfer the
      necessary components, so when presented with a unit, we check the
      log n components required to get to the root
   3. The protocol actually provides the capability of 2 (like the SCSI
      DIF/DIX), so to IMA all the pieces get presented instead of IMA
      having to manage the tree

There are also a load of minor things like how we get the head hash,
which must be presented and verified ahead of time for each of the
above 3.

James




^ permalink raw reply

* Re: [PATCH 4.19/4.14/4.9/4.4] Smack: fix use-after-free in smk_write_relabel_self()
From: Greg KH @ 2020-08-10 13:39 UTC (permalink / raw)
  To: Eric Biggers
  Cc: stable, linux-security-module, syzbot+e6416dabb497a650da40,
	Casey Schaufler
In-Reply-To: <20200807161324.1690303-1-ebiggers@kernel.org>

On Fri, Aug 07, 2020 at 09:13:24AM -0700, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> commit beb4ee6770a89646659e6a2178538d2b13e2654e upstream.
> 
> smk_write_relabel_self() frees memory from the task's credentials with
> no locking, which can easily cause a use-after-free because multiple
> tasks can share the same credentials structure.
> 
> Fix this by using prepare_creds() and commit_creds() to correctly modify
> the task's credentials.
> 
> Reproducer for "BUG: KASAN: use-after-free in smk_write_relabel_self":
> 
> 	#include <fcntl.h>
> 	#include <pthread.h>
> 	#include <unistd.h>
> 
> 	static void *thrproc(void *arg)
> 	{
> 		int fd = open("/sys/fs/smackfs/relabel-self", O_WRONLY);
> 		for (;;) write(fd, "foo", 3);
> 	}
> 
> 	int main()
> 	{
> 		pthread_t t;
> 		pthread_create(&t, NULL, thrproc, NULL);
> 		thrproc(NULL);
> 	}
> 
> Reported-by: syzbot+e6416dabb497a650da40@syzkaller.appspotmail.com
> Fixes: 38416e53936e ("Smack: limited capability for changing process label")
> Cc: <stable@vger.kernel.org> # v4.4+
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> ---
>  security/smack/smackfs.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)

Thanks for the backport, now queued up.

greg k-h

^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Mimi Zohar @ 2020-08-09 17:16 UTC (permalink / raw)
  To: Chuck Lever, James Morris, James Bottomley
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <329E8DBA-049E-4959-AFD4-9D118DEB176E@gmail.com>

On Sat, 2020-08-08 at 13:47 -0400, Chuck Lever wrote:
> > On Aug 5, 2020, at 2:15 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:

<snip>

> > If block layer integrity was enough, there wouldn't have been a need
> > for fs-verity.   Even fs-verity is limited to read only filesystems,
> > which makes validating file integrity so much easier.  From the
> > beginning, we've said that fs-verity signatures should be included in
> > the measurement list.  (I thought someone signed on to add that support
> > to IMA, but have not yet seen anything.)
> 
> Mimi, when you and I discussed this during LSS NA 2019, I didn't fully
> understand that you expected me to implement signed Merkle trees for all
> filesystems. At the time, it sounded to me like you wanted signed Merkle
> trees only for NFS files. Is that still the case?

I definitely do not expect you to support signed Merkle trees for all
filesystems.  My interested is from an IMA perspective of measuring and
verifying the fs-verity Merkle tree root (and header info) signature. 
This is independent of which filesystems support it.

> 
> The first priority (for me, anyway) therefore is getting the ability to
> move IMA metadata between NFS clients and servers shoveled into the NFS
> protocol, but that's been blocked for various legal reasons.

Up to now, verifying remote filesystem file integrity has been out of
scope for IMA.   With fs-verity file signatures I can at least grasp
how remote file integrity could possibly work.  I don't understand how
remote file integrity with existing IMA formats could be supported. You
might want to consider writing a whitepaper, which could later be used
as the basis for a patch set cover letter.

Mimi

> 
> IMO we need agreement from everyone (integrity developers, FS
> implementers, and Linux distributors) that a signed Merkle tree IMA
> metadata format, stored in either an xattr or appended to an executable
> file, will be the way forward for IMA in all filesystems.


^ permalink raw reply

* Re: [PATCH v1 0/4] [RFC] Implement Trampoline File Descriptor
From: Pavel Machek @ 2020-08-08 22:17 UTC (permalink / raw)
  To: Madhavan T. Venkataraman
  Cc: Mark Rutland, kernel-hardening, linux-api, linux-arm-kernel,
	linux-fsdevel, linux-integrity, linux-kernel,
	linux-security-module, oleg, x86
In-Reply-To: <b3368692-afe6-89b5-d634-12f4f0a601f8@linux.microsoft.com>

Hi!

> Thanks for the lively discussion. I have tried to answer some of the
> comments below.

> > There are options today, e.g.
> >
> > a) If the restriction is only per-alias, you can have distinct aliases
> >    where one is writable and another is executable, and you can make it
> >    hard to find the relationship between the two.
> >
> > b) If the restriction is only temporal, you can write instructions into
> >    an RW- buffer, transition the buffer to R--, verify the buffer
> >    contents, then transition it to --X.
> >
> > c) You can have two processes A and B where A generates instrucitons into
> >    a buffer that (only) B can execute (where B may be restricted from
> >    making syscalls like write, mprotect, etc).
> 
> The general principle of the mitigation is W^X. I would argue that
> the above options are violations of the W^X principle. If they are
> allowed today, they must be fixed. And they will be. So, we cannot
> rely on them.

Would you mind describing your threat model?

Because I believe you are using model different from everyone else.

In particular, I don't believe b) is a problem or should be fixed.

I'll add d), application mmaps a file(R--), and uses write syscall to change
trampolines in it.

> b) This is again a violation. The kernel should refuse to give execute
> ???????? permission to a page that was writeable in the past and refuse to
> ???????? give write permission to a page that was executable in the past.

Why?

										Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

^ permalink raw reply

* Re: [dm-devel] [RFC PATCH v5 00/11] Integrity Policy Enforcement LSM (IPE)
From: Chuck Lever @ 2020-08-08 17:47 UTC (permalink / raw)
  To: Mimi Zohar, James Morris, James Bottomley
  Cc: Deven Bowers, Pavel Machek, Sasha Levin, snitzer, dm-devel,
	tyhicks, agk, Paul Moore, Jonathan Corbet, nramas, serge,
	pasha.tatashin, Jann Horn, linux-block, Al Viro, Jens Axboe,
	mdsakib, open list, eparis, linux-security-module, linux-audit,
	linux-fsdevel, linux-integrity, jaskarankhurana
In-Reply-To: <b08ae82102f35936427bf138085484f75532cff1.camel@linux.ibm.com>



> On Aug 5, 2020, at 2:15 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Wed, 2020-08-05 at 09:59 -0700, James Morris wrote:
>> On Wed, 5 Aug 2020, James Bottomley wrote:
>> 
>>> I'll leave Mimi to answer, but really this is exactly the question that
>>> should have been asked before writing IPE.  However, since we have the
>>> cart before the horse, let me break the above down into two specific
>>> questions.
>> 
>> The question is valid and it was asked. We decided to first prototype what 
>> we needed and then evaluate if it should be integrated with IMA. We 
>> discussed this plan in person with Mimi (at LSS-NA in 2019), and presented 
>> a more mature version of IPE to LSS-NA in 2020, with the expectation that 
>> such a discussion may come up (it did not).
> 
> When we first spoke the concepts weren't fully formulated, at least to
> me.
>> 
>> These patches are still part of this process and 'RFC' status.
>> 
>>>   1. Could we implement IPE in IMA (as in would extensions to IMA cover
>>>      everything).  I think the answers above indicate this is a "yes".
>> 
>> It could be done, if needed.
>> 
>>>   2. Should we extend IMA to implement it?  This is really whether from a
>>>      usability standpoint two seperate LSMs would make sense to cover the
>>>      different use cases.
>> 
>> One issue here is that IMA is fundamentally a measurement & appraisal 
>> scheme which has been extended to include integrity enforcement. IPE was 
>> designed from scratch to only perform integrity enforcement. As such, it 
>> is a cleaner design -- "do one thing and do it well" is a good design 
>> pattern.
>> 
>> In our use-case, we utilize _both_ IMA and IPE, for attestation and code 
>> integrity respectively. It is useful to be able to separate these 
>> concepts. They really are different:
>> 
>> - Code integrity enforcement ensures that code running locally is of known 
>> provenance and has not been modified prior to execution.

My interest is in code integrity enforcement for executables stored
in NFS files.

My struggle with IPE is that due to its dependence on dm-verity, it
does not seem to able to protect content that is stored separately
from its execution environment and accessed via a file access
protocol (FUSE, SMB, NFS, etc).


>> - Attestation is about measuring the health of a system and having that 
>> measurement validated by a remote system. (Local attestation is useless).
>> 
>> I'm not sure there is value in continuing to shoe-horn both of these into 
>> IMA.
> 
> True, IMA was originally limited to measurement and attestation, but
> most of the original EVM concepts were subsequently included in IMA. 
> (Remember, Reiner Sailer wrote the original IMA, which I inherited.  I
> was originially working on EVM code integrity.)  From a naming
> perspective including EVM code integrity in IMA was a mistake.  My
> thinking at the time was that as IMA was already calculating the file
> hash, instead of re-calculating the file hash for integrity, calculate
> the file hash once and re-use it for multiple things - measurement, 
> integrity, and audit.   At the same time define a single system wide
> policy.
> 
> When we first started working on IMA, EVM, trusted, and encrypted keys,
> the general kernel community didn't see a need for any of it.  Thus, a
> lot of what was accomplished has been accomplished without the backing
> of the real core filesystem people.
> 
> If block layer integrity was enough, there wouldn't have been a need
> for fs-verity.   Even fs-verity is limited to read only filesystems,
> which makes validating file integrity so much easier.  From the
> beginning, we've said that fs-verity signatures should be included in
> the measurement list.  (I thought someone signed on to add that support
> to IMA, but have not yet seen anything.)

Mimi, when you and I discussed this during LSS NA 2019, I didn't fully
understand that you expected me to implement signed Merkle trees for all
filesystems. At the time, it sounded to me like you wanted signed Merkle
trees only for NFS files. Is that still the case?

The first priority (for me, anyway) therefore is getting the ability to
move IMA metadata between NFS clients and servers shoveled into the NFS
protocol, but that's been blocked for various legal reasons.

IMO we need agreement from everyone (integrity developers, FS
implementers, and Linux distributors) that a signed Merkle tree IMA
metadata format, stored in either an xattr or appended to an executable
file, will be the way forward for IMA in all filesystems.


> Going forward I see a lot of what we've accomplished being incorporated
> into the filesystems.  When IMA will be limited to defining a system
> wide policy, I'll have completed my job.
> 
> Mimi
> 
>> 
>>> I've got to say the least attractive thing
>>>      about separation is the fact that you now both have a policy parser.
>>>       You've tried to differentiate yours by making it more Kconfig
>>>      based, but policy has a way of becoming user space supplied because
>>>      the distros hate config options, so I think you're going to end up
>>>      with a policy parser very like IMAs.


^ permalink raw reply

* Re: [PATCH v6 1/3] Add a new LSM-supporting anonymous inode interface
From: Al Viro @ 2020-08-07 23:02 UTC (permalink / raw)
  To: Lokesh Gidra
  Cc: James Morris, Stephen Smalley, Casey Schaufler, Eric Biggers,
	Serge E. Hallyn, Paul Moore, Eric Paris, Daniel Colascione,
	Kees Cook, Eric W. Biederman, KP Singh, David Howells,
	Thomas Cedeno, Anders Roxell, Sami Tolvanen, Matthew Garrett,
	Aaron Goidel, Randy Dunlap, Joel Fernandes (Google), YueHaibing,
	Christian Brauner, Alexei Starovoitov, Alexey Budankov,
	Adrian Reber, Aleksa Sarai, linux-fsdevel, linux-kernel,
	linux-security-module, selinux, kaleshsingh, calin, surenb, nnk,
	jeffv, kernel-team, Daniel Colascione
In-Reply-To: <20200807224941.3440722-2-lokeshgidra@google.com>

On Fri, Aug 07, 2020 at 03:49:39PM -0700, Lokesh Gidra wrote:

> The new functions accept an optional context_inode parameter that
> callers can use to provide additional contextual information to
> security modules, e.g., indicating that one anonymous struct file is a
> logical child of another, allowing a security model to propagate
> security information from one to the other.

What the hell is "logical child" and what are the lifetime rules implied
by that relationship?

^ permalink raw reply

* [PATCH v6 3/3] Wire UFFD up to SELinux
From: Lokesh Gidra @ 2020-08-07 22:49 UTC (permalink / raw)
  To: Alexander Viro, James Morris, Stephen Smalley, Casey Schaufler,
	Eric Biggers
  Cc: Serge E. Hallyn, Paul Moore, Eric Paris, Lokesh Gidra,
	Daniel Colascione, Kees Cook, Eric W. Biederman, KP Singh,
	David Howells, Thomas Cedeno, Anders Roxell, Sami Tolvanen,
	Matthew Garrett, Aaron Goidel, Randy Dunlap,
	Joel Fernandes (Google), YueHaibing, Christian Brauner,
	Alexei Starovoitov, Alexey Budankov, Adrian Reber, Aleksa Sarai,
	linux-fsdevel, linux-kernel, linux-security-module, selinux,
	kaleshsingh, calin, surenb, nnk, jeffv, kernel-team,
	Daniel Colascione
In-Reply-To: <20200807224941.3440722-1-lokeshgidra@google.com>

From: Daniel Colascione <dancol@google.com>

This change gives userfaultfd file descriptors a real security
context, allowing policy to act on them.

Signed-off-by: Daniel Colascione <dancol@google.com>

[Remove owner inode from userfaultfd_ctx]
[Use anon_inode_getfd_secure() instead of anon_inode_getfile_secure()
 in userfaultfd syscall]
[Use inode of file in userfaultfd_read() in resolve_userfault_fork()]

Signed-off-by: Lokesh Gidra <lokeshgidra@google.com>
---
 fs/userfaultfd.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c
index 6e264dded46e..23c8618ebe35 100644
--- a/fs/userfaultfd.c
+++ b/fs/userfaultfd.c
@@ -978,14 +978,16 @@ static __poll_t userfaultfd_poll(struct file *file, poll_table *wait)
 
 static const struct file_operations userfaultfd_fops;
 
-static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
-				  struct userfaultfd_ctx *new,
+static int resolve_userfault_fork(struct userfaultfd_ctx *new,
+				  struct inode *inode,
 				  struct uffd_msg *msg)
 {
 	int fd;
 
-	fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, new,
-			      O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS));
+	fd = anon_inode_getfd_secure(
+		"[userfaultfd]", &userfaultfd_fops, new,
+		O_RDWR | (new->flags & UFFD_SHARED_FCNTL_FLAGS),
+		inode);
 	if (fd < 0)
 		return fd;
 
@@ -995,7 +997,7 @@ static int resolve_userfault_fork(struct userfaultfd_ctx *ctx,
 }
 
 static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
-				    struct uffd_msg *msg)
+				    struct uffd_msg *msg, struct inode *inode)
 {
 	ssize_t ret;
 	DECLARE_WAITQUEUE(wait, current);
@@ -1106,7 +1108,7 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait,
 	spin_unlock_irq(&ctx->fd_wqh.lock);
 
 	if (!ret && msg->event == UFFD_EVENT_FORK) {
-		ret = resolve_userfault_fork(ctx, fork_nctx, msg);
+		ret = resolve_userfault_fork(fork_nctx, inode, msg);
 		spin_lock_irq(&ctx->event_wqh.lock);
 		if (!list_empty(&fork_event)) {
 			/*
@@ -1166,6 +1168,7 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf,
 	ssize_t _ret, ret = 0;
 	struct uffd_msg msg;
 	int no_wait = file->f_flags & O_NONBLOCK;
+	struct inode *inode = file_inode(file);
 
 	if (ctx->state == UFFD_STATE_WAIT_API)
 		return -EINVAL;
@@ -1173,7 +1176,7 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf,
 	for (;;) {
 		if (count < sizeof(msg))
 			return ret ? ret : -EINVAL;
-		_ret = userfaultfd_ctx_read(ctx, no_wait, &msg);
+		_ret = userfaultfd_ctx_read(ctx, no_wait, &msg, inode);
 		if (_ret < 0)
 			return ret ? ret : _ret;
 		if (copy_to_user((__u64 __user *) buf, &msg, sizeof(msg)))
@@ -1995,8 +1998,10 @@ SYSCALL_DEFINE1(userfaultfd, int, flags)
 	/* prevent the mm struct to be freed */
 	mmgrab(ctx->mm);
 
-	fd = anon_inode_getfd("[userfaultfd]", &userfaultfd_fops, ctx,
-			      O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS));
+	fd = anon_inode_getfd_secure("[userfaultfd]",
+		&userfaultfd_fops, ctx,
+		O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS),
+		NULL);
 	if (fd < 0) {
 		mmdrop(ctx->mm);
 		kmem_cache_free(userfaultfd_ctx_cachep, ctx);
-- 
2.28.0.236.gb10cc79966-goog


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox