Linux-Integrity Archive on lore.kernel.org
 help / color / Atom feed
From: Rob Landley <rob@landley.net>
To: Roberto Sassu <roberto.sassu@huawei.com>,
	Arvind Sankar <niveditas98@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-api@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-integrity@vger.kernel.org,
	initramfs@vger.kernel.org
Subject: Re: [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk
Date: Tue, 14 May 2019 01:06:06 -0500
Message-ID: <49965ffd-dd57-ffe5-4a2f-73cdfb387848@landley.net> (raw)
In-Reply-To: <f7bc547c-61f4-1a17-735c-7e8df97d7965@huawei.com>

On 5/13/19 7:47 AM, Roberto Sassu wrote:
> On 5/13/2019 11:07 AM, Rob Landley wrote:
>>>> 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.

If /init is in the internal one and you can't overwrite files with an external
one, all your init has to be is something that applies the xattrs, enables your
paranoia mode, and then execs something else.

Heck, I do that sort of set up in shell scripts all the time. Running the shell
script as PID 1 and then having it exec the "real init" binary at the end:

https://github.com/landley/mkroot/blob/83def3cbae21/mkroot.sh#L205

If your first init binary is in the initramfs statically linked into the kernel
image, and the cpio code is doing open(O_EXCL), then it's as verified as any
other kernel code and runs "securely" until it decides to run something else.

> Also, requiring an embedded initramfs for xattrs would be an issue for
> systems that use it for other purposes.

I'm the guy who wrote the initmpfs code. (And has pending patches to improve it
that will probably never go upstream because I'm a hobbyist and dealing with the
 linux-kernel clique is the opposite of fun. I'm only in this conversation
because I was cc'd.)

You can totally use initramfs for lots of purposes simultaneously.

>>> 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.

Which isn't a problem if it was statically linked in the kernel, or if your
external cpio.gz was signed. You want a signed binary but don't want the
signature _in_ the binary...

>>> 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.

Which is why there's a cpio in the kernel and an external cpio loaded via the
old initrd mechanism and BOTH files wind up in the cpio and there's a way to
make it O_EXCL so it can't overwrite, and then the /init binary inside the
kernel's cpio can do any other weird verification you need to do before anything
else gets a chance to run so why are you having ring 0 kernel code read a file
out of the filesystem and act upon it?

(Heck, you can mv /newinit /init before the exec /init so the file isn't on the
system anymore by the time the other stuff gets to run...)

>>> 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.

If it's in the file's contents you get uniform behavior regardless of the
filesystem used. And "mandatory access controls do that" is basically restating
what _I_ said in the paragraph above.

The "infrastructure you have that works a certain way" is called "mandatory
access controls". Good to know. Your patch changes the rest of the system to
match the assumptions of the new code, because changing those assumptions
appears literally unthinkable.

>>> 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?

That you're not remotely the first person to do this?

You're attempting to prevent anyone from running third party code on your system
without buying a license from you first. You're creating a system with no user
serviceable parts, that only runs authorized software from the Apple Store or
other walled garden. No sideloading allowed.

Which is your choice, sure. But why do you need new infrastructure to do it?
People have already _done_ this. They're just by nature proprietary and don't
like sharing with the group when not forced by lawyers, so they come up with
ways that don't involve modifying GPLv2 software (or shipping GPLv3 software,
ever, for any reason).

>>> 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.

So you have _more_ assumptions tripping you up. Great. So add a signature in a
format your bootloader doesn't recognize, since it's the kernel that should
verify it, not your bootloader?

It sounds like your problem is bureaucratic, not technical.

>> 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.

Call the elf sections SIG1 SIG2 SIG3, or have a section full of keyword=value
strings? How is this a hard problem?

> Also appending the signature alone won't solve the parsing issue. Still,
> the kernel has to parse something that could be malformed.

Your new in-band signaling file you're making xattrs from could be malformed,
one of the xattrs you add could be "banana=aaaaaaaaaaaaaaaaaaaaaaaaaaa..." going
on for 12 megabytes...

