LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Eric Snowberg <eric.snowberg@oracle.com>
To: Mimi Zohar <zohar@linux.ibm.com>
Cc: keyrings@vger.kernel.org,
	linux-integrity <linux-integrity@vger.kernel.org>,
	David Howells <dhowells@redhat.com>,
	David Woodhouse <dwmw2@infradead.org>,
	Herbert Xu <herbert@gondor.apana.org.au>,
	"David S . Miller" <davem@davemloft.net>,
	Jarkko Sakkinen <jarkko@kernel.org>,
	James Morris <jmorris@namei.org>,
	"Serge E . Hallyn" <serge@hallyn.com>,
	keescook@chromium.org, gregkh@linuxfoundation.org,
	torvalds@linux-foundation.org, scott.branden@broadcom.com,
	weiyongjun1@huawei.com, nayna@linux.ibm.com, ebiggers@google.com,
	ardb@kernel.org, nramas@linux.microsoft.com, lszubowi@redhat.com,
	linux-kernel@vger.kernel.org, linux-crypto@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	James.Bottomley@HansenPartnership.com, pjones@redhat.com,
	glin@suse.com, "konrad.wilk@oracle.com" <konrad.wilk@oracle.com>
Subject: Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys
Date: Thu, 5 Aug 2021 19:29:44 -0600	[thread overview]
Message-ID: <44ADB68B-4310-462B-96A8-2F69759BA2D8@oracle.com> (raw)
In-Reply-To: <6c751dadf4ce7385d0391ea26f1c7e4e910219e0.camel@linux.ibm.com>


> On Aug 5, 2021, at 7:58 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Mon, 2021-07-26 at 13:13 -0400, Eric Snowberg wrote:
> 
>> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
>> index dcaf74102ab2..b27ae30eaadc 100644
>> --- a/certs/system_keyring.c
>> +++ b/certs/system_keyring.c
>> @@ -45,6 +45,15 @@ int restrict_link_by_builtin_trusted(struct key *dest_keyring,
>> 				     const union key_payload *payload,
>> 				     struct key *restriction_key)
>> {
>> +	/* If the secondary trusted keyring is not enabled, we may link
>> +	 * through to the mok keyring and the search may follow that link.
>> +	 */
> 
> Refer to section "8) Commenting" of Documentation/process/coding-
> style.rst for the format of multi line comments.

Sure, I’ll fix this in the next version.

>> +	if (mok_trusted_keys && type == &key_type_keyring &&
>> +	    dest_keyring == builtin_trusted_keys &&
>> +	    payload == &mok_trusted_keys->payload)
>> +		/* Allow the mok keyring to be added to the builtin */
>> +		return 0;
>> +
> 
> Unless you're changing the meaning of the restriction, then a new
> restriction needs to be defined.  In this case, please don't change the
> meaning of restrict_link_by_builtin_trusted().  Instead define a new
> restriction named restrict_link_by_builtin_and_ca_trusted().


Along with this

>> 	return restrict_link_by_signature(dest_keyring, type, payload,
>> 					  builtin_trusted_keys);
>> }
>> @@ -91,6 +100,15 @@ int restrict_link_by_builtin_and_secondary_trusted(
>> 		/* Allow the builtin keyring to be added to the secondary */
>> 		return 0;
>> 
>> +	/* If we have a secondary trusted keyring, it may contain a link
>> +	 * through to the mok keyring and the search may follow that link.
>> +	 */
>> +	if (mok_trusted_keys && type == &key_type_keyring &&
>> +	    dest_keyring == secondary_trusted_keys &&
>> +	    payload == &mok_trusted_keys->payload)
>> +		/* Allow the mok keyring to be added to the secondary */
>> +		return 0;
>> +
> 
> Similarly here, please define a new restriction maybe named
> restrict_link_by_builtin_secondary_and_ca_trusted().   To avoid code
> duplication, the new restriction could be a wrapper around the existing
> function.

and this too.

> 
>> 	return restrict_link_by_signature(dest_keyring, type, payload,
>> 					  secondary_trusted_keys);
>> }
>> @@ -321,5 +339,8 @@ void __init set_platform_trusted_keys(struct key *keyring)
>> void __init set_mok_trusted_keys(struct key *keyring)
>> {
>> 	mok_trusted_keys = keyring;
>> +
>> +	if (key_link(system_trusted_keys, mok_trusted_keys) < 0)
>> +		panic("Can't link (mok) trusted keyrings\n");
>> }
> 
> From the thread discussion on 00/12:
> 
> Only the builtin keys should ever be on the builtin keyring.  The
> builtin keyring would need to be linked to the mok keyring.  But in the
> secondary keyring case, the mok keyring would be linked to the
> secondary keyring, similar to how the builtin keyring is linked to the
> secondary keyring.
> 
>        if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
>                panic("Can't link trusted keyrings\n");


This part is confusing me though.

Here are some of the tests I’m performing with the current series:

Initial setup:
Create and enroll my own key into the MOK.
Sign a kernel, kernel module and IMA key with my new CA key.
Boot with lockdown enabled (to enforce sig validation).

