[PATCHv3 0/6] integrity: few EVM patches

[PATCHv3 0/6] integrity: few EVM patches

[Dmitry Kasatkin]

Hi,

IMA module provides functionality to load x509 certificates into the
trusted '.ima' keyring. This is patchset adds the same functionality
to the EVM as well. Also it provides functionality to set EVM key from
the kernel crypto HW driver. This is an update for the patchset which was
previously sent for review few months ago. Please refer to the patch
descriptions for details.

BR,

Dmitry

Dmitry Kasatkin (6):
  integrity: define '.evm' as a builtin 'trusted' keyring
  evm: load x509 certificate from the kernel
  evm: enable EVM when X509 certificate is loaded
  evm: provide a function to set EVM key from the kernel
  evm: define EVM key max and min sizes
  evm: reset EVM status when file attributes changes

 include/linux/evm.h                 | 10 +++++++
 security/integrity/Kconfig          | 11 ++++++++
 security/integrity/digsig.c         | 14 ++++++++--
 security/integrity/evm/Kconfig      | 17 ++++++++++++
 security/integrity/evm/evm.h        |  3 +++
 security/integrity/evm/evm_crypto.c | 54 ++++++++++++++++++++++++++++++-------
 security/integrity/evm/evm_main.c   | 32 +++++++++++++++++++---
 security/integrity/evm/evm_secfs.c  | 12 +++------
 security/integrity/iint.c           |  1 +
 security/integrity/ima/Kconfig      |  5 +++-
 security/integrity/ima/ima.h        | 12 ---------
 security/integrity/ima/ima_init.c   |  2 +-
 security/integrity/integrity.h      | 13 ++++++---
 13 files changed, 146 insertions(+), 40 deletions(-)

-- 
2.1.4

[PATCHv3 1/6] integrity: define '.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.

This patch also switches IMA to use integrity_init_keyring().

Changes in v3:
* Added 'init_keyring' config based variable to skip initializing
  keyring instead of using  __integrity_init_keyring() wrapper.
* Added dependency back to CONFIG_IMA_TRUSTED_KEYRING

Changes in v2:
* Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common
  CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option
* Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config
  file compatibility. (Mimi Zohar)

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 security/integrity/Kconfig        | 11 +++++++++++
 security/integrity/digsig.c       | 14 ++++++++++++--
 security/integrity/evm/evm_main.c |  8 +++++---
 security/integrity/ima/Kconfig    |  5 ++++-
 security/integrity/ima/ima.h      | 12 ------------
 security/integrity/ima/ima_init.c |  2 +-
 security/integrity/integrity.h    |  5 ++---
 7 files changed, 35 insertions(+), 22 deletions(-)

diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
index 73c457b..21d7568 100644
--- a/security/integrity/Kconfig
+++ b/security/integrity/Kconfig
@@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS
 	  This option enables digital signature verification using
 	  asymmetric keys.
 
+config INTEGRITY_TRUSTED_KEYRING
+	bool "Require all keys on the integrity keyrings be signed"
+	depends on SYSTEM_TRUSTED_KEYRING
+	depends on INTEGRITY_ASYMMETRIC_KEYS
+	select KEYS_DEBUG_PROC_KEYS
+	default y
+	help
+	   This option requires that all keys added to the .ima and
+	   .evm keyrings be signed by a key on the system trusted
+	   keyring.
+
 config INTEGRITY_AUDIT
 	bool "Enables integrity auditing support "
 	depends on AUDIT
diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
index 5be9ffb..8ef1511 100644
--- a/security/integrity/digsig.c
+++ b/security/integrity/digsig.c
@@ -24,15 +24,22 @@
 static struct key *keyring[INTEGRITY_KEYRING_MAX];
 
 static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
+#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING
 	"_evm",
-	"_module",
-#ifndef CONFIG_IMA_TRUSTED_KEYRING
 	"_ima",
 #else
+	".evm",
 	".ima",
 #endif
+	"_module",
 };
 
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+static bool init_keyring __initdata = true;
+#else
+static bool init_keyring __initdata;
+#endif
+
 int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 			    const char *digest, int digestlen)
 {
@@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id)
 	const struct cred *cred = current_cred();
 	int err = 0;
 
+	if (!init_keyring)
+		return 0;
+
 	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
 				    KGIDT_INIT(0), cred,
 				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 1334e02..75b7e30 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -478,15 +478,17 @@ static int __init init_evm(void)
 
 	evm_init_config();
 
+	error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+	if (error)
+		return error;
+
 	error = evm_init_secfs();
 	if (error < 0) {
 		pr_info("Error registering secfs\n");
-		goto err;
+		return error;
 	}
 
 	return 0;
-err:
-	return error;
 }
 
 /*
diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
index df30334..a292b88 100644
--- a/security/integrity/ima/Kconfig
+++ b/security/integrity/ima/Kconfig
@@ -123,14 +123,17 @@ config IMA_APPRAISE
 	  If unsure, say N.
 
 config IMA_TRUSTED_KEYRING
-	bool "Require all keys on the .ima keyring be signed"
+	bool "Require all keys on the .ima keyring be signed (deprecated)"
 	depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
 	depends on INTEGRITY_ASYMMETRIC_KEYS
+	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.
 
+	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
+
 config IMA_LOAD_X509
 	bool "Load X509 certificate onto the '.ima' trusted keyring"
 	depends on IMA_TRUSTED_KEYRING
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index e2a60c3..9e82367 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
 	return -EINVAL;
 }
 #endif /* CONFIG_IMA_LSM_RULES */
-
-#ifdef CONFIG_IMA_TRUSTED_KEYRING
-static inline int ima_init_keyring(const unsigned int id)
-{
-	return integrity_init_keyring(id);
-}
-#else
-static inline int ima_init_keyring(const unsigned int id)
-{
-	return 0;
-}
-#endif /* CONFIG_IMA_TRUSTED_KEYRING */
 #endif
diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
index e600cad..bd79f25 100644
--- a/security/integrity/ima/ima_init.c
+++ b/security/integrity/ima/ima_init.c
@@ -116,7 +116,7 @@ int __init ima_init(void)
 	if (!ima_used_chip)
 		pr_info("No TPM chip found, activating TPM-bypass!\n");
 
-	rc = ima_init_keyring(INTEGRITY_KEYRING_IMA);
+	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 	if (rc)
 		return rc;
 
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 9c61687..07726a7 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -125,8 +125,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
 int __init integrity_read_file(const char *path, char **data);
 
 #define INTEGRITY_KEYRING_EVM		0
-#define INTEGRITY_KEYRING_MODULE	1
-#define INTEGRITY_KEYRING_IMA		2
+#define INTEGRITY_KEYRING_IMA		1
+#define INTEGRITY_KEYRING_MODULE	2
 #define INTEGRITY_KEYRING_MAX		3
 
 #ifdef CONFIG_INTEGRITY_SIGNATURE
@@ -149,7 +149,6 @@ static inline int integrity_init_keyring(const unsigned int id)
 {
 	return 0;
 }
