[PATCH 00/20] in-kernel IMA/EVM initialization

[PATCH 00/20] in-kernel IMA/EVM initialization

[Dmitry Kasatkin]

Hi,

Currently secure IMA/EVM initialization has to be done from the
initramfs, embedded in the signed kernel image. Many systems do
not want to use initramfs or use unsigned locally generated images.

This patchset introduces kernel functionality, which allows to perform
IMA/EVM initialization without initramfs from the kernel, which
includes mainly following:
- loading EVM hmac encrypted keys
- loading and verification of signed X509 certificates
- loading and verification of signed IMA policy

Patchset introduces the set of new kernel configuration options,
which makes this functionality entirely configurable.
Not enabling any of the options does not change original IMA/EVM
behavior. In order not to bloat security configuration menu,
integrity subsystem options were moved to the separate menu.
It does not affect existing configuration. Re-configuration is
not needed.

-Dmitry

Dmitry Kasatkin (19):
  integrity: initialize EVM before IMA
  ima: move asymmetric keys config option
  integrity: move integrity subsystem options to a separate menu
  integrity: provide builtin 'trusted' keyrings
  ima: create '_ima' as a builtin 'trusted' keyring
  integrity: provide x509 certificate loading from the kernel
  ima: load x509 certificate from the kernel
  evm: create '_evm' as a builtin 'trusted' keyring
  evm: load x509 certificate from the kernel
  ima: added kernel parameter for disabling IMA
  ima: provide buffer hash calculation function
  ima: replace opencount with bitop
  ima: check if policy was set at open
  ima: path based policy loading interface
  ima: load policy from the kernel
  ima: make IMA policy replaceable at runtime
  evm: added kernel parameter for disabling EVM
  evm: try enable EVM from the kernel
  evm: read EVM key from the kernel

Mimi Zohar (1):
  KEYS: verify a certificate is signed by a 'trusted' key

 crypto/asymmetric_keys/x509_public_key.c |  85 +++++++++++++++++++++++-
 security/integrity/Kconfig               |  41 ++++++++----
 security/integrity/Makefile              |   4 +-
 security/integrity/digsig.c              | 103 +++++++++++++++++++++++++++++
 security/integrity/evm/Kconfig           |  32 +++++++--
 security/integrity/evm/evm.h             |  14 ++++
 security/integrity/evm/evm_crypto.c      | 101 ++++++++++++++++++++++++++++
 security/integrity/evm/evm_main.c        |  25 +++++--
 security/integrity/evm/evm_secfs.c       |  13 ++--
 security/integrity/ima/Kconfig           |  49 +++++++++++++-
 security/integrity/ima/ima.h             |  19 ++++++
 security/integrity/ima/ima_crypto.c      |  11 +++-
 security/integrity/ima/ima_fs.c          |  48 ++++++++++----
 security/integrity/ima/ima_init.c        |   3 +
 security/integrity/ima/ima_main.c        |  12 +++-
 security/integrity/ima/ima_policy.c      | 109 ++++++++++++++++++++++++++++---
 security/integrity/integrity.h           |  20 ++++++
 17 files changed, 626 insertions(+), 63 deletions(-)

-- 
1.8.3.2

[PATCH 01/20] KEYS: verify a certificate is signed by a 'trusted' key

[Dmitry Kasatkin]

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

Only public keys, with certificates signed by an existing
'trusted' key on the system trusted keyring, should be added
to a trusted keyring.  This patch adds support for verifying
a certificate's signature.

This is derived from David Howells pkcs7_request_asymmetric_key() patch.

Changes:
- Flaged out the code to prevent build break if system keyring
  is not enabled (Dmitry).

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
index 382ef0d..d279f43 100644
--- a/crypto/asymmetric_keys/x509_public_key.c
+++ b/crypto/asymmetric_keys/x509_public_key.c
@@ -18,6 +18,7 @@
 #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 "asymmetric_keys.h"
 #include "public_key.h"
@@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
 }
 EXPORT_SYMBOL_GPL(x509_check_signature);
 
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+/*
+ * Find a key in the given keyring by issuer and authority.
+ */
+static struct key *x509_request_asymmetric_key(
+	struct key *keyring,
+	const char *signer, size_t signer_len,
+	const char *authority, size_t auth_len)
+{
+	key_ref_t key;
+	char *id;
+
+	/* Construct an identifier. */
+	id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
+	if (!id)
+		return ERR_PTR(-ENOMEM);
+
+	memcpy(id, signer, signer_len);
+	id[signer_len + 0] = ':';
+	id[signer_len + 1] = ' ';
+	memcpy(id + signer_len + 2, authority, auth_len);
+	id[signer_len + 2 + auth_len] = 0;
+
+	pr_debug("Look up: \"%s\"\n", id);
+
+	key = keyring_search(make_key_ref(keyring, 1),
+			     &key_type_asymmetric, id);
+	if (IS_ERR(key))
+		pr_debug("Request for module key '%s' err %ld\n",
+			 id, PTR_ERR(key));
+	kfree(id);
+
+	if (IS_ERR(key)) {
+		switch (PTR_ERR(key)) {
+			/* Hide some search errors */
+		case -EACCES:
+		case -ENOTDIR:
+		case -EAGAIN:
+			return ERR_PTR(-ENOKEY);
+		default:
+			return ERR_CAST(key);
+		}
+	}
+
+	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
+	return key_ref_to_ptr(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.
+ */
+static int x509_validate_trust(struct x509_certificate *cert,
+			       struct key *trust_keyring)
+{
+	const struct public_key *pk;
+	struct key *key;
+	int ret = 1;
+
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->issuer, strlen(cert->issuer),
+					  cert->authority,
+					  strlen(cert->authority));
+	if (!IS_ERR(key))  {
+		pk = key->payload.data;
+		ret = x509_check_signature(pk, cert);
+	}
+	return ret;
+}
+#endif
+
 /*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */
@@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	/* Check the signature on the key if it appears to be self-signed */
 	if (!cert->authority ||
 	    strcmp(cert->fingerprint, cert->authority) == 0) {
-		ret = x509_check_signature(cert->pub, cert);
+		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
+	} else {
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+		ret = x509_validate_trust(cert, system_trusted_keyring);
+		if (!ret)
+			prep->trusted = 1;
+#endif
 	}
 
 	/* Propose a description */
-- 
1.8.3.2

[PATCH 02/20] integrity: initialize EVM before IMA

[Dmitry Kasatkin]

Initialize EVM before IMA to prevent appraisal failure when reading
EVM X509 certificate and HMAC key.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/integrity/Makefile b/security/integrity/Makefile
index 0793f48..2e17590 100644
--- a/security/integrity/Makefile
+++ b/security/integrity/Makefile
@@ -9,7 +9,7 @@ obj-$(CONFIG_INTEGRITY_ASYMMETRIC_KEYS) += digsig_asymmetric.o
 
 integrity-y := iint.o
 
-subdir-$(CONFIG_IMA)			+= ima
-obj-$(CONFIG_IMA)			+= ima/
 subdir-$(CONFIG_EVM)			+= evm
 obj-$(CONFIG_EVM)			+= evm/
+subdir-$(CONFIG_IMA)			+= ima
+obj-$(CONFIG_IMA)			+= ima/
-- 
1.8.3.2

[PATCH 03/20] ima: move asymmetric keys config option

[Dmitry Kasatkin]

For better visual appearance it is better to co-locate
asymmetric key option together with signature support.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/Kconfig | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 245c6d9..f79d853 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -17,6 +17,18 @@ config INTEGRITY_SIGNATURE
 	  This is useful for evm and module keyrings, when keys are
 	  usually only added from initramfs.
 
+config INTEGRITY_ASYMMETRIC_KEYS
+	boolean "Enable asymmetric keys support"
+	depends on INTEGRITY_SIGNATURE
+	default n
+        select ASYMMETRIC_KEY_TYPE
+        select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+        select PUBLIC_KEY_ALGO_RSA
+        select X509_CERTIFICATE_PARSER
+	help
+	  This option enables digital signature verification using
+	  asymmetric keys.
+
 config INTEGRITY_AUDIT
 	bool "Enables integrity auditing support "
 	depends on INTEGRITY && AUDIT
@@ -32,17 +44,5 @@ config INTEGRITY_AUDIT
 	  be enabled by specifying 'integrity_audit=1' on the kernel
 	  command line.
 
-config INTEGRITY_ASYMMETRIC_KEYS
-	boolean "Enable asymmetric keys support"
-	depends on INTEGRITY_SIGNATURE
-	default n
-        select ASYMMETRIC_KEY_TYPE
-        select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-        select PUBLIC_KEY_ALGO_RSA
-        select X509_CERTIFICATE_PARSER
-	help
-	  This option enables digital signature verification using
-	  asymmetric keys.
-
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig
-- 
1.8.3.2

[PATCH 04/20] integrity: move integrity subsystem options to a separate menu

[Dmitry Kasatkin]

Integrity subsystem got lots of options and takes more than half
of security menu.

This patch moves integrity subsystem options to a separate menu.
It does not affect existing configuration. Re-configuration is
not needed.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/Kconfig     | 11 ++++++++---
 security/integrity/evm/Kconfig |  9 +--------
 security/integrity/ima/Kconfig |  3 +--
 3 files changed, 10 insertions(+), 13 deletions(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index f79d853..b16c9cd 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -1,7 +1,10 @@
 #
-config INTEGRITY
-	def_bool y
-	depends on IMA || EVM
+menuconfig INTEGRITY
+	bool "Integrity Subsystem"
+	depends on SECURITY
+	default y
+
+if INTEGRITY
 
 config INTEGRITY_SIGNATURE
 	boolean "Digital signature verification using multiple keyrings"
@@ -46,3 +49,5 @@ config INTEGRITY_AUDIT
 
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig
+
+endif   # if INTEGRITY
diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index d606f3d..df20a2f 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -1,6 +1,6 @@
 config EVM
 	boolean "EVM support"
-	depends on SECURITY
+	depends on INTEGRITY
 	select KEYS
 	select ENCRYPTED_KEYS
 	select CRYPTO_HMAC
@@ -12,10 +12,6 @@ config EVM
 
 	  If you are unsure how to answer this question, answer N.
 
-if EVM
-
-menu "EVM options"
-
 config EVM_ATTR_FSUUID
 	bool "FSUUID (version 2)"
 	default y
@@ -47,6 +43,3 @@ config EVM_EXTRA_SMACK_XATTRS
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
-endmenu
-
-endif
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 81a2797..5f95e50 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -2,8 +2,7 @@
 #
 config IMA
 	bool "Integrity Measurement Architecture(IMA)"
-	depends on SECURITY
-	select INTEGRITY
+	depends on INTEGRITY
 	select SECURITYFS
 	select CRYPTO
 	select CRYPTO_HMAC
-- 
1.8.3.2

[PATCH 05/20] integrity: provide builtin 'trusted' keyrings

[Dmitry Kasatkin]

Provide creation of trusted keyrings, which require all keys
added to the keyrings be signed by an existing trusted key
on the system trusted keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/Kconfig     |  4 ++++
 security/integrity/digsig.c    | 31 +++++++++++++++++++++++++++++++
 security/integrity/integrity.h | 10 ++++++++++
 3 files changed, 45 insertions(+)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index b16c9cd..89f226a 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -47,6 +47,10 @@ config INTEGRITY_AUDIT
 	  be enabled by specifying 'integrity_audit=1' on the kernel
 	  command line.
 
+config INTEGRITY_TRUSTED_KEYRING
+	def_bool n
+	depends on IMA_TRUSTED_KEYRING || EVM_TRUSTED_KEYRING
+
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig
 
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index b4af4eb..45adc07 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -13,7 +13,9 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/err.h>
+#include <linux/sched.h>
 #include <linux/rbtree.h>
+#include <linux/cred.h>
 #include <linux/key-type.h>
 #include <linux/digsig.h>
 
@@ -56,3 +58,32 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 
 	return -EOPNOTSUPP;
 }
+
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+int integrity_init_keyring(const unsigned int id)
+{
+	const struct cred *cred = current_cred();
+	const struct user_struct *user = cred->user;
+
+	pr_notice("initialize trusted keyring: %s\n", keyring_name[id]);
+
+	/* this function relies that init_root_keyring() was executed
+	 * in 'keys' subsystem, which is initialized before integrity
+	 */
+
+	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_ALLOC_NOT_IN_QUOTA, user->uid_keyring);
+	if (IS_ERR(keyring[id])) {
+		long rc = PTR_ERR(keyring[id]);
+		pr_err("Can't allocate %s keyring (%ld)\n",
+		       keyring_name[id], rc);
+		keyring[id] = NULL;
+		return rc;
+	}
+	set_bit(KEY_FLAG_TRUSTED_ONLY, &keyring[id]->flags);
+	return 0;
+}
+#endif
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2fb5e53..dd26ad0 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -137,6 +137,7 @@ static inline int integrity_digsig_verify(const unsigned int id,
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
 int asymmetric_verify(struct key *keyring, const char *sig,
 		      int siglen, const char *data, int datalen);
+
 #else
 static inline int asymmetric_verify(struct key *keyring, const char *sig,
 				    int siglen, const char *data, int datalen)
@@ -145,6 +146,15 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
 }
 #endif
 
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+int integrity_init_keyring(const unsigned int id);
+#else
+static inline int integrity_init_keyring(const unsigned int id)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_AUDIT
 /* declarations */
 void integrity_audit_msg(int audit_msgno, struct inode *inode,
-- 
1.8.3.2

[PATCH 06/20] ima: create '_ima' as a builtin 'trusted' keyring

[Dmitry Kasatkin]

Require all keys added to the IMA keyring be signed by an
existing trusted key on the system trusted keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/Kconfig    | 9 +++++++++
 security/integrity/ima/ima_init.c | 1 +
 2 files changed, 10 insertions(+)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 5f95e50..be11c01 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -122,3 +122,12 @@ config IMA_APPRAISE
 	  For more information on integrity appraisal refer to:
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
+
+config IMA_TRUSTED_KEYRING
+	bool "Require all keys on the _ima keyring be signed"
+	depends on IMA && SYSTEM_TRUSTED_KEYRING
+	select INTEGRITY_TRUSTED_KEYRING
+	default y
+	help
+	   This option requires that all keys added to the _ima
+	   keyring be signed by a key on the system trusted keyring.
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index e8f9d70..515c1a2 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -107,6 +107,7 @@ int __init ima_init(void)
 
 	ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	ima_init_policy();
+	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 
 	return ima_fs_init();
 }
