in-kernel IMA/EVM initialization

---
 crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

crypto/asymmetric_keys/x509_public_key.c +84 -1

index 382ef0d..d279f43 100644
@@ -18,6 +18,7 @@
 #include <linux/asn1_decoder.h>
 #include <keys/asymmetric-subtype.h>
 #include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
 #include <crypto/hash.h>
 #include "asymmetric_keys.h"
 #include "public_key.h"

@@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
 }
 EXPORT_SYMBOL_GPL(x509_check_signature);
 
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+/*
+ * Find a key in the given keyring by issuer and authority.
+ */
+static struct key *x509_request_asymmetric_key(
+	struct key *keyring,
+	const char *signer, size_t signer_len,
+	const char *authority, size_t auth_len)
+{
+	key_ref_t key;
+	char *id;
+
+	/* Construct an identifier. */
+	id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
+	if (!id)
+		return ERR_PTR(-ENOMEM);
+
+	memcpy(id, signer, signer_len);
+	id[signer_len + 0] = ':';
+	id[signer_len + 1] = ' ';
+	memcpy(id + signer_len + 2, authority, auth_len);
+	id[signer_len + 2 + auth_len] = 0;
+
+	pr_debug("Look up: \"%s\"\n", id);
+
+	key = keyring_search(make_key_ref(keyring, 1),
+			     &key_type_asymmetric, id);
+	if (IS_ERR(key))
+		pr_debug("Request for module key '%s' err %ld\n",
+			 id, PTR_ERR(key));
+	kfree(id);
+
+	if (IS_ERR(key)) {
+		switch (PTR_ERR(key)) {
+			/* Hide some search errors */
+		case -EACCES:
+		case -ENOTDIR:
+		case -EAGAIN:
+			return ERR_PTR(-ENOKEY);
+		default:
+			return ERR_CAST(key);
+		}
+	}
+
+	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
+	return key_ref_to_ptr(key);
+}
+
+/*
+ * Check the new certificate against the ones in the trust keyring.  If one of
+ * those is the signing key and validates the new certificate, then mark the
+ * new certificate as being trusted.
+ *
+ * Return 0 if the new certificate was successfully validated, 1 if we couldn't
+ * find a matching parent certificate in the trusted list and an error if there
+ * is a matching certificate but the signature check fails.
+ */
+static int x509_validate_trust(struct x509_certificate *cert,
+			       struct key *trust_keyring)
+{
+	const struct public_key *pk;
+	struct key *key;
+	int ret = 1;
+
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->issuer, strlen(cert->issuer),
+					  cert->authority,
+					  strlen(cert->authority));
+	if (!IS_ERR(key))  {
+		pk = key->payload.data;
+		ret = x509_check_signature(pk, cert);
+	}
+	return ret;
+}
+#endif
+
 /*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */

@@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	/* Check the signature on the key if it appears to be self-signed */
 	if (!cert->authority ||
 	    strcmp(cert->fingerprint, cert->authority) == 0) {
-		ret = x509_check_signature(cert->pub, cert);
+		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
+	} else {
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+		ret = x509_validate_trust(cert, system_trusted_keyring);
+		if (!ret)
+			prep->trusted = 1;
+#endif
 	}
 
 	/* Propose a description */

Signature · 1 line

1.8.3.2

Mimi Zohar

2014-04-24 16:54 UTC #2 lore raw

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

Quoted 7 lines · Dmitry Kasatkin

From: Mimi Zohar <[email protected]>
Only public keys, with certificates signed by an existing 'trusted' key on the system trusted keyring, should be added to a trusted keyring. This patch adds support for verifying a certificate's signature.

Quoted 5 lines

This is derived from David Howells pkcs7_request_asymmetric_key() patch.
Changes:
- Flaged out the code to prevent build break if system keyring is not enabled (Dmitry).

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

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

thanks,

Mimi

Quoted 120 lines

Signed-off-by Mimi Zohar <[email protected]>

---
 crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

crypto/asymmetric_keys/x509_public_key.c +84 -1

index 382ef0d..d279f43 100644
@@ -18,6 +18,7 @@
 #include <linux/asn1_decoder.h>
 #include <keys/asymmetric-subtype.h>
 #include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
 #include <crypto/hash.h>
 #include "asymmetric_keys.h"
 #include "public_key.h"

@@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
 }
 EXPORT_SYMBOL_GPL(x509_check_signature);
 
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+/*
+ * Find a key in the given keyring by issuer and authority.
+ */
+static struct key *x509_request_asymmetric_key(
+	struct key *keyring,
+	const char *signer, size_t signer_len,
+	const char *authority, size_t auth_len)
+{
+	key_ref_t key;
+	char *id;
+
+	/* Construct an identifier. */
+	id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
+	if (!id)
+		return ERR_PTR(-ENOMEM);
+
+	memcpy(id, signer, signer_len);
+	id[signer_len + 0] = ':';
+	id[signer_len + 1] = ' ';
+	memcpy(id + signer_len + 2, authority, auth_len);
+	id[signer_len + 2 + auth_len] = 0;
+
+	pr_debug("Look up: \"%s\"\n", id);
+
+	key = keyring_search(make_key_ref(keyring, 1),
+			     &key_type_asymmetric, id);
+	if (IS_ERR(key))
+		pr_debug("Request for module key '%s' err %ld\n",
+			 id, PTR_ERR(key));
+	kfree(id);
+
+	if (IS_ERR(key)) {
+		switch (PTR_ERR(key)) {
+			/* Hide some search errors */
+		case -EACCES:
+		case -ENOTDIR:
+		case -EAGAIN:
+			return ERR_PTR(-ENOKEY);
+		default:
+			return ERR_CAST(key);
+		}
+	}
+
+	pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
+	return key_ref_to_ptr(key);
+}
+
+/*
+ * Check the new certificate against the ones in the trust keyring.  If one of
+ * those is the signing key and validates the new certificate, then mark the
+ * new certificate as being trusted.
+ *
+ * Return 0 if the new certificate was successfully validated, 1 if we couldn't
+ * find a matching parent certificate in the trusted list and an error if there
+ * is a matching certificate but the signature check fails.
+ */
+static int x509_validate_trust(struct x509_certificate *cert,
+			       struct key *trust_keyring)
+{
+	const struct public_key *pk;
+	struct key *key;
+	int ret = 1;
+
+	key = x509_request_asymmetric_key(trust_keyring,
+					  cert->issuer, strlen(cert->issuer),
+					  cert->authority,
+					  strlen(cert->authority));
+	if (!IS_ERR(key))  {
+		pk = key->payload.data;
+		ret = x509_check_signature(pk, cert);
+	}
+	return ret;
+}
+#endif
+
 /*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */

@@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
 	/* Check the signature on the key if it appears to be self-signed */
 	if (!cert->authority ||
 	    strcmp(cert->fingerprint, cert->authority) == 0) {
-		ret = x509_check_signature(cert->pub, cert);
+		ret = x509_check_signature(cert->pub, cert); /* self-signed */
 		if (ret < 0)
 			goto error_free_cert;
+	} else {
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+		ret = x509_validate_trust(cert, system_trusted_keyring);
+		if (!ret)
+			prep->trusted = 1;
+#endif
 	}
 
 	/* Propose a description */

Dmitry Kasatkin

maintainer

2014-04-24 20:07 UTC #3 lore raw

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

Quoted 14 lines · Mimi Zohar

Quoted 7 lines · Dmitry Kasatkin
From: Mimi Zohar <[email protected]>
Only public keys, with certificates signed by an existing 'trusted' key on the system trusted keyring, should be added to a trusted keyring. This patch adds support for verifying a certificate's signature.
Quoted 5 lines
This is derived from David Howells pkcs7_request_asymmetric_key() patch.
Changes:
- Flaged out the code to prevent build break if system keyring is not enabled (Dmitry).

Quoted 6 lines

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

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

Quoted 3 lines

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

Right, we know it.

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

- Dmitry

Quoted 130 lines

thanks,

Mimi

Quoted 110 lines

Signed-off-by Mimi Zohar <[email protected]>

---
 crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

crypto/asymmetric_keys/x509_public_key.c +78 -1

index 382ef0d..d279f43 100644
@@ -18,6 +18,7 @@
 #include <linux/asn1_decoder.h>
 #include <keys/asymmetric-subtype.h>
 #include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
 #include <crypto/hash.h>
 #include "asymmetric_keys.h"
 #include "public_key.h"