Kernel built with CONFIG_SECONDARY_TRUSTED_KEYRING=y

$ keyctl show %:.secondary_trusted_keys
Keyring
 530463486 ---lswrv      0     0  keyring: .secondary_trusted_keys
 411466727 ---lswrv      0     0   \_ keyring: .builtin_trusted_keys
 979167715 ---lswrv      0     0   |   \_ asymmetric: Build time autogenerated kernel key: 07a56e29cfa1e21379aff2c522efff7d1963202a
 534573591 ---lswrv      0     0   |   \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
 968109018 ---lswrv      0     0   \_ keyring: .mok
 857795115 ---lswrv      0     0       \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b

With this setup I can:
* load a kernel module signed with my CA key
* run "kexec -ls" with the kernel signed with my CA key
* run "kexec -ls" with a kernel signed by a key in the platform keyring
* load another key into the secondary trusted keyring that is signed by my CA key
* load a key into the ima keyring, signed by my CA key

Kernel built without CONFIG_SECONDARY_TRUSTED_KEYRING defined

$ keyctl show %:.builtin_trusted_keys
Keyring
 812785375 ---lswrv      0     0  keyring: .builtin_trusted_keys
 455418681 ---lswrv      0     0   \_ keyring: .mok
 910809006 ---lswrv      0     0   |   \_ asymmetric: Erics-CA: UEK signing key: 9bfa6860483aa46bd83f7fa1289d9fc35799e93b
 115345009 ---lswrv      0     0   \_ asymmetric: Oracle-CA: Oracle certificate signing key: aeefb4bfde095cacaabff81dd266974b1b4e23b8
 513131506 ---lswrv      0     0   \_ asymmetric: Build time autogenerated kernel key: 22353509f203b55b84f15d0aadeddc134b646185

With this setup I can:
* load a kernel module signed with my CA key
* run "kexec -ls" with the kernel signed with my CA key
* run "kexec -ls" with a kernel signed by a key in the platform keyring
* load a key into the ima keyring, signed by my CA key

So why would the linking need to be switched?  Is there a test I’m
missing?  Thanks.


  reply	other threads:[~2021-08-06  1:30 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-26 17:13 [PATCH RFC v2 00/12] Enroll kernel keys thru MOK Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 01/12] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK) Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 02/12] KEYS: CA link restriction Eric Snowberg
2021-08-05 14:00   ` Mimi Zohar
2021-07-26 17:13 ` [PATCH RFC v2 03/12] integrity: Trust MOK keys if MokListTrustedRT found Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 04/12] integrity: add add_to_mok_keyring Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 05/12] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_system_trusted_or_ca Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 06/12] integrity: accessor function to get trust_moklist Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 07/12] integrity: add new keyring handler for mok keys Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 08/12] integrity: Suppress error message for keys added to the mok keyring Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 09/12] KEYS: add a reference to " Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys Eric Snowberg
2021-08-05 13:58   ` Mimi Zohar
2021-08-06  1:29     ` Eric Snowberg [this message]
2021-08-06  3:19       ` Mimi Zohar
2021-08-06 15:00         ` Eric Snowberg
2021-08-06 15:18           ` Mimi Zohar
2021-08-06 21:20             ` Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 11/12] integrity: Do not allow mok keyring updates following init Eric Snowberg
2021-07-26 17:13 ` [PATCH RFC v2 12/12] integrity: store reference to mok keyring Eric Snowberg
2021-08-03 17:01 ` [PATCH RFC v2 00/12] Enroll kernel keys thru MOK Mimi Zohar
2021-08-03 19:52   ` Eric Snowberg
2021-08-04  1:14     ` Mimi Zohar
2021-08-04  2:56       ` Eric Snowberg
2021-08-05 13:58         ` Mimi Zohar

Reply instructions:

You may reply publicly 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=44ADB68B-4310-462B-96A8-2F69759BA2D8@oracle.com \
    --to=eric.snowberg@oracle.com \
    --cc=James.Bottomley@HansenPartnership.com \
    --cc=ardb@kernel.org \
    --cc=davem@davemloft.net \
    --cc=dhowells@redhat.com \
    --cc=dwmw2@infradead.org \
    --cc=ebiggers@google.com \
    --cc=glin@suse.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=herbert@gondor.apana.org.au \
    --cc=jarkko@kernel.org \
    --cc=jmorris@namei.org \
    --cc=keescook@chromium.org \
    --cc=keyrings@vger.kernel.org \
    --cc=konrad.wilk@oracle.com \
    --cc=linux-crypto@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=lszubowi@redhat.com \
    --cc=nayna@linux.ibm.com \
    --cc=nramas@linux.microsoft.com \
    --cc=pjones@redhat.com \
    --cc=scott.branden@broadcom.com \
    --cc=serge@hallyn.com \
    --cc=torvalds@linux-foundation.org \
    --cc=weiyongjun1@huawei.com \
    --cc=zohar@linux.ibm.com \
    --subject='Re: [PATCH RFC v2 10/12] KEYS: link system_trusted_keys to mok_trusted_keys' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).