-- 
1.8.3.2

[PATCH 07/20] integrity: provide x509 certificate loading from the kernel

[Dmitry Kasatkin]

Provide API to load x509 certificates from the kernel into the
integrity kernel keyrings.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/Kconfig     |  4 +++
 security/integrity/digsig.c    | 72 ++++++++++++++++++++++++++++++++++++++++++
 security/integrity/integrity.h | 10 ++++++
 3 files changed, 86 insertions(+)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 89f226a..8f34b28 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -51,6 +51,10 @@ config INTEGRITY_TRUSTED_KEYRING
 	def_bool n
 	depends on IMA_TRUSTED_KEYRING || EVM_TRUSTED_KEYRING
 
+config INTEGRITY_LOAD_X509
+	def_bool n
+	depends on IMA_LOAD_X509 || EVM_LOAD_X509
+
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig
 
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 45adc07..cba38e3 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 <linux/slab.h>
+#include <linux/file.h>
 
 #include "integrity.h"
 
@@ -59,6 +61,76 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 	return -EOPNOTSUPP;
 }
 
+#ifdef CONFIG_INTEGRITY_LOAD_X509
+int integrity_read_file(const char *path, char **data)
+{
+	struct file *file;
+	loff_t size;
+	char *buf;
+	int rc =  -EINVAL;
+
+	file = filp_open(path, O_RDONLY, 0);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	size = i_size_read(file_inode(file));
+	if (size <= 0)
+		goto out;
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf) {
+		rc = -ENOMEM;
+		goto out;
+	}
+
+	rc = kernel_read(file, 0, buf, size);
+	if (rc < 0)
+		kfree(buf);
+	else if (rc != size)
+		rc = -EIO;
+	else
+		*data = buf;
+out:
+	fput(file);
+	return rc;
+}
+
+int integrity_load_x509(const unsigned int id, char *path)
+{
+	key_ref_t key;
+	char *data;
+	int rc;
+
+	if (!keyring[id])
+		return -EINVAL;
+
+	rc = integrity_read_file(path, &data);
+	if (rc < 0)
+		return rc;
+
+	key = key_create_or_update(make_key_ref(keyring[id], 1),
+				   "asymmetric",
+				   NULL,
+				   data,
+				   rc,
+				   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				   KEY_USR_VIEW | KEY_USR_READ),
+				   KEY_ALLOC_NOT_IN_QUOTA |
+				   KEY_ALLOC_TRUSTED);
+	if (IS_ERR(key)) {
+		rc = PTR_ERR(key);
+		pr_err("Problem loading X.509 certificate (%d): %s\n",
+		       rc, path);
+	} else {
+		pr_notice("Loaded X.509 cert '%s': %s\n",
+			  key_ref_to_ptr(key)->description, path);
+		key_ref_put(key);
+	}
+	kfree(data);
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
 int integrity_init_keyring(const unsigned int id)
 {
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index dd26ad0..96960fb 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -146,6 +146,16 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
 }
 #endif
 
+#ifdef CONFIG_INTEGRITY_LOAD_X509
+int integrity_read_file(const char *path, char **data);
+int integrity_load_x509(const unsigned int id, char *path);
+#else
+static inline int integrity_load_x509(const unsigned int id, char *path)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
 int integrity_init_keyring(const unsigned int id);
 #else
-- 
1.8.3.2

[PATCH 08/20] ima: load x509 certificate from the kernel

[Dmitry Kasatkin]

Provide configuration option to load X509 certificate into the
_ima kernel keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/Kconfig    | 9 +++++++++
 security/integrity/ima/ima_init.c | 1 +
 2 files changed, 10 insertions(+)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index be11c01..5474c47 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -131,3 +131,12 @@ config IMA_TRUSTED_KEYRING
 	help
 	   This option requires that all keys added to the _ima
 	   keyring be signed by a key on the system trusted keyring.
+
+config IMA_LOAD_X509
+	bool "Load X509 certificate to the '_ima' trusted keyring"
+	depends on IMA_TRUSTED_KEYRING
+	select INTEGRITY_LOAD_X509
+	default n
+	help
+	   This option enables X509 certificate loading from the kernel
+	   to the '_ima' trusted keyring.
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index 515c1a2..c13d6a8 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -108,6 +108,7 @@ int __init ima_init(void)
 	ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	ima_init_policy();
 	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
+	integrity_load_x509(INTEGRITY_KEYRING_IMA, "/etc/keys/x509_ima.der");
 
 	return ima_fs_init();
 }
-- 
1.8.3.2

[PATCH 09/20] evm: create '_evm' as a builtin 'trusted' keyring

[Dmitry Kasatkin]

Require all keys added to the EVM keyring be signed by an
existing trusted key on the system trusted keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig    | 8 ++++++++
 security/integrity/evm/evm_main.c | 2 ++
 2 files changed, 10 insertions(+)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index df20a2f..3f9098c6 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -43,3 +43,11 @@ config EVM_EXTRA_SMACK_XATTRS
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
+config EVM_TRUSTED_KEYRING
+	bool "Require all keys on the _evm keyring be signed"
+	depends on EVM && SYSTEM_TRUSTED_KEYRING
+	select INTEGRITY_TRUSTED_KEYRING
+	default n
+	help
+	   This option requires that all keys added to the _evm
+	   keyring be signed by a key on the system trusted keyring.
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 4c00adb..8a11920 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -447,6 +447,8 @@ static int __init init_evm(void)
 
 	evm_init_config();
 