@@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
 }
 EXPORT_SYMBOL_GPL(x509_check_signature);
 
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+/*
+ * Find a key in the given keyring by issuer and authority.
+ */
+static struct key *x509_request_asymmetric_key(
+     struct key *keyring,
+     const char *signer, size_t signer_len,
+     const char *authority, size_t auth_len)
+{
+     key_ref_t key;
+     char *id;
+
+     /* Construct an identifier. */
+     id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
+     if (!id)
+             return ERR_PTR(-ENOMEM);
+
+     memcpy(id, signer, signer_len);
+     id[signer_len + 0] = ':';
+     id[signer_len + 1] = ' ';
+     memcpy(id + signer_len + 2, authority, auth_len);
+     id[signer_len + 2 + auth_len] = 0;
+
+     pr_debug("Look up: \"%s\"\n", id);
+
+     key = keyring_search(make_key_ref(keyring, 1),
+                          &key_type_asymmetric, id);
+     if (IS_ERR(key))
+             pr_debug("Request for module key '%s' err %ld\n",
+                      id, PTR_ERR(key));
+     kfree(id);
+
+     if (IS_ERR(key)) {
+             switch (PTR_ERR(key)) {
+                     /* Hide some search errors */
+             case -EACCES:
+             case -ENOTDIR:
+             case -EAGAIN:
+                     return ERR_PTR(-ENOKEY);
+             default:
+                     return ERR_CAST(key);
+             }
+     }
+
+     pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
+     return key_ref_to_ptr(key);
+}
+
+/*
+ * Check the new certificate against the ones in the trust keyring.  If one of
+ * those is the signing key and validates the new certificate, then mark the
+ * new certificate as being trusted.
+ *
+ * Return 0 if the new certificate was successfully validated, 1 if we couldn't
+ * find a matching parent certificate in the trusted list and an error if there
+ * is a matching certificate but the signature check fails.
+ */
+static int x509_validate_trust(struct x509_certificate *cert,
+                            struct key *trust_keyring)
+{
+     const struct public_key *pk;
+     struct key *key;
+     int ret = 1;
+
+     key = x509_request_asymmetric_key(trust_keyring,
+                                       cert->issuer, strlen(cert->issuer),
+                                       cert->authority,
+                                       strlen(cert->authority));
+     if (!IS_ERR(key))  {
+             pk = key->payload.data;
+             ret = x509_check_signature(pk, cert);
+     }
+     return ret;
+}
+#endif
+
 /*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */

@@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
      /* Check the signature on the key if it appears to be self-signed */
      if (!cert->authority ||
          strcmp(cert->fingerprint, cert->authority) == 0) {
-             ret = x509_check_signature(cert->pub, cert);
+             ret = x509_check_signature(cert->pub, cert); /* self-signed */
              if (ret < 0)

crypto/asymmetric_keys/x509_public_key.c static int x509_key_preparse(struct key_preparsed_payload *prep) · line 246

Quoted 10 lines

crypto/asymmetric_keys/x509_public_key.c +6 -0

                      goto error_free_cert;
+     } else {
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+             ret = x509_validate_trust(cert, system_trusted_keyring);
+             if (!ret)
+                     prep->trusted = 1;
+#endif
      }
 
      /* Propose a description */

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Thanks,
Dmitry

Mimi Zohar

2014-04-24 21:04 UTC #4 lore raw

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

Quoted 22 lines · Dmitry Kasatkin

Quoted 14 lines · Mimi Zohar
Quoted 7 lines · Dmitry Kasatkin
From: Mimi Zohar <[email protected]>
Only public keys, with certificates signed by an existing 'trusted' key on the system trusted keyring, should be added to a trusted keyring. This patch adds support for verifying a certificate's signature.
Quoted 5 lines
This is derived from David Howells pkcs7_request_asymmetric_key() patch.
Changes:
- Flaged out the code to prevent build break if system keyring is not enabled (Dmitry).
Quoted 6 lines
An updated version of this patch was posted, which resolves the Kconfig issues. There are a number of other issues which need to be addressed, before this patch can be upstreamed. Please refer to the patch description for more details -
http://marc.info/?l=linux-security-module&m=138672063109662&w=2

Quoted 3 lines

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

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

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

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

thanks,

Mimi

Quoted 143 lines

Quoted 3 lines

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

Right, we know it.

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

- Dmitry

Quoted 130 lines

thanks,

Mimi

Quoted 110 lines

Signed-off-by Mimi Zohar <[email protected]>

---
 crypto/asymmetric_keys/x509_public_key.c | 85 +++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

crypto/asymmetric_keys/x509_public_key.c +78 -1

index 382ef0d..d279f43 100644
@@ -18,6 +18,7 @@
 #include <linux/asn1_decoder.h>
 #include <keys/asymmetric-subtype.h>
 #include <keys/asymmetric-parser.h>
+#include <keys/system_keyring.h>
 #include <crypto/hash.h>
 #include "asymmetric_keys.h"
 #include "public_key.h"

@@ -102,6 +103,82 @@ int x509_check_signature(const struct public_key *pub,
 }
 EXPORT_SYMBOL_GPL(x509_check_signature);
 
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+/*
+ * Find a key in the given keyring by issuer and authority.
+ */
+static struct key *x509_request_asymmetric_key(
+     struct key *keyring,
+     const char *signer, size_t signer_len,
+     const char *authority, size_t auth_len)
+{
+     key_ref_t key;
+     char *id;
+
+     /* Construct an identifier. */
+     id = kmalloc(signer_len + 2 + auth_len + 1, GFP_KERNEL);
+     if (!id)
+             return ERR_PTR(-ENOMEM);
+
+     memcpy(id, signer, signer_len);
+     id[signer_len + 0] = ':';
+     id[signer_len + 1] = ' ';
+     memcpy(id + signer_len + 2, authority, auth_len);
+     id[signer_len + 2 + auth_len] = 0;
+
+     pr_debug("Look up: \"%s\"\n", id);
+
+     key = keyring_search(make_key_ref(keyring, 1),
+                          &key_type_asymmetric, id);
+     if (IS_ERR(key))
+             pr_debug("Request for module key '%s' err %ld\n",
+                      id, PTR_ERR(key));
+     kfree(id);
+
+     if (IS_ERR(key)) {
+             switch (PTR_ERR(key)) {
+                     /* Hide some search errors */
+             case -EACCES:
+             case -ENOTDIR:
+             case -EAGAIN:
+                     return ERR_PTR(-ENOKEY);
+             default:
+                     return ERR_CAST(key);
+             }
+     }
+
+     pr_devel("<==%s() = 0 [%x]\n", __func__, key_serial(key_ref_to_ptr(key)));
+     return key_ref_to_ptr(key);
+}
+
+/*
+ * Check the new certificate against the ones in the trust keyring.  If one of
+ * those is the signing key and validates the new certificate, then mark the
+ * new certificate as being trusted.
+ *
+ * Return 0 if the new certificate was successfully validated, 1 if we couldn't
+ * find a matching parent certificate in the trusted list and an error if there
+ * is a matching certificate but the signature check fails.
+ */
+static int x509_validate_trust(struct x509_certificate *cert,
+                            struct key *trust_keyring)
+{
+     const struct public_key *pk;
+     struct key *key;
+     int ret = 1;
+
+     key = x509_request_asymmetric_key(trust_keyring,
+                                       cert->issuer, strlen(cert->issuer),
+                                       cert->authority,
+                                       strlen(cert->authority));
+     if (!IS_ERR(key))  {
+             pk = key->payload.data;
+             ret = x509_check_signature(pk, cert);
+     }
+     return ret;
+}
+#endif
+
 /*
  * Attempt to parse a data blob for a key as an X509 certificate.
  */

@@ -155,9 +232,15 @@ static int x509_key_preparse(struct key_preparsed_payload *prep)
      /* Check the signature on the key if it appears to be self-signed */
      if (!cert->authority ||
          strcmp(cert->fingerprint, cert->authority) == 0) {
-             ret = x509_check_signature(cert->pub, cert);
+             ret = x509_check_signature(cert->pub, cert); /* self-signed */
              if (ret < 0)

crypto/asymmetric_keys/x509_public_key.c static int x509_key_preparse(struct key_preparsed_payload *prep) · line 246

Quoted 10 lines

crypto/asymmetric_keys/x509_public_key.c +6 -0

                      goto error_free_cert;
+     } else {
+#ifdef CONFIG_SYSTEM_TRUSTED_KEYRING
+             ret = x509_validate_trust(cert, system_trusted_keyring);
+             if (!ret)
+                     prep->trusted = 1;
+#endif
      }
 
      /* Propose a description */

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Dmitry Kasatkin

author

2014-04-23 13:58 UTC #5 lore raw

PATCH 2/20 integrity: initialize EVM before IMA

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

---
 security/integrity/Makefile | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

security/integrity/Makefile +2 -2

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

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:44 UTC #6 lore raw

PATCH 3/20 ima: move asymmetric keys config option

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

---
 security/integrity/Kconfig | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

security/integrity/Kconfig +12 ~~-12~~

index 245c6d9..f79d853 100644
@@ -17,6 +17,18 @@ config INTEGRITY_SIGNATURE
 	  This is useful for evm and module keyrings, when keys are
 	  usually only added from initramfs.
 
+config INTEGRITY_ASYMMETRIC_KEYS
+	boolean "Enable asymmetric keys support"
+	depends on INTEGRITY_SIGNATURE
+	default n
+        select ASYMMETRIC_KEY_TYPE
+        select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
+        select PUBLIC_KEY_ALGO_RSA
+        select X509_CERTIFICATE_PARSER
+	help
+	  This option enables digital signature verification using
+	  asymmetric keys.
+
 config INTEGRITY_AUDIT
 	bool "Enables integrity auditing support "
 	depends on INTEGRITY && AUDIT