-
 #endif /* CONFIG_INTEGRITY_SIGNATURE */
 
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
-- 
2.1.4

[PATCHv3 2/6] evm: load x509 certificate from the kernel

[Dmitry Kasatkin]

This patch defines configuration option and the evm_load_x509() hook
to load X509 certificate into the EVM trusted kernel keyring.

Changes in v4:
* Patch description updated

Changes in v3:
* Removed EVM_X509_PATH definition. CONFIG_EVM_X509_PATH is used
  directly.

Changes in v2:
* default key patch changed to /etc/keys

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 security/integrity/evm/Kconfig    | 17 +++++++++++++++++
 security/integrity/evm/evm_main.c |  7 +++++++
 security/integrity/iint.c         |  1 +
 security/integrity/integrity.h    |  8 ++++++++
 4 files changed, 33 insertions(+)

diff --git a/security/integrity/evm/Kconfig b/security/integrity/evm/Kconfig
index bf19723..b1433e9 100644
--- a/security/integrity/evm/Kconfig
+++ b/security/integrity/evm/Kconfig
@@ -42,3 +42,20 @@ config EVM_EXTRA_SMACK_XATTRS
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
+config EVM_LOAD_X509
+	bool "Load X509 certificate to the '.evm' trusted keyring"
+	depends on INTEGRITY_TRUSTED_KEYRING
+	default n
+	help
+	   Load X509 certificate to the '.evm' trusted keyring.
+
+	   This option enables X509 certificate loading from the kernel
+	   to the '.evm' trusted keyring. Public key can be used to
+	   verify EVM integrity starting from 'init' process.
+
+config EVM_X509_PATH
+	string "EVM X509 certificate path"
+	depends on EVM_LOAD_X509
+	default "/etc/keys/x509_evm.der"
+	help
+	   This option defines X509 certificate path.
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 75b7e30..519de0a 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -472,6 +472,13 @@ out:
 }
 EXPORT_SYMBOL_GPL(evm_inode_init_security);
 
+#ifdef CONFIG_EVM_LOAD_X509
+void __init evm_load_x509(void)
+{
+	integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
+}
+#endif
+
 static int __init init_evm(void)
 {
 	int error;
diff --git a/security/integrity/iint.c b/security/integrity/iint.c
index 3d2f5b4..2de9c82 100644
--- a/security/integrity/iint.c
+++ b/security/integrity/iint.c
@@ -254,4 +254,5 @@ out:
 void __init integrity_load_keys(void)
 {
 	ima_load_x509();
+	evm_load_x509();
 }
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 07726a7..5efe2ec 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -170,6 +170,14 @@ static inline void ima_load_x509(void)
 }
 #endif
 
+#ifdef CONFIG_EVM_LOAD_X509
+void __init evm_load_x509(void);
+#else
+static inline void evm_load_x509(void)
+{
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_AUDIT
 /* declarations */
 void integrity_audit_msg(int audit_msgno, struct inode *inode,
-- 
2.1.4

[PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded

[Dmitry Kasatkin]

In order to enable EVM before starting 'init' process,
evm_initialized needs to be non-zero. Before it was
indicating that HMAC key is loaded. When EVM loads
X509 before calling 'init', it is possible to enable
EVM to start signature based verification.

This patch defines bits to enable EVM if key of any type
is loaded.

Changes in v2:
* EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC
* EVM_STATE_X509_SET replaced by EVM_INIT_X509

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 security/integrity/evm/evm.h        | 3 +++
 security/integrity/evm/evm_crypto.c | 2 ++
 security/integrity/evm/evm_main.c   | 6 +++++-
 security/integrity/evm/evm_secfs.c  | 4 ++--
 4 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 88bfe77..f5f1272 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -21,6 +21,9 @@
 
 #include "../integrity.h"
 
+#define EVM_INIT_HMAC	0x0001
+#define EVM_INIT_X509	0x0002
+
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 159ef3e..34e1a6f 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type)
 	struct shash_desc *desc;
 
 	if (type == EVM_XATTR_HMAC) {
+		if (!(evm_initialized & EVM_INIT_HMAC))
+			return ERR_PTR(-ENOKEY);
 		tfm = &hmac_tfm;
 		algo = evm_hmac;
 	} else {
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 519de0a..420d94d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security);
 #ifdef CONFIG_EVM_LOAD_X509
 void __init evm_load_x509(void)
 {
-	integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
+	int rc;
+
+	rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
+	if (!rc)
+		evm_initialized |= EVM_INIT_X509;
 }
 #endif
 
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index cf12a04..3f775df 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -64,7 +64,7 @@ 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_INIT_HMAC))
 		return -EPERM;
 
 	if (count >= sizeof(temp) || count == 0)
@@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 
 	error = evm_init_key();
 	if (!error) {
-		evm_initialized = 1;
+		evm_initialized |= EVM_INIT_HMAC;
 		pr_info("initialized\n");
 	} else
 		pr_err("initialization failed\n");
-- 
2.1.4

[PATCHv3 4/6] evm: provide a function to set EVM key from the kernel

[Dmitry Kasatkin]

Crypto HW kernel module can possibly initialize EVM key from the
kernel __init code to enable EVM before calling 'init' process.
This patch provide a function evm_set_key() which can be used to
set custom key directly to EVM without using KEY subsystem.

Changes in v3:
* error reporting moved to evm_set_key
* EVM_INIT_HMAC moved to evm_set_key
* added bitop to prevent key setting race

Changes in v2:
* use size_t for key size instead of signed int
* provide EVM_MAX_KEY_SIZE macro in <linux/evm.h>
* provide EVM_MIN_KEY_SIZE macro in <linux/evm.h>

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 include/linux/evm.h                 |  7 ++++++
 security/integrity/evm/evm_crypto.c | 47 +++++++++++++++++++++++++++++++------
 security/integrity/evm/evm_secfs.c  | 10 +++-----
 3 files changed, 50 insertions(+), 14 deletions(-)

diff --git a/include/linux/evm.h b/include/linux/evm.h
index 1fcb88c..35ed9a8 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -14,6 +14,7 @@
 struct integrity_iint_cache;
 
 #ifdef CONFIG_EVM
+extern int evm_set_key(void *key, size_t keylen);
 extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     const char *xattr_name,
 					     void *xattr_value,
@@ -42,6 +43,12 @@ static inline int posix_xattr_acl(const char *xattrname)
 }
 #endif
 #else
+
+static inline int evm_set_key(void *key, size_t keylen)
+{
+	return -EOPNOTSUPP;
+}
+
 #ifdef CONFIG_INTEGRITY
 static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
 						    const char *xattr_name,
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 34e1a6f..7aec93e 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -18,6 +18,7 @@
 #include <linux/module.h>
 #include <linux/crypto.h>
 #include <linux/xattr.h>
+#include <linux/evm.h>
 #include <keys/encrypted-type.h>
 #include <crypto/hash.h>
 #include "evm.h"
@@ -32,6 +33,41 @@ struct crypto_shash *hash_tfm;
 
 static DEFINE_MUTEX(mutex);
 
