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 B2CFC8A1 for ; Sun, 31 Jul 2016 18:20:15 +0000 (UTC) Received: from mail.kernel.org (mail.kernel.org [198.145.29.136]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id F29A6212 for ; Sun, 31 Jul 2016 18:20:14 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id C2CA420306 for ; Sun, 31 Jul 2016 18:20:13 +0000 (UTC) Received: from mail-vk0-f47.google.com (mail-vk0-f47.google.com [209.85.213.47]) (using TLSv1.2 with cipher AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPSA id 1BDD72026F for ; Sun, 31 Jul 2016 18:20:11 +0000 (UTC) Received: by mail-vk0-f47.google.com with SMTP id x130so85885780vkc.0 for ; Sun, 31 Jul 2016 11:20:11 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <1469986138.23563.312.camel@linux.vnet.ibm.com> References: <20160716005213.GL30372@sirena.org.uk> <1469544138.120686.327.camel@infradead.org> <14209.1469636040@warthog.procyon.org.uk> <1469636881.27356.70.camel@HansenPartnership.com> <1469637367.27356.73.camel@HansenPartnership.com> <1469648220.23563.15.camel@linux.vnet.ibm.com> <20160728165728.GR4541@io.lakedaemon.net> <1469830256.23563.200.camel@linux.vnet.ibm.com> <20160730163626.GP3296@wotan.suse.de> <1469934481.23563.274.camel@linux.vnet.ibm.com> <1469979098.23563.300.camel@linux.vnet.ibm.com> <1469986138.23563.312.camel@linux.vnet.ibm.com> From: Andy Lutomirski Date: Sun, 31 Jul 2016 11:20:08 -0700 Message-ID: To: Mimi Zohar Content-Type: multipart/alternative; boundary=001a1144212ab16f810538f2888b Cc: James Bottomley , Mark Brown , "ksummit-discuss@lists.linuxfoundation.org" , Jason Cooper Subject: Re: [Ksummit-discuss] Last minute nominations: mcgrof and toshi List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , --001a1144212ab16f810538f2888b Content-Type: text/plain; charset=UTF-8 On Jul 31, 2016 10:29 AM, "Mimi Zohar" wrote: > > On So, 2016-07-31 at 09:19 -0700, Andy Lutomirski wrote: > > On Sun, Jul 31, 2016 at 8:31 AM, Mimi Zohar wrote: > > > On Sa, 2016-07-30 at 20:09 -0700, Andy Lutomirski wrote: > > >> On Jul 30, 2016 8:08 PM, "Mimi Zohar" wrote: > > >> > > > >> > On Sa, 2016-07-30 at 18:36 +0200, Luis R. Rodriguez wrote: > > >> > > On Fri, Jul 29, 2016 at 03:25:09PM -0700, Andy Lutomirski wrote: > > >> > > > On Fri, Jul 29, 2016 at 3:10 PM, Mimi Zohar < zohar@linux.vnet.ibm.com> > > >> wrote: > > >> > > > > On Do, 2016-07-28 at 16:57 +0000, Jason Cooper wrote: > > >> > > > >> Hi Andy, > > >> > > > >> > > >> > > > >> On Wed, Jul 27, 2016 at 01:09:37PM -0700, Andy Lutomirski wrote: > > >> > > > >> ... > > >> > > > >> > I would like someone to explain why using the keyring mechanism > > >> for > > >> > > > >> > this in the first place is a good idea. > > >> > > > >> > > > >> > > > >> > As far as I can tell, the design goals of "keys trusted by the > > >> kernel > > >> > > > >> > for modules, firmware, etc" are: > > >> > > > >> > > > >> > > > >> > - Keys are added at build time or, according to potentially > > >> > > > >> > system-specific rules, at boot time. > > >> > > > >> > > > >> > > > >> > - Keys should specify what they're trusted *for*. > > >> > > > >> > > >> > > > >> Well, I'd argue that keys should specify what they are *intended* > > >> for by > > >> > > > >> the keyholder. A useful security system could further restrict > > >> the key > > >> > > > >> as needed. > > >> > > > > > > >> > > > > We've already started. Currently the kernel_read_file() and family > > >> of > > >> > > > > functions provide the LSM hooks needed for verifying file > > >> signatures of > > >> > > > > files read by the kernel. The kernel_read_file_id enumeration is > > >> used > > >> > > > > to differentiate between callers. Based on this enumeration, the > > >> > > > > *intended* for could be defined. It would make sense to extend the > > >> IMA > > >> > > > > policy language to support *intended* for. > > >> > > > > > > >> > > > > > >> > > > Having kernel_read_file know the purpose is a big step in the right > > >> > > > direction, although, as I think I and others have pointed out, just an > > >> > > > enum is insufficient -- for firmware, at least, the *name* is > > >> > > > relevant. > > >> > > > >> > > The name is passed for firmware, the wrapper > > >> kernel_read_file_from_path() > > >> > > is used. So if we wanted an LSM extension on name I think we can do that > > >> > > on kernel_read_file_from_path() ? > > >> > > > >> > It shouldn't make a difference whether kernel_read_file() is called > > >> > directly, or the kernel_read_file_by_path/fd() are called. The pathname > > >> > is accessible from the "file" argument. > > >> > > > >>l > > >> What happens if a symlink is involved? > > > > > > For callers of kernel_read_file_by_path(), like firmware, we could pass > > > the pathname, but for the other callers of kernel_read_file/_by_fd() we > > > could use d_absolute_path(). > > > > That seems pointlessly fragile to me, and this issue has been known > > for longer than the code in question has even existed. How about: > > > > struct kernel_trusted_file_description { > > enum kernel_read_file_id type; > > const char *specific_purpose; /* may be NULL for KEXEC_IMAGE, etc. */ > > }; > > > > int kernel_read_file(struct file *, void **, loff_t *, const struct > > trusted_file_description *); > > > > rather than trying to guess. > > > > Also, are there any plans to move module signature verification into > > .kernel_post_read_file? > > The whole point of defining the kernel_read_file() family of functions > was to close a class of measurement/appraisal gaps. And to enable firmware verification? > To answer your > question, yes IMA measures and/or appraises files on the security post > kernel read hook, based on policy. I wasn't asking anything about IMA. > > Andy, other than IMA-appraisal being xattr based, I'm not sure why > you're so against it. if you're going to be at LinuxCon/LSS, perhaps > we could speak in person there. I have nothing against IMA. I have plenty of objection to the core hooks being simultaneously messy and not supplying enough information to provide the security properties they should provide. If IMA doesn't want or need to verify the purpose of the loaded file, fine. I'll be at KS but not LSS. --Andy --001a1144212ab16f810538f2888b Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable

On Jul 31, 2016 10:29 AM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wrote= :
>
> On So, 2016-07-31 at 09:19 -0700, Andy Lutomirski wrote:
> > On Sun, Jul 31, 2016 at 8:31 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > > On Sa, 2016-07-30 at 20:09 -0700, Andy Lutomirski wrote:
> > >> On Jul 30, 2016 8:08 PM, "Mimi Zohar" <zohar@linux.vnet.ibm.com> wr= ote:
> > >> >
> > >> > On Sa, 2016-07-30 at 18:36 +0200, Luis R. Rodriguez= wrote:
> > >> > > On Fri, Jul 29, 2016 at 03:25:09PM -0700, Andy= Lutomirski wrote:
> > >> > > > On Fri, Jul 29, 2016 at 3:10 PM, Mimi Zoh= ar <zohar@linux.vnet.ibm.com= >
> > >> wrote:
> > >> > > > > On Do, 2016-07-28 at 16:57 +0000, Ja= son Cooper wrote:
> > >> > > > >> Hi Andy,
> > >> > > > >>
> > >> > > > >> On Wed, Jul 27, 2016 at 01:09:37= PM -0700, Andy Lutomirski wrote:
> > >> > > > >> ...
> > >> > > > >> > I would like someone to exp= lain why using the keyring mechanism
> > >> for
> > >> > > > >> > this in the first place is = a good idea.
> > >> > > > >> >
> > >> > > > >> > As far as I can tell, the d= esign goals of "keys trusted by the
> > >> kernel
> > >> > > > >> > for modules, firmware, etc&= quot; are:
> > >> > > > >> >
> > >> > > > >> >=C2=A0 - Keys are added at b= uild time or, according to potentially
> > >> > > > >> > system-specific rules, at b= oot time.
> > >> > > > >> >
> > >> > > > >> >=C2=A0 - Keys should specify= what they're trusted *for*.
> > >> > > > >>
> > >> > > > >> Well, I'd argue that keys sh= ould specify what they are *intended*
> > >> for by
> > >> > > > >> the keyholder.=C2=A0 A useful se= curity system could further restrict
> > >> the key
> > >> > > > >> as needed.
> > >> > > > >
> > >> > > > > We've already started.=C2=A0 Cur= rently the kernel_read_file() and family
> > >> of
> > >> > > > > functions provide the LSM hooks need= ed for verifying file
> > >> signatures of
> > >> > > > > files read by the kernel.=C2=A0 The = kernel_read_file_id enumeration is
> > >> used
> > >> > > > > to differentiate between callers.=C2= =A0 Based on this enumeration, the
> > >> > > > > *intended* for could be defined.=C2= =A0 It would make sense to extend the
> > >> IMA
> > >> > > > > policy language to support *intended= * for.
> > >> > > > >
> > >> > > >
> > >> > > > Having kernel_read_file know the purpose = is a big step in the right
> > >> > > > direction, although, as I think I and oth= ers have pointed out, just an
> > >> > > > enum is insufficient -- for firmware, at = least, the *name* is
> > >> > > > relevant.
> > >> >
> > >> > > The name is passed for firmware, the wrapper > > >> kernel_read_file_from_path()
> > >> > > is used. So if we wanted an LSM extension on n= ame I think we can do that
> > >> > > on kernel_read_file_from_path() ?
> > >> >
> > >> > It shouldn't make a difference whether kernel_r= ead_file() is called
> > >> > directly, or the kernel_read_file_by_path/fd() are = called.=C2=A0 The pathname
> > >> > is accessible from the "file" argument. > > >> >
> > >>l
> > >> What happens if a symlink is involved?
> > >
> > > For callers of kernel_read_file_by_path(), like firmware, we= could pass
> > > the pathname, but for the other callers of kernel_read_file/= _by_fd() we
> > > could use d_absolute_path().
> >
> > That seems pointlessly fragile to me, and this issue has been kno= wn
> > for longer than the code in question has even existed.=C2=A0 How = about:
> >
> > struct kernel_trusted_file_description {
> >=C2=A0 =C2=A0enum kernel_read_file_id type;
> >=C2=A0 =C2=A0const char *specific_purpose;=C2=A0 /* may be NULL fo= r KEXEC_IMAGE, etc. */
> > };
> >
> > int kernel_read_file(struct file *, void **, loff_t *, const stru= ct
> > trusted_file_description *);
> >
> > rather than trying to guess.
> >
> > Also, are there any plans to move module signature verification i= nto
> > .kernel_post_read_file?
>
> The whole point of defining the kernel_read_file() family of functions=
> was to close a class of measurement/appraisal gaps.

And to enable firmware verification?

>=C2=A0 =C2=A0To answer your
> question, yes IMA measures and/or appraises files on the security post=
> kernel read hook, based on policy.

I wasn't asking anything about IMA.

>
> Andy, other than IMA-appraisal being xattr based,=C2=A0 I'm not su= re why
> you're so against it.=C2=A0 =C2=A0if you're going to be at Lin= uxCon/LSS, perhaps
> we could speak in person there.

I have nothing against IMA.=C2=A0 I have plenty of objection= to the core hooks being simultaneously messy and not supplying enough info= rmation to provide the security properties they should provide.

If IMA doesn't want or need to verify the purpose of the= loaded file, fine.

I'll be at KS but not LSS.

--Andy

--001a1144212ab16f810538f2888b--