@@ -32,17 +44,5 @@ config INTEGRITY_AUDIT
 	  be enabled by specifying 'integrity_audit=1' on the kernel
 	  command line.
 
-config INTEGRITY_ASYMMETRIC_KEYS
-	boolean "Enable asymmetric keys support"
-	depends on INTEGRITY_SIGNATURE
-	default n
-        select ASYMMETRIC_KEY_TYPE
-        select ASYMMETRIC_PUBLIC_KEY_SUBTYPE
-        select PUBLIC_KEY_ALGO_RSA
-        select X509_CERTIFICATE_PARSER
-	help
-	  This option enables digital signature verification using
-	  asymmetric keys.
-
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:44 UTC #7 lore raw

PATCH 4/20 integrity: move integrity subsystem options to a separate menu

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

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

---
 security/integrity/Kconfig     | 11 ++++++++---
 security/integrity/evm/Kconfig |  9 +--------
 security/integrity/ima/Kconfig |  3 +--
 3 files changed, 10 insertions(+), 13 deletions(-)

security/integrity/Kconfig +8 -3

index f79d853..b16c9cd 100644
@@ -1,7 +1,10 @@
 #
-config INTEGRITY
-	def_bool y
-	depends on IMA || EVM
+menuconfig INTEGRITY
+	bool "Integrity Subsystem"
+	depends on SECURITY
+	default y
+
+if INTEGRITY
 
 config INTEGRITY_SIGNATURE
 	boolean "Digital signature verification using multiple keyrings"

@@ -46,3 +49,5 @@ config INTEGRITY_AUDIT
 
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig
+
+endif   # if INTEGRITY

security/integrity/evm/Kconfig +1 -8

index d606f3d..df20a2f 100644
@@ -1,6 +1,6 @@
 config EVM
 	boolean "EVM support"
-	depends on SECURITY
+	depends on INTEGRITY
 	select KEYS
 	select ENCRYPTED_KEYS
 	select CRYPTO_HMAC

@@ -12,10 +12,6 @@ config EVM
 
 	  If you are unsure how to answer this question, answer N.
 
-if EVM
-
-menu "EVM options"
-
 config EVM_ATTR_FSUUID
 	bool "FSUUID (version 2)"
 	default y

@@ -47,6 +43,3 @@ config EVM_EXTRA_SMACK_XATTRS
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
-endmenu
-
-endif

security/integrity/ima/Kconfig +1 -2

index 81a2797..5f95e50 100644
@@ -2,8 +2,7 @@
 #
 config IMA
 	bool "Integrity Measurement Architecture(IMA)"
-	depends on SECURITY
-	select INTEGRITY
+	depends on INTEGRITY
 	select SECURITYFS
 	select CRYPTO
 	select CRYPTO_HMAC

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:29 UTC #8 lore raw

PATCH 5/20 integrity: provide builtin 'trusted' keyrings

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

---
 security/integrity/Kconfig     |  4 ++++
 security/integrity/digsig.c    | 31 +++++++++++++++++++++++++++++++
 security/integrity/integrity.h | 10 ++++++++++
 3 files changed, 45 insertions(+)

security/integrity/Kconfig +4 -0

index b16c9cd..89f226a 100644
@@ -47,6 +47,10 @@ config INTEGRITY_AUDIT
 	  be enabled by specifying 'integrity_audit=1' on the kernel
 	  command line.
 
+config INTEGRITY_TRUSTED_KEYRING
+	def_bool n
+	depends on IMA_TRUSTED_KEYRING || EVM_TRUSTED_KEYRING
+
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig

security/integrity/digsig.c +31 -0

index b4af4eb..45adc07 100644
@@ -13,7 +13,9 @@
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
 #include <linux/err.h>
+#include <linux/sched.h>
 #include <linux/rbtree.h>
+#include <linux/cred.h>
 #include <linux/key-type.h>
 #include <linux/digsig.h>

@@ -56,3 +58,32 @@ int integrity_digsig_verify(const unsigned int id, const char *sig, int siglen,
 
 	return -EOPNOTSUPP;
 }
+
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+int integrity_init_keyring(const unsigned int id)
+{
+	const struct cred *cred = current_cred();
+	const struct user_struct *user = cred->user;
+
+	pr_notice("initialize trusted keyring: %s\n", keyring_name[id]);
+
+	/* this function relies that init_root_keyring() was executed
+	 * in 'keys' subsystem, which is initialized before integrity
+	 */
+
+	keyring[id] = keyring_alloc(keyring_name[id], KUIDT_INIT(0),
+				    KGIDT_INIT(0), cred,
+				    ((KEY_POS_ALL & ~KEY_POS_SETATTR) |
+				     KEY_USR_VIEW | KEY_USR_READ),
+				    KEY_ALLOC_NOT_IN_QUOTA, user->uid_keyring);
+	if (IS_ERR(keyring[id])) {
+		long rc = PTR_ERR(keyring[id]);
+		pr_err("Can't allocate %s keyring (%ld)\n",
+		       keyring_name[id], rc);
+		keyring[id] = NULL;
+		return rc;
+	}
+	set_bit(KEY_FLAG_TRUSTED_ONLY, &keyring[id]->flags);
+	return 0;
+}
+#endif

security/integrity/integrity.h +10 -0

index 2fb5e53..dd26ad0 100644
@@ -137,6 +137,7 @@ static inline int integrity_digsig_verify(const unsigned int id,
 #ifdef CONFIG_INTEGRITY_ASYMMETRIC_KEYS
 int asymmetric_verify(struct key *keyring, const char *sig,
 		      int siglen, const char *data, int datalen);
+
 #else
 static inline int asymmetric_verify(struct key *keyring, const char *sig,
 				    int siglen, const char *data, int datalen)

@@ -145,6 +146,15 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
 }
 #endif
 