+#define EVM_SET_KEY_BUSY 0
+
+static unsigned long evm_set_key_flags;
+
+/* evm_set_key - set EVM HMAC key from the kernel
+ *
+ * This function allows to set EVM HMAC key from the kernel
+ * without using key subsystem 'encrypted' keys. It can be used
+ * by the crypto HW kernel module which has own way of managing
+ * keys.
+ *
+ * key length should be between 32 and 128 bytes long
+ */
+int evm_set_key(void *key, size_t keylen)
+{
+	int rc;
+
+	rc = -EBUSY;
+	if (test_and_set_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags))
+		goto busy;
+	rc = -EINVAL;
+	if (keylen > MAX_KEY_SIZE)
+		goto inval;
+	memcpy(evmkey, key, keylen);
+	evm_initialized |= EVM_INIT_HMAC;
+	pr_info("key initialized\n");
+	return 0;
+inval:
+	clear_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags);
+busy:
+	pr_err("key initialization failed\n");
+	return rc;
+}
+EXPORT_SYMBOL_GPL(evm_set_key);
+
 static struct shash_desc *init_desc(char type)
 {
 	long rc;
@@ -242,7 +278,7 @@ int evm_init_key(void)
 {
 	struct key *evm_key;
 	struct encrypted_key_payload *ekp;
-	int rc = 0;
+	int rc;
 
 	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
 	if (IS_ERR(evm_key))
@@ -250,12 +286,9 @@ int evm_init_key(void)
 
 	down_read(&evm_key->sem);
 	ekp = evm_key->payload.data;
-	if (ekp->decrypted_datalen > MAX_KEY_SIZE) {
-		rc = -EINVAL;
-		goto out;
-	}
-	memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen);
-out:
+
+	rc = evm_set_key(ekp->decrypted_data, ekp->decrypted_datalen);
+
 	/* burn the original key contents */
 	memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
 	up_read(&evm_key->sem);
diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index 3f775df..c8dccd5 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_INIT_HMAC))
 		return -EPERM;
@@ -78,12 +78,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 |= EVM_INIT_HMAC;
-		pr_info("initialized\n");
-	} else
-		pr_err("initialization failed\n");
+	evm_init_key();
+
 	return count;
 }
 
-- 
2.1.4

[PATCHv3 5/6] evm: define EVM key max and min sizes

[Dmitry Kasatkin]

This patch imposes minimum key size limit.
It declares EVM_MIN_KEY_SIZE and EVM_MAX_KEY_SIZE in public header file.

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 include/linux/evm.h                 | 3 +++
 security/integrity/evm/evm_crypto.c | 7 +++----
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/include/linux/evm.h b/include/linux/evm.h
index 35ed9a8..0aeedec 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -11,6 +11,9 @@
 #include <linux/integrity.h>
 #include <linux/xattr.h>
 
+#define EVM_MAX_KEY_SIZE	128
+#define EVM_MIN_KEY_SIZE	64
+
 struct integrity_iint_cache;
 
 #ifdef CONFIG_EVM
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 7aec93e..cfb788c 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -24,9 +24,8 @@
 #include "evm.h"
 
 #define EVMKEY "evm-key"
-#define MAX_KEY_SIZE 128
-static unsigned char evmkey[MAX_KEY_SIZE];
-static int evmkey_len = MAX_KEY_SIZE;
+static unsigned char evmkey[EVM_MAX_KEY_SIZE];
+static int evmkey_len = EVM_MAX_KEY_SIZE;
 
 struct crypto_shash *hmac_tfm;
 struct crypto_shash *hash_tfm;
@@ -54,7 +53,7 @@ int evm_set_key(void *key, size_t keylen)
 	if (test_and_set_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags))
 		goto busy;
 	rc = -EINVAL;
-	if (keylen > MAX_KEY_SIZE)
+	if (keylen < EVM_MIN_KEY_SIZE || keylen > EVM_MAX_KEY_SIZE)
 		goto inval;
 	memcpy(evmkey, key, keylen);
 	evm_initialized |= EVM_INIT_HMAC;
-- 
2.1.4

[PATCHv3 6/6] evm: reset EVM status when file attributes changes

[Dmitry Kasatkin]

EVM verification status is cached in iint->evm_status
and if it was successful, never re-verified again when
IMA passes 'iint' to evm_verifyxattr().

When file attribute or extended attributes changes we may
wish to re-verify EVM integrity as well. For example,
after setting digital signature we may need to re-verify
the signature and update iint->flags that there is EVM
signature.

This patch enables that by resetting evm_status to
INTEGRITY_UKNOWN state.

Changes in v2:
* Flag setting moved to EVM layer

Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
---
 security/integrity/evm/evm_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 420d94d..f716025 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -358,6 +358,15 @@ int evm_inode_removexattr(struct dentry *dentry, const char *xattr_name)
 	return evm_protect_xattr(dentry, xattr_name, NULL, 0);
 }
 
