Merge branch 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security

Pull integrity updates from James Morris:
 "This contains a mixture of bug fixes, code cleanup, and new
  functionality. Of note is the integrity cache locking fix, file change
  detection, and support for a new EVM portable and immutable signature
  type.

  The re-introduction of the integrity cache lock (iint) fixes the
  problem of attempting to take the i_rwsem shared a second time, when
  it was previously taken exclusively. Defining atomic flags resolves
  the original iint/i_rwsem circular locking - accessing the file data
  vs. modifying the file metadata. Although it fixes the O_DIRECT
  problem as well, a subsequent patch is needed to remove the explicit
  O_DIRECT prevention.

  For performance reasons, detecting when a file has changed and needs
  to be re-measured, re-appraised, and/or re-audited, was limited to
  after the last writer has closed, and only if the file data has
  changed. Detecting file change is based on i_version. For filesystems
  that do not support i_version, remote filesystems, or userspace
  filesystems, the file was measured, appraised and/or audited once and
  never re-evaluated. Now local filesystems, which do not support
  i_version or are not mounted with the i_version option, assume the
  file has changed and are required to re-evaluate the file. This change
  does not address detecting file change on remote or userspace
  filesystems.

  Unlike file data signatures, which can be included and distributed in
  software packages (eg. rpm, deb), the existing EVM signature, which
  protects the file metadata, could not be included in software
  packages, as it includes file system specific information (eg. i_ino,
  possibly the UUID). This pull request defines a new EVM portable and
  immutable file metadata signature format, which can be included in
  software packages"

* 'next-integrity' of git://git.kernel.org/pub/scm/linux/kernel/git/jmorris/linux-security:
  ima/policy: fix parsing of fsuuid
  ima: Use i_version only when filesystem supports it
  integrity: remove unneeded initializations in integrity_iint_cache entries
  ima: log message to module appraisal error
  ima: pass filename to ima_rdwr_violation_check()
  ima: Fix line continuation format
  ima: support new "hash" and "dont_hash" policy actions
  ima: re-introduce own integrity cache lock
  EVM: Add support for portable signature format
  EVM: Allow userland to permit modification of EVM-protected metadata
  ima: relax requiring a file signature for new files with zero length

1 files changed, 66 insertions, 26 deletions

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 06a70c5a2329..061425dd6400 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -85,10 +85,10 @@ static void ima_rdwr_violation_check(struct file *file,
 				     struct integrity_iint_cache *iint,
 				     int must_measure,
 				     char **pathbuf,
-				     const char **pathname)
+				     const char **pathname,
+				     char *filename)
 {
 	struct inode *inode = file_inode(file);
-	char filename[NAME_MAX];
 	fmode_t mode = file->f_mode;
 	bool send_tomtou = false, send_writers = false;
 
@@ -97,10 +97,13 @@ static void ima_rdwr_violation_check(struct file *file,
 			if (!iint)
 				iint = integrity_iint_find(inode);
 			/* IMA_MEASURE is set from reader side */
-			if (iint && (iint->flags & IMA_MEASURE))
+			if (iint && test_bit(IMA_MUST_MEASURE,
+						&iint->atomic_flags))
 				send_tomtou = true;
 		}
 	} else {
+		if (must_measure)
+			set_bit(IMA_MUST_MEASURE, &iint->atomic_flags);
 		if ((atomic_read(&inode->i_writecount) > 0) && must_measure)
 			send_writers = true;
 	}
@@ -122,22 +125,25 @@ static void ima_check_last_writer(struct integrity_iint_cache *iint,
 				  struct inode *inode, struct file *file)
 {
 	fmode_t mode = file->f_mode;
+	bool update;
 
 	if (!(mode & FMODE_WRITE))
 		return;
 
-	inode_lock(inode);
+	mutex_lock(&iint->mutex);
 	if (atomic_read(&inode->i_writecount) == 1) {
+		update = test_and_clear_bit(IMA_UPDATE_XATTR,
+					    &iint->atomic_flags);
 		if (!IS_I_VERSION(inode) ||
 		    inode_cmp_iversion(inode, iint->version) ||
 		    (iint->flags & IMA_NEW_FILE)) {
 			iint->flags &= ~(IMA_DONE_MASK | IMA_NEW_FILE);
 			iint->measured_pcrs = 0;
-			if (iint->flags & IMA_APPRAISE)
+			if (update)
 				ima_update_xattr(iint, file);
 		}
 	}
-	inode_unlock(inode);
+	mutex_unlock(&iint->mutex);
 }
 
 /**
@@ -170,7 +176,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	char *pathbuf = NULL;
 	char filename[NAME_MAX];
 	const char *pathname = NULL;
-	int rc = -ENOMEM, action, must_appraise;
+	int rc = 0, action, must_appraise = 0;
 	int pcr = CONFIG_IMA_MEASURE_PCR_IDX;
 	struct evm_ima_xattr_data *xattr_value = NULL;
 	int xattr_len = 0;
@@ -201,17 +207,31 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	if (action) {
 		iint = integrity_inode_get(inode);
 		if (!iint)
-			goto out;
+			rc = -ENOMEM;
 	}
 
-	if (violation_check) {
+	if (!rc && violation_check)
 		ima_rdwr_violation_check(file, iint, action & IMA_MEASURE,
-					 &pathbuf, &pathname);
-		if (!action) {
-			rc = 0;
-			goto out_free;
-		}
-	}
+					 &pathbuf, &pathname, filename);
+
+	inode_unlock(inode);
+
+	if (rc)
+		goto out;
+	if (!action)
+		goto out;
+
+	mutex_lock(&iint->mutex);
+
+	if (test_and_clear_bit(IMA_CHANGE_ATTR, &iint->atomic_flags))
+		/* reset appraisal flags if ima_inode_post_setattr was called */
+		iint->flags &= ~(IMA_APPRAISE | IMA_APPRAISED |
+				 IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK |
+				 IMA_ACTION_FLAGS);
+
+	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags))
+		/* reset all flags if ima_inode_setxattr was called */
+		iint->flags &= ~IMA_DONE_MASK;
 
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
@@ -225,11 +245,23 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	if ((action & IMA_MEASURE) && (iint->measured_pcrs & (0x1 << pcr)))
 		action ^= IMA_MEASURE;
 
+	/* HASH sets the digital signature and update flags, nothing else */
+	if ((action & IMA_HASH) &&
+	    !(test_bit(IMA_DIGSIG, &iint->atomic_flags))) {
+		xattr_len = ima_read_xattr(file_dentry(file), &xattr_value);
+		if ((xattr_value && xattr_len > 2) &&
+		    (xattr_value->type == EVM_IMA_XATTR_DIGSIG))
+			set_bit(IMA_DIGSIG, &iint->atomic_flags);
+		iint->flags |= IMA_HASHED;
+		action ^= IMA_HASH;
+		set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags);
+	}
+
 	/* Nothing to do, just return existing appraised status */
 	if (!action) {
 		if (must_appraise)
 			rc = ima_get_cache_status(iint, func);
-		goto out_digsig;
+		goto out_locked;
 	}
 
 	template_desc = ima_template_desc_current();
@@ -242,7 +274,7 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 
 	rc = ima_collect_measurement(iint, file, buf, size, hash_algo);
 	if (rc != 0 && rc != -EBADF && rc != -EINVAL)
-		goto out_digsig;
+		goto out_locked;
 
 	if (!pathbuf)	/* ima_rdwr_violation possibly pre-fetched */
 		pathname = ima_d_path(&file->f_path, &pathbuf, filename);
@@ -250,26 +282,32 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 	if (action & IMA_MEASURE)
 		ima_store_measurement(iint, file, pathname,
 				      xattr_value, xattr_len, pcr);
-	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK))
+	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
+		inode_lock(inode);
 		rc = ima_appraise_measurement(func, iint, file, pathname,
 					      xattr_value, xattr_len, opened);
+		inode_unlock(inode);
+	}
 	if (action & IMA_AUDIT)
 		ima_audit_measurement(iint, pathname);
 
 	if ((file->f_flags & O_DIRECT) && (iint->flags & IMA_PERMIT_DIRECTIO))
 		rc = 0;
-out_digsig:
-	if ((mask & MAY_WRITE) && (iint->flags & IMA_DIGSIG) &&
+out_locked:
+	if ((mask & MAY_WRITE) && test_bit(IMA_DIGSIG, &iint->atomic_flags) &&
 	     !(iint->flags & IMA_NEW_FILE))
 		rc = -EACCES;
+	mutex_unlock(&iint->mutex);
 	kfree(xattr_value);
-out_free:
+out:
 	if (pathbuf)
 		__putname(pathbuf);
-out:
-	inode_unlock(inode);
-	if ((rc && must_appraise) && (ima_appraise & IMA_APPRAISE_ENFORCE))
-		return -EACCES;
+	if (must_appraise) {
+		if (rc && (ima_appraise & IMA_APPRAISE_ENFORCE))
+			return -EACCES;
+		if (file->f_mode & FMODE_WRITE)
+			set_bit(IMA_UPDATE_XATTR, &iint->atomic_flags);
+	}
 	return 0;
 }
 
@@ -368,8 +406,10 @@ int ima_read_file(struct file *file, enum kernel_read_file_id read_id)
 
 	if (!file && read_id == READING_MODULE) {
 		if (!sig_enforce && (ima_appraise & IMA_APPRAISE_MODULES) &&
-		    (ima_appraise & IMA_APPRAISE_ENFORCE))
+		    (ima_appraise & IMA_APPRAISE_ENFORCE)) {
+			pr_err("impossible to appraise a module without a file descriptor. sig_enforce kernel parameter might help\n");
 			return -EACCES;	/* INTEGRITY_UNKNOWN */
+		}
 		return 0;	/* We rely on module signature checking */
 	}
 	return 0;