+#ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
+int integrity_init_keyring(const unsigned int id);
+#else
+static inline int integrity_init_keyring(const unsigned int id)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_AUDIT
 /* declarations */
 void integrity_audit_msg(int audit_msgno, struct inode *inode,

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:30 UTC #9 lore raw

PATCH 6/20 ima: create '_ima' as a builtin 'trusted' keyring

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

---
 security/integrity/ima/Kconfig    | 9 +++++++++
 security/integrity/ima/ima_init.c | 1 +
 2 files changed, 10 insertions(+)

security/integrity/ima/Kconfig +9 -0

index 5f95e50..be11c01 100644
@@ -122,3 +122,12 @@ config IMA_APPRAISE
 	  For more information on integrity appraisal refer to:
 	  <http://linux-ima.sourceforge.net>
 	  If unsure, say N.
+
+config IMA_TRUSTED_KEYRING
+	bool "Require all keys on the _ima keyring be signed"
+	depends on IMA && SYSTEM_TRUSTED_KEYRING
+	select INTEGRITY_TRUSTED_KEYRING
+	default y
+	help
+	   This option requires that all keys added to the _ima
+	   keyring be signed by a key on the system trusted keyring.

security/integrity/ima/ima_init.c +1 -0

index e8f9d70..515c1a2 100644
@@ -107,6 +107,7 @@ int __init ima_init(void)
 
 	ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	ima_init_policy();
+	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 
 	return ima_fs_init();
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:29 UTC #10 lore raw

PATCH 7/20 integrity: provide x509 certificate loading from the kernel

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

---
 security/integrity/Kconfig     |  4 +++
 security/integrity/digsig.c    | 72 ++++++++++++++++++++++++++++++++++++++++++
 security/integrity/integrity.h | 10 ++++++
 3 files changed, 86 insertions(+)

security/integrity/Kconfig +4 -0

index 89f226a..8f34b28 100644
@@ -51,6 +51,10 @@ config INTEGRITY_TRUSTED_KEYRING
 	def_bool n
 	depends on IMA_TRUSTED_KEYRING || EVM_TRUSTED_KEYRING
 
+config INTEGRITY_LOAD_X509
+	def_bool n
+	depends on IMA_LOAD_X509 || EVM_LOAD_X509
+
 source security/integrity/ima/Kconfig
 source security/integrity/evm/Kconfig

security/integrity/digsig.c +72 -0

84 lines not on this page · raw message

security/integrity/integrity.h +10 -0

index dd26ad0..96960fb 100644
@@ -146,6 +146,16 @@ static inline int asymmetric_verify(struct key *keyring, const char *sig,
 }
 #endif
 
+#ifdef CONFIG_INTEGRITY_LOAD_X509
+int integrity_read_file(const char *path, char **data);
+int integrity_load_x509(const unsigned int id, char *path);
+#else
+static inline int integrity_load_x509(const unsigned int id, char *path)
+{
+	return 0;
+}
+#endif
+
 #ifdef CONFIG_INTEGRITY_TRUSTED_KEYRING
 int integrity_init_keyring(const unsigned int id);
 #else

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:29 UTC #11 lore raw

PATCH 8/20 ima: load x509 certificate from the kernel

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

---
 security/integrity/ima/Kconfig    | 9 +++++++++
 security/integrity/ima/ima_init.c | 1 +
 2 files changed, 10 insertions(+)

security/integrity/ima/Kconfig +9 -0

index be11c01..5474c47 100644
@@ -131,3 +131,12 @@ config IMA_TRUSTED_KEYRING
 	help
 	   This option requires that all keys added to the _ima
 	   keyring be signed by a key on the system trusted keyring.
+
+config IMA_LOAD_X509
+	bool "Load X509 certificate to the '_ima' trusted keyring"
+	depends on IMA_TRUSTED_KEYRING
+	select INTEGRITY_LOAD_X509
+	default n
+	help
+	   This option enables X509 certificate loading from the kernel
+	   to the '_ima' trusted keyring.

security/integrity/ima/ima_init.c +1 -0

index 515c1a2..c13d6a8 100644
@@ -108,6 +108,7 @@ int __init ima_init(void)
 	ima_add_boot_aggregate();	/* boot aggregate must be first entry */
 	ima_init_policy();
 	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
+	integrity_load_x509(INTEGRITY_KEYRING_IMA, "/etc/keys/x509_ima.der");
 
 	return ima_fs_init();
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:30 UTC #12 lore raw

PATCH 9/20 evm: create '_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.

---
 security/integrity/evm/Kconfig    | 8 ++++++++
 security/integrity/evm/evm_main.c | 2 ++
 2 files changed, 10 insertions(+)

security/integrity/evm/Kconfig +8 -0

index df20a2f..3f9098c6 100644
@@ -43,3 +43,11 @@ config EVM_EXTRA_SMACK_XATTRS
 	  additional info to the calculation, requires existing EVM
 	  labeled file systems to be relabeled.
 
+config EVM_TRUSTED_KEYRING
+	bool "Require all keys on the _evm keyring be signed"
+	depends on EVM && SYSTEM_TRUSTED_KEYRING
+	select INTEGRITY_TRUSTED_KEYRING
+	default n
+	help
+	   This option requires that all keys added to the _evm
+	   keyring be signed by a key on the system trusted keyring.

security/integrity/evm/evm_main.c +2 -0

index 4c00adb..8a11920 100644
@@ -447,6 +447,8 @@ static int __init init_evm(void)
 
 	evm_init_config();
 
+	integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+
 	error = evm_init_secfs();
 	if (error < 0) {
 		pr_info("Error registering secfs\n");

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:51 UTC #13 lore raw

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

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

---
 security/integrity/evm/Kconfig    | 9 +++++++++
 security/integrity/evm/evm_main.c | 1 +
 2 files changed, 10 insertions(+)

security/integrity/evm/Kconfig +9 -0

index 3f9098c6..70d12f7 100644
@@ -51,3 +51,12 @@ config EVM_TRUSTED_KEYRING
 	help
 	   This option requires that all keys added to the _evm
 	   keyring be signed by a key on the system trusted keyring.
+
+config EVM_LOAD_X509
+	bool "Load X509 certificate to the '_evm' trusted keyring"
+	depends on EVM_TRUSTED_KEYRING
+	select INTEGRITY_LOAD_X509
+	default n
+	help
+	   This option enables X509 certificate loading from the kernel
+	   to the '_evm' trusted keyring.

security/integrity/evm/evm_main.c +1 -0

index 8a11920..0f1b489 100644
@@ -448,6 +448,7 @@ static int __init init_evm(void)
 	evm_init_config();
 
 	integrity_init_keyring(INTEGRITY_KEYRING_EVM);
+	integrity_load_x509(INTEGRITY_KEYRING_EVM, "/etc/keys/x509_ima.der");
 
 	error = evm_init_secfs();
 	if (error < 0) {

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:58 UTC #14 lore raw

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

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

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

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

---
 security/integrity/ima/ima_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

security/integrity/ima/ima_main.c +11 -1

index c0aaed8..6af4966 100644
@@ -67,6 +67,15 @@ out:
 }
 __setup("ima_hash=", hash_setup);
 
+static int ima_mode = 1;
+static int __init ima_setup(char *str)
+{
+	if (strncmp(str, "off", 3) == 0)
+		ima_mode = 0;
+	return 1;
+}
+__setup("ima=", ima_setup);
+
 /*
  * ima_rdwr_violation_check
  *

@@ -324,8 +333,9 @@ static int __init init_ima(void)
 	int error;
 
 	hash_setup(CONFIG_IMA_DEFAULT_HASH);
+
 	error = ima_init();
-	if (!error)
+	if (!error && ima_mode)
 		ima_initialized = 1;
 	return error;
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:50 UTC #15 lore raw

PATCH 12/20 ima: provide buffer hash calculation function

This patch provides convenient buffer hash calculation function.

---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_crypto.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

security/integrity/ima/ima.h +1 -0

index f4c1e8dd..a5d5ccb 100644
@@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
 			      struct ima_template_desc *desc, int num_fields,
 			      struct ima_digest_data *hash);

security/integrity/ima/ima_crypto.c +9 -2

index 139e7f7..50c78c0 100644
@@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
 		u8 *data_to_hash = field_data[i].data;
 		u32 datalen = field_data[i].len;
 
-		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
 			rc = crypto_shash_update(&desc.shash,
 						(const u8 *) &field_data[i].len,
 						sizeof(field_data[i].len));
 			if (rc)
 				break;
-		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
+		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
 			memcpy(buffer, data_to_hash, datalen);
 			data_to_hash = buffer;
 			datalen = IMA_EVENT_NAME_LEN_MAX + 1;

@@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+{
+	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
+
+	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)

Signature · 1 line

1.8.3.2

Mimi Zohar

2014-04-24 21:05 UTC #16 lore raw

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

Quoted 3 lines · Dmitry Kasatkin

This patch provides convenient buffer hash calculation function.
Signed-off-by Dmitry Kasatkin <[email protected]>

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

Mimi

Quoted 51 lines

---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_crypto.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

security/integrity/ima/ima.h +1 -0

index f4c1e8dd..a5d5ccb 100644
@@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
 			      struct ima_template_desc *desc, int num_fields,
 			      struct ima_digest_data *hash);

security/integrity/ima/ima_crypto.c +9 -2

index 139e7f7..50c78c0 100644
@@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
 		u8 *data_to_hash = field_data[i].data;
 		u32 datalen = field_data[i].len;
 
-		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
 			rc = crypto_shash_update(&desc.shash,
 						(const u8 *) &field_data[i].len,
 						sizeof(field_data[i].len));
 			if (rc)
 				break;
-		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
+		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
 			memcpy(buffer, data_to_hash, datalen);
 			data_to_hash = buffer;
 			datalen = IMA_EVENT_NAME_LEN_MAX + 1;

@@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+{
+	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
+
+	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)

Dmitry Kasatkin

author

2014-04-25 14:51 UTC #17 lore raw

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

Quoted 8 lines · Mimi Zohar

Quoted 3 lines · Dmitry Kasatkin
This patch provides convenient buffer hash calculation function.
Signed-off-by Dmitry Kasatkin <[email protected]>
Where/how is it being used? We normally don't upstream a new function without it being used. Is the usage in another patch?
Mimi

Sure.. it is used in PATCH 15.

Quoted 53 lines

Quoted 51 lines

---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_crypto.c | 11 +++++++++--
 2 files changed, 10 insertions(+), 2 deletions(-)

security/integrity/ima/ima.h +1 -0

index f4c1e8dd..a5d5ccb 100644
@@ -98,6 +98,7 @@ int ima_add_template_entry(struct ima_template_entry *entry, int violation,
 			   const char *op, struct inode *inode,
 			   const unsigned char *filename);
 int ima_calc_file_hash(struct file *file, struct ima_digest_data *hash);
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash);
 int ima_calc_field_array_hash(struct ima_field_data *field_data,
 			      struct ima_template_desc *desc, int num_fields,
 			      struct ima_digest_data *hash);

security/integrity/ima/ima_crypto.c +9 -2

index 139e7f7..50c78c0 100644
@@ -434,13 +434,13 @@ static int ima_calc_field_array_hash_tfm(struct ima_field_data *field_data,
 		u8 *data_to_hash = field_data[i].data;
 		u32 datalen = field_data[i].len;
 
-		if (strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
+		if (td && strcmp(td->name, IMA_TEMPLATE_IMA_NAME) != 0) {
 			rc = crypto_shash_update(&desc.shash,
 						(const u8 *) &field_data[i].len,
 						sizeof(field_data[i].len));
 			if (rc)
 				break;
-		} else if (strcmp(td->fields[i]->field_id, "n") == 0) {
+		} else if (td && strcmp(td->fields[i]->field_id, "n") == 0) {
 			memcpy(buffer, data_to_hash, datalen);
 			data_to_hash = buffer;
 			datalen = IMA_EVENT_NAME_LEN_MAX + 1;

@@ -475,6 +475,13 @@ int ima_calc_field_array_hash(struct ima_field_data *field_data,
 	return rc;
 }
 
+int ima_calc_buffer_hash(const void *buf, int len, struct ima_digest_data *hash)
+{
+	struct ima_field_data fd = { .data = (u8 *)buf, .len = len };
+
+	return ima_calc_field_array_hash(&fd, NULL, 1, hash);
+}
+
 static void __init ima_pcrread(int idx, u8 *pcr)
 {
 	if (!ima_used_chip)

Dmitry Kasatkin

author

2014-04-23 13:43 UTC #18 lore raw

PATCH 13/20 ima: replace opencount with bitop

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

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

This patch replaces usage of opencount with busy bit.

---
 security/integrity/ima/ima_fs.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

security/integrity/ima/ima_fs.c +14 -9

index da92fcc..71f0d8f 100644
@@ -288,7 +288,12 @@ static struct dentry *runtime_measurements_count;
 static struct dentry *violations;
 static struct dentry *ima_policy;
 
-static atomic_t policy_opencount = ATOMIC_INIT(1);
+enum ima_fs_flags {
+	IMA_FS_BUSY = 0,
+};
+
+static unsigned long ima_fs_flags;
+
 /*
  * ima_open_policy: sequentialize access to the policy file
  */

@@ -297,9 +302,9 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 	/* No point in being allowed to open it if you aren't going to write */
 	if (!(filp->f_flags & O_WRONLY))
 		return -EACCES;
-	if (atomic_dec_and_test(&policy_opencount))
-		return 0;
-	return -EBUSY;
+	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+		return -EBUSY;
+	return 0;
 }
 
 /*

@@ -314,12 +319,12 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 	if (!valid_policy) {
 		ima_delete_rules();
 		valid_policy = 1;
-		atomic_set(&policy_opencount, 1);
-		return 0;
+	} else {
+		ima_update_policy();
+		securityfs_remove(ima_policy);
+		ima_policy = NULL;
 	}
-	ima_update_policy();
-	securityfs_remove(ima_policy);
-	ima_policy = NULL;
+	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 	return 0;
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:43 UTC #19 lore raw

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

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

---
 security/integrity/ima/ima.h        |  1 +
 security/integrity/ima/ima_fs.c     |  7 +++++--
 security/integrity/ima/ima_policy.c | 16 +++++++++-------
 3 files changed, 15 insertions(+), 9 deletions(-)

security/integrity/ima/ima.h +1 -0

index a5d5ccb..3b90b60 100644
@@ -166,6 +166,7 @@ int ima_match_policy(struct dentry *dentry, enum ima_hooks func, int mask,
 		     int flags);
 void ima_init_policy(void);
 void ima_update_policy(void);
+bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);

security/integrity/ima/ima_fs.c +5 -2

index 71f0d8f..34ae5f2 100644
@@ -304,6 +304,11 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return -EACCES;
 	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
 		return -EBUSY;
+	if (!ima_default_policy()) {
+		/* policy was already set*/
+		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+		return -EACCES;
+	}
 	return 0;
 }

