| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <0dcd6ccea49026950608d8ddde5700ace84a2548.camel@linux.ibm.com>
Date: Mon, 23 Dec 2024 19:01:04 -0500
From: Mimi Zohar <zohar@...ux.ibm.com>
To: Eric Snowberg <eric.snowberg@...cle.com>,
linux-security-module@...r.kernel.org
Cc: dhowells@...hat.com, dwmw2@...radead.org, herbert@...dor.apana.org.au,
davem@...emloft.net, ardb@...nel.org, jarkko@...nel.org,
paul@...l-moore.com, jmorris@...ei.org, serge@...lyn.com,
roberto.sassu@...wei.com, dmitry.kasatkin@...il.com, mic@...ikod.net,
casey@...aufler-ca.com, stefanb@...ux.ibm.com, ebiggers@...nel.org,
rdunlap@...radead.org, linux-kernel@...r.kernel.org,
keyrings@...r.kernel.org, linux-crypto@...r.kernel.org,
linux-efi@...r.kernel.org, linux-integrity@...r.kernel.org
Subject: Re: [RFC PATCH v3 03/13] clavis: Introduce a new system keyring
called clavis
On Thu, 2024-10-17 at 09:55 -0600, Eric Snowberg wrote:
> Introduce a new system keyring called clavis. This keyring shall contain
> a single asymmetric key. This key may be a linked to a key already
> contained in one of the system keyrings (builtin, secondary, or platform).
Although "This key may be a linked to ..." is might be correct. Being
introduced in this patch is only the ability of loading a key by specifying it
on the boot command line. In this case, the key must be on one of the system
keyrings.
> One way to add this key into this keyring is during boot by passing in the
> asymmetric key id within the new "clavis=" boot param. If a matching key
> is found in one of the system keyrings, a link shall be created. This
> keyring will be used in the future by the new Clavis LSM.
>
> Signed-off-by: Eric Snowberg <eric.snowberg@...cle.com>
> ---
> .../admin-guide/kernel-parameters.txt | 6 +
> include/linux/integrity.h | 8 ++
> security/Kconfig | 1 +
> security/Makefile | 1 +
> security/clavis/Kconfig | 11 ++
> security/clavis/Makefile | 3 +
> security/clavis/clavis.h | 13 ++
> security/clavis/clavis_keyring.c | 115 ++++++++++++++++++
> security/integrity/iint.c | 2 +
> 9 files changed, 160 insertions(+)
> create mode 100644 security/clavis/Kconfig
> create mode 100644 security/clavis/Makefile
> create mode 100644 security/clavis/clavis.h
> create mode 100644 security/clavis/clavis_keyring.c
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 1518343bbe22..d71397e7d254 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -645,6 +645,12 @@
> cio_ignore= [S390]
> See Documentation/arch/s390/common_io.rst for details.
>
> + clavis= [SECURITY,EARLY]
> + Identifies a specific key contained in one of the system
> + keyrings (builtin, secondary, or platform) to be used as
> + the Clavis root of trust.
> + Format: { <keyid> }
Include .machine keyring here.
> +
> clearcpuid=X[,X...] [X86]
> Disable CPUID feature X for the kernel. See
> arch/x86/include/asm/cpufeatures.h for the valid bit
> diff --git a/include/linux/integrity.h b/include/linux/integrity.h
> index f5842372359b..837c52e1d83b 100644
> --- a/include/linux/integrity.h
> +++ b/include/linux/integrity.h
> @@ -23,6 +23,14 @@ enum integrity_status {
> #ifdef CONFIG_INTEGRITY
> extern void __init integrity_load_keys(void);
>
> +#ifdef CONFIG_SECURITY_CLAVIS
> +void __init late_init_clavis_setup(void);
> +#else
> +static inline void late_init_clavis_setup(void)
> +{
> +}
> +#endif
> +
> #else
> static inline void integrity_load_keys(void)
> {
> diff --git a/security/Kconfig b/security/Kconfig
> index 28e685f53bd1..714ec08dda96 100644
> --- a/security/Kconfig
> +++ b/security/Kconfig
> @@ -225,6 +225,7 @@ source "security/safesetid/Kconfig"
> source "security/lockdown/Kconfig"
> source "security/landlock/Kconfig"
> source "security/ipe/Kconfig"
> +source "security/clavis/Kconfig"
>
> source "security/integrity/Kconfig"
>
> diff --git a/security/Makefile b/security/Makefile
> index cc0982214b84..69576551007a 100644
> --- a/security/Makefile
> +++ b/security/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_CGROUPS) += device_cgroup.o
> obj-$(CONFIG_BPF_LSM) += bpf/
> obj-$(CONFIG_SECURITY_LANDLOCK) += landlock/
> obj-$(CONFIG_SECURITY_IPE) += ipe/
> +obj-$(CONFIG_SECURITY_CLAVIS) += clavis/
>
> # Object integrity file lists
> obj-$(CONFIG_INTEGRITY) += integrity/
> diff --git a/security/clavis/Kconfig b/security/clavis/Kconfig
> new file mode 100644
> index 000000000000..04f7565f2e2b
> --- /dev/null
> +++ b/security/clavis/Kconfig
> @@ -0,0 +1,11 @@
> +config SECURITY_CLAVIS
> + bool "Clavis keyring"
Isn't SECURITY_CLAVIS the new LSM? Why is the bool defined as just "Clavis
keyring"?
> + depends on SECURITY
> + select SYSTEM_DATA_VERIFICATION
> + select CRYPTO_SHA256
> + help
> + Enable the clavis keyring. This keyring shall contain a single asymmetric key.
> + This key shall be linked to a key already contained in one of the system
> + keyrings (builtin, secondary, or platform). One way to add this key
> + is during boot by passing in the asymmetric key id within the "clavis=" boot
> + param. This keyring is required by the Clavis LSM.
If SECURITY_CLAVIS is a new LSM, the 'help' shouldn't be limited to just the
clavis keyring, but written at a higher level describing the new LSM. For
example,
This option enables the Clavis LSM, which provides the ability to configure and
enforce the usage of keys contained on the system keyrings -
.builtin_trusted_keys, .secondary_trusted_keys, .machine, and .platform
keyrings. The clavis LSM defines a keyring named "clavis", which contains a
single asymmetric key and the key usage rules.
The single asymmetric key may be specified on the boot command line ...
[The patch that introduces the key usage rules would add additional info here.]
[The patch that adds the Documentatoin would add a reference here.]
> diff --git a/security/clavis/Makefile b/security/clavis/Makefile
> new file mode 100644
> index 000000000000..16c451f45f37
> --- /dev/null
> +++ b/security/clavis/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_SECURITY_CLAVIS) += clavis_keyring.o
> diff --git a/security/clavis/clavis.h b/security/clavis/clavis.h
> new file mode 100644
> index 000000000000..5e397b55a60a
> --- /dev/null
> +++ b/security/clavis/clavis.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef _SECURITY_CLAVIS_H_
> +#define _SECURITY_CLAVIS_H_
> +#include <keys/asymmetric-type.h>
> +
> +/* Max length for the asymmetric key id contained on the boot param */
> +#define CLAVIS_BIN_KID_MAX 32
> +
> +struct asymmetric_setup_kid {
> + struct asymmetric_key_id id;
> + unsigned char data[CLAVIS_BIN_KID_MAX];
> +};
> +#endif /* _SECURITY_CLAVIS_H_ */
> diff --git a/security/clavis/clavis_keyring.c b/security/clavis/clavis_keyring.c
> new file mode 100644
> index 000000000000..400ed455a3a2
> --- /dev/null
> +++ b/security/clavis/clavis_keyring.c
> @@ -0,0 +1,115 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/security.h>
> +#include <linux/integrity.h>
> +#include <keys/asymmetric-type.h>
> +#include <keys/system_keyring.h>
> +#include "clavis.h"
> +
> +static struct key *clavis_keyring;
> +static struct asymmetric_key_id *clavis_boot_akid;
> +static struct asymmetric_setup_kid clavis_setup_akid;
> +static bool clavis_enforced;
> +
> +static bool clavis_acl_enforced(void)
> +{
> + return clavis_enforced;
> +}
Add blank line between functions.
Mimi
> +static int restrict_link_for_clavis(struct key *dest_keyring, const struct key_type *type,
> + const union key_payload *payload, struct key *restrict_key)
> +{
> + /*
> + * Allow a single asymmetric key into this keyring. This key is used as the
> + * root of trust for anything added afterwards.
> + */
> + if (type == &key_type_asymmetric && dest_keyring == clavis_keyring &&
> + !clavis_acl_enforced()) {
> + clavis_enforced = true;
> + return 0;
> + }
> +
> + return -EOPNOTSUPP;
> +}
> +
> +static struct asymmetric_key_id *clavis_parse_boot_param(char *kid, struct asymmetric_key_id *akid,
> + int akid_max_len)
> +{
> + int error, hex_len;
> +
> + if (!kid)
> + return 0;
> +
> + hex_len = strlen(kid) / 2;
> +
> + if (hex_len > akid_max_len)
> + return 0;
> +
> + akid->len = hex_len;
> + error = hex2bin(akid->data, kid, akid->len);
> +
> + if (error < 0) {
> + pr_err("Unparsable clavis key id\n");
> + return 0;
> + }
> +
> + return akid;
> +}
> +
> +static int __init clavis_param(char *kid)
> +{
> + clavis_boot_akid = clavis_parse_boot_param(kid, &clavis_setup_akid.id,
> + ARRAY_SIZE(clavis_setup_akid.data));
> +
> + return 1;
> +}
> +
> +__setup("clavis=", clavis_param);
> +
> +static struct key *clavis_keyring_alloc(const char *desc, struct key_restriction *restriction)
> +{
> + struct key *keyring;
> +
> + keyring = keyring_alloc(desc, GLOBAL_ROOT_UID, GLOBAL_ROOT_GID, current_cred(),
> + KEY_POS_VIEW | KEY_POS_READ | KEY_POS_SEARCH | KEY_POS_WRITE |
> + KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH | KEY_USR_WRITE,
> + KEY_ALLOC_NOT_IN_QUOTA | KEY_ALLOC_SET_KEEP,
> + restriction, NULL);
> + return keyring;
> +}
> +
> +static struct key_restriction *clavis_restriction_alloc(key_restrict_link_func_t check_func)
> +{
> + struct key_restriction *restriction;
> +
> + restriction = kzalloc(sizeof(*restriction), GFP_KERNEL);
> +
> + if (restriction)
> + restriction->check = check_func;
> +
> + return restriction;
> +}
> +
> +static int __init clavis_keyring_init(void)
> +{
> + struct key_restriction *restriction;
> +
> + restriction = clavis_restriction_alloc(restrict_link_for_clavis);
> + if (!restriction)
> + panic("Can't allocate clavis keyring restriction\n");
> +
> + clavis_keyring = clavis_keyring_alloc(".clavis", restriction);
> + if (IS_ERR(clavis_keyring))
> + panic("Can't allocate clavis keyring\n");
> +
> + return 0;
> +}
> +
> +void __init late_init_clavis_setup(void)
> +{
> + clavis_keyring_init();
> +
> + if (!clavis_boot_akid)
> + return;
> +
> + system_key_link(clavis_keyring, clavis_boot_akid);
> +}
> diff --git a/security/integrity/iint.c b/security/integrity/iint.c
> index 068ac6c2ae1e..87a8bfc0662f 100644
> --- a/security/integrity/iint.c
> +++ b/security/integrity/iint.c
> @@ -36,6 +36,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
> */
> void __init integrity_load_keys(void)
> {
> + late_init_clavis_setup();
> +
> ima_load_x509();
>
> if (!IS_ENABLED(CONFIG_IMA_LOAD_X509))
Powered by blists - more mailing lists