Message-Id: <21EFCB58-2D89-4D30-8DA2-B952A7E1B1BD@oracle.com>
Date:   Thu, 8 Jul 2021 11:59:45 -0600
From:   Eric Snowberg <eric.snowberg@...cle.com>
To:     Mimi Zohar <zohar@...ux.ibm.com>
Cc:     keyrings@...r.kernel.org,
        linux-integrity <linux-integrity@...r.kernel.org>,
        David Howells <dhowells@...hat.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Herbert Xu <herbert@...dor.apana.org.au>,
        "David S . Miller" <davem@...emloft.net>,
        Jarkko Sakkinen <jarkko@...nel.org>,
        James Morris <jmorris@...ei.org>,
        "Serge E . Hallyn" <serge@...lyn.com>, keescook@...omium.org,
        gregkh@...uxfoundation.org, torvalds@...ux-foundation.org,
        scott.branden@...adcom.com, weiyongjun1@...wei.com,
        nayna@...ux.ibm.com, ebiggers@...gle.com, ardb@...nel.org,
        nramas@...ux.microsoft.com, lszubowi@...hat.com,
        linux-kernel@...r.kernel.org, linux-crypto@...r.kernel.org,
        linux-security-module@...r.kernel.org,
        James.Bottomley@...senPartnership.com, pjones@...hat.com,
        glin@...e.com, "konrad.wilk@...cle.com" <konrad.wilk@...cle.com>
Subject: Re: [PATCH RFC 00/12] Enroll kernel keys thru MOK


