Message-ID: <20160108183505.25960.56568.stgit@warthog.procyon.org.uk>
Date:	Fri, 08 Jan 2016 18:35:05 +0000
From:	David Howells <dhowells@...hat.com>
To:	zohar@...ux.vnet.ibm.com
Cc:	dhowells@...hat.com, linux-security-module@...r.kernel.org,
	keyrings@...r.kernel.org, petkan@...-labs.com,
	linux-kernel@...r.kernel.org
Subject: [RFC PATCH 14/15] KEYS: Move the point of trust determination to
 __key_link()

Move the point at which a key is determined to be trustworthy to
__key_link() so that we use the contents of the keyring being linked in to
to determine whether the key being linked in is trusted or not.

What is 'trusted' then becomes a matter of what's in the keyring.

Currently, the test is done when the key is parsed, but given that at that
point we can only sensibly refer to the contents of the system trusted
keyring, we can only use that as the basis for working out the
trustworthiness of a new key.

With this change, a trusted keyring is a set of keys that once the
trusted-only flag is set cannot be added to except by verification through
one of the contained keys.

Further, adding a key into a trusted keyring, whilst it might grant
trustworthiness in the context of that keyring, does not automatically
grant trustworthiness in the context of a second keyring to which it could
be secondarily linked.

To accomplish this, the authentication data associated with the key source
must now be retained.  For an X.509 cert, this means the contents of the
AuthorityKeyIdentifier and the signature data.

Signed-off-by: David Howells <dhowells@...hat.com>
---

 certs/system_keyring.c                    |   19 ++++++-
 crypto/asymmetric_keys/public_key.h       |    6 ++
 crypto/asymmetric_keys/public_key_trust.c |   75 ++++++++++++++++++-----------
 crypto/asymmetric_keys/x509_parser.h      |    6 --
 crypto/asymmetric_keys/x509_public_key.c  |   20 --------
 include/crypto/public_key.h               |    7 +++
 include/keys/system_keyring.h             |   16 ++----
 kernel/module_signing.c                   |    2 -
 security/integrity/digsig.c               |   31 ++++++++++++
 security/integrity/ima/ima_mok.c          |    5 +-
 10 files changed, 115 insertions(+), 72 deletions(-)

diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 417d65882870..6e069246c168 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -18,12 +18,25 @@
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
 
-struct key *system_trusted_keyring;
-EXPORT_SYMBOL_GPL(system_trusted_keyring);
+static struct key *system_trusted_keyring;
 
 extern __initconst const u8 system_certificate_list[];
 extern __initconst const unsigned long system_certificate_list_size;
 
+/**
+ * restrict_link_to_system_trusted - Restrict keyring addition by system CA
+ *
+ * Restrict the addition of keys into a keyring based on the key-to-be-added
+ * being vouched for by a key in the system keyring.
+ */
+int restrict_link_by_system_trusted(struct key *keyring,
+				    const struct key_type *type,
+				    unsigned long flags,
+				    const union key_payload *payload)
+{
+	return public_key_restrict_link(system_trusted_keyring, type, payload);
+}
+
 /*
  * Load the compiled-in keys
  */
@@ -37,7 +50,7 @@ static __init int system_trusted_keyring_init(void)
 			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
 			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
 			      KEY_ALLOC_NOT_IN_QUOTA,
-			      keyring_restrict_trusted_only, NULL);
+			      restrict_link_by_system_trusted, NULL);
 	if (IS_ERR(system_trusted_keyring))
 		panic("Can't allocate system trusted keyring\n");
 	return 0;
diff --git a/crypto/asymmetric_keys/public_key.h b/crypto/asymmetric_keys/public_key.h
index 5c37a22a0637..2962025e1c09 100644
--- a/crypto/asymmetric_keys/public_key.h
+++ b/crypto/asymmetric_keys/public_key.h
@@ -34,3 +34,9 @@ extern const struct public_key_algorithm RSA_public_key_algorithm;
  */
 extern int public_key_verify_signature(const struct public_key *pk,
 				       const struct public_key_signature *sig);
+
+/*
+ * public_key_trust.c
+ */
+extern int public_key_validate_trust(const struct public_key_signature *sig,
+				     struct key *trust_keyring);
diff --git a/crypto/asymmetric_keys/public_key_trust.c b/crypto/asymmetric_keys/public_key_trust.c
index afb2a3eb583a..4196794ee75f 100644
--- a/crypto/asymmetric_keys/public_key_trust.c
+++ b/crypto/asymmetric_keys/public_key_trust.c
@@ -15,7 +15,6 @@
 #include <linux/err.h>
 #include "asymmetric_keys.h"
 #include "public_key.h"
-#include "x509_parser.h"
 
 static bool use_builtin_keys;
 static struct asymmetric_key_id *ca_keyid;
