| Message ID | 20211124044124.998170-17-eric.snowberg@oracle.com (mailing list archive) |
|---|---|
| State | Not Applicable |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | Enroll kernel keys thru MOK | expand |
On Tuesday, 2021-11-23 at 23:41:23 -05, Eric Snowberg wrote: > A new Machine Owner Key (MOK) variable called MokListTrustedRT has been > introduced in shim. When this UEFI variable is set, it indicates the > end-user has made the decision themselves that they wish to trust MOK keys > within the Linux trust boundary. It is not an error if this variable > does not exist. If it does not exist, the MOK keys should not be trusted > within the kernel. > > Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> Reviewed-by: Darren Kenny <darren.kenny@oracle.com> > --- > v1: Initial version > v2: Removed mok_keyring_trust_setup function > v4: Unmodified from v2 > v5: Rename to machine keyring > v6: Unmodified from v5 > v7: Use mokvar table instead of EFI var (suggested by Peter Jones) > v8: Unmodified from v7 > --- > .../platform_certs/machine_keyring.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > > diff --git a/security/integrity/platform_certs/machine_keyring.c b/security/integrity/platform_certs/machine_keyring.c > index ea2ac2f9f2b5..09fd8f20c756 100644 > --- a/security/integrity/platform_certs/machine_keyring.c > +++ b/security/integrity/platform_certs/machine_keyring.c > @@ -5,6 +5,7 @@ > * Copyright (c) 2021, Oracle and/or its affiliates. > */ > > +#include <linux/efi.h> > #include "../integrity.h" > > static __init int machine_keyring_init(void) > @@ -40,3 +41,21 @@ void __init add_to_machine_keyring(const char *source, const void *data, size_t > if (rc) > pr_info("Error adding keys to machine keyring %s\n", source); > } > + > +/* > + * Try to load the MokListTrustedRT MOK variable to see if we should trust > + * the MOK keys within the kernel. It is not an error if this variable > + * does not exist. If it does not exist, MOK keys should not be trusted > + * within the machine keyring. > + */ > +static __init bool uefi_check_trust_mok_keys(void) > +{ > + struct efi_mokvar_table_entry *mokvar_entry; > + > + mokvar_entry = efi_mokvar_entry_find("MokListTrustedRT"); > + > + if (mokvar_entry) > + return true; > + > + return false; > +} > -- > 2.18.4
On Tue, Nov 23, 2021 at 11:41:23PM -0500, Eric Snowberg wrote: > A new Machine Owner Key (MOK) variable called MokListTrustedRT has been > introduced in shim. When this UEFI variable is set, it indicates the > end-user has made the decision themselves that they wish to trust MOK keys > within the Linux trust boundary. It is not an error if this variable > does not exist. If it does not exist, the MOK keys should not be trusted > within the kernel. Hi Eric, I've been milling around on this patch-set for a while and I have a few issues with the description of the commit and what the code actually does. efi_mokvar_entry_find doesn't simply read an UEFI variable as the commit message suggests, it will look for the MOK variable loaded into the EFI configuration table. This implies we need this table setup in early boot to take usage of this patch set. The only bootloader that does setup this table, is the `shim` as described. But no other bootloader implements support for the MOK EFI configuration table. This effectively means that there is still no way for Machine Owners to load keys into the keyring, for things like module signing, without the shim present in the bootchain. I find this a bit weird. Is this an intentional design decision, or could other ways be supported as well?
> On Nov 9, 2022, at 5:01 PM, Morten Linderud <morten@linderud.pw> wrote: > > On Tue, Nov 23, 2021 at 11:41:23PM -0500, Eric Snowberg wrote: >> A new Machine Owner Key (MOK) variable called MokListTrustedRT has been >> introduced in shim. When this UEFI variable is set, it indicates the >> end-user has made the decision themselves that they wish to trust MOK keys >> within the Linux trust boundary. It is not an error if this variable >> does not exist. If it does not exist, the MOK keys should not be trusted >> within the kernel. > > Hi Eric, > > I've been milling around on this patch-set for a while and I have a few issues > with the description of the commit and what the code actually does. > > efi_mokvar_entry_find doesn't simply read an UEFI variable as the commit message > suggests, it will look for the MOK variable loaded into the EFI configuration > table. This implies we need this table setup in early boot to take usage of this > patch set. > > The only bootloader that does setup this table, is the `shim` as described. But > no other bootloader implements support for the MOK EFI configuration table. > > This effectively means that there is still no way for Machine Owners to load > keys into the keyring, for things like module signing, without the shim present > in the bootchain. I find this a bit weird. > > Is this an intentional design decision, or could other ways be supported as > well? In v6 I had it as a RT variable, during the review a request was made [1] to just use the EFI configuration table. If there are other boot loaders that want to use this, I don’t see why the code in v6 couldn’t be added back. If the configuration table isn’t available, it could try reading the RT var next. 1. https://patchwork.kernel.org/project/linux-integrity/patch/20210914211416.34096-13-eric.snowberg@oracle.com/#24453409
On Thu, 10 Nov 2022 at 01:01, Morten Linderud <morten@linderud.pw> wrote: > > On Tue, Nov 23, 2021 at 11:41:23PM -0500, Eric Snowberg wrote: > > A new Machine Owner Key (MOK) variable called MokListTrustedRT has been > > introduced in shim. When this UEFI variable is set, it indicates the > > end-user has made the decision themselves that they wish to trust MOK keys > > within the Linux trust boundary. It is not an error if this variable > > does not exist. If it does not exist, the MOK keys should not be trusted > > within the kernel. > > Hi Eric, > > I've been milling around on this patch-set for a while and I have a few issues > with the description of the commit and what the code actually does. > > efi_mokvar_entry_find doesn't simply read an UEFI variable as the commit message > suggests, it will look for the MOK variable loaded into the EFI configuration > table. This implies we need this table setup in early boot to take usage of this > patch set. > > The only bootloader that does setup this table, is the `shim` as described. But > no other bootloader implements support for the MOK EFI configuration table. > Does any other bootloader implement support for the (volatile) MokListTrustedRT variable? Note that this variable is intentionally volatile, and should be rejected by the kernel if it is not. The point of these RT variables or the config tables is that they can only be set at boot if a signed and therefore trusted agent created them. Permitting non-volatile variables here defeats the purpose of secure boot, which aims to prevent exploits from gaining persistence. It would be bad if you could corrupt the trusted boot chain forever by setting a variable once. > This effectively means that there is still no way for Machine Owners to load > keys into the keyring, for things like module signing, without the shim present > in the bootchain. I find this a bit weird. > > Is this an intentional design decision, or could other ways be supported as > well? > Yes. If we are looking for a way to use EFI variables to inject additional certificates into the keyring without the ability to authenticate them, we should I'd strongly recommend that we disable that by default and add a big fat warning that it is incompatible with the guarantees secure boot aims to provide.
On Thu, 2022-11-10 at 01:01 +0100, Morten Linderud wrote: [...] > efi_mokvar_entry_find doesn't simply read an UEFI variable as the > commit message suggests, it will look for the MOK variable loaded > into the EFI configuration table. This implies we need this table > setup in early boot to take usage of this patch set. > > The only bootloader that does setup this table, is the `shim` as > described. But no other bootloader implements support for the MOK EFI > configuration table. Just to be precise: shim isn't a boot loader. It's a trust pivot device away from the built in UEFI keys to the Machine Owner Keys. Shim is designed to be used with another bootloader like grub or sd- boot. Now you could load a kernel directly with shim, in the same way you could load it directly from UEFI, but that doesn't make it a bootloader. > > This effectively means that there is still no way for Machine Owners > to load keys into the keyring, for things like module signing, > without the shim present in the bootchain. I find this a bit weird. > > Is this an intentional design decision, or could other ways be > supported as well? Yes, rather than try to have all bootloaders conform to the MoK protocol, it's easier to implement it in a single purpose component that can be used with any of them. Essentially if you want to rely on the UEFI keys and not do an MoK pivot (as some people do) then you can remove shim from the sequence. In many ways that's part of the problem with this patch set: The underlying assumption is everyone does this trust pivot. If you don't do this trust pivot (I don't for instance, having replaced my UEFI keys with my own) you can't add keys to the kernel this way. However, how would the kernel know whether you trust the UEFI keys or not? The other problem is that without the shim protocol being present, grub can't check the kernel signature, which means that even if you do own your own UEFI keys, you need something to replace shim, like: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/efitools.git/tree/ShimReplace.c James
On Thu, Nov 10, 2022 at 08:42:08AM +0100, Ard Biesheuvel wrote: > On Thu, 10 Nov 2022 at 01:01, Morten Linderud <morten@linderud.pw> wrote: > > > > On Tue, Nov 23, 2021 at 11:41:23PM -0500, Eric Snowberg wrote: > > > A new Machine Owner Key (MOK) variable called MokListTrustedRT has been > > > introduced in shim. When this UEFI variable is set, it indicates the > > > end-user has made the decision themselves that they wish to trust MOK keys > > > within the Linux trust boundary. It is not an error if this variable > > > does not exist. If it does not exist, the MOK keys should not be trusted > > > within the kernel. > > > > Hi Eric, > > > > I've been milling around on this patch-set for a while and I have a few issues > > with the description of the commit and what the code actually does. > > > > efi_mokvar_entry_find doesn't simply read an UEFI variable as the commit message > > suggests, it will look for the MOK variable loaded into the EFI configuration > > table. This implies we need this table setup in early boot to take usage of this > > patch set. > > > > The only bootloader that does setup this table, is the `shim` as described. But > > no other bootloader implements support for the MOK EFI configuration table. > > > > Does any other bootloader implement support for the (volatile) > MokListTrustedRT variable? > No. The efforts seems to be mostly focused on supporting a setup where shim boots grub. Anything besides this has never really been important for the people writing it. The main reason for the email is to try and figure out what the EFI configuration table adds to the threat model. > Note that this variable is intentionally volatile, and should be > rejected by the kernel if it is not. The point of these RT variables > or the config tables is that they can only be set at boot if a signed > and therefore trusted agent created them. So the only trusted agent currently is the shim? It claims to be a "trivial EFI application" but the interplay between mokutils and shim isn't documented and not trivial to understand. > Permitting non-volatile variables here defeats the purpose of secure > boot, which aims to prevent exploits from gaining persistence. It > would be bad if you could corrupt the trusted boot chain forever by > setting a variable once. Would this change if the variable could be read from the EFI configuration table or as an EFI variable? Instead of the current situation where we only support the configuration table? > > This effectively means that there is still no way for Machine Owners to load > > keys into the keyring, for things like module signing, without the shim present > > in the bootchain. I find this a bit weird. > > > > Is this an intentional design decision, or could other ways be supported as > > well? > > > > Yes. I assume it's a yes to "intentional design decision", and my followup question to this would be to ask where it's documented? > If we are looking for a way to use EFI variables to inject additional > certificates into the keyring without the ability to authenticate > them, we should I'd strongly recommend that we disable that by default > and add a big fat warning that it is incompatible with the guarantees > secure boot aims to provide. The present features are all disabled by default I believe? Most of this seems to be knowledge residing between a few people that was present at the implementation, and as a user-space tool author comming into this 10 years later, it's really hard to figure out how a lot of these decisions came to be.
On Thu, Nov 10, 2022 at 12:54:43AM +0000, Eric Snowberg wrote: > > > > On Nov 9, 2022, at 5:01 PM, Morten Linderud <morten@linderud.pw> wrote: > > > > On Tue, Nov 23, 2021 at 11:41:23PM -0500, Eric Snowberg wrote: > >> A new Machine Owner Key (MOK) variable called MokListTrustedRT has been > >> introduced in shim. When this UEFI variable is set, it indicates the > >> end-user has made the decision themselves that they wish to trust MOK keys > >> within the Linux trust boundary. It is not an error if this variable > >> does not exist. If it does not exist, the MOK keys should not be trusted > >> within the kernel. > > > > Hi Eric, > > > > I've been milling around on this patch-set for a while and I have a few issues > > with the description of the commit and what the code actually does. > > > > efi_mokvar_entry_find doesn't simply read an UEFI variable as the commit message > > suggests, it will look for the MOK variable loaded into the EFI configuration > > table. This implies we need this table setup in early boot to take usage of this > > patch set. > > > > The only bootloader that does setup this table, is the `shim` as described. But > > no other bootloader implements support for the MOK EFI configuration table. > > > > This effectively means that there is still no way for Machine Owners to load > > keys into the keyring, for things like module signing, without the shim present > > in the bootchain. I find this a bit weird. > > > > Is this an intentional design decision, or could other ways be supported as > > well? > > In v6 I had it as a RT variable, during the review a request was made [1] to just > use the EFI configuration table. If there are other boot loaders that want to use this, > I don’t see why the code in v6 couldn’t be added back. If the configuration table isn’t > available, it could try reading the RT var next. > > 1. https://patchwork.kernel.org/project/linux-integrity/patch/20210914211416.34096-13-eric.snowberg@oracle.com/#24453409 > If we could support both the EFI variables and the EFI configuration table setup it would hopefully be easier for others to implement the interface? I wouldn't mind trying to write a patch for that if others think it's a good idea. I'm not really sure what Peter means with "much more reliable" though.
On Thu, 2022-11-10 at 16:06 +0100, Morten Linderud wrote: > I'm not really sure what Peter means with "much more reliable" > though. It's that in-head knowledge you referred to. You can't see the true MoK variables because they're BootServices, meaning they're not visible in the RunTime, which is why the shadow RT variables exist (this is a security property: BS only variables can only be altered by trusted, signed entities). However lots of things can create RT variables so you have to run through a sequence of checks on the RT shadows to try to defeat clever attackers (like verifying the variable attributes), because the chain of custody from BS to RT is not guaranteed. If you use a configuration table instead, that is BS only, the kernel (which is also a trusted entity) has to pick it out before ExitBootServices, so if the kernel has the table, you have a reliable chain of custody for the entries. James
On Thu, 10 Nov 2022 at 16:27, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > On Thu, 2022-11-10 at 16:06 +0100, Morten Linderud wrote: > > I'm not really sure what Peter means with "much more reliable" > > though. > > It's that in-head knowledge you referred to. You can't see the true > MoK variables because they're BootServices, meaning they're not visible > in the RunTime, which is why the shadow RT variables exist (this is a > security property: BS only variables can only be altered by trusted, > signed entities). However lots of things can create RT variables so > you have to run through a sequence of checks on the RT shadows to try > to defeat clever attackers (like verifying the variable attributes), > because the chain of custody from BS to RT is not guaranteed. If you > use a configuration table instead, that is BS only, the kernel (which > is also a trusted entity) has to pick it out before ExitBootServices, > so if the kernel has the table, you have a reliable chain of custody > for the entries. > No config table are always accessible, also at runtime under the OS. But they are volatile so they can only have been created since the last reset of the system, so in that sense they are similar to the volatile RT variables aliases. The reason for preferring config tables is that you can access them much earlier, and without mapping the EFI runtime memory regions etc etc
diff --git a/security/integrity/platform_certs/machine_keyring.c b/security/integrity/platform_certs/machine_keyring.c index ea2ac2f9f2b5..09fd8f20c756 100644 --- a/security/integrity/platform_certs/machine_keyring.c +++ b/security/integrity/platform_certs/machine_keyring.c @@ -5,6 +5,7 @@ * Copyright (c) 2021, Oracle and/or its affiliates. */ +#include <linux/efi.h> #include "../integrity.h" static __init int machine_keyring_init(void) @@ -40,3 +41,21 @@ void __init add_to_machine_keyring(const char *source, const void *data, size_t if (rc) pr_info("Error adding keys to machine keyring %s\n", source); } + +/* + * Try to load the MokListTrustedRT MOK variable to see if we should trust + * the MOK keys within the kernel. It is not an error if this variable + * does not exist. If it does not exist, MOK keys should not be trusted + * within the machine keyring. + */ +static __init bool uefi_check_trust_mok_keys(void) +{ + struct efi_mokvar_table_entry *mokvar_entry; + + mokvar_entry = efi_mokvar_entry_find("MokListTrustedRT"); + + if (mokvar_entry) + return true; + + return false; +}
A new Machine Owner Key (MOK) variable called MokListTrustedRT has been introduced in shim. When this UEFI variable is set, it indicates the end-user has made the decision themselves that they wish to trust MOK keys within the Linux trust boundary. It is not an error if this variable does not exist. If it does not exist, the MOK keys should not be trusted within the kernel. Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com> --- v1: Initial version v2: Removed mok_keyring_trust_setup function v4: Unmodified from v2 v5: Rename to machine keyring v6: Unmodified from v5 v7: Use mokvar table instead of EFI var (suggested by Peter Jones) v8: Unmodified from v7 --- .../platform_certs/machine_keyring.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)