@@ -321,8 +326,6 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 		valid_policy = 1;
 	} else {
 		ima_update_policy();
-		securityfs_remove(ima_policy);
-		ima_policy = NULL;
 	}
 	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 	return 0;

security/integrity/ima/ima_policy.c +9 -7

index 9b8121b..b24e7d1 100644
@@ -345,6 +345,11 @@ void __init ima_init_policy(void)
 	ima_rules = &ima_default_rules;
 }
 
+bool ima_default_policy(void)
+{
+	return ima_rules == &ima_default_rules;
+}
+
 /**
  * ima_update_policy - update default_rules with new measure rules
  *

@@ -355,15 +360,12 @@ void __init ima_init_policy(void)
 void ima_update_policy(void)
 {
 	static const char op[] = "policy_update";
-	const char *cause = "already exists";
-	int result = 1;
+	const char *cause = "complete";
+	int result = 0;
 	int audit_info = 0;
 
-	if (ima_rules == &ima_default_rules) {
-		ima_rules = &ima_policy_rules;
-		cause = "complete";
-		result = 0;
-	}
+	ima_rules = &ima_policy_rules;
+
 	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
 			    NULL, op, cause, result, audit_info);
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:42 UTC #20 lore raw

PATCH 15/20 ima: path based policy loading interface

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

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

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

---
 security/integrity/ima/Kconfig      | 13 +++++++
 security/integrity/ima/ima.h        |  9 +++++
 security/integrity/ima/ima_fs.c     |  2 +-
 security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)

security/integrity/ima/Kconfig +13 -0

index 5474c47..465cef4 100644
@@ -140,3 +140,16 @@ config IMA_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_ima' trusted keyring.
+
+config IMA_POLICY_LOADER
+	bool "Path based policy loading interface"
+	depends on IMA_TRUSTED_KEYRING
+	default n
+	help
+	  This option enables path based signed policy loading interface.
+	  Policy signature must be provided in the <policy>.sig file
+	  along with the policy. When this option is enabled, kernel
+	  tries to load default policy from /etc/ima_policy.
+
+	  Loading policy is like:
+	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

security/integrity/ima/ima.h +9 -0

index 3b90b60..f2722bb 100644
@@ -170,6 +170,15 @@ bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);
 
+#ifdef CONFIG_IMA_POLICY_LOADER
+ssize_t ima_read_policy(char *path);
+#else
+static inline ssize_t ima_read_policy(char *data)
+{
+	return ima_parse_add_rule(data);
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02

security/integrity/ima/ima_fs.c +1 -1

index 34ae5f2..bde7a0e 100644
@@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	if (copy_from_user(data, buf, datalen))
 		goto out;
 
-	result = ima_parse_add_rule(data);
+	result = ima_read_policy(data);
 out:
 	if (result < 0)
 		valid_policy = 0;

security/integrity/ima/ima_policy.c +74 -0

83 lines not on this page · raw message

Signature · 1 line

1.8.3.2

Mimi Zohar

2014-04-24 21:05 UTC #21 lore raw

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

Quoted 6 lines · Dmitry Kasatkin

Currently policy is loaded by writing policy content to '<securityfs>/ima/policy' file.
This patch extends policy loading meachanism with possibility to load signed policy using a path to the policy. Policy signature must be available in the <policy>.sig file.

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

thanks,

Mimi

Quoted 155 lines

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

---
 security/integrity/ima/Kconfig      | 13 +++++++
 security/integrity/ima/ima.h        |  9 +++++
 security/integrity/ima/ima_fs.c     |  2 +-
 security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)

security/integrity/ima/Kconfig +13 -0

index 5474c47..465cef4 100644
@@ -140,3 +140,16 @@ config IMA_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_ima' trusted keyring.
+
+config IMA_POLICY_LOADER
+	bool "Path based policy loading interface"
+	depends on IMA_TRUSTED_KEYRING
+	default n
+	help
+	  This option enables path based signed policy loading interface.
+	  Policy signature must be provided in the <policy>.sig file
+	  along with the policy. When this option is enabled, kernel
+	  tries to load default policy from /etc/ima_policy.
+
+	  Loading policy is like:
+	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

security/integrity/ima/ima.h +9 -0

index 3b90b60..f2722bb 100644
@@ -170,6 +170,15 @@ bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);
 
+#ifdef CONFIG_IMA_POLICY_LOADER
+ssize_t ima_read_policy(char *path);
+#else
+static inline ssize_t ima_read_policy(char *data)
+{
+	return ima_parse_add_rule(data);
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02

security/integrity/ima/ima_fs.c +1 -1

index 34ae5f2..bde7a0e 100644
@@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	if (copy_from_user(data, buf, datalen))
 		goto out;
 
-	result = ima_parse_add_rule(data);
+	result = ima_read_policy(data);
 out:
 	if (result < 0)
 		valid_policy = 0;

security/integrity/ima/ima_policy.c +74 -0

index b24e7d1..c6da801 100644
@@ -17,6 +17,9 @@
 #include <linux/parser.h>
 #include <linux/slab.h>
 #include <linux/genhd.h>
+#ifdef CONFIG_IMA_POLICY_LOADER
+#include <linux/file.h>
+#endif
 
 #include "ima.h"

@@ -747,3 +750,74 @@ void ima_delete_rules(void)
 	}
 	mutex_unlock(&ima_rules_mutex);
 }
+
+#ifdef CONFIG_IMA_POLICY_LOADER
+
+ssize_t ima_read_policy(char *path)
+{
+	char *data, *datap, *sig;
+	int rc, psize, pathlen = strlen(path);
+	char *p, *sigpath;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
+
+	if (path[0] != '/')
+		return ima_parse_add_rule(path);
+
+	/* remove \n */
+	datap = path;
+	strsep(&datap, "\n");
+
+	/* we always want signature? */
+	sigpath = __getname();
+	if (!sigpath)
+		return -ENOMEM;
+
+	rc = integrity_read_file(path, &data);
+	if (rc < 0)
+		goto free_path;
+
+	psize = rc;
+	datap = data;
+
+	sprintf(sigpath, "%s.sig", path);
+	/* we always want signature? */
+	rc = integrity_read_file(sigpath, &sig);
+	if (rc < 0)
+		goto free_data;
+
+	hash.hdr.algo = ima_hash_algo;
+	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
+	ima_calc_buffer_hash(data, psize, &hash.hdr);
+	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+				     (const char *)sig, rc,
+				     hash.hdr.digest, hash.hdr.length);
+	if (rc) {
+		pr_err("integrity_digsig_verify() = %d\n", rc);
+		goto free_sig;
+	}
+
+	while (psize > 0 && (p = strsep(&datap, "\n"))) {
+		pr_debug("rule: %s\n", p);
+		rc = ima_parse_add_rule(p);
+		if (rc < 0)
+			break;
+		psize -= rc;
+	}
+free_sig:
+	kfree(sig);
+free_data:
+	kfree(data);
+free_path:
+	__putname(sigpath);
+	if (rc < 0)
+		return rc;
+	else if (psize)
+		return -EINVAL;
+	else
+		return pathlen;
+}
+
+#endif

Dmitry Kasatkin

author

2014-04-25 15:17 UTC #22 lore raw

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

Quoted 10 lines · Mimi Zohar

Quoted 6 lines · Dmitry Kasatkin
Currently policy is loaded by writing policy content to '<securityfs>/ima/policy' file.
This patch extends policy loading meachanism with possibility to load signed policy using a path to the policy. Policy signature must be available in the <policy>.sig file.
Assuming (big assumption) you're permitted to open the policy file from the kernel, why are you verifying the signature inline based on a .sig? Shouldn't this be a new integrity/security hook?

What kind of hook do you mean?

Actually I was considering 2 approaches.

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

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

- Dmitry

Quoted 160 lines

thanks,

Mimi

Quoted 154 lines

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

---
 security/integrity/ima/Kconfig      | 13 +++++++
 security/integrity/ima/ima.h        |  9 +++++
 security/integrity/ima/ima_fs.c     |  2 +-
 security/integrity/ima/ima_policy.c | 74 +++++++++++++++++++++++++++++++++++++
 4 files changed, 97 insertions(+), 1 deletion(-)

security/integrity/ima/Kconfig +13 -0

index 5474c47..465cef4 100644
@@ -140,3 +140,16 @@ config IMA_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_ima' trusted keyring.
+
+config IMA_POLICY_LOADER
+	bool "Path based policy loading interface"
+	depends on IMA_TRUSTED_KEYRING
+	default n
+	help
+	  This option enables path based signed policy loading interface.
+	  Policy signature must be provided in the <policy>.sig file
+	  along with the policy. When this option is enabled, kernel
+	  tries to load default policy from /etc/ima_policy.
+
+	  Loading policy is like:
+	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy

security/integrity/ima/ima.h +9 -0

index 3b90b60..f2722bb 100644
@@ -170,6 +170,15 @@ bool ima_default_policy(void);
 ssize_t ima_parse_add_rule(char *);
 void ima_delete_rules(void);
 
+#ifdef CONFIG_IMA_POLICY_LOADER
+ssize_t ima_read_policy(char *path);
+#else
+static inline ssize_t ima_read_policy(char *data)
+{
+	return ima_parse_add_rule(data);
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02

security/integrity/ima/ima_fs.c +1 -1

index 34ae5f2..bde7a0e 100644
@@ -273,7 +273,7 @@ static ssize_t ima_write_policy(struct file *file, const char __user *buf,
 	if (copy_from_user(data, buf, datalen))
 		goto out;
 
-	result = ima_parse_add_rule(data);
+	result = ima_read_policy(data);
 out:
 	if (result < 0)
 		valid_policy = 0;

security/integrity/ima/ima_policy.c +74 -0

index b24e7d1..c6da801 100644
@@ -17,6 +17,9 @@
 #include <linux/parser.h>
 #include <linux/slab.h>
 #include <linux/genhd.h>
+#ifdef CONFIG_IMA_POLICY_LOADER
+#include <linux/file.h>
+#endif
 
 #include "ima.h"

@@ -747,3 +750,74 @@ void ima_delete_rules(void)
 	}
 	mutex_unlock(&ima_rules_mutex);
 }
+
+#ifdef CONFIG_IMA_POLICY_LOADER
+
+ssize_t ima_read_policy(char *path)
+{
+	char *data, *datap, *sig;
+	int rc, psize, pathlen = strlen(path);
+	char *p, *sigpath;
+	struct {
+		struct ima_digest_data hdr;
+		char digest[IMA_MAX_DIGEST_SIZE];
+	} hash;
+
+	if (path[0] != '/')
+		return ima_parse_add_rule(path);
+
+	/* remove \n */
+	datap = path;
+	strsep(&datap, "\n");
+
+	/* we always want signature? */
+	sigpath = __getname();
+	if (!sigpath)
+		return -ENOMEM;
+
+	rc = integrity_read_file(path, &data);
+	if (rc < 0)
+		goto free_path;
+
+	psize = rc;
+	datap = data;
+
+	sprintf(sigpath, "%s.sig", path);
+	/* we always want signature? */
+	rc = integrity_read_file(sigpath, &sig);
+	if (rc < 0)
+		goto free_data;
+
+	hash.hdr.algo = ima_hash_algo;
+	ima_get_hash_algo((void *)sig, rc, &hash.hdr);
+	ima_calc_buffer_hash(data, psize, &hash.hdr);
+	rc = integrity_digsig_verify(INTEGRITY_KEYRING_IMA,
+				     (const char *)sig, rc,
+				     hash.hdr.digest, hash.hdr.length);
+	if (rc) {
+		pr_err("integrity_digsig_verify() = %d\n", rc);
+		goto free_sig;
+	}
+
+	while (psize > 0 && (p = strsep(&datap, "\n"))) {
+		pr_debug("rule: %s\n", p);
+		rc = ima_parse_add_rule(p);
+		if (rc < 0)
+			break;
+		psize -= rc;
+	}
+free_sig:
+	kfree(sig);
+free_data:
+	kfree(data);
+free_path:
+	__putname(sigpath);
+	if (rc < 0)
+		return rc;
+	else if (psize)
+		return -EINVAL;
+	else
+		return pathlen;
+}
+
+#endif

Dmitry Kasatkin

author

2014-04-23 13:42 UTC #23 lore raw

PATCH 16/20 ima: load policy from the kernel

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

---
 security/integrity/ima/Kconfig    |  7 +++++++
 security/integrity/ima/ima.h      |  8 ++++++++
 security/integrity/ima/ima_fs.c   | 18 +++++++++++++++++-
 security/integrity/ima/ima_init.c |  1 +
 4 files changed, 33 insertions(+), 1 deletion(-)

security/integrity/ima/Kconfig +7 -0

index 465cef4..b00044f 100644
@@ -153,3 +153,10 @@ config IMA_POLICY_LOADER
 
 	  Loading policy is like:
 	  echo /etc/ima/ima_policy > /sys/kernel/security/ima/policy
+
+config IMA_KERNEL_POLICY
+	bool "Load IMA policy from the kernel"
+	depends on IMA_POLICY_LOADER
+	default n
+	help
+	  This option enables IMA policy loading from the kernel.

security/integrity/ima/ima.h +8 -0

index f2722bb..3727cf7 100644
@@ -179,6 +179,14 @@ static inline ssize_t ima_read_policy(char *data)
 }
 #endif
 
+#ifdef CONFIG_IMA_KERNEL_POLICY
+void ima_load_policy(char *path);
+#else
+static inline void ima_load_policy(char *path)
+{
+}
+#endif
+
 /* Appraise integrity measurements */
 #define IMA_APPRAISE_ENFORCE	0x01
 #define IMA_APPRAISE_FIX	0x02

security/integrity/ima/ima_fs.c +17 -1

index bde7a0e..d050a5c 100644
@@ -319,7 +319,7 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
  * point to the new policy rules, and remove the securityfs policy file,
  * assuming a valid policy.
  */
-static int ima_release_policy(struct inode *inode, struct file *file)
+static void ima_check_policy(void)
 {
 	if (!valid_policy) {
 		ima_delete_rules();

@@ -328,6 +328,11 @@ static int ima_release_policy(struct inode *inode, struct file *file)
 		ima_update_policy();
 	}
 	clear_bit(IMA_FS_BUSY, &ima_fs_flags);
+}
+
+static int ima_release_policy(struct inode *inode, struct file *file)
+{
+	ima_check_policy();
 	return 0;
 }

@@ -338,6 +343,17 @@ static const struct file_operations ima_measure_policy_ops = {
 	.llseek = generic_file_llseek,
 };
 
