Message-ID: <CAHC9VhR9z2WVp+Q1hBkr5jduAH-J1DW12-k7U1eaQGgrP4c1Yw@mail.gmail.com>
Date:	Wed, 26 Aug 2015 22:35:19 -0400
From:	Paul Moore <paul@...l-moore.com>
To:	"Luis R. Rodriguez" <mcgrof@...e.com>
Cc:	David Howells <dhowells@...hat.com>,
	Mimi Zohar <zohar@...ux.vnet.ibm.com>,
	Andy Lutomirski <luto@...capital.net>,
	Kees Cook <keescook@...omium.org>,
	"Roberts, William C" <william.c.roberts@...el.com>,
	"linux-security-module@...r.kernel.org" 
	<linux-security-module@...r.kernel.org>,
	linux-kernel@...r.kernel.org, linux-wireless@...r.kernel.org,
	"james.l.morris@...cle.com" <james.l.morris@...cle.com>,
	"serge@...lyn.com" <serge@...lyn.com>,
	Vitaly Kuznetsov <vkuznets@...hat.com>,
	Eric Paris <eparis@...isplace.org>, selinux@...ho.nsa.gov,
	Stephen Smalley <sds@...ho.nsa.gov>,
	"Schaufler, Casey" <casey.schaufler@...el.com>,
	"Luis R. Rodriguez" <mcgrof@...not-panic.com>,
	Dmitry Kasatkin <dmitry.kasatkin@...il.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Peter Jones <pjones@...hat.com>, Takashi Iwai <tiwai@...e.de>,
	dwmw2@...radead.org, Ming Lei <ming.lei@...onical.com>,
	Joey Lee <jlee@...e.de>,
	Vojtěch Pavlík <vojtech@...e.com>,
	Kyle McMartin <kyle@...nel.org>,
	Seth Forshee <seth.forshee@...onical.com>,
	Matthew Garrett <mjg59@...f.ucam.org>,
	Johannes Berg <johannes@...solutions.net>
Subject: Re: Linux Firmware Signing

On Wed, Aug 26, 2015 at 7:26 PM, Luis R. Rodriguez <mcgrof@...e.com> wrote:
> On Wed, Aug 26, 2015 at 03:33:04PM +0100, David Howells wrote:
> Now let's review the SELinux stuff before we jump back into firmware / system
> data stuff again as there is a joint criteria to consider for all of these.
> For other people's refrence the enum you quote above was added through your
> patch pending on linux-next:
>
> "PKCS#7: Appropriately restrict authenticated attributes and content type"
>
> Based on what Roberts seems to want to do for SELinux policy files it would
> seems we may also need VERIFYING_SELINUX_POLICY. SELinux policy loading is
> unique in the at it uses its own fs and uses a load trigger node (sel_load_ops)
> to kick off  security_load_policy(data, count), so its not exactly a
> yet-another-API to read arbitrary files from the file system. Its policy files
> are also very distribution specific. Because of all this its not really
> suitable for /lib/firmware/ or sharing code even futher. It seems its a prime
> candidate already to make use of the system_verify_data() APIs you added David,
> provided the items below are taken care of as well.

One thing to keep in mind is that not only are SELinux policy files
distribution specific, they are machine specific as administrators
can, and do, customize the policy for their usage.  I really like the
idea of providing signed SELinux policies to the kernel but I question
how practical it will be for normal users/admins.

Some of the Machine Owner Key (MOK) work would likely be necessary for
signed SELinux policies to be even remotely practical.

>> Also, do I actually need to label signatures with the name that will be passed
>> to request_firmware()?  Or is it better to do it just by requiring each driver
>> that wants a piece of firmware to hold a key or a reference to a key or a hash
>> for the particular piece of firmware that it wants?  Having a name in the
>> signature at least makes for more readable error messages, but...
>
> I'll chime in, but I'd like other people's feeback as well, in
> particular Andy and Roberts. As it stands, you had this patch which was
> not yet posted, in it you have a named attributes for firmware:
>
> "PKCS#7: Add an optional authenticated attribute to hold firmware name"
> https://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/commit/?h=fwsign-pkcs7&id=1448377a369993f864915743cfb34772e730213good
>
>         1.3.6.1.4.1.2312.16     Linux kernel
>         1.3.6.1.4.1.2312.16.2   - PKCS#7/CMS SignerInfo attribute types
>         1.3.6.1.4.1.2312.16.2.1   - firmwareName
>
> I take it you are referring to this? If we follow this model we'd then need
> something like:
>
>         1.3.6.1.4.1.2312.16.2.2   - seLinuxPolicyName

Assuming I'm understanding the firmwareName attribute idea correctly,
we don't need to worry about that from a SELinux policy point of view.
As others have already stated, the kernel just reads a binary blob
that is pushed into it by userspace using securityfs.

> If the driver specifies the trusted key through its desciptor, as you suggest,
> I do agree we could do away with this. The change here would be that each
> driver that wants firmware signing should opt-in carefully with the key that it
> does trust. Most active vendors would swiftly want to only trust their own key.

See my earlier comments about SELinux policy customization.

> With this model then we would have these options:
>
>   * SYSTEM_DATA_SIG enabled
>
> Enables system data signing, drivers need to be explicit about wanting
> this as well. By default Kyle's key is trusted if the API is used, this
> does require an explicit change to the new API though. Support for signing
> is available but by default its permissive. A kernel parameter exists so
> that upon bootup its in enforced mode. Drivers can still override
> this and *require* the signature always, but if they want that they need to be
> explicit about it on their driver call. They can do this through their own
> Kconfig option or just always have it, up to them.
>
>   * SYSTEM_DATA_SIG_FORCE enabled
>
> Requires all uses of the API to have a trusted key before passing data back.
>
>   * No Kconfig option for SYSTEM_DATA_SIG set
>
> Uses of desc->signature_required will fail to compile, folks will need to
> wrap this descriptor's setting with either SYSTEM_DATA_SIG or their own
> Kconfig option which selects SYSTEM_DATA_SIG.
>
> SELinux could copy this model, they could end up with for instance:
>
>   * SECURITY_SELINUX_POLICY_SIGNED enabled
>
> Enables signature on the policy file. A kernel parameter could kick the
> signature to be required.
>
>   * SECURITY_SELINUX_POLICY_SIGNED_FORCE enabled
>
> The policy files require a signature.
>
> Does this seem fine? If so then ideally we'd want the dynamic OID registry but
> I don't think that's a requirement, for now we could add users as time goes by.
> Other than that, I think we'd need to define the allowed crypto options that can
> be passed to system_verify_data() ? Uptimately this is also what we want to
> enable drivers to fill in through a static descriptor.
>
>   Luis

-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists