[Linux-ima-user] [PATCH v3] evm: provide portable EVM signature version.

[Mikhail K. <vie...@vi...>]

version 3 changes:
1) Prevent ima_fix_xattr() use __vfs_setxattr_noperm() if EVM signature verification failed.
Since, __vfs_setxattr_noperm() don't count on evm_inode_setxattr(), during file unpacking from atchive EVM zero-filled sign could be updated by IMA without EVM signature status checking. So, even if you have EVM portable signature signed by another key, IMA will update EVM signature to HMAC any way, no matter what you have in security.evm xattr or you don't have it at all.
2) Added integrity_audit_msg() in order to get some error in audit syslog on EVM portable signature verification fail. 

version 2 changes:
1) evm_verify_hmac() prevent work with EVM portable signature. If file signed by EVM portable signature, evm_verify_hmac() return INTEGRITY_FAIL;
2) evm_inode_post_setxattr() now count on EVM portable signature. 


~~~~~~~~~~~~~~~~~~
patch for tar v1.29 provide EVM portable signature support for testing.
~~~~~~~~~~~~~~~~~~
--- a/src/extract.c
+++ b/src/extract.c
@@ -391,6 +391,10 @@ set_stat (char const *file_name,
   xattrs_xattrs_set (st, file_name, typeflag, 1);
   xattrs_acls_set (st, file_name, typeflag);
   xattrs_selinux_set (st, file_name, typeflag);
+  /* IMA signature. */
+  xattrs_xattrs_set (st, file_name, typeflag, 2);
+  /* EVM portable signature should be the last one. */
+  xattrs_xattrs_set (st, file_name, typeflag, 3);
 }
 
 /* For each entry H in the leading prefix of entries in HEAD that do

--- a/src/xattrs.c
+++ b/src/xattrs.c
@@ -668,9 +668,22 @@ xattrs_xattrs_set (struct tar_stat_info const *st,
              For a regular files: all extended attributes are restored during
              the first run except 'security.capability' which is restored in
              'later_run == 1'.  */
-          if (typeflag == REGTYPE
-              && later_run == !!strcmp (keyword, "security.capability"))
-            continue;
+          if (typeflag == REGTYPE) {
+              if (later_run != 1 && !strcmp (keyword, "security.capability"))
+                continue;
+              if (later_run == 1 && strcmp (keyword, "security.capability"))
+                continue;
+          /* IMA signature. */
+              if (later_run != 2 && !strcmp (keyword, "security.ima"))
+                continue;
+              if (later_run == 2 && strcmp (keyword, "security.ima"))
+                continue;
+          /* EVM portable signature should be the last one. */
+              if (later_run != 3 && !strcmp (keyword, "security.evm"))
+                continue;
+              if (later_run == 3 && strcmp (keyword, "security.evm"))
+                continue;
+          }
 
           if (xattrs_masked_out (keyword, false /* extracting */ ))
             /* we don't want to restore this keyword */

~~~~~~~~~~~~~~~~~~




Signed-off-by: Dmitry Kasatkin <d.k...@sa...>
Signed-off-by: Mikhail Kurinnoi <vie...@vi...>

--- a/security/integrity/digsig.c	2016-12-31 13:06:26.331828091 +0300
+++ b/security/integrity/digsig.c	2017-01-05 23:33:23.614541837 +0300
@@ -71,6 +71,7 @@ int integrity_digsig_verify(const unsign
 		return digsig_verify(keyring[id], sig + 1, siglen - 1,
 				     digest, digestlen);
 	case 2:
+	case 3:
 		return asymmetric_verify(keyring[id], sig, siglen,
 					 digest, digestlen);
 	}
--- a/security/integrity/evm/evm_crypto.c	2016-12-31 13:06:26.331828091 +0300
+++ b/security/integrity/evm/evm_crypto.c	2017-01-05 23:35:48.577389813 +0300
@@ -169,6 +169,23 @@ static void hmac_add_misc(struct shash_d
 	crypto_shash_final(desc, digest);
 }
 
