Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1350277 > unrolled thread

[RFC PATCH 00/12] KEYS: Restrict additions to 'trusted' keyrings [ver #2]

Back to article view | Back to linux.kernel

Contents

  [RFC PATCH 00/12] KEYS: Restrict additions to 'trusted' keyrings  [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 04/12] KEYS: Move x509_request_asymmetric_key() to  asymmetric_type.c [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead  of .ima_mok [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
      Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 03:30 +0100
        Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 14:10 +0100
          Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Petko Manolov <petkan@mip-labs.com> - 2016-03-08 15:20 +0100
            Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 15:40 +0100
            Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 15:50 +0100
              Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Petko Manolov <petkan@mip-labs.com> - 2016-03-08 16:50 +0100
            Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 17:10 +0100
              Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Petko Manolov <petkan@mip-labs.com> - 2016-03-08 17:40 +0100
                Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 23:10 +0100
          Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings  instead of .ima_mok [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 15:20 +0100
    [RFC PATCH 07/12] X.509: Move the trust validation code out to its  own file [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 11/12] certs: Add a secondary system keyring that can be  added to dynamically [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
      Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that  can be added to dynamically [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 03:10 +0100
        Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 14:20 +0100
          Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can  be added to dynamically [ver #2] Petko Manolov <petkan@mip-labs.com> - 2016-03-08 15:20 +0100
          Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that  can be added to dynamically [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 15:40 +0100
            Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 15:50 +0100
              Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that  can be added to dynamically [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 16:10 +0100
                Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2] David Howells <dhowells@redhat.com> - 2016-03-08 16:40 +0100
                  Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that  can be added to dynamically [ver #2] Mimi Zohar <zohar@linux.vnet.ibm.com> - 2016-03-08 17:20 +0100
    [RFC PATCH 05/12] KEYS: Generalise x509_request_asymmetric_key()  [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 09/12] KEYS: Move the point of trust determination to  __key_link() [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 02/12] PKCS#7: Make trust determination dependent on  contents of trust keyring [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 06/12] X.509: Use verify_signature() if we have a struct  key * to use [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100
    [RFC PATCH 10/12] KEYS: Remove KEY_FLAG_TRUSTED and  KEY_ALLOC_TRUSTED [ver #2] David Howells <dhowells@redhat.com> - 2016-03-04 16:10 +0100

Page 1 of 2  [1] 2  Next page →

#1350277 — [RFC PATCH 00/12] KEYS: Restrict additions to 'trusted' keyrings [ver #2]

Here's a set of patches that changes how certificates/keys are determined
to be trusted.  That's currently a two-step process:

 (1) Up until recently, when an X.509 certificate was parsed - no matter
     the source - it was judged against the keys in .system_keyring,
     assuming those keys to be trusted if they have KEY_FLAG_TRUSTED set
     upon them.

     This has just been changed such that any key in the .ima_mok keyring,
     if configured, may also be used to judge the trustworthiness of a new
     certificate, whether or not the .ima_mok keyring is meant to be
     consulted for whatever process is being undertaken.

     If a certificate is determined to be trustworthy, KEY_FLAG_TRUSTED
     will be set upon a key it is loaded into (if it is loaded into one),
     no matter what the key is going to be loaded for.

 (2) If an X.509 certificate is loaded into a key, then that key - if
     KEY_FLAG_TRUSTED gets set upon it - can be linked into any keyring
     with KEY_FLAG_TRUSTED_ONLY set upon it.  This was meant to be the
     system keyring only, but has been extended to various IMA keyrings.

     A user can at will link any key marked KEY_FLAG_TRUSTED into any
     keyring marked KEY_FLAG_TRUSTED_ONLY if the relevant permissions masks
     permit it.

These patches change that:

 (1) Trust becomes a matter of consulting the ring of trusted keys supplied
     when the trust is evaluated only.

 (3) Every keyring can be supplied with its own manager function to
     restrict what may be added to that keyring.  This is called whenever a
     key is to be linked into the keyring to guard against a key being
     created in one keyring and then linked across.

     This function is supplied with the keyring and the key type and
     payload[*] of the key being linked in for use in its evaluation.  It
     is permitted to use other data also, such as the contents of other
     keyrings such as the system keyrings.

     [*] The type and payload are supplied instead of a key because as an
     	 optimisation this function may be called whilst creating a key and
     	 so may reject the proposed key between preparse and allocation.

 (4) A default manager function is provided that permits keys to be
     restricted to only asymmetric keys that are vouched for by the
     contents of the system keyring.

     A second manager function is provided that just rejects with EPERM.

 (5) A key allocation flag, KEY_ALLOC_BYPASS_RESTRICTION, is made available
     so that the kernel can initialise keyrings with keys that form the
     root of the trust relationship.

 (6) KEY_FLAG_TRUSTED and KEY_FLAG_TRUSTED_ONLY are removed, along with
     key_preparsed_payload::trusted.

This change also makes it possible in future for userspace to create a private
set of trusted keys and then to have it sealed by setting a manager function
where the private set is wholly independent of the kernel's trust
relationships.

Further changes in the set involve extracting certain IMA special keyrings
and making them generally global:

 (*) .system_keyring is renamed to .builtin_trusted_keys and remains read
     only.  It carries only keys built in to the kernel.  It may be where
     UEFI keys should be loaded - though that could better be the new
     secondary keyring (see below) or a separate UEFI keyring.

 (*) An optional secondary system keyring (called .secondary_trusted_keys)
     is added to replace the IMA MOK keyring.

     (*) Keys can be added to the secondary keyring by root if the keys can
     	 be vouched for by either ring of system keys.

 (*) Module signing and kexec only use .builtin_trusted_keys and do not use
     the new secondary keyring.

 (*) Config option SYSTEM_TRUSTED_KEYS now depends on ASYMMETRIC_KEY_TYPE as
     that's the only type currently permitted on the system keyrings.

 (*) A new config option, IMA_PERMIT_ADD_TO_IMA_KEYRINGS, is provided to allow
     keys to be added to IMA keyrings - subject to the restriction that such
     keys are validly signed by a key already in the system keyrings.

     If IMA_PERMIT_ADD_TO_IMA_KEYRINGS is enabled, but secondary keyrings
     aren't, additions to the IMA keyrings will be restricted to signatures
     verifiable by keys in the builtin system keyring only.

The patches can be found here also:

	http://git.kernel.org/cgit/linux/kernel/git/dhowells/linux-fs.git/log/?h=keys-trust

This patchset is based on top of 

	[RFC PATCH 0/7] KEYS: Adjust public key signature handling

which is on branch keys-sig in the GIT repo mentioned above.

David
---
David Howells (12):
      KEYS: Generalise system_verify_data() to provide access to internal content
      PKCS#7: Make trust determination dependent on contents of trust keyring
      KEYS: Add a facility to restrict new links into a keyring
      KEYS: Move x509_request_asymmetric_key() to asymmetric_type.c
      KEYS: Generalise x509_request_asymmetric_key()
      X.509: Use verify_signature() if we have a struct key * to use
      X.509: Move the trust validation code out to its own file
      KEYS: Make the system trusted keyring depend on the asymmetric key type
      KEYS: Move the point of trust determination to __key_link()
      KEYS: Remove KEY_FLAG_TRUSTED and KEY_ALLOC_TRUSTED
      certs: Add a secondary system keyring that can be added to dynamically
      IMA: Use the the system trusted keyrings instead of .ima_mok


 Documentation/security/keys.txt          |   22 +++
 arch/x86/kernel/kexec-bzimage64.c        |   18 +--
 certs/Kconfig                            |    9 +
 certs/system_keyring.c                   |  137 +++++++++++++++++-----
 crypto/asymmetric_keys/Kconfig           |    4 -
 crypto/asymmetric_keys/Makefile          |    5 +
 crypto/asymmetric_keys/asymmetric_keys.h |    2 
 crypto/asymmetric_keys/asymmetric_type.c |   89 ++++++++++++++
 crypto/asymmetric_keys/mscode_parser.c   |   21 +--
 crypto/asymmetric_keys/pkcs7_key_type.c  |   72 ++++-------
 crypto/asymmetric_keys/pkcs7_parser.c    |   21 ++-
 crypto/asymmetric_keys/pkcs7_parser.h    |    1 
 crypto/asymmetric_keys/pkcs7_trust.c     |   35 ++---
 crypto/asymmetric_keys/restrict.c        |  108 +++++++++++++++++
 crypto/asymmetric_keys/verify_pefile.c   |   40 +-----
 crypto/asymmetric_keys/verify_pefile.h   |    5 -
 crypto/asymmetric_keys/x509_parser.h     |    1 
 crypto/asymmetric_keys/x509_public_key.c |  191 ------------------------------
 fs/cifs/cifsacl.c                        |    2 
 fs/nfs/nfs4idmap.c                       |    2 
 include/crypto/pkcs7.h                   |    6 -
 include/crypto/public_key.h              |   27 +---
 include/keys/asymmetric-type.h           |    6 +
 include/keys/system_keyring.h            |   36 ++----
 include/linux/key-type.h                 |    1 
 include/linux/key.h                      |   44 +++++--
 include/linux/verification.h             |   49 ++++++++
 include/linux/verify_pefile.h            |   22 ---
 kernel/module_signing.c                  |    7 +
 net/dns_resolver/dns_key.c               |    2 
 net/rxrpc/ar-key.c                       |    4 -
 security/integrity/digsig.c              |   16 ++-
 security/integrity/ima/Kconfig           |   36 ++++--
 security/integrity/ima/ima_mok.c         |   18 +--
 security/keys/key.c                      |   42 +++++--
 security/keys/keyring.c                  |   46 ++++++-
 security/keys/persistent.c               |    4 -
 security/keys/process_keys.c             |   16 ++-
 security/keys/request_key.c              |    4 -
 security/keys/request_key_auth.c         |    2 
 40 files changed, 664 insertions(+), 509 deletions(-)
 create mode 100644 crypto/asymmetric_keys/restrict.c
 create mode 100644 include/linux/verification.h
 delete mode 100644 include/linux/verify_pefile.h

[toc] | [next] | [standalone]

#1350280 — [RFC PATCH 04/12] KEYS: Move x509_request_asymmetric_key() to asymmetric_type.c [ver #2]

Move x509_request_asymmetric_key() to asymmetric_type.c so that it can be
generalised.

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

 crypto/asymmetric_keys/asymmetric_type.c |   89 ++++++++++++++++++++++++++++++
 crypto/asymmetric_keys/x509_public_key.c |   89 ------------------------------
 include/crypto/public_key.h              |    6 --
 include/keys/asymmetric-type.h           |    5 ++
 4 files changed, 94 insertions(+), 95 deletions(-)

diff --git a/crypto/asymmetric_keys/asymmetric_type.c b/crypto/asymmetric_keys/asymmetric_type.c
index a79d30128821..c4d66cd82860 100644
--- a/crypto/asymmetric_keys/asymmetric_type.c
+++ b/crypto/asymmetric_keys/asymmetric_type.c
@@ -35,6 +35,95 @@ static LIST_HEAD(asymmetric_key_parsers);
 static DECLARE_RWSEM(asymmetric_key_parsers_sem);
 
 /**
+ * x509_request_asymmetric_key - Request a key by X.509 certificate params.
+ * @keyring: The keys to search.
+ * @id: The issuer & serialNumber to look for or NULL.
+ * @skid: The subjectKeyIdentifier to look for or NULL.
+ * @partial: Use partial match if true, exact if false.
+ *
+ * Find a key in the given keyring by identifier.  The preferred identifier is
+ * the issuer + serialNumber and the fallback identifier is the
+ * subjectKeyIdentifier.  If both are given, the lookup is by the former, but
+ * the latter must also match.
+ */
+struct key *x509_request_asymmetric_key(struct key *keyring,
+					const struct asymmetric_key_id *id,
+					const struct asymmetric_key_id *skid,
+					bool partial)
+{
+	struct key *key;
+	key_ref_t ref;
+	const char *lookup;
+	char *req, *p;
+	int len;
+
+	if (id) {
+		lookup = id->data;
+		len = id->len;
+	} else {
+		lookup = skid->data;
+		len = skid->len;
+	}
+
+	/* Construct an identifier "id:<keyid>". */
+	p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
+	if (!req)
+		return ERR_PTR(-ENOMEM);
+
+	if (partial) {
+		*p++ = 'i';
+		*p++ = 'd';
+	} else {
+		*p++ = 'e';
+		*p++ = 'x';
+	}
+	*p++ = ':';
+	p = bin2hex(p, lookup, len);
+	*p = 0;
+
+	pr_debug("Look up: \"%s\"\n", req);
+
+	ref = keyring_search(make_key_ref(keyring, 1),
+			     &key_type_asymmetric, req);
+	if (IS_ERR(ref))
+		pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
+	kfree(req);
+
+	if (IS_ERR(ref)) {
+		switch (PTR_ERR(ref)) {
+			/* Hide some search errors */
+		case -EACCES:
+		case -ENOTDIR:
+		case -EAGAIN:
+			return ERR_PTR(-ENOKEY);
+		default:
+			return ERR_CAST(ref);
+		}
+	}
+
+	key = key_ref_to_ptr(ref);
+	if (id && skid) {
+		const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
+		if (!kids->id[1]) {
+			pr_debug("issuer+serial match, but expected SKID missing\n");
+			goto reject;
+		}
+		if (!asymmetric_key_id_same(skid, kids->id[1])) {
+			pr_debug("issuer+serial match, but SKID does not\n");
+			goto reject;
+		}
+	}
+
+	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
+	return key;
+
+reject:
+	key_put(key);
+	return ERR_PTR(-EKEYREJECTED);
+}
+EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
+
+/**
  * asymmetric_key_generate_id: Construct an asymmetric key ID
  * @val_1: First binary blob
  * @len_1: Length of first binary blob
diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index fc77a2bd70ba..2fb594175cef 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -58,95 +58,6 @@ static int __init ca_keys_setup(char *str)
 __setup("ca_keys=", ca_keys_setup);
 #endif
 
-/**
- * x509_request_asymmetric_key - Request a key by X.509 certificate params.
- * @keyring: The keys to search.
- * @id: The issuer & serialNumber to look for or NULL.
- * @skid: The subjectKeyIdentifier to look for or NULL.
- * @partial: Use partial match if true, exact if false.
- *
- * Find a key in the given keyring by identifier.  The preferred identifier is
- * the issuer + serialNumber and the fallback identifier is the
- * subjectKeyIdentifier.  If both are given, the lookup is by the former, but
- * the latter must also match.
- */
-struct key *x509_request_asymmetric_key(struct key *keyring,
-					const struct asymmetric_key_id *id,
-					const struct asymmetric_key_id *skid,
-					bool partial)
-{
-	struct key *key;
-	key_ref_t ref;
-	const char *lookup;
-	char *req, *p;
-	int len;
-
-	if (id) {
-		lookup = id->data;
-		len = id->len;
-	} else {
-		lookup = skid->data;
-		len = skid->len;
-	}
-
-	/* Construct an identifier "id:<keyid>". */
-	p = req = kmalloc(2 + 1 + len * 2 + 1, GFP_KERNEL);
-	if (!req)
-		return ERR_PTR(-ENOMEM);
-
-	if (partial) {
-		*p++ = 'i';
-		*p++ = 'd';
-	} else {
-		*p++ = 'e';
-		*p++ = 'x';
-	}
-	*p++ = ':';
-	p = bin2hex(p, lookup, len);
-	*p = 0;
-
-	pr_debug("Look up: \"%s\"\n", req);
-
-	ref = keyring_search(make_key_ref(keyring, 1),
-			     &key_type_asymmetric, req);
-	if (IS_ERR(ref))
-		pr_debug("Request for key '%s' err %ld\n", req, PTR_ERR(ref));
-	kfree(req);
-
-	if (IS_ERR(ref)) {
-		switch (PTR_ERR(ref)) {
-			/* Hide some search errors */
-		case -EACCES:
-		case -ENOTDIR:
-		case -EAGAIN:
-			return ERR_PTR(-ENOKEY);
-		default:
-			return ERR_CAST(ref);
-		}
-	}
-
-	key = key_ref_to_ptr(ref);
-	if (id && skid) {
-		const struct asymmetric_key_ids *kids = asymmetric_key_ids(key);
-		if (!kids->id[1]) {
-			pr_debug("issuer+serial match, but expected SKID missing\n");
-			goto reject;
-		}
-		if (!asymmetric_key_id_same(skid, kids->id[1])) {
-			pr_debug("issuer+serial match, but SKID does not\n");
-			goto reject;
-		}
-	}
-
-	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key));
-	return key;
-
-reject:
-	key_put(key);
-	return ERR_PTR(-EKEYREJECTED);
-}
-EXPORT_SYMBOL_GPL(x509_request_asymmetric_key);
-
 /*
  * Set up the signature parameters in an X.509 certificate.  This involves
  * digesting the signed data and extracting the signature.
diff --git a/include/crypto/public_key.h b/include/crypto/public_key.h
index b3928e801b8c..96ef27b8dd41 100644
--- a/include/crypto/public_key.h
+++ b/include/crypto/public_key.h
@@ -50,12 +50,6 @@ struct key;
 extern int verify_signature(const struct key *key,
 			    const struct public_key_signature *sig);
 
-struct asymmetric_key_id;
-extern struct key *x509_request_asymmetric_key(struct key *keyring,
-					       const struct asymmetric_key_id *id,
-					       const struct asymmetric_key_id *skid,
-					       bool partial);
-
 int public_key_verify_signature(const struct public_key *pkey,
 				const struct public_key_signature *sig);
 
diff --git a/include/keys/asymmetric-type.h b/include/keys/asymmetric-type.h
index d1e23dda4363..735db697c4d2 100644
--- a/include/keys/asymmetric-type.h
+++ b/include/keys/asymmetric-type.h
@@ -76,6 +76,11 @@ const struct asymmetric_key_ids *asymmetric_key_ids(const struct key *key)
 	return key->payload.data[asym_key_ids];
 }
 
+extern struct key *x509_request_asymmetric_key(struct key *keyring,
+					       const struct asymmetric_key_id *id,
+					       const struct asymmetric_key_id *skid,
+					       bool partial);
+
 /*
  * The payload is at the discretion of the subtype.
  */

[toc] | [prev] | [next] | [standalone]

#1350281 — [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

Provide a config option (IMA_PERMIT_ADD_TO_IMA_KEYRINGS) that, when
enabled, allows keys to be added to the IMA keyrings by userspace - with
the restriction that each must be signed by a key in the system trusted
keyrings.

EPERM will be returned if this option is disabled, ENOKEY will be returned if
no authoritative key can be found and EKEYREJECTED will be returned if the
signature doesn't match.  Other errors such as ENOPKG may also be returned.

If this new option is enabled, the builtin system keyring is searched, as is
the secondary system keyring if that is also enabled.  Intermediate keys
between the builtin system keyring and the key being added can be added to
the secondary keyring (which replaces .ima_mok) to form a trust chain -
provided they are also validly signed by a key in one of the trusted keyrings.

The .ima_mok keyring is removed.

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

 include/keys/system_keyring.h    |    9 ---------
 security/integrity/digsig.c      |   31 +++++--------------------------
 security/integrity/ima/Kconfig   |   36 ++++++++++++++++++++++++------------
 security/integrity/ima/ima_mok.c |   12 ++----------
 4 files changed, 31 insertions(+), 57 deletions(-)

diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index c6c05f1513f5..0764a8c1c131 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -29,22 +29,13 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,
 #endif
 
 #ifdef CONFIG_IMA_MOK_KEYRING
-extern struct key *ima_mok_keyring;
 extern struct key *ima_blacklist_keyring;
 
-static inline struct key *get_ima_mok_keyring(void)
-{
-	return ima_mok_keyring;
-}
 static inline struct key *get_ima_blacklist_keyring(void)
 {
 	return ima_blacklist_keyring;
 }
 #else
-static inline struct key *get_ima_mok_keyring(void)
-{
-	return NULL;
-}
 static inline struct key *get_ima_blacklist_keyring(void)
 {
 	return NULL;
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index ac2d39232567..aef3ac7be868 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -42,32 +42,10 @@ static bool init_keyring __initdata = true;
 static bool init_keyring __initdata;
 #endif
 
-#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
-/*
- * 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,
-				    const union key_payload *payload)
-{
-	int ret;
-
-	ret = restrict_link_by_system_trusted(keyring, type, payload);
-	if (ret != -ENOKEY)
-		return ret;
-
-	return restrict_link_by_signature(get_ima_mok_keyring(),
-					  type, payload);
-}
+#ifdef CONFIG_IMA_PERMIT_ADD_TO_IMA_KEYRINGS
+#define restrict_link_to_ima restrict_link_by_system_trusted
 #else
-/*
- * If there's no system trusted keyring, then keys cannot be loaded into
- * .ima_mok and added keys cannot be marked trusted.
- */
-#define restrict_link_by_ima_mok restrict_link_reject
+#define restrict_link_to_ima restrict_link_reject
 #endif
 
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
@@ -114,7 +92,8 @@ int __init integrity_init_keyring(const unsigned int id)
 				     KEY_USR_VIEW | KEY_USR_READ |
 				     KEY_USR_WRITE | KEY_USR_SEARCH),
 				    KEY_ALLOC_NOT_IN_QUOTA,
-				    restrict_link_by_ima_mok, NULL);
+				    restrict_link_to_ima,
+				    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/Kconfig b/security/integrity/ima/Kconfig
index e54a8a8dae94..890e8aa8ccb4 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -155,23 +155,35 @@ config IMA_TRUSTED_KEYRING
 
 	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
 
+config IMA_PERMIT_ADD_TO_IMA_KEYRINGS
+	bool "Allow keys to be added to the ima keyrings, with restrictions"
+	depends on IMA_APPRAISE
+	depends on SYSTEM_TRUSTED_KEYRING
+	select INTEGRITY_TRUSTED_KEYRING
+	default n
+	help
+	  This option permits keys to be added to the ima keyrings using
+	  add_key() or KEYCTL_LINK - with the restriction that the key must be
+	  validly signed by a key in one of the system trusted keyrings.
+
+	  Intermediate keys between those the kernel has compiled in and the
+	  keys to be added may be added to the system secondary keyring if that
+	  is enabled - provided those are validly signed by a key in the system
+	  trusted keyrings.
+
+	  If this is not set, attempts to add to the ima keyrings will be
+	  rejected with EPERM.
+
 config IMA_MOK_KEYRING
-	bool "Create IMA machine owner keys (MOK) and blacklist keyrings"
+	bool "Create IMA machine owner blacklist keyrings"
 	depends on SYSTEM_TRUSTED_KEYRING
 	depends on IMA_TRUSTED_KEYRING
 	default n
 	help
-	   This option creates IMA MOK and blacklist keyrings.  IMA MOK is an
-	   intermediate keyring that sits between .system and .ima keyrings,
-	   effectively forming a simple CA hierarchy.  To successfully import a
-	   key into .ima_mok it must be signed by a key which CA is in .system
-	   keyring.  On turn any key that needs to go in .ima keyring must be
-	   signed by CA in either .system or .ima_mok keyrings. IMA MOK is empty
-	   at kernel boot.
-
-	   IMA blacklist keyring contains all revoked IMA keys.  It is consulted
-	   before any other keyring.  If the search is successful the requested
-	   operation is rejected and error is returned to the caller.
+	   This option creates IMA blacklist keyring.  This contains all
+	   revoked IMA keys.  It is consulted before any other keyring.  If the
+	   search is successful the requested operation is rejected and error
+	   is returned to the caller.
 
 config IMA_LOAD_X509
 	bool "Load X509 certificate onto the '.ima' trusted keyring"
diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
index be0da013622c..031d65a91297 100644
--- a/security/integrity/ima/ima_mok.c
+++ b/security/integrity/ima/ima_mok.c
@@ -28,15 +28,7 @@ struct key *ima_blacklist_keyring;
  */
 __init int ima_mok_init(void)
 {
-	pr_notice("Allocating IMA MOK and blacklist keyrings.\n");
-
-	ima_mok_keyring = keyring_alloc(".ima_mok",
-			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
-			      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
-			      KEY_USR_VIEW | KEY_USR_READ |
-			      KEY_USR_WRITE | KEY_USR_SEARCH,
-			      KEY_ALLOC_NOT_IN_QUOTA,
-			      restrict_link_by_system_trusted, NULL);
+	pr_notice("Allocating IMA blacklist keyrings.\n");
 
 	ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
 				KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
@@ -46,7 +38,7 @@ __init int ima_mok_init(void)
 				KEY_ALLOC_NOT_IN_QUOTA,
 				restrict_link_by_system_trusted, NULL);
 
-	if (IS_ERR(ima_mok_keyring) || IS_ERR(ima_blacklist_keyring))
+	if (IS_ERR(ima_blacklist_keyring))
 		panic("Can't allocate IMA MOK or blacklist keyrings.");
 
 	set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);

[toc] | [prev] | [next] | [standalone]

#1352559 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On Fri, 2016-03-04 at 15:01 +0000, David Howells wrote:
> Provide a config option (IMA_PERMIT_ADD_TO_IMA_KEYRINGS) that, when
> enabled, allows keys to be added to the IMA keyrings by userspace - with
> the restriction that each must be signed by a key in the system trusted
> keyrings.

Here is an example of using the term "system trusted keyrings", which
the previous patch removes.

> EPERM will be returned if this option is disabled, ENOKEY will be returned if
> no authoritative key can be found and EKEYREJECTED will be returned if the
> signature doesn't match.  Other errors such as ENOPKG may also be returned.
> 
> If this new option is enabled, the builtin system keyring is searched, as is
> the secondary system keyring if that is also enabled.  Intermediate keys
> between the builtin system keyring and the key being added can be added to
> the secondary keyring (which replaces .ima_mok) to form a trust chain -
> provided they are also validly signed by a key in one of the trusted keyrings.

Only certificates signed by a key on the system keyring were added to
the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
certificate could be signed by a either a key on the system or ima_mok
keyrings.  To replicate this behavior, the default behavior should be to
only permit certificates signed by a key on the builtin keyring, unless
this new Kconfig is enabled.  Only then, permit certificates signed by a
key on either the builtin or secondary keyrings to be added to the IMA
keyring.

Mimi


> The .ima_mok keyring is removed.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>
> ---
> 
>  include/keys/system_keyring.h    |    9 ---------
>  security/integrity/digsig.c      |   31 +++++--------------------------
>  security/integrity/ima/Kconfig   |   36 ++++++++++++++++++++++++------------
>  security/integrity/ima/ima_mok.c |   12 ++----------
>  4 files changed, 31 insertions(+), 57 deletions(-)
> 
> diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
> index c6c05f1513f5..0764a8c1c131 100644
> --- a/include/keys/system_keyring.h
> +++ b/include/keys/system_keyring.h
> @@ -29,22 +29,13 @@ extern int restrict_link_by_builtin_trusted(struct key *keyring,
>  #endif
> 
>  #ifdef CONFIG_IMA_MOK_KEYRING
> -extern struct key *ima_mok_keyring;
>  extern struct key *ima_blacklist_keyring;
> 
> -static inline struct key *get_ima_mok_keyring(void)
> -{
> -	return ima_mok_keyring;
> -}
>  static inline struct key *get_ima_blacklist_keyring(void)
>  {
>  	return ima_blacklist_keyring;
>  }
>  #else
> -static inline struct key *get_ima_mok_keyring(void)
> -{
> -	return NULL;
> -}
>  static inline struct key *get_ima_blacklist_keyring(void)
>  {
>  	return NULL;
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index ac2d39232567..aef3ac7be868 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -42,32 +42,10 @@ static bool init_keyring __initdata = true;
>  static bool init_keyring __initdata;
>  #endif
> 
> -#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> -/*
> - * 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,
> -				    const union key_payload *payload)
> -{
> -	int ret;
> -
> -	ret = restrict_link_by_system_trusted(keyring, type, payload);
> -	if (ret != -ENOKEY)
> -		return ret;
> -
> -	return restrict_link_by_signature(get_ima_mok_keyring(),
> -					  type, payload);
> -}
> +#ifdef CONFIG_IMA_PERMIT_ADD_TO_IMA_KEYRINGS
> +#define restrict_link_to_ima restrict_link_by_system_trusted
>  #else
> -/*
> - * If there's no system trusted keyring, then keys cannot be loaded into
> - * .ima_mok and added keys cannot be marked trusted.
> - */
> -#define restrict_link_by_ima_mok restrict_link_reject
> +#define restrict_link_to_ima restrict_link_reject
>  #endif
> 
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
> @@ -114,7 +92,8 @@ int __init integrity_init_keyring(const unsigned int id)
>  				     KEY_USR_VIEW | KEY_USR_READ |
>  				     KEY_USR_WRITE | KEY_USR_SEARCH),
>  				    KEY_ALLOC_NOT_IN_QUOTA,
> -				    restrict_link_by_ima_mok, NULL);
> +				    restrict_link_to_ima,
> +				    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/Kconfig b/security/integrity/ima/Kconfig
> index e54a8a8dae94..890e8aa8ccb4 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -155,23 +155,35 @@ config IMA_TRUSTED_KEYRING
> 
>  	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> 
> +config IMA_PERMIT_ADD_TO_IMA_KEYRINGS
> +	bool "Allow keys to be added to the ima keyrings, with restrictions"
> +	depends on IMA_APPRAISE
> +	depends on SYSTEM_TRUSTED_KEYRING
> +	select INTEGRITY_TRUSTED_KEYRING
> +	default n
> +	help
> +	  This option permits keys to be added to the ima keyrings using
> +	  add_key() or KEYCTL_LINK - with the restriction that the key must be
> +	  validly signed by a key in one of the system trusted keyrings.
> +
> +	  Intermediate keys between those the kernel has compiled in and the
> +	  keys to be added may be added to the system secondary keyring if that
> +	  is enabled - provided those are validly signed by a key in the system
> +	  trusted keyrings.
> +
> +	  If this is not set, attempts to add to the ima keyrings will be
> +	  rejected with EPERM.
> +
>  config IMA_MOK_KEYRING
> -	bool "Create IMA machine owner keys (MOK) and blacklist keyrings"
> +	bool "Create IMA machine owner blacklist keyrings"
>  	depends on SYSTEM_TRUSTED_KEYRING
>  	depends on IMA_TRUSTED_KEYRING
>  	default n
>  	help
> -	   This option creates IMA MOK and blacklist keyrings.  IMA MOK is an
> -	   intermediate keyring that sits between .system and .ima keyrings,
> -	   effectively forming a simple CA hierarchy.  To successfully import a
> -	   key into .ima_mok it must be signed by a key which CA is in .system
> -	   keyring.  On turn any key that needs to go in .ima keyring must be
> -	   signed by CA in either .system or .ima_mok keyrings. IMA MOK is empty
> -	   at kernel boot.
> -
> -	   IMA blacklist keyring contains all revoked IMA keys.  It is consulted
> -	   before any other keyring.  If the search is successful the requested
> -	   operation is rejected and error is returned to the caller.
> +	   This option creates IMA blacklist keyring.  This contains all
> +	   revoked IMA keys.  It is consulted before any other keyring.  If the
> +	   search is successful the requested operation is rejected and error
> +	   is returned to the caller.
> 
>  config IMA_LOAD_X509
>  	bool "Load X509 certificate onto the '.ima' trusted keyring"
> diff --git a/security/integrity/ima/ima_mok.c b/security/integrity/ima/ima_mok.c
> index be0da013622c..031d65a91297 100644
> --- a/security/integrity/ima/ima_mok.c
> +++ b/security/integrity/ima/ima_mok.c
> @@ -28,15 +28,7 @@ struct key *ima_blacklist_keyring;
>   */
>  __init int ima_mok_init(void)
>  {
> -	pr_notice("Allocating IMA MOK and blacklist keyrings.\n");
> -
> -	ima_mok_keyring = keyring_alloc(".ima_mok",
> -			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
> -			      (KEY_POS_ALL & ~KEY_POS_SETATTR) |
> -			      KEY_USR_VIEW | KEY_USR_READ |
> -			      KEY_USR_WRITE | KEY_USR_SEARCH,
> -			      KEY_ALLOC_NOT_IN_QUOTA,
> -			      restrict_link_by_system_trusted, NULL);
> +	pr_notice("Allocating IMA blacklist keyrings.\n");
> 
>  	ima_blacklist_keyring = keyring_alloc(".ima_blacklist",
>  				KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
> @@ -46,7 +38,7 @@ __init int ima_mok_init(void)
>  				KEY_ALLOC_NOT_IN_QUOTA,
>  				restrict_link_by_system_trusted, NULL);
> 
> -	if (IS_ERR(ima_mok_keyring) || IS_ERR(ima_blacklist_keyring))
> +	if (IS_ERR(ima_blacklist_keyring))
>  		panic("Can't allocate IMA MOK or blacklist keyrings.");
> 
>  	set_bit(KEY_FLAG_KEEP, &ima_blacklist_keyring->flags);
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]

#1353007 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> Only certificates signed by a key on the system keyring were added to
> the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
> certificate could be signed by a either a key on the system or ima_mok
> keyrings.  To replicate this behavior, the default behavior should be to
> only permit certificates signed by a key on the builtin keyring, unless
> this new Kconfig is enabled.  Only then, permit certificates signed by a
> key on either the builtin or secondary keyrings to be added to the IMA
> keyring.

How about I change it to a choice-type item, with the following options:

 (1) No addition.

 (2) Addition restricted by built-in keyring.

 (3) Addition restricted by secondary keyring + built-in keyring.

where the second and third options then depend on the appropriate keyrings
being enabled.

David

[toc] | [prev] | [next] | [standalone]

#1353090 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On 16-03-08 13:08:36, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Only certificates signed by a key on the system keyring were added to
> > the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
> > certificate could be signed by a either a key on the system or ima_mok
> > keyrings.  To replicate this behavior, the default behavior should be to
> > only permit certificates signed by a key on the builtin keyring, unless
> > this new Kconfig is enabled.  Only then, permit certificates signed by a
> > key on either the builtin or secondary keyrings to be added to the IMA
> > keyring.
> 
> How about I change it to a choice-type item, with the following options:
> 
>  (1) No addition.
> 
>  (2) Addition restricted by built-in keyring.
> 
>  (3) Addition restricted by secondary keyring + built-in keyring.
> 
> where the second and third options then depend on the appropriate keyrings 
> being enabled.

I would suggest leaving (1) and (3).  Since secondary keyring only accepts keys 
signed by certificate in the system keyring I think (2) is redundant.  It adds 
extra complexity (Kconfig is vague enough already) while it doesn't increase the 
overall security by much.


cheers,
Petko

[toc] | [prev] | [next] | [standalone]

#1353103 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On Tue, 2016-03-08 at 16:14 +0200, Petko Manolov wrote:
> On 16-03-08 13:08:36, David Howells wrote:
> > Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > 
> > > Only certificates signed by a key on the system keyring were added to
> > > the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
> > > certificate could be signed by a either a key on the system or ima_mok
> > > keyrings.  To replicate this behavior, the default behavior should be to
> > > only permit certificates signed by a key on the builtin keyring, unless
> > > this new Kconfig is enabled.  Only then, permit certificates signed by a
> > > key on either the builtin or secondary keyrings to be added to the IMA
> > > keyring.
> > 
> > How about I change it to a choice-type item, with the following options:
> > 
> >  (1) No addition.
> > 
> >  (2) Addition restricted by built-in keyring.
> > 
> >  (3) Addition restricted by secondary keyring + built-in keyring.
> > 
> > where the second and third options then depend on the appropriate keyrings 
> > being enabled.
> 
> I would suggest leaving (1) and (3).  Since secondary keyring only accepts keys 
> signed by certificate in the system keyring I think (2) is redundant.  It adds 
> extra complexity (Kconfig is vague enough already) while it doesn't increase the 
> overall security by much.

I think you mean option 2 or 3, as option 1 implies not allowing any
keys to be added to the IMA keyring.

Mimi

[toc] | [prev] | [next] | [standalone]

#1353114 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

Petko Manolov <petkan@mip-labs.com> wrote:

> I would suggest leaving (1) and (3).

Do you mean dropping option (2) and leaving (1) and (3)?  Or do you mean
dropping options (1) and (3)?

David

[toc] | [prev] | [next] | [standalone]

#1353157 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On 16-03-08 14:44:24, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > I would suggest leaving (1) and (3).
> 
> Do you mean dropping option (2) and leaving (1) and (3)?  Or do you mean 
> dropping options (1) and (3)?

Dropping option (2) and leaving (1) and (3).

(2) is subset of (3) and IMHO adds unnecessary complexity.


		Petko

[toc] | [prev] | [next] | [standalone]

#1353184 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

Petko Manolov <petkan@mip-labs.com> wrote:

> > How about I change it to a choice-type item, with the following options:
> > 
> >  (1) No addition.
> > 
> >  (2) Addition restricted by built-in keyring.
> > 
> >  (3) Addition restricted by secondary keyring + built-in keyring.
> > 
> > where the second and third options then depend on the appropriate keyrings 
> > being enabled.
> 
> I would suggest leaving (1) and (3).  Since secondary keyring only accepts
> keys signed by certificate in the system keyring I think (2) is redundant.
> It adds extra complexity (Kconfig is vague enough already) while it doesn't
> increase the overall security by much.

If I remove option (2), that would mean that if you want to allow keys to be
added to .ima if they're signed by the built-in keyring, then you also allow
keys to be added to .ima if they're signed by the secondary keyring if
enabled.

Remember - these keyrings aren't necessarily restricted to IMA.

David

[toc] | [prev] | [next] | [standalone]

#1353201 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On 16-03-08 16:07:00, David Howells wrote:
> Petko Manolov <petkan@mip-labs.com> wrote:
> 
> > > How about I change it to a choice-type item, with the following options:
> > > 
> > >  (1) No addition.
> > > 
> > >  (2) Addition restricted by built-in keyring.
> > > 
> > >  (3) Addition restricted by secondary keyring + built-in keyring.
> > > 
> > > where the second and third options then depend on the appropriate keyrings 
> > > being enabled.
> > 
> > I would suggest leaving (1) and (3).  Since secondary keyring only accepts
> > keys signed by certificate in the system keyring I think (2) is redundant.
> > It adds extra complexity (Kconfig is vague enough already) while it doesn't
> > increase the overall security by much.
> 
> If I remove option (2), that would mean that if you want to allow keys to be 
> added to .ima if they're signed by the built-in keyring, then you also allow 
> keys to be added to .ima if they're signed by the secondary keyring if 
> enabled.

Exactly.  The primary difference between the built-in and secondary keyring is 
that the latter is R/W.  Chances are the user want either no addition or need 
dynamic key add/remove.

I don't have strong opinions against (2).  This is more of a discussion whether 
we should sacrifice in favor of simplicity or flexibility.

> Remember - these keyrings aren't necessarily restricted to IMA.

I am well aware of that.  At some point (perhaps not now) i'd like to discuss 
allowing kernel module loading based on keys in the secondary keyring.  It is a 
niche feature for those machines that have uptime measured in years.  I 
certainly don't expect it to be something the regular desktop or embedded users 
need.

Another issue that we left unresolved is the system-wide blacklist keyring.  It 
is at the same hierarchy level as the secondary keyring and serves a similar 
purpose although in opposite direction.


		Petko

[toc] | [prev] | [next] | [standalone]

#1353442 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On Tue, 2016-03-08 at 18:37 +0200, Petko Manolov wrote:
> On 16-03-08 16:07:00, David Howells wrote:
> > Petko Manolov <petkan@mip-labs.com> wrote:
> > 
> > > > How about I change it to a choice-type item, with the following options:
> > > > 
> > > >  (1) No addition.
> > > > 
> > > >  (2) Addition restricted by built-in keyring.
> > > > 
> > > >  (3) Addition restricted by secondary keyring + built-in keyring.
> > > > 
> > > > where the second and third options then depend on the appropriate keyrings 
> > > > being enabled.
> > > 
> > > I would suggest leaving (1) and (3).  Since secondary keyring only accepts
> > > keys signed by certificate in the system keyring I think (2) is redundant.
> > > It adds extra complexity (Kconfig is vague enough already) while it doesn't
> > > increase the overall security by much.
> > 
> > If I remove option (2), that would mean that if you want to allow keys to be 
> > added to .ima if they're signed by the built-in keyring, then you also allow 
> > keys to be added to .ima if they're signed by the secondary keyring if 
> > enabled.
> 
> Exactly.  The primary difference between the built-in and secondary keyring is 
> that the latter is R/W.  Chances are the user want either no addition or need 
> dynamic key add/remove.

Option 1 will prevent ANY keys from being added to the IMA keyring that
were not builtin and loaded by the kernel, similar to the existing
system certificate list.  The keyring itself would need to allow the
builtin keys to be added with the "KEY_ALLOC_BYPASS_RESTRICTION"
override.

Option 2 only allows certificates signed by a key on the builtin keyring
to be added to the IMA keyring.  (This should be the default.)

> I don't have strong opinions against (2).  This is more of a discussion whether 
> we should sacrifice in favor of simplicity or flexibility.

I disagree.  There is a major difference between option 2 and 3.

Mimi

[toc] | [prev] | [next] | [standalone]

#1353091 — Re: [RFC PATCH 12/12] IMA: Use the the system trusted keyrings instead of .ima_mok [ver #2]

On Tue, 2016-03-08 at 13:08 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > Only certificates signed by a key on the system keyring were added to
> > the IMA keyring, unless IMA_MOK_KEYRING was configured.  Then, the
> > certificate could be signed by a either a key on the system or ima_mok
> > keyrings.  To replicate this behavior, the default behavior should be to
> > only permit certificates signed by a key on the builtin keyring, unless
> > this new Kconfig is enabled.  Only then, permit certificates signed by a
> > key on either the builtin or secondary keyrings to be added to the IMA
> > keyring.
> 
> How about I change it to a choice-type item, with the following options:
> 
>  (1) No addition.
> 
>  (2) Addition restricted by built-in keyring.
> 
>  (3) Addition restricted by secondary keyring + built-in keyring.
> 
> where the second and third options then depend on the appropriate keyrings
> being enabled.

So option 1 is where IMA-appraisal is configured, but neither the
builtin nor the secondary keys are enabled.   This would be the
equivalent of having a set of "builtin" keys loaded directly onto the
IMA keyring, without the ability of adding additional keys.

Ok, I'm fine this solution.

thanks,

Mimi

[toc] | [prev] | [next] | [standalone]

#1350282 — [RFC PATCH 07/12] X.509: Move the trust validation code out to its own file [ver #2]

Move the X.509 trust validation code out to its own file so that it can be
generalised.

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

 crypto/asymmetric_keys/Makefile          |    5 +
 crypto/asymmetric_keys/restrict.c        |  106 ++++++++++++++++++++++++++++++
 crypto/asymmetric_keys/x509_parser.h     |    6 ++
 crypto/asymmetric_keys/x509_public_key.c |   79 ----------------------
 4 files changed, 116 insertions(+), 80 deletions(-)
 create mode 100644 crypto/asymmetric_keys/restrict.c

diff --git a/crypto/asymmetric_keys/Makefile b/crypto/asymmetric_keys/Makefile
index f90486256f01..6516855bec18 100644
--- a/crypto/asymmetric_keys/Makefile
+++ b/crypto/asymmetric_keys/Makefile
@@ -4,7 +4,10 @@
 
 obj-$(CONFIG_ASYMMETRIC_KEY_TYPE) += asymmetric_keys.o
 
-asymmetric_keys-y := asymmetric_type.o signature.o
+asymmetric_keys-y := \
+	asymmetric_type.o \
+	restrict.o \
+	signature.o
 
 obj-$(CONFIG_ASYMMETRIC_PUBLIC_KEY_SUBTYPE) += public_key.o
 
diff --git a/crypto/asymmetric_keys/restrict.c b/crypto/asymmetric_keys/restrict.c
new file mode 100644
index 000000000000..b4c10f2f5034
--- /dev/null
+++ b/crypto/asymmetric_keys/restrict.c
@@ -0,0 +1,106 @@
+/* Instantiate a public key crypto key from an X.509 Certificate
+ *
+ * Copyright (C) 2012 Red Hat, Inc. All Rights Reserved.
+ * Written by David Howells (dhowells@redhat.com)
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public Licence
+ * as published by the Free Software Foundation; either version
+ * 2 of the Licence, or (at your option) any later version.
+ */
+
+#define pr_fmt(fmt) "X.509: "fmt
+#include <linux/module.h>
+#include <linux/kernel.h>
+#include <linux/slab.h>
+#include <linux/err.h>
+#include <linux/mpi.h>
+#include <linux/asn1_decoder.h>
+#include <keys/asymmetric-subtype.h>
+#include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
+#include <crypto/hash.h>
+#include <crypto/public_key.h>
+#include "asymmetric_keys.h"
+#include "x509_parser.h"
+
+static bool use_builtin_keys;
+static struct asymmetric_key_id *ca_keyid;
+
+#ifndef MODULE
+static struct {
+	struct asymmetric_key_id id;
+	unsigned char data[10];
+} cakey;
+
+static int __init ca_keys_setup(char *str)
+{
+	if (!str)		/* default system keyring */
+		return 1;
+
+	if (strncmp(str, "id:", 3) == 0) {
+		struct asymmetric_key_id *p = &cakey.id;
+		size_t hexlen = (strlen(str) - 3) / 2;
+		int ret;
+
+		if (hexlen == 0 || hexlen > sizeof(cakey.data)) {
+			pr_err("Missing or invalid ca_keys id\n");
+			return 1;
+		}
+
+		ret = __asymmetric_key_hex_to_key_id(str + 3, p, hexlen);
+		if (ret < 0)
+			pr_err("Unparsable ca_keys id hex string\n");
+		else
+			ca_keyid = p;	/* owner key 'id:xxxxxx' */
+	} else if (strcmp(str, "builtin") == 0) {
+		use_builtin_keys = true;
+	}
+
+	return 1;
+}
+__setup("ca_keys=", ca_keys_setup);
+#endif
+
+/*
+ * 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.
+ */
+int x509_validate_trust(struct x509_certificate *cert,
+			struct key *trust_keyring)
+{
+	struct public_key_signature *sig = cert->sig;
+	struct key *key;
+	int ret = 1;
+
+	if (!sig->auth_ids[0] && !sig->auth_ids[1])
+		return 1;
+
+	if (!trust_keyring)
+		return -EOPNOTSUPP;
+	if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
+		return -EPERM;
+	if (cert->unsupported_sig)
+		return -ENOPKG;
+
+	key = find_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 = verify_signature(key, cert->sig);
+		if (ret == -ENOPKG)
+			cert->unsupported_sig = true;
+	}
+	key_put(key);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(x509_validate_trust);
diff --git a/crypto/asymmetric_keys/x509_parser.h b/crypto/asymmetric_keys/x509_parser.h
index 05eef1c68881..7a802b09a509 100644
--- a/crypto/asymmetric_keys/x509_parser.h
+++ b/crypto/asymmetric_keys/x509_parser.h
@@ -58,3 +58,9 @@ 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 117a6ee71a4d..6d7f42f0de9a 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -20,44 +20,6 @@
 #include "asymmetric_keys.h"
 #include "x509_parser.h"
 
-static bool use_builtin_keys;
-static struct asymmetric_key_id *ca_keyid;
-
-#ifndef MODULE
-static struct {
-	struct asymmetric_key_id id;
-	unsigned char data[10];
-} cakey;
-
-static int __init ca_keys_setup(char *str)
-{
-	if (!str)		/* default system keyring */
-		return 1;
-
-	if (strncmp(str, "id:", 3) == 0) {
-		struct asymmetric_key_id *p = &cakey.id;
-		size_t hexlen = (strlen(str) - 3) / 2;
-		int ret;
-
-		if (hexlen == 0 || hexlen > sizeof(cakey.data)) {
-			pr_err("Missing or invalid ca_keys id\n");
-			return 1;
-		}
-
-		ret = __asymmetric_key_hex_to_key_id(str + 3, p, hexlen);
-		if (ret < 0)
-			pr_err("Unparsable ca_keys id hex string\n");
-		else
-			ca_keyid = p;	/* owner key 'id:xxxxxx' */
-	} else if (strcmp(str, "builtin") == 0) {
-		use_builtin_keys = true;
-	}
-
-	return 1;
-}
-__setup("ca_keys=", ca_keys_setup);
-#endif
-
 /*
  * Set up the signature parameters in an X.509 certificate.  This involves
  * digesting the signed data and extracting the signature.
@@ -188,47 +150,6 @@ not_self_signed:
 }
 
 /*
- * 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.
- */
-static int x509_validate_trust(struct x509_certificate *cert,
-			       struct key *trust_keyring)
-{
-	struct public_key_signature *sig = cert->sig;
-	struct key *key;
-	int ret = 1;
-
-	if (!sig->auth_ids[0] && !sig->auth_ids[1])
-		return 1;
-
-	if (!trust_keyring)
-		return -EOPNOTSUPP;
-	if (ca_keyid && !asymmetric_key_id_partial(sig->auth_ids[1], ca_keyid))
-		return -EPERM;
-	if (cert->unsupported_sig)
-		return -ENOPKG;
-
-	key = find_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 = verify_signature(key, cert->sig);
-		if (ret == -ENOPKG)
-			cert->unsupported_sig = true;
-	}
-	key_put(key);
-	return ret;
-}
-
-/*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */
 static int x509_key_preparse(struct key_preparsed_payload *prep)

[toc] | [prev] | [next] | [standalone]

#1350284 — [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

Add a secondary system keyring that can be added to by root whilst the
system is running - provided the key being added is vouched for by a key
built into the kernel or already added to the secondary keyring.

Rename .system_keyring to .builtin_trusted_keys to distinguish it more
obviously from the new keyring (called .secondary_trusted_keys).

The new keyring needs to be enabled with CONFIG_SECONDARY_TRUSTED_KEYRING.

If the secondary keyring is enabled, a link is created from that to
.builtin_trusted_keys so that the the latter will automatically be searched
too if the secondary keyring is searched.

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

 certs/Kconfig                 |    8 ++++
 certs/system_keyring.c        |   79 ++++++++++++++++++++++++++++++++++-------
 include/keys/system_keyring.h |    4 ++
 3 files changed, 78 insertions(+), 13 deletions(-)

diff --git a/certs/Kconfig b/certs/Kconfig
index 743d480f5f6f..fc5955f5fc8a 100644
--- a/certs/Kconfig
+++ b/certs/Kconfig
@@ -56,4 +56,12 @@ config SYSTEM_EXTRA_CERTIFICATE_SIZE
 	  This is the number of bytes reserved in the kernel image for a
 	  certificate to be inserted.
 
+config SECONDARY_TRUSTED_KEYRING
+	bool "Provide a keyring to which extra trustable keys may be added"
+	depends on SYSTEM_TRUSTED_KEYRING
+	help
+	  If set, provide a keyring to which extra keys may be added, provided
+	  those keys are not blacklisted and are vouched for by a key built
+	  into the kernel or already in the secondary trusted keyring.
+
 endmenu
diff --git a/certs/system_keyring.c b/certs/system_keyring.c
index 3cfc88c76aee..9700f37350ff 100644
--- a/certs/system_keyring.c
+++ b/certs/system_keyring.c
@@ -18,7 +18,10 @@
 #include <keys/system_keyring.h>
 #include <crypto/pkcs7.h>
 
-static struct key *system_trusted_keyring;
+static struct key *builtin_trusted_keys;
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+static struct key *secondary_trusted_keys;
+#endif
 
 extern __initconst const u8 system_certificate_list[];
 extern __initconst const unsigned long system_certificate_list_size;
@@ -33,26 +36,68 @@ int restrict_link_by_system_trusted(struct key *keyring,
 				    const struct key_type *type,
 				    const union key_payload *payload)
 {
-	return restrict_link_by_signature(system_trusted_keyring,
-					  type, payload);
+	/* If we have a secondary trusted keyring, then that contains a link
+	 * through to the builtin keyring and the search will follow that link.
+	 */
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+	if (type == &key_type_keyring &&
+	    keyring == secondary_trusted_keys &&
+	    payload == &builtin_trusted_keys->payload)
+		/* Allow the builtin keyring to be added to the secondary */
+		return 0;
+
+	return restrict_link_by_signature(secondary_trusted_keys, type, payload);
+#else
+	return restrict_link_by_signature(builtin_trusted_keys, type, payload);
+#endif
+}
+
+/**
+ * restrict_link_to_builtin_trusted - Restrict keyring addition by built in CA
+ *
+ * Restrict the addition of keys into a keyring based on the key-to-be-added
+ * being vouched for by a key in the built in system keyring.
+ */
+int restrict_link_by_builtin_trusted(struct key *keyring,
+				     const struct key_type *type,
+				     const union key_payload *payload)
+{
+	return restrict_link_by_signature(builtin_trusted_keys, type, payload);
 }
 
 /*
- * Load the compiled-in keys
+ * Create the trusted keyrings
  */
 static __init int system_trusted_keyring_init(void)
 {
-	pr_notice("Initialise system trusted keyring\n");
+	pr_notice("Initialise system trusted keyrings\n");
 
-	system_trusted_keyring =
-		keyring_alloc(".system_keyring",
+	builtin_trusted_keys =
+		keyring_alloc(".builtin_trusted_keys",
 			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
 			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
 			      KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH),
 			      KEY_ALLOC_NOT_IN_QUOTA,
+			      NULL, NULL);
+	if (IS_ERR(builtin_trusted_keys))
+		panic("Can't allocate builtin trusted keyring\n");
+
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+	secondary_trusted_keys =
+		keyring_alloc(".secondary_trusted_keys",
+			      KUIDT_INIT(0), KGIDT_INIT(0), current_cred(),
+			      ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+			       KEY_USR_VIEW | KEY_USR_READ | KEY_USR_SEARCH |
+			       KEY_USR_WRITE),
+			      KEY_ALLOC_NOT_IN_QUOTA,
 			      restrict_link_by_system_trusted, NULL);
-	if (IS_ERR(system_trusted_keyring))
-		panic("Can't allocate system trusted keyring\n");
+	if (IS_ERR(secondary_trusted_keys))
+		panic("Can't allocate secondary trusted keyring\n");
+
+	if (key_link(secondary_trusted_keys, builtin_trusted_keys) < 0)
+		panic("Can't link trusted keyrings\n");
+#endif
+
 	return 0;
 }
 
@@ -88,7 +133,7 @@ static __init int load_system_certificate_list(void)
 		if (plen > end - p)
 			goto dodgy_cert;
 
-		key = key_create_or_update(make_key_ref(system_trusted_keyring, 1),
+		key = key_create_or_update(make_key_ref(builtin_trusted_keys, 1),
 					   "asymmetric",
 					   NULL,
 					   p,
@@ -125,7 +170,8 @@ late_initcall(load_system_certificate_list);
  * @len: Size of @data.
  * @raw_pkcs7: The PKCS#7 message that is the signature.
  * @pkcs7_len: The size of @raw_pkcs7.
- * @trusted_keys: Trusted keys to use (NULL for system_trusted_keyring).
+ * @trusted_keys: Trusted keys to use (NULL for builtin trusted keys only,
+ *					(void *)1UL for all trusted keys).
  * @usage: The use to which the key is being put.
  * @view_content: Callback to gain access to content.
  * @ctx: Context for callback.
@@ -157,8 +203,15 @@ int verify_pkcs7_signature(const void *data, size_t len,
 	if (ret < 0)
 		goto error;
 
-	if (!trusted_keys)
-		trusted_keys = system_trusted_keyring;
+	if (!trusted_keys) {
+		trusted_keys = builtin_trusted_keys;
+	} else if (trusted_keys == (void *)1UL) {
+#ifdef CONFIG_SECONDARY_TRUSTED_KEYRING
+		trusted_keys = secondary_trusted_keys;
+#else
+		trusted_keys = builtin_trusted_keys;
+#endif
+	}
 	ret = pkcs7_validate_trust(pkcs7, trusted_keys);
 	if (ret < 0) {
 		if (ret == -ENOKEY)
diff --git a/include/keys/system_keyring.h b/include/keys/system_keyring.h
index 71f3e2523767..c6c05f1513f5 100644
--- a/include/keys/system_keyring.h
+++ b/include/keys/system_keyring.h
@@ -19,9 +19,13 @@
 extern int restrict_link_by_system_trusted(struct key *keyring,
 					   const struct key_type *type,
 					   const union key_payload *payload);
+extern int restrict_link_by_builtin_trusted(struct key *keyring,
+					    const struct key_type *type,
+					    const union key_payload *payload);
 
 #else
 #define restrict_link_by_system_trusted restrict_link_reject
+#define restrict_link_by_builtin_trusted restrict_link_reject
 #endif
 
 #ifdef CONFIG_IMA_MOK_KEYRING

[toc] | [prev] | [next] | [standalone]

#1352554 — Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

On Fri, 2016-03-04 at 15:01 +0000, David Howells wrote:
> Add a secondary system keyring that can be added to by root whilst the
> system is running - provided the key being added is vouched for by a key
> built into the kernel or already added to the secondary keyring.
> 
> Rename .system_keyring to .builtin_trusted_keys to distinguish it more
> obviously from the new keyring (called .secondary_trusted_keys).

Renaming of the system_trusted_keyring to builtin_trusted_keys is fine,
but we're left with a lot of references to "system_trusted" (eg.
restrict_link_to_system_trusted, depends on SYSTEM_TRUSTED_KEYRING,  the
subsequent patch description and Kconfig use "system trusted keyrings",
etc).   Without changing these references, I'm not convinced this is an
improvement. 

Mimi

> The new keyring needs to be enabled with CONFIG_SECONDARY_TRUSTED_KEYRING.
> 
> If the secondary keyring is enabled, a link is created from that to
> .builtin_trusted_keys so that the the latter will automatically be searched
> too if the secondary keyring is searched.
> 
> Signed-off-by: David Howells <dhowells@redhat.com>

[toc] | [prev] | [next] | [standalone]

#1353022 — Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> but we're left with a lot of references to "system_trusted" (eg.
> restrict_link_to_system_trusted, depends on SYSTEM_TRUSTED_KEYRING

How about I pluralise it to SYSTEM_TRUSTED_KEYRINGS?  The fact that one is
called builtin and the other secondary doesn't detract from the fact that
they're both system-wide rings of trusted keys.

Or would you prefer .system_trusted_keys and .secondary_trusted_keys?  Even
though the second is also a "system" trusted keyring.

David

[toc] | [prev] | [next] | [standalone]

#1353092 — Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

On 16-03-08 13:13:59, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > but we're left with a lot of references to "system_trusted" (eg.
> > restrict_link_to_system_trusted, depends on SYSTEM_TRUSTED_KEYRING
> 
> How about I pluralise it to SYSTEM_TRUSTED_KEYRINGS?  The fact that one is
> called builtin and the other secondary doesn't detract from the fact that
> they're both system-wide rings of trusted keys.
> 
> Or would you prefer .system_trusted_keys and .secondary_trusted_keys?  Even
> though the second is also a "system" trusted keyring.

Ah, naming things...  This is true science... :-)


cheers,
Petko

[toc] | [prev] | [next] | [standalone]

#1353100 — Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

On Tue, 2016-03-08 at 13:13 +0000, David Howells wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> > but we're left with a lot of references to "system_trusted" (eg.
> > restrict_link_to_system_trusted, depends on SYSTEM_TRUSTED_KEYRING
> 
> How about I pluralise it to SYSTEM_TRUSTED_KEYRINGS?  The fact that one is
> called builtin and the other secondary doesn't detract from the fact that
> they're both system-wide rings of trusted keys.

Would then restrict_link_to_system_trusted imply both the builtin and
secondary keyrings or just the builtin keyrings?   Changing the system
keyring name to builtin keys, without changing the corresponding
restrict_link name, obfuscates what is really happening.

Mimi

[toc] | [prev] | [next] | [standalone]

#1353115 — Re: [RFC PATCH 11/12] certs: Add a secondary system keyring that can be added to dynamically [ver #2]

Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> Would then restrict_link_to_system_trusted imply both the builtin and
> secondary keyrings or just the builtin keyrings?

Both, if available; just builtin if the secondary is not available.

restrict_link_by_builtin_trusted() does only the builtin.

> Changing the system keyring name to builtin keys, without changing the
> corresponding restrict_link name, obfuscates what is really happening.

You can still look at the code, it's not as if it's particularly hidden.

The problem boils down to a difficulty in concocting a name that describes a
complex situation that may change depending on the configuration.  I can make
it "restrict_link_by_any_system_trusted" if you'd prefer.

That's why I want "system trusted keyrings" to refer to the builtin and the
secondary - *and* an extra UEFI keyring if we grow one of those.  It's a
collection of related keyrings.

David

[toc] | [prev] | [next] | [standalone]

Page 1 of 2  [1] 2  Next page →

Back to top | Article view | linux.kernel

csiph-web