@@ -145,43 +144,63 @@ reject:
 EXPORT_SYMBOL_GPL(request_asymmetric_key);
 
 /*
+ * Try to find a trust relationship for a new key.
+ */
+static int public_key_verify_trust(struct key *trust_keyring,
+				   const struct public_key_signature *sig)
+{
+	struct key *key;
+	int ret;
+
+
+	/* See if we have a key that signed this one. */
+	key = request_asymmetric_key(trust_keyring,
+				     sig->auth_ids[0],
+				     sig->auth_ids[1],
+				     false);
+	if (IS_ERR(key))
+		return -ENOKEY;
+
+	if (use_builtin_keys && !test_bit(KEY_FLAG_BUILTIN, &key->flags))
+		ret = -ENOKEY;
+	else
+		ret = verify_signature(key, sig);
+	key_put(key);
+	return ret;
+}
+
+/**
+ * public_key_restrict_link - Restrict additions to a ring of public keys
+ * @trust_keyring: A ring of keys that can be used to vouch for the new cert.
+ * @type: The type of key being added.
+ * @payload: The payload of the new key.
+ *
  * Check the new certificate against the ones in the trust keyring.  If one of
  * those is the signing key and validates the new certificate, then mark the
  * new certificate as being trusted.
  *
- * Return 0 if the new certificate was successfully validated, 1 if we couldn't
- * find a matching parent certificate in the trusted list and an error if there
- * is a matching certificate but the signature check fails.
+ * Returns 0 if the new certificate was accepted, -ENOKEY if we couldn't find a
+ * matching parent certificate in the trusted list, -EKEYREJECTED if the
+ * signature check fails or the key is blacklisted and some other error if
+ * there is a matching certificate but the signature check cannot be performed.
  */
-int x509_validate_trust(struct x509_certificate *cert,
-			struct key *trust_keyring)
+int public_key_restrict_link(struct key *trust_keyring,
+			     const struct key_type *type,
+			     const union key_payload *payload)
 {
-	struct public_key_signature *sig = cert->sig;
-	struct key *key;
-	int ret = 1;
+	const struct public_key_signature *sig;
 
-	if (!sig->auth_ids[0] && !sig->auth_ids[1])
-		return 1;
+	pr_devel("==>%s()\n", __func__);
 
-	if (!trust_keyring)
+	if (type != &key_type_asymmetric)
 		return -EOPNOTSUPP;
+
+	sig = payload->data[asym_auth];
+	if (!sig->auth_ids[0] && !sig->auth_ids[1])
+		return 0;
+
 	if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
 		return -EPERM;
-	if (cert->unsupported_sig)
-		return -ENOPKG;
 
-	key = request_asymmetric_key(trust_keyring,
-				     sig->auth_ids[0], sig->auth_ids[1], false);
-	if (IS_ERR(key))
-		return PTR_ERR(key);
-
-	if (!use_builtin_keys ||
-	    test_bit(KEY_FLAG_BUILTIN, &key->flags)) {
-		ret = public_key_verify_signature(
-			key->payload.data[asym_crypto], cert->sig);
-		if (ret == -ENOPKG)
-			cert->unsupported_sig = true;
-	}
-	key_put(key);
-	return ret;
+	return public_key_verify_trust(trust_keyring, sig);
 }
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 7a802b09a509..05eef1c68881 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -58,9 +58,3 @@ extern int x509_decode_time(time64_t *_t,  size_t hdrlen,
  */
 extern int x509_get_sig_params(struct x509_certificate *cert);
 extern int x509_check_for_self_signed(struct x509_certificate *cert);
-
-/*
- * public_key_trust.c
- */
-extern int x509_validate_trust(struct x509_certificate *cert,
-			       struct key *trust_keyring);
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index c5c2fbc8c847..dc790af77061 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -97,7 +97,6 @@ error:
 	pr_devel("<==%s() = %d\n", __func__, ret);
 	return ret;
 }