+	integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+
 	error = evm_init_secfs();
 	if (error < 0) {
 		pr_info("Error registering secfs\n");
-- 
1.8.3.2

[PATCH 10/20] evm: load x509 certificate from the kernel

[Dmitry Kasatkin]

Provide configuration option to load X509 certificate into the
_evm kernel keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig    | 9 +++++++++
 security/integrity/evm/evm_main.c | 1 +
 2 files changed, 10 insertions(+)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index 3f9098c6..70d12f7 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -51,3 +51,12 @@ config EVM_TRUSTED_KEYRING
 	help
 	   This option requires that all keys added to the _evm
 	   keyring be signed by a key on the system trusted keyring.
+
+config EVM_LOAD_X509
+	bool "Load X509 certificate to the '_evm' trusted keyring"
+	depends on EVM_TRUSTED_KEYRING
+	select INTEGRITY_LOAD_X509
+	default n
+	help
+	   This option enables X509 certificate loading from the kernel
+	   to the '_evm' trusted keyring.
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 8a11920..0f1b489 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -448,6 +448,7 @@ static int __init init_evm(void)
 	evm_init_config();
 
 	integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+	integrity_load_x509(INTEGRITY_KEYRING_EVM, "/etc/keys/x509_ima.der");
 
 	error = evm_init_secfs();
 	if (error < 0) {
-- 
1.8.3.2

[PATCH 11/20] ima: added kernel parameter for disabling IMA

[Dmitry Kasatkin]

If ima_tcb and ima_appraise_tcb are not specified on the kernel command
line, IMA policy remains empty and IMA functionality is disabled.

When policy is loaded from the kernel, IMA becomes enabled without specifying
ima_tcb and ima_appraise_tcb. There should be a possibiliy to disable IMA.
Distributions might want to compile IMA support, but leave for the user
to decide if to enable or disable IMA functionality.

This patch provides kernel parameter 'ima=off' that allows to disable IMA.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index c0aaed8..6af4966 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -67,6 +67,15 @@ out:
 }
 __setup("ima_hash=", hash_setup);
 
+static int ima_mode = 1;
+static int __init ima_setup(char *str)
+{
+	if (strncmp(str, "off", 3) == 0)
+		ima_mode = 0;
+	return 1;
+}
+__setup("ima=", ima_setup);
+
 /*
  * ima_rdwr_violation_check
  *
@@ -324,8 +333,9 @@ static int __init init_ima(void)
 	int error;
 
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
+
 	error = ima_init();
-	if (!error)
+	if (!error && ima_mode)
 		ima_initialized = 1;
 	return error;
 }
-- 
1.8.3.2

[PATCH 12/20] ima: provide buffer hash calculation function

[Dmitry Kasatkin]

This patch provides convenient buffer hash calculation function.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_crypto.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f4c1e8dd..a5d5ccb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
 			      struct ima_template_desc *desc, int num_fields,
 			      struct ima_digest_data *hash);
diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
index 139e7f7..50c78c0 100644
--- a/security/integrity/ima/ima_crypto.c
+++ b/security/integrity/ima/ima_crypto.c
@@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
 		u8 *data_to_hash = field_data[i].data;
 		u32 datalen = field_data[i].len;
 
-		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
 			rc = crypto_shash_update(&desc.shash,
 						(const u8 *) &field_data[i].len,
 						sizeof(field_data[i].len));
 			if (rc)
 				break;
-		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
+		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
 			memcpy(buffer, data_to_hash, datalen);
 			data_to_hash = buffer;
 			datalen = IMA_EVENT_NAME_LEN_MAX + 1;
@@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+{
+	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
+
+	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)
-- 
1.8.3.2

[PATCH 13/20] ima: replace opencount with bitop

[Dmitry Kasatkin]

Current implementation uses opencount to allow only single writer
to update policy at a time. After policy update 'ima/policy' sysfs
entry is removed which disallow future updates.

Following patches might introduce posibility to update policy multiple
times. Current open count implementation will not be valid.

This patch replaces usage of opencount with busy bit.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima_fs.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index da92fcc..71f0d8f 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -288,7 +288,12 @@ static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
 
-static atomic_t policy_opencount = ATOMIC_INIT(1);
+enum ima_fs_flags {
+	IMA_FS_BUSY = 0,
+};
+
+static unsigned long ima_fs_flags;
+
 /*
  * ima_open_policy: sequentialize access to the policy file
  */
@@ -297,9 +302,9 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 	/* No point in being allowed to open it if you aren't going to write */
 	if (!(filp->f_flags & O_WRONLY))
 		return -EACCES;
-	if (atomic_dec_and_test(&policy_opencount))
-		return 0;
-	return -EBUSY;
+	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+		return -EBUSY;
+	return 0;
 }
 
 /*
@@ -314,12 +319,12 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	if (!valid_policy) {
 		ima_delete_rules();
 		valid_policy = 1;
-		atomic_set(&policy_opencount, 1);
-		return 0;
+	} else {
+		ima_update_policy();
+		securityfs_remove(ima_policy);
+		ima_policy = NULL;
 	}
-	ima_update_policy();
-	securityfs_remove(ima_policy);
-	ima_policy = NULL;
+	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 	return 0;
 }
 
-- 
1.8.3.2

[PATCH 14/20] ima: check if policy was set at open

[Dmitry Kasatkin]

IMA default behavior is to forbid more than one policy update.
It easier to check at open phase if policy was already set,
so it would not be necessary to perform useless policy parsing
and removing of sysfs entry.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_fs.c     |  7 +++++--
 security/integrity/ima/ima_policy.c | 16 +++++++++-------
 3 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index a5d5ccb..3b90b60 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -166,6 +166,7 @@ int ima_match_policy(struct dentry *dentry, enum ima_hooks func, int mask,
 		     int flags);
 void ima_init_policy(void);
 void ima_update_policy(void);
+bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);
 
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 71f0d8f..34ae5f2 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -304,6 +304,11 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return -EACCES;
 	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
 		return -EBUSY;
+	if (!ima_default_policy()) {
+		/* policy was already set*/
+		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+		return -EACCES;
+	}
 	return 0;
 }
 
@@ -321,8 +326,6 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 		valid_policy = 1;
 	} else {
 		ima_update_policy();
-		securityfs_remove(ima_policy);
-		ima_policy = NULL;
 	}
 	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 	return 0;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 9b8121b..b24e7d1 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -345,6 +345,11 @@ void __init ima_init_policy(void)
 	ima_rules = &ima_default_rules;
 }
 
+bool ima_default_policy(void)
+{
+	return ima_rules == &ima_default_rules;
+}
+
 /**
  * ima_update_policy - update default_rules with new measure rules
  *
@@ -355,15 +360,12 @@ void __init ima_init_policy(void)
 void ima_update_policy(void)
 {
 	static const char op[] = "policy_update";
-	const char *cause = "already exists";
-	int result = 1;
+	const char *cause = "complete";
+	int result = 0;
 	int audit_info = 0;
 
-	if (ima_rules == &ima_default_rules) {
-		ima_rules = &ima_policy_rules;
-		cause = "complete";
-		result = 0;
-	}
+	ima_rules = &ima_policy_rules;
+
 	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
 			    NULL, op, cause, result, audit_info);
 }
-- 
1.8.3.2

[PATCH 15/20] ima: path based policy loading interface

[Dmitry Kasatkin]

Currently policy is loaded by writing policy content to
'<securityfs>/ima/policy' file.

This patch extends policy loading meachanism with possibility
to load signed policy using a path to the policy.
Policy signature must be available in the <policy>.sig file.

Policy can be loaded like:
echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/Kconfig      | 13 +++++++
 security/integrity/ima/ima.h        |  9 +++++
 security/integrity/ima/ima_fs.c     |  2 +-
 security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 5474c47..465cef4 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -140,3 +140,16 @@ config IMA_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_ima' trusted keyring.
+
+config IMA_POLICY_LOADER
+	bool "Path based policy loading interface"
+	depends on IMA_TRUSTED_KEYRING
+	default n
+	help
+	  This option enables path based signed policy loading interface.
+	  Policy signature must be provided in the <policy>.sig file
+	  along with the policy. When this option is enabled, kernel
+	  tries to load default policy from /etc/ima_policy.
+
+	  Loading policy is like:
+	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 3b90b60..f2722bb 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -170,6 +170,15 @@ bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);
 
+#ifdef CONFIG_IMA_POLICY_LOADER
+ssize_t ima_read_policy(char *path);
+#else
+static inline ssize_t ima_read_policy(char *data)
+{
+	return ima_parse_add_rule(data);
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index 34ae5f2..bde7a0e 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	if (copy_from_user(data, buf, datalen))
 		goto out;
 
-	result = ima_parse_add_rule(data);
+	result = ima_read_policy(data);
 out:
 	if (result < 0)
 		valid_policy = 0;
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index b24e7d1..c6da801 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -17,6 +17,9 @@
 #include <linux/parser.h>
 #include <linux/slab.h>
 #include <linux/genhd.h>
+#ifdef CONFIG_IMA_POLICY_LOADER
+#include <linux/file.h>
+#endif
 
 #include "ima.h"
 
@@ -747,3 +750,74 @@ void ima_delete_rules(void)
 	}
 	mutex_unlock(&ima_rules_mutex);
 }
+
+#ifdef CONFIG_IMA_POLICY_LOADER
+
+ssize_t ima_read_policy(char *path)
+{
+	char *data, *datap, *sig;
+	int rc, psize, pathlen = strlen(path);
+	char *p, *sigpath;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
+
+	if (path[0] != '/')
+		return ima_parse_add_rule(path);
+
+	/* remove \n */
+	datap = path;
+	strsep(&datap, "\n");
+
+	/* we always want signature? */
+	sigpath = __getname();
+	if (!sigpath)
+		return -ENOMEM;
+
+	rc = integrity_read_file(path, &data);
+	if (rc < 0)
+		goto free_path;
+
+	psize = rc;
+	datap = data;
+
+	sprintf(sigpath, "%s.sig", path);
+	/* we always want signature? */
+	rc = integrity_read_file(sigpath, &sig);
+	if (rc < 0)
+		goto free_data;
+
+	hash.hdr.algo = ima_hash_algo;
+	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
+	ima_calc_buffer_hash(data, psize, &hash.hdr);
+	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+				     (const char *)sig, rc,
+				     hash.hdr.digest, hash.hdr.length);
+	if (rc) {
+		pr_err("integrity_digsig_verify() = %d\n", rc);
+		goto free_sig;
+	}
+
+	while (psize > 0 && (p = strsep(&datap, "\n"))) {
+		pr_debug("rule: %s\n", p);
+		rc = ima_parse_add_rule(p);
+		if (rc < 0)
+			break;
+		psize -= rc;
+	}
+free_sig:
+	kfree(sig);
+free_data:
+	kfree(data);
+free_path:
+	__putname(sigpath);
+	if (rc < 0)
+		return rc;
+	else if (psize)
+		return -EINVAL;
+	else
+		return pathlen;
+}
+
+#endif
-- 
1.8.3.2

[PATCH 16/20] ima: load policy from the kernel

[Dmitry Kasatkin]

This patch provide IMA policy loading from the kernel.
When CONFIG_IMA_KERNEL_POLICY is enabled, kernel tries
to load default /etc/ima_policy. Policy signature must
be located in /etc/ima_policy.sig.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/Kconfig    |  7 +++++++
 security/integrity/ima/ima.h      |  8 ++++++++
 security/integrity/ima/ima_fs.c   | 18 +++++++++++++++++-
 security/integrity/ima/ima_init.c |  1 +
 4 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index 465cef4..b00044f 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -153,3 +153,10 @@ config IMA_POLICY_LOADER
 
 	  Loading policy is like:
 	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
+
+config IMA_KERNEL_POLICY
+	bool "Load IMA policy from the kernel"
+	depends on IMA_POLICY_LOADER
+	default n
+	help
+	  This option enables IMA policy loading from the kernel.
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index f2722bb..3727cf7 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -179,6 +179,14 @@ static inline ssize_t ima_read_policy(char *data)
 }
 #endif
 
