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.
next prev parent 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 \
/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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).