Rob

  parent reply index

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-05-09 11:24 Roberto Sassu
2019-05-09 11:24 ` [PATCH v2 1/3] fs: add ksys_lsetxattr() wrapper Roberto Sassu
2019-05-10 21:28   ` Jann Horn
2019-05-09 11:24 ` [PATCH v2 2/3] initramfs: set extended attributes Roberto Sassu
2019-05-09 11:24 ` [PATCH v2 3/3] initramfs: introduce do_readxattrs() Roberto Sassu
2019-05-10 21:33   ` Jann Horn
2019-05-13 13:03     ` Roberto Sassu
2019-05-09 18:34 ` [PATCH v2 0/3] initramfs: add support for xattrs in the initial ram disk Rob Landley
2019-05-10  6:56   ` Roberto Sassu
2019-05-10 11:49     ` Mimi Zohar
2019-05-10 20:46       ` Rob Landley
2019-05-10 22:38         ` Mimi Zohar
2019-05-11 22:44 ` Andy Lutomirski
2019-05-12  4:04   ` Rob Landley
2019-05-12  4:12     ` Rob Landley
2019-05-12  9:17 ` Dominik Brodowski
2019-05-12 10:18   ` hpa
2019-05-12 15:31     ` Dominik Brodowski
2019-05-13  0:02       ` Mimi Zohar
2019-05-13  0:21         ` hpa
2019-05-13  0:23       ` hpa
2019-05-12 12:52   ` Mimi Zohar
2019-05-12 17:05     ` Rob Landley
2019-05-12 19:43       ` Arvind Sankar
2019-05-13  7:49         ` Roberto Sassu
2019-05-13  9:07           ` Rob Landley
2019-05-13 12:08             ` Mimi Zohar
2019-05-13 12:47             ` Roberto Sassu
2019-05-13 17:20               ` Arvind Sankar
2019-05-13 17:51                 ` Arvind Sankar
2019-05-13 17:52                 ` Arvind Sankar
2019-05-13 18:36                   ` Mimi Zohar
2019-05-13 18:47                     ` Arvind Sankar
2019-05-13 22:09                       ` Mimi Zohar
2019-05-14  6:06                         ` Rob Landley
2019-05-14 14:44                           ` Arvind Sankar
2019-05-14  6:06               ` Rob Landley [this message]
2019-05-14 11:52                 ` Roberto Sassu
2019-05-14 15:12                   ` Rob Landley
2019-05-14 15:27                   ` Arvind Sankar
2019-05-14 15:57                     ` Arvind Sankar
2019-05-14 17:44                       ` Roberto Sassu
2019-05-15  1:00                         ` Arvind Sankar
2019-05-14 15:19               ` Andy Lutomirski
2019-05-14 16:33                 ` Roberto Sassu
2019-05-14 16:58                   ` Greg KH
2019-05-14 17:20                     ` Roberto Sassu
2019-05-15  0:25                       ` Arvind Sankar
2019-05-14 19:18                 ` James Bottomley
2019-05-14 23:39                   ` Rob Landley
2019-05-14 23:54                     ` James Bottomley
2019-05-15  0:52                       ` Arvind Sankar
2019-05-15 11:19                         ` Roberto Sassu
2019-05-15 16:08                           ` Arvind Sankar
2019-05-15 17:06                             ` Roberto Sassu
2019-05-16  5:29                               ` Arvind Sankar
2019-05-16 11:42                                 ` Roberto Sassu
2019-05-16 13:31                                 ` Mimi Zohar

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=49965ffd-dd57-ffe5-4a2f-73cdfb387848@landley.net \
    --to=rob@landley.net \
    --cc=initramfs@vger.kernel.org \
    --cc=linux-api@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=niveditas98@gmail.com \
    --cc=roberto.sassu@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Integrity Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-integrity/0 linux-integrity/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-integrity linux-integrity/ https://lore.kernel.org/linux-integrity \
		linux-integrity@vger.kernel.org linux-integrity@archiver.kernel.org
	public-inbox-index linux-integrity


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-integrity


AGPL code for this site: git clone https://public-inbox.org/ public-inbox