Re: [PATCH 0/7] Add CA enforcement keyring restrictions

From: Mimi Zohar <zohar@linux.ibm.com>
To: Eric Snowberg <eric.snowberg@oracle.com>
Cc: David Howells <dhowells@redhat.com>,
	"dwmw2@infradead.org" <dwmw2@infradead.org>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"herbert@gondor.apana.org.au" <herbert@gondor.apana.org.au>,
	"davem@davemloft.net" <davem@davemloft.net>,
	"dmitry.kasatkin@gmail.com" <dmitry.kasatkin@gmail.com>,
	"jmorris@namei.org" <jmorris@namei.org>,
	"serge@hallyn.com" <serge@hallyn.com>,
	"roberto.sassu@huawei.com" <roberto.sassu@huawei.com>,
	Lakshmi Ramasubramanian <nramas@linux.microsoft.com>,
	"pvorel@suse.cz" <pvorel@suse.cz>,
	"tiwai@suse.de" <tiwai@suse.de>,
	"keyrings@vger.kernel.org" <keyrings@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-crypto@vger.kernel.org" <linux-crypto@vger.kernel.org>,
	"linux-security-module@vger.kernel.org" 
	<linux-security-module@vger.kernel.org>
Subject: Re: [PATCH 0/7] Add CA enforcement keyring restrictions
Date: Fri, 08 Apr 2022 10:41:26 -0400	[thread overview]
Message-ID: <fd5e88eb66db909ddc9f2fe6d788465a51a979b4.camel@linux.ibm.com> (raw)
In-Reply-To: <8ECDC8D2-433B-4F7E-9EEC-BB85C75ED198@oracle.com>

On Wed, 2022-04-06 at 22:53 +0000, Eric Snowberg wrote:
> 
> > On Apr 6, 2022, at 2:45 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > Hi Eric,
> > 
> > On Tue, 2022-04-05 at 21:53 -0400, Eric Snowberg wrote:
> >> A key added to the ima keyring must be signed by a key contained within 
> >> either the builtin trusted or secondary trusted keyrings. Currently, there are 
> >> CA restrictions described in IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY,
> >> but these restrictions are not enforced within code. Therefore, keys within 
> >> either the builtin or secondary may not be a CA and could be used to
> >> vouch for an ima key.
> >> 
> >> The machine keyring can not be used as another trust anchor for adding keys 
> >> to the ima keyring, since CA enforcement does not currently exist [1]. This 
> >> would expand the current integrity gap.
> >> 
> >> Introduce a new root of trust key flag to close this integrity gap for
> >> all keyrings.  The first key type to use this is X.509.  When a X.509 
> >> certificate is self signed, contains kernCertSign Key Usage and contains 
> >> the CA bit, the new flag is set.  Introduce new keyring restrictions 
> >> that not only validates a key is signed by a key contained within the 
> >> keyring, but also validates the key has the new root of trust key flag 
> >> set.  Use this new restriction for keys added to the ima keyring.  Now 
> >> that we have CA enforcement, allow the machine keyring to be used as another 
> >> trust anchor for the ima keyring.
> >> 
> >> To recap, all keys that previously loaded into the builtin, secondary or
> >> machine keyring will still load after applying this series.  Keys
> >> contained within these keyrings may carry the root of trust flag. The
> >> ima keyring will use the new root of trust restriction to validate
> >> CA enforcement. Other keyrings that require a root of trust could also 
> >> use this in the future.
> > 
> > Your initial patch set indicated that you were addressing Linus'
> > request to allow end-users the ability "to add their own keys and sign
> > modules they trust".  However, from the design of the previous patch
> > set and now this one, everything indicates a lot more is going on than
> > just allowing end-users to add their own keys.  There would be no
> > reason for loading all the MOK keys, rather than just the CA keys, onto
> > the "machine" keyring.  Please provide the motivation for this design.
> 
> The motivation is to satisfy both Linus and your requests. Linus requested 
> the ability to allow users to add their own keys and sign modules they trust.  
> A code signing CA certificate does not require kernCertSign in the usage. Adding 
> this as a requirement for kernel modules would be a regression (or a bug).

Of course a code signing CA certificate should not also be a
certificate signing key (keyCertSign).  Remember the
"builtin_trusted_keys" and "secondary_trusted_keys" keyrings are
special.  Their root of trust is based on a secure boot signature chain
of trust up to and including a signed kernel image.  The "machine"
keyring is totally different in this regard.  Establishing a new root
of trust is really difficult.  Requiring a root-CA to have key
certifcate signing usage is a level of indirection, which I would
consider a small price to pay for being able to establish a, hopefully
safe or at least safer, new root of trust for trusting "end-user" keys.

> 
> This series addresses your request to only trust validly signed CA certs. 
> As you pointed out in the Kconfig help for 
> IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY:
> 
> help
>   Keys may be added to the IMA or IMA blacklist keyrings, if the
>   key is validly signed by a CA cert in the system built-in or
>   secondary trusted keyrings.
> 
>   Intermediate keys between those the kernel has compiled in and the 
>   IMA keys to be added may be added to the system secondary keyring,
>   provided they are validly signed by a key already resident in the
>   built-in or secondary trusted keyrings.
> 
> requires keys to be “validly” signed by a CA cert. Later the definition of a 
> validly signed CA cert was defined as: self signed, contains kernCertSign 
> key usage and contains the CA bit. While this help file states the CA restriction, 
> nothing in code enforces it.  One can place any type of self signed cert in either 
> keyring and ima will use it.  The motivation is for all keys added to the ima 
> keyring to abide by the restriction defined in the Kconfig help.  With this series 
> this can be accomplished without introducing a regression on keys placed in 
> any of the system keyrings.
> 
> > Please note that Patch 6/7 permits intermediary CA keys, without any
> > mention of it in the cover letter.  Please include this in the
> > motivation for this design.
> 
> Ok, I’ll add that in the next round.

Your cover letter should say that this patch series enables
verification of 3rd party modules.

thanks,

Mimi