+#ifdef CONFIG_IMA_KERNEL_POLICY
+void __init ima_load_policy(char *path)
+{
+	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
+		return;
+	if (ima_read_policy(path) < 0)
+		valid_policy = 0;
+	ima_check_policy();
+}
+#endif
+
 int __init ima_fs_init(void)
 {
 	ima_dir = securityfs_create_dir("ima", NULL);

security/integrity/ima/ima_init.c +1 -0

index c13d6a8..d1a6483 100644
@@ -109,6 +109,7 @@ int __init ima_init(void)
 	ima_init_policy();
 	integrity_init_keyring(INTEGRITY_KEYRING_IMA);
 	integrity_load_x509(INTEGRITY_KEYRING_IMA, "/etc/keys/x509_ima.der");
+	ima_load_policy("/etc/ima_policy");
 
 	return ima_fs_init();
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:40 UTC #24 lore raw

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

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

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

---
 security/integrity/ima/Kconfig      |  8 ++++++++
 security/integrity/ima/ima_fs.c     |  2 ++
 security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
 3 files changed, 29 insertions(+), 4 deletions(-)

security/integrity/ima/Kconfig +8 -0

index b00044f..b60a315 100644
@@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
 	default n
 	help
 	  This option enables IMA policy loading from the kernel.
+
+config IMA_POLICY_REPLACEABLE
+	bool "Allows to replace policy at runtime"
+	depends on IMA_POLICY_LOADER
+	default n
+	help
+	  Enabling this option allows to replace policy at runtime.
+	  Only signed policy is allowed.

security/integrity/ima/ima_fs.c +2 -0

index d050a5c..b4144b4 100644
@@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return -EACCES;
 	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
 		return -EBUSY;
+#ifndef CONFIG_IMA_POLICY_REPLACEABLE
 	if (!ima_default_policy()) {
 		/* policy was already set*/
 		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 		return -EACCES;
 	}
+#endif
 	return 0;
 }

security/integrity/ima/ima_policy.c +19 -4

index c6da801..981e953 100644
@@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
 
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
+static LIST_HEAD(ima_active_rules);
 static struct list_head *ima_rules;
 static bool path_rules;
 
 static DEFINE_MUTEX(ima_rules_mutex);
 
+static void ima_do_delete_rules(struct list_head *rules);
+
 static bool ima_use_tcb __initdata;
 static int __init default_measure_policy_setup(char *str)
 {

@@ -367,7 +370,14 @@ void ima_update_policy(void)
 	int result = 0;
 	int audit_info = 0;
 
-	ima_rules = &ima_policy_rules;
+	if (ima_default_policy()) {
+		/* set new policy head */
+		ima_rules = &ima_active_rules;
+	} else {
+		/* FIXME: must be protected by lock */
+		ima_do_delete_rules(ima_rules);
+	}
+	list_replace_init(&ima_policy_rules, ima_rules);
 
 	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
 			    NULL, op, cause, result, audit_info);

@@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
 	return len;
 }
 
-/* ima_delete_rules called to cleanup invalid policy */
-void ima_delete_rules(void)
+/* ima_delete_rules called to cleanup invalid or old policy */
+static void ima_do_delete_rules(struct list_head *rules)
 {
 	struct ima_rule_entry *entry, *tmp;
 	int i;
 
 	mutex_lock(&ima_rules_mutex);
-	list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+	list_for_each_entry_safe(entry, tmp, rules, list) {
 		for (i = 0; i < MAX_LSM_RULES; i++)
 			kfree(entry->lsm[i].args_p);

@@ -751,6 +761,11 @@ void ima_delete_rules(void)
 	mutex_unlock(&ima_rules_mutex);
 }
 
+void ima_delete_rules(void)
+{
+	ima_do_delete_rules(&ima_policy_rules);
+}
+
 #ifdef CONFIG_IMA_POLICY_LOADER
 
 ssize_t ima_read_policy(char *path)

Signature · 1 line

1.8.3.2

Mimi Zohar

2014-05-14 23:46 UTC #25 lore raw

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

Quoted 8 lines · Dmitry Kasatkin

This patch provides functionality to replace the IMA policy at runtime.
By default, the IMA policy can be successfully updated only once, but with this patch when the kernel configuration option CONFIG_IMA_POLICY_REPLACEABLE is enabled, the IMA policy can be replaced multiple times at runtime.
Signed-off-by Dmitry Kasatkin <[email protected]>

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

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

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

Please provide the motivation for such a use case scenario.

Mimi

Quoted 105 lines

---
 security/integrity/ima/Kconfig      |  8 ++++++++
 security/integrity/ima/ima_fs.c     |  2 ++
 security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
 3 files changed, 29 insertions(+), 4 deletions(-)

security/integrity/ima/Kconfig +8 -0

index b00044f..b60a315 100644
@@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
 	default n
 	help
 	  This option enables IMA policy loading from the kernel.
+
+config IMA_POLICY_REPLACEABLE
+	bool "Allows to replace policy at runtime"
+	depends on IMA_POLICY_LOADER
+	default n
+	help
+	  Enabling this option allows to replace policy at runtime.
+	  Only signed policy is allowed.

security/integrity/ima/ima_fs.c +2 -0

index d050a5c..b4144b4 100644
@@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
 		return -EACCES;
 	if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
 		return -EBUSY;
+#ifndef CONFIG_IMA_POLICY_REPLACEABLE
 	if (!ima_default_policy()) {
 		/* policy was already set*/
 		clear_bit(IMA_FS_BUSY, &ima_fs_flags);
 		return -EACCES;
 	}
+#endif
 	return 0;
 }

security/integrity/ima/ima_policy.c +19 -4

index c6da801..981e953 100644
@@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
 
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
+static LIST_HEAD(ima_active_rules);
 static struct list_head *ima_rules;
 static bool path_rules;
 
 static DEFINE_MUTEX(ima_rules_mutex);
 
+static void ima_do_delete_rules(struct list_head *rules);
+
 static bool ima_use_tcb __initdata;
 static int __init default_measure_policy_setup(char *str)
 {

@@ -367,7 +370,14 @@ void ima_update_policy(void)
 	int result = 0;
 	int audit_info = 0;
 
-	ima_rules = &ima_policy_rules;
+	if (ima_default_policy()) {
+		/* set new policy head */
+		ima_rules = &ima_active_rules;
+	} else {
+		/* FIXME: must be protected by lock */
+		ima_do_delete_rules(ima_rules);
+	}
+	list_replace_init(&ima_policy_rules, ima_rules);
 
 	integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
 			    NULL, op, cause, result, audit_info);

@@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
 	return len;
 }
 
-/* ima_delete_rules called to cleanup invalid policy */
-void ima_delete_rules(void)
+/* ima_delete_rules called to cleanup invalid or old policy */
+static void ima_do_delete_rules(struct list_head *rules)
 {
 	struct ima_rule_entry *entry, *tmp;
 	int i;
 
 	mutex_lock(&ima_rules_mutex);
-	list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+	list_for_each_entry_safe(entry, tmp, rules, list) {
 		for (i = 0; i < MAX_LSM_RULES; i++)
 			kfree(entry->lsm[i].args_p);

@@ -751,6 +761,11 @@ void ima_delete_rules(void)
 	mutex_unlock(&ima_rules_mutex);
 }
 
+void ima_delete_rules(void)
+{
+	ima_do_delete_rules(&ima_policy_rules);
+}
+
 #ifdef CONFIG_IMA_POLICY_LOADER
 
 ssize_t ima_read_policy(char *path)

Dmitry Kasatkin

maintainer

2014-05-15 06:08 UTC #26 lore raw

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

Quoted 8 lines · Mimi Zohar

Quoted 7 lines · Dmitry Kasatkin
This patch provides functionality to replace the IMA policy at runtime.
By default, the IMA policy can be successfully updated only once, but with this patch when the kernel configuration option CONFIG_IMA_POLICY_REPLACEABLE is enabled, the IMA policy can be replaced multiple times at runtime.

Quoted 9 lines

Quoted 1 line
Signed-off-by Dmitry Kasatkin <[email protected]>
I have a couple of concerns with replacing the IMA policy.
- Currently opened files might now be in policy, that previously weren't. Do these files need to be measured, appraised, or audited? These files could have been modified, but the 'security.ima' xattr hasn't been updated yet. In such cases, appraisal would fail.

Hi,

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

Quoted 7 lines

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

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

- Dmitry

Quoted 111 lines

Quoted 95 lines

---
 security/integrity/ima/Kconfig      |  8 ++++++++
 security/integrity/ima/ima_fs.c     |  2 ++
 security/integrity/ima/ima_policy.c | 23 +++++++++++++++++++----
 3 files changed, 29 insertions(+), 4 deletions(-)

security/integrity/ima/Kconfig +8 -0

index b00044f..b60a315 100644
@@ -160,3 +160,11 @@ config IMA_KERNEL_POLICY
      default n
      help
        This option enables IMA policy loading from the kernel.
+
+config IMA_POLICY_REPLACEABLE
+     bool "Allows to replace policy at runtime"
+     depends on IMA_POLICY_LOADER
+     default n
+     help
+       Enabling this option allows to replace policy at runtime.
+       Only signed policy is allowed.

security/integrity/ima/ima_fs.c +2 -0

index d050a5c..b4144b4 100644
@@ -304,11 +304,13 @@ static int ima_open_policy(struct inode *inode, struct file *filp)
              return -EACCES;
      if (test_and_set_bit(IMA_FS_BUSY, &ima_fs_flags))
              return -EBUSY;
+#ifndef CONFIG_IMA_POLICY_REPLACEABLE
      if (!ima_default_policy()) {
              /* policy was already set*/
              clear_bit(IMA_FS_BUSY, &ima_fs_flags);
              return -EACCES;
      }
+#endif
      return 0;
 }