-EXPORT_SYMBOL_GPL(x509_get_sig_params);
 
 /*
  * Check for self-signedness in an X.509 cert and if found, check the signature
@@ -185,30 +184,13 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	cert->pub->algo = pkey_algo[cert->pub->pkey_algo];
 	cert->pub->id_type = PKEY_ID_X509;
 
-	/* See if we can derive the trustability of this certificate.
-	 *
-	 * When it comes to self-signed certificates, we cannot evaluate
-	 * trustedness except by the fact that we obtained it from a trusted
-	 * location.  So we just rely on x509_validate_trust() failing in this
-	 * case.
-	 *
-	 * Note that there's a possibility of a self-signed cert matching a
-	 * cert that we have (most likely a duplicate that we already trust) -
-	 * in which case it will be marked trusted.
-	 */
-	if (cert->unsupported_sig || cert->self_signed) {
+	if (cert->unsupported_sig) {
 		public_key_free(NULL, cert->sig);
 		cert->sig = NULL;
 	} else {
 		pr_devel("Cert Signature: %s + %s\n",
 			 pkey_algo_name[cert->sig->pkey_algo],
 			 hash_algo_name[cert->sig->pkey_hash_algo]);
-
-		ret = x509_validate_trust(cert, get_system_trusted_keyring());
-		if (ret == -EKEYREJECTED)
-			goto error_free_cert;
-		if (!ret)
-			prep->trusted = true;
 	}
 
 	/* Propose a description */
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index eaaf261d398a..c2250f2c1450 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -98,6 +98,13 @@ extern void public_key_free(struct public_key *key,
 			    struct public_key_signature *sig);
 
 struct key;
+struct key_type;
+union key_payload;
+
+extern int public_key_restrict_link(struct key *trust_keyring,
+				    const struct key_type *type,
+				    const union key_payload *payload);
+
 extern int verify_signature(const struct key *key,
 			    const struct public_key_signature *sig);
 
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index b2d645ac35a0..8c63f88b47b1 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -15,19 +15,11 @@
 #ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
 
 #include <linux/key.h>
-#include <linux/verification.h>
-#include <crypto/public_key.h>
 
-extern struct key *system_trusted_keyring;
-static inline struct key *get_system_trusted_keyring(void)
-{
-	return system_trusted_keyring;
-}
-#else
-static inline struct key *get_system_trusted_keyring(void)
-{
-	return NULL;
-}
+extern int restrict_link_by_system_trusted(struct key *keyring,
+					   const struct key_type *type,
+					   unsigned long flags,
+					   const union key_payload *payload);
 #endif
 
 #ifdef CONFIG_IMA_MOK_KEYRING
diff --git a/kernel/module_signing.c b/kernel/module_signing.c
index b3dafe4fd320..50eab38e0e34 100644
--- a/kernel/module_signing.c
+++ b/kernel/module_signing.c
@@ -11,7 +11,7 @@
 
 #include <linux/kernel.h>
 #include <linux/errno.h>
-#include <keys/system_keyring.h>
+#include <linux/verification.h>
 #include <crypto/public_key.h>
 #include "module-internal.h"
 
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 659566c2200b..c802556f8f1c 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -18,6 +18,8 @@
 #include <linux/cred.h>
 #include <linux/key-type.h>
 #include <linux/digsig.h>
+#include <crypto/public_key.h>
+#include <keys/system_keyring.h>
 
 #include "integrity.h"
 
@@ -40,6 +42,26 @@ static bool init_keyring __initdata = true;
 static bool init_keyring __initdata;
 #endif
 
+/*
+ * Restrict the addition of keys into the IMA keyring.
+ *
+ * Any key that needs to go in .ima keyring must be signed by CA in
+ * either .system or .ima_mok keyrings.
+ */
+static int restrict_link_by_ima_mok(struct key *keyring,
+				    const struct key_type *type,
+				    unsigned long flags,
+				    const union key_payload *payload)
+{
+	int ret;
+
+	ret = restrict_link_by_system_trusted(keyring, type, flags, payload);
+	if (ret != -ENOKEY)
+		return ret;
+
+	return public_key_restrict_link(ima_mok_keyring, type, payload);
+}
+
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 			    const char *digest, int digestlen)
 {
@@ -72,19 +94,26 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 
 int __init integrity_init_keyring(const unsigned int id)
 {
+	int (*restrict_link)(struct key *,
+			     const struct key_type *,
+			     unsigned long,
+			     const union key_payload *) = NULL;
 	const struct cred *cred = current_cred();
 	int err = 0;
 
 	if (!init_keyring)
 		return 0;
 
+	if (id == 1)
+		restrict_link = restrict_link_by_ima_mok;
+
 	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
 				    KGIDT_INIT(0), cred,
 				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
 				     KEY_USR_VIEW | KEY_USR_READ |
 				     KEY_USR_WRITE | KEY_USR_SEARCH),
 				    KEY_ALLOC_NOT_IN_QUOTA,
-				    NULL, NULL);
+				    restrict_link, NULL);
 	if (IS_ERR(keyring[id])) {
 		err = PTR_ERR(keyring[id]);
 		pr_info("Can't allocate %s keyring (%d)\n",
diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
index ef91248cb934..fdcbf0ade180 100644
--- a/security/integrity/ima/ima_mok.c
+++ b/security/integrity/ima/ima_mok.c
@@ -18,6 +18,7 @@
 #include <linux/err.h>
 #include <linux/init.h>
 #include <keys/asymmetric-type.h>
+#include <crypto/public_key.h>
 
 
 struct key *ima_mok_keyring;
@@ -36,7 +37,7 @@ __init int ima_mok_init(void)
 			      KEY_USR_VIEW | KEY_USR_READ |
 			      KEY_USR_WRITE | KEY_USR_SEARCH,
 			      KEY_ALLOC_NOT_IN_QUOTA,
-			      keyring_restrict_trusted_only, NULL);
+			      public_key_restrict_link, NULL);
 
 	ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
 				KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
@@ -44,7 +45,7 @@ __init int ima_mok_init(void)
 				KEY_USR_VIEW | KEY_USR_READ |
 				KEY_USR_WRITE | KEY_USR_SEARCH,
 				KEY_ALLOC_NOT_IN_QUOTA,
-				keyring_restrict_trusted_only, NULL);
+				public_key_restrict_link, NULL);
 
 	if (IS_ERR(ima_mok_keyring) || IS_ERR(ima_blacklist_keyring))
 		panic("Can't allocate IMA MOK or blacklist keyrings.");

Powered by blists - more mailing lists