+#ifdef CONFIG_IMA_KERNEL_POLICY
+void ima_load_policy(char *path);
+#else
+static inline void ima_load_policy(char *path)
+{
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index bde7a0e..d050a5c 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -319,7 +319,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
  * point to the new policy rules, and remove the securityfs policy file,
  * assuming a valid policy.
  */
-static int ima_release_policy(struct inode *inode, struct file *file)
+static void ima_check_policy(void)
 {
 	if (!valid_policy) {
 		ima_delete_rules();
@@ -328,6 +328,11 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 		ima_update_policy();
 	}
 	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+}
+
+static int ima_release_policy(struct inode *inode, struct file *file)
+{
+	ima_check_policy();
 	return 0;
 }
 
@@ -338,6 +343,17 @@ static const struct file_operations ima_measure_policy_ops = {
 	.llseek = generic_file_llseek,
 };
 
+#ifdef CONFIG_IMA_KERNEL_POLICY
+void __init ima_load_policy(char *path)
+{
+	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+		return;
+	if (ima_read_policy(path) < 0)
+		valid_policy = 0;
+	ima_check_policy();
+}
+#endif
+
 int __init ima_fs_init(void)
 {
 	ima_dir = securityfs_create_dir("ima", NULL);
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index c13d6a8..d1a6483 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -109,6 +109,7 @@ int __init ima_init(void)
 	ima_init_policy();
 	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 	integrity_load_x509(INTEGRITY_KEYRING_IMA, "/etc/keys/x509_ima.der");
+	ima_load_policy("/etc/ima_policy");
 
 	return ima_fs_init();
 }
-- 
1.8.3.2

[PATCH 17/20] ima: make IMA policy replaceable at runtime

[Dmitry Kasatkin]

This patch provides functionality to replace the IMA policy at runtime.

By default, the IMA policy can be successfully updated only once,
but with this patch when the kernel configuration option
CONFIG_IMA_POLICY_REPLACEABLE is enabled, the IMA policy can be replaced
multiple times at runtime.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/ima/Kconfig      |  8 ++++++++
 security/integrity/ima/ima_fs.c     |  2 ++
 security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
 3 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index b00044f..b60a315 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
 	default n
 	help
 	  This option enables IMA policy loading from the kernel.
+
+config IMA_POLICY_REPLACEABLE
+	bool "Allows to replace policy at runtime"
+	depends on IMA_POLICY_LOADER
+	default n
+	help
+	  Enabling this option allows to replace policy at runtime.
+	  Only signed policy is allowed.
diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
index d050a5c..b4144b4 100644
--- a/security/integrity/ima/ima_fs.c
+++ b/security/integrity/ima/ima_fs.c
@@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return -EACCES;
 	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
 		return -EBUSY;
+#ifndef CONFIG_IMA_POLICY_REPLACEABLE
 	if (!ima_default_policy()) {
 		/* policy was already set*/
 		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 		return -EACCES;
 	}
+#endif
 	return 0;
 }
 
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index c6da801..981e953 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
 
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
+static LIST_HEAD(ima_active_rules);
 static struct list_head *ima_rules;
 static bool path_rules;
 
 static DEFINE_MUTEX(ima_rules_mutex);
 
+static void ima_do_delete_rules(struct list_head *rules);
+
 static bool ima_use_tcb __initdata;
 static int __init default_measure_policy_setup(char *str)
 {
@@ -367,7 +370,14 @@ void ima_update_policy(void)
 	int result = 0;
 	int audit_info = 0;
 
-	ima_rules = &ima_policy_rules;
+	if (ima_default_policy()) {
+		/* set new policy head */
+		ima_rules = &ima_active_rules;
+	} else {
+		/* FIXME: must be protected by lock */
+		ima_do_delete_rules(ima_rules);
+	}
+	list_replace_init(&ima_policy_rules, ima_rules);
 
 	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
 			    NULL, op, cause, result, audit_info);
@@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
 	return len;
 }
 
-/* ima_delete_rules called to cleanup invalid policy */
-void ima_delete_rules(void)
+/* ima_delete_rules called to cleanup invalid or old policy */
+static void ima_do_delete_rules(struct list_head *rules)
 {
 	struct ima_rule_entry *entry, *tmp;
 	int i;
 
 	mutex_lock(&ima_rules_mutex);
-	list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+	list_for_each_entry_safe(entry, tmp, rules, list) {
 		for (i = 0; i < MAX_LSM_RULES; i++)
 			kfree(entry->lsm[i].args_p);
 
@@ -751,6 +761,11 @@ void ima_delete_rules(void)
 	mutex_unlock(&ima_rules_mutex);
 }
 
+void ima_delete_rules(void)
+{
+	ima_do_delete_rules(&ima_policy_rules);
+}
+
 #ifdef CONFIG_IMA_POLICY_LOADER
 
 ssize_t ima_read_policy(char *path)
-- 
1.8.3.2

[PATCH 18/20] evm: added kernel parameter for disabling EVM

[Dmitry Kasatkin]

EVM remained to be disabled until evm-key is initialized via sysfs.
When the key is loaded from the kernel, EVM becomes enabled from start.
Distributions might want to compile EVM support, but leave for the user
to decide if to enable or disable EVM functionality.

This patch provides kernel parameter 'evm=off' that allows to disable EVM.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/evm.h       |  5 +++++
 security/integrity/evm/evm_main.c  | 19 +++++++++++++------
 security/integrity/evm/evm_secfs.c |  3 ++-
 3 files changed, 20 insertions(+), 7 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 88bfe77..1db78b8 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -21,6 +21,11 @@
 
 #include "../integrity.h"
 
+#define EVM_MODE_OFF	0
+#define EVM_MODE_ON	1
+#define EVM_MODE_FIX	2
+
+extern int evm_mode;
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 0f1b489..ad5e641 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -53,14 +53,17 @@ char *evm_config_xattrnames[] = {
 	NULL
 };
 
-static int evm_fixmode;
-static int __init evm_set_fixmode(char *str)
+int evm_mode = EVM_MODE_ON;
+
+static int __init evm_setup(char *str)
 {
-	if (strncmp(str, "fix", 3) == 0)
-		evm_fixmode = 1;
+	if (strncmp(str, "off", 3) == 0)
+		evm_mode = EVM_MODE_OFF;
+	else if (strncmp(str, "fix", 3) == 0)
+		evm_mode = EVM_MODE_FIX;
 	return 0;
 }
-__setup("evm=", evm_set_fixmode);
+__setup("evm=", evm_setup);
 
 static void __init evm_init_config(void)
 {
@@ -249,7 +252,8 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode)
+	if (!evm_initialized || !S_ISREG(inode->i_mode) ||
+		    evm_mode == EVM_MODE_FIX)
 		return 0;
 	return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
 }
@@ -445,6 +449,9 @@ static int __init init_evm(void)
 {
 	int error;
 
+	if (evm_mode == EVM_MODE_OFF)
+		return 0;
+
 	evm_init_config();
 
 	integrity_init_keyring(INTEGRITY_KEYRING_EVM);
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index cf12a04..4c81ef6 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -64,7 +64,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 	char temp[80];
 	int i, error;
 
-	if (!capable(CAP_SYS_ADMIN) || evm_initialized)
+	if (!capable(CAP_SYS_ADMIN) || evm_initialized ||
+		    evm_mode == EVM_MODE_OFF)
 		return -EPERM;
 
 	if (count >= sizeof(temp) || count == 0)
-- 
1.8.3.2

[PATCH 19/20] evm: try enable EVM from the kernel

[Dmitry Kasatkin]

EVM key might be initialzed in the kernel by kernel module
using HW specific way. For example such method would suite
devices with ARM Trust Zone technology.

This patch tries enable EVM by checking if evm-key already
exists in the kernel keyring.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/evm_crypto.c |  5 +++++
 security/integrity/evm/evm_main.c   |  2 ++
 security/integrity/evm/evm_secfs.c  | 10 +++-------
 3 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 5396769..f79ebf5 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -258,5 +258,10 @@ out:
 	memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
 	up_read(&evm_key->sem);
 	key_put(evm_key);
+	if (!rc) {
+		evm_initialized = 1;
+		pr_info("initialized\n");
+	} else
+		pr_err("initialization failed\n");
 	return rc;
 }
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index ad5e641..d2c06d3 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -463,6 +463,8 @@ static int __init init_evm(void)
 		goto err;
 	}
 
+	evm_init_key();
+
 	return 0;
 err:
 	return error;
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 4c81ef6..d7b5d11 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -62,7 +62,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 			     size_t count, loff_t *ppos)
 {
 	char temp[80];
-	int i, error;
+	int i;
 
 	if (!capable(CAP_SYS_ADMIN) || evm_initialized ||
 		    evm_mode == EVM_MODE_OFF)
@@ -79,12 +79,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 	if ((sscanf(temp, "%d", &i) != 1) || (i != 1))
 		return -EINVAL;
 
-	error = evm_init_key();
-	if (!error) {
-		evm_initialized = 1;
-		pr_info("initialized\n");
-	} else
-		pr_err("initialization failed\n");
+	evm_init_key();
+
 	return count;
 }
 
-- 
1.8.3.2

[PATCH 20/20] evm: read EVM key from the kernel

[Dmitry Kasatkin]

Currently EVM key needs to be added from the user space
and it has to be done before mounting filesystems.
It requires initramfs.
Many systems often does not want to use initramfs.

This patch provide support for loading EVM key from the kernel.

It supports both 'trusted' and 'user' master keys.
However, it is recommended to use 'trusted' master key,
because 'user' master key is in non-encrypted form.

Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
---
 security/integrity/evm/Kconfig      |  8 ++++
 security/integrity/evm/evm.h        |  9 ++++
 security/integrity/evm/evm_crypto.c | 96 +++++++++++++++++++++++++++++++++++++
 security/integrity/evm/evm_main.c   |  1 +
 4 files changed, 114 insertions(+)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index 70d12f7..953ef05 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -60,3 +60,11 @@ config EVM_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_evm' trusted keyring.
+
+config EVM_LOAD_KEY
+	bool "Load EVM HMAC key from the kernel"
+	depends on EVM
+	default n
+	help
+	   This option enables EVM HMAC key loading from the kernel.
+	   It enables EVM.
diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 1db78b8..1890eac 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -40,6 +40,15 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
+#ifdef CONFIG_EVM_LOAD_KEY
+int evm_load_key(const char *key, const char *kmk);
+#else
+static inline int evm_load_key(const char *key, const char *kmk)
+{
+	return 0;
+}
+#endif
+
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
 			const char *req_xattr_name,
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index f79ebf5..c45b71b 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -18,6 +18,8 @@
 #include <linux/module.h>
 #include <linux/crypto.h>
 #include <linux/xattr.h>
+#include <linux/sched.h>
+#include <linux/cred.h>
 #include <keys/encrypted-type.h>
 #include <crypto/hash.h>
 #include "evm.h"
@@ -265,3 +267,97 @@ out:
 		pr_err("initialization failed\n");
 	return rc;
 }