+static void evm_reset_status(struct inode *inode)
+{
+	struct integrity_iint_cache *iint;
+
+	iint = integrity_iint_find(inode);
+	if (iint)
+		iint->evm_status = INTEGRITY_UNKNOWN;
+}
+
 /**
  * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
  * @dentry: pointer to the affected dentry
@@ -378,6 +387,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 				 && !posix_xattr_acl(xattr_name)))
 		return;
 
+	evm_reset_status(dentry->d_inode);
+
 	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
 }
 
@@ -396,6 +407,8 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
 	if (!evm_initialized || !evm_protected_xattr(xattr_name))
 		return;
 
+	evm_reset_status(dentry->d_inode);
+
 	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
 }
 
-- 
2.1.4

Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

[Petko Manolov]

On 15-10-22 21:49:25, Dmitry Kasatkin wrote:
> Require all keys added to the EVM keyring be signed by an
> existing trusted key on the system trusted keyring.
> 
> This patch also switches IMA to use integrity_init_keyring().
> 
> Changes in v3:
> * Added 'init_keyring' config based variable to skip initializing
>   keyring instead of using  __integrity_init_keyring() wrapper.
> * Added dependency back to CONFIG_IMA_TRUSTED_KEYRING
> 
> Changes in v2:
> * Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common
>   CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option
> * Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config
>   file compatibility. (Mimi Zohar)
> 
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> ---
>  security/integrity/Kconfig        | 11 +++++++++++
>  security/integrity/digsig.c       | 14 ++++++++++++--
>  security/integrity/evm/evm_main.c |  8 +++++---
>  security/integrity/ima/Kconfig    |  5 ++++-
>  security/integrity/ima/ima.h      | 12 ------------
>  security/integrity/ima/ima_init.c |  2 +-
>  security/integrity/integrity.h    |  5 ++---
>  7 files changed, 35 insertions(+), 22 deletions(-)
> 
> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index 73c457b..21d7568 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS
>  	  This option enables digital signature verification using
>  	  asymmetric keys.
>  
> +config INTEGRITY_TRUSTED_KEYRING
> +	bool "Require all keys on the integrity keyrings be signed"
> +	depends on SYSTEM_TRUSTED_KEYRING
> +	depends on INTEGRITY_ASYMMETRIC_KEYS
> +	select KEYS_DEBUG_PROC_KEYS
> +	default y
> +	help
> +	   This option requires that all keys added to the .ima and
> +	   .evm keyrings be signed by a key on the system trusted
> +	   keyring.
> +
>  config INTEGRITY_AUDIT
>  	bool "Enables integrity auditing support "
>  	depends on AUDIT
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 5be9ffb..8ef1511 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -24,15 +24,22 @@
>  static struct key *keyring[INTEGRITY_KEYRING_MAX];
>  
>  static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
> +#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING
>  	"_evm",
> -	"_module",
> -#ifndef CONFIG_IMA_TRUSTED_KEYRING
>  	"_ima",
>  #else
> +	".evm",
>  	".ima",
>  #endif
> +	"_module",
>  };
>  
> +#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
> +static bool init_keyring __initdata = true;
> +#else
> +static bool init_keyring __initdata;
> +#endif
> +
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>  			    const char *digest, int digestlen)
>  {
> @@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id)
>  	const struct cred *cred = current_cred();
>  	int err = 0;
>  
> +	if (!init_keyring)
> +		return 0;
> +
>  	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
>  				    KGIDT_INIT(0), cred,
>  				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 1334e02..75b7e30 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -478,15 +478,17 @@ static int __init init_evm(void)
>  
>  	evm_init_config();
>  
> +	error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
> +	if (error)
> +		return error;
> +
>  	error = evm_init_secfs();
>  	if (error < 0) {
>  		pr_info("Error registering secfs\n");
> -		goto err;
> +		return error;
>  	}
>  
>  	return 0;
> -err:
> -	return error;
>  }
>  
>  /*
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index df30334..a292b88 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -123,14 +123,17 @@ config IMA_APPRAISE
>  	  If unsure, say N.
>  
>  config IMA_TRUSTED_KEYRING
> -	bool "Require all keys on the .ima keyring be signed"
> +	bool "Require all keys on the .ima keyring be signed (deprecated)"
>  	depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
>  	depends on INTEGRITY_ASYMMETRIC_KEYS
> +	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.
>  
> +	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> +
>  config IMA_LOAD_X509
>  	bool "Load X509 certificate onto the '.ima' trusted keyring"
>  	depends on IMA_TRUSTED_KEYRING


I guess we may as well remove this switch.  Otherwise somebody have to remember 
to post a patch that does so a few kernel releases after this one goes mainline.


> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e2a60c3..9e82367 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
>  	return -EINVAL;
>  }
>  #endif /* CONFIG_IMA_LSM_RULES */
> -
> -#ifdef CONFIG_IMA_TRUSTED_KEYRING
> -static inline int ima_init_keyring(const unsigned int id)
> -{
> -	return integrity_init_keyring(id);
> -}
> -#else
> -static inline int ima_init_keyring(const unsigned int id)
> -{
> -	return 0;
> -}
> -#endif /* CONFIG_IMA_TRUSTED_KEYRING */
>  #endif
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index e600cad..bd79f25 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -116,7 +116,7 @@ int __init ima_init(void)
>  	if (!ima_used_chip)
>  		pr_info("No TPM chip found, activating TPM-bypass!\n");
>  
> -	rc = ima_init_keyring(INTEGRITY_KEYRING_IMA);
> +	rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
>  	if (rc)
>  		return rc;
>  
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 9c61687..07726a7 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -125,8 +125,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>  int __init integrity_read_file(const char *path, char **data);
>  
>  #define INTEGRITY_KEYRING_EVM		0
> -#define INTEGRITY_KEYRING_MODULE	1
> -#define INTEGRITY_KEYRING_IMA		2
> +#define INTEGRITY_KEYRING_IMA		1
> +#define INTEGRITY_KEYRING_MODULE	2
>  #define INTEGRITY_KEYRING_MAX		3
>  
>  #ifdef CONFIG_INTEGRITY_SIGNATURE
> @@ -149,7 +149,6 @@ static inline int integrity_init_keyring(const unsigned int id)
>  {
>  	return 0;
>  }
> -
>  #endif /* CONFIG_INTEGRITY_SIGNATURE */
>  
>  #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> -- 
> 2.1.4

ACK to the rest of the code.



		Petko

RE: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

[Dmitry Kasatkin]

________________________________________
From: Petko Manolov [petkan@mip-labs.com]
Sent: Friday, October 23, 2015 4:05 PM
To: Dmitry Kasatkin
Cc: zohar@linux.vnet.ibm.com; linux-ima-devel@lists.sourceforge.net; linux-security-module@vger.kernel.org; linux-kernel@vger.kernel.org; Dmitry Kasatkin
Subject: Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

