* Re: [PATCH v4 2/2] tpm: ibmvtpm: Rename tpm_process_cmd to tpm_status and define flag
From: Jarkko Sakkinen @ 2021-08-12 19:48 UTC (permalink / raw)
To: Stefan Berger
Cc: Stefan Berger, nasastry, linux-integrity, linux-security-module,
linux-kernel, Nayna Jain, George Wilson
In-Reply-To: <4eff0296-78da-52b6-322d-56e0f9d78dc2@linux.ibm.com>
On Wed, Aug 11, 2021 at 08:15:14AM -0400, Stefan Berger wrote:
>
> On 8/10/21 10:10 PM, Jarkko Sakkinen wrote:
> > On Tue, Aug 10, 2021 at 09:50:55PM -0400, Stefan Berger wrote:
> > > On 8/10/21 1:58 PM, Jarkko Sakkinen wrote:
> > > > On Mon, Aug 09, 2021 at 03:21:59PM -0400, Stefan Berger wrote:
> > > > > From: Stefan Berger <stefanb@linux.ibm.com>
> > > > >
> > > > > Rename the field tpm_processing_cmd to tpm_status in ibmvtpm_dev and set
> > > > > the TPM_STATUS_BUSY flag while the vTPM is busy processing a command.
> > > > >
> > > > >
> > > > > default:
> > > > > diff --git a/drivers/char/tpm/tpm_ibmvtpm.h b/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > index 51198b137461..252f1cccdfc5 100644
> > > > > --- a/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > +++ b/drivers/char/tpm/tpm_ibmvtpm.h
> > > > > @@ -41,7 +41,8 @@ struct ibmvtpm_dev {
> > > > > wait_queue_head_t wq;
> > > > > u16 res_len;
> > > > > u32 vtpm_version;
> > > > > - u8 tpm_processing_cmd;
> > > > > + u8 tpm_status;
> > > > > +#define TPM_STATUS_BUSY (1 << 0) /* vtpm is processing a command */
> > > > Declare this already in the fix, and just leave the rename here.
> > > You mean the fix patch does not use 'true' anymore but uses the
> > > TPM_STATUS_BUSY flag already but the name is still tpm_processing_cmd? And
> > > literally only the renaming of this field is done in the 2nd patch?
> > I can fixup these patches, and use '1', instead of true. No need to send
> > new ones.
> >
> > Acked-by: Jarkko Sakkinen <jarkko@kernel.org>
I applied the first. If you have only one flag that you even
document as "processing the command" in the inline comment,
it makes absolutely no sense to rename it, as the current
name perfectly documents what it exactly is.
/Jarkko
^ permalink raw reply
* Re: [PATCH v3 10/14] KEYS: change link restriction for secondary to also trust mok
From: Mimi Zohar @ 2021-08-12 19:46 UTC (permalink / raw)
To: Eric Snowberg, keyrings, linux-integrity, dhowells, dwmw2,
herbert, davem, jarkko, jmorris, serge
Cc: keescook, gregkh, torvalds, scott.branden, weiyongjun1, nayna,
ebiggers, ardb, nramas, lszubowi, linux-kernel, linux-crypto,
linux-security-module, James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-11-eric.snowberg@oracle.com>
Hi Eric,
On Wed, 2021-08-11 at 22:18 -0400, Eric Snowberg wrote:
> With the introduction of the mok keyring, the end-user may choose to
> trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
> trust them, the .mok keyring will contain these keys. If not, the mok
> keyring will always be empty. Update the restriction check to allow the
> secondary trusted keyring to also trust mok keys.
>
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> ---
> v3: Initial version
> ---
> certs/system_keyring.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/certs/system_keyring.c b/certs/system_keyring.c
> index cb773e09ea67..8cc19a1ff051 100644
> --- a/certs/system_keyring.c
> +++ b/certs/system_keyring.c
> @@ -110,7 +110,7 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
> if (!restriction)
> panic("Can't allocate secondary trusted keyring restriction\n");
>
> - restriction->check = restrict_link_by_builtin_and_secondary_trusted;
> + restriction->check = restrict_link_by_builtin_secondary_and_ca_trusted;
>
> return restriction;
> }
Not everyone needs to build a generic kernel, like the distros. As
previously discussed, not everyone is willing to trust the new MOK
keyring nor the UEFI variable for enabling it. For those environments,
they should be able to totally disable the MOK keyring.
Please define a Kconfig similar to "CONFIG_SECONDARY_TRUSTED_KEYRING"
for MOK. The "restriction" would be based on the new Kconfig being
enabled.
thanks,
Mimi
^ permalink raw reply
* Re: [PATCH v3 04/14] integrity: add add_to_mok_keyring
From: Jarkko Sakkinen @ 2021-08-12 19:32 UTC (permalink / raw)
To: Eric Snowberg
Cc: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jmorris, serge, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-5-eric.snowberg@oracle.com>
On Wed, Aug 11, 2021 at 10:18:45PM -0400, Eric Snowberg wrote:
> Add the ability to load Machine Owner Key (MOK) keys to the mok keyring.
> If the permissions do not allow the key to be added to the mok keyring
> this is not an error, add it to the platform keyring instead.
Should state why it isn't an error for clarity.
/Jarkko
>
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> ---
> v1: Initial version
> v3: Unmodified from v1
> ---
> security/integrity/integrity.h | 4 ++++
> .../integrity/platform_certs/mok_keyring.c | 21 +++++++++++++++++++
> 2 files changed, 25 insertions(+)
>
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index e0e17ccba2e6..60d5c7ba05b2 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -278,9 +278,13 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
> #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
> void __init add_to_platform_keyring(const char *source, const void *data,
> size_t len);
> +void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
> #else
> static inline void __init add_to_platform_keyring(const char *source,
> const void *data, size_t len)
> {
> }
> +void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
> +{
> +}
> #endif
> diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
> index fe4f2d336260..f260edac0863 100644
> --- a/security/integrity/platform_certs/mok_keyring.c
> +++ b/security/integrity/platform_certs/mok_keyring.c
> @@ -21,6 +21,27 @@ static __init int mok_keyring_init(void)
> }
> device_initcall(mok_keyring_init);
>
> +void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
> +{
> + key_perm_t perm;
> + int rc;
> +
> + perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
> + rc = integrity_load_cert(INTEGRITY_KEYRING_MOK, source, data, len, perm);
> +
> + /*
> + * If the mok keyring restrictions prevented the cert from loading,
> + * this is not an error. Just load it into the platform keyring
> + * instead.
> + */
> + if (rc)
> + rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source,
> + data, len, perm);
> +
> + if (rc)
> + pr_info("Error adding keys to mok keyring %s\n", source);
> +}
> +
> /*
> * Try to load the MokListTrustedRT UEFI variable to see if we should trust
> * the mok keys within the kernel. It is not an error if this variable
> --
> 2.18.4
>
>
^ permalink raw reply
* Re: [PATCH v3 01/14] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
From: Jarkko Sakkinen @ 2021-08-12 18:58 UTC (permalink / raw)
To: Eric Snowberg
Cc: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jmorris, serge, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-2-eric.snowberg@oracle.com>
On Wed, Aug 11, 2021 at 10:18:42PM -0400, Eric Snowberg wrote:
> Many UEFI Linux distributions boot using shim. The UEFI shim provides
> what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
> Boot DB and MOK keys to validate the next step in the boot chain. The
> MOK facility can be used to import user generated keys. These keys can
> be used to sign an end-users development kernel build. When Linux
> boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux
> .platform keyring.
>
> Add a new Linux keyring called .mok. This keyring shall contain just
I would consider ".machine" instead. It holds MOK keys but is not a
MOK key.
> MOK keys and not the remaining keys in the platform keyring. This new
> .mok keyring will be used in follow on patches. Unlike keys in the
> platform keyring, keys contained in the .mok keyring will be trusted
> within the kernel if the end-user has chosen to do so.
>
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
> ---
> v1: Initial version
> v2: Removed destory keyring code
> v3: Unmodified from v2
> ---
> security/integrity/Makefile | 3 ++-
> security/integrity/digsig.c | 1 +
> security/integrity/integrity.h | 3 ++-
> .../integrity/platform_certs/mok_keyring.c | 21 +++++++++++++++++++
> 4 files changed, 26 insertions(+), 2 deletions(-)
> create mode 100644 security/integrity/platform_certs/mok_keyring.c
>
> diff --git a/security/integrity/Makefile b/security/integrity/Makefile
> index 7ee39d66cf16..8e2e98cba1f6 100644
> --- a/security/integrity/Makefile
> +++ b/security/integrity/Makefile
> @@ -9,7 +9,8 @@ integrity-y := iint.o
> integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
> integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
> integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
> -integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
> +integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o \
> + platform_certs/mok_keyring.o
> integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
> platform_certs/load_uefi.o \
> platform_certs/keyring_handler.o
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 3b06a01bd0fd..e07334504ef1 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -30,6 +30,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
> ".ima",
> #endif
> ".platform",
> + ".mok",
> };
>
> #ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 547425c20e11..e0e17ccba2e6 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -151,7 +151,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> #define INTEGRITY_KEYRING_EVM 0
> #define INTEGRITY_KEYRING_IMA 1
> #define INTEGRITY_KEYRING_PLATFORM 2
> -#define INTEGRITY_KEYRING_MAX 3
> +#define INTEGRITY_KEYRING_MOK 3
> +#define INTEGRITY_KEYRING_MAX 4
>
> extern struct dentry *integrity_dir;
>
> diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
> new file mode 100644
> index 000000000000..b1ee45b77731
> --- /dev/null
> +++ b/security/integrity/platform_certs/mok_keyring.c
> @@ -0,0 +1,21 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * MOK keyring routines.
> + *
> + * Copyright (c) 2021, Oracle and/or its affiliates.
> + */
> +
> +#include "../integrity.h"
> +
> +static __init int mok_keyring_init(void)
> +{
> + int rc;
> +
> + rc = integrity_init_keyring(INTEGRITY_KEYRING_MOK);
> + if (rc)
> + return rc;
> +
> + pr_notice("MOK Keyring initialized\n");
> + return 0;
> +}
> +device_initcall(mok_keyring_init);
> --
> 2.18.4
>
>
/Jarkko
^ permalink raw reply
* Re: [PATCH 1/1] NAX LSM: Add initial support support
From: Igor Zhbanov @ 2021-08-12 16:43 UTC (permalink / raw)
To: THOBY Simon
Cc: Igor Zhbanov, linux-integrity, linux-security-module, Mimi Zohar
In-Reply-To: <d97d7fdb-1676-9670-6cf5-2427f780ec6f@viveris.fr>
Hi Simon,
Thanks for thorough review. Will post the corrected version soon.
> > @@ -278,11 +279,11 @@ endchoice
> >
> > config LSM
> > string "Ordered list of enabled LSMs"
> > - default "landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
> > - default "landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
> > - default "landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
> > - default "landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
> > - default "landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
> > + default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,smack,selinux,tomoyo,apparmor,bpf" if DEFAULT_SECURITY_SMACK
> > + default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,apparmor,selinux,smack,tomoyo,bpf" if DEFAULT_SECURITY_APPARMOR
> > + default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,tomoyo,bpf" if DEFAULT_SECURITY_TOMOYO
> > + default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,bpf" if DEFAULT_SECURITY_DAC
> > + default "nax,landlock,lockdown,yama,loadpin,safesetid,integrity,selinux,smack,tomoyo,apparmor,bpf"
>
> I don't know the policy with regard to new LSMs, but enabling this one by default is maybe a bit violent?
> That said, considering the default mode is SECURITY_NAX_MODE_LOG, this would just log kernel messages and
> not break existing systems, so this shouldn't be a problem.
I've just oriended on landlock and lockdown. They are handled in the similar
way. But, yes, by default NAX module doesn't block anything.
If you suggest not to do that, I can remove it.
> > +__setup("nax_mode=", setup_mode);
> > +
> > +static int __init setup_quiet(char *str)
> > +{
> > + unsigned long val;
> > +
> > + if (!locked && !kstrtoul(str, 0, &val))
> > + quiet = val ? 1 : 0;
>
> The order of the kernel parameters will have an impact on NAX behavior.
>
> "nax_mode=1 nax_locked=1" and "nax_locked=1 nax_mode=1" will end up with the same behavior.
> in the first case the mode is enforced, in the second case it isn't (well unless you changed
> the kernel configuration from the default) and it can't be enabled later either.
>
> Is that desired?
Yes. The idea is that on boot you can set-up the proper options then block
further changing (especially turning the module off).
Initially it was implemented for sysctl parameters, so, you can change
something in init-scripts, then lock. Then, I've extended it to command-line
parameters.
I can ignore "locked" flag in setup_* functions to defer locking to sysctl
parsing. (Unless our command-line is glued by the bootloader from several
parts, so we want to lock it as early as possible...).
Thanks.
^ permalink raw reply
* Re: [RFC PATCH v2 5/9] fs: add anon_inode_getfile_secure() similar to anon_inode_getfd_secure()
From: Mickaël Salaün @ 2021-08-12 15:35 UTC (permalink / raw)
To: Paul Moore
Cc: linux-security-module, selinux, linux-audit, io-uring,
linux-fsdevel, Kumar Kartikeya Dwivedi, Jens Axboe,
Pavel Begunkov
In-Reply-To: <CAHC9VhRe3cgYuaV7w-BUwj_i=8_uuy3+5-8oA6QVsdXp3JgVtw@mail.gmail.com>
On 12/08/2021 16:32, Paul Moore wrote:
> On Thu, Aug 12, 2021 at 5:32 AM Mickaël Salaün <mic@digikod.net> wrote:
>> On 11/08/2021 22:48, Paul Moore wrote:
>>> Extending the secure anonymous inode support to other subsystems
>>> requires that we have a secure anon_inode_getfile() variant in
>>> addition to the existing secure anon_inode_getfd() variant.
>>>
>>> Thankfully we can reuse the existing __anon_inode_getfile() function
>>> and just wrap it with the proper arguments.
>>>
>>> Signed-off-by: Paul Moore <paul@paul-moore.com>
>>>
>>> ---
>>> v2:
>>> - no change
>>> v1:
>>> - initial draft
>>> ---
>>> fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
>>> include/linux/anon_inodes.h | 4 ++++
>>> 2 files changed, 33 insertions(+)
>>>
>>> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
>>> index a280156138ed..e0c3e33c4177 100644
>>> --- a/fs/anon_inodes.c
>>> +++ b/fs/anon_inodes.c
>>> @@ -148,6 +148,35 @@ struct file *anon_inode_getfile(const char *name,
>>> }
>>> EXPORT_SYMBOL_GPL(anon_inode_getfile);
>>>
>>> +/**
>>> + * anon_inode_getfile_secure - Like anon_inode_getfile(), but creates a new
>>> + * !S_PRIVATE anon inode rather than reuse the
>>> + * singleton anon inode and calls the
>>> + * inode_init_security_anon() LSM hook. This
>>> + * allows for both the inode to have its own
>>> + * security context and for the LSM to enforce
>>> + * policy on the inode's creation.
>>> + *
>>> + * @name: [in] name of the "class" of the new file
>>> + * @fops: [in] file operations for the new file
>>> + * @priv: [in] private data for the new file (will be file's private_data)
>>> + * @flags: [in] flags
>>> + * @context_inode:
>>> + * [in] the logical relationship with the new inode (optional)
>>> + *
>>> + * The LSM may use @context_inode in inode_init_security_anon(), but a
>>> + * reference to it is not held. Returns the newly created file* or an error
>>> + * pointer. See the anon_inode_getfile() documentation for more information.
>>> + */
>>> +struct file *anon_inode_getfile_secure(const char *name,
>>> + const struct file_operations *fops,
>>> + void *priv, int flags,
>>> + const struct inode *context_inode)
>>> +{
>>> + return __anon_inode_getfile(name, fops, priv, flags,
>>> + context_inode, true);
>>
>> This is not directly related to this patch but why using the "secure"
>> boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
>> checking that context_inode is not NULL? This would simplify the code,
>> remove this anon_inode_getfile_secure() wrapper and avoid potential
>> inconsistencies.
>
> The issue is that it is acceptable for the context_inode to be either
> valid or NULL for callers who request the "secure" code path.
>
> Look at the SELinux implementation of the anonymous inode hook in
> selinux_inode_init_security_anon() and you will see that in cases
> where the context_inode is valid we simply inherit the label from the
> given inode, whereas if context_inode is NULL we do a type transition
> using the requesting task and the anonymous inode's "name".
>
Indeed.
Acked-by: Mickaël Salaün <mic@linux.microsoft.com>
^ permalink raw reply
* Re: [PATCH 1/1] NAX LSM: Add initial support support
From: THOBY Simon @ 2021-08-12 14:47 UTC (permalink / raw)
To: J Freyensee, Igor Zhbanov, linux-integrity, linux-security-module
Cc: Igor Zhbanov, Mimi Zohar
In-Reply-To: <98408368-119a-b00c-97eb-8ea9fd1d5244@gmail.com>
On 8/10/21 6:52 AM, J Freyensee wrote:
[snip]
>> Have you considered writing to the audit log instead of the kernel messages directly?
>> (not saying that this is necessarily better, but is there a reasoning to prefer one or
>> the other here? Audit logs are often consumed by automated tools and it may be more pratical
>> for people to detect and treat violations if the messages were pushed to the audit log
>> - but conversely, that requires defining and maintaining a stable log format for consumers)
>
> It's a good idea to writing to the audit log, HOWEVER I'd want to know
> what all the rest of the LSMs are doing in a case like this. If all of
> them just write kernel messages, I'd want this module to also write just
> kernel messages for consistency sake for use with say, log harvesters
> for a SIEM/XDR system solution.
Right, after taking a quick look through the SafeSetID, YAMA and the future BRUTE
LSM, it looks like they all use pr_warn/pr_notice. Only the MACs seem to make use of
the audit log, so you can forget what I said about writing to the audit log, this
shouldn't be necessary, and is probably a bad idea for consistency, as Jay said.
Simon
>
> Just in general I like the thought of this LSM. I used to work for a
> security company in which their cloud "watched" situations where
> mmap()/mprotect() would use anonymous executable pages for possible
> "dodgy" behavior.
>
> Jay
>
^ permalink raw reply
* Re: [RFC PATCH v2 5/9] fs: add anon_inode_getfile_secure() similar to anon_inode_getfd_secure()
From: Paul Moore @ 2021-08-12 14:32 UTC (permalink / raw)
To: Mickaël Salaün
Cc: linux-security-module, selinux, linux-audit, io-uring,
linux-fsdevel, Kumar Kartikeya Dwivedi, Jens Axboe,
Pavel Begunkov
In-Reply-To: <1d19ca85-c6f9-7aa5-162a-f9728e0a8ccd@digikod.net>
On Thu, Aug 12, 2021 at 5:32 AM Mickaël Salaün <mic@digikod.net> wrote:
> On 11/08/2021 22:48, Paul Moore wrote:
> > Extending the secure anonymous inode support to other subsystems
> > requires that we have a secure anon_inode_getfile() variant in
> > addition to the existing secure anon_inode_getfd() variant.
> >
> > Thankfully we can reuse the existing __anon_inode_getfile() function
> > and just wrap it with the proper arguments.
> >
> > Signed-off-by: Paul Moore <paul@paul-moore.com>
> >
> > ---
> > v2:
> > - no change
> > v1:
> > - initial draft
> > ---
> > fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
> > include/linux/anon_inodes.h | 4 ++++
> > 2 files changed, 33 insertions(+)
> >
> > diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> > index a280156138ed..e0c3e33c4177 100644
> > --- a/fs/anon_inodes.c
> > +++ b/fs/anon_inodes.c
> > @@ -148,6 +148,35 @@ struct file *anon_inode_getfile(const char *name,
> > }
> > EXPORT_SYMBOL_GPL(anon_inode_getfile);
> >
> > +/**
> > + * anon_inode_getfile_secure - Like anon_inode_getfile(), but creates a new
> > + * !S_PRIVATE anon inode rather than reuse the
> > + * singleton anon inode and calls the
> > + * inode_init_security_anon() LSM hook. This
> > + * allows for both the inode to have its own
> > + * security context and for the LSM to enforce
> > + * policy on the inode's creation.
> > + *
> > + * @name: [in] name of the "class" of the new file
> > + * @fops: [in] file operations for the new file
> > + * @priv: [in] private data for the new file (will be file's private_data)
> > + * @flags: [in] flags
> > + * @context_inode:
> > + * [in] the logical relationship with the new inode (optional)
> > + *
> > + * The LSM may use @context_inode in inode_init_security_anon(), but a
> > + * reference to it is not held. Returns the newly created file* or an error
> > + * pointer. See the anon_inode_getfile() documentation for more information.
> > + */
> > +struct file *anon_inode_getfile_secure(const char *name,
> > + const struct file_operations *fops,
> > + void *priv, int flags,
> > + const struct inode *context_inode)
> > +{
> > + return __anon_inode_getfile(name, fops, priv, flags,
> > + context_inode, true);
>
> This is not directly related to this patch but why using the "secure"
> boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
> checking that context_inode is not NULL? This would simplify the code,
> remove this anon_inode_getfile_secure() wrapper and avoid potential
> inconsistencies.
The issue is that it is acceptable for the context_inode to be either
valid or NULL for callers who request the "secure" code path.
Look at the SELinux implementation of the anonymous inode hook in
selinux_inode_init_security_anon() and you will see that in cases
where the context_inode is valid we simply inherit the label from the
given inode, whereas if context_inode is NULL we do a type transition
using the requesting task and the anonymous inode's "name".
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [RFC PATCH v2 5/9] fs: add anon_inode_getfile_secure() similar to anon_inode_getfd_secure()
From: Mickaël Salaün @ 2021-08-12 9:32 UTC (permalink / raw)
To: Paul Moore, linux-security-module, selinux, linux-audit, io-uring,
linux-fsdevel, Kumar Kartikeya Dwivedi, Jens Axboe,
Pavel Begunkov
In-Reply-To: <162871492283.63873.8743976556992924333.stgit@olly>
On 11/08/2021 22:48, Paul Moore wrote:
> Extending the secure anonymous inode support to other subsystems
> requires that we have a secure anon_inode_getfile() variant in
> addition to the existing secure anon_inode_getfd() variant.
>
> Thankfully we can reuse the existing __anon_inode_getfile() function
> and just wrap it with the proper arguments.
>
> Signed-off-by: Paul Moore <paul@paul-moore.com>
>
> ---
> v2:
> - no change
> v1:
> - initial draft
> ---
> fs/anon_inodes.c | 29 +++++++++++++++++++++++++++++
> include/linux/anon_inodes.h | 4 ++++
> 2 files changed, 33 insertions(+)
>
> diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
> index a280156138ed..e0c3e33c4177 100644
> --- a/fs/anon_inodes.c
> +++ b/fs/anon_inodes.c
> @@ -148,6 +148,35 @@ struct file *anon_inode_getfile(const char *name,
> }
> EXPORT_SYMBOL_GPL(anon_inode_getfile);
>
> +/**
> + * anon_inode_getfile_secure - Like anon_inode_getfile(), but creates a new
> + * !S_PRIVATE anon inode rather than reuse the
> + * singleton anon inode and calls the
> + * inode_init_security_anon() LSM hook. This
> + * allows for both the inode to have its own
> + * security context and for the LSM to enforce
> + * policy on the inode's creation.
> + *
> + * @name: [in] name of the "class" of the new file
> + * @fops: [in] file operations for the new file
> + * @priv: [in] private data for the new file (will be file's private_data)
> + * @flags: [in] flags
> + * @context_inode:
> + * [in] the logical relationship with the new inode (optional)
> + *
> + * The LSM may use @context_inode in inode_init_security_anon(), but a
> + * reference to it is not held. Returns the newly created file* or an error
> + * pointer. See the anon_inode_getfile() documentation for more information.
> + */
> +struct file *anon_inode_getfile_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode)
> +{
> + return __anon_inode_getfile(name, fops, priv, flags,
> + context_inode, true);
This is not directly related to this patch but why using the "secure"
boolean in __anon_inode_getfile() and __anon_inode_getfd() instead of
checking that context_inode is not NULL? This would simplify the code,
remove this anon_inode_getfile_secure() wrapper and avoid potential
inconsistencies.
> +}
> +
> static int __anon_inode_getfd(const char *name,
> const struct file_operations *fops,
> void *priv, int flags,
> diff --git a/include/linux/anon_inodes.h b/include/linux/anon_inodes.h
> index 71881a2b6f78..5deaddbd7927 100644
> --- a/include/linux/anon_inodes.h
> +++ b/include/linux/anon_inodes.h
> @@ -15,6 +15,10 @@ struct inode;
> struct file *anon_inode_getfile(const char *name,
> const struct file_operations *fops,
> void *priv, int flags);
> +struct file *anon_inode_getfile_secure(const char *name,
> + const struct file_operations *fops,
> + void *priv, int flags,
> + const struct inode *context_inode);
> int anon_inode_getfd(const char *name, const struct file_operations *fops,
> void *priv, int flags);
> int anon_inode_getfd_secure(const char *name,
>
^ permalink raw reply
* [PATCH v3 12/14] integrity: Do not allow mok keyring updates following init
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
The mok keyring is setup during init. No additional keys should be allowed
to be added afterwards. Leave the permission as read only.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: Unmodified from v2
---
security/integrity/digsig.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index ec94d564c68a..0601ef458e03 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -145,7 +145,8 @@ int __init integrity_init_keyring(const unsigned int id)
else
restriction->check = restrict_link_to_ima;
- perm |= KEY_USR_WRITE;
+ if (id != INTEGRITY_KEYRING_MOK)
+ perm |= KEY_USR_WRITE;
out:
return __integrity_init_keyring(id, perm, restriction);
--
2.18.4
^ permalink raw reply related
* [PATCH v3 14/14] integrity: change ima link restriction to include mok keys
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
With the introduction of the mok keyring, the end-user may choose to
trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
trust them, the .mok keyring will contain these keys. If not, the mok
keyring will always be empty. Update the restriction check to allow the
ima keyring to also trust mok keys when the secondary keyring is also
trusted.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
---
security/integrity/digsig.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 996bea950972..1419ff4fc2b9 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -34,7 +34,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
};
#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
-#define restrict_link_to_ima restrict_link_by_builtin_and_secondary_trusted
+#define restrict_link_to_ima restrict_link_by_builtin_secondary_and_ca_trusted
#else
#define restrict_link_to_ima restrict_link_by_builtin_trusted
#endif
--
2.18.4
^ permalink raw reply related
* [PATCH v3 13/14] integrity: store reference to mok keyring
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Store a reference to the mok keyring in system keyring code. The
reference is only set when trust_moklist is true. This prevents the mok
keyring from linking to the secondary trusted keyrings with an empty mok
list.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: Unmodified from v2
---
security/integrity/digsig.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 0601ef458e03..996bea950972 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -112,6 +112,8 @@ static int __init __integrity_init_keyring(const unsigned int id,
} else {
if (id == INTEGRITY_KEYRING_PLATFORM)
set_platform_trusted_keys(keyring[id]);
+ if (id == INTEGRITY_KEYRING_MOK && trust_moklist())
+ set_mok_trusted_keys(keyring[id]);
if (id == INTEGRITY_KEYRING_IMA)
load_module_cert(keyring[id]);
}
--
2.18.4
^ permalink raw reply related
* [PATCH v3 02/14] KEYS: CA link restriction
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Add a new link restriction. Restrict the addition of keys in a keyring
based on the key to be added being a CA (self-signed).
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed secondary keyring references
v3: Removed restrict_link_by_system_trusted_or_ca
Simplify restrict_link_by_ca - only see if the key is a CA
Did not add __init in front of restrict_link_by_ca in case
restriction could be resued in the future
---
crypto/asymmetric_keys/restrict.c | 40 +++++++++++++++++++++++++++++++
include/crypto/public_key.h | 5 ++++
2 files changed, 45 insertions(+)
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 84cefe3b3585..9ae43d3f862b 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,46 @@ int restrict_link_by_signature(struct key *dest_keyring,
return ret;
}
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of CA keys
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trusted: Unused.
+ *
+ * Check if the new certificate is a CA. If it is a CA, then mark the new
+ * certificate as being ok to link.
+ *
+ * Returns 0 if the new certificate was accepted, -ENOKEY if we could not find
+ * a matching parent certificate in the trusted list. -ENOPKG if the signature
+ * uses unsupported crypto, or some other error if there is a matching
+ * certificate but the signature check cannot be performed.
+ */
+int restrict_link_by_ca(struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *trust_keyring)
+{
+ const struct public_key_signature *sig;
+ const struct public_key *pkey;
+
+ if (type != &key_type_asymmetric)
+ return -EOPNOTSUPP;
+
+ sig = payload->data[asym_auth];
+ if (!sig)
+ return -ENOPKG;
+
+ if (!sig->auth_ids[0] && !sig->auth_ids[1])
+ return -ENOKEY;
+
+ pkey = payload->data[asym_crypto];
+ if (!pkey)
+ return -ENOPKG;
+
+ return public_key_verify_signature(pkey, sig);
+}
+
static bool match_either_id(const struct asymmetric_key_ids *pair,
const struct asymmetric_key_id *single)
{
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index 47accec68cb0..545af1ea57de 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -71,6 +71,11 @@ extern int restrict_link_by_key_or_keyring_chain(struct key *trust_keyring,
const union key_payload *payload,
struct key *trusted);
+extern int restrict_link_by_ca(struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *trust_keyring);
+
extern int query_asymmetric_key(const struct kernel_pkey_params *,
struct kernel_pkey_query *);
--
2.18.4
^ permalink raw reply related
* [PATCH v3 06/14] integrity: accessor function to get trust_moklist
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Add an accessor function to see if the mok list should be trusted.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Added trust_moklist function
v3: Unmodified from v2
---
security/integrity/integrity.h | 5 +++++
security/integrity/platform_certs/mok_keyring.c | 16 ++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 60d5c7ba05b2..1fcefceb0da1 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -279,6 +279,7 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
void __init add_to_platform_keyring(const char *source, const void *data,
size_t len);
void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
+bool __init trust_moklist(void);
#else
static inline void __init add_to_platform_keyring(const char *source,
const void *data, size_t len)
@@ -287,4 +288,8 @@ static inline void __init add_to_platform_keyring(const char *source,
void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
{
}
+static inline bool __init trust_moklist(void)
+{
+ return false;
+}
#endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index f260edac0863..c7820d9136f3 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -8,6 +8,8 @@
#include <linux/efi.h>
#include "../integrity.h"
+bool trust_mok;
+
static __init int mok_keyring_init(void)
{
int rc;
@@ -67,3 +69,17 @@ static __init bool uefi_check_trust_mok_keys(void)
*/
return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
}
+
+bool __init trust_moklist(void)
+{
+ static bool initialized;
+
+ if (!initialized) {
+ initialized = true;
+
+ if (uefi_check_trust_mok_keys())
+ trust_mok = true;
+ }
+
+ return trust_mok;
+}
--
2.18.4
^ permalink raw reply related
* [PATCH v3 10/14] KEYS: change link restriction for secondary to also trust mok
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
With the introduction of the mok keyring, the end-user may choose to
trust Machine Owner Keys (MOK) within the kernel. If they have chosen to
trust them, the .mok keyring will contain these keys. If not, the mok
keyring will always be empty. Update the restriction check to allow the
secondary trusted keyring to also trust mok keys.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
---
certs/system_keyring.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index cb773e09ea67..8cc19a1ff051 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -110,7 +110,7 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
if (!restriction)
panic("Can't allocate secondary trusted keyring restriction\n");
- restriction->check = restrict_link_by_builtin_and_secondary_trusted;
+ restriction->check = restrict_link_by_builtin_secondary_and_ca_trusted;
return restriction;
}
--
2.18.4
^ permalink raw reply related
* [PATCH v3 11/14] KEYS: link secondary_trusted_keys to mok trusted keys
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Allow the .mok keyring to be linked to the secondary_trusted_keys. After
the link is created, keys contained in the .mok keyring will automatically
be searched when searching secondary_trusted_keys.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
---
certs/system_keyring.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 8cc19a1ff051..f6fcd53e3a0e 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -117,6 +117,9 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
void __init set_mok_trusted_keys(struct key *keyring)
{
mok_trusted_keys = keyring;
+
+ if (key_link(secondary_trusted_keys, mok_trusted_keys) < 0)
+ panic("Can't link (mok) trusted keyrings\n");
}
#endif
--
2.18.4
^ permalink raw reply related
* [PATCH v3 04/14] integrity: add add_to_mok_keyring
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Add the ability to load Machine Owner Key (MOK) keys to the mok keyring.
If the permissions do not allow the key to be added to the mok keyring
this is not an error, add it to the platform keyring instead.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v3: Unmodified from v1
---
security/integrity/integrity.h | 4 ++++
.../integrity/platform_certs/mok_keyring.c | 21 +++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index e0e17ccba2e6..60d5c7ba05b2 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -278,9 +278,13 @@ integrity_audit_log_start(struct audit_context *ctx, gfp_t gfp_mask, int type)
#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
void __init add_to_platform_keyring(const char *source, const void *data,
size_t len);
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len);
#else
static inline void __init add_to_platform_keyring(const char *source,
const void *data, size_t len)
{
}
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
+{
+}
#endif
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index fe4f2d336260..f260edac0863 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -21,6 +21,27 @@ static __init int mok_keyring_init(void)
}
device_initcall(mok_keyring_init);
+void __init add_to_mok_keyring(const char *source, const void *data, size_t len)
+{
+ key_perm_t perm;
+ int rc;
+
+ perm = (KEY_POS_ALL & ~KEY_POS_SETATTR) | KEY_USR_VIEW;
+ rc = integrity_load_cert(INTEGRITY_KEYRING_MOK, source, data, len, perm);
+
+ /*
+ * If the mok keyring restrictions prevented the cert from loading,
+ * this is not an error. Just load it into the platform keyring
+ * instead.
+ */
+ if (rc)
+ rc = integrity_load_cert(INTEGRITY_KEYRING_PLATFORM, source,
+ data, len, perm);
+
+ if (rc)
+ pr_info("Error adding keys to mok keyring %s\n", source);
+}
+
/*
* Try to load the MokListTrustedRT UEFI variable to see if we should trust
* the mok keys within the kernel. It is not an error if this variable
--
2.18.4
^ permalink raw reply related
* [PATCH v3 03/14] integrity: Trust MOK keys if MokListTrustedRT found
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
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 themself 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.
MOK variables are mirrored from Boot Services to Runtime Services. When
shim sees the new MokTML BS variable, it will create a new variable
(before Exit Boot Services is called) called MokListTrustedRT without
EFI_VARIABLE_NON_VOLATILE set. Following Exit Boot Services, UEFI
variables can only be set and created with SetVariable if both
EFI_VARIABLE_RUNTIME_ACCESS & EFI_VARIABLE_NON_VOLATILE are set.
Therefore, this can not be defeated by simply creating a
MokListTrustedRT variable from Linux, the existence of
EFI_VARIABLE_NON_VOLATILE will cause uefi_check_trust_mok_keys to return
false.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed mok_keyring_trust_setup function
v3: Unmodified from v2
---
.../integrity/platform_certs/mok_keyring.c | 27 +++++++++++++++++++
1 file changed, 27 insertions(+)
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
index b1ee45b77731..fe4f2d336260 100644
--- a/security/integrity/platform_certs/mok_keyring.c
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -5,6 +5,7 @@
* Copyright (c) 2021, Oracle and/or its affiliates.
*/
+#include <linux/efi.h>
#include "../integrity.h"
static __init int mok_keyring_init(void)
@@ -19,3 +20,29 @@ static __init int mok_keyring_init(void)
return 0;
}
device_initcall(mok_keyring_init);
+
+/*
+ * Try to load the MokListTrustedRT UEFI 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 kernel.
+ */
+static __init bool uefi_check_trust_mok_keys(void)
+{
+ efi_status_t status;
+ unsigned int mtrust = 0;
+ unsigned long size = sizeof(mtrust);
+ efi_guid_t guid = EFI_SHIM_LOCK_GUID;
+ u32 attr;
+
+ status = efi.get_variable(L"MokListTrustedRT", &guid, &attr, &size, &mtrust);
+
+ /*
+ * The EFI_VARIABLE_NON_VOLATILE check is to verify MokListTrustedRT
+ * was set thru shim mirrioring and not by a user from the host os.
+ * According to the UEFI spec, once EBS is performed, SetVariable()
+ * will succeed only when both EFI_VARIABLE_RUNTIME_ACCESS &
+ * EFI_VARIABLE_NON_VOLATILE are set.
+ */
+ return (status == EFI_SUCCESS && (!(attr & EFI_VARIABLE_NON_VOLATILE)));
+}
--
2.18.4
^ permalink raw reply related
* [PATCH v3 01/14] integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Many UEFI Linux distributions boot using shim. The UEFI shim provides
what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
Boot DB and MOK keys to validate the next step in the boot chain. The
MOK facility can be used to import user generated keys. These keys can
be used to sign an end-users development kernel build. When Linux
boots, both UEFI Secure Boot DB and MOK keys get loaded in the Linux
.platform keyring.
Add a new Linux keyring called .mok. This keyring shall contain just
MOK keys and not the remaining keys in the platform keyring. This new
.mok keyring will be used in follow on patches. Unlike keys in the
platform keyring, keys contained in the .mok keyring will be trusted
within the kernel if the end-user has chosen to do so.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed destory keyring code
v3: Unmodified from v2
---
security/integrity/Makefile | 3 ++-
security/integrity/digsig.c | 1 +
security/integrity/integrity.h | 3 ++-
.../integrity/platform_certs/mok_keyring.c | 21 +++++++++++++++++++
4 files changed, 26 insertions(+), 2 deletions(-)
create mode 100644 security/integrity/platform_certs/mok_keyring.c
diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 7ee39d66cf16..8e2e98cba1f6 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -9,7 +9,8 @@ integrity-y := iint.o
integrity-$(CONFIG_INTEGRITY_AUDIT) += integrity_audit.o
integrity-$(CONFIG_INTEGRITY_SIGNATURE) += digsig.o
integrity-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
-integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o
+integrity-$(CONFIG_INTEGRITY_PLATFORM_KEYRING) += platform_certs/platform_keyring.o \
+ platform_certs/mok_keyring.o
integrity-$(CONFIG_LOAD_UEFI_KEYS) += platform_certs/efi_parser.o \
platform_certs/load_uefi.o \
platform_certs/keyring_handler.o
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 3b06a01bd0fd..e07334504ef1 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -30,6 +30,7 @@ static const char * const keyring_name[INTEGRITY_KEYRING_MAX] = {
".ima",
#endif
".platform",
+ ".mok",
};
#ifdef CONFIG_IMA_KEYRINGS_PERMIT_SIGNED_BY_BUILTIN_OR_SECONDARY
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 547425c20e11..e0e17ccba2e6 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -151,7 +151,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
#define INTEGRITY_KEYRING_EVM 0
#define INTEGRITY_KEYRING_IMA 1
#define INTEGRITY_KEYRING_PLATFORM 2
-#define INTEGRITY_KEYRING_MAX 3
+#define INTEGRITY_KEYRING_MOK 3
+#define INTEGRITY_KEYRING_MAX 4
extern struct dentry *integrity_dir;
diff --git a/security/integrity/platform_certs/mok_keyring.c b/security/integrity/platform_certs/mok_keyring.c
new file mode 100644
index 000000000000..b1ee45b77731
--- /dev/null
+++ b/security/integrity/platform_certs/mok_keyring.c
@@ -0,0 +1,21 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * MOK keyring routines.
+ *
+ * Copyright (c) 2021, Oracle and/or its affiliates.
+ */
+
+#include "../integrity.h"
+
+static __init int mok_keyring_init(void)
+{
+ int rc;
+
+ rc = integrity_init_keyring(INTEGRITY_KEYRING_MOK);
+ if (rc)
+ return rc;
+
+ pr_notice("MOK Keyring initialized\n");
+ return 0;
+}
+device_initcall(mok_keyring_init);
--
2.18.4
^ permalink raw reply related
* [PATCH v3 00/14] Enroll kernel keys thru MOK
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
Many UEFI Linux distributions boot using shim. The UEFI shim provides
what is called Machine Owner Keys (MOK). Shim uses both the UEFI Secure
Boot DB and MOK keys to validate the next step in the boot chain. The
MOK facility can be used to import user generated keys. These keys can
be used to sign an end-user development kernel build. When Linux boots,
pre-boot keys (both UEFI Secure Boot DB and MOK keys) get loaded in the
Linux .platform keyring.
Currently, pre-boot keys are not trusted within the Linux trust boundary
[1]. These platform keys 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 [2]. 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 introduces a new Linux kernel keyring containing the Machine
Owner Keys (MOK) called .mok. It also 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
[3]. This would work similar to how the kernel uses MOK variables 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
mok keyring instead of the platform keyring. Mimi has suggested that
only CA keys be loaded into this keyring. All other certs will load
into the platform keyring instead.
The .mok keyring contains a new keyring permission that only allows CA
keys to be loaded. If the permission fails, the key is later loaded into
the platform keyring. After all keys are added into the .mok keyring,
they are linked to the secondary trusted keyring. After the link is
created, keys contained in the .mok keyring will automatically be
searched when searching the secondary trusted keys.
Secure Boot keys will never be trusted. They will always be loaded into
the platform keyring. If an end-user wanted to trust one, they would
need to enroll it into the MOK.
I have included links to both the mokutil [3] and shim [4] changes I
have made to support this new functionality.
V2 changes:
- The .mok keyring persists past boot
- Removed the unrestricted move into the secondary keyring
- Removed the keyring move bypass patch
- Added restrictions to allow the .mok to be linked to either the
builtin or secondary keyrings
- Secondary keyring dependency has been removed
V3 changes:
- Only CA keys contained in the MOKList are loaded, nothing else
- Support for kernels built without the secondary trusted keyring
has been dropped.
[1] https://lore.kernel.org/lkml/1556221605.24945.3.camel@HansenPartnership.com/
[2] https://lore.kernel.org/patchwork/cover/902768/
[3] https://github.com/esnowberg/mokutil/tree/0.3.0-mokvars-v2
[4] https://github.com/esnowberg/shim/tree/mokvars-v2
Eric Snowberg (14):
integrity: Introduce a Linux keyring for the Machine Owner Key (MOK)
KEYS: CA link restriction
integrity: Trust MOK keys if MokListTrustedRT found
integrity: add add_to_mok_keyring
integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca
integrity: accessor function to get trust_moklist
integrity: add new keyring handler for mok keys
KEYS: add a reference to mok keyring
KEYS: Introduce link restriction to include builtin, secondary and mok
keys
KEYS: change link restriction for secondary to also trust mok
KEYS: link secondary_trusted_keys to mok trusted keys
integrity: Do not allow mok keyring updates following init
integrity: store reference to mok keyring
integrity: change ima link restriction to include mok keys
certs/system_keyring.c | 33 ++++++-
crypto/asymmetric_keys/restrict.c | 40 +++++++++
include/crypto/public_key.h | 5 ++
include/keys/system_keyring.h | 10 +++
security/integrity/Makefile | 3 +-
security/integrity/digsig.c | 15 +++-
security/integrity/integrity.h | 12 ++-
.../platform_certs/keyring_handler.c | 17 +++-
.../platform_certs/keyring_handler.h | 5 ++
security/integrity/platform_certs/load_uefi.c | 4 +-
.../integrity/platform_certs/mok_keyring.c | 85 +++++++++++++++++++
11 files changed, 220 insertions(+), 9 deletions(-)
create mode 100644 security/integrity/platform_certs/mok_keyring.c
base-commit: 36a21d51725af2ce0700c6ebcb6b9594aac658a6
--
2.18.4
^ permalink raw reply
* [PATCH v3 09/14] KEYS: Introduce link restriction to include builtin, secondary and mok keys
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Introduce a new link restriction that includes the trusted builtin,
secondary and mok keys. The restriction is based on the key to be added
being vouched for by a key in any of these three keyrings.
Suggested-by: Mimi Zohar <zohar@linux.ibm.com>
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v3: Initial version
---
certs/system_keyring.c | 23 +++++++++++++++++++++++
include/keys/system_keyring.h | 6 ++++++
2 files changed, 29 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 2baf5447b116..cb773e09ea67 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -74,6 +74,29 @@ int restrict_link_by_builtin_and_secondary_trusted(
secondary_trusted_keys);
}
+/**
+ * restrict_link_by_builtin_secondary_and_ca_trusted
+ *
+ * Restrict the addition of keys into a keyring based on the key-to-be-added
+ * being vouched for by a key in either the built-in, the secondary, or
+ * the mok keyrings.
+ */
+int restrict_link_by_builtin_secondary_and_ca_trusted(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key)
+{
+ 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_builtin_and_secondary_trusted(dest_keyring, type,
+ payload, restrict_key);
+}
+
/**
* Allocate a struct key_restriction for the "builtin and secondary trust"
* keyring. Only for use in system_trusted_keyring_init().
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 4fe9cca58685..c9fcbfada567 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -34,9 +34,15 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
const struct key_type *type,
const union key_payload *payload,
struct key *restriction_key);
+extern int restrict_link_by_builtin_secondary_and_ca_trusted(
+ struct key *dest_keyring,
+ const struct key_type *type,
+ const union key_payload *payload,
+ struct key *restrict_key);
extern void __init set_mok_trusted_keys(struct key *keyring);
#else
#define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
+#define restrict_link_by_builtin_secondary_and_ca_trusted restrict_link_by_builtin_trusted
static inline void __init set_mok_trusted_keys(struct key *keyring)
{
}
--
2.18.4
^ permalink raw reply related
* [PATCH v3 05/14] integrity: restrict INTEGRITY_KEYRING_MOK to restrict_link_by_ca
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Set the restriction check for INTEGRITY_KEYRING_MOK keys to
restrict_link_by_ca. This will only allow CA keys into the mok
keyring.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Added !IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING check so mok
keyring gets created even when it isn't enabled
v3: Rename restrict_link_by_system_trusted_or_ca to restrict_link_by_ca
---
security/integrity/digsig.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index e07334504ef1..ec94d564c68a 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -132,7 +132,7 @@ int __init integrity_init_keyring(const unsigned int id)
goto out;
}
- if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING))
+ if (!IS_ENABLED(CONFIG_INTEGRITY_TRUSTED_KEYRING) && id != INTEGRITY_KEYRING_MOK)
return 0;
restriction = kzalloc(sizeof(struct key_restriction), GFP_KERNEL);
@@ -140,6 +140,11 @@ int __init integrity_init_keyring(const unsigned int id)
return -ENOMEM;
restriction->check = restrict_link_to_ima;
+ if (id == INTEGRITY_KEYRING_MOK)
+ restriction->check = restrict_link_by_ca;
+ else
+ restriction->check = restrict_link_to_ima;
+
perm |= KEY_USR_WRITE;
out:
--
2.18.4
^ permalink raw reply related
* [PATCH v3 08/14] KEYS: add a reference to mok keyring
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Expose the .mok keyring created in integrity code by adding
a reference. This makes the mok keyring accessible for keyring
restrictions in the future.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v2: Initial version
v3: set_mok_trusted_keys only available when secondary is enabled
---
certs/system_keyring.c | 5 +++++
include/keys/system_keyring.h | 4 ++++
2 files changed, 9 insertions(+)
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..2baf5447b116 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -21,6 +21,7 @@
static struct key *builtin_trusted_keys;
#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
static struct key *secondary_trusted_keys;
+static struct key *mok_trusted_keys;
#endif
#ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
static struct key *platform_trusted_keys;
@@ -90,6 +91,10 @@ static __init struct key_restriction *get_builtin_and_secondary_restriction(void
return restriction;
}
+void __init set_mok_trusted_keys(struct key *keyring)
+{
+ mok_trusted_keys = keyring;
+}
#endif
/*
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 6acd3cf13a18..4fe9cca58685 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -34,8 +34,12 @@ extern int restrict_link_by_builtin_and_secondary_trusted(
const struct key_type *type,
const union key_payload *payload,
struct key *restriction_key);
+extern void __init set_mok_trusted_keys(struct key *keyring);
#else
#define restrict_link_by_builtin_and_secondary_trusted restrict_link_by_builtin_trusted
+static inline void __init set_mok_trusted_keys(struct key *keyring)
+{
+}
#endif
extern struct pkcs7_message *pkcs7;
--
2.18.4
^ permalink raw reply related
* [PATCH v3 07/14] integrity: add new keyring handler for mok keys
From: Eric Snowberg @ 2021-08-12 2:18 UTC (permalink / raw)
To: keyrings, linux-integrity, zohar, dhowells, dwmw2, herbert, davem,
jarkko, jmorris, serge
Cc: eric.snowberg, keescook, gregkh, torvalds, scott.branden,
weiyongjun1, nayna, ebiggers, ardb, nramas, lszubowi,
linux-kernel, linux-crypto, linux-security-module,
James.Bottomley, pjones, glin, konrad.wilk
In-Reply-To: <20210812021855.3083178-1-eric.snowberg@oracle.com>
Currently both Secure Boot DB and Machine Owner Keys (MOK) go through
the same keyring handler (get_handler_for_db). With the addition of the
new mok keyring, the end-user may choose to trust MOK keys.
Introduce a new keyring handler specific for mok keys. If mok keys are
trusted by the end-user, use the new keyring handler instead.
Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v3: Only change the keyring handler if the secondary is enabled
---
.../integrity/platform_certs/keyring_handler.c | 17 ++++++++++++++++-
.../integrity/platform_certs/keyring_handler.h | 5 +++++
security/integrity/platform_certs/load_uefi.c | 4 ++--
3 files changed, 23 insertions(+), 3 deletions(-)
diff --git a/security/integrity/platform_certs/keyring_handler.c b/security/integrity/platform_certs/keyring_handler.c
index 5604bd57c990..b6daeb1e3de5 100644
--- a/security/integrity/platform_certs/keyring_handler.c
+++ b/security/integrity/platform_certs/keyring_handler.c
@@ -66,7 +66,7 @@ static __init void uefi_revocation_list_x509(const char *source,
/*
* Return the appropriate handler for particular signature list types found in
- * the UEFI db and MokListRT tables.
+ * the UEFI db tables.
*/
__init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
{
@@ -75,6 +75,21 @@ __init efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type)
return 0;
}
+/*
+ * Return the appropriate handler for particular signature list types found in
+ * the MokListRT tables.
+ */
+__init efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type)
+{
+ if (efi_guidcmp(*sig_type, efi_cert_x509_guid) == 0) {
+ if (IS_ENABLED(CONFIG_SECONDARY_TRUSTED_KEYRING) && trust_moklist())
+ return add_to_mok_keyring;
+ else
+ return add_to_platform_keyring;
+ }
+ return 0;
+}
+
/*
* Return the appropriate handler for particular signature list types found in
* the UEFI dbx and MokListXRT tables.
diff --git a/security/integrity/platform_certs/keyring_handler.h b/security/integrity/platform_certs/keyring_handler.h
index 2462bfa08fe3..284558f30411 100644
--- a/security/integrity/platform_certs/keyring_handler.h
+++ b/security/integrity/platform_certs/keyring_handler.h
@@ -24,6 +24,11 @@ void blacklist_binary(const char *source, const void *data, size_t len);
*/
efi_element_handler_t get_handler_for_db(const efi_guid_t *sig_type);
+/*
+ * Return the handler for particular signature list types found in the mok.
+ */
+efi_element_handler_t get_handler_for_mok(const efi_guid_t *sig_type);
+
/*
* Return the handler for particular signature list types found in the dbx.
*/
diff --git a/security/integrity/platform_certs/load_uefi.c b/security/integrity/platform_certs/load_uefi.c
index f290f78c3f30..c1bfd1cd7cc3 100644
--- a/security/integrity/platform_certs/load_uefi.c
+++ b/security/integrity/platform_certs/load_uefi.c
@@ -94,7 +94,7 @@ static int __init load_moklist_certs(void)
rc = parse_efi_signature_list("UEFI:MokListRT (MOKvar table)",
mokvar_entry->data,
mokvar_entry->data_size,
- get_handler_for_db);
+ get_handler_for_mok);
/* All done if that worked. */
if (!rc)
return rc;
@@ -109,7 +109,7 @@ static int __init load_moklist_certs(void)
mok = get_cert_list(L"MokListRT", &mok_var, &moksize, &status);
if (mok) {
rc = parse_efi_signature_list("UEFI:MokListRT",
- mok, moksize, get_handler_for_db);
+ mok, moksize, get_handler_for_mok);
kfree(mok);
if (rc)
pr_err("Couldn't parse MokListRT signatures: %d\n", rc);
--
2.18.4
^ permalink raw reply related
* Re: [PATCH v2] fscrypt: support trusted keys
From: Mimi Zohar @ 2021-08-12 0:54 UTC (permalink / raw)
To: Eric Biggers
Cc: Jarkko Sakkinen, Ahmad Fatoum, Theodore Y. Ts'o, Jaegeuk Kim,
kernel, James Morris, Serge E. Hallyn, James Bottomley,
Sumit Garg, David Howells, linux-fscrypt, linux-crypto,
linux-integrity, linux-security-module, keyrings, linux-kernel
In-Reply-To: <YRQF09f8st95yrFZ@gmail.com>
On Wed, 2021-08-11 at 10:16 -0700, Eric Biggers wrote:
> Neither of you actually answered my question, which is whether the support for
> trusted keys in dm-crypt is a mistake. I think you're saying that it is? That
> would imply that fscrypt shouldn't support trusted keys, but rather encrypted
> keys -- which conflicts with Ahmad's patch which is adding support for trusted
> keys. Note that your reasoning for this is not documented at all in the
> trusted-encrypted keys documentation; it needs to be (email threads don't really
> matter), otherwise how would anyone know when/how to use this feature?
True, but all of the trusted-encrypted key examples in the
documentation are "encrypted" type keys, encrypted/decrypted based on a
"trusted" type key. There are no examples of using the "trusted" key
type directly. Before claiming that adding "trusted" key support in
dm-crypt was a mistake, we should ask Ahmad why he felt dm-crypt needed
to directly support "trusted" type keys.
Mimi
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox