RFC: Make it practical to ship EVM signatures

RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

These are basically untested, but I'd like to get some feedback on the
problem I'm trying to solve here. We'd like to be able to ship packages
with verifiable security xattrs, but right now EVM makes this difficult
due to its requirement that the inode number be encoded in the hmac. This
patchset is intended to make it possible to protect a subset of metadata
rather than all of it, and also to permit using EVM digital signatures in
a similar way to how IMA digital signatures can be used now (ie, protecting
the metadata using public/private crypto rather than having a local
symmetric key and generating the HMACs locally). The expected workflow is:

1) During package build or mirroring process, appropriate security metadata
   is added (IMA hash, selinux label, etc)
2) An EVM digital signature is generated based purely on the security
   metadata present during the build or mirroring process
3) IMA is extended to allow it to force EVM validation during appraisal even
   if no symmetric EVM key has been added, which allows IMA appraisal to
   appraise not only the IMA hash but also the additional metadata
4) If EVM is never enabled, binaries are purely validated using the EVM
   digital signatures and are not transitioned to using HMACs
5) If EVM is desired, userland can set the set of metadata to be incorporated
   into the EVM HMAC before enabling EVM

[PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key

[Matthew Garrett]

A reasonable configuration is to use IMA to appraise a subset of files
(based on user, security label or other features supported by IMA) but
to also want to use EVM to validate not only the state of the IMA hash
but also additional metadata on the file. Right now this is only
possible if a symmetric key has been loaded, which may not be desirable
in all cases (eg, one where EVM digital signatures are shipped to end
systems rather than EVM HMACs being generated locally). Add an
additional "require_evm" keyword to the IMA policy language in order to
permit the local admin to indicate that they wish EVM validation to
occur even if no symmetric key has been loaded.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 Documentation/ABI/testing/ima_policy  |  3 ++-
 include/linux/evm.h                   |  6 ++++--
 security/integrity/evm/evm_main.c     |  6 ++++--
 security/integrity/ima/ima_appraise.c | 11 ++++++++++-
 security/integrity/ima/ima_policy.c   | 12 +++++++++++-
 security/integrity/integrity.h        |  3 ++-
 6 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 5dc9eed035fb..ea2703c847f6 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -23,7 +23,8 @@ Description:
 				[euid=] [fowner=]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
-			option:	[[appraise_type=]] [permit_directio]
+			option:	[[appraise_type=] [permit_directio]
+			         [require_evm]]
 
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
diff --git a/include/linux/evm.h b/include/linux/evm.h
index 35ed9a8a403a..7661f3085942 100644
--- a/include/linux/evm.h
+++ b/include/linux/evm.h
@@ -19,7 +19,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
 					     const char *xattr_name,
 					     void *xattr_value,
 					     size_t xattr_value_len,
-					     struct integrity_iint_cache *iint);
+					     struct integrity_iint_cache *iint,
+					     bool force);
 extern int evm_inode_setattr(struct dentry *dentry, struct iattr *attr);
 extern void evm_inode_post_setattr(struct dentry *dentry, int ia_valid);
 extern int evm_inode_setxattr(struct dentry *dentry, const char *name,
@@ -54,7 +55,8 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
 						    const char *xattr_name,
 						    void *xattr_value,
 						    size_t xattr_value_len,
-					struct integrity_iint_cache *iint)
+					      struct integrity_iint_cache *iint,
+						    bool force)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 063d38aef64e..44e4f4fda965 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -223,6 +223,7 @@ static int evm_protected_xattr(const char *req_xattr_name)
  * @xattr_name: requested xattr
  * @xattr_value: requested xattr value
  * @xattr_value_len: requested xattr value length
+ * @force: force verification even if no EVM symmetric key is loaded
  *
  * Calculate the HMAC for the given dentry and verify it against the stored
  * security.evm xattr. For performance, use the xattr value and length
@@ -236,9 +237,10 @@ static int evm_protected_xattr(const char *req_xattr_name)
 enum integrity_status evm_verifyxattr(struct dentry *dentry,
 				      const char *xattr_name,
 				      void *xattr_value, size_t xattr_value_len,
-				      struct integrity_iint_cache *iint)
+				      struct integrity_iint_cache *iint,
+				      bool force)
 {
-	if (!evm_initialized || !evm_protected_xattr(xattr_name))
+	if ((!evm_initialized || !evm_protected_xattr(xattr_name)) && !force)
 		return INTEGRITY_UNKNOWN;
 
 	if (!iint) {
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index edb82e722a0d..9df1148f17cc 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -217,6 +217,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	struct inode *inode = d_backing_inode(dentry);
 	enum integrity_status status = INTEGRITY_UNKNOWN;
 	int rc = xattr_len, hash_start = 0;
+	bool evm_force = false;
 
 	if (!(inode->i_opflags & IOP_XATTR))
 		return INTEGRITY_UNKNOWN;
@@ -236,7 +237,15 @@ int ima_appraise_measurement(enum ima_hooks func,
 		goto out;
 	}
 
-	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
+	/*
+	 * Check if policy specifies that we should perform EVM
+	 * validation even in the absence of an EVM symmetric key
+	 */
+	if (iint->flags & IMA_EVM_REQUIRED)
+		evm_force = true;
+
+	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint,
+				 evm_force);
 	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
 		if ((status == INTEGRITY_NOLABEL)
 		    || (status == INTEGRITY_NOXATTRS))
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index a6e14c532627..db4a0c968e00 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -531,7 +531,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr
+	Opt_pcr, Opt_require_evm,
 };
 
 static match_table_t policy_tokens = {
@@ -562,6 +562,7 @@ static match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_require_evm, "require_evm"},
 	{Opt_err, NULL}
 };
 
@@ -876,6 +877,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 			else
 				entry->flags |= IMA_PCR;
 
+			break;
+		case Opt_require_evm:
+			if (entry->action != APPRAISE) {
+				result = -EINVAL;
+				break;
+			}
+			entry->flags |= IMA_EVM_REQUIRED;
 			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
@@ -1142,6 +1150,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 			}
 		}
 	}
+	if (entry->flags & IMA_EVM_REQUIRED)
+		seq_puts(m, "require_evm ");
 	if (entry->flags & IMA_DIGSIG_REQUIRED)
 		seq_puts(m, "appraise_type=imasig ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 45ba0e4501d6..2fa0d7bc55fb 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -28,11 +28,12 @@
 
 /* iint cache flags */
 #define IMA_ACTION_FLAGS	0xff000000
-#define IMA_ACTION_RULE_FLAGS	0x06000000
+#define IMA_ACTION_RULE_FLAGS	0x16000000
 #define IMA_DIGSIG		0x01000000
 #define IMA_DIGSIG_REQUIRED	0x02000000
 #define IMA_PERMIT_DIRECTIO	0x04000000
 #define IMA_NEW_FILE		0x08000000
+#define IMA_EVM_REQUIRED	0x10000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_APPRAISE_SUBMASK)
-- 
2.14.2.822.g60be5d43e6-goog

Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key

[Mimi Zohar]

On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote:
> A reasonable configuration is to use IMA to appraise a subset of files
> (based on user, security label or other features supported by IMA) but
> to also want to use EVM to validate not only the state of the IMA hash
> but also additional metadata on the file. Right now this is only
> possible if a symmetric key has been loaded, which may not be desirable
> in all cases (eg, one where EVM digital signatures are shipped to end
> systems rather than EVM HMACs being generated locally). 

Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded"
already allows EVM to be enabled without loading a symmetric key.

Mimi

> Add an
> additional "require_evm" keyword to the IMA policy language in order to
> permit the local admin to indicate that they wish EVM validation to
> occur even if no symmetric key has been loaded.
> 
> Signed-off-by: Matthew Garrett <mjg59@google.com>
> ---
>  Documentation/ABI/testing/ima_policy  |  3 ++-
>  include/linux/evm.h                   |  6 ++++--
>  security/integrity/evm/evm_main.c     |  6 ++++--
>  security/integrity/ima/ima_appraise.c | 11 ++++++++++-
>  security/integrity/ima/ima_policy.c   | 12 +++++++++++-
>  security/integrity/integrity.h        |  3 ++-
>  6 files changed, 33 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
> index 5dc9eed035fb..ea2703c847f6 100644
> --- a/Documentation/ABI/testing/ima_policy
> +++ b/Documentation/ABI/testing/ima_policy
> @@ -23,7 +23,8 @@ Description:
>  				[euid=] [fowner=]]
>  			lsm:	[[subj_user=] [subj_role=] [subj_type=]
>  				 [obj_user=] [obj_role=] [obj_type=]]
> -			option:	[[appraise_type=]] [permit_directio]
> +			option:	[[appraise_type=] [permit_directio]
> +			         [require_evm]]
> 
>  		base: 	func:= [BPRM_CHECK][MMAP_CHECK][CREDS_CHECK][FILE_CHECK][MODULE_CHECK]
>  				[FIRMWARE_CHECK]
> diff --git a/include/linux/evm.h b/include/linux/evm.h
> index 35ed9a8a403a..7661f3085942 100644
> --- a/include/linux/evm.h
> +++ b/include/linux/evm.h
> @@ -19,7 +19,8 @@ extern enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  					     const char *xattr_name,
>  					     void *xattr_value,
>  					     size_t xattr_value_len,
> -					     struct integrity_iint_cache *iint);
> +					     struct integrity_iint_cache *iint,
> +					     bool force);
>  extern int evm_inode_setattr(struct dentry *dentry, struct iattr *attr);
>  extern void evm_inode_post_setattr(struct dentry *dentry, int ia_valid);
>  extern int evm_inode_setxattr(struct dentry *dentry, const char *name,
> @@ -54,7 +55,8 @@ static inline enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  						    const char *xattr_name,
>  						    void *xattr_value,
>  						    size_t xattr_value_len,
> -					struct integrity_iint_cache *iint)
> +					      struct integrity_iint_cache *iint,
> +						    bool force)
>  {
>  	return INTEGRITY_UNKNOWN;
>  }
> diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
> index 063d38aef64e..44e4f4fda965 100644
> --- a/security/integrity/evm/evm_main.c
> +++ b/security/integrity/evm/evm_main.c
> @@ -223,6 +223,7 @@ static int evm_protected_xattr(const char *req_xattr_name)
>   * @xattr_name: requested xattr
>   * @xattr_value: requested xattr value
>   * @xattr_value_len: requested xattr value length
> + * @force: force verification even if no EVM symmetric key is loaded
>   *
>   * Calculate the HMAC for the given dentry and verify it against the stored
>   * security.evm xattr. For performance, use the xattr value and length
> @@ -236,9 +237,10 @@ static int evm_protected_xattr(const char *req_xattr_name)
>  enum integrity_status evm_verifyxattr(struct dentry *dentry,
>  				      const char *xattr_name,
>  				      void *xattr_value, size_t xattr_value_len,
> -				      struct integrity_iint_cache *iint)
> +				      struct integrity_iint_cache *iint,
> +				      bool force)
>  {
> -	if (!evm_initialized || !evm_protected_xattr(xattr_name))
> +	if ((!evm_initialized || !evm_protected_xattr(xattr_name)) && !force)
>  		return INTEGRITY_UNKNOWN;
> 
>  	if (!iint) {
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index edb82e722a0d..9df1148f17cc 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -217,6 +217,7 @@ int ima_appraise_measurement(enum ima_hooks func,
>  	struct inode *inode = d_backing_inode(dentry);
>  	enum integrity_status status = INTEGRITY_UNKNOWN;
>  	int rc = xattr_len, hash_start = 0;
> +	bool evm_force = false;
> 
>  	if (!(inode->i_opflags & IOP_XATTR))
>  		return INTEGRITY_UNKNOWN;
> @@ -236,7 +237,15 @@ int ima_appraise_measurement(enum ima_hooks func,
>  		goto out;
>  	}
> 
> -	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint);
> +	/*
> +	 * Check if policy specifies that we should perform EVM
> +	 * validation even in the absence of an EVM symmetric key
> +	 */
> +	if (iint->flags & IMA_EVM_REQUIRED)
> +		evm_force = true;
> +
> +	status = evm_verifyxattr(dentry, XATTR_NAME_IMA, xattr_value, rc, iint,
> +				 evm_force);
>  	if ((status != INTEGRITY_PASS) && (status != INTEGRITY_UNKNOWN)) {
>  		if ((status == INTEGRITY_NOLABEL)
>  		    || (status == INTEGRITY_NOXATTRS))
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index a6e14c532627..db4a0c968e00 100644
> --- a/security/integrity/ima/ima_policy.c
> +++ b/security/integrity/ima/ima_policy.c
> @@ -531,7 +531,7 @@ enum {
>  	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
>  	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
>  	Opt_appraise_type, Opt_permit_directio,
> -	Opt_pcr
> +	Opt_pcr, Opt_require_evm,
>  };
> 
>  static match_table_t policy_tokens = {
> @@ -562,6 +562,7 @@ static match_table_t policy_tokens = {
>  	{Opt_appraise_type, "appraise_type=%s"},
>  	{Opt_permit_directio, "permit_directio"},
>  	{Opt_pcr, "pcr=%s"},
> +	{Opt_require_evm, "require_evm"},
>  	{Opt_err, NULL}
>  };
> 
> @@ -876,6 +877,13 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
>  			else
>  				entry->flags |= IMA_PCR;
> 
> +			break;
> +		case Opt_require_evm:
> +			if (entry->action != APPRAISE) {
> +				result = -EINVAL;
> +				break;
> +			}
> +			entry->flags |= IMA_EVM_REQUIRED;
>  			break;
>  		case Opt_err:
>  			ima_log_string(ab, "UNKNOWN", p);
> @@ -1142,6 +1150,8 @@ int ima_policy_show(struct seq_file *m, void *v)
>  			}
>  		}
>  	}
> +	if (entry->flags & IMA_EVM_REQUIRED)
> +		seq_puts(m, "require_evm ");
>  	if (entry->flags & IMA_DIGSIG_REQUIRED)
>  		seq_puts(m, "appraise_type=imasig ");
>  	if (entry->flags & IMA_PERMIT_DIRECTIO)
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 45ba0e4501d6..2fa0d7bc55fb 100644
> --- a/security/integrity/integrity.h
> +++ b/security/integrity/integrity.h
> @@ -28,11 +28,12 @@
> 
>  /* iint cache flags */
>  #define IMA_ACTION_FLAGS	0xff000000
> -#define IMA_ACTION_RULE_FLAGS	0x06000000
> +#define IMA_ACTION_RULE_FLAGS	0x16000000
>  #define IMA_DIGSIG		0x01000000
>  #define IMA_DIGSIG_REQUIRED	0x02000000
>  #define IMA_PERMIT_DIRECTIO	0x04000000
>  #define IMA_NEW_FILE		0x08000000
> +#define IMA_EVM_REQUIRED	0x10000000
> 
>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>  				 IMA_APPRAISE_SUBMASK)

Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key

[Matthew Garrett]

On Sat, Sep 30, 2017 at 7:08 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote:
>> A reasonable configuration is to use IMA to appraise a subset of files
>> (based on user, security label or other features supported by IMA) but
>> to also want to use EVM to validate not only the state of the IMA hash
>> but also additional metadata on the file. Right now this is only
>> possible if a symmetric key has been loaded, which may not be desirable
>> in all cases (eg, one where EVM digital signatures are shipped to end
>> systems rather than EVM HMACs being generated locally).
>
> Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded"
> already allows EVM to be enabled without loading a symmetric key.

This only seems to be set if CONFIG_EVM_LOAD_X509 is set. Should there
be some sort of callback to set this if a key is loaded onto the evm
keyring at runtime?

Re: [PATCH 1/6] IMA: Allow EVM validation on appraisal even without a symmetric key

[Mimi Zohar]

On Mon, 2017-10-02 at 10:02 -0700, Matthew Garrett wrote:
> On Sat, Sep 30, 2017 at 7:08 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote:
> >> A reasonable configuration is to use IMA to appraise a subset of files
> >> (based on user, security label or other features supported by IMA) but
> >> to also want to use EVM to validate not only the state of the IMA hash
> >> but also additional metadata on the file. Right now this is only
> >> possible if a symmetric key has been loaded, which may not be desirable
> >> in all cases (eg, one where EVM digital signatures are shipped to end
> >> systems rather than EVM HMACs being generated locally).
> >
> > Commit 26ddabfe96bb "evm: enable EVM when X509 certificate is loaded"
> > already allows EVM to be enabled without loading a symmetric key.
> 
> This only seems to be set if CONFIG_EVM_LOAD_X509 is set. Should there
> be some sort of callback to set this if a key is loaded onto the evm
> keyring at runtime?

Currently writing 1 to the securityfs file causes the EVM key to be
loaded.  I would extend the existing evm_write_key().  Writing 2, for
example, might skip attempting to load the EVM key.  You'll probably
want to make sure that a public key has been loaded first.

Mimi

[PATCH 2/6] EVM: Add infrastructure for making EVM fields optional

[Matthew Garrett]

The set of information used for EVM calculations is currently hardcoded
at kernel build time. Add infrastructure for allowing individual
components to be enabled or disabled, but keep the same default
behaviour for the moment.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 security/integrity/evm/evm.h        | 22 +++++++++++++----
 security/integrity/evm/evm_crypto.c | 48 ++++++++++++++++++++++---------------
 security/integrity/evm/evm_main.c   | 45 +++++++++++++++++++++-------------
 3 files changed, 75 insertions(+), 40 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index f5f12727771a..1d8201b1fb8a 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -28,9 +28,23 @@ extern int evm_initialized;
 extern char *evm_hmac;
 extern char *evm_hash;
 
-#define EVM_ATTR_FSUUID		0x0001
+/* Extended attributes */
+#define EVM_SELINUX (1L << 0)
+#define EVM_SMACK (1L << 1)
+#define EVM_SMACKEXEC (1L << 2)
+#define EVM_SMACKTRANSMUTE (1L << 3)
+#define EVM_SMACKMMAP (1L << 4)
+#define EVM_IMA (1L << 5)
+#define EVM_CAPS (1L << 6)
+/* Other metadata */
+#define EVM_INODE (1L << 32)
+#define EVM_OWNERSHIP (1L << 33)
+#define EVM_MODE (1L << 34)
+#define EVM_FSUUID (1L << 35)
+/* Behavioural flags */
+#define EVM_HMAC_CONVERT (1L << 63)
 
-extern int evm_hmac_attrs;
+extern u64 evm_default_flags;
 
 extern struct crypto_shash *hmac_tfm;
 extern struct crypto_shash *hash_tfm;
@@ -45,10 +59,10 @@ int evm_update_evmxattr(struct dentry *dentry,
 			size_t req_xattr_value_len);
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
 		  const char *req_xattr_value,
-		  size_t req_xattr_value_len, char *digest);
+		  size_t req_xattr_value_len, u64 flags, char *digest);
 int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
 		  const char *req_xattr_value,
-		  size_t req_xattr_value_len, char *digest);
+		  size_t req_xattr_value_len, u64 flags, char *digest);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
 		  char *hmac_val);
 int evm_init_secfs(void);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 1d32cd20009a..9ce55ac6781e 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -138,7 +138,7 @@ static struct shash_desc *init_desc(char type)
  * protection.)
  */
 static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
-			  char *digest)
+			  u64 flags, char *digest)
 {
 	struct h_misc {
 		unsigned long ino;
@@ -149,8 +149,10 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	} hmac_misc;
 
 	memset(&hmac_misc, 0, sizeof(hmac_misc));
-	hmac_misc.ino = inode->i_ino;
-	hmac_misc.generation = inode->i_generation;
+	if (flags & EVM_INODE) {
+		hmac_misc.ino = inode->i_ino;
+		hmac_misc.generation = inode->i_generation;
+	}
 	/* The hmac uid and gid must be encoded in the initial user
 	 * namespace (not the filesystems user namespace) as encoding
 	 * them in the filesystems user namespace allows an attack
@@ -159,11 +161,14 @@ static void hmac_add_misc(struct shash_desc *desc, struct inode *inode,
 	 * filesystem for real on next boot and trust it because
 	 * everything is signed.
 	 */
-	hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
-	hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
-	hmac_misc.mode = inode->i_mode;
+	if (flags & EVM_OWNERSHIP) {
+		hmac_misc.uid = from_kuid(&init_user_ns, inode->i_uid);
+		hmac_misc.gid = from_kgid(&init_user_ns, inode->i_gid);
+	}
+	if (flags & EVM_MODE)
+		hmac_misc.mode = inode->i_mode;
 	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
-	if (evm_hmac_attrs & EVM_ATTR_FSUUID)
+	if (flags & EVM_FSUUID)
 		crypto_shash_update(desc, &inode->i_sb->s_uuid.b[0],
 				    sizeof(inode->i_sb->s_uuid));
 	crypto_shash_final(desc, digest);
@@ -180,15 +185,16 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 				const char *req_xattr_name,
 				const char *req_xattr_value,
 				size_t req_xattr_value_len,
-				char type, char *digest)
+				char type, u64 flags, char *digest)
 {
 	struct inode *inode = d_backing_inode(dentry);
 	struct shash_desc *desc;
-	char **xattrname;
+	char *xattrname;
 	size_t xattr_size = 0;
 	char *xattr_value = NULL;
 	int error;
 	int size;
+	int i;
 
 	if (!(inode->i_opflags & IOP_XATTR))
 		return -EOPNOTSUPP;
@@ -198,15 +204,19 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		return PTR_ERR(desc);
 
 	error = -ENODATA;
-	for (xattrname = evm_config_xattrnames; *xattrname != NULL; xattrname++) {
+	for (i = 0; evm_config_xattrnames[i] != NULL; i++) {
+		if (!(flags & (1 << i)))
+			continue;
+
+		xattrname = evm_config_xattrnames[i];
 		if ((req_xattr_name && req_xattr_value)
-		    && !strcmp(*xattrname, req_xattr_name)) {
+		    && !strcmp(xattrname, req_xattr_name)) {
 			error = 0;
 			crypto_shash_update(desc, (const u8 *)req_xattr_value,
 					     req_xattr_value_len);
 			continue;
 		}
-		size = vfs_getxattr_alloc(dentry, *xattrname,
+		size = vfs_getxattr_alloc(dentry, xattrname,
 					  &xattr_value, xattr_size, GFP_NOFS);
 		if (size == -ENOMEM) {
 			error = -ENOMEM;
@@ -219,7 +229,7 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 		xattr_size = size;
 		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
 	}
-	hmac_add_misc(desc, inode, digest);
+	hmac_add_misc(desc, inode, flags, digest);
 
 out:
 	kfree(xattr_value);
@@ -229,18 +239,18 @@ static int evm_calc_hmac_or_hash(struct dentry *dentry,
 
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
 		  const char *req_xattr_value, size_t req_xattr_value_len,
-		  char *digest)
+		  u64 flags, char *digest)
 {
 	return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value,
-				req_xattr_value_len, EVM_XATTR_HMAC, digest);
+			   req_xattr_value_len, EVM_XATTR_HMAC, flags, digest);
 }
 
 int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
 		  const char *req_xattr_value, size_t req_xattr_value_len,
-		  char *digest)
+		  u64 flags, char *digest)
 {
 	return evm_calc_hmac_or_hash(dentry, req_xattr_name, req_xattr_value,
-				req_xattr_value_len, IMA_XATTR_DIGEST, digest);
+			 req_xattr_value_len, IMA_XATTR_DIGEST, flags, digest);
 }
 
 /*
@@ -256,7 +266,7 @@ int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
 	int rc = 0;
 
 	rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
-			   xattr_value_len, xattr_data.digest);
+			xattr_value_len, evm_default_flags, xattr_data.digest);
 	if (rc == 0) {
 		xattr_data.type = EVM_XATTR_HMAC;
 		rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
@@ -280,7 +290,7 @@ int evm_init_hmac(struct inode *inode, const struct xattr *lsm_xattr,
 	}
 
 	crypto_shash_update(desc, lsm_xattr->value, lsm_xattr->value_len);
-	hmac_add_misc(desc, inode, hmac_val);
+	hmac_add_misc(desc, inode, evm_default_flags, hmac_val);
 	kfree(desc);
 	return 0;
 }
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 44e4f4fda965..52b6fff91f8d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -35,23 +35,32 @@ static char *integrity_status_msg[] = {
 };
 char *evm_hmac = "hmac(sha1)";
 char *evm_hash = "sha1";
-int evm_hmac_attrs;
 
-char *evm_config_xattrnames[] = {
+u64 evm_default_flags =
 #ifdef CONFIG_SECURITY_SELINUX
-	XATTR_NAME_SELINUX,
+	EVM_SELINUX |
 #endif
 #ifdef CONFIG_SECURITY_SMACK
-	XATTR_NAME_SMACK,
+	EVM_SMACK |
 #ifdef CONFIG_EVM_EXTRA_SMACK_XATTRS
-	XATTR_NAME_SMACKEXEC,
-	XATTR_NAME_SMACKTRANSMUTE,
-	XATTR_NAME_SMACKMMAP,
+	EVM_SMACKEXEC |	EVM_SMACKTRANSMUTE | EVM_SMACKMMAP |
 #endif
 #endif
 #ifdef CONFIG_IMA_APPRAISE
-	XATTR_NAME_IMA,
+	EVM_IMA |
 #endif
+#ifdef CONFIG_EVM_ATTR_FSUUID
+	EVM_FSUUID |
+#endif
+	EVM_CAPS | EVM_INODE | EVM_OWNERSHIP | EVM_MODE;
+
+char *evm_config_xattrnames[] = {
+	XATTR_NAME_SELINUX,
+	XATTR_NAME_SMACK,
+	XATTR_NAME_SMACKEXEC,
+	XATTR_NAME_SMACKTRANSMUTE,
+	XATTR_NAME_SMACKMMAP,
+	XATTR_NAME_IMA,
 	XATTR_NAME_CAPS,
 	NULL
 };
@@ -67,24 +76,26 @@ __setup("evm=", evm_set_fixmode);
 
 static void __init evm_init_config(void)
 {
-#ifdef CONFIG_EVM_ATTR_FSUUID
-	evm_hmac_attrs |= EVM_ATTR_FSUUID;
-#endif
-	pr_info("HMAC attrs: 0x%x\n", evm_hmac_attrs);
+	pr_info("HMAC attrs: 0x%llx\n", evm_default_flags);
 }
 
 static int evm_find_protected_xattrs(struct dentry *dentry)
 {
 	struct inode *inode = d_backing_inode(dentry);
-	char **xattr;
+	char *xattr;
 	int error;
 	int count = 0;
+	int i;
 
 	if (!(inode->i_opflags & IOP_XATTR))
 		return -EOPNOTSUPP;
 
-	for (xattr = evm_config_xattrnames; *xattr != NULL; xattr++) {
-		error = __vfs_getxattr(dentry, inode, *xattr, NULL, 0);
+	for (i = 0; evm_config_xattrnames[i] != NULL; i++) {
+		if (!(evm_default_flags & (1 << i)))
+			continue;
+
+		xattr = evm_config_xattrnames[i];
+		error = __vfs_getxattr(dentry, inode, xattr, NULL, 0);
 		if (error < 0) {
 			if (error == -ENODATA)
 				continue;
@@ -152,7 +163,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 			goto out;
 		}
 		rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
-				   xattr_value_len, calc.digest);
+			      xattr_value_len, evm_default_flags, calc.digest);
 		if (rc)
 			break;
 		rc = crypto_memneq(xattr_data->digest, calc.digest,
@@ -162,7 +173,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		break;
 	case EVM_IMA_XATTR_DIGSIG:
 		rc = evm_calc_hash(dentry, xattr_name, xattr_value,
-				xattr_value_len, calc.digest);
+			      xattr_value_len, evm_default_flags, calc.digest);
 		if (rc)
 			break;
 		rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
-- 
2.14.2.822.g60be5d43e6-goog

[PATCH 3/6] EVM: Allow userland to override the default EVM attributes

[Matthew Garrett]

Local policy may wish to provide a different set of metadata to measure
when compared to the kernel defaults. Allow this to be overridden before
the EVM key is initialised, giving userland an opportunity to define it
but locking it down after EVM is enabled.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 security/integrity/evm/evm_secfs.c | 74 +++++++++++++++++++++++++++++++++++---
 1 file changed, 70 insertions(+), 4 deletions(-)

diff --git a/security/integrity/evm/evm_secfs.c b/security/integrity/evm/evm_secfs.c
index c8dccd54d501..b6678f01ec39 100644
--- a/security/integrity/evm/evm_secfs.c
+++ b/security/integrity/evm/evm_secfs.c
@@ -20,6 +20,7 @@
 #include "evm.h"
 
 static struct dentry *evm_init_tpm;
+static struct dentry *evm_init_flags;
 
 /**
  * evm_read_key - read() for <securityfs>/evm
@@ -88,13 +89,78 @@ static const struct file_operations evm_key_ops = {
 	.write		= evm_write_key,
 };
 
-int __init evm_init_secfs(void)
+/**
+ * evm_read_flags - read() for <securityfs>/evm_flags
+ *
+ * @filp: file pointer, not actually used
+ * @buf: where to put the result
+ * @count: maximum to send along
+ * @ppos: where to start
+ *
+ * Returns number of bytes read or error code, as appropriate
+ */
+static ssize_t evm_read_flags(struct file *filp, char __user *buf,
+			      size_t count, loff_t *ppos)
 {
-	int error = 0;
+	char temp[19];
+	ssize_t rc;
+
+	if (*ppos != 0)
+		return 0;
+
+	sprintf(temp, "0x%llx", evm_default_flags);
+	rc = simple_read_from_buffer(buf, count, ppos, temp, strlen(temp));
+
+	return rc;
+}
 
+/**
+ * evm_write_flags - write() for <securityfs>/evm_flags
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * - sets the components that will be measured in the EVM hash
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_flags(struct file *file, const char __user *buf,
+			       size_t count, loff_t *ppos)
+{
+	int err;
+
+	if (!capable(CAP_SYS_ADMIN) || (evm_initialized & EVM_INIT_HMAC))
+		return -EPERM;
+
+	if (evm_initialized & EVM_INIT_HMAC)
+		return -EINVAL;
+
+	err = kstrtoull_from_user(buf, count, 0, &evm_default_flags);
+	if (err)
+		return err;
+
+	return count;
+}
+
+static const struct file_operations evm_flags_ops = {
+	.read		= evm_read_flags,
+	.write		= evm_write_flags,
+};
+
+int __init evm_init_secfs(void)
+{
 	evm_init_tpm = securityfs_create_file("evm", S_IRUSR | S_IRGRP,
 					      NULL, NULL, &evm_key_ops);
 	if (!evm_init_tpm || IS_ERR(evm_init_tpm))
-		error = -EFAULT;
-	return error;
+		return -EFAULT;
+
+	evm_init_flags = securityfs_create_file("evm_flags", S_IRUSR | S_IRGRP,
+						NULL, NULL, &evm_flags_ops);
+
+	if (!evm_init_flags || IS_ERR(evm_init_flags)) {
+		securityfs_remove(evm_init_tpm);
+		return -EFAULT;
+	}
+
+	return 0;
 }
-- 
2.14.2.822.g60be5d43e6-goog

[PATCH 4/6] EVM: Add an hmac_ng xattr format

[Matthew Garrett]

Create an additional HMAC on-disk xattr format, identical to the current
one but with an additional 64 bits of data to indicate which metadata
was used to create the HMAC. Make use of this information when
calculating the value to compare against it.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 security/integrity/evm/evm_main.c | 18 ++++++++++++++++++
 security/integrity/integrity.h    | 11 +++++++++++
 2 files changed, 29 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 52b6fff91f8d..383f003b428e 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -127,6 +127,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 					     struct integrity_iint_cache *iint)
 {
 	struct evm_ima_xattr_data *xattr_data = NULL;
+	struct evm_hmac_ng_data *hmac_ng_data;
 	struct evm_ima_xattr_data calc;
 	enum integrity_status evm_status = INTEGRITY_PASS;
 	int rc, xattr_len;
@@ -190,6 +191,23 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 						    xattr_value_len);
 		}
 		break;
+	case EVM_XATTR_HMAC_NG:
+		hmac_ng_data = (struct evm_hmac_ng_data *)xattr_data;
+		flags = be64_to_cpu(digsig_ng_data->hdr.flags);
+
+		if (xattr_len != sizeof(struct evm_hmac_ng_data)) {
+			evm_status = INTEGRITY_FAIL;
+			goto out;
+		}
+		rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
+				   xattr_value_len, flags, calc.digest);
+		if (rc)
+			break;
+		rc = crypto_memneq(hmac_ng_data->digest, calc.digest,
+				   sizeof(calc.digest));
+		if (rc)
+			rc = -EINVAL;
+		break;
 	default:
 		rc = -EINVAL;
 		break;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 2fa0d7bc55fb..9abd99224916 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -63,6 +63,7 @@ enum evm_ima_xattr_type {
 	EVM_XATTR_HMAC,
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
+	EVM_XATTR_HMAC_NG,
 	IMA_XATTR_LAST
 };
 
@@ -71,6 +72,11 @@ struct evm_ima_xattr_data {
 	u8 digest[SHA1_DIGEST_SIZE];
 } __packed;
 
+struct evm_ima_xattr_ng_hdr {
+	u8 type;
+	__be64 flags;
+} __packed;
+
 #define IMA_MAX_DIGEST_SIZE	64
 
 struct ima_digest_data {
@@ -102,6 +108,11 @@ struct signature_v2_hdr {
 	uint8_t sig[0];		/* signature payload */
 } __packed;
 
+struct evm_hmac_ng_data {
+	struct evm_ima_xattr_ng_hdr hdr;
+	u8 digest[SHA1_DIGEST_SIZE];
+} __packed;
+
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
-- 
2.14.2.822.g60be5d43e6-goog

[PATCH 5/6] EVM: Write out HMAC xattrs in the new format

[Matthew Garrett]

Write out HMACs in the NG format rather than the original format.

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 security/integrity/evm/evm.h        |  2 +-
 security/integrity/evm/evm_crypto.c | 10 ++++++----
 security/integrity/evm/evm_main.c   | 10 ++++++----
 3 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/security/integrity/evm/evm.h b/security/integrity/evm/evm.h
index 1d8201b1fb8a..e4de787508f2 100644
--- a/security/integrity/evm/evm.h
+++ b/security/integrity/evm/evm.h
@@ -56,7 +56,7 @@ int evm_init_key(void);
 int evm_update_evmxattr(struct dentry *dentry,
 			const char *req_xattr_name,
 			const char *req_xattr_value,
-			size_t req_xattr_value_len);
+			size_t req_xattr_value_len, u64 flags);
 int evm_calc_hmac(struct dentry *dentry, const char *req_xattr_name,
 		  const char *req_xattr_value,
 		  size_t req_xattr_value_len, u64 flags, char *digest);
diff --git a/security/integrity/evm/evm_crypto.c b/security/integrity/evm/evm_crypto.c
index 9ce55ac6781e..a00c48c52307 100644
--- a/security/integrity/evm/evm_crypto.c
+++ b/security/integrity/evm/evm_crypto.c
@@ -259,16 +259,18 @@ int evm_calc_hash(struct dentry *dentry, const char *req_xattr_name,
  * Expects to be called with i_mutex locked.
  */
 int evm_update_evmxattr(struct dentry *dentry, const char *xattr_name,
-			const char *xattr_value, size_t xattr_value_len)
+			const char *xattr_value, size_t xattr_value_len,
+			u64 flags)
 {
 	struct inode *inode = d_backing_inode(dentry);
-	struct evm_ima_xattr_data xattr_data;
+	struct evm_hmac_ng_data xattr_data;
 	int rc = 0;
 
 	rc = evm_calc_hmac(dentry, xattr_name, xattr_value,
-			xattr_value_len, evm_default_flags, xattr_data.digest);
+			   xattr_value_len, flags, xattr_data.digest);
 	if (rc == 0) {
-		xattr_data.type = EVM_XATTR_HMAC;
+		xattr_data.hdr.type = EVM_XATTR_HMAC_NG;
+		xattr_data.hdr.flags = cpu_to_be64(flags);
 		rc = __vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
 					   &xattr_data,
 					   sizeof(xattr_data), 0);
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 383f003b428e..77eda423824d 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -188,7 +188,8 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 			    !IS_IMMUTABLE(d_backing_inode(dentry)))
 				evm_update_evmxattr(dentry, xattr_name,
 						    xattr_value,
-						    xattr_value_len);
+						    xattr_value_len,
+						    evm_default_flags);
 		}
 		break;
 	case EVM_XATTR_HMAC_NG:
@@ -427,7 +428,8 @@ void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 
 	evm_reset_status(dentry->d_inode);
 
-	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
+	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len,
+			    evm_default_flags);
 }
 
 /**
@@ -447,7 +449,7 @@ void evm_inode_post_removexattr(struct dentry *dentry, const char *xattr_name)
 
 	evm_reset_status(dentry->d_inode);
 
-	evm_update_evmxattr(dentry, xattr_name, NULL, 0);
+	evm_update_evmxattr(dentry, xattr_name, NULL, 0, evm_default_flags);
 }
 
 /**
@@ -488,7 +490,7 @@ void evm_inode_post_setattr(struct dentry *dentry, int ia_valid)
 		return;
 
 	if (ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID))
-		evm_update_evmxattr(dentry, NULL, NULL, 0);
+		evm_update_evmxattr(dentry, NULL, NULL, 0, evm_default_flags);
 }
 
 /*
-- 
2.14.2.822.g60be5d43e6-goog

[PATCH 6/6] EVM: Add a new digital signature format

[Matthew Garrett]

As a companion to

EVM: Add an hmac_ng xattr format

add an equivalent format for digital signatures, identical to the
previous format but with an additional 64 bits of information about
which metadata was used to generate the signature. This allows for
distributing digital signatures that protect the metadata without having
to include the inode number (something that's not known in advance)

Signed-off-by: Matthew Garrett <mjg59@google.com>
---
 security/integrity/evm/evm_main.c | 26 ++++++++++++++++++++++++++
 security/integrity/integrity.h    |  6 ++++++
 2 files changed, 32 insertions(+)

diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 77eda423824d..a58ff5d8caf6 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -128,9 +128,11 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 {
 	struct evm_ima_xattr_data *xattr_data = NULL;
 	struct evm_hmac_ng_data *hmac_ng_data;
+	struct evm_ima_xattr_ng_data *digsig_ng_data;
 	struct evm_ima_xattr_data calc;
 	enum integrity_status evm_status = INTEGRITY_PASS;
 	int rc, xattr_len;
+	u64 flags;
 
 	if (iint && iint->evm_status == INTEGRITY_PASS)
 		return iint->evm_status;
@@ -209,6 +211,30 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		if (rc)
 			rc = -EINVAL;
 		break;
+	case EVM_IMA_XATTR_DIGSIG_NG:
+		digsig_ng_data = (struct evm_ima_xattr_ng_data *)xattr_data;
+		flags = be64_to_cpu(digsig_ng_data->hdr.flags);
+
+		rc = evm_calc_hash(dentry, xattr_name, xattr_value,
+				   xattr_value_len, flags, calc.digest);
+		if (rc)
+			break;
+		rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
+				 (const char *)&digsig_ng_data->sig,
+				 xattr_len-sizeof(struct evm_ima_xattr_ng_hdr),
+				 calc.digest, sizeof(calc.digest));
+		if (!rc) {
+			/* Replace RSA with HMAC if not mounted readonly and
+			 * not immutable
+			 */
+			if (!IS_RDONLY(d_backing_inode(dentry)) &&
+			    !IS_IMMUTABLE(d_backing_inode(dentry)))
+				evm_update_evmxattr(dentry, xattr_name,
+						    xattr_value,
+						    xattr_value_len,
+						    evm_default_flags);
+		}
+		break;
 	default:
 		rc = -EINVAL;
 		break;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 9abd99224916..f41ccf42df65 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -64,6 +64,7 @@ enum evm_ima_xattr_type {
 	EVM_IMA_XATTR_DIGSIG,
 	IMA_XATTR_DIGEST_NG,
 	EVM_XATTR_HMAC_NG,
+	EVM_IMA_XATTR_DIGSIG_NG,
 	IMA_XATTR_LAST
 };
 
@@ -113,6 +114,11 @@ struct evm_hmac_ng_data {
 	u8 digest[SHA1_DIGEST_SIZE];
 } __packed;
 
+struct evm_ima_xattr_ng_data {
+	struct evm_ima_xattr_ng_hdr hdr;
+	struct signature_v2_hdr sig;
+} __packed;
+
 /* integrity data associated with an inode */
 struct integrity_iint_cache {
 	struct rb_node rb_node;	/* rooted in integrity_iint_tree */
-- 
2.14.2.822.g60be5d43e6-goog

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

Hi Matthew,

[Cc'ing Mikhail Kurinnoi]

On Wed, 2017-09-27 at 15:16 -0700, Matthew Garrett wrote:
> These are basically untested, but I'd like to get some feedback on the
> problem I'm trying to solve here. We'd like to be able to ship packages
> with verifiable security xattrs, but right now EVM makes this difficult
> due to its requirement that the inode number be encoded in the hmac. This
> patchset is intended to make it possible to protect a subset of metadata
> rather than all of it, and also to permit using EVM digital signatures in
> a similar way to how IMA digital signatures can be used now (ie, protecting
> the metadata using public/private crypto rather than having a local
> symmetric key and generating the HMACs locally). The expected workflow is:
> 
> 1) During package build or mirroring process, appropriate security metadata
>    is added (IMA hash, selinux label, etc)
> 2) An EVM digital signature is generated based purely on the security
>    metadata present during the build or mirroring process
> 3) IMA is extended to allow it to force EVM validation during appraisal even
>    if no symmetric EVM key has been added, which allows IMA appraisal to
>    appraise not only the IMA hash but also the additional metadata
> 4) If EVM is never enabled, binaries are purely validated using the EVM
>    digital signatures and are not transitioned to using HMACs
> 5) If EVM is desired, userland can set the set of metadata to be incorporated
>    into the EVM HMAC before enabling EVM

Earlier this year there were discussions on defining a portable EVM
signature, that could be included in software packages. 

The reason for including as much metadata as possible in the HMAC is
to limit cut & paste attacks.  For this reason, the portable data is
only used in transmission, not on disk.

A new EVM type is defined that does not convert the EVM signature to
an HMAC.

Mikhail's patches:
https://sourceforge.net/p/linux-ima/mailman/linux-ima-user/thread/2017
0113072602.4ffaa30a@totoro/

I've been negligent in reviewing and testing his patches.  Perhaps
they will meet your needs.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Thu, Sep 28, 2017 at 1:12 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Earlier this year there were discussions on defining a portable EVM
> signature, that could be included in software packages.
>
> The reason for including as much metadata as possible in the HMAC is
> to limit cut & paste attacks.  For this reason, the portable data is
> only used in transmission, not on disk.

The concern is that two identical copies of a file may exist, but with
different security contexts, and that not protecting the inode would
allow an attacker to copy the security metadata from one file onto the
other? This presumably only has meaning if they're on separate
filesystems, since otherwise the attacker could just delete one file
and hardlink the other to the former's location? I think that for our
purposes this isn't a big deal.

> A new EVM type is defined that does not convert the EVM signature to
> an HMAC.
>
> Mikhail's patches:
> https://sourceforge.net/p/linux-ima/mailman/linux-ima-user/thread/2017
> 0113072602.4ffaa30a@totoro/

That looks broadly like what we want, but I think there's some
advantage in maintaining the flexibility of choosing which information
is embedded. One additional option would be to allow userland to place
a restriction on which options *must* be present, ie local policy
could refuse to allow any signatures that didn't include a specific
set of metadata.

One of the reasons we're interested in allowing the use of signatures
rather than HMACs is to avoid the case where a machine being
compromised would allow an attacker to obtain the symmetric key and
drop new appropriately HMACed binaries on the system that would
persist even if the kernel was updated to fix the vulnerability.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Thu, 2017-09-28 at 14:13 -0700, Matthew Garrett wrote:
> On Thu, Sep 28, 2017 at 1:12 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Earlier this year there were discussions on defining a portable EVM
> > signature, that could be included in software packages.
> >
> > The reason for including as much metadata as possible in the HMAC is
> > to limit cut & paste attacks.  For this reason, the portable data is
> > only used in transmission, not on disk.
> 
> The concern is that two identical copies of a file may exist, but with
> different security contexts, and that not protecting the inode would
> allow an attacker to copy the security metadata from one file onto the
> other? This presumably only has meaning if they're on separate
> filesystems, since otherwise the attacker could just delete one file
> and hardlink the other to the former's location? I think that for our
> purposes this isn't a big deal.

Without the inode included in the HMAC calculation, the same file
could exist as a different file name on the same file system.  No need
for the two files to be on different file systems.

> > A new EVM type is defined that does not convert the EVM signature to
> > an HMAC.
> >
> > Mikhail's patches:
> > https://sourceforge.net/p/linux-ima/mailman/linux-ima-user/thread/2017
> > 0113072602.4ffaa30a@totoro/
> 
> That looks broadly like what we want, but I think there's some
> advantage in maintaining the flexibility of choosing which information
> is embedded. One additional option would be to allow userland to place
> a restriction on which options *must* be present, ie local policy
> could refuse to allow any signatures that didn't include a specific
> set of metadata.

This introduces a new level of complexity, which I'm not sure is warranted.

> One of the reasons we're interested in allowing the use of signatures
> rather than HMACs is to avoid the case where a machine being
> compromised would allow an attacker to obtain the symmetric key and
> drop new appropriately HMACed binaries on the system that would
> persist even if the kernel was updated to fix the vulnerability.

Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
the EVM key (trusted key), then such an attack would require root
privileges with the ability to read kernel memory.  The EVM key is
never exposed to userspace in the clear.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Thu, Sep 28, 2017 at 5:53 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Without the inode included in the HMAC calculation, the same file
> could exist as a different file name on the same file system.  No need
> for the two files to be on different file systems.

But if I create a hardlink to an existing file then I get the same
file with the same inode number and two different names on the same
filesystem, and so security.evm would still match?


>> One of the reasons we're interested in allowing the use of signatures
>> rather than HMACs is to avoid the case where a machine being
>> compromised would allow an attacker to obtain the symmetric key and
>> drop new appropriately HMACed binaries on the system that would
>> persist even if the kernel was updated to fix the vulnerability.
>
> Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
> the EVM key (trusted key), then such an attack would require root
> privileges with the ability to read kernel memory.  The EVM key is
> never exposed to userspace in the clear.

That's a case that we need to be worried about. Trusted boot means we
can ensure that a system boots an updated kernel, but if the attacker
has been able to drop a modified sshd with a valid hmac onto the
system then we have fewer guarantees about the integrity. We could
continue using signatures for security.ima to avoid that, but then we
lose the performance benefits of the hmac and also don't have the same
level of guarantees around the other security metadata.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Fri, 2017-09-29 at 11:09 -0700, Matthew Garrett wrote:
> On Thu, Sep 28, 2017 at 5:53 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Without the inode included in the HMAC calculation, the same file
> > could exist as a different file name on the same file system.  No need
> > for the two files to be on different file systems.
> 
> But if I create a hardlink to an existing file then I get the same
> file with the same inode number and two different names on the same
> filesystem, and so security.evm would still match?

True, with a hard link that would be the case, but by the same file, I
meant a copy of the original file, not a hard link to the file.

> >> One of the reasons we're interested in allowing the use of signatures
> >> rather than HMACs is to avoid the case where a machine being
> >> compromised would allow an attacker to obtain the symmetric key and
> >> drop new appropriately HMACed binaries on the system that would
> >> persist even if the kernel was updated to fix the vulnerability.
> >
> > Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
> > the EVM key (trusted key), then such an attack would require root
> > privileges with the ability to read kernel memory.  The EVM key is
> > never exposed to userspace in the clear.
> 
> That's a case that we need to be worried about. Trusted boot means we
> can ensure that a system boots an updated kernel, but if the attacker
> has been able to drop a modified sshd with a valid hmac onto the
> system then we have fewer guarantees about the integrity. We could
> continue using signatures for security.ima to avoid that, but then we
> lose the performance benefits of the hmac and also don't have the same
> level of guarantees around the other security metadata.

I think you mean "secure boot", not "trusted boot", in this case.

The original understanding was that IMA/EVM would enforce integrity
and not enforce mandatory access control (MAC).  The LSMs (eg.
SELinux) would be responsible for MAC.  Separation of duties.

With that understanding, if the LSM allows a file to be "dropped" onto
 the file system of a running system, IMA/EVM will hash and hmac the
permitted file.

I don't understand where you're going with this train of thought.  If
you're trying to make a case for EVM to run with only security.evm
signatures, then you wouldn't refer to the HMAC benefits.  If you're
trying to make a case for EVM signatures, with the inode they're not
portable, without the inode, they are susceptible to a cut and paste
attack.

Mickhail's proposed patches resolves this by having a portable EVM
signature that is never written to disk, but converted to an HMAC.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Fri, Sep 29, 2017 at 12:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2017-09-29 at 11:09 -0700, Matthew Garrett wrote:
>> On Thu, Sep 28, 2017 at 5:53 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> > Without the inode included in the HMAC calculation, the same file
>> > could exist as a different file name on the same file system.  No need
>> > for the two files to be on different file systems.
>>
>> But if I create a hardlink to an existing file then I get the same
>> file with the same inode number and two different names on the same
>> filesystem, and so security.evm would still match?
>
> True, with a hard link that would be the case, but by the same file, I
> meant a copy of the original file, not a hard link to the file.

The difference doesn't seem that great, really? If I can copy a file
while retaining ownership, I can create a hardlink.

>> > Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
>> > the EVM key (trusted key), then such an attack would require root
>> > privileges with the ability to read kernel memory.  The EVM key is
>> > never exposed to userspace in the clear.
>>
>> That's a case that we need to be worried about. Trusted boot means we
>> can ensure that a system boots an updated kernel, but if the attacker
>> has been able to drop a modified sshd with a valid hmac onto the
>> system then we have fewer guarantees about the integrity. We could
>> continue using signatures for security.ima to avoid that, but then we
>> lose the performance benefits of the hmac and also don't have the same
>> level of guarantees around the other security metadata.
>
> I think you mean "secure boot", not "trusted boot", in this case.

No, trusted - secure boot wouldn't reliably tell us which version of a
kernel had booted, trusted boot will.

> The original understanding was that IMA/EVM would enforce integrity
> and not enforce mandatory access control (MAC).  The LSMs (eg.
> SELinux) would be responsible for MAC.  Separation of duties.

The kernel's security track record isn't sufficient to be able to
assert that any kernel secrets can be kept secret, so we're left
wanting integrity guarantees without relying on that.

> With that understanding, if the LSM allows a file to be "dropped" onto
>  the file system of a running system, IMA/EVM will hash and hmac the
> permitted file.
>
> I don't understand where you're going with this train of thought.  If
> you're trying to make a case for EVM to run with only security.evm
> signatures, then you wouldn't refer to the HMAC benefits.  If you're
> trying to make a case for EVM signatures, with the inode they're not
> portable, without the inode, they are susceptible to a cut and paste
> attack.

I'm arguing that (for our case at least) the only way we can use IMA
is to rely on using a digital signature - we can either have that
digital signature be in security.ima, or we can have it in
security.evm. Since we need that signature in any case, we derive no
benefit from having security.evm be an hmac - our two reasonable
choices are:

1) security.ima as a digital signature, security.evm as an hmac
2) security.ima as a hash, security.evm as a digital signature

I'm not really clear on what attacks are prevented by using the inode
number. If I'm able to preserve all the other security metadata when
copying a file, I can just create a hardlink to the original instead
and have the same outcome.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Fri, 2017-09-29 at 12:17 -0700, Matthew Garrett wrote:

> > The original understanding was that IMA/EVM would enforce integrity
> > and not enforce mandatory access control (MAC).  The LSMs (eg.
> > SELinux) would be responsible for MAC.  Separation of duties.
> 
> The kernel's security track record isn't sufficient to be able to
> assert that any kernel secrets can be kept secret, so we're left
> wanting integrity guarantees without relying on that.
> 
> > With that understanding, if the LSM allows a file to be "dropped" onto
> >  the file system of a running system, IMA/EVM will hash and hmac the
> > permitted file.
> >
> > I don't understand where you're going with this train of thought.  If
> > you're trying to make a case for EVM to run with only security.evm
> > signatures, then you wouldn't refer to the HMAC benefits.  If you're
> > trying to make a case for EVM signatures, with the inode they're not
> > portable, without the inode, they are susceptible to a cut and paste
> > attack.
> 
> I'm arguing that (for our case at least) the only way we can use IMA
> is to rely on using a digital signature - we can either have that
> digital signature be in security.ima, or we can have it in
> security.evm. Since we need that signature in any case, we derive no
> benefit from having security.evm be an hmac - our two reasonable
> choices are:
> 
> 1) security.ima as a digital signature, security.evm as an hmac
> 2) security.ima as a hash, security.evm as a digital signature

There's a major difference between security.ima containing a file hash
vs a signature.  A signature, assuming the file_check hook is in
policy, prevents the file from being modified, making the file
"immutable".

> I'm not really clear on what attacks are prevented by using the inode
> number. If I'm able to preserve all the other security metadata when
> copying a file, I can just create a hardlink to the original instead
> and have the same outcome.

The issue is the ability of having different security metadata, not
the same security metadata. (I need to refresh my memory as to hard
links, and whether they can have different security metadata.)

Originally the UUID was not included in the HMAC calculation.  Perhaps
the patch description of commit 74de668 "evm: add file system uuid to
EVM hmac" will help clarify the reasoning for including the inode and
uuid in the hmac calculation.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Fri, Sep 29, 2017 at 1:01 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2017-09-29 at 12:17 -0700, Matthew Garrett wrote:
>> I'm arguing that (for our case at least) the only way we can use IMA
>> is to rely on using a digital signature - we can either have that
>> digital signature be in security.ima, or we can have it in
>> security.evm. Since we need that signature in any case, we derive no
>> benefit from having security.evm be an hmac - our two reasonable
>> choices are:
>>
>> 1) security.ima as a digital signature, security.evm as an hmac
>> 2) security.ima as a hash, security.evm as a digital signature
>
> There's a major difference between security.ima containing a file hash
> vs a signature.  A signature, assuming the file_check hook is in
> policy, prevents the file from being modified, making the file
> "immutable".

But the same is effectively true if security.evm is a digital
signature and there's no symmetric key? For what we want to do (ensure
that executables that are allowed to run with elevated privileges
haven't been tampered with), that's completely ok.

>> I'm not really clear on what attacks are prevented by using the inode
>> number. If I'm able to preserve all the other security metadata when
>> copying a file, I can just create a hardlink to the original instead
>> and have the same outcome.
>
> The issue is the ability of having different security metadata, not
> the same security metadata. (I need to refresh my memory as to hard
> links, and whether they can have different security metadata.)

If the security metadata is different then copying another
security.evm will fail, surely?

> Originally the UUID was not included in the HMAC calculation.  Perhaps
> the patch description of commit 74de668 "evm: add file system uuid to
> EVM hmac" will help clarify the reasoning for including the inode and
> uuid in the hmac calculation.

Not really - if you're not crossing a filesystem boundary then you're
still able to just create a hardlink.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Fri, 2017-09-29 at 13:09 -0700, Matthew Garrett wrote:
> On Fri, Sep 29, 2017 at 1:01 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Fri, 2017-09-29 at 12:17 -0700, Matthew Garrett wrote:
> >> I'm arguing that (for our case at least) the only way we can use IMA
> >> is to rely on using a digital signature - we can either have that
> >> digital signature be in security.ima, or we can have it in
> >> security.evm. Since we need that signature in any case, we derive no
> >> benefit from having security.evm be an hmac - our two reasonable
> >> choices are:
> >>
> >> 1) security.ima as a digital signature, security.evm as an hmac
> >> 2) security.ima as a hash, security.evm as a digital signature
> >
> > There's a major difference between security.ima containing a file hash
> > vs a signature.  A signature, assuming the file_check hook is in
> > policy, prevents the file from being modified, making the file
> > "immutable".
> 
> But the same is effectively true if security.evm is a digital
> signature and there's no symmetric key? For what we want to do (ensure
> that executables that are allowed to run with elevated privileges
> haven't been tampered with), that's completely ok.
> 
> >> I'm not really clear on what attacks are prevented by using the inode
> >> number. If I'm able to preserve all the other security metadata when
> >> copying a file, I can just create a hardlink to the original instead
> >> and have the same outcome.
> >
> > The issue is the ability of having different security metadata, not
> > the same security metadata. (I need to refresh my memory as to hard
> > links, and whether they can have different security metadata.)
> 
> If the security metadata is different then copying another
> security.evm will fail, surely?

A copy of the file could exist with a valid hmac on the system with
different security xattrs.  Without the inode/uuid, the xattrs could
be cut & pasted.

Ok, I agree this would be less of an issue for security.evm
signatures, since the signatures are not being generated on the
locally running system.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Sat, Sep 30, 2017 at 7:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Fri, 2017-09-29 at 13:09 -0700, Matthew Garrett wrote:
>> If the security metadata is different then copying another
>> security.evm will fail, surely?
>
> A copy of the file could exist with a valid hmac on the system with
> different security xattrs.  Without the inode/uuid, the xattrs could
> be cut & pasted.

So we have /usr/bin/a and /usr/bin/b, which are identical but have
different security contexts. Outside some unusual cases, if I have the
ability to modify /usr/bin/b's security.evm, I can delete /usr/bin/b.
I can then also just do:

ln -f /usr/bin/a /usr/bin/b

and /usr/bin/b now has the same security context as /usr/bin/a,
including security.evm.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Mon, 2017-10-02 at 10:09 -0700, Matthew Garrett wrote:
> On Sat, Sep 30, 2017 at 7:36 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Fri, 2017-09-29 at 13:09 -0700, Matthew Garrett wrote:
> >> If the security metadata is different then copying another
> >> security.evm will fail, surely?
> >
> > A copy of the file could exist with a valid hmac on the system with
> > different security xattrs.  Without the inode/uuid, the xattrs could
> > be cut & pasted.
> 
> So we have /usr/bin/a and /usr/bin/b, which are identical but have
> different security contexts. Outside some unusual cases, if I have the
> ability to modify /usr/bin/b's security.evm, I can delete /usr/bin/b.
> I can then also just do:
> 
> ln -f /usr/bin/a /usr/bin/b
> 
> and /usr/bin/b now has the same security context as /usr/bin/a,
> including security.evm.

All true, but instead of /usr/bin/a and /usr/bin/b, which most likely
have the same LSM labels, think of it in terms of /home/bin/b.  An
offline attack wouldn't require access to the EVM HMAC key or any
privileges.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Mon, Oct 2, 2017 at 1:02 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Mon, 2017-10-02 at 13:00 -0700, Matthew Garrett wrote:
>> If /home is on the same filesystem as / (which is a pretty common
>> case) then ln -f /usr/bin/a /home/bin/b will work just as well.
>
> Yes, assuming you have permissions on the running system.

But if I don't have permissions, I can't just cut and paste
security.evm either - even if I can write to security.evm, the only
way the HMAC would match would be if I could change the ownership as
well, which implies being root.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

> >> I'm not really clear on what attacks are prevented by using the inode
> >> number. If I'm able to preserve all the other security metadata when
> >> copying a file, I can just create a hardlink to the original instead
> >> and have the same outcome.
> >
> > The issue is the ability of having different security metadata, not
> > the same security metadata. (I need to refresh my memory as to hard
> > links, and whether they can have different security metadata.)
> 
> If the security metadata is different then copying another
> security.evm will fail, surely?

The assumption here is that security.ima exists and is included in the
HMAC calculation.  For files which are not included in the IMA policy,
the only thing binding the file data and metadata is the i_ino and
uuid.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Mon, Oct 9, 2017 at 10:51 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> >> I'm not really clear on what attacks are prevented by using the inode
>> >> number. If I'm able to preserve all the other security metadata when
>> >> copying a file, I can just create a hardlink to the original instead
>> >> and have the same outcome.
>> >
>> > The issue is the ability of having different security metadata, not
>> > the same security metadata. (I need to refresh my memory as to hard
>> > links, and whether they can have different security metadata.)
>>
>> If the security metadata is different then copying another
>> security.evm will fail, surely?
>
> The assumption here is that security.ima exists and is included in the
> HMAC calculation.  For files which are not included in the IMA policy,
> the only thing binding the file data and metadata is the i_ino and
> uuid.

Ok, that makes sense. But for cases where we do have security.ima, the
inode doesn't seem to provide additional security but does make
deployment more difficult. Does supporting this use case seem
reasonable?

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Mon, 2017-10-09 at 10:59 -0700, Matthew Garrett wrote:
> On Mon, Oct 9, 2017 at 10:51 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> >> I'm not really clear on what attacks are prevented by using the inode
> >> >> number. If I'm able to preserve all the other security metadata when
> >> >> copying a file, I can just create a hardlink to the original instead
> >> >> and have the same outcome.
> >> >
> >> > The issue is the ability of having different security metadata, not
> >> > the same security metadata. (I need to refresh my memory as to hard
> >> > links, and whether they can have different security metadata.)
> >>
> >> If the security metadata is different then copying another
> >> security.evm will fail, surely?
> >
> > The assumption here is that security.ima exists and is included in the
> > HMAC calculation.  For files which are not included in the IMA policy,
> > the only thing binding the file data and metadata is the i_ino and
> > uuid.
> 
> Ok, that makes sense. But for cases where we do have security.ima, the
> inode doesn't seem to provide additional security but does make
> deployment more difficult. Does supporting this use case seem
> reasonable?

Yes!

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Mon, Oct 9, 2017 at 11:15 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Mon, 2017-10-09 at 10:59 -0700, Matthew Garrett wrote:
>> Ok, that makes sense. But for cases where we do have security.ima, the
>> inode doesn't seem to provide additional security but does make
>> deployment more difficult. Does supporting this use case seem
>> reasonable?
>
> Yes!

Excellent. This means defining a new signature type - the two options
seem to be Mikhail's portable format, or the approach I took of having
the signature define which metadata is included. Do you have a
preference?

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Mon, 2017-10-09 at 11:18 -0700, Matthew Garrett wrote:
> On Mon, Oct 9, 2017 at 11:15 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > On Mon, 2017-10-09 at 10:59 -0700, Matthew Garrett wrote:
> >> Ok, that makes sense. But for cases where we do have security.ima, the
> >> inode doesn't seem to provide additional security but does make
> >> deployment more difficult. Does supporting this use case seem
> >> reasonable?
> >
> > Yes!
> 
> Excellent. This means defining a new signature type - the two options
> seem to be Mikhail's portable format, or the approach I took of having
> the signature define which metadata is included. Do you have a
> preference?

We now understand that as long as the EVM signature includes
security.ima, it is safe not to include the i_ino/uuid.  This new
format can be written to disk.

Based on the previous discussions, Mikhail's patches never write the
portable EVM signature format to disk, but verify the signature,
before calculating and writing the HMAC.  Based on our current
understanding that isn't required.  The new EVM signature can be
written out.

Let's keep the change as simple as possible.

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

[Cc'ing Dmitry Kasatkin]

On Tue, 2017-10-10 at 00:33 +0300, Mikhail Kurinnoi wrote:
> ? Mon, 09 Oct 2017 17:10:49 -0400
> Mimi Zohar <zohar@linux.vnet.ibm.com> ?????:
> 
> > On Mon, 2017-10-09 at 23:23 +0300, Mikhail Kurinnoi wrote:
> > > ? Mon, 09 Oct 2017 14:40:41 -0400
> > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????:
> > >   
> > > > On Mon, 2017-10-09 at 11:18 -0700, Matthew Garrett wrote:  
> > > > > On Mon, Oct 9, 2017 at 11:15 AM, Mimi Zohar
> > > > > <zohar@linux.vnet.ibm.com> wrote:    
> > > > > > On Mon, 2017-10-09 at 10:59 -0700, Matthew Garrett wrote:    
> > > > > >> Ok, that makes sense. But for cases where we do have
> > > > > >> security.ima, the inode doesn't seem to provide additional
> > > > > >> security but does make deployment more difficult. Does
> > > > > >> supporting this use case seem reasonable?    
> > > > > >
> > > > > > Yes!    
> > > > > 
> > > > > Excellent. This means defining a new signature type - the two
> > > > > options seem to be Mikhail's portable format, or the approach I
> > > > > took of having the signature define which metadata is included.
> > > > > Do you have a preference?    
> > > > 
> > > > We now understand that as long as the EVM signature includes
> > > > security.ima, it is safe not to include the i_ino/uuid.  This new
> > > > format can be written to disk.  
> > > 
> > > But, isn't this mean we could have this scenario of offline
> > > manipulations:
> > > 1) store old file with xattrs;
> > > 2) wait;
> > > 3) replace new file with fixed exploits on old one.
> > > 
> > > Since we don't have directory tree protection yet and we don't use
> > > i_ino, someone could reuse old files more easy during offline
> > > manipulations. Right?  
> > 
> > As long as the new EVM signature format requires the existence of a
> > security.ima xattr, I don't see how.  The new EVM signature format
> > would not be replaced with an HMAC.
> 
> 
> I understood the idea.
> 
> Looks like portable EVM format support proposed by Matthew, could be
> easy realized. Probably, will be good idea add only this one, test it,
> and later add immutable EVM format and portable EVM format that
> converted into HMAC (that I proposed in patch set)?
> 
> If no one ask for portable EVM format that converted into HMAC, do we
> really need push all in one bunch?

It looks like Dmitry already defined a new "immutable" EVM  signature
format in ima-evm-utils and posted the corresponding patch to linux-
ima-devel a long time ago.

The new "immutable" EVM signature doesn't include the i_ino or
generation.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Mikhail Kurinnoi]

? Mon, 09 Oct 2017 17:40:53 -0400
Mimi Zohar <zohar@linux.vnet.ibm.com> ?????:

> [Cc'ing Dmitry Kasatkin]
> 
> On Tue, 2017-10-10 at 00:33 +0300, Mikhail Kurinnoi wrote:
> > ? Mon, 09 Oct 2017 17:10:49 -0400
> > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????:
> >   
> > > On Mon, 2017-10-09 at 23:23 +0300, Mikhail Kurinnoi wrote:  
> > > > ? Mon, 09 Oct 2017 14:40:41 -0400
> > > > Mimi Zohar <zohar@linux.vnet.ibm.com> ?????:
> > > >     
> > > > > On Mon, 2017-10-09 at 11:18 -0700, Matthew Garrett wrote:    
> > > > > > On Mon, Oct 9, 2017 at 11:15 AM, Mimi Zohar
> > > > > > <zohar@linux.vnet.ibm.com> wrote:      
> > > > > > > On Mon, 2017-10-09 at 10:59 -0700, Matthew Garrett
> > > > > > > wrote:      
> > > > > > >> Ok, that makes sense. But for cases where we do have
> > > > > > >> security.ima, the inode doesn't seem to provide
> > > > > > >> additional security but does make deployment more
> > > > > > >> difficult. Does supporting this use case seem
> > > > > > >> reasonable?      
> > > > > > >
> > > > > > > Yes!      
> > > > > > 
> > > > > > Excellent. This means defining a new signature type - the
> > > > > > two options seem to be Mikhail's portable format, or the
> > > > > > approach I took of having the signature define which
> > > > > > metadata is included. Do you have a preference?      
> > > > > 
> > > > > We now understand that as long as the EVM signature includes
> > > > > security.ima, it is safe not to include the i_ino/uuid.  This
> > > > > new format can be written to disk.    
> > > > 
> > > > But, isn't this mean we could have this scenario of offline
> > > > manipulations:
> > > > 1) store old file with xattrs;
> > > > 2) wait;
> > > > 3) replace new file with fixed exploits on old one.
> > > > 
> > > > Since we don't have directory tree protection yet and we don't
> > > > use i_ino, someone could reuse old files more easy during
> > > > offline manipulations. Right?    
> > > 
> > > As long as the new EVM signature format requires the existence of
> > > a security.ima xattr, I don't see how.  The new EVM signature
> > > format would not be replaced with an HMAC.  
> > 
> > 
> > I understood the idea.
> > 
> > Looks like portable EVM format support proposed by Matthew, could be
> > easy realized. Probably, will be good idea add only this one, test
> > it, and later add immutable EVM format and portable EVM format that
> > converted into HMAC (that I proposed in patch set)?
> > 
> > If no one ask for portable EVM format that converted into HMAC, do
> > we really need push all in one bunch?  
> 
> It looks like Dmitry already defined a new "immutable" EVM  signature
> format in ima-evm-utils and posted the corresponding patch to linux-
> ima-devel a long time ago.
> 
> The new "immutable" EVM signature doesn't include the i_ino or
> generation.