On 15-10-22 21:49:25, Dmitry Kasatkin wrote:
> Require all keys added to the EVM keyring be signed by an
> existing trusted key on the system trusted keyring.
>
> This patch also switches IMA to use integrity_init_keyring().
>
> Changes in v3:
> * Added 'init_keyring' config based variable to skip initializing
>   keyring instead of using  __integrity_init_keyring() wrapper.
> * Added dependency back to CONFIG_IMA_TRUSTED_KEYRING
>
> Changes in v2:
> * Replace CONFIG_EVM_TRUSTED_KEYRING with IMA and EVM common
>   CONFIG_INTEGRITY_TRUSTED_KEYRING configuration option
> * Deprecate CONFIG_IMA_TRUSTED_KEYRING but keep it for config
>   file compatibility. (Mimi Zohar)
>
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> ---
>  security/integrity/Kconfig        | 11 +++++++++++
>  security/integrity/digsig.c       | 14 ++++++++++++--
>  security/integrity/evm/evm_main.c |  8 +++++---
>  security/integrity/ima/Kconfig    |  5 ++++-
>  security/integrity/ima/ima.h      | 12 ------------
>  security/integrity/ima/ima_init.c |  2 +-
>  security/integrity/integrity.h    |  5 ++---
>  7 files changed, 35 insertions(+), 22 deletions(-)
>
> diff --git a/security/integrity/Kconfig b/security/integrity/Kconfig
> index 73c457b..21d7568 100644
> --- a/security/integrity/Kconfig
> +++ b/security/integrity/Kconfig
> @@ -41,6 +41,17 @@ config INTEGRITY_ASYMMETRIC_KEYS
>         This option enables digital signature verification using
>         asymmetric keys.
>
> +config INTEGRITY_TRUSTED_KEYRING
> +     bool "Require all keys on the integrity keyrings be signed"
> +     depends on SYSTEM_TRUSTED_KEYRING
> +     depends on INTEGRITY_ASYMMETRIC_KEYS
> +     select KEYS_DEBUG_PROC_KEYS
> +     default y
> +     help
> +        This option requires that all keys added to the .ima and
> +        .evm keyrings be signed by a key on the system trusted
> +        keyring.
> +
>  config INTEGRITY_AUDIT
>       bool "Enables integrity auditing support "
>       depends on AUDIT
> diff --git a/security/integrity/digsig.c b/security/integrity/digsig.c
> index 5be9ffb..8ef1511 100644
> --- a/security/integrity/digsig.c
> +++ b/security/integrity/digsig.c
> @@ -24,15 +24,22 @@
>  static struct key *keyring[INTEGRITY_KEYRING_MAX];
>
>  static const char *keyring_name[INTEGRITY_KEYRING_MAX] = {
> +#ifndef CONFIG_INTEGRITY_TRUSTED_KEYRING
>       "_evm",
> -     "_module",
> -#ifndef CONFIG_IMA_TRUSTED_KEYRING
>       "_ima",
>  #else
> +     ".evm",
>       ".ima",
>  #endif
> +     "_module",
>  };
>
> +#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
> +static bool init_keyring __initdata = true;
> +#else
> +static bool init_keyring __initdata;
> +#endif
> +
>  int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
>                           const char *digest, int digestlen)
>  {
> @@ -68,6 +75,9 @@ int __init integrity_init_keyring(const unsigned int id)
>       const struct cred *cred = current_cred();
>       int err = 0;
>
> +     if (!init_keyring)
> +             return 0;
> +
>       keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
>                                   KGIDT_INIT(0), cred,
>                                   ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 1334e02..75b7e30 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -478,15 +478,17 @@ static int __init init_evm(void)
>
>       evm_init_config();
>
> +     error = integrity_init_keyring(INTEGRITY_KEYRING_EVM);
> +     if (error)
> +             return error;
> +
>       error = evm_init_secfs();
>       if (error < 0) {
>               pr_info("Error registering secfs\n");
> -             goto err;
> +             return error;
>       }
>
>       return 0;
> -err:
> -     return error;
>  }
>
>  /*
> diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> index df30334..a292b88 100644
> --- a/security/integrity/ima/Kconfig
> +++ b/security/integrity/ima/Kconfig
> @@ -123,14 +123,17 @@ config IMA_APPRAISE
>         If unsure, say N.
>
>  config IMA_TRUSTED_KEYRING
> -     bool "Require all keys on the .ima keyring be signed"
> +     bool "Require all keys on the .ima keyring be signed (deprecated)"
>       depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
>       depends on INTEGRITY_ASYMMETRIC_KEYS
> +     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.
>
> +        This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> +
>  config IMA_LOAD_X509
>       bool "Load X509 certificate onto the '.ima' trusted keyring"
>       depends on IMA_TRUSTED_KEYRING


I guess we may as well remove this switch.  Otherwise somebody have to remember
to post a patch that does so a few kernel releases after this one goes mainline.

It was actually removed in one of first versions, but Mimi suggested to keep it 1 release to allow "seamless" migration from kernel version to kernel version.
In the next release we may remove it completely.

Dmitry

> diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
> index e2a60c3..9e82367 100644
> --- a/security/integrity/ima/ima.h
> +++ b/security/integrity/ima/ima.h
> @@ -251,16 +251,4 @@ static inline int security_filter_rule_match(u32 secid, u32 field, u32 op,
>       return -EINVAL;
>  }
>  #endif /* CONFIG_IMA_LSM_RULES */
> -
> -#ifdef CONFIG_IMA_TRUSTED_KEYRING
> -static inline int ima_init_keyring(const unsigned int id)
> -{
> -     return integrity_init_keyring(id);
> -}
> -#else
> -static inline int ima_init_keyring(const unsigned int id)
> -{
> -     return 0;
> -}
> -#endif /* CONFIG_IMA_TRUSTED_KEYRING */
>  #endif
> diff --git a/security/integrity/ima/ima_init.c b/security/integrity/ima/ima_init.c
> index e600cad..bd79f25 100644
> --- a/security/integrity/ima/ima_init.c
> +++ b/security/integrity/ima/ima_init.c
> @@ -116,7 +116,7 @@ int __init ima_init(void)
>       if (!ima_used_chip)
>               pr_info("No TPM chip found, activating TPM-bypass!\n");
>
> -     rc = ima_init_keyring(INTEGRITY_KEYRING_IMA);
> +     rc = integrity_init_keyring(INTEGRITY_KEYRING_IMA);
>       if (rc)
>               return rc;
>
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 9c61687..07726a7 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -125,8 +125,8 @@ int integrity_kernel_read(struct file *file, loff_t offset,
>  int __init integrity_read_file(const char *path, char **data);
>
>  #define INTEGRITY_KEYRING_EVM                0
> -#define INTEGRITY_KEYRING_MODULE     1
> -#define INTEGRITY_KEYRING_IMA                2
> +#define INTEGRITY_KEYRING_IMA                1
> +#define INTEGRITY_KEYRING_MODULE     2
>  #define INTEGRITY_KEYRING_MAX                3
>
>  #ifdef CONFIG_INTEGRITY_SIGNATURE
> @@ -149,7 +149,6 @@ static inline int integrity_init_keyring(const unsigned int id)
>  {
>       return 0;
>  }
> -
>  #endif /* CONFIG_INTEGRITY_SIGNATURE */
>
>  #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
> --
> 2.1.4

ACK to the rest of the code.



                Petko

Re: [PATCHv3 4/6] evm: provide a function to set EVM key from the kernel

[Mimi Zohar]

On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
> Crypto HW kernel module can possibly initialize EVM key from the
> kernel __init code to enable EVM before calling 'init' process.
> This patch provide a function evm_set_key() which can be used to
> set custom key directly to EVM without using KEY subsystem.

Thanks, Dmitry.  There's a minor comment inline.
> 
> Changes in v3:
> * error reporting moved to evm_set_key
> * EVM_INIT_HMAC moved to evm_set_key
> * added bitop to prevent key setting race
> 
> Changes in v2:
> * use size_t for key size instead of signed int
> * provide EVM_MAX_KEY_SIZE macro in <linux/evm.h>
> * provide EVM_MIN_KEY_SIZE macro in <linux/evm.h>
> 
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> ---
>  include/linux/evm.h                 |  7 ++++++
>  security/integrity/evm/evm_crypto.c | 47 +++++++++++++++++++++++++++++++------
>  security/integrity/evm/evm_secfs.c  | 10 +++-----
>  3 files changed, 50 insertions(+), 14 deletions(-)
> 
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 1fcb88c..35ed9a8 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -14,6 +14,7 @@
>  struct integrity_iint_cache;
> 
>  #ifdef CONFIG_EVM
> +extern int evm_set_key(void *key, size_t keylen);
>  extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  					     const char *xattr_name,
>  					     void *xattr_value,
> @@ -42,6 +43,12 @@ static inline int posix_xattr_acl(const char *xattrname)
>  }
>  #endif
>  #else
> +
> +static inline int evm_set_key(void *key, size_t keylen)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
>  #ifdef CONFIG_INTEGRITY
>  static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  						    const char *xattr_name,
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index 34e1a6f..7aec93e 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -18,6 +18,7 @@
>  #include <linux/module.h>
>  #include <linux/crypto.h>
>  #include <linux/xattr.h>
> +#include <linux/evm.h>
>  #include <keys/encrypted-type.h>
>  #include <crypto/hash.h>
>  #include "evm.h"
> @@ -32,6 +33,41 @@ struct crypto_shash *hash_tfm;
> 
>  static DEFINE_MUTEX(mutex);
> 
> +#define EVM_SET_KEY_BUSY 0
> +
> +static unsigned long evm_set_key_flags;
> +
> +/* evm_set_key - set EVM HMAC key from the kernel
> + *

For exported functions this should be "kernel-doc" format.

Mimi  

> + * This function allows to set EVM HMAC key from the kernel
> + * without using key subsystem 'encrypted' keys. It can be used
> + * by the crypto HW kernel module which has own way of managing
> + * keys.
> + *
> + * key length should be between 32 and 128 bytes long
> + */
> +int evm_set_key(void *key, size_t keylen)
> +{
> +	int rc;
> +
> +	rc = -EBUSY;
> +	if (test_and_set_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags))
> +		goto busy;
> +	rc = -EINVAL;
> +	if (keylen > MAX_KEY_SIZE)
> +		goto inval;
> +	memcpy(evmkey, key, keylen);
> +	evm_initialized |= EVM_INIT_HMAC;
> +	pr_info("key initialized\n");
> +	return 0;
> +inval:
> +	clear_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags);
> +busy:
> +	pr_err("key initialization failed\n");
> +	return rc;
> +}
> +EXPORT_SYMBOL_GPL(evm_set_key);
> +
>  static struct shash_desc *init_desc(char type)
>  {
>  	long rc;
> @@ -242,7 +278,7 @@ int evm_init_key(void)
>  {
>  	struct key *evm_key;
>  	struct encrypted_key_payload *ekp;
> -	int rc = 0;
> +	int rc;
> 
>  	evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
>  	if (IS_ERR(evm_key))
> @@ -250,12 +286,9 @@ int evm_init_key(void)
> 
>  	down_read(&evm_key->sem);
>  	ekp = evm_key->payload.data;
> -	if (ekp->decrypted_datalen > MAX_KEY_SIZE) {
> -		rc = -EINVAL;
> -		goto out;
> -	}
> -	memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen);
> -out:
> +
> +	rc = evm_set_key(ekp->decrypted_data, ekp->decrypted_datalen);
> +
>  	/* burn the original key contents */
>  	memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
>  	up_read(&evm_key->sem);
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index 3f775df..c8dccd5 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_INIT_HMAC))
>  		return -EPERM;
> @@ -78,12 +78,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 |= EVM_INIT_HMAC;
> -		pr_info("initialized\n");
> -	} else
> -		pr_err("initialization failed\n");
> +	evm_init_key();
> +
>  	return count;
>  }
> 