security/integrity/ima/ima_policy.c +14 -4

index c6da801..981e953 100644
@@ -108,11 +108,14 @@ static struct ima_rule_entry default_appraise_rules[] = {
 
 static LIST_HEAD(ima_default_rules);
 static LIST_HEAD(ima_policy_rules);
+static LIST_HEAD(ima_active_rules);
 static struct list_head *ima_rules;
 static bool path_rules;
 
 static DEFINE_MUTEX(ima_rules_mutex);
 
+static void ima_do_delete_rules(struct list_head *rules);
+
 static bool ima_use_tcb __initdata;
 static int __init default_measure_policy_setup(char *str)
 {

@@ -367,7 +370,14 @@ void ima_update_policy(void)
      int result = 0;
      int audit_info = 0;
 
-     ima_rules = &ima_policy_rules;
+     if (ima_default_policy()) {
+             /* set new policy head */
+             ima_rules = &ima_active_rules;
+     } else {
+             /* FIXME: must be protected by lock */
+             ima_do_delete_rules(ima_rules);
+     }
+     list_replace_init(&ima_policy_rules, ima_rules);
 
      integrity_audit_msg(AUDIT_INTEGRITY_STATUS, NULL,
                          NULL, op, cause, result, audit_info);

@@ -734,14 +744,14 @@ ssize_t ima_parse_add_rule(char *rule)
      return len;
 }
 
-/* ima_delete_rules called to cleanup invalid policy */
-void ima_delete_rules(void)
+/* ima_delete_rules called to cleanup invalid or old policy */
+static void ima_do_delete_rules(struct list_head *rules)
 {
      struct ima_rule_entry *entry, *tmp;
      int i;
 
      mutex_lock(&ima_rules_mutex);
-     list_for_each_entry_safe(entry, tmp, &ima_policy_rules, list) {
+     list_for_each_entry_safe(entry, tmp, rules, list) {
              for (i = 0; i < MAX_LSM_RULES; i++)
                      kfree(entry->lsm[i].args_p);

@@ -751,6 +761,11 @@ void ima_delete_rules(void)
      mutex_unlock(&ima_rules_mutex);

security/integrity/ima/ima_policy.c void ima_delete_rules(void) · line 771

Quoted 10 lines

security/integrity/ima/ima_policy.c +5 -0

 }
 
+void ima_delete_rules(void)
+{
+     ima_do_delete_rules(&ima_policy_rules);
+}
+
 #ifdef CONFIG_IMA_POLICY_LOADER
 
 ssize_t ima_read_policy(char *path)

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Signature · 2 lines

Thanks,
Dmitry

Dmitry Kasatkin

author

2014-04-23 13:38 UTC #27 lore raw

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

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

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

---
 security/integrity/evm/evm.h       |  5 +++++
 security/integrity/evm/evm_main.c  | 19 +++++++++++++------
 security/integrity/evm/evm_secfs.c |  3 ++-
 3 files changed, 20 insertions(+), 7 deletions(-)

security/integrity/evm/evm.h +5 -0

index 88bfe77..1db78b8 100644
@@ -21,6 +21,11 @@
 
 #include "../integrity.h"
 
+#define EVM_MODE_OFF	0
+#define EVM_MODE_ON	1
+#define EVM_MODE_FIX	2
+
+extern int evm_mode;
 extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;

security/integrity/evm/evm_main.c +13 -6

index 0f1b489..ad5e641 100644
@@ -53,14 +53,17 @@ char *evm_config_xattrnames[] = {
 	NULL
 };
 
-static int evm_fixmode;
-static int __init evm_set_fixmode(char *str)
+int evm_mode = EVM_MODE_ON;
+
+static int __init evm_setup(char *str)
 {
-	if (strncmp(str, "fix", 3) == 0)
-		evm_fixmode = 1;
+	if (strncmp(str, "off", 3) == 0)
+		evm_mode = EVM_MODE_OFF;
+	else if (strncmp(str, "fix", 3) == 0)
+		evm_mode = EVM_MODE_FIX;
 	return 0;
 }
-__setup("evm=", evm_set_fixmode);
+__setup("evm=", evm_setup);
 
 static void __init evm_init_config(void)
 {

@@ -249,7 +252,8 @@ static enum integrity_status evm_verify_current_integrity(struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
 
-	if (!evm_initialized || !S_ISREG(inode->i_mode) || evm_fixmode)
+	if (!evm_initialized || !S_ISREG(inode->i_mode) ||
+		    evm_mode == EVM_MODE_FIX)
 		return 0;
 	return evm_verify_hmac(dentry, NULL, NULL, 0, NULL);
 }

@@ -445,6 +449,9 @@ static int __init init_evm(void)
 {
 	int error;
 
+	if (evm_mode == EVM_MODE_OFF)
+		return 0;
+
 	evm_init_config();
 
 	integrity_init_keyring(INTEGRITY_KEYRING_EVM);

security/integrity/evm/evm_secfs.c +2 -1

index cf12a04..4c81ef6 100644
@@ -64,7 +64,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 	char temp[80];
 	int i, error;
 
-	if (!capable(CAP_SYS_ADMIN) || evm_initialized)
+	if (!capable(CAP_SYS_ADMIN) || evm_initialized ||
+		    evm_mode == EVM_MODE_OFF)
 		return -EPERM;
 
 	if (count >= sizeof(temp) || count == 0)

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:37 UTC #28 lore raw

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

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

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

---
 security/integrity/evm/evm_crypto.c |  5 +++++
 security/integrity/evm/evm_main.c   |  2 ++
 security/integrity/evm/evm_secfs.c  | 10 +++-------
 3 files changed, 10 insertions(+), 7 deletions(-)

security/integrity/evm/evm_crypto.c +5 -0

index 5396769..f79ebf5 100644
@@ -258,5 +258,10 @@ out:
 	memset(ekp->decrypted_data, 0, ekp->decrypted_datalen);
 	up_read(&evm_key->sem);
 	key_put(evm_key);
+	if (!rc) {
+		evm_initialized = 1;
+		pr_info("initialized\n");
+	} else
+		pr_err("initialization failed\n");
 	return rc;
 }

security/integrity/evm/evm_main.c +2 -0

index ad5e641..d2c06d3 100644
@@ -463,6 +463,8 @@ static int __init init_evm(void)
 		goto err;
 	}
 
+	evm_init_key();
+
 	return 0;
 err:
 	return error;

security/integrity/evm/evm_secfs.c +3 -7

index 4c81ef6..d7b5d11 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_mode == EVM_MODE_OFF)

@@ -79,12 +79,8 @@ static ssize_t evm_write_key(struct file *file, const char __user *buf,
 	if ((sscanf(temp, "%d", &i) != 1) || (i != 1))
 		return -EINVAL;
 
-	error = evm_init_key();
-	if (!error) {
-		evm_initialized = 1;
-		pr_info("initialized\n");
-	} else
-		pr_err("initialization failed\n");
+	evm_init_key();
+
 	return count;
 }

Signature · 1 line

1.8.3.2

Dmitry Kasatkin

author

2014-04-23 13:35 UTC #29 lore raw

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

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

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

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

---
 security/integrity/evm/Kconfig      |  8 ++++
 security/integrity/evm/evm.h        |  9 ++++
 security/integrity/evm/evm_crypto.c | 96 +++++++++++++++++++++++++++++++++++++
 security/integrity/evm/evm_main.c   |  1 +
 4 files changed, 114 insertions(+)

security/integrity/evm/Kconfig +8 -0

index 70d12f7..953ef05 100644
@@ -60,3 +60,11 @@ config EVM_LOAD_X509
 	help
 	   This option enables X509 certificate loading from the kernel
 	   to the '_evm' trusted keyring.
+
+config EVM_LOAD_KEY
+	bool "Load EVM HMAC key from the kernel"
+	depends on EVM
+	default n
+	help
+	   This option enables EVM HMAC key loading from the kernel.
+	   It enables EVM.

security/integrity/evm/evm.h +9 -0

index 1db78b8..1890eac 100644
@@ -40,6 +40,15 @@ extern struct crypto_shash *hash_tfm;
 /* List of EVM protected security xattrs */
 extern char *evm_config_xattrnames[];
 
+#ifdef CONFIG_EVM_LOAD_KEY
+int evm_load_key(const char *key, const char *kmk);
+#else
+static inline int evm_load_key(const char *key, const char *kmk)
+{
+	return 0;
+}
+#endif
+
 int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
 			const char *req_xattr_name,

security/integrity/evm/evm_crypto.c +96 -0

105 lines not on this page · raw message

security/integrity/evm/evm_main.c +1 -0

index d2c06d3..4808596 100644
@@ -463,6 +463,7 @@ static int __init init_evm(void)
 		goto err;
 	}
 
+	evm_load_key("/etc/keys/evm-key", "/etc/keys/kmk");
 	evm_init_key();
 
 	return 0;

Signature · 1 line

1.8.3.2

Mimi Zohar

2014-04-24 18:45 UTC #30 lore raw