+
+#ifdef CONFIG_EVM_LOAD_KEY
+int evm_load_key(const char *key, const char *kmk)
+{
+	key_ref_t key_ref, keyring_ref;
+	char *data, *tdata = NULL, *cmd, *type, ch = '\0';
+	int rc, len;
+	bool trusted = false;
+
+	keyring_ref = make_key_ref(current_cred()->user->uid_keyring, 1);
+
+	len = integrity_read_file(key, &data);
+	if (len < 0)
+		return len;
+
+	swap(data[len - 1], ch);
+	if (strstr(data, "trusted"))
+		trusted = true;
+	swap(data[len - 1], ch);
+
+	rc = integrity_read_file(kmk, &tdata);
+	if (rc < 0)
+		goto out;
+
+	/* padd does not like \n - remove it*/
+	if (strchr(tdata, '\n'))
+		rc--;
+
+	if (trusted) {
+		/* we need 'load' keyword */
+		cmd = kmalloc(rc + 5, GFP_KERNEL);
+		if (!cmd)
+			goto out;
+
+		memcpy(cmd, "load ", 5);
+		memcpy(cmd + 5, tdata, rc);
+		rc += 5;
+	} else {
+		cmd = tdata;
+	}
+
+	key_ref = key_create_or_update(keyring_ref,
+					trusted ? "trusted" : "user", "kmk",
+					cmd, rc,
+					((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+					KEY_USR_VIEW | KEY_USR_READ),
+					KEY_ALLOC_NOT_IN_QUOTA);
+	if (trusted)
+		kfree(cmd);
+	type = trusted ? "trusted" : "user";
+	if (IS_ERR(key_ref)) {
+		rc = PTR_ERR(key_ref);
+		pr_err("problem loading EVM kmk (%s) (%d): %s\n",
+		       type, rc, kmk);
+		goto out;
+	} else {
+		pr_notice("loaded EVM kmk (%s) %d': %s\n",
+			  type, key_ref_to_ptr(key_ref)->serial, kmk);
+		key_ref_put(key_ref);
+	}
+
+	/* padd does not like \n - remove it*/
+	if (strchr(data, '\n'))
+		len--;
+
+	/* we need 'load' keyword */
+	cmd = kmalloc(len + 5, GFP_KERNEL);
+	if (!cmd)
+		goto out;
+
+	memcpy(cmd, "load ", 5);
+	memcpy(cmd + 5, data, len);
+
+	key_ref = key_create_or_update(keyring_ref,
+					"encrypted", EVMKEY, cmd, len + 5,
+					((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+					KEY_USR_VIEW | KEY_USR_READ),
+					KEY_ALLOC_NOT_IN_QUOTA);
+	kfree(cmd);
+	if (IS_ERR(key_ref)) {
+		rc = PTR_ERR(key_ref);
+		pr_err("problem loading EVM key (%d): %s\n", rc, key);
+	} else {
+		pr_notice("loaded EVM key %d': %s\n",
+			  key_ref_to_ptr(key_ref)->serial, key);
+		key_ref_put(key_ref);
+	}
+
+out:
+	kfree(tdata);
+	kfree(data);
+	return rc;
+}
+#endif
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index d2c06d3..4808596 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -463,6 +463,7 @@ static int __init init_evm(void)
 		goto err;
 	}
 
+	evm_load_key("/etc/keys/evm-key", "/etc/keys/kmk");
 	evm_init_key();
 
 	return 0;
-- 
1.8.3.2

Re: [PATCH 01/20] KEYS: verify a certificate is signed by a 'trusted' key

[Mimi Zohar]

On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> 
> Only public keys, with certificates signed by an existing
> 'trusted' key on the system trusted keyring, should be added
> to a trusted keyring.  This patch adds support for verifying
> a certificate's signature.
> 
> This is derived from David Howells pkcs7_request_asymmetric_key() patch.
> 
> Changes:
> - Flaged out the code to prevent build break if system keyring
>   is not enabled (Dmitry).

An updated version of this patch was posted, which resolves the Kconfig
issues.  There are a number of other issues which need to be addressed,
before this patch can be upstreamed.  Please refer to the patch
description for more details -
http://marc.info/?l=linux-security-module&m=138672063109662&w=2

Reminder, as per Documentation/SubmittingPatches: "#ifdefs are ugly",
please no ifdefs in C code.

thanks,

Mimi

> 
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Signed-off-by: David Howells <dhowells@redhat.com>
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
>  1 file changed, 84 insertions(+), 1 deletion(-)
> 
> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> index 382ef0d..d279f43 100644
> --- a/crypto/asymmetric_keys/x509_public_key.c
> +++ b/crypto/asymmetric_keys/x509_public_key.c
> @@ -18,6 +18,7 @@
>  #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 "asymmetric_keys.h"
>  #include "public_key.h"
> @@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
>  }
>  EXPORT_SYMBOL_GPL(x509_check_signature);
> 
> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> +/*
> + * Find a key in the given keyring by issuer and authority.
> + */
> +static struct key *x509_request_asymmetric_key(
> +	struct key *keyring,
> +	const char *signer, size_t signer_len,
> +	const char *authority, size_t auth_len)
> +{
> +	key_ref_t key;
> +	char *id;
> +
> +	/* Construct an identifier. */
> +	id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
> +	if (!id)
> +		return ERR_PTR(-ENOMEM);
> +
> +	memcpy(id, signer, signer_len);
> +	id[signer_len + 0] = ':';
> +	id[signer_len + 1] = ' ';
> +	memcpy(id + signer_len + 2, authority, auth_len);
> +	id[signer_len + 2 + auth_len] = 0;
> +
> +	pr_debug("Look up: \"%s\"\n", id);
> +
> +	key = keyring_search(make_key_ref(keyring, 1),
> +			     &key_type_asymmetric, id);
> +	if (IS_ERR(key))
> +		pr_debug("Request for module key '%s' err %ld\n",
> +			 id, PTR_ERR(key));
> +	kfree(id);
> +
> +	if (IS_ERR(key)) {
> +		switch (PTR_ERR(key)) {
> +			/* Hide some search errors */
> +		case -EACCES:
> +		case -ENOTDIR:
> +		case -EAGAIN:
> +			return ERR_PTR(-ENOKEY);
> +		default:
> +			return ERR_CAST(key);
> +		}
> +	}
> +
> +	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
> +	return key_ref_to_ptr(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.
> + */
> +static int x509_validate_trust(struct x509_certificate *cert,
> +			       struct key *trust_keyring)
> +{
> +	const struct public_key *pk;
> +	struct key *key;
> +	int ret = 1;
> +
> +	key = x509_request_asymmetric_key(trust_keyring,
> +					  cert->issuer, strlen(cert->issuer),
> +					  cert->authority,
> +					  strlen(cert->authority));
> +	if (!IS_ERR(key))  {
> +		pk = key->payload.data;
> +		ret = x509_check_signature(pk, cert);
> +	}
> +	return ret;
> +}
> +#endif
> +
>  /*
>   * Attempt to parse a data blob for a key as an X509 certificate.
>   */
> @@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>  	/* Check the signature on the key if it appears to be self-signed */
>  	if (!cert->authority ||
>  	    strcmp(cert->fingerprint, cert->authority) == 0) {
> -		ret = x509_check_signature(cert->pub, cert);
> +		ret = x509_check_signature(cert->pub, cert); /* self-signed */
>  		if (ret < 0)
>  			goto error_free_cert;
> +	} else {
> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> +		ret = x509_validate_trust(cert, system_trusted_keyring);
> +		if (!ret)
> +			prep->trusted = 1;
> +#endif
>  	}
> 
>  	/* Propose a description */

Re: [PATCH 00/20] in-kernel IMA/EVM initialization

[Mimi Zohar]

On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
> Hi,
> 
> Currently secure IMA/EVM initialization has to be done from the
> initramfs, embedded in the signed kernel image. Many systems do
> not want to use initramfs or use unsigned locally generated images.
> 
> This patchset introduces kernel functionality, which allows to perform
> IMA/EVM initialization without initramfs from the kernel, which
> includes mainly following:
> - loading EVM hmac encrypted keys
> - loading and verification of signed X509 certificates
> - loading and verification of signed IMA policy
> 
> Patchset introduces the set of new kernel configuration options,
> which makes this functionality entirely configurable.
> Not enabling any of the options does not change original IMA/EVM
> behavior. In order not to bloat security configuration menu,
> integrity subsystem options were moved to the separate menu.
> It does not affect existing configuration. Re-configuration is
> not needed.

Loading the IMA/EVM keys onto their respective keyrings by the kernel,
as early as possible, is a good idea, but unfortunately, at least in the
past, having the kernel open files (eg. configuration, policies, kernel
module public key) has not been permitted.  LSM policies were initially
loaded by the initramfs, but more recently by dracut.  As for the kernel
module public key, the key is built into the kernel and loaded onto the
system keyring.  I'm not aware that this limitation of opening files
from the kernel has been removed.

thanks,

Mimi

> 
> Dmitry Kasatkin (19):
>   integrity: initialize EVM before IMA
>   ima: move asymmetric keys config option
>   integrity: move integrity subsystem options to a separate menu
>   integrity: provide builtin 'trusted' keyrings
>   ima: create '_ima' as a builtin 'trusted' keyring
>   integrity: provide x509 certificate loading from the kernel
>   ima: load x509 certificate from the kernel
>   evm: create '_evm' as a builtin 'trusted' keyring
>   evm: load x509 certificate from the kernel
>   ima: added kernel parameter for disabling IMA
>   ima: provide buffer hash calculation function
>   ima: replace opencount with bitop
>   ima: check if policy was set at open
>   ima: path based policy loading interface
>   ima: load policy from the kernel
>   ima: make IMA policy replaceable at runtime
>   evm: added kernel parameter for disabling EVM
>   evm: try enable EVM from the kernel
>   evm: read EVM key from the kernel
> 
> Mimi Zohar (1):
>   KEYS: verify a certificate is signed by a 'trusted' key
> 
>  crypto/asymmetric_keys/x509_public_key.c |  85 +++++++++++++++++++++++-
>  security/integrity/Kconfig               |  41 ++++++++----
>  security/integrity/Makefile              |   4 +-
>  security/integrity/digsig.c              | 103 +++++++++++++++++++++++++++++
>  security/integrity/evm/Kconfig           |  32 +++++++--
>  security/integrity/evm/evm.h             |  14 ++++
>  security/integrity/evm/evm_crypto.c      | 101 ++++++++++++++++++++++++++++
>  security/integrity/evm/evm_main.c        |  25 +++++--
>  security/integrity/evm/evm_secfs.c       |  13 ++--
>  security/integrity/ima/Kconfig           |  49 +++++++++++++-
>  security/integrity/ima/ima.h             |  19 ++++++
>  security/integrity/ima/ima_crypto.c      |  11 +++-
>  security/integrity/ima/ima_fs.c          |  48 ++++++++++----
>  security/integrity/ima/ima_init.c        |   3 +
>  security/integrity/ima/ima_main.c        |  12 +++-
>  security/integrity/ima/ima_policy.c      | 109 ++++++++++++++++++++++++++++---
>  security/integrity/integrity.h           |  20 ++++++
>  17 files changed, 626 insertions(+), 63 deletions(-)
> 

Re: [PATCH 01/20] KEYS: verify a certificate is signed by a 'trusted' key

[Dmitry Kasatkin]

On 24 April 2014 19:53, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote:
>> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
>>
>> Only public keys, with certificates signed by an existing
>> 'trusted' key on the system trusted keyring, should be added
>> to a trusted keyring.  This patch adds support for verifying
>> a certificate's signature.
>>
>> This is derived from David Howells pkcs7_request_asymmetric_key() patch.
>>
>> Changes:
>> - Flaged out the code to prevent build break if system keyring
>>   is not enabled (Dmitry).
>
> An updated version of this patch was posted, which resolves the Kconfig
> issues.  There are a number of other issues which need to be addressed,
> before this patch can be upstreamed.  Please refer to the patch
> description for more details -
> http://marc.info/?l=linux-security-module&m=138672063109662&w=2
>

Oh. I was using this patch from your public tree..
Updated version is missing there and I missed it out.
Will rebase on the top of it as soon as it is available.


> Reminder, as per Documentation/SubmittingPatches: "#ifdefs are ugly",
> please no ifdefs in C code.
>

Right, we know it.

Making separate C file for one function isn't ugly?

- Dmitry

> thanks,
>
> Mimi
>
>>
>> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
>> Signed-off-by: David Howells <dhowells@redhat.com>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
>>  1 file changed, 84 insertions(+), 1 deletion(-)
>>
>> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
>> index 382ef0d..d279f43 100644
>> --- a/crypto/asymmetric_keys/x509_public_key.c
>> +++ b/crypto/asymmetric_keys/x509_public_key.c
>> @@ -18,6 +18,7 @@
>>  #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 "asymmetric_keys.h"
>>  #include "public_key.h"
>> @@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
>>  }
>>  EXPORT_SYMBOL_GPL(x509_check_signature);
>>
>> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
>> +/*
>> + * Find a key in the given keyring by issuer and authority.
>> + */
>> +static struct key *x509_request_asymmetric_key(
>> +     struct key *keyring,
>> +     const char *signer, size_t signer_len,
>> +     const char *authority, size_t auth_len)
>> +{
>> +     key_ref_t key;
>> +     char *id;
>> +
>> +     /* Construct an identifier. */
>> +     id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
>> +     if (!id)
>> +             return ERR_PTR(-ENOMEM);
>> +
>> +     memcpy(id, signer, signer_len);
>> +     id[signer_len + 0] = ':';
>> +     id[signer_len + 1] = ' ';
>> +     memcpy(id + signer_len + 2, authority, auth_len);
>> +     id[signer_len + 2 + auth_len] = 0;
>> +
>> +     pr_debug("Look up: \"%s\"\n", id);
>> +
>> +     key = keyring_search(make_key_ref(keyring, 1),
>> +                          &key_type_asymmetric, id);
>> +     if (IS_ERR(key))
>> +             pr_debug("Request for module key '%s' err %ld\n",
>> +                      id, PTR_ERR(key));
>> +     kfree(id);
>> +
>> +     if (IS_ERR(key)) {
>> +             switch (PTR_ERR(key)) {
>> +                     /* Hide some search errors */
>> +             case -EACCES:
>> +             case -ENOTDIR:
>> +             case -EAGAIN:
>> +                     return ERR_PTR(-ENOKEY);
>> +             default:
>> +                     return ERR_CAST(key);
>> +             }
>> +     }
>> +
>> +     pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
>> +     return key_ref_to_ptr(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.
>> + */
>> +static int x509_validate_trust(struct x509_certificate *cert,
>> +                            struct key *trust_keyring)
>> +{
>> +     const struct public_key *pk;
>> +     struct key *key;
>> +     int ret = 1;
>> +
>> +     key = x509_request_asymmetric_key(trust_keyring,
>> +                                       cert->issuer, strlen(cert->issuer),
>> +                                       cert->authority,
>> +                                       strlen(cert->authority));
>> +     if (!IS_ERR(key))  {
>> +             pk = key->payload.data;
>> +             ret = x509_check_signature(pk, cert);
>> +     }
>> +     return ret;
>> +}
>> +#endif
>> +
>>  /*
>>   * Attempt to parse a data blob for a key as an X509 certificate.
>>   */
>> @@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
>>       /* Check the signature on the key if it appears to be self-signed */
>>       if (!cert->authority ||
>>           strcmp(cert->fingerprint, cert->authority) == 0) {
>> -             ret = x509_check_signature(cert->pub, cert);
>> +             ret = x509_check_signature(cert->pub, cert); /* self-signed */
>>               if (ret < 0)
>>                       goto error_free_cert;
>> +     } else {
>> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
>> +             ret = x509_validate_trust(cert, system_trusted_keyring);
>> +             if (!ret)
>> +                     prep->trusted = 1;
>> +#endif
>>       }
>>
>>       /* Propose a description */
>
>
> --
> 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



-- 
Thanks,
Dmitry

Re: [PATCH 01/20] KEYS: verify a certificate is signed by a 'trusted' key

[Mimi Zohar]

On Thu, 2014-04-24 at 23:07 +0300, Dmitry Kasatkin wrote: 
> On 24 April 2014 19:53, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote:
> >> From: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >>
> >> Only public keys, with certificates signed by an existing
> >> 'trusted' key on the system trusted keyring, should be added
> >> to a trusted keyring.  This patch adds support for verifying
> >> a certificate's signature.
> >>
> >> This is derived from David Howells pkcs7_request_asymmetric_key() patch.
> >>
> >> Changes:
> >> - Flaged out the code to prevent build break if system keyring
> >>   is not enabled (Dmitry).
> >
> > An updated version of this patch was posted, which resolves the Kconfig
> > issues.  There are a number of other issues which need to be addressed,
> > before this patch can be upstreamed.  Please refer to the patch
> > description for more details -
> > http://marc.info/?l=linux-security-module&m=138672063109662&w=2
> >
> 
> Oh. I was using this patch from your public tree..
> Updated version is missing there and I missed it out.
> Will rebase on the top of it as soon as it is available.

Ok. The patch set was posted as an RFC, but didn't receive any review.
The issues still need to be resolved (eg. associating a specific public
keyring to verify a new key being added to the trusted keyring,
userspace being able to replace a trusted keyring) before it can be
upstreamed.

While you're rebasing this patch set, please consider breaking it up
into smaller, logical groups.

[- trusted keyring support (me)]
- Kconfig cleanup
- kernel loading x509 keys
- kernel IMA policy update

thanks,

Mimi


> > Reminder, as per Documentation/SubmittingPatches: "#ifdefs are ugly",
> > please no ifdefs in C code.
> >
> 
> Right, we know it.
> 
> Making separate C file for one function isn't ugly?
> 
> - Dmitry
> 
> > thanks,
> >
> > Mimi
> >
> >>
> >> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> >> Signed-off-by: David Howells <dhowells@redhat.com>
> >> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> >> ---
> >>  crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
> >>  1 file changed, 84 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/crypto/asymmetric_keys/x509_public_key.c b/crypto/asymmetric_keys/x509_public_key.c
> >> index 382ef0d..d279f43 100644
> >> --- a/crypto/asymmetric_keys/x509_public_key.c
> >> +++ b/crypto/asymmetric_keys/x509_public_key.c
> >> @@ -18,6 +18,7 @@
> >>  #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 "asymmetric_keys.h"
> >>  #include "public_key.h"
> >> @@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
> >>  }
> >>  EXPORT_SYMBOL_GPL(x509_check_signature);
> >>
> >> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> >> +/*
> >> + * Find a key in the given keyring by issuer and authority.
> >> + */
> >> +static struct key *x509_request_asymmetric_key(
> >> +     struct key *keyring,
> >> +     const char *signer, size_t signer_len,
> >> +     const char *authority, size_t auth_len)
> >> +{
> >> +     key_ref_t key;
> >> +     char *id;
> >> +
> >> +     /* Construct an identifier. */
> >> +     id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
> >> +     if (!id)
> >> +             return ERR_PTR(-ENOMEM);
> >> +
> >> +     memcpy(id, signer, signer_len);
> >> +     id[signer_len + 0] = ':';
> >> +     id[signer_len + 1] = ' ';
> >> +     memcpy(id + signer_len + 2, authority, auth_len);
> >> +     id[signer_len + 2 + auth_len] = 0;
> >> +
> >> +     pr_debug("Look up: \"%s\"\n", id);
> >> +
> >> +     key = keyring_search(make_key_ref(keyring, 1),
> >> +                          &key_type_asymmetric, id);
> >> +     if (IS_ERR(key))
> >> +             pr_debug("Request for module key '%s' err %ld\n",
> >> +                      id, PTR_ERR(key));
> >> +     kfree(id);
> >> +
> >> +     if (IS_ERR(key)) {
> >> +             switch (PTR_ERR(key)) {
> >> +                     /* Hide some search errors */
> >> +             case -EACCES:
> >> +             case -ENOTDIR:
> >> +             case -EAGAIN:
> >> +                     return ERR_PTR(-ENOKEY);
> >> +             default:
> >> +                     return ERR_CAST(key);
> >> +             }
> >> +     }
> >> +
> >> +     pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
> >> +     return key_ref_to_ptr(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.
> >> + */
> >> +static int x509_validate_trust(struct x509_certificate *cert,
> >> +                            struct key *trust_keyring)
> >> +{
> >> +     const struct public_key *pk;
> >> +     struct key *key;
> >> +     int ret = 1;
> >> +
> >> +     key = x509_request_asymmetric_key(trust_keyring,
> >> +                                       cert->issuer, strlen(cert->issuer),
> >> +                                       cert->authority,
> >> +                                       strlen(cert->authority));
> >> +     if (!IS_ERR(key))  {
> >> +             pk = key->payload.data;
> >> +             ret = x509_check_signature(pk, cert);
> >> +     }
> >> +     return ret;
> >> +}
> >> +#endif
> >> +
> >>  /*
> >>   * Attempt to parse a data blob for a key as an X509 certificate.
> >>   */
> >> @@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
> >>       /* Check the signature on the key if it appears to be self-signed */
> >>       if (!cert->authority ||
> >>           strcmp(cert->fingerprint, cert->authority) == 0) {
> >> -             ret = x509_check_signature(cert->pub, cert);
> >> +             ret = x509_check_signature(cert->pub, cert); /* self-signed */
> >>               if (ret < 0)
> >>                       goto error_free_cert;
> >> +     } else {
> >> +#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
> >> +             ret = x509_validate_trust(cert, system_trusted_keyring);
> >> +             if (!ret)
> >> +                     prep->trusted = 1;
> >> +#endif
> >>       }
> >>
> >>       /* Propose a description */
> >
> >
> > --
> > 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
> 
> 
> 

Re: [PATCH 15/20] ima: path based policy loading interface

[Mimi Zohar]

On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
> Currently policy is loaded by writing policy content to
> '<securityfs>/ima/policy' file.
> 
> This patch extends policy loading meachanism with possibility
> to load signed policy using a path to the policy.
> Policy signature must be available in the <policy>.sig file.

Assuming (big assumption) you're permitted to open the policy file from
the kernel, why are you verifying the signature inline based on a .sig?
Shouldn't this be a new integrity/security hook?

thanks,

Mimi

> 
> Policy can be loaded like:
> echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> ---
>  security/integrity/ima/Kconfig      | 13 +++++++
>  security/integrity/ima/ima.h        |  9 +++++
>  security/integrity/ima/ima_fs.c     |  2 +-
>  security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
>  4 files changed, 97 insertions(+), 1 deletion(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index 5474c47..465cef4 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -140,3 +140,16 @@ config IMA_LOAD_X509
>  	help
>  	   This option enables X509 certificate loading from the kernel
>  	   to the '_ima' trusted keyring.
> +
> +config IMA_POLICY_LOADER
> +	bool "Path based policy loading interface"
> +	depends on IMA_TRUSTED_KEYRING
> +	default n
> +	help
> +	  This option enables path based signed policy loading interface.
> +	  Policy signature must be provided in the <policy>.sig file
> +	  along with the policy. When this option is enabled, kernel
> +	  tries to load default policy from /etc/ima_policy.
> +
> +	  Loading policy is like:
> +	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index 3b90b60..f2722bb 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -170,6 +170,15 @@ bool ima_default_policy(void);
>  ssize_t ima_parse_add_rule(char *);
>  void ima_delete_rules(void);
> 
> +#ifdef CONFIG_IMA_POLICY_LOADER
> +ssize_t ima_read_policy(char *path);
> +#else
> +static inline ssize_t ima_read_policy(char *data)
> +{
> +	return ima_parse_add_rule(data);
> +}
> +#endif
> +
>  /* Appraise integrity measurements */
>  #define IMA_APPRAISE_ENFORCE	0x01
>  #define IMA_APPRAISE_FIX	0x02
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index 34ae5f2..bde7a0e 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
>  	if (copy_from_user(data, buf, datalen))
>  		goto out;
> 
> -	result = ima_parse_add_rule(data);
> +	result = ima_read_policy(data);
>  out:
>  	if (result < 0)
>  		valid_policy = 0;
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index b24e7d1..c6da801 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -17,6 +17,9 @@
>  #include <linux/parser.h>
>  #include <linux/slab.h>
>  #include <linux/genhd.h>
> +#ifdef CONFIG_IMA_POLICY_LOADER
> +#include <linux/file.h>
> +#endif
> 
>  #include "ima.h"
> 
> @@ -747,3 +750,74 @@ void ima_delete_rules(void)
>  	}
>  	mutex_unlock(&ima_rules_mutex);
>  }
> +
> +#ifdef CONFIG_IMA_POLICY_LOADER
> +
> +ssize_t ima_read_policy(char *path)
> +{
> +	char *data, *datap, *sig;
> +	int rc, psize, pathlen = strlen(path);
> +	char *p, *sigpath;
> +	struct {
> +		struct ima_digest_data hdr;
> +		char digest[IMA_MAX_DIGEST_SIZE];
> +	} hash;
> +
> +	if (path[0] != '/')
> +		return ima_parse_add_rule(path);
> +
> +	/* remove \n */
> +	datap = path;
> +	strsep(&datap, "\n");
> +
> +	/* we always want signature? */
> +	sigpath = __getname();
> +	if (!sigpath)
> +		return -ENOMEM;
> +
> +	rc = integrity_read_file(path, &data);
> +	if (rc < 0)
> +		goto free_path;
> +
> +	psize = rc;
> +	datap = data;
> +
> +	sprintf(sigpath, "%s.sig", path);
> +	/* we always want signature? */
> +	rc = integrity_read_file(sigpath, &sig);
> +	if (rc < 0)
> +		goto free_data;
> +
> +	hash.hdr.algo = ima_hash_algo;
> +	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
> +	ima_calc_buffer_hash(data, psize, &hash.hdr);
> +	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
> +				     (const char *)sig, rc,
> +				     hash.hdr.digest, hash.hdr.length);
> +	if (rc) {
> +		pr_err("integrity_digsig_verify() = %d\n", rc);
> +		goto free_sig;
> +	}
> +
> +	while (psize > 0 && (p = strsep(&datap, "\n"))) {
> +		pr_debug("rule: %s\n", p);
> +		rc = ima_parse_add_rule(p);
> +		if (rc < 0)
> +			break;
> +		psize -= rc;
> +	}
> +free_sig:
> +	kfree(sig);
> +free_data:
> +	kfree(data);
> +free_path:
> +	__putname(sigpath);
> +	if (rc < 0)
> +		return rc;
> +	else if (psize)
> +		return -EINVAL;
> +	else
> +		return pathlen;
> +}
> +
> +#endif

Re: [PATCH 12/20] ima: provide buffer hash calculation function

[Mimi Zohar]

On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
> This patch provides convenient buffer hash calculation function.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

Where/how is it being used?  We normally don't upstream a new function
without it being used.  Is the usage in another patch?

Mimi

> ---
>  security/integrity/ima/ima.h        |  1 +
>  security/integrity/ima/ima_crypto.c | 11 +++++++++--
>  2 files changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index f4c1e8dd..a5d5ccb 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>  			   const char *op, struct inode *inode,
>  			   const unsigned char *filename);
>  int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
> +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
>  int ima_calc_field_array_hash(struct ima_field_data *field_data,
>  			      struct ima_template_desc *desc, int num_fields,
>  			      struct ima_digest_data *hash);
> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
> index 139e7f7..50c78c0 100644
> --- a/security/integrity/ima/ima_crypto.c
> +++ b/security/integrity/ima/ima_crypto.c
> @@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
>  		u8 *data_to_hash = field_data[i].data;
>  		u32 datalen = field_data[i].len;
> 
> -		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
> +		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
>  			rc = crypto_shash_update(&desc.shash,
>  						(const u8 *) &field_data[i].len,
>  						sizeof(field_data[i].len));
>  			if (rc)
>  				break;
> -		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
> +		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
>  			memcpy(buffer, data_to_hash, datalen);
>  			data_to_hash = buffer;
>  			datalen = IMA_EVENT_NAME_LEN_MAX + 1;
> @@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
>  	return rc;
>  }
> 
> +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
> +{
> +	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
> +
> +	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
> +}
> +
>  static void __init ima_pcrread(int idx, u8 *pcr)
>  {
>  	if (!ima_used_chip)

Re: [PATCH 12/20] ima: provide buffer hash calculation function

[Dmitry Kasatkin]

On 25/04/14 00:04, Mimi Zohar wrote:
> On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
>> This patch provides convenient buffer hash calculation function.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
> Where/how is it being used?  We normally don't upstream a new function
> without it being used.  Is the usage in another patch?
>
> Mimi

Sure.. it is used in PATCH 15.

>> ---
>>  security/integrity/ima/ima.h        |  1 +
>>  security/integrity/ima/ima_crypto.c | 11 +++++++++--
>>  2 files changed, 10 insertions(+), 2 deletions(-)
>>
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index f4c1e8dd..a5d5ccb 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
>>  			   const char *op, struct inode *inode,
>>  			   const unsigned char *filename);
>>  int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
>> +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
>>  int ima_calc_field_array_hash(struct ima_field_data *field_data,
>>  			      struct ima_template_desc *desc, int num_fields,
>>  			      struct ima_digest_data *hash);
>> diff --git a/security/integrity/ima/ima_crypto.c b/security/integrity/ima/ima_crypto.c
>> index 139e7f7..50c78c0 100644
>> --- a/security/integrity/ima/ima_crypto.c
>> +++ b/security/integrity/ima/ima_crypto.c
>> @@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
>>  		u8 *data_to_hash = field_data[i].data;
>>  		u32 datalen = field_data[i].len;
>>
>> -		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
>> +		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
>>  			rc = crypto_shash_update(&desc.shash,
>>  						(const u8 *) &field_data[i].len,
>>  						sizeof(field_data[i].len));
>>  			if (rc)
>>  				break;
>> -		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
>> +		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
>>  			memcpy(buffer, data_to_hash, datalen);
>>  			data_to_hash = buffer;
>>  			datalen = IMA_EVENT_NAME_LEN_MAX + 1;
>> @@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
>>  	return rc;
>>  }
>>
>> +int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
>> +{
>> +	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
>> +
>> +	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
>> +}
>> +
>>  static void __init ima_pcrread(int idx, u8 *pcr)
>>  {
>>  	if (!ima_used_chip)
>
>

Re: [PATCH 15/20] ima: path based policy loading interface

[Dmitry Kasatkin]

On 25/04/14 00:03, Mimi Zohar wrote:
> On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
>> Currently policy is loaded by writing policy content to
>> '<securityfs>/ima/policy' file.
>>
>> This patch extends policy loading meachanism with possibility
>> to load signed policy using a path to the policy.
>> Policy signature must be available in the <policy>.sig file.
> Assuming (big assumption) you're permitted to open the policy file from
> the kernel, why are you verifying the signature inline based on a .sig?
> Shouldn't this be a new integrity/security hook?

What kind of hook do you mean?

Actually I was considering 2 approaches.

1. Introduce additional IMA function similar to ima_module_check() and
then retrieve verification status OK+sig
That would be using normal measurement code.
This requires anyway to open file from the kernel.
But it would be a bit tricky to move policy and signature together and
does not work with auto-generated initramfs
images on the distros as they do not have xattrs...

2. Read policy to the buffer as it is needed anyway and then just verify
the signature.
This was just simple enough as initial thought to implement.
Easy to copy policy and works with initramfs.


- Dmitry

> thanks,
>
> Mimi
>
>> Policy can be loaded like:
>> echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>> ---
>>  security/integrity/ima/Kconfig      | 13 +++++++
>>  security/integrity/ima/ima.h        |  9 +++++
>>  security/integrity/ima/ima_fs.c     |  2 +-
>>  security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
>>  4 files changed, 97 insertions(+), 1 deletion(-)
>>
>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>> index 5474c47..465cef4 100644
>> --- a/security/integrity/ima/Kconfig
>> +++ b/security/integrity/ima/Kconfig
>> @@ -140,3 +140,16 @@ config IMA_LOAD_X509
>>  	help
>>  	   This option enables X509 certificate loading from the kernel
>>  	   to the '_ima' trusted keyring.
>> +
>> +config IMA_POLICY_LOADER
>> +	bool "Path based policy loading interface"
>> +	depends on IMA_TRUSTED_KEYRING
>> +	default n
>> +	help
>> +	  This option enables path based signed policy loading interface.
>> +	  Policy signature must be provided in the <policy>.sig file
>> +	  along with the policy. When this option is enabled, kernel
>> +	  tries to load default policy from /etc/ima_policy.
>> +
>> +	  Loading policy is like:
>> +	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
>> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
>> index 3b90b60..f2722bb 100644
>> --- a/security/integrity/ima/ima.h
>> +++ b/security/integrity/ima/ima.h
>> @@ -170,6 +170,15 @@ bool ima_default_policy(void);
>>  ssize_t ima_parse_add_rule(char *);
>>  void ima_delete_rules(void);
>>
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +ssize_t ima_read_policy(char *path);
>> +#else
>> +static inline ssize_t ima_read_policy(char *data)
>> +{
>> +	return ima_parse_add_rule(data);
>> +}
>> +#endif
>> +
>>  /* Appraise integrity measurements */
>>  #define IMA_APPRAISE_ENFORCE	0x01
>>  #define IMA_APPRAISE_FIX	0x02
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index 34ae5f2..bde7a0e 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
>>  	if (copy_from_user(data, buf, datalen))
>>  		goto out;
>>
>> -	result = ima_parse_add_rule(data);
>> +	result = ima_read_policy(data);
>>  out:
>>  	if (result < 0)
>>  		valid_policy = 0;
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index b24e7d1..c6da801 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -17,6 +17,9 @@
>>  #include <linux/parser.h>
>>  #include <linux/slab.h>
>>  #include <linux/genhd.h>
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +#include <linux/file.h>
>> +#endif
>>
>>  #include "ima.h"
>>
>> @@ -747,3 +750,74 @@ void ima_delete_rules(void)
>>  	}
>>  	mutex_unlock(&ima_rules_mutex);
>>  }
>> +
>> +#ifdef CONFIG_IMA_POLICY_LOADER
>> +
>> +ssize_t ima_read_policy(char *path)
>> +{
>> +	char *data, *datap, *sig;
>> +	int rc, psize, pathlen = strlen(path);
>> +	char *p, *sigpath;
>> +	struct {
>> +		struct ima_digest_data hdr;
>> +		char digest[IMA_MAX_DIGEST_SIZE];
>> +	} hash;
>> +
>> +	if (path[0] != '/')
>> +		return ima_parse_add_rule(path);
>> +
>> +	/* remove \n */
>> +	datap = path;
>> +	strsep(&datap, "\n");
>> +
>> +	/* we always want signature? */
>> +	sigpath = __getname();
>> +	if (!sigpath)
>> +		return -ENOMEM;
>> +
>> +	rc = integrity_read_file(path, &data);
>> +	if (rc < 0)
>> +		goto free_path;
>> +
>> +	psize = rc;
>> +	datap = data;
>> +
>> +	sprintf(sigpath, "%s.sig", path);
>> +	/* we always want signature? */
>> +	rc = integrity_read_file(sigpath, &sig);
>> +	if (rc < 0)
>> +		goto free_data;
>> +
>> +	hash.hdr.algo = ima_hash_algo;
>> +	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
>> +	ima_calc_buffer_hash(data, psize, &hash.hdr);
>> +	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
>> +				     (const char *)sig, rc,
>> +				     hash.hdr.digest, hash.hdr.length);
>> +	if (rc) {
>> +		pr_err("integrity_digsig_verify() = %d\n", rc);
>> +		goto free_sig;
>> +	}
>> +
>> +	while (psize > 0 && (p = strsep(&datap, "\n"))) {
>> +		pr_debug("rule: %s\n", p);
>> +		rc = ima_parse_add_rule(p);
>> +		if (rc < 0)
>> +			break;
>> +		psize -= rc;
>> +	}
>> +free_sig:
>> +	kfree(sig);
>> +free_data:
>> +	kfree(data);
>> +free_path:
>> +	__putname(sigpath);
>> +	if (rc < 0)
>> +		return rc;
>> +	else if (psize)
>> +		return -EINVAL;
>> +	else
>> +		return pathlen;
>> +}
>> +
>> +#endif
>
>