Re: [PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded

[Mimi Zohar]

On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
> In order to enable EVM before starting 'init' process,
> evm_initialized needs to be non-zero. Before it was
> indicating that HMAC key is loaded. When EVM loads
> X509 before calling 'init', it is possible to enable
> EVM to start signature based verification.
> 
> This patch defines bits to enable EVM if key of any type
> is loaded.

Thanks, Dmitry.  There's one comment inline.

> Changes in v2:
> * EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC
> * EVM_STATE_X509_SET replaced by EVM_INIT_X509
> 
> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
> ---
>  security/integrity/evm/evm.h        | 3 +++
>  security/integrity/evm/evm_crypto.c | 2 ++
>  security/integrity/evm/evm_main.c   | 6 +++++-
>  security/integrity/evm/evm_secfs.c  | 4 ++--
>  4 files changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
> index 88bfe77..f5f1272 100644
> --- a/security/integrity/evm/evm.h
> +++ b/security/integrity/evm/evm.h
> @@ -21,6 +21,9 @@
> 
>  #include "../integrity.h"
> 
> +#define EVM_INIT_HMAC	0x0001
> +#define EVM_INIT_X509	0x0002
> +
>  extern int evm_initialized;
>  extern char *evm_hmac;
>  extern char *evm_hash;
> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
> index 159ef3e..34e1a6f 100644
> --- a/security/integrity/evm/evm_crypto.c
> +++ b/security/integrity/evm/evm_crypto.c
> @@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type)
>  	struct shash_desc *desc;
> 
>  	if (type == EVM_XATTR_HMAC) {
> +		if (!(evm_initialized & EVM_INIT_HMAC))
> +			return ERR_PTR(-ENOKEY);

init_desc() is called from a couple of different places.  In some
instances, like when converting from a signature to an hmac, if
init_desc() fails, the xattr isn't converted to an HMAC.  No big deal.
But there are other cases, like when a protected xattr is modified,
failing the write will make the file inaccessible.  Does there need to
be an error msg of some sort or an audit msg?

Mimi

>  		tfm = &hmac_tfm;
>  		algo = evm_hmac;
>  	} else {
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 519de0a..420d94d 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security);
>  #ifdef CONFIG_EVM_LOAD_X509
>  void __init evm_load_x509(void)
>  {
> -	integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
> +	int rc;
> +
> +	rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
> +	if (!rc)
> +		evm_initialized |= EVM_INIT_X509;
>  }
>  #endif
> 
> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
> index cf12a04..3f775df 100644
> --- a/security/integrity/evm/evm_secfs.c
> +++ b/security/integrity/evm/evm_secfs.c
> @@ -64,7 +64,7 @@ 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_INIT_HMAC))
>  		return -EPERM;
> 
>  	if (count >= sizeof(temp) || count == 0)
> @@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
> 
>  	error = evm_init_key();
>  	if (!error) {
> -		evm_initialized = 1;
> +		evm_initialized |= EVM_INIT_HMAC;
>  		pr_info("initialized\n");
>  	} else
>  		pr_err("initialization failed\n");

Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

[Mimi Zohar]

On Fri, 2015-10-23 at 16:05 +0300, Petko Manolov wrote:
> On 15-10-22 21:49:25, Dmitry Kasatkin wrote:

> > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > index df30334..a292b88 100644
> > --- a/security/integrity/ima/Kconfig
> > +++ b/security/integrity/ima/Kconfig
> > @@ -123,14 +123,17 @@ config IMA_APPRAISE
> >  	  If unsure, say N.
> >  
> >  config IMA_TRUSTED_KEYRING
> > -	bool "Require all keys on the .ima keyring be signed"
> > +	bool "Require all keys on the .ima keyring be signed (deprecated)"
> >  	depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> >  	depends on INTEGRITY_ASYMMETRIC_KEYS
> > +	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.
> >  
> > +	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> > +
> >  config IMA_LOAD_X509
> >  	bool "Load X509 certificate onto the '.ima' trusted keyring"
> >  	depends on IMA_TRUSTED_KEYRING
> 
> 
> I guess we may as well remove this switch.  Otherwise somebody have to remember 
> to post a patch that does so a few kernel releases after this one goes mainline.

