EVM: Allow runtime modification of the set of verified xattrs

Sites may wish to provide additional metadata alongside files in order to make more fine-grained security decisions[1]. The security of this is enhanced if this metadata is protected, something that EVM makes possible. However, the kernel cannot know about the set of extended attributes that local admins may wish to protect, and hardcoding this policy in the kernel makes it difficult to change over time and less convenient for distributions to enable.

This patch adds a new /sys/kernel/security/integrity/evm/evm_xattrs node, which can be read to obtain the current set of EVM-protected extended attributes or written to in order to add new entries. Extending this list will not change the validity of any existing signatures provided that the file in question does not have any of the additional extended attributes - missing xattrs are skipped when calculating the EVM hash.

[1] For instance, a package manager could install information about the package uploader in an additional extended attribute. Local LSM policy could then be associated with that extended attribute in order to restrict the privileges available to packages from less trusted uploaders.

Signed-off-by Matthew Garrett <[email protected]>

---

Calling simple_setattr() directly here - this should be safe, since there'd only be a different setattr() implementation if we'd passed one in in the first place.

security/integrity/evm/evm_secfs.c +94 -0

+/**
+ * evm_write_xattrs - write() for <securityfs>/evm_xattrs
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
+				size_t count, loff_t *ppos)
+{
+	int len, err;
+	struct xattr_list *xattr, *tmp;
+	struct audit_buffer *ab;
+	struct iattr newattrs;
+	struct inode *inode;
+
+	if (!capable(CAP_SYS_ADMIN) || evm_xattrs_locked)
+		return -EPERM;
+
+	if (*ppos != 0)
+		return -EINVAL;
+
+	if (count > XATTR_NAME_MAX)
+		return -E2BIG;
+
+	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_EVM_XATTR);
+	if (IS_ERR(ab))
+		return PTR_ERR(ab);
+
+	xattr = kmalloc(sizeof(struct xattr_list), GFP_KERNEL);
+	if (!xattr) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	xattr->name = memdup_user_nul(buf, count);
+	if (IS_ERR(xattr->name)) {
+		err = PTR_ERR(xattr->name);
+		xattr->name = NULL;
+		goto out;
+	}
+
+	/* Remove any trailing newline */
+	len = strlen(xattr->name);
+	if (xattr->name[len-1] == '\n')
+		xattr->name[len-1] = '\0';
+
+	if (strcmp(xattr->name, ".") == 0) {
+		evm_xattrs_locked = 1;
+		newattrs.ia_mode = S_IFREG | 0440;
+		newattrs.ia_valid = ATTR_MODE;
+		inode = evm_xattrs->d_inode;
+		inode_lock(inode);
+		err = simple_setattr(evm_xattrs, &newattrs);
+		inode_unlock(inode);
+		audit_log_format(ab, "locked");
+		if (!err)
+			err = count;
+		goto out;
+	}
+
+	audit_log_format(ab, "xattr=");
+	audit_log_untrustedstring(ab, xattr->name);
+
+	if (strncmp(xattr->name, XATTR_SECURITY_PREFIX,
+		    XATTR_SECURITY_PREFIX_LEN) != 0) {
+		err = -EINVAL;
+		goto out;
+	}
+
+	/* Guard against races in evm_read_xattrs */
+	mutex_lock(&xattr_list_mutex);
+	list_for_each_entry(tmp, &evm_config_xattrnames, list) {
+		if (strcmp(xattr->name, tmp->name) == 0) {
+			err = -EEXIST;
+			mutex_unlock(&xattr_list_mutex);
+			goto out;
+		}
+	}
+	list_add_tail_rcu(&xattr->list, &evm_config_xattrnames);
+	mutex_unlock(&xattr_list_mutex);
+
+	audit_log_format(ab, " res=0");
+	audit_log_end(ab);
+	return count;
+out:
+	audit_log_format(ab, " res=%d", err);
+	audit_log_end(ab);
+	kfree(xattr->name);
+	kfree(xattr);
+	return err;
+}

Hi Matthew,

On Tue, 2018-05-15 at 10:38 -0700, Matthew Garrett wrote:

Based on the "IMA: work on audit records produced by IMA" discussion, should this new audit record be adding the task info by calling "audit_log_task_info(ab, current);" too?

Could you provide Steve with an example of the audit rule?

thanks!

Mimi

[...]

Quoted 94 lines

security/integrity/evm/evm_secfs.c +94 -0

+/**
+ * evm_write_xattrs - write() for <securityfs>/evm_xattrs
+ * @file: file pointer, not actually used
+ * @buf: where to get the data from
+ * @count: bytes sent
+ * @ppos: where to start
+ *
+ * Returns number of bytes written or error code, as appropriate
+ */
+static ssize_t evm_write_xattrs(struct file *file, const char __user *buf,
+                             size_t count, loff_t *ppos)
+{
+     int len, err;
+     struct xattr_list *xattr, *tmp;
+     struct audit_buffer *ab;
+     struct iattr newattrs;
+     struct inode *inode;
+
+     if (!capable(CAP_SYS_ADMIN) || evm_xattrs_locked)
+             return -EPERM;
+
+     if (*ppos != 0)
+             return -EINVAL;
+
+     if (count > XATTR_NAME_MAX)
+             return -E2BIG;
+
+     ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_INTEGRITY_EVM_XATTR);
+     if (IS_ERR(ab))
+             return PTR_ERR(ab);
+
+     xattr = kmalloc(sizeof(struct xattr_list), GFP_KERNEL);
+     if (!xattr) {
+             err = -ENOMEM;
+             goto out;
+     }
+
+     xattr->name = memdup_user_nul(buf, count);
+     if (IS_ERR(xattr->name)) {
+             err = PTR_ERR(xattr->name);
+             xattr->name = NULL;
+             goto out;
+     }
+
+     /* Remove any trailing newline */
+     len = strlen(xattr->name);
+     if (xattr->name[len-1] == '\n')
+             xattr->name[len-1] = '\0';
+
+     if (strcmp(xattr->name, ".") == 0) {
+             evm_xattrs_locked = 1;
+             newattrs.ia_mode = S_IFREG | 0440;
+             newattrs.ia_valid = ATTR_MODE;
+             inode = evm_xattrs->d_inode;
+             inode_lock(inode);
+             err = simple_setattr(evm_xattrs, &newattrs);
+             inode_unlock(inode);
+             audit_log_format(ab, "locked");
+             if (!err)
+                     err = count;
+             goto out;
+     }
+
+     audit_log_format(ab, "xattr=");
+     audit_log_untrustedstring(ab, xattr->name);
+
+     if (strncmp(xattr->name, XATTR_SECURITY_PREFIX,
+                 XATTR_SECURITY_PREFIX_LEN) != 0) {
+             err = -EINVAL;
+             goto out;
+     }
+
+     /* Guard against races in evm_read_xattrs */
+     mutex_lock(&xattr_list_mutex);
+     list_for_each_entry(tmp, &evm_config_xattrnames, list) {
+             if (strcmp(xattr->name, tmp->name) == 0) {
+                     err = -EEXIST;
+                     mutex_unlock(&xattr_list_mutex);
+                     goto out;
+             }
+     }
+     list_add_tail_rcu(&xattr->list, &evm_config_xattrnames);
+     mutex_unlock(&xattr_list_mutex);
+
+     audit_log_format(ab, " res=0");
+     audit_log_end(ab);
+     return count;
+out:
+     audit_log_format(ab, " res=%d", err);
+     audit_log_end(ab);
+     kfree(xattr->name);
+     kfree(xattr);
+     return err;
+}