+static void hmac_add_misc_portable_digsig(struct shash_desc *desc, struct inode *inode,
+				 char *digest)
+{
+	struct h_misc {
+		uid_t uid;
+		gid_t gid;
+		umode_t mode;
+	} hmac_misc;
+
+	memset(&hmac_misc, 0, sizeof(hmac_misc));
+	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;
+	crypto_shash_update(desc, (const u8 *)&hmac_misc, sizeof(hmac_misc));
+	crypto_shash_final(desc, digest);
+}
+
 /*
  * Calculate the HMAC value across the set of protected security xattrs.
  *
@@ -219,7 +236,10 @@ static int evm_calc_hmac_or_hash(struct
 		xattr_size = size;
 		crypto_shash_update(desc, (const u8 *)xattr_value, xattr_size);
 	}
-	hmac_add_misc(desc, inode, digest);
+	if (type == EVM_IMA_XATTR_DIGSIG)
+		hmac_add_misc_portable_digsig(desc, inode, digest);
+	else
+		hmac_add_misc(desc, inode, digest);
 
 out:
 	kfree(xattr_value);
@@ -237,10 +257,12 @@ int evm_calc_hmac(struct dentry *dentry,
 
 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)
+		  int version, 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,
+				version == 3 ? EVM_IMA_XATTR_DIGSIG : IMA_XATTR_DIGEST,
+				digest);
 }
 
 /*
--- a/security/integrity/evm/evm.h	2016-11-29 01:32:07.248219481 +0300
+++ b/security/integrity/evm/evm.h	2017-01-05 23:34:06.551383033 +0300
@@ -48,7 +48,7 @@ int evm_calc_hmac(struct dentry *dentry,
 		  size_t req_xattr_value_len, 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, int version, char *digest);
 int evm_init_hmac(struct inode *inode, const struct xattr *xattr,
 		  char *hmac_val);
 int evm_init_secfs(void);
--- a/security/integrity/evm/evm_main.c	2016-12-31 13:06:26.331828091 +0300
+++ b/security/integrity/evm/evm_main.c	2017-01-07 00:39:24.414082237 +0300
@@ -116,7 +116,7 @@ static enum integrity_status evm_verify_
 	struct evm_ima_xattr_data *xattr_data = NULL;
 	struct evm_ima_xattr_data calc;
 	enum integrity_status evm_status = INTEGRITY_PASS;
-	int rc, xattr_len;
+	int rc, xattr_len, version;
 
 	if (iint && iint->evm_status == INTEGRITY_PASS)
 		return iint->evm_status;
@@ -159,8 +159,11 @@ static enum integrity_status evm_verify_
 			rc = -EINVAL;
 		break;
 	case EVM_IMA_XATTR_DIGSIG:
+		version = ((const char *)xattr_data)[1];
+		if (version == 3)
+			break;
 		rc = evm_calc_hash(dentry, xattr_name, xattr_value,
-				xattr_value_len, calc.digest);
+				xattr_value_len, version, calc.digest);
 		if (rc)
 			break;
 		rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
@@ -236,7 +239,7 @@ enum integrity_status evm_verifyxattr(st
 				      void *xattr_value, size_t xattr_value_len,
 				      struct integrity_iint_cache *iint)
 {
-	if (!evm_initialized || !evm_protected_xattr(xattr_name))
+	if (!evm_initialized || (xattr_name && !evm_protected_xattr(xattr_name)))
 		return INTEGRITY_UNKNOWN;
 
 	if (!iint) {
@@ -372,6 +375,47 @@ static void evm_reset_status(struct inod
 		iint->evm_status = INTEGRITY_UNKNOWN;
 }
 
+void evm_update_portable_digsig(struct dentry *dentry, const char *xattr_name,
+			 const void *xattr_value, size_t xattr_value_len)
+{
+	const struct evm_ima_xattr_data *xattr_data = xattr_value;
+	struct evm_ima_xattr_data calc;
+	struct evm_ima_xattr_data xattr_data_fail;
+	int version, rc;
+
+	if ((xattr_data->type != EVM_IMA_XATTR_DIGSIG) || \
+	    (strcmp(xattr_name, XATTR_NAME_EVM) != 0))
+		return;
+
+	version = ((const char *)xattr_data)[1];
+	if (version != 3)
+		return;
+
+	evm_reset_status(dentry->d_inode);
+
+	rc = evm_calc_hash(dentry, xattr_name, xattr_value,
+			   xattr_value_len, version, calc.digest);
+	if (rc)
+		goto digsig_fail;
+	rc = integrity_digsig_verify(INTEGRITY_KEYRING_EVM,
+				     (const char *)xattr_data, xattr_value_len,
+				     calc.digest, sizeof(calc.digest));
+	if (rc)
+		goto digsig_fail;
+
+	evm_update_evmxattr(dentry, xattr_name, xattr_value, xattr_value_len);
+	return;
+
+digsig_fail:
+	integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
+			    dentry->d_name.name, "update_metadata",
+			    integrity_status_msg[INTEGRITY_FAIL], -EPERM, 0);
+	memset(&xattr_data_fail, 0, sizeof(xattr_data_fail));
+	__vfs_setxattr_noperm(dentry, XATTR_NAME_EVM,
+					   &xattr_data_fail,
+					   sizeof(xattr_data_fail), 0);
+}
+
 /**
  * evm_inode_post_setxattr - update 'security.evm' to reflect the changes
  * @dentry: pointer to the affected dentry
@@ -388,8 +432,12 @@ static void evm_reset_status(struct inod
 void evm_inode_post_setxattr(struct dentry *dentry, const char *xattr_name,
 			     const void *xattr_value, size_t xattr_value_len)
 {
-	if (!evm_initialized || (!evm_protected_xattr(xattr_name)
-				 && !posix_xattr_acl(xattr_name)))
+	if (!evm_initialized)
+		return;
+
+	evm_update_portable_digsig(dentry, xattr_name, xattr_value, xattr_value_len);
+
+	if (!evm_protected_xattr(xattr_name) && !posix_xattr_acl(xattr_name))
 		return;
 
 	evm_reset_status(dentry->d_inode);
--- a/security/integrity/ima/ima_appraise.c	2017-01-03 19:30:16.340669333 +0300
+++ b/security/integrity/ima/ima_appraise.c	2017-01-07 00:39:03.525673274 +0300
@@ -48,8 +48,13 @@ static int ima_fix_xattr(struct dentry *
 			 struct integrity_iint_cache *iint)
 {
 	int rc, offset;
+	enum integrity_status status = INTEGRITY_UNKNOWN;
 	u8 algo = iint->ima_hash->algo;
 
+	status = evm_verifyxattr(dentry, NULL, NULL, 0, iint);
+	if (status == INTEGRITY_FAIL)
+		return -EINVAL;
+
 	if (algo <= HASH_ALGO_SHA1) {
 		offset = 1;
 		iint->ima_hash->xattr.sha1.type = IMA_XATTR_DIGEST;