* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-13 18:36 UTC (permalink / raw)
To: Arvind Sankar
Cc: Roberto Sassu, Rob Landley, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190513175250.GC69717@rani.riverdale.lan>
> > How does this work today then? Is it actually the case that initramfs
> > just cannot be used on an IMA-enabled system, or it can but it leaves
> > the initramfs unverified and we're trying to fix that? I had assumed the
> > latter.
> Oooh, it's done not by starting IMA appraisal later, but by loading a
> default policy to ignore initramfs?
Right, when rootfs is a tmpfs filesystem, it supports xattrs, allowing
for finer grained policies to be defined. This patch set would allow
a builtin IMA appraise policy to be defined which includes tmpfs.
Mimi
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-13 17:52 UTC (permalink / raw)
To: Arvind Sankar
Cc: Roberto Sassu, Rob Landley, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190513172007.GA69717@rani.riverdale.lan>
On Mon, May 13, 2019 at 01:20:08PM -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > >
> > >
> > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > >>>> 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?
> > >>>>
> > >>>> Ro
> > >>>
> > >>> Wouldn't the below work even before enforcing signatures on external
> > >>> initramfs:
> > >>> 1. Create an embedded initramfs with an /init that does the xattr
> > >>> parsing/setting. This will be verified as part of the kernel image
> > >>> signature, so no new code required.
> > >>> 2. Add a config option/boot parameter to panic the kernel if an external
> > >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> > >>> prevents overwriting the embedded /init even if the external initramfs
> > >>> is unverified.
> > >>
> > >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> > >> verify /init in the embedded initial ram disk.
> How does this work today then? Is it actually the case that initramfs
> just cannot be used on an IMA-enabled system, or it can but it leaves
> the initramfs unverified and we're trying to fix that? I had assumed the
> latter.
Oooh, it's done not by starting IMA appraisal later, but by loading a
default policy to ignore initramfs?
> > >
> > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> >
> > The idea is to be able to verify anything that is accessed, as soon as
> > rootfs is available, without distinction between embedded or external
> > initial ram disk.
> >
> > Also, requiring an embedded initramfs for xattrs would be an issue for
> > systems that use it for other purposes.
> >
> The embedded initramfs can do other things, it just has to do
> the xattr stuff in addition, no?
> >
> > >> The only reason why
> > >> opening .xattr-list works is that IMA is not yet initialized
> > >> (late_initcall vs rootfs_initcall).
> > >
> > > Launching init before enabling ima is bad because... you didn't think of it?
> >
> > No, because /init can potentially compromise the integrity of the
> > system.
> >
> How? The /init in the embedded initramfs is part of a trusted kernel
> image that has been verified by the bootloader.
> >
> > >> Allowing a kernel with integrity enforcement to parse the CPIO image
> > >> without verifying it first is the weak point.
> > >
> > > If you don't verify the CPIO image then in theory it could have anything in it,
> > > yes. You seem to believe that signing individual files is more secure than
> > > signing the archive. This is certainly a point of view.
> >
> > As I wrote above, signing the CPIO image would be more secure, if this
> > option is available. However, a disadvantage would be that you have to
> > sign the CPIO image every time a file changes.
> >
> >
> > >> However, extracted files
> > >> are not used, and before they are used they are verified. At the time
> > >> they are verified, they (included /init) must already have a signature
> > >> or otherwise access would be denied.
> > >
> > > You build infrastructure that works a certain way, the rest of the system
> > > doesn't fit your assumptions, so you need to change the rest of the system to
> > > fit your assumptions.
> >
> > Requiring file metadata to make decisions seems reasonable. Also
> > mandatory access controls do that. The objective of this patch set is to
> > have uniform behavior regardless of the filesystem used.
> >
> >
> > >> This scheme relies on the ability of the kernel to not be corrupted in
> > >> the event it parses a malformed CPIO image.
> > >
> > > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > > extraction code. You can fill up all physical memory with initramfs and lock the
> > > system hard, though.
> > >
> > > It still only parses them at boot time before launching PID 1, right? So you
> > > have a local physical exploit and you're trying to prevent people from working
> > > around your Xbox copy protection without a mod chip?
> >
> > What do you mean exactly?
> >
> >
> > >> Mimi suggested to use
> > >> digital signatures to prevent this issue, but it cannot be used in all
> > >> scenarios, since conventional systems generate the initial ram disk
> > >> locally.
> > >
> > > So you use a proprietary init binary you can't rebuild from source, and put it
> > > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > > clearly keeping the kernel up to date is less important to security...)
> >
> > By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> > as the image would be rejected by the boot loader if the signature is
> > invalid.
> >
> If it were signed yes, but you just said that it isn't possible to sign
> it in all cases (if initramfs is generated locally). I actually didn't
> follow that bit -- if initramfs is generated locally, and it isn't
> possible to sign locally, where would the IMA hashes for the contents of
> the initramfs come from? Is the idea that each file within the initramfs
> would be an existing, signed, file, but you could locally create an initramfs
> with some subset of those unmodified files? Even assuming this is the
> case, isn't the eventual intention to also appraise directories, to
> prevent holes where files might be moved around/deleted/renamed etc, so
> this problem would resurface anyway?
> Also eventually we need to check special nodes like device nodes etc to
> make sure they haven't been tampered with, as in Rob's urandom
> suggestion?
> >
> > > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > > in-band in the file, but you need xattrs for some reason?
> >
> > Appending just the signature would be possible. It won't work if you
> > have multiple metadata for the same file.
> >
> > Also appending the signature alone won't solve the parsing issue. Still,
> > the kernel has to parse something that could be malformed.
> >
> > Roberto
> >
> >
> > >> Roberto
> > >
> > > Rob
> > >
> >
> > --
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-13 17:51 UTC (permalink / raw)
To: Arvind Sankar
Cc: Roberto Sassu, Rob Landley, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <20190513172007.GA69717@rani.riverdale.lan>
On Mon, May 13, 2019 at 01:20:08PM -0400, Arvind Sankar wrote:
> On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> > On 5/13/2019 11:07 AM, Rob Landley wrote:
> > >
> > >
> > > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> > >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> > >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> > >>>> 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?
> > >>>>
> > >>>> Ro
> > >>>
> > >>> Wouldn't the below work even before enforcing signatures on external
> > >>> initramfs:
> > >>> 1. Create an embedded initramfs with an /init that does the xattr
> > >>> parsing/setting. This will be verified as part of the kernel image
> > >>> signature, so no new code required.
> > >>> 2. Add a config option/boot parameter to panic the kernel if an external
> > >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> > >>> prevents overwriting the embedded /init even if the external initramfs
> > >>> is unverified.
> > >>
> > >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> > >> verify /init in the embedded initial ram disk.
> How does this work today then? Is it actually the case that initramfs
> just cannot be used on an IMA-enabled system, or it can but it leaves
> the initramfs unverified and we're trying to fix that? I had assumed the
> latter.
Oooh, it's done not by starting IMA later, but by loading a default
policy that ignores the initramfs?
> > >
> > > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> >
> > The idea is to be able to verify anything that is accessed, as soon as
> > rootfs is available, without distinction between embedded or external
> > initial ram disk.
> >
> > Also, requiring an embedded initramfs for xattrs would be an issue for
> > systems that use it for other purposes.
> >
> The embedded initramfs can do other things, it just has to do
> the xattr stuff in addition, no?
> >
> > >> The only reason why
> > >> opening .xattr-list works is that IMA is not yet initialized
> > >> (late_initcall vs rootfs_initcall).
> > >
> > > Launching init before enabling ima is bad because... you didn't think of it?
> >
> > No, because /init can potentially compromise the integrity of the
> > system.
> >
> How? The /init in the embedded initramfs is part of a trusted kernel
> image that has been verified by the bootloader.
> >
> > >> Allowing a kernel with integrity enforcement to parse the CPIO image
> > >> without verifying it first is the weak point.
> > >
> > > If you don't verify the CPIO image then in theory it could have anything in it,
> > > yes. You seem to believe that signing individual files is more secure than
> > > signing the archive. This is certainly a point of view.
> >
> > As I wrote above, signing the CPIO image would be more secure, if this
> > option is available. However, a disadvantage would be that you have to
> > sign the CPIO image every time a file changes.
> >
> >
> > >> However, extracted files
> > >> are not used, and before they are used they are verified. At the time
> > >> they are verified, they (included /init) must already have a signature
> > >> or otherwise access would be denied.
> > >
> > > You build infrastructure that works a certain way, the rest of the system
> > > doesn't fit your assumptions, so you need to change the rest of the system to
> > > fit your assumptions.
> >
> > Requiring file metadata to make decisions seems reasonable. Also
> > mandatory access controls do that. The objective of this patch set is to
> > have uniform behavior regardless of the filesystem used.
> >
> >
> > >> This scheme relies on the ability of the kernel to not be corrupted in
> > >> the event it parses a malformed CPIO image.
> > >
> > > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > > extraction code. You can fill up all physical memory with initramfs and lock the
> > > system hard, though.
> > >
> > > It still only parses them at boot time before launching PID 1, right? So you
> > > have a local physical exploit and you're trying to prevent people from working
> > > around your Xbox copy protection without a mod chip?
> >
> > What do you mean exactly?
> >
> >
> > >> Mimi suggested to use
> > >> digital signatures to prevent this issue, but it cannot be used in all
> > >> scenarios, since conventional systems generate the initial ram disk
> > >> locally.
> > >
> > > So you use a proprietary init binary you can't rebuild from source, and put it
> > > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > > clearly keeping the kernel up to date is less important to security...)
> >
> > By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> > as the image would be rejected by the boot loader if the signature is
> > invalid.
> >
> If it were signed yes, but you just said that it isn't possible to sign
> it in all cases (if initramfs is generated locally). I actually didn't
> follow that bit -- if initramfs is generated locally, and it isn't
> possible to sign locally, where would the IMA hashes for the contents of
> the initramfs come from? Is the idea that each file within the initramfs
> would be an existing, signed, file, but you could locally create an initramfs
> with some subset of those unmodified files? Even assuming this is the
> case, isn't the eventual intention to also appraise directories, to
> prevent holes where files might be moved around/deleted/renamed etc, so
> this problem would resurface anyway?
> Also eventually we need to check special nodes like device nodes etc to
> make sure they haven't been tampered with, as in Rob's urandom
> suggestion?
> >
> > > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > > in-band in the file, but you need xattrs for some reason?
> >
> > Appending just the signature would be possible. It won't work if you
> > have multiple metadata for the same file.
> >
> > Also appending the signature alone won't solve the parsing issue. Still,
> > the kernel has to parse something that could be malformed.
> >
> > Roberto
> >
> >
> > >> Roberto
> > >
> > > Rob
> > >
> >
> > --
> > HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> > Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-13 17:20 UTC (permalink / raw)
To: Roberto Sassu
Cc: Rob Landley, Arvind Sankar, linux-kernel, linux-api,
linux-fsdevel, linux-integrity, initramfs
In-Reply-To: <f7bc547c-61f4-1a17-735c-7e8df97d7965@huawei.com>
On Mon, May 13, 2019 at 02:47:04PM +0200, Roberto Sassu wrote:
> On 5/13/2019 11:07 AM, Rob Landley wrote:
> >
> >
> > On 5/13/19 2:49 AM, Roberto Sassu wrote:
> >> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> >>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> >>>> 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?
> >>>>
> >>>> Ro
> >>>
> >>> Wouldn't the below work even before enforcing signatures on external
> >>> initramfs:
> >>> 1. Create an embedded initramfs with an /init that does the xattr
> >>> parsing/setting. This will be verified as part of the kernel image
> >>> signature, so no new code required.
> >>> 2. Add a config option/boot parameter to panic the kernel if an external
> >>> initramfs attempts to overwrite anything in the embedded initramfs. This
> >>> prevents overwriting the embedded /init even if the external initramfs
> >>> is unverified.
> >>
> >> Unfortunately, it wouldn't work. IMA is already initialized and it would
> >> verify /init in the embedded initial ram disk.
How does this work today then? Is it actually the case that initramfs
just cannot be used on an IMA-enabled system, or it can but it leaves
the initramfs unverified and we're trying to fix that? I had assumed the
latter.
> >
> > So you made broken infrastructure that's causing you problems. Sounds unfortunate.
>
> The idea is to be able to verify anything that is accessed, as soon as
> rootfs is available, without distinction between embedded or external
> initial ram disk.
>
> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.
>
The embedded initramfs can do other things, it just has to do
the xattr stuff in addition, no?
>
> >> The only reason why
> >> opening .xattr-list works is that IMA is not yet initialized
> >> (late_initcall vs rootfs_initcall).
> >
> > Launching init before enabling ima is bad because... you didn't think of it?
>
> No, because /init can potentially compromise the integrity of the
> system.
>
How? The /init in the embedded initramfs is part of a trusted kernel
image that has been verified by the bootloader.
>
> >> Allowing a kernel with integrity enforcement to parse the CPIO image
> >> without verifying it first is the weak point.
> >
> > If you don't verify the CPIO image then in theory it could have anything in it,
> > yes. You seem to believe that signing individual files is more secure than
> > signing the archive. This is certainly a point of view.
>
> As I wrote above, signing the CPIO image would be more secure, if this
> option is available. However, a disadvantage would be that you have to
> sign the CPIO image every time a file changes.
>
>
> >> However, extracted files
> >> are not used, and before they are used they are verified. At the time
> >> they are verified, they (included /init) must already have a signature
> >> or otherwise access would be denied.
> >
> > You build infrastructure that works a certain way, the rest of the system
> > doesn't fit your assumptions, so you need to change the rest of the system to
> > fit your assumptions.
>
> Requiring file metadata to make decisions seems reasonable. Also
> mandatory access controls do that. The objective of this patch set is to
> have uniform behavior regardless of the filesystem used.
>
>
> >> This scheme relies on the ability of the kernel to not be corrupted in
> >> the event it parses a malformed CPIO image.
> >
> > I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> > extraction code. You can fill up all physical memory with initramfs and lock the
> > system hard, though.
> >
> > It still only parses them at boot time before launching PID 1, right? So you
> > have a local physical exploit and you're trying to prevent people from working
> > around your Xbox copy protection without a mod chip?
>
> What do you mean exactly?
>
>
> >> Mimi suggested to use
> >> digital signatures to prevent this issue, but it cannot be used in all
> >> scenarios, since conventional systems generate the initial ram disk
> >> locally.
> >
> > So you use a proprietary init binary you can't rebuild from source, and put it
> > in a cpio where /dev/urandom is a file with known contents? Clearly, not
> > exploitable at all. (And we update the initramfs.cpio but not the kernel because
> > clearly keeping the kernel up to date is less important to security...)
>
> By signing the CPIO image, the kernel wouldn't even attempt to parse it,
> as the image would be rejected by the boot loader if the signature is
> invalid.
>
If it were signed yes, but you just said that it isn't possible to sign
it in all cases (if initramfs is generated locally). I actually didn't
follow that bit -- if initramfs is generated locally, and it isn't
possible to sign locally, where would the IMA hashes for the contents of
the initramfs come from? Is the idea that each file within the initramfs
would be an existing, signed, file, but you could locally create an initramfs
with some subset of those unmodified files? Even assuming this is the
case, isn't the eventual intention to also appraise directories, to
prevent holes where files might be moved around/deleted/renamed etc, so
this problem would resurface anyway?
Also eventually we need to check special nodes like device nodes etc to
make sure they haven't been tampered with, as in Rob's urandom
suggestion?
>
> > Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> > in-band in the file, but you need xattrs for some reason?
>
> Appending just the signature would be possible. It won't work if you
> have multiple metadata for the same file.
>
> Also appending the signature alone won't solve the parsing issue. Still,
> the kernel has to parse something that could be malformed.
>
> Roberto
>
>
> >> Roberto
> >
> > Rob
> >
>
> --
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 3/3] initramfs: introduce do_readxattrs()
From: Roberto Sassu @ 2019-05-13 13:03 UTC (permalink / raw)
To: Jann Horn
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: <20190510213340.GE253532@google.com>
On 5/10/2019 11:33 PM, Jann Horn wrote:
> 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.
Ok. Thanks for the suggestion.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-13 12:47 UTC (permalink / raw)
To: Rob Landley, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <4f522e28-29c8-5930-5d90-e0086b503613@landley.net>
On 5/13/2019 11:07 AM, Rob Landley wrote:
>
>
> On 5/13/19 2:49 AM, Roberto Sassu wrote:
>> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>>> 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?
>>>>
>>>> Ro
>>>
>>> Wouldn't the below work even before enforcing signatures on external
>>> initramfs:
>>> 1. Create an embedded initramfs with an /init that does the xattr
>>> parsing/setting. This will be verified as part of the kernel image
>>> signature, so no new code required.
>>> 2. Add a config option/boot parameter to panic the kernel if an external
>>> initramfs attempts to overwrite anything in the embedded initramfs. This
>>> prevents overwriting the embedded /init even if the external initramfs
>>> is unverified.
>>
>> Unfortunately, it wouldn't work. IMA is already initialized and it would
>> verify /init in the embedded initial ram disk.
>
> So you made broken infrastructure that's causing you problems. Sounds unfortunate.
The idea is to be able to verify anything that is accessed, as soon as
rootfs is available, without distinction between embedded or external
initial ram disk.
Also, requiring an embedded initramfs for xattrs would be an issue for
systems that use it for other purposes.
>> The only reason why
>> opening .xattr-list works is that IMA is not yet initialized
>> (late_initcall vs rootfs_initcall).
>
> Launching init before enabling ima is bad because... you didn't think of it?
No, because /init can potentially compromise the integrity of the
system.
>> Allowing a kernel with integrity enforcement to parse the CPIO image
>> without verifying it first is the weak point.
>
> If you don't verify the CPIO image then in theory it could have anything in it,
> yes. You seem to believe that signing individual files is more secure than
> signing the archive. This is certainly a point of view.
As I wrote above, signing the CPIO image would be more secure, if this
option is available. However, a disadvantage would be that you have to
sign the CPIO image every time a file changes.
>> However, extracted files
>> are not used, and before they are used they are verified. At the time
>> they are verified, they (included /init) must already have a signature
>> or otherwise access would be denied.
>
> You build infrastructure that works a certain way, the rest of the system
> doesn't fit your assumptions, so you need to change the rest of the system to
> fit your assumptions.
Requiring file metadata to make decisions seems reasonable. Also
mandatory access controls do that. The objective of this patch set is to
have uniform behavior regardless of the filesystem used.
>> This scheme relies on the ability of the kernel to not be corrupted in
>> the event it parses a malformed CPIO image.
>
> I'm unaware of any buffer overruns or wild pointer traversals in the cpio
> extraction code. You can fill up all physical memory with initramfs and lock the
> system hard, though.
>
> It still only parses them at boot time before launching PID 1, right? So you
> have a local physical exploit and you're trying to prevent people from working
> around your Xbox copy protection without a mod chip?
What do you mean exactly?
>> Mimi suggested to use
>> digital signatures to prevent this issue, but it cannot be used in all
>> scenarios, since conventional systems generate the initial ram disk
>> locally.
>
> So you use a proprietary init binary you can't rebuild from source, and put it
> in a cpio where /dev/urandom is a file with known contents? Clearly, not
> exploitable at all. (And we update the initramfs.cpio but not the kernel because
> clearly keeping the kernel up to date is less important to security...)
By signing the CPIO image, the kernel wouldn't even attempt to parse it,
as the image would be rejected by the boot loader if the signature is
invalid.
> Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
> in-band in the file, but you need xattrs for some reason?
Appending just the signature would be possible. It won't work if you
have multiple metadata for the same file.
Also appending the signature alone won't solve the parsing issue. Still,
the kernel has to parse something that could be malformed.
Roberto
>> Roberto
>
> Rob
>
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Mimi Zohar @ 2019-05-13 12:08 UTC (permalink / raw)
To: Rob Landley, Roberto Sassu, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <4f522e28-29c8-5930-5d90-e0086b503613@landley.net>
On Mon, 2019-05-13 at 04:07 -0500, Rob Landley wrote:
> > Allowing a kernel with integrity enforcement to parse the CPIO image
> > without verifying it first is the weak point.
>
> If you don't verify the CPIO image then in theory it could have anything in it,
> yes. You seem to believe that signing individual files is more secure than
> signing the archive. This is certainly a point of view.
Nobody is claiming that signing and verifying individual files is more
secure. We are saying that in some environments BOTH are needed. In
many environments today the initramfs IS being signed and verified.
Unfortunately not all environments can sign the initramfs today,
because the initramfs is not distributed with the kernel image, but
generated on the target system.
Mimi
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Rob Landley @ 2019-05-13 9:07 UTC (permalink / raw)
To: Roberto Sassu, Arvind Sankar
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <3fe0e74b-19ca-6081-3afe-e05921b1bfe6@huawei.com>
On 5/13/19 2:49 AM, Roberto Sassu wrote:
> On 5/12/2019 9:43 PM, Arvind Sankar wrote:
>> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>>> 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?
>>>
>>> Ro
>>
>> Wouldn't the below work even before enforcing signatures on external
>> initramfs:
>> 1. Create an embedded initramfs with an /init that does the xattr
>> parsing/setting. This will be verified as part of the kernel image
>> signature, so no new code required.
>> 2. Add a config option/boot parameter to panic the kernel if an external
>> initramfs attempts to overwrite anything in the embedded initramfs. This
>> prevents overwriting the embedded /init even if the external initramfs
>> is unverified.
>
> Unfortunately, it wouldn't work. IMA is already initialized and it would
> verify /init in the embedded initial ram disk.
So you made broken infrastructure that's causing you problems. Sounds unfortunate.
> The only reason why
> opening .xattr-list works is that IMA is not yet initialized
> (late_initcall vs rootfs_initcall).
Launching init before enabling ima is bad because... you didn't think of it?
> Allowing a kernel with integrity enforcement to parse the CPIO image
> without verifying it first is the weak point.
If you don't verify the CPIO image then in theory it could have anything in it,
yes. You seem to believe that signing individual files is more secure than
signing the archive. This is certainly a point of view.
> However, extracted files
> are not used, and before they are used they are verified. At the time
> they are verified, they (included /init) must already have a signature
> or otherwise access would be denied.
You build infrastructure that works a certain way, the rest of the system
doesn't fit your assumptions, so you need to change the rest of the system to
fit your assumptions.
> This scheme relies on the ability of the kernel to not be corrupted in
> the event it parses a malformed CPIO image.
I'm unaware of any buffer overruns or wild pointer traversals in the cpio
extraction code. You can fill up all physical memory with initramfs and lock the
system hard, though.
It still only parses them at boot time before launching PID 1, right? So you
have a local physical exploit and you're trying to prevent people from working
around your Xbox copy protection without a mod chip?
> Mimi suggested to use
> digital signatures to prevent this issue, but it cannot be used in all
> scenarios, since conventional systems generate the initial ram disk
> locally.
So you use a proprietary init binary you can't rebuild from source, and put it
in a cpio where /dev/urandom is a file with known contents? Clearly, not
exploitable at all. (And we update the initramfs.cpio but not the kernel because
clearly keeping the kernel up to date is less important to security...)
Whatever happened to https://lwn.net/Articles/532778/ ? Modules are signed
in-band in the file, but you need xattrs for some reason?
> Roberto
Rob
^ permalink raw reply
* Re: [PATCH v9 00/24] ILP32 for ARM64
From: Andreas Schwab @ 2019-05-13 8:48 UTC (permalink / raw)
To: Yury Norov
Cc: Yury Norov, Catalin Marinas, Arnd Bergmann, linux-arm-kernel,
linux-kernel, linux-doc, linux-arch, linux-api, Adam Borowski,
Alexander Graf, Alexey Klimov, Andrew Pinski, Bamvor Zhangjian,
Chris Metcalf, Christoph Muellner, Dave Martin, David S . Miller,
Florian Weimer, Geert Uytterhoeven, Heiko Carstens, James
In-Reply-To: <20190508225900.GA14091@yury-thinkpad>
There is a problem with the stack size accounting during execve when
there is no stack limit:
$ ulimit -s
8192
$ ./hello.ilp32
Hello World!
$ ulimit -s unlimited
$ ./hello.ilp32
Segmentation fault
$ strace ./hello.ilp32
execve("./hello.ilp32", ["./hello.ilp32"], 0xfffff10548f0 /* 77 vars */) = -1 ENOMEM (Cannot allocate memory)
+++ killed by SIGSEGV +++
Segmentation fault (core dumped)
Andreas.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
^ permalink raw reply
* Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Roberto Sassu @ 2019-05-13 7:49 UTC (permalink / raw)
To: Arvind Sankar, Rob Landley
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <20190512194322.GA71658@rani.riverdale.lan>
On 5/12/2019 9:43 PM, Arvind Sankar wrote:
> On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
>> 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?
>>
>> Ro
>
> Wouldn't the below work even before enforcing signatures on external
> initramfs:
> 1. Create an embedded initramfs with an /init that does the xattr
> parsing/setting. This will be verified as part of the kernel image
> signature, so no new code required.
> 2. Add a config option/boot parameter to panic the kernel if an external
> initramfs attempts to overwrite anything in the embedded initramfs. This
> prevents overwriting the embedded /init even if the external initramfs
> is unverified.
Unfortunately, it wouldn't work. IMA is already initialized and it would
verify /init in the embedded initial ram disk. The only reason why
opening .xattr-list works is that IMA is not yet initialized
(late_initcall vs rootfs_initcall).
Allowing a kernel with integrity enforcement to parse the CPIO image
without verifying it first is the weak point. However, extracted files
are not used, and before they are used they are verified. At the time
they are verified, they (included /init) must already have a signature
or otherwise access would be denied.
This scheme relies on the ability of the kernel to not be corrupted in
the event it parses a malformed CPIO image. Mimi suggested to use
digital signatures to prevent this issue, but it cannot be used in all
scenarios, since conventional systems generate the initial ram disk
locally.
Roberto
--
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Jian LI, Yanli SHI
^ permalink raw reply
* 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: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
From: Arvind Sankar @ 2019-05-12 19:43 UTC (permalink / raw)
To: Rob Landley
Cc: linux-kernel, linux-api, linux-fsdevel, linux-integrity,
initramfs
In-Reply-To: <dca50ee1-62d8-2256-6fdb-9a786e6cea5a@landley.net>
On Sun, May 12, 2019 at 05:05:48PM +0000, Rob Landley wrote:
> 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?
>
> Ro
Wouldn't the below work even before enforcing signatures on external
initramfs:
1. Create an embedded initramfs with an /init that does the xattr
parsing/setting. This will be verified as part of the kernel image
signature, so no new code required.
2. Add a config option/boot parameter to panic the kernel if an external
initramfs attempts to overwrite anything in the embedded initramfs. This
prevents overwriting the embedded /init even if the external initramfs
is unverified.
^ 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: [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 v6 5/6] binfmt_*: scope path resolution of interpreters
From: Andy Lutomirski @ 2019-05-12 14:34 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Linus Torvalds, Andrew Lutomirski, Jann Horn, Al Viro,
Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Eric Biederman, Andrew Morton, Alexei Starovoitov, Kees Cook,
Christian Brauner, Tycho Andersen, David Drysdale, Chanho Min,
Oleg Nesterov, Aleksa Sarai, Linux Containers, linux-fsdevel
In-Reply-To: <20190512133549.ymx5yg5rdqvavzyq@yavin>
> On May 12, 2019, at 6:35 AM, Aleksa Sarai <cyphar@cyphar.com> wrote:
>
>> On 2019-05-12, Linus Torvalds <torvalds@linux-foundation.org> wrote:
>>> On Sat, May 11, 2019 at 7:37 PM Andy Lutomirski <luto@amacapital.net> wrote:
>>> I bet this will break something that already exists. An execveat()
>>> flag to turn off /proc/self/exe would do the trick, though.
>>
>> Thinking more about it, I suspect it is (once again) wrong to let the
>> thing that does the execve() control that bit.
>>
>> Generally, the less we allow people to affect the lifetime and
>> environment of a suid executable, the better off we are.
>>
>> But maybe we could limit /proc/*/exe to at least not honor suid'ness
>> of the target? Or does chrome/runc depend on that too?
>
> Speaking on the runc side, we don't depend on this. It's possible
> someone depends on this for fexecve(3) -- but as mentioned before in
> newer kernels glibc uses execve(AT_EMPTY_PATH).
Why are we concerned about suid? Don’t we already block suid if the path being execed doesn’t come from the current mountns? That should mostly cover the things we care about, no?
I suppose we could also block suid for deleted files, so that deleting a known-buggy suid binary becomes more reliable. But every sensible package tool should already be chmoding the suid away before unlinking.
^ permalink raw reply
* Re: [PATCH v6 5/6] binfmt_*: scope path resolution of interpreters
From: Aleksa Sarai @ 2019-05-12 13:38 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Andrew Lutomirski, Jann Horn, Al Viro,
Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Eric Biederman, Andrew Morton, Alexei Starovoitov, Kees Cook,
Christian Brauner, Tycho Andersen, David Drysdale, Chanho Min,
Oleg Nesterov, Aleksa Sarai, Linux Containers, linux-fsdevel
In-Reply-To: <20190512133549.ymx5yg5rdqvavzyq@yavin>
[-- Attachment #1: Type: text/plain, Size: 2459 bytes --]
On 2019-05-12, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2019-05-12, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > On Sat, May 11, 2019 at 7:37 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > > I bet this will break something that already exists. An execveat()
> > > flag to turn off /proc/self/exe would do the trick, though.
> >
> > Thinking more about it, I suspect it is (once again) wrong to let the
> > thing that does the execve() control that bit.
> >
> > Generally, the less we allow people to affect the lifetime and
> > environment of a suid executable, the better off we are.
> >
> > But maybe we could limit /proc/*/exe to at least not honor suid'ness
> > of the target? Or does chrome/runc depend on that too?
>
> Speaking on the runc side, we don't depend on this. It's possible
> someone depends on this for fexecve(3) -- but as mentioned before in
> newer kernels glibc uses execve(AT_EMPTY_PATH).
>
> I would like to point out though that I'm a little bit cautious about
> /proc/self/exe-specific restrictions -- because a trivial way to get
> around them would be to just open it with O_PATH (and you end up with a
> /proc/self/fd/ which is equivalent). Unfortunately blocking setuid exec
> on all O_PATH descriptors would break even execve(AT_EMPTY_PATH) of
> setuid descriptors.
>
> The patches I mentioned (which Andy and I discussed off-list) would
> effectively make the magiclink modes in /proc/ affect how you can
> operate on the path (no write bit in the mode, cannot re-open it write).
> One aspect of this is how to handle O_PATH and in particular how do we
> handle an O_PATH re-open of an already-restricted magiclink.
>
> Maybe we could make it so that setuid is disallowed if you are dealing
> with an O_PATH fd which was a magiclink. Effectively, on O_PATH open you
> get an fmode_t saying FMODE_SETUID_EXEC_ALLOWED *but* if the path is a
> magiclink this fmode gets dropped and when the fd is given to
> execveat(AT_EMPTY_PATH) the fmode is checked and setuid-exec is not
> allowed.
... and obviously /proc/self/exe would have an fmode
~FMODE_SETUID_EXEC_ALLOWED from the outset. The reason for this slightly
odd semantic would be to continue to allow O_PATH setuid-exec as long as
the O_PATH was opened from an actual path rather than a magiclink.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v6 5/6] binfmt_*: scope path resolution of interpreters
From: Aleksa Sarai @ 2019-05-12 13:35 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andy Lutomirski, Andrew Lutomirski, Jann Horn, Al Viro,
Jeff Layton, J. Bruce Fields, Arnd Bergmann, David Howells,
Eric Biederman, Andrew Morton, Alexei Starovoitov, Kees Cook,
Christian Brauner, Tycho Andersen, David Drysdale, Chanho Min,
Oleg Nesterov, Aleksa Sarai, Linux Containers, linux-fsdevel
In-Reply-To: <CAHk-=wh3dT7=SMjvSZreXSu36Cg7gsfSipLhfTz5ioDKXV5uHg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2131 bytes --]
On 2019-05-12, Linus Torvalds <torvalds@linux-foundation.org> wrote:
> On Sat, May 11, 2019 at 7:37 PM Andy Lutomirski <luto@amacapital.net> wrote:
> > I bet this will break something that already exists. An execveat()
> > flag to turn off /proc/self/exe would do the trick, though.
>
> Thinking more about it, I suspect it is (once again) wrong to let the
> thing that does the execve() control that bit.
>
> Generally, the less we allow people to affect the lifetime and
> environment of a suid executable, the better off we are.
>
> But maybe we could limit /proc/*/exe to at least not honor suid'ness
> of the target? Or does chrome/runc depend on that too?
Speaking on the runc side, we don't depend on this. It's possible
someone depends on this for fexecve(3) -- but as mentioned before in
newer kernels glibc uses execve(AT_EMPTY_PATH).
I would like to point out though that I'm a little bit cautious about
/proc/self/exe-specific restrictions -- because a trivial way to get
around them would be to just open it with O_PATH (and you end up with a
/proc/self/fd/ which is equivalent). Unfortunately blocking setuid exec
on all O_PATH descriptors would break even execve(AT_EMPTY_PATH) of
setuid descriptors.
The patches I mentioned (which Andy and I discussed off-list) would
effectively make the magiclink modes in /proc/ affect how you can
operate on the path (no write bit in the mode, cannot re-open it write).
One aspect of this is how to handle O_PATH and in particular how do we
handle an O_PATH re-open of an already-restricted magiclink.
Maybe we could make it so that setuid is disallowed if you are dealing
with an O_PATH fd which was a magiclink. Effectively, on O_PATH open you
get an fmode_t saying FMODE_SETUID_EXEC_ALLOWED *but* if the path is a
magiclink this fmode gets dropped and when the fd is given to
execveat(AT_EMPTY_PATH) the fmode is checked and setuid-exec is not
allowed.
[I assume in this discussion "setuid" means "setuid + setcap", right?]
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ 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 v6 5/6] binfmt_*: scope path resolution of interpreters
From: Linus Torvalds @ 2019-05-12 10:44 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andrew Lutomirski, Jann Horn, Aleksa Sarai, Al Viro, Jeff Layton,
J. Bruce Fields, Arnd Bergmann, David Howells, Eric Biederman,
Andrew Morton, Alexei Starovoitov, Kees Cook, Christian Brauner,
Tycho Andersen, David Drysdale, Chanho Min, Oleg Nesterov,
Aleksa Sarai, Linux Containers, linux-fsdevel
In-Reply-To: <9CD2B97D-A6BD-43BE-9040-B410D996A195@amacapital.net>
On Sat, May 11, 2019 at 7:37 PM Andy Lutomirski <luto@amacapital.net> wrote:
>
> I bet this will break something that already exists. An execveat() flag to turn off /proc/self/exe would do the trick, though.
Thinking more about it, I suspect it is (once again) wrong to let the
thing that does the execve() control that bit.
Generally, the less we allow people to affect the lifetime and
environment of a suid executable, the better off we are.
But maybe we could limit /proc/*/exe to at least not honor suid'ness
of the target? Or does chrome/runc depend on that too?
Linus
^ permalink raw reply
* Re: [PATCH v6 5/6] binfmt_*: scope path resolution of interpreters
From: Christian Brauner @ 2019-05-12 10:19 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Lutomirski, Jann Horn, Aleksa Sarai, Al Viro, Jeff Layton,
J. Bruce Fields, Arnd Bergmann, David Howells, Eric Biederman,
Andrew Morton, Alexei Starovoitov, Kees Cook, Tycho Andersen,
David Drysdale, Chanho Min, Oleg Nesterov, Aleksa Sarai,
Linux Containers, linux-fsdevel, Linux API
In-Reply-To: <CAHk-=wg3+3GfHsHdB4o78jNiPh_5ShrzxBuTN-Y8EZfiFMhCvw@mail.gmail.com>
On Sat, May 11, 2019 at 07:08:49PM -0400, Linus Torvalds wrote:
> [ on mobile again, power is off and the wifi doesn't work, so I'm reading
> email on my phone and apologizing once more for horrible html email.. ]
>
> On Sat, May 11, 2019, 18:40 Andy Lutomirski <luto@kernel.org> wrote:
>
> >
> > a) Change all my UIDs and GIDs to match a container, enter that
> > container's namespaces, and run some binary in the container's
For the namespace part, an idea I had and presented at LPC a while ago
was to make setns() interpret the nstype argument as a flag argument and
to introduce an fd that can refer to multiple namespaces at the same
time. This way you could do:
setns(namespaces_fd, CLONE_NEWNS | CLONE_NEWUSER | CLONE_NEWPID)
that could still be done. But I since have come to think that there's a
better way now that we have CLONE_PIDFD. We should just make setns()
take a pidfd as argument and then be able to call:
setns(pidfd, 0);
which would cause the calling thread to join all of the namespaces of
the process referred to by pidfd at the same time. That really shouldn't
be hard to do. I could probably get this going rather quickly and it
would really help out container managers a lot.
> > filesystem, all atomically enough that I don't need to worry about
> > accidentally leaking privileges into the container. A
> > super-duper-non-dumpable mode would kind of allow this, but I'd worry
> > that there's some other hole besides ptrace() and /proc/self.
> >
>
> So I would expect that you'd want to do this even *without* doing an execve
> at all, which is why I still don't think this is actually about
> spawn/execve at all.
>
> But I agree that what you that what you want sounds reasonable. But I think
I have pitched an api like that a while ago (see [1]) - when I first
started this fd for processes thing - that would allow you to do things
like that. The gist was:
1. int pidfd_create aka CLONE_PIDFD now
will return an fd that creates a process context. The fd returned by
does not refer to any existing process and has not actually been
started yet. So non-configuration operations on it or trying to
interact with it would fail with e.g. ESRCH/EINVAL.
We essentially have this now with CLONE_PIDFD. The bit that is missing
is an "unscheduled" process.
2. int pidfd_config
takes a CLONE_PIDFD and can be used to configure a process context
before it is alive.
Some things that I would like to be able to do with this syscall are:
- configure signals
- set clone flags
- write idmappings if the process runs in a new user namespace
- configure what happens when all procfds referring to the process are gone
- ...
3. int pidfd_info
4. int pidfd_manage
Parts of that are captured in pidfd_send_signal().
Just to get a very rough feel for this without detailing parameters right now:
/* process should have own mountns */
pidfd_config(fd, PROC_SET_NAMESPACE, CLONE_NEWNS | CLONE_NEWPID | CLONE_NEWUSR, <potentially additional arguments>)
/* process should get SIGKILL when all procfds are closed */
pidfd_config(fd, PROC_SET_CLOSE, SIGKILL, <potentially additional arguments>)
After the caller is done configuring the process there would be a final step:
pidfd_config(fd, PROC_CREATE, 0, <potentially additional arguments>)
which would create the process and (either as return value or through a
parameter) return the pid of the newly created process.
I had more thoughts on this and had started prototyping some of it but
then there wasn't much interest it seemed. Maybe because it's crazy.
[1]: https://lore.kernel.org/lkml/20181118174148.nvkc4ox2uorfatbm@brauner.io/
> the "dumpable" flag has always been a complete hack, and very unreliable
> with random meanings (and random ways to then get around it).
>
> We have actually had lots of people wanting to run "lists of system calls"
> kinds of things. Sometimes for performance reasons, sometimes for other
> random reasons Maybe that "atomicity" angle would be another one, although
> we would have to make the setuid etc things be something that happens at
> the end.
>
> So rather than "spawn", is much rather see people think about ways to just
> batch operations in general, rather than think it is about batching things
> just before a process change.
>
> b) Change all my UIDs and GIDs to match a container, enter that
> > container's namespaces, and run some binary that is *not* in the
> > container's filesystem.
> >
>
> Hey, you could probably do something very close to that by opening the
> executable you want to run first, making it O_CLOEXEC, then doing all the
> other things (which now makes the executable inaccessible), and finally
> doing execveat() on the file descriptor..
I think that's somewhat similar to what I've suggested above.
>
> I say "something very close" because even though it's O_CLOEXEC, only the
> file would be closed, and /proc/self/exe would still exist.
>
> But we really could make that execveat of a O_CLOEXEC file thing also
> disable access to /proc/*/exe, and it sounds like a sane and simple
> extension in general to do..
>
> Linus
>
> >
^ 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
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