diff mbox series

[RFC,v3,03/13] clavis: Introduce a new system keyring called clavis

Message ID 20241017155516.2582369-4-eric.snowberg@oracle.com (mailing list archive)
State Not Applicable
Delegated to: Herbert Xu
Headers show
Series Clavis LSM | expand

Commit Message

Eric Snowberg Oct. 17, 2024, 3:55 p.m. UTC
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).
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@oracle.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

Comments

Jarkko Sakkinen Oct. 17, 2024, 4:50 p.m. UTC | #1
On Thu, 2024-10-17 at 09:55 -0600, Eric Snowberg wrote:
> +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;

Hmmm... I'd consider sanity checking this:

	size_t len;

	/* ... */

	len = strlen(kid);
	if (len % 2) {
		pr_err("Clavis key id has invalid length %lu\n", len);
		return 0;
	}

	hex_len = len / 2;
	
BR, Jarkko
Eric Snowberg Oct. 17, 2024, 8:34 p.m. UTC | #2
> On Oct 17, 2024, at 10:50 AM, Jarkko Sakkinen <jarkko@kernel.org> wrote:
> 
> On Thu, 2024-10-17 at 09:55 -0600, Eric Snowberg wrote:
>> +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;
> 
> Hmmm... I'd consider sanity checking this:
> 
> size_t len;
> 
> /* ... */
> 
> len = strlen(kid);
> if (len % 2) {
> pr_err("Clavis key id has invalid length %lu\n", len);
> return 0;
> }
> 
> hex_len = len / 2;
> 

Good catch, I will include this in the next round.  I have also added a kunit test 
for this as well.  Thanks.
Jarkko Sakkinen Oct. 17, 2024, 9:16 p.m. UTC | #3
On Thu, 2024-10-17 at 20:34 +0000, Eric Snowberg wrote:
> 
> 
> > On Oct 17, 2024, at 10:50 AM, Jarkko Sakkinen <jarkko@kernel.org>
> > wrote:
> > 
> > On Thu, 2024-10-17 at 09:55 -0600, Eric Snowberg wrote:
> > > +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;
> > 
> > Hmmm... I'd consider sanity checking this:
> > 
> > size_t len;
> > 
> > /* ... */
> > 
> > len = strlen(kid);
> > if (len % 2) {
> > pr_err("Clavis key id has invalid length %lu\n", len);
> > return 0;
> > }
> > 
> > hex_len = len / 2;
> > 
> 
> Good catch, I will include this in the next round.  I have also added
> a kunit test 
> for this as well.  Thanks.

I guess hex2bin() would eventually catch this issue, i.e. not an actual
bug, but I still think that you are better off doing also check here and
get an appropriate message to klog if that ever happens :-)

BR, Jarkko
diff mbox series

Patch

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> }
+
 	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"
+	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.
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;
+}
+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))