* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: hpa @ 2019-05-13 0:23 UTC (permalink / raw)
To: Dominik Brodowski, Mimi Zohar
Cc: Roberto Sassu, viro, linux-security-module, linux-integrity,
initramfs, linux-api, linux-fsdevel, linux-kernel, zohar,
silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190512153105.GA25254@light.dominikbrodowski.net>
On May 12, 2019 8:31:05 AM PDT, Dominik Brodowski <linux@dominikbrodowski.net> wrote:
>On Sun, May 12, 2019 at 03:18:16AM -0700, hpa@zytor.com wrote:
>> > Couldn't this parsing of the .xattr-list file and the setting of
>the xattrs
>> > be done equivalently by the initramfs' /init? Why is kernel
>involvement
>> > actually required here?
>>
>> There are a lot of things that could/should be done that way...
>
>Indeed... so why not try to avoid adding more such "things", and
>keeping
>them in userspace (or in a fork_usermode_blob)?
>
>
>On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
>> It's too late. The /init itself should be signed and verified.
>
>Could you elaborate a bit more about the threat model, and why
>deferring
>this to the initramfs is too late?
>
>Thanks,
> Dominik
I tried over 10 years ago to make exactly that happen... it was called the klibc project. Linus turned it down because he felt that it didn't provide enough immediate benefit to justify the complexity, which of course creates the thousand-cuts problem: there will never be *one single* event that *by itself* justifies the transition.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: hpa @ 2019-05-13 0:21 UTC (permalink / raw)
To: Mimi Zohar, Dominik Brodowski
Cc: Roberto Sassu, viro, linux-security-module, linux-integrity,
initramfs, linux-api, linux-fsdevel, linux-kernel, zohar,
silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky, arnd, rob,
james.w.mcmechan
In-Reply-To: <1557705750.10635.264.camel@linux.ibm.com>
On May 12, 2019 5:02:30 PM PDT, Mimi Zohar <zohar@linux.ibm.com> wrote:
>On Sun, 2019-05-12 at 17:31 +0200, Dominik Brodowski wrote:
>> On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
>
>
>> > It's too late. The /init itself should be signed and verified.
>>
>> Could you elaborate a bit more about the threat model, and why
>deferring
>> this to the initramfs is too late?
>
>The IMA policy defines a number of different methods of identifying
>which files to measure, appraise, audit.[1] Without xattrs, the
>granularity of the policy rules is severely limited. Without xattrs,
>a filesystem is either in policy, or not.
>
>With an IMA policy rule requiring rootfs (tmpfs) files to be verified,
>then /init needs to be properly labeled, otherwise /init will fail to
>execute.
>
>Mimi
>
>[1] Documentation/ABI/testing/ima_policy
And the question is what is the sense in that, especially if /init is provided as play of the kernel itself.
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-13 0:02 UTC (permalink / raw)
To: Dominik Brodowski, hpa
Cc: Roberto Sassu, viro, linux-security-module, linux-integrity,
initramfs, linux-api, linux-fsdevel, linux-kernel, zohar,
silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190512153105.GA25254@light.dominikbrodowski.net>
On Sun, 2019-05-12 at 17:31 +0200, Dominik Brodowski wrote:
> On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
> > It's too late. The /init itself should be signed and verified.
>
> Could you elaborate a bit more about the threat model, and why deferring
> this to the initramfs is too late?
The IMA policy defines a number of different methods of identifying
which files to measure, appraise, audit.[1] Without xattrs, the
granularity of the policy rules is severely limited. Without xattrs,
a filesystem is either in policy, or not.
With an IMA policy rule requiring rootfs (tmpfs) files to be verified,
then /init needs to be properly labeled, otherwise /init will fail to
execute.
Mimi
[1] Documentation/ABI/testing/ima_policy
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: Linus Torvalds @ 2019-05-12 19:17 UTC (permalink / raw)
To: Paul Moore
Cc: Casey Schaufler, James Morris, LSM List,
Linux List Kernel Mailing
In-Reply-To: <CAHC9VhR-oqJwyvB2JhzTu2_nuZuENA=Y9f4rtfUrSGtLMnGZfw@mail.gmail.com>
On Sun, May 12, 2019 at 11:33 AM Paul Moore <paul@paul-moore.com> wrote:
>
> As far as I'm concerned, whatever makes it easier for Linus to consume
> the changes is the preferred path.
So for me it's not so much the pulling itself that ends up being a
problem, it's just that I want to feel like the pull request makes
sense, and I want to feel "safe" in pulling it.
The audit and selinux trees haven't had any issues that I can recall,
so that has worked fine. And it's been easier now that most of the
security layer things have been coming in separately, so it's easy for
me to see what's coming in, when it's in sane chunks that stand alone.
And then I also feel like _if_ there are problems in one area, it's
not affecting any other area the things are now. So if there's
something I want to look at a bit more, I might pull all the other
requests that I don't have any questions about, so that I can then
look more closely at the part I want to understand better.
I basically want to see pulls being "well-defined". That's both in the
area they affect, but also in the explanations for the pull request,
and preferably really also in the history (ie both the whole "starting
at a well-defined point", but also a "history is nice and clear", so I
like seeing topic branches and well-defined merges).
I guess it's kind of hard to explain, but it just gives me the warm
and fuzzies if I do a
gitk ORIG_HEAD..
after having done a merge, and the history and merges I see "make sense".
Then I feel like even if there are problems with the code I pull, even
the problems will hopefully be well-defined. Sometimes that is
literally "ok, I'm bisecting some issue, but even before I've bisected
the whole way, I can see that it came in through a particular pull
request or two" because the changes came in through a few clearly
separated topic branch and it's clear which pull request it is even if
there are still a hundred more commits to bisect all the way..
Side note: this depends on the subsystem. When I pull huge subsystems
like networking from Davem or the big driver pulls from Greg, I don't
even look at the gitk history and ask myself "does this make sense",
because I have ceded that kind of worries over to Greg and Davem. The
history in their areas are their concern, and their subsystems are so
big that I don't expect them to make it make sense to me, if you see
what I mean. But if you want to see an area where you can see how
people have split up development in topic branches, you can look at my
"x86 merges", where I get separate pull requests for each topic
branch, and they all tend to be very clearly defined (but then there
might often a "misc leftovers" branch, or a couple of branches that
are just one or two commits).
> My guess is that you are right and
> any *significant* changes to the LSM layer itself, e.g. security/*, is
> best sent via James' tree. For smaller changes to the LSM layer I
> think it's okay if they go in via an individual LSM tree so long as
> all the other LSMs agree-on/ack the changes; which pretty much fits
> what we've been doing for some time now and it seems to work well
> enough.
Yeah, I think that's the sane model. And I think it's mostly been working.
Linus
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-12 17:05 UTC (permalink / raw)
To: Mimi Zohar, Dominik Brodowski, Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan
In-Reply-To: <1557665567.10635.222.camel@linux.ibm.com>
On 5/12/19 7:52 AM, Mimi Zohar wrote:
> On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
>> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>>> This proposal consists in marshaling pathnames and xattrs in a file called
>>> .xattr-list. They are unmarshaled by the CPIO parser after all files have
>>> been extracted.
>>
>> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
>> be done equivalently by the initramfs' /init? Why is kernel involvement
>> actually required here?
>
> It's too late. The /init itself should be signed and verified.
If the initramfs cpio.gz image was signed and verified by the extractor, how is
the init in it _not_ verified?
Rob
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: Paul Moore @ 2019-05-12 15:33 UTC (permalink / raw)
To: Casey Schaufler
Cc: Linus Torvalds, James Morris, LSM List, Linux List Kernel Mailing
In-Reply-To: <24d602d2-a1a7-7b1e-9035-a2d732cd822b@schaufler-ca.com>
On Sat, May 11, 2019 at 6:08 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
> On 5/11/2019 11:13 AM, Paul Moore wrote:
> > On Sat, May 11, 2019 at 10:38 AM Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> >> On Fri, May 10, 2019 at 6:09 PM James Morris <jmorris@namei.org> wrote:
> >>> These patches include fixes to enable fuzz testing, and a fix for
> >>> calculating whether a filesystem is user-modifiable.
> >> So now these have been very recently rebased (on top of a random
> >> merge-window "tree of the day" version) instead of having multiple
> >> merges.
> >>
> >> That makes the history cleaner, but has its own issues.
> >>
> >> We really need to find a different model for the security layer patches.
> >
> > If it helps, the process I use for the SELinux and audit trees is
> > documented below. While it's far from perfect (I still don't like
> > basing the -next trees on -rcX releases) it has seemed to work
> > reasonably well for some time now.
> >
> > * https://github.com/SELinuxProject/selinux-kernel/blob/master/README.md
>
> On the whole this looks fine to me. I am less comfortable than Paul
> is regarding changes that happen elsewhere, so I would be more likely
> to base in the rc-1 than Paul. More developers test with SELinux than
> Smack. I am in the process of putting an appropriate GPG environment
> together for 5.3.
I share your concern about external changes outside the subsystem
breaking things; this doesn't happen all that often with the audit
tree, but it seems to happen every couple of releases with the SELinux
tree. This is one of the reasons why I test linux/master +
selinux/next + audit/next every few days, if not more often (see link
below). I've found this regular testing to be extremely helpful, and
if anyone is interested I'd be happy to help you get something similar
going.
* http://www.paul-moore.com/blog/d/2019/04/kernel_secnext_repo.html
FWIW, the reason I don't like basing against -rc1 (or any -rcX tag for
that matter) is that the -rcX releases tend to be buggier than the
"proper" releases. However, in practice I find myself basing the
selinux/next and audit/next branches against -rc1 almost every release
now; the rate of change outside the subsystem trees is simply too
high.
> The LSM infrastructure work I've been doing should still go through
> James, as it has global implications.
As far as I'm concerned, whatever makes it easier for Linus to consume
the changes is the preferred path. My guess is that you are right and
any *significant* changes to the LSM layer itself, e.g. security/*, is
best sent via James' tree. For smaller changes to the LSM layer I
think it's okay if they go in via an individual LSM tree so long as
all the other LSMs agree-on/ack the changes; which pretty much fits
what we've been doing for some time now and it seems to work well
enough.
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Dominik Brodowski @ 2019-05-12 15:31 UTC (permalink / raw)
To: hpa, Mimi Zohar
Cc: Roberto Sassu, viro, linux-security-module, linux-integrity,
initramfs, linux-api, linux-fsdevel, linux-kernel, zohar,
silviu.vlasceanu, dmitry.kasatkin, takondra, kamensky, arnd, rob,
james.w.mcmechan
In-Reply-To: <4E92753A-04BD-4018-A3A4-5E3E4242D8B9@zytor.com>
On Sun, May 12, 2019 at 03:18:16AM -0700, hpa@zytor.com wrote:
> > Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> > be done equivalently by the initramfs' /init? Why is kernel involvement
> > actually required here?
>
> There are a lot of things that could/should be done that way...
Indeed... so why not try to avoid adding more such "things", and keeping
them in userspace (or in a fork_usermode_blob)?
On Sun, May 12, 2019 at 08:52:47AM -0400, Mimi Zohar wrote:
> It's too late. The /init itself should be signed and verified.
Could you elaborate a bit more about the threat model, and why deferring
this to the initramfs is too late?
Thanks,
Dominik
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-12 12:52 UTC (permalink / raw)
To: Dominik Brodowski, Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190512091748.s6fvy2f5p2a2o6ja@isilmar-4.linta.de>
On Sun, 2019-05-12 at 11:17 +0200, Dominik Brodowski wrote:
> On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> > This proposal consists in marshaling pathnames and xattrs in a file called
> > .xattr-list. They are unmarshaled by the CPIO parser after all files have
> > been extracted.
>
> Couldn't this parsing of the .xattr-list file and the setting of the xattrs
> be done equivalently by the initramfs' /init? Why is kernel involvement
> actually required here?
It's too late. The /init itself should be signed and verified.
Mimi
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: hpa @ 2019-05-12 10:18 UTC (permalink / raw)
To: Dominik Brodowski, Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, arnd, rob, james.w.mcmechan
In-Reply-To: <20190512091748.s6fvy2f5p2a2o6ja@isilmar-4.linta.de>
On May 12, 2019 2:17:48 AM PDT, Dominik Brodowski <linux@dominikbrodowski.net> wrote:
>On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
>> This proposal consists in marshaling pathnames and xattrs in a file
>called
>> .xattr-list. They are unmarshaled by the CPIO parser after all files
>have
>> been extracted.
>
>Couldn't this parsing of the .xattr-list file and the setting of the
>xattrs
>be done equivalently by the initramfs' /init? Why is kernel involvement
>actually required here?
>
>Thanks,
> Dominik
There are a lot of things that could/should be done that way...
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Dominik Brodowski @ 2019-05-12 9:17 UTC (permalink / raw)
To: Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190509112420.15671-1-roberto.sassu@huawei.com>
On Thu, May 09, 2019 at 01:24:17PM +0200, Roberto Sassu wrote:
> This proposal consists in marshaling pathnames and xattrs in a file called
> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> been extracted.
Couldn't this parsing of the .xattr-list file and the setting of the xattrs
be done equivalently by the initramfs' /init? Why is kernel involvement
actually required here?
Thanks,
Dominik
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-12 4:12 UTC (permalink / raw)
To: Andy Lutomirski, Roberto Sassu
Cc: Al Viro, LSM List, linux-integrity, initramfs, Linux API,
Linux FS Devel, LKML, Mimi Zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, H. Peter Anvin,
Arnd Bergmann, james.w.mcmechan
In-Reply-To: <4aee6e10-0eec-1d76-af66-dc8c7b68b766@landley.net>
On 5/11/19 11:04 PM, Rob Landley wrote:
> P.P.S. Sadly, if you want an actually standardized standard format where
> implementations adhere to the standard: IETF RFC 1991 was published in 1996 and
Nope, darn it, checked my notes and that wasn't it. I thought zip had an RFC,
it's just zlib, deflate, and gzip, and that's not the number of any of them.
I still think sticking with a lightly modified cpio makes the most sense,
just... in band signalling that _doesn't_ solve the y2038 problem, the file size
limit, or address sparse files seems kinda silly.
Rob
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-12 4:04 UTC (permalink / raw)
To: Andy Lutomirski, Roberto Sassu
Cc: Al Viro, LSM List, linux-integrity, initramfs, Linux API,
Linux FS Devel, LKML, Mimi Zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, H. Peter Anvin,
Arnd Bergmann, james.w.mcmechan
In-Reply-To: <CALCETrXy7gqmmy37=nrMAisGadZ+qbjZjXtWFF8Crq86xNpsfA@mail.gmail.com>
On 5/11/19 5:44 PM, Andy Lutomirski wrote:
> I read some of those emails. ISTM that adding TAR support should be
> seriously considered. Sure, it's baroque, but it's very, very well
> supported, and it does exactly what we need.
Which means you now have two parsers supported in parallel forevermore, and are
reversing the design decision initially made when this went in without new info.
Also, I just did a tar implementation for toybox: It took me a month to debug it
(_not_ starting from scratch but from a submission), I only just added sparse
file support (because something in the android build was generating a sparse
file), there are historical tarballs I know it won't extract (I'm just testing
against what the current one produces with the default flags), and I haven't
even started on xattr support yet.
Instead I was experimenting with corner cases like "S records replace the
prefix[] field starting at byte 386 with an offset/length pair array, but
prefix[] starts at 345, do those first 41 bytes still function as a prefix and
is there any circumstance under which existing tar binaries will populate them?
Also, why does every instance of an S record generated by gnu/tar end with a
gratuitous length zero segment?"
"cpio -H newc" is a _much_ simpler format. And posix no longer specifies
_either_ format usefully, hasn't for years. From toybox tar's header comment:
* For the command, see
* http://pubs.opengroup.org/onlinepubs/007908799/xcu/tar.html
* For the modern file format, see
*
http://pubs.opengroup.org/onlinepubs/9699919799/utilities/pax.html#tag_20_92_13_06
* https://en.wikipedia.org/wiki/Tar_(computing)#File_format
* https://www.gnu.org/software/tar/manual/html_node/Tar-Internals.html
And no, that isn't _enough_ information, you still have to "tar | hd" a lot and
squint. (There's no current spec, it's pieced together from multiple sources
because posix abdicated responsibility for this to Jorg Schilling.)
Rob
P.S. Yes that gnu/dammit page starts with a "this will be deleted" comment which
according to archive.org has been there for over a dozen years.
P.P.S. Sadly, if you want an actually standardized standard format where
implementations adhere to the standard: IETF RFC 1991 was published in 1996 and
remains compatible with files an archivers in service. Or we could stick with
cpio and make minor changes to it, since we have to remain backwards compatible
with it _anyway_....
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Andy Lutomirski @ 2019-05-11 22:44 UTC (permalink / raw)
To: Roberto Sassu
Cc: Al Viro, LSM List, linux-integrity, initramfs, Linux API,
Linux FS Devel, LKML, Mimi Zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, H. Peter Anvin,
Arnd Bergmann, Rob Landley, james.w.mcmechan
In-Reply-To: <20190509112420.15671-1-roberto.sassu@huawei.com>
On Thu, May 9, 2019 at 4:27 AM Roberto Sassu <roberto.sassu@huawei.com> wrote:
>
> This patch set aims at solving the following use case: appraise files from
> the initial ram disk. To do that, IMA checks the signature/hash from the
> security.ima xattr. Unfortunately, this use case cannot be implemented
> currently, as the CPIO format does not support xattrs.
>
> This proposal consists in marshaling pathnames and xattrs in a file called
> .xattr-list. They are unmarshaled by the CPIO parser after all files have
> been extracted.
>
> The difference from v1 (https://lkml.org/lkml/2018/11/22/1182) is that all
> xattrs are stored in a single file and not per file (solves the file name
> limitation issue, as it is not necessary to add a suffix to files
> containing xattrs).
>
> The difference with another proposal
> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> included in an image without changing the image format, as opposed to
> defining a new one. As seen from the discussion, if a new format has to be
> defined, it should fix the issues of the existing format, which requires
> more time.
I read some of those emails. ISTM that adding TAR support should be
seriously considered. Sure, it's baroque, but it's very, very well
supported, and it does exactly what we need.
--Andy
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: Casey Schaufler @ 2019-05-11 22:08 UTC (permalink / raw)
To: Paul Moore, Linus Torvalds, James Morris
Cc: LSM List, Linux List Kernel Mailing, casey
In-Reply-To: <CAHC9VhSSwYk6isqz8N3nOO_O17C30E2EyCHKf5OqsdESeMoT7g@mail.gmail.com>
On 5/11/2019 11:13 AM, Paul Moore wrote:
> On Sat, May 11, 2019 at 10:38 AM Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
>> On Fri, May 10, 2019 at 6:09 PM James Morris <jmorris@namei.org> wrote:
>>> These patches include fixes to enable fuzz testing, and a fix for
>>> calculating whether a filesystem is user-modifiable.
>> So now these have been very recently rebased (on top of a random
>> merge-window "tree of the day" version) instead of having multiple
>> merges.
>>
>> That makes the history cleaner, but has its own issues.
>>
>> We really need to find a different model for the security layer patches.
> If it helps, the process I use for the SELinux and audit trees is
> documented below. While it's far from perfect (I still don't like
> basing the -next trees on -rcX releases) it has seemed to work
> reasonably well for some time now.
>
> * https://github.com/SELinuxProject/selinux-kernel/blob/master/README.md
On the whole this looks fine to me. I am less comfortable than Paul
is regarding changes that happen elsewhere, so I would be more likely
to base in the rc-1 than Paul. More developers test with SELinux than
Smack. I am in the process of putting an appropriate GPG environment
together for 5.3.
The LSM infrastructure work I've been doing should still go through
James, as it has global implications.
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: Paul Moore @ 2019-05-11 18:13 UTC (permalink / raw)
To: Linus Torvalds, James Morris; +Cc: LSM List, Linux List Kernel Mailing
In-Reply-To: <CAHk-=wg8UFHD_KmTWF3LMnDf_VN7cv_pofpc4eOHmx_8kmMPWw@mail.gmail.com>
On Sat, May 11, 2019 at 10:38 AM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Fri, May 10, 2019 at 6:09 PM James Morris <jmorris@namei.org> wrote:
> >
> > These patches include fixes to enable fuzz testing, and a fix for
> > calculating whether a filesystem is user-modifiable.
>
> So now these have been very recently rebased (on top of a random
> merge-window "tree of the day" version) instead of having multiple
> merges.
>
> That makes the history cleaner, but has its own issues.
>
> We really need to find a different model for the security layer patches.
If it helps, the process I use for the SELinux and audit trees is
documented below. While it's far from perfect (I still don't like
basing the -next trees on -rcX releases) it has seemed to work
reasonably well for some time now.
* https://github.com/SELinuxProject/selinux-kernel/blob/master/README.md
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: pr-tracker-bot @ 2019-05-11 15:00 UTC (permalink / raw)
To: James Morris; +Cc: Linus Torvalds, linux-security-module, linux-kernel
In-Reply-To: <alpine.LRH.2.21.1905110801350.9392@namei.org>
The pull request you sent on Sat, 11 May 2019 08:09:22 +1000 (AEST):
> git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c367dc8d0dd2a1e1ed9fdc2dd831053bdfdf0968
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: Linus Torvalds @ 2019-05-11 14:38 UTC (permalink / raw)
To: James Morris; +Cc: LSM List, Linux List Kernel Mailing
In-Reply-To: <alpine.LRH.2.21.1905110801350.9392@namei.org>
On Fri, May 10, 2019 at 6:09 PM James Morris <jmorris@namei.org> wrote:
>
> These patches include fixes to enable fuzz testing, and a fix for
> calculating whether a filesystem is user-modifiable.
So now these have been very recently rebased (on top of a random
merge-window "tree of the day" version) instead of having multiple
merges.
That makes the history cleaner, but has its own issues.
We really need to find a different model for the security layer patches.
Linus
^ permalink raw reply
* Re: [PATCH 3/4] gfp: mm: introduce __GFP_NOINIT
From: Souptick Joarder @ 2019-05-11 7:28 UTC (permalink / raw)
To: Alexander Potapenko
Cc: Kees Cook, Andrew Morton, Christoph Lameter, Laura Abbott,
Linux-MM, linux-security-module, Kernel Hardening,
Masahiro Yamada, James Morris, Serge E. Hallyn, Nick Desaulniers,
Kostya Serebryany, Dmitry Vyukov, Sandeep Patil, Randy Dunlap,
Jann Horn, Mark Rutland, Matthew Wilcox
In-Reply-To: <CAG_fn=VbJXHsqAeBD+g6zJ8WVTko4Ev2xytXrcJ-ztEWm7kOOA@mail.gmail.com>
On Thu, May 9, 2019 at 6:53 PM Alexander Potapenko <glider@google.com> wrote:
>
> From: Kees Cook <keescook@chromium.org>
> Date: Wed, May 8, 2019 at 9:16 PM
> To: Alexander Potapenko
> Cc: Andrew Morton, Christoph Lameter, Kees Cook, Laura Abbott,
> Linux-MM, linux-security-module, Kernel Hardening, Masahiro Yamada,
> James Morris, Serge E. Hallyn, Nick Desaulniers, Kostya Serebryany,
> Dmitry Vyukov, Sandeep Patil, Randy Dunlap, Jann Horn, Mark Rutland
>
> > On Wed, May 8, 2019 at 8:38 AM Alexander Potapenko <glider@google.com> wrote:
> > > When passed to an allocator (either pagealloc or SL[AOU]B), __GFP_NOINIT
> > > tells it to not initialize the requested memory if the init_on_alloc
> > > boot option is enabled. This can be useful in the cases newly allocated
> > > memory is going to be initialized by the caller right away.
> > >
> > > __GFP_NOINIT doesn't affect init_on_free behavior, except for SLOB,
> > > where init_on_free implies init_on_alloc.
> > >
> > > __GFP_NOINIT basically defeats the hardening against information leaks
> > > provided by init_on_alloc, so one should use it with caution.
> > >
> > > This patch also adds __GFP_NOINIT to alloc_pages() calls in SL[AOU]B.
> > > Doing so is safe, because the heap allocators initialize the pages they
> > > receive before passing memory to the callers.
> > >
> > > Slowdown for the initialization features compared to init_on_free=0,
> > > init_on_alloc=0:
> > >
> > > hackbench, init_on_free=1: +6.84% sys time (st.err 0.74%)
> > > hackbench, init_on_alloc=1: +7.25% sys time (st.err 0.72%)
> > >
> > > Linux build with -j12, init_on_free=1: +8.52% wall time (st.err 0.42%)
> > > Linux build with -j12, init_on_free=1: +24.31% sys time (st.err 0.47%)
> > > Linux build with -j12, init_on_alloc=1: -0.16% wall time (st.err 0.40%)
> > > Linux build with -j12, init_on_alloc=1: +1.24% sys time (st.err 0.39%)
> > >
> > > The slowdown for init_on_free=0, init_on_alloc=0 compared to the
> > > baseline is within the standard error.
> > >
Not sure, but I think this patch will clash with Matthew's posted patch series
*Remove 'order' argument from many mm functions*.
> > > Signed-off-by: Alexander Potapenko <glider@google.com>
> > > Cc: Andrew Morton <akpm@linux-foundation.org>
> > > Cc: Masahiro Yamada <yamada.masahiro@socionext.com>
> > > Cc: James Morris <jmorris@namei.org>
> > > Cc: "Serge E. Hallyn" <serge@hallyn.com>
> > > Cc: Nick Desaulniers <ndesaulniers@google.com>
> > > Cc: Kostya Serebryany <kcc@google.com>
> > > Cc: Dmitry Vyukov <dvyukov@google.com>
> > > Cc: Kees Cook <keescook@chromium.org>
> > > Cc: Sandeep Patil <sspatil@android.com>
> > > Cc: Laura Abbott <labbott@redhat.com>
> > > Cc: Randy Dunlap <rdunlap@infradead.org>
> > > Cc: Jann Horn <jannh@google.com>
> > > Cc: Mark Rutland <mark.rutland@arm.com>
> > > Cc: linux-mm@kvack.org
> > > Cc: linux-security-module@vger.kernel.org
> > > Cc: kernel-hardening@lists.openwall.com
> > > ---
> > > include/linux/gfp.h | 6 +++++-
> > > include/linux/mm.h | 2 +-
> > > kernel/kexec_core.c | 2 +-
> > > mm/slab.c | 2 +-
> > > mm/slob.c | 3 ++-
> > > mm/slub.c | 1 +
> > > 6 files changed, 11 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> > > index fdab7de7490d..66d7f5604fe2 100644
> > > --- a/include/linux/gfp.h
> > > +++ b/include/linux/gfp.h
> > > @@ -44,6 +44,7 @@ struct vm_area_struct;
> > > #else
> > > #define ___GFP_NOLOCKDEP 0
> > > #endif
> > > +#define ___GFP_NOINIT 0x1000000u
> >
> > I mentioned this in the other patch, but I think this needs to be
> > moved ahead of GFP_NOLOCKDEP and adjust the values for GFP_NOLOCKDEP
> > and to leave the IS_ENABLED() test in __GFP_BITS_SHIFT alone.
> Do we really need this blinking GFP_NOLOCKDEP bit at all?
> This approach doesn't scale, we can't even have a second feature that
> has a bit depending on the config settings.
> Cannot we just fix the number of bits instead?
>
> > > /* If the above are modified, __GFP_BITS_SHIFT may need updating */
> > >
> > > /*
> > > @@ -208,16 +209,19 @@ struct vm_area_struct;
> > > * %__GFP_COMP address compound page metadata.
> > > *
> > > * %__GFP_ZERO returns a zeroed page on success.
> > > + *
> > > + * %__GFP_NOINIT requests non-initialized memory from the underlying allocator.
> > > */
> > > #define __GFP_NOWARN ((__force gfp_t)___GFP_NOWARN)
> > > #define __GFP_COMP ((__force gfp_t)___GFP_COMP)
> > > #define __GFP_ZERO ((__force gfp_t)___GFP_ZERO)
> > > +#define __GFP_NOINIT ((__force gfp_t)___GFP_NOINIT)
> > >
> > > /* Disable lockdep for GFP context tracking */
> > > #define __GFP_NOLOCKDEP ((__force gfp_t)___GFP_NOLOCKDEP)
> > >
> > > /* Room for N __GFP_FOO bits */
> > > -#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP))
> > > +#define __GFP_BITS_SHIFT (25)
> >
> > AIUI, this will break non-CONFIG_LOCKDEP kernels: it should just be:
> >
> > -#define __GFP_BITS_SHIFT (23 + IS_ENABLED(CONFIG_LOCKDEP))
> > +#define __GFP_BITS_SHIFT (24 + IS_ENABLED(CONFIG_LOCKDEP))
> >
> > > #define __GFP_BITS_MASK ((__force gfp_t)((1 << __GFP_BITS_SHIFT) - 1))
> > >
> > > /**
> > > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > > index ee1a1092679c..8ab152750eb4 100644
> > > --- a/include/linux/mm.h
> > > +++ b/include/linux/mm.h
> > > @@ -2618,7 +2618,7 @@ DECLARE_STATIC_KEY_FALSE(init_on_alloc);
> > > static inline bool want_init_on_alloc(gfp_t flags)
> > > {
> > > if (static_branch_unlikely(&init_on_alloc))
> > > - return true;
> > > + return !(flags & __GFP_NOINIT);
> > > return flags & __GFP_ZERO;
> >
> > What do you think about renaming __GFP_NOINIT to __GFP_NO_AUTOINIT or something?
> >
> > Regardless, yes, this is nice.
> >
> > --
> > Kees Cook
>
>
>
> --
> Alexander Potapenko
> Software Engineer
>
> Google Germany GmbH
> Erika-Mann-Straße, 33
> 80636 München
>
> Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
> Registergericht und -nummer: Hamburg, HRB 86891
> Sitz der Gesellschaft: Hamburg
>
^ permalink raw reply
* Carrying over the ima log during kexec_file_load
From: prakhar srivastava @ 2019-05-11 0:16 UTC (permalink / raw)
To: linux-kernel, linux-integrity, linux-security-module, kexec
Cc: jmorris, Mimi Zohar, dyoung, bhe, vgoyal
Hi,
I am currently looking at carrying over the ima log from the current
kernel to the next kernel during soft reboot(kexec_file_load) for arm64
and x86_64.
During soft reboot(kexec_file_load) TPM boot PCR’s(PCRs 0 through 7)
are not reset or extended and thus the boot aggregate does not change,
leaving the new kernel with a sense of secure boot.
During kexec_file_load the kernel file signature is validated through PE
file signature validation.
The boot cmdline args will also be measured with
“kexec cmdline buffer measure” change which is in progress.
https://lkml.org/lkml/2019/5/10/728
Looking at the powerpc implementation of kexec_file_load, making
change to the kimage_arch as below seems most reasonable.
Struct kimage_arch {
…
ima_log_buffer
ima_log_buffer_size
};
Add respective entries in dtb/fdt and read the same in the next kernel.
No changes to the purgatory should be needed since no kernel segments
are changed.
Is anyone already looking at this?
If not, I want to understand what’s the best approach for this is?
Thanks,
Prakhar Srivastava
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-10 22:38 UTC (permalink / raw)
To: Rob Landley, Roberto Sassu, viro
Cc: linux-security-module, linux-integrity, initramfs, linux-api,
linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan
In-Reply-To: <3a9d717e-0e12-9d62-a3cf-afb7a5dbf166@landley.net>
On Fri, 2019-05-10 at 15:46 -0500, Rob Landley wrote:
> On 5/10/19 6:49 AM, Mimi Zohar wrote:
> > On Fri, 2019-05-10 at 08:56 +0200, Roberto Sassu wrote:
> >> On 5/9/2019 8:34 PM, Rob Landley wrote:
> >>> On 5/9/19 6:24 AM, Roberto Sassu wrote:
> >
> >>>> The difference with another proposal
> >>>> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
> >>>> included in an image without changing the image format, as opposed to
> >>>> defining a new one. As seen from the discussion, if a new format has to be
> >>>> defined, it should fix the issues of the existing format, which requires
> >>>> more time.
> >>>
> >>> So you've explicitly chosen _not_ to address Y2038 while you're there.
> >>
> >> Can you be more specific?
> >
> > Right, this patch set avoids incrementing the CPIO magic number and
> > the resulting changes required (eg. increasing the timestamp field
> > size), by including a file with the security xattrs in the CPIO. In
> > either case, including the security xattrs in the initramfs header or
> > as a separate file, the initramfs, itself, needs to be signed.
>
> The /init binary in the initramfs runs as root and launches all other processes
> on the system. Presumably it can write any xattrs it wants to, and doesn't need
> any extra permissions granted to it to do so. But as soon as you start putting
> xattrs on _other_ files within the initramfs that are _not_ necessarily running
> as PID 1, _that's_ when the need to sign the initramfs comes in?
>
> Presumably the signing occurs on the gzipped file. How does that affect the cpio
> parsing _after_ it's decompressed? Why would that be part of _this_ patch?
The signing and verification of the initramfs is a separate issue, not
part of this patch set. The only reason for mentioning it here was to
say that both methods of including the security xattrs require the
initramfs be signed. Just as the kernel image needs to be signed and
verified, the initramfs should be too.
Mimi
^ permalink raw reply
* [GIT PULL] security subsystem: Tomoyo updates for v5.2
From: James Morris @ 2019-05-10 22:09 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-security-module, linux-kernel
Please pull.
These patches include fixes to enable fuzz testing, and a fix for
calculating whether a filesystem is user-modifiable.
The following changes since commit 1fb3b526df3bd7647e7854915ae6b22299408baf:
Merge tag 'docs-5.2a' of git://git.lwn.net/linux (2019-05-10 13:24:53 -0400)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security.git next-tomoyo2
for you to fetch changes up to 4ad98ac46490d5f8441025930070eaf028cfd0f2:
tomoyo: Don't emit WARNING: string while fuzzing testing. (2019-05-10 14:58:35 -0700)
----------------------------------------------------------------
Tetsuo Handa (4):
tomoyo: Add a kernel config option for fuzzing testing.
tomoyo: Check address length before reading address family
tomoyo: Change pathname calculation for read-only filesystems.
tomoyo: Don't emit WARNING: string while fuzzing testing.
security/tomoyo/Kconfig | 10 ++++++++++
security/tomoyo/common.c | 13 ++++++++++++-
security/tomoyo/network.c | 4 ++++
security/tomoyo/realpath.c | 3 ++-
security/tomoyo/util.c | 2 ++
5 files changed, 30 insertions(+), 2 deletions(-)
^ permalink raw reply
* Re: [PATCH v2 3/3] initramfs: introduce do_readxattrs()
From: Jann Horn @ 2019-05-10 21:33 UTC (permalink / raw)
To: Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190509112420.15671-4-roberto.sassu@huawei.com>
On Thu, May 09, 2019 at 01:24:20PM +0200, Roberto Sassu wrote:
> This patch adds support for an alternative method to add xattrs to files in
> the rootfs filesystem. Instead of extracting them directly from the ram
> disk image, they are extracted from a regular file called .xattr-list, that
> can be added by any ram disk generator available today.
[...]
> +struct path_hdr {
> + char p_size[10]; /* total size including p_size field */
> + char p_data[]; /* <path>\0<xattrs> */
> +};
> +
> +static int __init do_readxattrs(void)
> +{
> + struct path_hdr hdr;
> + char str[sizeof(hdr.p_size) + 1];
> + unsigned long file_entry_size;
> + size_t size, name_buf_size, total_size;
> + struct kstat st;
> + int ret, fd;
> +
> + ret = vfs_lstat(XATTR_LIST_FILENAME, &st);
> + if (ret < 0)
> + return ret;
> +
> + total_size = st.size;
> +
> + fd = ksys_open(XATTR_LIST_FILENAME, O_RDONLY, 0);
> + if (fd < 0)
> + return fd;
> +
> + while (total_size) {
> + size = ksys_read(fd, (char *)&hdr, sizeof(hdr));
[...]
> + ksys_close(fd);
> +
> + if (ret < 0)
> + error("Unable to parse xattrs");
> +
> + return ret;
> +}
Please use something like filp_open()+kernel_read()+fput() instead of
ksys_open()+ksys_read()+ksys_close(). I understand that some of the init
code needs to use the syscall wrappers because no equivalent VFS
functions are available, but please use the VFS functions when that's
easy to do.
^ permalink raw reply
* Re: [PATCH v2 1/3] fs: add ksys_lsetxattr() wrapper
From: Jann Horn @ 2019-05-10 21:28 UTC (permalink / raw)
To: Roberto Sassu
Cc: viro, linux-security-module, linux-integrity, initramfs,
linux-api, linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, rob,
james.w.mcmechan
In-Reply-To: <20190509112420.15671-2-roberto.sassu@huawei.com>
On Thu, May 09, 2019 at 01:24:18PM +0200, Roberto Sassu wrote:
> Similarly to commit 03450e271a16 ("fs: add ksys_fchmod() and do_fchmodat()
> helpers and ksys_chmod() wrapper; remove in-kernel calls to syscall"), this
> patch introduces the ksys_lsetxattr() helper to avoid in-kernel calls to
> the sys_lsetxattr() syscall.
>
> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
[...]
> +int ksys_lsetxattr(const char __user *pathname,
> + const char __user *name, const void __user *value,
> + size_t size, int flags)
> +{
> + return path_setxattr(pathname, name, value, size, flags, 0);
> +}
Instead of exposing ksys_lsetxattr(), wouldn't it be cleaner to use
kern_path() and vfs_setxattr(), or something like that? Otherwise you're
adding more code that has to cast between kernel and user pointers.
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-10 20:46 UTC (permalink / raw)
To: Mimi Zohar, Roberto Sassu, viro
Cc: linux-security-module, linux-integrity, initramfs, linux-api,
linux-fsdevel, linux-kernel, zohar, silviu.vlasceanu,
dmitry.kasatkin, takondra, kamensky, hpa, arnd, james.w.mcmechan
In-Reply-To: <1557488971.10635.102.camel@linux.ibm.com>
On 5/10/19 6:49 AM, Mimi Zohar wrote:
> On Fri, 2019-05-10 at 08:56 +0200, Roberto Sassu wrote:
>> On 5/9/2019 8:34 PM, Rob Landley wrote:
>>> On 5/9/19 6:24 AM, Roberto Sassu wrote:
>
>>>> The difference with another proposal
>>>> (https://lore.kernel.org/patchwork/cover/888071/) is that xattrs can be
>>>> included in an image without changing the image format, as opposed to
>>>> defining a new one. As seen from the discussion, if a new format has to be
>>>> defined, it should fix the issues of the existing format, which requires
>>>> more time.
>>>
>>> So you've explicitly chosen _not_ to address Y2038 while you're there.
>>
>> Can you be more specific?
>
> Right, this patch set avoids incrementing the CPIO magic number and
> the resulting changes required (eg. increasing the timestamp field
> size), by including a file with the security xattrs in the CPIO. In
> either case, including the security xattrs in the initramfs header or
> as a separate file, the initramfs, itself, needs to be signed.
The /init binary in the initramfs runs as root and launches all other processes
on the system. Presumably it can write any xattrs it wants to, and doesn't need
any extra permissions granted to it to do so. But as soon as you start putting
xattrs on _other_ files within the initramfs that are _not_ necessarily running
as PID 1, _that's_ when the need to sign the initramfs comes in?
Presumably the signing occurs on the gzipped file. How does that affect the cpio
parsing _after_ it's decompressed? Why would that be part of _this_ patch?
Rob
^ permalink raw reply
* Re: [GIT PULL] Security subsystem: Smack updates for v5.2
From: Linus Torvalds @ 2019-05-10 16:54 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: LSM List
In-Reply-To: <f7420347-a926-e923-9914-714ead9298ec@I-love.SAKURA.ne.jp>
On Fri, May 10, 2019 at 3:50 AM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> but I don't maintain a git tree for sending pull requests.
Any chance you would do so?
Anyway, I am not suggesting we change anything this merge window,
that's just too painful and would be too chaotic. But I'd love for
this to be sorted out by the next one, perhaps?
Linus
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox