From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 891CC941 for ; Mon, 1 Aug 2016 23:45:24 +0000 (UTC) Received: from mail-vk0-f46.google.com (mail-vk0-f46.google.com [209.85.213.46]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 7F3DC1F3 for ; Mon, 1 Aug 2016 23:45:23 +0000 (UTC) Received: by mail-vk0-f46.google.com with SMTP id w127so110852908vkh.2 for ; Mon, 01 Aug 2016 16:45:23 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: References: <1469979098.23563.300.camel@linux.vnet.ibm.com> <1469986138.23563.312.camel@linux.vnet.ibm.com> <20160801172920.GU3296@wotan.suse.de> <1470090069.23563.475.camel@linux.vnet.ibm.com> <20160801230435.GF4541@io.lakedaemon.net> <20160801233036.GH4541@io.lakedaemon.net> From: Andy Lutomirski Date: Mon, 1 Aug 2016 16:45:20 -0700 Message-ID: To: Jason Cooper Content-Type: multipart/alternative; boundary=001a1143ad7c8e806205390b31e1 Cc: James Bottomley , Mark Brown , "ksummit-discuss@lists.linuxfoundation.org" Subject: Re: [Ksummit-discuss] Last minute nominations: mcgrof and toshi List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --001a1143ad7c8e806205390b31e1 Content-Type: text/plain; charset=UTF-8 On Aug 1, 2016 4:30 PM, "Jason Cooper" wrote: > > On Mon, Aug 01, 2016 at 04:13:50PM -0700, Andy Lutomirski wrote: > > On Aug 1, 2016 4:04 PM, "Jason Cooper" wrote: > > > > > > On Mon, Aug 01, 2016 at 03:36:51PM -0700, Andy Lutomirski wrote: > > > > On Mon, Aug 1, 2016 at 3:21 PM, Mimi Zohar > > wrote: > > > > > On Mo, 2016-08-01 at 10:59 -0700, Andy Lutomirski wrote: > > > > > > > > > >> Mimi, I'm curious: I don't fully understand what is covered by IMA > > > > >> policy. How does the IMA kernel_read_file stuff deal with symlinks? > > > > >> For example, if I symlink /lib/firmware/iwlwifi-8265-21.ucode to > > > > >> /home/badguy/iwlwifi-8265-21.ucode, what happens? What if I symlink > > > > >> /lib/firmware/iwlwifi-8265-21.ucode to /home/badguy/something_else? > > > > >> Or even /lib/modules/kernel/foo/bar.ko to /home/badguy/evil.ko? The > > > > >> interesting case is where the "badguy" user is duly authorized to > > > > >> write to /home/badguy and holds whatever keys may be needed. > > > > > > > > > > Lets step back a second. In order for a key to be added to the IMA > > > > > keyring, the key must be signed by a key on the builtin keyring. The > > > > > key on the builtin keyring can be compiled into the kernel image or > > > > > added post build using Mehmet Kayaalp's patches. > > > > > > > > > > True, any key on the IMA keyring could be used to verify file > > signatures > > > > > (in IMA terminology appraise the file's integrity). The enumeration > > is > > > > > a first step to making sure that only properly signed code is read by > > > > > the kernel. The next step requires finer grain key management. In > > > > > general, pathname based policies are not a good idea. Whatever method > > > > > is defined, it should not be limited to just firmware or files read by > > > > > the kernel, but to all files. > > > > > > > > > > > > > Unless I'm mistaken (which is quite possible), IMA is primarily > > > > intended to appraise the content of POSIX filesystems. So, if IMA is > > > > in use, then doing: > > > > > > > > $ cat /foo/bar > > > > > > > > should only succeed if /foo/bar is signed according to loaded policy. > > > > It's the system administrator's decision what filesystem is actually > > > > mounted at /foo, and root can presumably mess around with application > > > > expectations by, say, bind-mounting something over /foo. > > > > > > > > Modules and firmware are special: even root should not be able to > > > > avoid the full signature policy. This means that, for example: > > > > > > > > # mount --bind /evil /lib/firmware > > > > > > > > should not result in violating policy. So the pathname should not be > > > > used as such. However, firmware is a bit special in that the driver > > > > chooses the pathname to request, and it really does uniquely identify > > > > the intended firmware. So, when a driver asks for: > > > > > > > > "iwlwifi-whatever.ucode" > > > > > > > > and the driver core tries to read "/lib/firmware/iwlwifi-whatever.ucode" > > > > > > > > it's entirely possible that we'll follow a symlink and end up > > > > elsewhere (Fedora, for example, does exactly this), but the file > > > > that's loaded should be appraised (or verified using a non-IMA means, > > > > etc.) to verify that whatever blob gets found is actually signed by > > > > the holder of an authorized key for the purpose of being used as > > > > "iwlwifi-whatever.ucode". > > > > > > Assuming Andy's lightweight signature scheme, it would probably be best > > > to do a lookup based on the sha256 hash of the file. Then pathnames > > > don't matter, and bad files don't even get to the signature checking > > > code. > > > > > > > I'm not sure I understand what you mean. What table would we look the hash > > up in? What are we finding in that table? > > From the other subthread: > > > Then, to verify a signature, the kernel hashes the blob, generates its > > own linux_blob_signed_data, memcmps it to the one that Kyle signed > > (and rejects if they differ *at all*), and then verifies the > > signature. (Do not try to be clever and parse the supplied > > linux_blob_signed_data -- there is a long and storied history of > > equivalent ideas being implemented incorrectly, and I can dig out > > literature references if you really want. Just generate your own and > > memcmp it, which leaves no room for ambiguity.) > > > > So, I'm suggesting that when "the kernel hashes the blob", it use that > hash to locate *which* "Kyle-signed" linux_blob_signed_data it needs to > compare against. That's all, just removing the filename from the > equation. :-) So Kyle would generate a list of signatures indexed by the blob's hash instead of generating things like "iwlwifi-whatever.ucode.sig"? Seems okay. It'll keep the existing hooks working, I think. Of course, we still need to check the "iwlwifi-whatever.ucode" bit to confirm that it matches Kyle's signed data. --001a1143ad7c8e806205390b31e1 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On Aug 1, 2016 4:30 PM, "Jason Cooper" <jason@lakedaemon.net> wrote:
>
> On Mon, Aug 01, 2016 at 04:13:50PM -0700, Andy Lutomirski wrote:
> > On Aug 1, 2016 4:04 PM, "Jason Cooper" <jason@lakedaemon.net> wrote:
> > >
> > > On Mon, Aug 01, 2016 at 03:36:51PM -0700, Andy Lutomirski wr= ote:
> > > > On Mon, Aug 1, 2016 at 3:21 PM, Mimi Zohar <zohar@linux.vnet.ibm.com>
> > wrote:
> > > > > On Mo, 2016-08-01 at 10:59 -0700, Andy Lutomirski = wrote:
> > > > >
> > > > >> Mimi, I'm curious: I don't fully under= stand what is covered by IMA
> > > > >> policy.=C2=A0 How does the IMA kernel_read_fil= e stuff deal with symlinks?
> > > > >> For example, if I symlink /lib/firmware/iwlwif= i-8265-21.ucode to
> > > > >> /home/badguy/iwlwifi-8265-21.ucode, what happe= ns?=C2=A0 What if I symlink
> > > > >> /lib/firmware/iwlwifi-8265-21.ucode to /home/b= adguy/something_else?
> > > > >> Or even /lib/modules/kernel/foo/bar.ko to /hom= e/badguy/evil.ko?=C2=A0 The
> > > > >> interesting case is where the "badguy&quo= t; user is duly authorized to
> > > > >> write to /home/badguy and holds whatever keys = may be needed.
> > > > >
> > > > > Lets step back a second.=C2=A0 In order for a key = to be added to the IMA
> > > > > keyring, the key must be signed by a key on the bu= iltin keyring.=C2=A0 The
> > > > > key on the builtin keyring can be compiled into th= e kernel image or
> > > > > added post build using Mehmet Kayaalp's patche= s.
> > > > >
> > > > > True, any key on the IMA keyring could be used to = verify file
> > signatures
> > > > > (in IMA terminology appraise the file's integr= ity).=C2=A0 The enumeration
> > is
> > > > > a first step to making sure that only properly sig= ned code is read by
> > > > > the kernel.=C2=A0 The next step requires finer gra= in key management.=C2=A0 In
> > > > > general, pathname based policies are not a good id= ea.=C2=A0 Whatever method
> > > > > is defined, it should not be limited to just firmw= are or files read by
> > > > > the kernel, but to all files.
> > > > >
> > > >
> > > > Unless I'm mistaken (which is quite possible), IMA = is primarily
> > > > intended to appraise the content of POSIX filesystems.= =C2=A0 So, if IMA is
> > > > in use, then doing:
> > > >
> > > > $ cat /foo/bar
> > > >
> > > > should only succeed if /foo/bar is signed according to = loaded policy.
> > > > It's the system administrator's decision what f= ilesystem is actually
> > > > mounted at /foo, and root can presumably mess around wi= th application
> > > > expectations by, say, bind-mounting something over /foo= .
> > > >
> > > > Modules and firmware are special: even root should not = be able to
> > > > avoid the full signature policy.=C2=A0 This means that,= for example:
> > > >
> > > > # mount --bind /evil /lib/firmware
> > > >
> > > > should not result in violating policy.=C2=A0 So the pat= hname should not be
> > > > used as such.=C2=A0 However, firmware is a bit special = in that the driver
> > > > chooses the pathname to request, and it really does uni= quely identify
> > > > the intended firmware.=C2=A0 So, when a driver asks for= :
> > > >
> > > > "iwlwifi-whatever.ucode"
> > > >
> > > > and the driver core tries to read "/lib/firmware/i= wlwifi-whatever.ucode"
> > > >
> > > > it's entirely possible that we'll follow a syml= ink and end up
> > > > elsewhere (Fedora, for example, does exactly this), but= the file
> > > > that's loaded should be appraised (or verified usin= g a non-IMA means,
> > > > etc.) to verify that whatever blob gets found is actual= ly signed by
> > > > the holder of an authorized key for the purpose of bein= g used as
> > > > "iwlwifi-whatever.ucode".
> > >
> > > Assuming Andy's lightweight signature scheme, it would p= robably be best
> > > to do a lookup based on the sha256 hash of the file.=C2=A0 T= hen pathnames
> > > don't matter, and bad files don't even get to the si= gnature checking
> > > code.
> > >
> >
> > I'm not sure I understand what you mean.=C2=A0 What table wou= ld we look the hash
> > up in?=C2=A0 What are we finding in that table?
>
> From the other subthread:
>
> > Then, to verify a signature, the kernel hashes the blob, generate= s its
> > own linux_blob_signed_data, memcmps it to the one that Kyle signe= d
> > (and rejects if they differ *at all*), and then verifies the
> > signature.=C2=A0 (Do not try to be clever and parse the supplied<= br> > > linux_blob_signed_data -- there is a long and storied history of<= br> > > equivalent ideas being implemented incorrectly, and I can dig out=
> > literature references if you really want.=C2=A0 Just generate you= r own and
> > memcmp it, which leaves no room for ambiguity.)
> >
>
> So, I'm suggesting that when "the kernel hashes the blob"= ;, it use that
> hash to locate *which* "Kyle-signed" linux_blob_signed_data = it needs to
> compare against.=C2=A0 That's all, just removing the filename from= the
> equation. :-)

So Kyle would generate a list of signatures indexed by the b= lob's hash instead of generating things like "iwlwifi-whatever.uco= de.sig"?=C2=A0 Seems okay.=C2=A0 It'll keep the existing hooks wor= king, I think.=C2=A0 Of course, we still need to check the "iwlwifi-wh= atever.ucode" bit to confirm that it matches Kyle's signed data.

--001a1143ad7c8e806205390b31e1--