> On Aug 5, 2021, at 7:58 AM, Mimi Zohar <zohar@xxxxxxxxxxxxx> 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.