Yes. Btw, Dmitry's patch is the base of proposed by me "portable"
version support patch:
https://sourceforge.net/p/linux-ima/mailman/message/35587348/
since for portable format we use same idea - EVM signature format
without i_ino/generation/uuid.
Plus, that was the reason for keep for "portable" format
signature version number "3", since Dmitry already did all changes in
ima-evm-utils, portable format could be tested.

In the same time, I proposed "immutable" EVM signature, that
identically to current EVM digsig (with i_ino,...), but can't be
changed into HMAC. This patch don't related to Dmitry's "immutable" EVM
signature format support:
https://sourceforge.net/p/linux-ima/mailman/message/35601151/
I didn't really work on "immutable" EVM signature, only "portable"
version support needs was taken into account.

For now, we don't have ready for upstream "immutable" EVM signature
format support patch. Both, Dmitry's and my, patches need more work
in order to prevent file's data changes (in case of IMA hash) and
metadata changes for files signed by "immutable" EVM xattr (same idea
as we already have for IMA digsig, that prevent file's data change).


-- 
Best regards,
Mikhail Kurinnoi

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Tue, 2017-10-10 at 02:10 +0300, Mikhail Kurinnoi wrote:
> For now, we don't have ready for upstream "immutable" EVM signature
> format support patch. Both, Dmitry's and my, patches need more work
> in order to prevent file's data changes (in case of IMA hash) and
> metadata changes for files signed by "immutable" EVM xattr (same idea
> as we already have for IMA digsig, that prevent file's data change).

After looking at your patches again, I think we should combine the
"immutable" and "portable" concepts so that the new "portable"
signature type is written out and considered "immutable". 

Dmitry's patch does prevent the file from changing, but that code is
in IMA, but should be in EVM.  I agree we can defer preventing the
file from changing.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

Hi all,

[switched to plain text]

I will check Mikhail's patches.
Give me a moment.

Thanks,
Dmitry


On Tue, Oct 10, 2017 at 10:07 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Tue, 2017-10-10 at 02:10 +0300, Mikhail Kurinnoi wrote:
>> For now, we don't have ready for upstream "immutable" EVM signature
>> format support patch. Both, Dmitry's and my, patches need more work
>> in order to prevent file's data changes (in case of IMA hash) and
>> metadata changes for files signed by "immutable" EVM xattr (same idea
>> as we already have for IMA digsig, that prevent file's data change).
>
> After looking at your patches again, I think we should combine the
> "immutable" and "portable" concepts so that the new "portable"
> signature type is written out and considered "immutable".
>
> Dmitry's patch does prevent the file from changing, but that code is
> in IMA, but should be in EVM.  I agree we can defer preventing the
> file from changing.
>
> Mimi
>



-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

Can you please point me to the patchset email?

Thanks,
Dmitry


On Fri, Oct 13, 2017 at 2:09 AM, Dmitry Kasatkin
<dmitry.kasatkin@gmail.com> wrote:
> Hi all,
>
> [switched to plain text]
>
> I will check Mikhail's patches.
> Give me a moment.
>
> Thanks,
> Dmitry
>
>
> On Tue, Oct 10, 2017 at 10:07 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> On Tue, 2017-10-10 at 02:10 +0300, Mikhail Kurinnoi wrote:
>>> For now, we don't have ready for upstream "immutable" EVM signature
>>> format support patch. Both, Dmitry's and my, patches need more work
>>> in order to prevent file's data changes (in case of IMA hash) and
>>> metadata changes for files signed by "immutable" EVM xattr (same idea
>>> as we already have for IMA digsig, that prevent file's data change).
>>
>> After looking at your patches again, I think we should combine the
>> "immutable" and "portable" concepts so that the new "portable"
>> signature type is written out and considered "immutable".
>>
>> Dmitry's patch does prevent the file from changing, but that code is
>> in IMA, but should be in EVM.  I agree we can defer preventing the
>> file from changing.
>>
>> Mimi
>>
>
>
>
> --
> Thanks,
> Dmitry



-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Wed, 2017-10-18 at 22:48 +0300, Dmitry Kasatkin wrote:
> Can you please point me to the patchset email?

This was the start of the lengthy discussion -
https://www.spinics.net/lists/linux-integrity/msg00035.html


> 
> On Fri, Oct 13, 2017 at 2:09 AM, Dmitry Kasatkin
> <dmitry.kasatkin@gmail.com> wrote:
> > Hi all,
> >
> > [switched to plain text]
> >
> > I will check Mikhail's patches.
> > Give me a moment.
> >
> > Thanks,
> > Dmitry
> >
> >
> > On Tue, Oct 10, 2017 at 10:07 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> On Tue, 2017-10-10 at 02:10 +0300, Mikhail Kurinnoi wrote:
> >>> For now, we don't have ready for upstream "immutable" EVM signature
> >>> format support patch. Both, Dmitry's and my, patches need more work
> >>> in order to prevent file's data changes (in case of IMA hash) and
> >>> metadata changes for files signed by "immutable" EVM xattr (same idea
> >>> as we already have for IMA digsig, that prevent file's data change).
> >>
> >> After looking at your patches again, I think we should combine the
> >> "immutable" and "portable" concepts so that the new "portable"
> >> signature type is written out and considered "immutable".
> >>
> >> Dmitry's patch does prevent the file from changing, but that code is
> >> in IMA, but should be in EVM.  I agree we can defer preventing the
> >> file from changing.
> >>
> >> Mimi
> >>
> >
> >
> >
> > --
> > Thanks,
> > Dmitry
> 
> 
> 

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

May be Mikhail could share GIT url to look somewhere.
To see latest bits.

Dmitry

On Wed, Oct 18, 2017 at 11:30 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> On Wed, 2017-10-18 at 22:48 +0300, Dmitry Kasatkin wrote:
>> Can you please point me to the patchset email?
>
> This was the start of the lengthy discussion -
> https://www.spinics.net/lists/linux-integrity/msg00035.html
>
>
>>
>> On Fri, Oct 13, 2017 at 2:09 AM, Dmitry Kasatkin
>> <dmitry.kasatkin@gmail.com> wrote:
>> > Hi all,
>> >
>> > [switched to plain text]
>> >
>> > I will check Mikhail's patches.
>> > Give me a moment.
>> >
>> > Thanks,
>> > Dmitry
>> >
>> >
>> > On Tue, Oct 10, 2017 at 10:07 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> >> On Tue, 2017-10-10 at 02:10 +0300, Mikhail Kurinnoi wrote:
>> >>> For now, we don't have ready for upstream "immutable" EVM signature
>> >>> format support patch. Both, Dmitry's and my, patches need more work
>> >>> in order to prevent file's data changes (in case of IMA hash) and
>> >>> metadata changes for files signed by "immutable" EVM xattr (same idea
>> >>> as we already have for IMA digsig, that prevent file's data change).
>> >>
>> >> After looking at your patches again, I think we should combine the
>> >> "immutable" and "portable" concepts so that the new "portable"
>> >> signature type is written out and considered "immutable".
>> >>
>> >> Dmitry's patch does prevent the file from changing, but that code is
>> >> in IMA, but should be in EVM.  I agree we can defer preventing the
>> >> file from changing.
>> >>
>> >> Mimi
>> >>
>> >
>> >
>> >
>> > --
>> > Thanks,
>> > Dmitry
>>
>>
>>
>



-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Mikhail Kurinnoi]

? Wed, 18 Oct 2017 23:37:26 +0300
Dmitry Kasatkin <dmitry.kasatkin@gmail.com> ?????:

> May be Mikhail could share GIT url to look somewhere.
> To see latest bits.
> 
> Dmitry
>

I freezed the "portable" patchset work on 2017-01-13  
https://sourceforge.net/p/linux-ima/mailman/message/35601149/
As I mentioned, "immutable" patch was not ready for upstream (and I
had no idea what should I do with it, since I don't use it by myself). I
did "portable" part in condition I was need for my use case, so, I sent
patchset and switched to another work.



-- 
Best regards,
Mikhail Kurinnoi

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

Hi Dmitry,

On Wed, 2017-10-18 at 23:37 +0300, Dmitry Kasatkin wrote:
> May be Mikhail could share GIT url to look somewhere.
> To see latest bits.

Please bottom post in the future.

Summary:
Mikhail's patches were posted earlier this year.  His patches defined
a portable EVM signature, which was never written out to disk, but
after being verified, was written out as an HMAC.  This was based on
my understanding that the i_ino/uuid is required to prevent a cut &
paste attack.

In the recent discussions, Matthew wanted to know why the i_ino/uuid
is required.  After going around and around discussing it, it turns
out including security.ima is equivalent to including the i_ino/uuid.
 The i_ino/uuid is only necessary to prevent a cut and paste attack,
when security.ima is not included in the security.evm hmac/signature.

We're at the point of making the portable EVM signature immutable. By
immutable, we mean that it isn't re-written as an HMAC.  It is based
on your ima-evm-utils support.

Mikhail, Matthew, did I leave anything out?

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

On Thu, Oct 19, 2017 at 12:07 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Hi Dmitry,
>
> On Wed, 2017-10-18 at 23:37 +0300, Dmitry Kasatkin wrote:
>> May be Mikhail could share GIT url to look somewhere.
>> To see latest bits.
>
> Please bottom post in the future.
>
> Summary:
> Mikhail's patches were posted earlier this year.  His patches defined
> a portable EVM signature, which was never written out to disk, but
> after being verified, was written out as an HMAC.  This was based on
> my understanding that the i_ino/uuid is required to prevent a cut &
> paste attack.

I checked Mikhail patches. In his patches, immutable is normal evm
signature but not replaceable with hmac.

2) portable EVM digsig version, aimed to protect archived file's meta
data from manipulations.

What is the case of manipulation? hmac protects that..

>
> In the recent discussions, Matthew wanted to know why the i_ino/uuid
> is required.  After going around and around discussing it, it turns
> out including security.ima is equivalent to including the i_ino/uuid.
>  The i_ino/uuid is only necessary to prevent a cut and paste attack,
> when security.ima is not included in the security.evm hmac/signature.
>

If I recall, we had such discussion in the chat about i_no/uuid.

if I recall right, not including them was a compromise for "portability"?
Archive could be unpacked with xattrs and signatures are still valid.
tar --xattrs
cp --preserve=xattr

But how security.ima will protect against cut and paste attack?
Attacker can take any other file together with metadata and it will be
valid one.

> We're at the point of making the portable EVM signature immutable. By
> immutable, we mean that it isn't re-written as an HMAC.  It is based
> on your ima-evm-utils support.
>
> Mikhail, Matthew, did I leave anything out?
>
> Mimi
>



-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Thu, 2017-10-19 at 13:14 +0300, Dmitry Kasatkin wrote:
> On Thu, Oct 19, 2017 at 12:07 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> > Hi Dmitry,
> >
> > On Wed, 2017-10-18 at 23:37 +0300, Dmitry Kasatkin wrote:
> >> May be Mikhail could share GIT url to look somewhere.
> >> To see latest bits.
> >
> > Please bottom post in the future.
> >
> > Summary:
> > Mikhail's patches were posted earlier this year.  His patches defined
> > a portable EVM signature, which was never written out to disk, but
> > after being verified, was written out as an HMAC.  This was based on
> > my understanding that the i_ino/uuid is required to prevent a cut &
> > paste attack.
> 
> I checked Mikhail patches. In his patches, immutable is normal evm
> signature but not replaceable with hmac.

Mikhail's version the EVM signature does not contain the i_ino/uuid
and is never written to disk.  On installation, an HMAC is written
out.

> 2) portable EVM digsig version, aimed to protect archived file's meta
> data from manipulations.

Right 
 
> What is the case of manipulation? hmac protects that..

Better would be to write out the portable signature on disk, assuming
it is safe to do so, and not replace it with an HMAC.

> > In the recent discussions, Matthew wanted to know why the i_ino/uuid
> > is required.  After going around and around discussing it, it turns
> > out including security.ima is equivalent to including the i_ino/uuid.
> >  The i_ino/uuid is only necessary to prevent a cut and paste attack,
> > when security.ima is not included in the security.evm hmac/signature.
> >
> 
> If I recall, we had such discussion in the chat about i_no/uuid.
> 
> if I recall right, not including them was a compromise for "portability"?
> Archive could be unpacked with xattrs and signatures are still valid.
> tar --xattrs
> cp --preserve=xattr
> 
> But how security.ima will protect against cut and paste attack?
> Attacker can take any other file together with metadata and it will be
> valid one.

Only if the file hash included in the EVM signature matches, right?

Mimi

> > We're at the point of making the portable EVM signature immutable. By
> > immutable, we mean that it isn't re-written as an HMAC.  It is based
> > on your ima-evm-utils support.
> >
> > Mikhail, Matthew, did I leave anything out?
> >
> > Mimi
> >
> 
> 
> 

Re: RFC: Make it practical to ship EVM signatures

[Matthew Garrett]

On Thu, Oct 19, 2017 at 3:14 AM, Dmitry Kasatkin
<dmitry.kasatkin@gmail.com> wrote:
> But how security.ima will protect against cut and paste attack?
> Attacker can take any other file together with metadata and it will be
> valid one.

Unless the hashing algorithm is broken, the two files will need to be
identical in order for security.ima to match. And if the two files are
identical then an attacker can simply delete one and create a hardlink
to the other, which will have the same inode.

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

On Thu, Oct 19, 2017 at 8:08 PM, Matthew Garrett <mjg59@google.com> wrote:
> On Thu, Oct 19, 2017 at 3:14 AM, Dmitry Kasatkin
> <dmitry.kasatkin@gmail.com> wrote:
>> But how security.ima will protect against cut and paste attack?
>> Attacker can take any other file together with metadata and it will be
>> valid one.
>
> Unless the hashing algorithm is broken, the two files will need to be
> identical in order for security.ima to match. And if the two files are
> identical then an attacker can simply delete one and create a hardlink
> to the other, which will have the same inode.

I actually meant a different thing.
For a moment I thought about placing a file from another location.
But that is directory protection issue.

While working on patches I was disputing with Mimi about usefulness of
ino in there.

-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Dmitry Kasatkin]

On Thu, Oct 19, 2017 at 12:07 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> Hi Dmitry,
>
> On Wed, 2017-10-18 at 23:37 +0300, Dmitry Kasatkin wrote:
>> May be Mikhail could share GIT url to look somewhere.
>> To see latest bits.
>
> Please bottom post in the future.
>
> Summary:
> Mikhail's patches were posted earlier this year.  His patches defined
> a portable EVM signature, which was never written out to disk, but
> after being verified, was written out as an HMAC.  This was based on
> my understanding that the i_ino/uuid is required to prevent a cut &
> paste attack.
>
> In the recent discussions, Matthew wanted to know why the i_ino/uuid
> is required.  After going around and around discussing it, it turns
> out including security.ima is equivalent to including the i_ino/uuid.
>  The i_ino/uuid is only necessary to prevent a cut and paste attack,
> when security.ima is not included in the security.evm hmac/signature.
>
> We're at the point of making the portable EVM signature immutable. By
> immutable, we mean that it isn't re-written as an HMAC.  It is based
> on your ima-evm-utils support.

Do you mean to have unconditionally immutable?

>
> Mikhail, Matthew, did I leave anything out?
>
> Mimi
>



-- 
Thanks,
Dmitry

Re: RFC: Make it practical to ship EVM signatures

[Mimi Zohar]

On Thu, 2017-10-19 at 13:36 +0300, Dmitry Kasatkin wrote:
> On Thu, Oct 19, 2017 at 12:07 AM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:

> >
> > We're at the point of making the portable EVM signature immutable. By
> > immutable, we mean that it isn't re-written as an HMAC.  It is based
> > on your ima-evm-utils support.
> 
> Do you mean to have unconditionally immutable?

The portable signature is then also immutable.

Mimi

Re: RFC: Make it practical to ship EVM signatures

[Roberto Sassu]

On 9/29/2017 9:02 PM, Mimi Zohar wrote:
> On Fri, 2017-09-29 at 11:09 -0700, Matthew Garrett wrote:
>> On Thu, Sep 28, 2017 at 5:53 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> Without the inode included in the HMAC calculation, the same file
>>> could exist as a different file name on the same file system.  No need
>>> for the two files to be on different file systems.
>>
>> But if I create a hardlink to an existing file then I get the same
>> file with the same inode number and two different names on the same
>> filesystem, and so security.evm would still match?
> 
> True, with a hard link that would be the case, but by the same file, I
> meant a copy of the original file, not a hard link to the file.
> 
>>>> One of the reasons we're interested in allowing the use of signatures
>>>> rather than HMACs is to avoid the case where a machine being
>>>> compromised would allow an attacker to obtain the symmetric key and
>>>> drop new appropriately HMACed binaries on the system that would
>>>> persist even if the kernel was updated to fix the vulnerability.
>>>
>>> Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
>>> the EVM key (trusted key), then such an attack would require root
>>> privileges with the ability to read kernel memory.  The EVM key is
>>> never exposed to userspace in the clear.
>>
>> That's a case that we need to be worried about. Trusted boot means we
>> can ensure that a system boots an updated kernel, but if the attacker
>> has been able to drop a modified sshd with a valid hmac onto the
>> system then we have fewer guarantees about the integrity. We could
>> continue using signatures for security.ima to avoid that, but then we
>> lose the performance benefits of the hmac and also don't have the same
>> level of guarantees around the other security metadata.
> 
> I think you mean "secure boot", not "trusted boot", in this case.
> 
> The original understanding was that IMA/EVM would enforce integrity
> and not enforce mandatory access control (MAC).  The LSMs (eg.
> SELinux) would be responsible for MAC.  Separation of duties.

To enforce integrity at runtime, IMA must be able to capture all
integrity relevant operations and allow the TCB to read only high
integrity files (Biba model). But, if it is left to LSMs to decide
if a file can be written or not, then IMA alone cannot determine
which files should be appraised or not. It could happen that IMA does
not appraise files which have an impact on the integrity of the TCB.

For example, with the current appraisal policy, IMA appraises files
owned by root but does not consider permissions assigned to others.
What it could happen is that, if a file owned by root is writable
by others (which is possible with DAC), a process outside the TCB
reads a file not owned by root (not appraised), becomes compromised,
and writes to a file owned by root (appraised), which is read by
a process in the TCB. The integrity of the TCB becomes low.

The problem is that, with an IMA policy based on LSM labels or DAC
permissions applied to files, IMA relies on the fact that LSMs
or DAC would preserve the integrity of the files belonging to
the TCB, while the policy might not be designed to meet this goal.

To avoid this issue, IMA could instead enforce integrity depending
on process credentials (similarly to measurement). An HMAC is added
to a file, or updated, only if the initial state is known (the file
is signed, or it belongs to a digest list), and if it is written
only by processes in the TCB. Writes by processes outside the TCB
are denied.

Files with valid HMACs could be excluded from measurement.
If enforcement is not enabled, IMA should not update the HMAC if
a file is written by a process outside the TCB, and should create
a new measurement entry if a process in the TCB accesses it.

Roberto


> With that understanding, if the LSM allows a file to be "dropped" onto
>   the file system of a running system, IMA/EVM will hash and hmac the
> permitted file.
> 
> I don't understand where you're going with this train of thought.  If
> you're trying to make a case for EVM to run with only security.evm
> signatures, then you wouldn't refer to the HMAC benefits.  If you're
> trying to make a case for EVM signatures, with the inode they're not
> portable, without the inode, they are susceptible to a cut and paste
> attack.
> 
> Mickhail's proposed patches resolves this by having a portable EVM
> signature that is never written to disk, but converted to an HMAC.
> 
> Mimi
> 

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG

Re: RFC: Make it practical to ship EVM signatures

[Roberto Sassu]

On 9/29/2017 2:53 AM, Mimi Zohar wrote:
> On Thu, 2017-09-28 at 14:13 -0700, Matthew Garrett wrote:
>> On Thu, Sep 28, 2017 at 1:12 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> Earlier this year there were discussions on defining a portable EVM
>>> signature, that could be included in software packages.
>>>
>>> The reason for including as much metadata as possible in the HMAC is
>>> to limit cut & paste attacks.  For this reason, the portable data is
>>> only used in transmission, not on disk.
>>
>> The concern is that two identical copies of a file may exist, but with
>> different security contexts, and that not protecting the inode would
>> allow an attacker to copy the security metadata from one file onto the
>> other? This presumably only has meaning if they're on separate
>> filesystems, since otherwise the attacker could just delete one file
>> and hardlink the other to the former's location? I think that for our
>> purposes this isn't a big deal.
> 
> Without the inode included in the HMAC calculation, the same file
> could exist as a different file name on the same file system.  No need
> for the two files to be on different file systems.
> 
>>> A new EVM type is defined that does not convert the EVM signature to
>>> an HMAC.
>>>
>>> Mikhail's patches:
>>> https://sourceforge.net/p/linux-ima/mailman/linux-ima-user/thread/2017
>>> 0113072602.4ffaa30a@totoro/
>>
>> That looks broadly like what we want, but I think there's some
>> advantage in maintaining the flexibility of choosing which information
>> is embedded. One additional option would be to allow userland to place
>> a restriction on which options *must* be present, ie local policy
>> could refuse to allow any signatures that didn't include a specific
>> set of metadata.
> 
> This introduces a new level of complexity, which I'm not sure is warranted.
> 
>> One of the reasons we're interested in allowing the use of signatures
>> rather than HMACs is to avoid the case where a machine being
>> compromised would allow an attacker to obtain the symmetric key and
>> drop new appropriately HMACed binaries on the system that would
>> persist even if the kernel was updated to fix the vulnerability.

With a predictable system state, you would be able to prevent misuse
of the symmetric key. For example, suppose that the state is about
to change (with a digest list, IMA extends PCR 10 with the digest
of unknown files). Then, countermeasures can be implemented to avoid
using the symmetric key before the system is compromised.

The remaining issue is when vulnerabilities are discovered in known
software. In this case, the symmetric key should be revoked and
replaced with a new one. Files protected with the old key must
be checked and protected with the new key.

I agree on the fact that HMACs can be avoided for immutable files
but, for mutable files, HMACs could be very useful. Suppose that
a valid HMAC can be applied to a file only when the symmetric key
is available (system is in the expected state) and when the file
is written only by the TCB. Then, you can exclude those files from
measurement (unless the HMAC is invalid) and avoid unknown digests
(the system state remains predictable).


> Assuming you're using a trusted key (TPM based key) to encrypt/decrypt
> the EVM key (trusted key), then such an attack would require root
> privileges with the ability to read kernel memory.  The EVM key is
> never exposed to userspace in the clear.

A malicious admin can launch a new kernel with kexec and dump the key.

The symmetric key can be protected against malicious administrators
if it is generated by the TPM (sensitiveDataOrigin bit must be set)
and is released only after verifying the sealing policy (kexec is
disabled, only good software can be executed, ...).

Roberto

-- 
HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Bo PENG, Qiuen PENG, Shengli WANG