Skip to contentSkip to conversation

integrity: few EVM patches

PATCH In review Dmitry Kasatkin 4 participants 16 messages Oct 22, 2015 – Nov 5, 2015

Subsystems (3)
Security (LSM) Integrity measurement architecture (IMA) Extended Verification Module (EVM)
lore
Share & tools
lore

Dmitry Kasatkin

author maintainer
#0 lore raw

v3 cover integrity: few EVM patches

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(-)
Signature · 1 line
2.1.4
author maintainer
#1 lore raw

PATCH v3 1/6 integrity: define '.evm' as a builtin 'trusted' keyring

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 <[email protected]>
---
 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(-)
security/integrity/Kconfig +11 -0
index 73c457b..21d7568 100644
@@ -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
security/integrity/digsig.c +12 -2
index 5be9ffb..8ef1511 100644
@@ -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) |
security/integrity/evm/evm_main.c +5 -3
index 1334e02..75b7e30 100644
@@ -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;
 }
 
 /*
security/integrity/ima/Kconfig +4 -1
index df30334..a292b88 100644
@@ -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
security/integrity/ima/ima.h +0 -12
index e2a60c3..9e82367 100644
@@ -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
security/integrity/ima/ima_init.c +1 -1
index e600cad..bd79f25 100644
@@ -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;
security/integrity/integrity.h +2 -3
index 9c61687..07726a7 100644
@@ -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
Signature · 1 line
2.1.4
#2 lore raw

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

Quoted 126 lines · 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 <[email protected]>
---
 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(-)
security/integrity/Kconfig +11 -0
index 73c457b..21d7568 100644
@@ -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
security/integrity/digsig.c +12 -2
index 5be9ffb..8ef1511 100644
@@ -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) |
security/integrity/evm/evm_main.c +5 -3
index 1334e02..75b7e30 100644
@@ -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;
 }
 
 /*
security/integrity/ima/Kconfig +2 -1
index df30334..a292b88 100644
@@ -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

security/integrity/ima/Kconfig config IMA_APPRAISE · line 139

Quoted 10 lines
security/integrity/ima/Kconfig +2 -0
 	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.

Quoted 59 lines
security/integrity/ima/ima.h +0 -12
index e2a60c3..9e82367 100644
@@ -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
security/integrity/ima/ima_init.c +1 -1
index e600cad..bd79f25 100644
@@ -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;
security/integrity/integrity.h +2 -3
index 9c61687..07726a7 100644
@@ -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
Signature · 1 line
2.1.4

ACK to the rest of the code.

Petko

#3 lore raw

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

________________________________________
From: Petko Manolov [[email protected]]
Sent: Friday, October 23, 2015 4:05 PM
To: Dmitry Kasatkin
Cc: [email protected]; [email protected]; [email protected]; [email protected]; Dmitry Kasatkin
Subject: Re: [PATCHv3 1/6] integrity: define '.evm' as a builtin 'trusted' keyring

Quoted 126 lines · 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 <[email protected]>
---
 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(-)
security/integrity/Kconfig +11 -0
index 73c457b..21d7568 100644
@@ -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
security/integrity/digsig.c +12 -2
index 5be9ffb..8ef1511 100644
@@ -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) |
security/integrity/evm/evm_main.c +5 -3
index 1334e02..75b7e30 100644
@@ -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;
 }
 
 /*
security/integrity/ima/Kconfig +2 -1
index df30334..a292b88 100644
@@ -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

security/integrity/ima/Kconfig config IMA_APPRAISE · line 139

Quoted 10 lines
security/integrity/ima/Kconfig +2 -0
      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

Quoted 58 lines
security/integrity/ima/ima.h +0 -12
index e2a60c3..9e82367 100644
@@ -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
security/integrity/ima/ima_init.c +1 -1
index e600cad..bd79f25 100644
@@ -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;
security/integrity/integrity.h +2 -4
index 9c61687..07726a7 100644
@@ -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
--
Quoted 1 line

2.1.4

ACK to the rest of the code.

Petko

#4 lore raw

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

In reply to Petko Manolov
Quoted 20 lines · Petko Manolov
Quoted 18 lines · Dmitry Kasatkin
security/integrity/ima/Kconfig +2 -1
index df30334..a292b88 100644
@@ -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.
Quoted 9 lines

security/integrity/ima/Kconfig config IMA_APPRAISE · line 139

Quoted 5 lines
security/integrity/ima/Kconfig +2 -0
+	   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

#5 lore raw

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

Quoted 31 lines · Mimi Zohar
Quoted 20 lines · Petko Manolov
Quoted 18 lines · Dmitry Kasatkin
security/integrity/ima/Kconfig +2 -1
index df30334..a292b88 100644
@@ -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.
Quoted 9 lines

security/integrity/ima/Kconfig config IMA_APPRAISE · line 139

Quoted 5 lines
security/integrity/ima/Kconfig +2 -0
+	   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.

Quoted 3 lines

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

author maintainer
#6 lore raw

PATCH v3 2/6 evm: load x509 certificate from the kernel

In reply to 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 <[email protected]>
---
 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(+)
security/integrity/evm/Kconfig +17 -0
index bf19723..b1433e9 100644
@@ -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.
security/integrity/evm/evm_main.c +7 -0
index 75b7e30..519de0a 100644
@@ -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;
security/integrity/iint.c +1 -0
index 3d2f5b4..2de9c82 100644
@@ -254,4 +254,5 @@ out:
 void __init integrity_load_keys(void)
 {
 	ima_load_x509();
+	evm_load_x509();
 }
security/integrity/integrity.h +8 -0
index 07726a7..5efe2ec 100644
@@ -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,
Signature · 1 line
2.1.4
author maintainer
#7 lore raw

PATCH v3 3/6 evm: enable EVM when X509 certificate is loaded

In reply to 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 <[email protected]>
---
 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(-)
security/integrity/evm/evm.h +3 -0
index 88bfe77..f5f1272 100644
@@ -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;
security/integrity/evm/evm_crypto.c +2 -0
index 159ef3e..34e1a6f 100644
@@ -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 {
security/integrity/evm/evm_main.c +5 -1
index 519de0a..420d94d 100644
@@ -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
security/integrity/evm/evm_secfs.c +2 -2
index cf12a04..3f775df 100644
@@ -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");
Signature · 1 line
2.1.4
#8 lore raw

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

Quoted 8 lines · 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.

Thanks, Dmitry. There's one comment inline.

Quoted 31 lines

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 <[email protected]>
---
 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(-)
security/integrity/evm/evm.h +3 -0
index 88bfe77..f5f1272 100644
@@ -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;
security/integrity/evm/evm_crypto.c +0 -0
index 159ef3e..34e1a6f 100644

security/integrity/evm/evm_crypto.c static struct shash_desc *init_desc(char type) · line 44

Quoted 5 lines
security/integrity/evm/evm_crypto.c +2 -0
@@ -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

Quoted 42 lines
security/integrity/evm/evm_crypto.c +0 -0
 		tfm = &hmac_tfm;
 		algo = evm_hmac;
 	} else {
security/integrity/evm/evm_main.c +5 -1
index 519de0a..420d94d 100644
@@ -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
security/integrity/evm/evm_secfs.c +2 -2
index cf12a04..3f775df 100644
@@ -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");
author maintainer
#9 lore raw

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

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

Quoted 102 lines · Mimi Zohar
Quoted 8 lines · 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.

Thanks, Dmitry. There's one comment inline.

Quoted 31 lines

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 <[email protected]>
---
 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(-)
security/integrity/evm/evm.h +3 -0
index 88bfe77..f5f1272 100644
@@ -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;
security/integrity/evm/evm_crypto.c +0 -0
index 159ef3e..34e1a6f 100644

security/integrity/evm/evm_crypto.c static struct shash_desc *init_desc(char type) · line 44

Quoted 5 lines
security/integrity/evm/evm_crypto.c +2 -0
@@ -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

Quoted 42 lines
security/integrity/evm/evm_crypto.c +0 -0
              tfm = &hmac_tfm;
              algo = evm_hmac;
      } else {
security/integrity/evm/evm_main.c +5 -1
index 519de0a..420d94d 100644
@@ -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
security/integrity/evm/evm_secfs.c +2 -2
index cf12a04..3f775df 100644
@@ -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");
Signature · 2 lines
Thanks,
Dmitry
author maintainer
#10 lore raw

PATCH v3 4/6 evm: provide a function to set EVM key from the kernel

In reply to 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 <[email protected]>
---
 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(-)
include/linux/evm.h +7 -0
index 1fcb88c..35ed9a8 100644
@@ -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,
security/integrity/evm/evm_crypto.c +40 -7

71 lines not on this page · raw message

security/integrity/evm/evm_secfs.c +3 -7
index 3f775df..c8dccd5 100644
@@ -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;
 }
Signature · 1 line
2.1.4
#11 lore raw

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

Quoted 4 lines · 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.

Thanks, Dmitry. There's a minor comment inline.

Quoted 56 lines

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 <[email protected]>
---
 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(-)
include/linux/evm.h +7 -0
index 1fcb88c..35ed9a8 100644
@@ -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,
security/integrity/evm/evm_crypto.c +1 -0
index 34e1a6f..7aec93e 100644
@@ -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"

security/integrity/evm/evm_crypto.c struct crypto_shash *hash_tfm; · line 41

Quoted 9 lines
security/integrity/evm/evm_crypto.c +6 -0
@@ -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

Quoted 85 lines
security/integrity/evm/evm_crypto.c +33 -7
+ * 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);
security/integrity/evm/evm_secfs.c +3 -7
index 3f775df..c8dccd5 100644
@@ -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;
 }
author maintainer
#12 lore raw

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

Hi,

Updated in the patch.

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

Dmitry

Quoted 164 lines · Mimi Zohar
Quoted 4 lines · 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.

Thanks, Dmitry. There's a minor comment inline.

Quoted 56 lines

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 <[email protected]>
---
 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(-)
include/linux/evm.h +7 -0
index 1fcb88c..35ed9a8 100644
@@ -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,
security/integrity/evm/evm_crypto.c +1 -0
index 34e1a6f..7aec93e 100644
@@ -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"

security/integrity/evm/evm_crypto.c struct crypto_shash *hash_tfm; · line 41

Quoted 9 lines
security/integrity/evm/evm_crypto.c +6 -0
@@ -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

Quoted 85 lines
security/integrity/evm/evm_crypto.c +33 -7
+ * 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);
security/integrity/evm/evm_secfs.c +3 -7
index 3f775df..c8dccd5 100644
@@ -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;
 }
Signature · 2 lines
Thanks,
Dmitry
author maintainer
#13 lore raw

PATCH v3 5/6 evm: define EVM key max and min sizes

In reply to 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 <[email protected]>
---
 include/linux/evm.h                 | 3 +++
 security/integrity/evm/evm_crypto.c | 7 +++----
 2 files changed, 6 insertions(+), 4 deletions(-)
include/linux/evm.h +3 -0
index 35ed9a8..0aeedec 100644
@@ -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
security/integrity/evm/evm_crypto.c +3 -4
index 7aec93e..cfb788c 100644
@@ -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;
Signature · 1 line
2.1.4
author maintainer
#14 lore raw

PATCH v3 6/6 evm: reset EVM status when file attributes changes

In reply to 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 <[email protected]>
---
 security/integrity/evm/evm_main.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)
security/integrity/evm/evm_main.c +13 -0
index 420d94d..f716025 100644
@@ -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);
 }
Signature · 1 line
2.1.4
#15 lore raw
In reply to Dmitry Kasatkin
Quoted 8 lines · 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.

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

Quoted 27 lines

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(-)