There's no harm in leaving the "IMA_TRUSTED_KEYRING" Kconfig option for
a couple of releases (or perhaps until it is enabled in a long term
release), so that the INTEGRITY_TRUSTED_KEYRING Kconfig option is
enabled automatically.

Mimi

Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

[Petko Manolov]

On 15-10-23 14:43:53, Mimi Zohar wrote:
> On Fri, 2015-10-23 at 16:05 +0300, Petko Manolov wrote:
> > On 15-10-22 21:49:25, Dmitry Kasatkin wrote:
> 
> > > diff --git a/security/integrity/ima/Kconfig b/security/integrity/ima/Kconfig
> > > index df30334..a292b88 100644
> > > --- a/security/integrity/ima/Kconfig
> > > +++ b/security/integrity/ima/Kconfig
> > > @@ -123,14 +123,17 @@ config IMA_APPRAISE
> > >  	  If unsure, say N.
> > >  
> > >  config IMA_TRUSTED_KEYRING
> > > -	bool "Require all keys on the .ima keyring be signed"
> > > +	bool "Require all keys on the .ima keyring be signed (deprecated)"
> > >  	depends on IMA_APPRAISE && SYSTEM_TRUSTED_KEYRING
> > >  	depends on INTEGRITY_ASYMMETRIC_KEYS
> > > +	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.
> > >  
> > > +	   This option is deprecated in favor of INTEGRITY_TRUSTED_KEYRING
> > > +
> > >  config IMA_LOAD_X509
> > >  	bool "Load X509 certificate onto the '.ima' trusted keyring"
> > >  	depends on IMA_TRUSTED_KEYRING
> > 
> > 
> > I guess we may as well remove this switch.  Otherwise somebody have to remember 
> > to post a patch that does so a few kernel releases after this one goes mainline.
> 
> There's no harm in leaving the "IMA_TRUSTED_KEYRING" Kconfig option for a 
> couple of releases (or perhaps until it is enabled in a long term release), so 
> that the INTEGRITY_TRUSTED_KEYRING Kconfig option is enabled automatically.

I have no strong opinion either way.  Just saying.  Let it be for the moment.


		Petko

Re: [PATCHv3 3/6] evm: enable EVM when X509 certificate is loaded

[Dmitry Kasatkin]

Hi,

I added error printing to the patch

http://git.kernel.org/cgit/linux/kernel/git/kasatkin/linux-digsig.git/log/?h=ima-next

Dmitry


On Fri, Oct 23, 2015 at 9:31 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
>> In order to enable EVM before starting 'init' process,
>> evm_initialized needs to be non-zero. Before it was
>> indicating that HMAC key is loaded. When EVM loads
>> X509 before calling 'init', it is possible to enable
>> EVM to start signature based verification.
>>
>> This patch defines bits to enable EVM if key of any type
>> is loaded.
>
> Thanks, Dmitry.  There's one comment inline.
>
>> Changes in v2:
>> * EVM_STATE_KEY_SET replaced by EVM_INIT_HMAC
>> * EVM_STATE_X509_SET replaced by EVM_INIT_X509
>>
>> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
>> ---
>>  security/integrity/evm/evm.h        | 3 +++
>>  security/integrity/evm/evm_crypto.c | 2 ++
>>  security/integrity/evm/evm_main.c   | 6 +++++-
>>  security/integrity/evm/evm_secfs.c  | 4 ++--
>>  4 files changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
>> index 88bfe77..f5f1272 100644
>> --- a/security/integrity/evm/evm.h
>> +++ b/security/integrity/evm/evm.h
>> @@ -21,6 +21,9 @@
>>
>>  #include "../integrity.h"
>>
>> +#define EVM_INIT_HMAC        0x0001
>> +#define EVM_INIT_X509        0x0002
>> +
>>  extern int evm_initialized;
>>  extern char *evm_hmac;
>>  extern char *evm_hash;
>> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> index 159ef3e..34e1a6f 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -40,6 +40,8 @@ static struct shash_desc *init_desc(char type)
>>       struct shash_desc *desc;
>>
>>       if (type == EVM_XATTR_HMAC) {
>> +             if (!(evm_initialized & EVM_INIT_HMAC))
>> +                     return ERR_PTR(-ENOKEY);
>
> init_desc() is called from a couple of different places.  In some
> instances, like when converting from a signature to an hmac, if
> init_desc() fails, the xattr isn't converted to an HMAC.  No big deal.
> But there are other cases, like when a protected xattr is modified,
> failing the write will make the file inaccessible.  Does there need to
> be an error msg of some sort or an audit msg?
>
> Mimi
>
>>               tfm = &hmac_tfm;
>>               algo = evm_hmac;
>>       } else {
>> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
>> index 519de0a..420d94d 100644
>> --- a/security/integrity/evm/evm_main.c
>> +++ b/security/integrity/evm/evm_main.c
>> @@ -475,7 +475,11 @@ EXPORT_SYMBOL_GPL(evm_inode_init_security);
>>  #ifdef CONFIG_EVM_LOAD_X509
>>  void __init evm_load_x509(void)
>>  {
>> -     integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
>> +     int rc;
>> +
>> +     rc = integrity_load_x509(INTEGRITY_KEYRING_EVM, CONFIG_EVM_X509_PATH);
>> +     if (!rc)
>> +             evm_initialized |= EVM_INIT_X509;
>>  }
>>  #endif
>>
>> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
>> index cf12a04..3f775df 100644
>> --- a/security/integrity/evm/evm_secfs.c
>> +++ b/security/integrity/evm/evm_secfs.c
>> @@ -64,7 +64,7 @@ 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_INIT_HMAC))
>>               return -EPERM;
>>
>>       if (count >= sizeof(temp) || count == 0)
>> @@ -80,7 +80,7 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
>>
>>       error = evm_init_key();
>>       if (!error) {
>> -             evm_initialized = 1;
>> +             evm_initialized |= EVM_INIT_HMAC;
>>               pr_info("initialized\n");
>>       } else
>>               pr_err("initialization failed\n");
>
>



-- 
Thanks,
Dmitry

Re: [PATCHv3 4/6] evm: provide a function to set EVM key from the kernel

[Dmitry Kasatkin]

Hi,

Updated in the patch.

http://git.kernel.org/cgit/linux/kernel/git/kasatkin/linux-digsig.git/log/?h=ima-next

Dmitry

