diff mbox series

[RFC,v2,10/12] KEYS: link system_trusted_keys to mok_trusted_keys

Message ID 20210726171319.3133879-11-eric.snowberg@oracle.com (mailing list archive)
State New
Headers show
Series Enroll kernel keys thru MOK | expand

Commit Message

Eric Snowberg July 26, 2021, 5:13 p.m. UTC
Allow the .mok keyring to be linked to either the builtin_trusted_keys
or the secondary_trusted_keys. If CONFIG_SECONDARY_TRUSTED_KEYRING is
enabled, mok keys are linked to the secondary_trusted_keys.  Otherwise
they are linked to the builtin_trusted_keys.  After the link is created,
keys contained in the .mok keyring will automatically be searched when
searching either builtin_trusted_keys or secondary_trusted_keys.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
---
 certs/system_keyring.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Mimi Zohar Aug. 5, 2021, 1:58 p.m. UTC | #1
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.

> +	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().

>  	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.

>  	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");


thanks,

Mimi

>  #endif
Eric Snowberg Aug. 6, 2021, 1:29 a.m. UTC | #2
> 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.
Mimi Zohar Aug. 6, 2021, 3:19 a.m. UTC | #3
On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:

> > 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.

It's a question of semantics.  The builtin keyring name is self
describing.  It should only contain the keys compiled into the kernel
or inserted post build into the reserved memory.  Not only the kernel
uses the builtin keyring, but userspace may as well[1].  Other users of
the builtin keyring might not want to trust the mok keyring as well.

thanks,

Mimi

[1] Refer to Mat Martineau's LSS 2019 talk titled "Using and
Implementing Keyring Restrictions in Userspace".
Eric Snowberg Aug. 6, 2021, 3 p.m. UTC | #4
> On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
> 
>>> 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.
> 
> It's a question of semantics.  The builtin keyring name is self
> describing.  It should only contain the keys compiled into the kernel
> or inserted post build into the reserved memory.  Not only the kernel
> uses the builtin keyring, but userspace may as well[1].  Other users of
> the builtin keyring might not want to trust the mok keyring as well.

Should this feature only work with kernels built with 
CONFIG_SECONDARY_TRUSTED_KEYRING defined?  If so, I could drop support in 
the next version for kernels built without it.
Mimi Zohar Aug. 6, 2021, 3:18 p.m. UTC | #5
On Fri, 2021-08-06 at 09:00 -0600, Eric Snowberg wrote:
> > On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> > 
> > On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
> > 
> >>> 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.
> > 
> > It's a question of semantics.  The builtin keyring name is self
> > describing.  It should only contain the keys compiled into the kernel
> > or inserted post build into the reserved memory.  Not only the kernel
> > uses the builtin keyring, but userspace may as well[1].  Other users of
> > the builtin keyring might not want to trust the mok keyring as well.
> 
> Should this feature only work with kernels built with 
> CONFIG_SECONDARY_TRUSTED_KEYRING defined?  If so, I could drop support in 
> the next version for kernels built without it.

Support for loading the CA keys stored in the MOK db onto the mok
keyring, only if the secondary keyring is configured would really
simplify the code.   Support for using the mok  keyring without the
secondary keyring being configured, could always be added later.  As
long as the other distros agree, I'm all for it.

thanks,

Mimi
Eric Snowberg Aug. 6, 2021, 9:20 p.m. UTC | #6
> On Aug 6, 2021, at 9:18 AM, Mimi Zohar <zohar@linux.ibm.com> wrote:
> 
> On Fri, 2021-08-06 at 09:00 -0600, Eric Snowberg wrote:
>>> On Aug 5, 2021, at 9:19 PM, Mimi Zohar <zohar@linux.ibm.com> wrote:
>>> 
>>> On Thu, 2021-08-05 at 19:29 -0600, Eric Snowberg wrote:
>>> 
>>>>> 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.
>>> 
>>> It's a question of semantics.  The builtin keyring name is self
>>> describing.  It should only contain the keys compiled into the kernel
>>> or inserted post build into the reserved memory.  Not only the kernel
>>> uses the builtin keyring, but userspace may as well[1].  Other users of
>>> the builtin keyring might not want to trust the mok keyring as well.
>> 
>> Should this feature only work with kernels built with 
>> CONFIG_SECONDARY_TRUSTED_KEYRING defined?  If so, I could drop support in 
>> the next version for kernels built without it.
> 
> Support for loading the CA keys stored in the MOK db onto the mok
> keyring, only if the secondary keyring is configured would really
> simplify the code.   Support for using the mok  keyring without the
> secondary keyring being configured, could always be added later.  As
> long as the other distros agree, I'm all for it.

Agreed, it will simplify the series and there is nothing preventing the 
dropped code from being added in the future if a different distro finds
it necessary. I’ll work on this in the next version along with the other 
changes you identified.  Thanks for your review.
diff mbox series

Patch

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.
+	 */
+	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;
+
 	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;
+
 	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");
 }
 #endif