> On Jul 8, 2021, at 7:56 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
> 
> On Wed, 2021-07-07 at 16:10 -0600, Eric Snowberg wrote:
>>> On Jul 7, 2021, at 11:00 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
>>> 
>>> On Wed, 2021-07-07 at 10:28 -0600, Eric Snowberg wrote:
>>>>> On Jul 7, 2021, at 6:39 AM, Mimi Zohar <zohar@...ux.ibm.com> wrote:
>>>>> 
>>>>> On Tue, 2021-07-06 at 22:43 -0400, Eric Snowberg wrote:
>>>>>> This is a follow up to the "Add additional MOK vars" [1] series I 
>>>>>> previously sent.  This series incorporates the feedback given 
>>>>>> both publicly on the mailing list and privately from Mimi. This 
>>>>>> series just focuses on getting end-user keys into the kernel trust 
>>>>>> boundary.
>>>>>> 
>>>>>> Currently, pre-boot keys are not trusted within the Linux boundary [2].
>>>>>> Pre-boot keys include UEFI Secure Boot DB keys and MOKList keys. These
>>>>>> keys are loaded into the platform keyring and can only be used for kexec.
>>>>>> If an end-user wants to use their own key within the Linux trust
>>>>>> boundary, they must either compile it into the kernel themselves or use
>>>>>> the insert-sys-cert script. Both options present a problem. Many
>>>>>> end-users do not want to compile their own kernels. With the
>>>>>> insert-sys-cert option, there are missing upstream changes [3].  Also,
>>>>>> with the insert-sys-cert option, the end-user must re-sign their kernel
>>>>>> again with their own key, and then insert that key into the MOK db.
>>>>>> Another problem with insert-sys-cert is that only a single key can be
>>>>>> inserted into a compressed kernel.
>>>>>> 
>>>>>> Having the ability to insert a key into the Linux trust boundary opens
>>>>>> up various possibilities.  The end-user can use a pre-built kernel and
>>>>>> sign their own kernel modules.  It also opens up the ability for an
>>>>>> end-user to more easily use digital signature based IMA-appraisal.  To
>>>>>> get a key into the ima keyring, it must be signed by a key within the
>>>>>> Linux trust boundary.
>>>>>> 
>>>>>> Downstream Linux distros try to have a single signed kernel for each
>>>>>> architecture.  Each end-user may use this kernel in entirely different
>>>>>> ways.  Some downstream kernels have chosen to always trust platform keys
>>>>>> within the Linux trust boundary for kernel module signing.  These
>>>>>> kernels have no way of using digital signature base IMA appraisal. 
>>>>>> 
>>>>>> This series adds a new MOK variable to shim.  This variable allows the
>>>>>> end-user to decide if they want to trust keys enrolled in the MOK within
>>>>>> the Linux trust boundary.  By default, nothing changes; MOK keys are
>>>>>> not trusted within the Linux kernel.  They are only trusted after the 
>>>>>> end-user makes the decision themselves.  The end-user would set this
>>>>>> through mokutil using a new --trust-mok option [4]. This would work
>>>>>> similar to how the kernel uses MOK variable to enable/disable signature
>>>>>> validation as well as use/ignore the db.
>>>>>> 
>>>>>> When shim boots, it mirrors the new MokTML Boot Services variable to a new
>>>>>> MokListTrustedRT Runtime Services variable and extends PCR14. 
>>>>>> MokListTrustedRT is written without EFI_VARIABLE_NON_VOLATILE set,
>>>>>> preventing an end-user from setting it after booting and doing a kexec.
>>>>>> 
>>>>>> When the kernel boots, if MokListTrustedRT is set and
>>>>>> EFI_VARIABLE_NON_VOLATILE is not set, the MokListRT is loaded into the
>>>>>> secondary trusted keyring instead of the platform keyring. Mimi has
>>>>>> suggested that only CA keys or keys that can be vouched for by other
>>>>>> kernel keys be loaded. All other certs will load into the platform
>>>>>> keyring instead.
>>>>> 
>>>>> Loading MOK CA keys onto the "secondary" keyring would need to be an
>>>>> exception.   Once CA keys are loaded onto the "secondary" keyring,  any
>>>>> certificates signed by those CA keys may be loaded normally, without
>>>>> needing an exception, onto the "secondary" keyring.  The kernel MAY
>>>>> load those keys onto the "secondary" keyring, but really doesn't need
>>>>> to be involved.
>>>>> 
>>>>> Loading ALL of the MOK db keys onto either the "secondary" or
>>>>> "platform" keyrings makes the code a lot more complicated.  Is it
>>>>> really necessary?
>>>> 
>>>> Today all keys are loaded into the platform keyring. For kexec_file_load, 
>>>> the platform and secondary keys are trusted the same.  If this series were 
>>>> not to load them all into either keyring, it would be a kexec_file_load 
>>>> regression, since keys that previously loaded into the platform keyring 
>>>> could be missing. The complexity arises from the CA key restriction.  
>>>> If that requirement was removed, this series would be much smaller.
>>> 
>>> To prevent the regression, allow the the existing firmware/UEFI keys to
>>> continue to be loaded on the platform keyring, as it is currently being
>>> done.  The new code would load just the MOK db CA keys onto the
>>> secondary keyring, based on the new UEFI variable.  This is the only
>>> code that would require a
>>> "restrict_link_by_builtin_and_secondary_trusted" exemption.  The code
>>> duplication would be minimal in comparison to the complexity being
>>> introduced.
>> 
>> This series was written with the following three requirements in mind:
>> 
>> 1. Only CA keys that were originally bound for the platform keyring 
>> can enter the secondary keyring.
>> 
>> 2. No key in the UEFI Secure Boot DB, CA or not, may enter the 
>> secondary keyring, only MOKList keys may be trusted.
>> 
>> 3. A new MOK variable is added to signify the user wants to trust 
>> MOKList keys.
> 
> Sounds good!
> 
>> 
>> Given these requirements, I started down the path I think you are 
>> suggesting.  However I found it to be more complex.  If we load all 
>> keys into the platform keyring first and later try to load only CA keys, 
>> we don’t have a way of knowing where the platform key came from.  
>> Platform keys can originate from the UEFI Secure Boot DB or the MOKList. 
>> This would violate the second requirement. This caused me to need to 
>> create a new keyring handler. [PATCH RFC 10/12] integrity: add new 
>> keyring handler.
> 
> To prevent the regression you mentioned, I was suggesting reading the
> MOK DB twice.  One time loading all the keys onto the platform keyring.
> The other time loading only the CA keys onto the secondary keyring.
> 
>> 
>> To satisfy the first requirement a new restriction is required.  This 
>> is contained in [PATCH RFC 03/12] KEYS: CA link restriction.
>> 
>> To satisfy the third requirement, we must read the new MOK var.  This 
>> is contained in [PATCH RFC 06/12] integrity: Trust mok keys if 
>> MokListTrustedRT found.
>> 
>> The patches above make up a majority of the new code.  
>> 
>> The remaining code of creating a new .mok keyring was done with code 
>> reuse in mind. Many of the required functions necessary to add this 
>> capability is already contained in integrity_ functions.  If the 
>> operation was done directly on the secondary keyring, similar code 
>> would need to be added to certs/system_keyring.c. Just like how the 
>> platform keyring is created within integrity code, the mok keyring 
>> is created in the same fashion.  When the platform keyring has 
>> completed initialization and loaded all its keys, the keyring is set 
>> into system_keyring code using set_platform_trusted_keys.  Instead of 
>> setting the mok keyring, I’m moving the keys directly into the secondary 
>> keyring, while bypassing the current restriction placed on this keyring.
>> Basically I'm trying to follow the same design pattern. 
>> 
>> If requirements #1, #2 or both (#1 and #2) could be dropped, most of 
>> this series would not be necessary.
> 
> But without these requirements, the source of trust is unclear.
> 
> Is there a reason why the MOK keyring is temporary?  

I suppose it doesn't have to be temporary.  I was trying not to introduce
another keyring within system_keyring code.

>  Asumming a
> function similar to "restrict_link_by_builtin_and_secondary_trusted" is
> defined to include the MOK keyring, the CA keys in the MOK db would be
> loaded onto the MOK keyring, the other keys that meet the secondary
> keyring restriction would be loaded directly onto the secondary
> keyring[1], and as you currently have, the remaining keys onto the
> platform keyring.
> 
> This eliminates the exemption needed for loading keys onto the
> secondary keyring.  The MOK keyring, containing just CA keys, becomes a
> new trust source.

I just want to make sure I understand. If we kept the .mok keyring around, 
we would store it into the system_keyring code, just like the platform 
keyring is stored.  This would allow the move exemption code to be removed.
If the mok keyring is a new trust source, whenever the secondary keyring 
is referenced in verify_ code, the mok keyring will be checked too.  If 
I have this right, let me know and I’ll work on a v2.  Thanks.

Powered by blists - more mailing lists