On Fri, Oct 23, 2015 at 9:30 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
>> Crypto HW kernel module can possibly initialize EVM key from the
>> kernel __init code to enable EVM before calling 'init' process.
>> This patch provide a function evm_set_key() which can be used to
>> set custom key directly to EVM without using KEY subsystem.
>
> Thanks, Dmitry.  There's a minor comment inline.
>>
>> Changes in v3:
>> * error reporting moved to evm_set_key
>> * EVM_INIT_HMAC moved to evm_set_key
>> * added bitop to prevent key setting race
>>
>> Changes in v2:
>> * use size_t for key size instead of signed int
>> * provide EVM_MAX_KEY_SIZE macro in <linux/evm.h>
>> * provide EVM_MIN_KEY_SIZE macro in <linux/evm.h>
>>
>> Signed-off-by: Dmitry Kasatkin <dmitry.kasatkin@huawei.com>
>> ---
>>  include/linux/evm.h                 |  7 ++++++
>>  security/integrity/evm/evm_crypto.c | 47 +++++++++++++++++++++++++++++++------
>>  security/integrity/evm/evm_secfs.c  | 10 +++-----
>>  3 files changed, 50 insertions(+), 14 deletions(-)
>>
>> diff --git a/include/linux/evm.h b/include/linux/evm.h
>> index 1fcb88c..35ed9a8 100644
>> --- a/include/linux/evm.h
>> +++ b/include/linux/evm.h
>> @@ -14,6 +14,7 @@
>>  struct integrity_iint_cache;
>>
>>  #ifdef CONFIG_EVM
>> +extern int evm_set_key(void *key, size_t keylen);
>>  extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>>                                            const char *xattr_name,
>>                                            void *xattr_value,
>> @@ -42,6 +43,12 @@ static inline int posix_xattr_acl(const char *xattrname)
>>  }
>>  #endif
>>  #else
>> +
>> +static inline int evm_set_key(void *key, size_t keylen)
>> +{
>> +     return -EOPNOTSUPP;
>> +}
>> +
>>  #ifdef CONFIG_INTEGRITY
>>  static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>>                                                   const char *xattr_name,
>> diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
>> index 34e1a6f..7aec93e 100644
>> --- a/security/integrity/evm/evm_crypto.c
>> +++ b/security/integrity/evm/evm_crypto.c
>> @@ -18,6 +18,7 @@
>>  #include <linux/module.h>
>>  #include <linux/crypto.h>
>>  #include <linux/xattr.h>
>> +#include <linux/evm.h>
>>  #include <keys/encrypted-type.h>
>>  #include <crypto/hash.h>
>>  #include "evm.h"
>> @@ -32,6 +33,41 @@ struct crypto_shash *hash_tfm;
>>
>>  static DEFINE_MUTEX(mutex);
>>
>> +#define EVM_SET_KEY_BUSY 0
>> +
>> +static unsigned long evm_set_key_flags;
>> +
>> +/* evm_set_key - set EVM HMAC key from the kernel
>> + *
>
> For exported functions this should be "kernel-doc" format.
>
> Mimi
>
>> + * This function allows to set EVM HMAC key from the kernel
>> + * without using key subsystem 'encrypted' keys. It can be used
>> + * by the crypto HW kernel module which has own way of managing
>> + * keys.
>> + *
>> + * key length should be between 32 and 128 bytes long
>> + */
>> +int evm_set_key(void *key, size_t keylen)
>> +{
>> +     int rc;
>> +
>> +     rc = -EBUSY;
>> +     if (test_and_set_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags))
>> +             goto busy;
>> +     rc = -EINVAL;
>> +     if (keylen > MAX_KEY_SIZE)
>> +             goto inval;
>> +     memcpy(evmkey, key, keylen);
>> +     evm_initialized |= EVM_INIT_HMAC;
>> +     pr_info("key initialized\n");
>> +     return 0;
>> +inval:
>> +     clear_bit(EVM_SET_KEY_BUSY, &evm_set_key_flags);
>> +busy:
>> +     pr_err("key initialization failed\n");
>> +     return rc;
>> +}
>> +EXPORT_SYMBOL_GPL(evm_set_key);
>> +
>>  static struct shash_desc *init_desc(char type)
>>  {
>>       long rc;
>> @@ -242,7 +278,7 @@ int evm_init_key(void)
>>  {
>>       struct key *evm_key;
>>       struct encrypted_key_payload *ekp;
>> -     int rc = 0;
>> +     int rc;
>>
>>       evm_key = request_key(&key_type_encrypted, EVMKEY, NULL);
>>       if (IS_ERR(evm_key))
>> @@ -250,12 +286,9 @@ int evm_init_key(void)
>>
>>       down_read(&evm_key->sem);
>>       ekp = evm_key->payload.data;
>> -     if (ekp->decrypted_datalen > MAX_KEY_SIZE) {
>> -             rc = -EINVAL;
>> -             goto out;
>> -     }
>> -     memcpy(evmkey, ekp->decrypted_data, ekp->decrypted_datalen);
>> -out:
>> +
>> +     rc = evm_set_key(ekp->decrypted_data, ekp->decrypted_datalen);
>> +
>>       /* burn the original key contents */
>>       memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
>>       up_read(&evm_key->sem);
>> diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
>> index 3f775df..c8dccd5 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_INIT_HMAC))
>>               return -EPERM;
>> @@ -78,12 +78,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 |= EVM_INIT_HMAC;
>> -             pr_info("initialized\n");
>> -     } else
>> -             pr_err("initialization failed\n");
>> +     evm_init_key();
>> +
>>       return count;
>>  }
>>
>
>



-- 
Thanks,
Dmitry

Re: [PATCHv3 0/6] integrity: few EVM patches

[Mimi Zohar]

On Thu, 2015-10-22 at 21:49 +0300, Dmitry Kasatkin wrote:
> Hi,
> 
> IMA module provides functionality to load x509 certificates into the
> trusted '.ima' keyring. This is patchset adds the same functionality
> to the EVM as well. Also it provides functionality to set EVM key from
> the kernel crypto HW driver. This is an update for the patchset which was
> previously sent for review few months ago. Please refer to the patch
> descriptions for details.

Other than patch "evm: define EVM key max and min sizes", which prevents
existing EVM keys from being loaded, the patches are queued
http://git.kernel.org/cgit/linux/kernel/git/zohar/linux-integrity.git/next-for-4.5.

Thanks!

Mimi

> BR,
>  
> Dmitry
> 
> Dmitry Kasatkin (6):
>   integrity: define '.evm' as a builtin 'trusted' keyring
>   evm: load x509 certificate from the kernel
>   evm: enable EVM when X509 certificate is loaded
>   evm: provide a function to set EVM key from the kernel
>   evm: define EVM key max and min sizes
>   evm: reset EVM status when file attributes changes
> 
>  include/linux/evm.h                 | 10 +++++++
>  security/integrity/Kconfig          | 11 ++++++++
>  security/integrity/digsig.c         | 14 ++++++++--
>  security/integrity/evm/Kconfig      | 17 ++++++++++++
>  security/integrity/evm/evm.h        |  3 +++
>  security/integrity/evm/evm_crypto.c | 54 ++++++++++++++++++++++++++++++-------
>  security/integrity/evm/evm_main.c   | 32 +++++++++++++++++++---
>  security/integrity/evm/evm_secfs.c  | 12 +++------
>  security/integrity/iint.c           |  1 +
>  security/integrity/ima/Kconfig      |  5 +++-
>  security/integrity/ima/ima.h        | 12 ---------
>  security/integrity/ima/ima_init.c   |  2 +-
>  security/integrity/integrity.h      | 13 ++++++---
>  13 files changed, 146 insertions(+), 40 deletions(-)
>