Re: [PATCH 17/20] ima: make IMA policy replaceable at runtime

[Mimi Zohar]

On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote: 
> This patch provides functionality to replace the IMA policy at runtime.
> 
> By default, the IMA policy can be successfully updated only once,
> but with this patch when the kernel configuration option
> CONFIG_IMA_POLICY_REPLACEABLE is enabled, the IMA policy can be replaced
> multiple times at runtime.
> 
> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>

I have a couple of concerns with replacing the IMA policy.

- Currently opened files might now be in policy, that previously
weren't.  Do these files need to be measured, appraised, or audited?
These files could have been modified, but the 'security.ima' xattr
hasn't been updated yet. In such cases, appraisal would fail.

- At minimum, after replacing the policy, the iint cache entry flags
need to be reset.

Please provide the motivation for such a use case scenario.

Mimi

> ---
>  security/integrity/ima/Kconfig      |  8 ++++++++
>  security/integrity/ima/ima_fs.c     |  2 ++
>  security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
>  3 files changed, 29 insertions(+), 4 deletions(-)
> 
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index b00044f..b60a315 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
>  	default n
>  	help
>  	  This option enables IMA policy loading from the kernel.
> +
> +config IMA_POLICY_REPLACEABLE
> +	bool "Allows to replace policy at runtime"
> +	depends on IMA_POLICY_LOADER
> +	default n
> +	help
> +	  Enabling this option allows to replace policy at runtime.
> +	  Only signed policy is allowed.
> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
> index d050a5c..b4144b4 100644
> --- a/security/integrity/ima/ima_fs.c
> +++ b/security/integrity/ima/ima_fs.c
> @@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
>  		return -EACCES;
>  	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
>  		return -EBUSY;
> +#ifndef CONFIG_IMA_POLICY_REPLACEABLE
>  	if (!ima_default_policy()) {
>  		/* policy was already set*/
>  		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
>  		return -EACCES;
>  	}
> +#endif
>  	return 0;
>  }
> 
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index c6da801..981e953 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
> 
>  static LIST_HEAD(ima_default_rules);
>  static LIST_HEAD(ima_policy_rules);
> +static LIST_HEAD(ima_active_rules);
>  static struct list_head *ima_rules;
>  static bool path_rules;
> 
>  static DEFINE_MUTEX(ima_rules_mutex);
> 
> +static void ima_do_delete_rules(struct list_head *rules);
> +
>  static bool ima_use_tcb __initdata;
>  static int __init default_measure_policy_setup(char *str)
>  {
> @@ -367,7 +370,14 @@ void ima_update_policy(void)
>  	int result = 0;
>  	int audit_info = 0;
> 
> -	ima_rules = &ima_policy_rules;
> +	if (ima_default_policy()) {
> +		/* set new policy head */
> +		ima_rules = &ima_active_rules;
> +	} else {
> +		/* FIXME: must be protected by lock */
> +		ima_do_delete_rules(ima_rules);
> +	}
> +	list_replace_init(&ima_policy_rules, ima_rules);
> 
>  	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
>  			    NULL, op, cause, result, audit_info);
> @@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
>  	return len;
>  }
> 
> -/* ima_delete_rules called to cleanup invalid policy */
> -void ima_delete_rules(void)
> +/* ima_delete_rules called to cleanup invalid or old policy */
> +static void ima_do_delete_rules(struct list_head *rules)
>  {
>  	struct ima_rule_entry *entry, *tmp;
>  	int i;
> 
>  	mutex_lock(&ima_rules_mutex);
> -	list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
> +	list_for_each_entry_safe(entry, tmp, rules, list) {
>  		for (i = 0; i < MAX_LSM_RULES; i++)
>  			kfree(entry->lsm[i].args_p);
> 
> @@ -751,6 +761,11 @@ void ima_delete_rules(void)
>  	mutex_unlock(&ima_rules_mutex);
>  }
> 
> +void ima_delete_rules(void)
> +{
> +	ima_do_delete_rules(&ima_policy_rules);
> +}
> +
>  #ifdef CONFIG_IMA_POLICY_LOADER
> 
>  ssize_t ima_read_policy(char *path)

Re: [PATCH 17/20] ima: make IMA policy replaceable at runtime

[Dmitry Kasatkin]

On 15 May 2014 02:45, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2014-04-23 at 16:30 +0300, Dmitry Kasatkin wrote:
>> This patch provides functionality to replace the IMA policy at runtime.
>>
>> By default, the IMA policy can be successfully updated only once,
>> but with this patch when the kernel configuration option
>> CONFIG_IMA_POLICY_REPLACEABLE is enabled, the IMA policy can be replaced
>> multiple times at runtime.
>>
>> Signed-off-by: Dmitry Kasatkin <d.kasatkin@samsung.com>
>
> I have a couple of concerns with replacing the IMA policy.
>
> - Currently opened files might now be in policy, that previously
> weren't.  Do these files need to be measured, appraised, or audited?
> These files could have been modified, but the 'security.ima' xattr
> hasn't been updated yet. In such cases, appraisal would fail.
>

Hi,

Yep. It is a "bit" larger problem.
But it will fail as well when using "chown" to change uid from user to
root, for example.

> - At minimum, after replacing the policy, the iint cache entry flags
> need to be reset.
>
> Please provide the motivation for such a use case scenario.
>
> Mimi
>

As I think I once mentioned, this patch should not be sent in that patchset.
We have some experimental stuff for controlling IMA at runtime.
That is not completed. There is no reason to discuss here a lot.


- Dmitry


>> ---
>>  security/integrity/ima/Kconfig      |  8 ++++++++
>>  security/integrity/ima/ima_fs.c     |  2 ++
>>  security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
>>  3 files changed, 29 insertions(+), 4 deletions(-)
>>
>> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
>> index b00044f..b60a315 100644
>> --- a/security/integrity/ima/Kconfig
>> +++ b/security/integrity/ima/Kconfig
>> @@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
>>       default n
>>       help
>>         This option enables IMA policy loading from the kernel.
>> +
>> +config IMA_POLICY_REPLACEABLE
>> +     bool "Allows to replace policy at runtime"
>> +     depends on IMA_POLICY_LOADER
>> +     default n
>> +     help
>> +       Enabling this option allows to replace policy at runtime.
>> +       Only signed policy is allowed.
>> diff --git a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c
>> index d050a5c..b4144b4 100644
>> --- a/security/integrity/ima/ima_fs.c
>> +++ b/security/integrity/ima/ima_fs.c
>> @@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
>>               return -EACCES;
>>       if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
>>               return -EBUSY;
>> +#ifndef CONFIG_IMA_POLICY_REPLACEABLE
>>       if (!ima_default_policy()) {
>>               /* policy was already set*/
>>               clear_bit(IMA_FS_BUSY, &ima_fs_flags);
>>               return -EACCES;
>>       }
>> +#endif
>>       return 0;
>>  }
>>
>> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
>> index c6da801..981e953 100644
>> --- a/security/integrity/ima/ima_policy.c
>> +++ b/security/integrity/ima/ima_policy.c
>> @@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
>>
>>  static LIST_HEAD(ima_default_rules);
>>  static LIST_HEAD(ima_policy_rules);
>> +static LIST_HEAD(ima_active_rules);
>>  static struct list_head *ima_rules;
>>  static bool path_rules;
>>
>>  static DEFINE_MUTEX(ima_rules_mutex);
>>
>> +static void ima_do_delete_rules(struct list_head *rules);
>> +
>>  static bool ima_use_tcb __initdata;
>>  static int __init default_measure_policy_setup(char *str)
>>  {
>> @@ -367,7 +370,14 @@ void ima_update_policy(void)
>>       int result = 0;
>>       int audit_info = 0;
>>
>> -     ima_rules = &ima_policy_rules;
>> +     if (ima_default_policy()) {
>> +             /* set new policy head */
>> +             ima_rules = &ima_active_rules;
>> +     } else {
>> +             /* FIXME: must be protected by lock */
>> +             ima_do_delete_rules(ima_rules);
>> +     }
>> +     list_replace_init(&ima_policy_rules, ima_rules);
>>
>>       integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
>>                           NULL, op, cause, result, audit_info);
>> @@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
>>       return len;
>>  }
>>
>> -/* ima_delete_rules called to cleanup invalid policy */
>> -void ima_delete_rules(void)
>> +/* ima_delete_rules called to cleanup invalid or old policy */
>> +static void ima_do_delete_rules(struct list_head *rules)
>>  {
>>       struct ima_rule_entry *entry, *tmp;
>>       int i;
>>
>>       mutex_lock(&ima_rules_mutex);
>> -     list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
>> +     list_for_each_entry_safe(entry, tmp, rules, list) {
>>               for (i = 0; i < MAX_LSM_RULES; i++)
>>                       kfree(entry->lsm[i].args_p);
>>
>> @@ -751,6 +761,11 @@ void ima_delete_rules(void)
>>       mutex_unlock(&ima_rules_mutex);
>>  }
>>
>> +void ima_delete_rules(void)
>> +{
>> +     ima_do_delete_rules(&ima_policy_rules);
>> +}
>> +
>>  #ifdef CONFIG_IMA_POLICY_LOADER
>>
>>  ssize_t ima_read_policy(char *path)
>
>
> --
> 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



-- 
Thanks,
Dmitry