diff mbox series

[RFC,v2,02/12] KEYS: CA link restriction

Message ID 20210726171319.3133879-3-eric.snowberg@oracle.com (mailing list archive)
State New
Headers show
Series Enroll kernel keys thru MOK | expand

Commit Message

Eric Snowberg July 26, 2021, 5:13 p.m. UTC
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) or by being
vouched for by a key in either the built-in or the secondary trusted
keyrings.

Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>
---
v1: Initial version
v2: Removed secondary keyring references
---
 certs/system_keyring.c            | 21 +++++++++++
 crypto/asymmetric_keys/restrict.c | 60 +++++++++++++++++++++++++++++++
 include/crypto/public_key.h       |  5 +++
 include/keys/system_keyring.h     |  6 ++++
 4 files changed, 92 insertions(+)

Comments

Mimi Zohar Aug. 5, 2021, 2 p.m. UTC | #1
On Mon, 2021-07-26 at 13:13 -0400, Eric Snowberg wrote:
> 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) or by being
> vouched for by a key in either the built-in or the secondary trusted
> keyrings.
> 
> Signed-off-by: Eric Snowberg <eric.snowberg@oracle.com>

As discussed, please remove "or by being vouched for by a key in
either the built-in or the secondary trusted keyrings."

If these keys were to be loaded onto a keyring other than the platform
keyring, they should be loaded onto the secondary keyring.  The
secondary restriction currently allows certificates signed by keys on
either the builtin or the secondary keyring, to be loaded onto the
secondary keyring.   A new restriction would also allow certificates
signed by keys on the ".mok" keyring.

> +/**
> + * restrict_link_by_ca - Restrict additions to a ring of public keys
> + * based on it being a CA
> + * @dest_keyring: Keyring being linked to.
> + * @type: The type of key being added.
> + * @payload: The payload of the new key.
> + * @trusted: A key or ring of keys that can be used to vouch for the new cert.
> + *
> + * Check if the new certificate is a CA or if they key can be vouched for
> + * by keys already linked in the destination keyring or the trusted
> + * keyring.  If one of those is the signing key or it is self signed, 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.
> + */

Please update the function description as discussed, removing "or if they
key can be vouched for by keys already linked in the destination
keyring or the trusted keyring."

The kernel doc "Brief description of function" should not wrap.  The
variable definition shouldn't be suffixed with a period.  Please refer
to Documentation/doc-guide/kernel-doc.rst.

> +int restrict_link_by_ca(struct key *dest_keyring,

The UEFI db CA keys should only be loaded on boot.  Should this be
annotated as __init?

> +			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;
> +	struct key *key;
> +	int ret;
> +
> +	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;
> +
> +	ret = public_key_verify_signature(pkey, sig);
> +	if (!ret)
> +		return 0;
> +
> +	if (!trust_keyring)
> +		return -ENOKEY;
> +
> +	key = find_asymmetric_key(trust_keyring,
> +				  sig->auth_ids[0], sig->auth_ids[1],
> +				  false);
> +	if (IS_ERR(key))
> +		return -ENOKEY;
> +
> +	ret = verify_signature(key, sig);
> +	key_put(key);
> +	return ret;
> +}
> +
>  static bool match_either_id(const struct asymmetric_key_ids *pair,
>  			    const struct asymmetric_key_id *single)
>  {


> +extern int restrict_link_by_system_trusted_or_ca(
> +	struct key *dest_keyring,
> +	const struct key_type *type,
> +	const union key_payload *payload,
> +	struct key *restrict_key);

After the discussed change, this shouldn't be needed.

thanks,

Mimi

> +
>  #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
>  extern int restrict_link_by_builtin_and_secondary_trusted(
>  	struct key *keyring,
diff mbox series

Patch

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 692365dee2bd..0a7b16c28a72 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -21,6 +21,9 @@ 
 static struct key *builtin_trusted_keys;
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 static struct key *secondary_trusted_keys;
+#define system_trusted_keys secondary_trusted_keys
+#else
+#define system_trusted_keys builtin_trusted_keys
 #endif
 #ifdef CONFIG_INTEGRITY_PLATFORM_KEYRING
 static struct key *platform_trusted_keys;
@@ -45,6 +48,24 @@  int restrict_link_by_builtin_trusted(struct key *dest_keyring,
 					  builtin_trusted_keys);
 }
 
+/**
+ * restrict_link_by_system_trusted_or_ca - Restrict keyring
+ *   addition by being a CA or vouched by the system trusted keyrings.
+ *
+ *  Restrict the addition of keys in a keyring based on the key-to-be-added
+ *  being a CA (self signed) or by being vouched for by a key in either
+ *  the built-in or the secondary system keyrings.
+ */
+int restrict_link_by_system_trusted_or_ca(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key)
+{
+	return restrict_link_by_ca(dest_keyring, type, payload,
+				   system_trusted_keys);
+}
+
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 /**
  * restrict_link_by_builtin_and_secondary_trusted - Restrict keyring
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
index 84cefe3b3585..75e4379226e8 100644
--- a/crypto/asymmetric_keys/restrict.c
+++ b/crypto/asymmetric_keys/restrict.c
@@ -108,6 +108,66 @@  int restrict_link_by_signature(struct key *dest_keyring,
 	return ret;
 }
 
+/**
+ * restrict_link_by_ca - Restrict additions to a ring of public keys
+ * based on it being a CA
+ * @dest_keyring: Keyring being linked to.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ * @trusted: A key or ring of keys that can be used to vouch for the new cert.
+ *
+ * Check if the new certificate is a CA or if they key can be vouched for
+ * by keys already linked in the destination keyring or the trusted
+ * keyring.  If one of those is the signing key or it is self signed, 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;
+	struct key *key;
+	int ret;
+
+	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;
+
+	ret = public_key_verify_signature(pkey, sig);
+	if (!ret)
+		return 0;
+
+	if (!trust_keyring)
+		return -ENOKEY;
+
+	key = find_asymmetric_key(trust_keyring,
+				  sig->auth_ids[0], sig->auth_ids[1],
+				  false);
+	if (IS_ERR(key))
+		return -ENOKEY;
+
+	ret = verify_signature(key, sig);
+	key_put(key);
+	return ret;
+}
+
 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 *);
 
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 6acd3cf13a18..2041254d74f4 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -28,6 +28,12 @@  static inline __init int load_module_cert(struct key *keyring)
 
 #endif
 
+extern int restrict_link_by_system_trusted_or_ca(
+	struct key *dest_keyring,
+	const struct key_type *type,
+	const union key_payload *payload,
+	struct key *restrict_key);
+
 #ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
 extern int restrict_link_by_builtin_and_secondary_trusted(
 	struct key *keyring,