Linux-Security-Module Archive on lore.kernel.org
 help / color / Atom feed
From: Roberto Sassu <roberto.sassu@huawei.com>
To: <zohar@linux.ibm.com>, <mjg59@google.com>
Cc: <linux-integrity@vger.kernel.org>,
	<linux-security-module@vger.kernel.org>,
	<linux-fsdevel@vger.kernel.org>, <linux-kernel@vger.kernel.org>,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: [PATCH v5 07/12] evm: Allow xattr/attr operations for portable signatures
Date: Wed, 7 Apr 2021 12:52:47 +0200
Message-ID: <20210407105252.30721-8-roberto.sassu@huawei.com> (raw)
In-Reply-To: <20210407105252.30721-1-roberto.sassu@huawei.com>

If files with portable signatures are copied from one location to another
or are extracted from an archive, verification can temporarily fail until
all xattrs/attrs are set in the destination. Only portable signatures may
be moved or copied from one file to another, as they don't depend on
system-specific information such as the inode generation. Instead portable
signatures must include security.ima.

Unlike other security.evm types, EVM portable signatures are also
immutable. Thus, it wouldn't be a problem to allow xattr/attr operations
when verification fails, as portable signatures will never be replaced with
the HMAC on possibly corrupted xattrs/attrs.

This patch first introduces a new integrity status called
INTEGRITY_FAIL_IMMUTABLE, that allows callers of
evm_verify_current_integrity() to detect that a portable signature didn't
pass verification and then adds an exception in evm_protect_xattr() and
evm_inode_setattr() for this status and returns 0 instead of -EPERM.

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
Reviewed-by: Mimi Zohar <zohar@linux.ibm.com>
---
 include/linux/integrity.h             |  1 +
 security/integrity/evm/evm_main.c     | 31 +++++++++++++++++++++------
 security/integrity/ima/ima_appraise.c |  2 ++
 3 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/include/linux/integrity.h b/include/linux/integrity.h
index 2271939c5c31..2ea0f2f65ab6 100644
--- a/include/linux/integrity.h
+++ b/include/linux/integrity.h
@@ -13,6 +13,7 @@ enum integrity_status {
 	INTEGRITY_PASS = 0,
 	INTEGRITY_PASS_IMMUTABLE,
 	INTEGRITY_FAIL,
+	INTEGRITY_FAIL_IMMUTABLE,
 	INTEGRITY_NOLABEL,
 	INTEGRITY_NOXATTRS,
 	INTEGRITY_UNKNOWN,
diff --git a/security/integrity/evm/evm_main.c b/security/integrity/evm/evm_main.c
index 6556e8c22da9..eab536fa260f 100644
--- a/security/integrity/evm/evm_main.c
+++ b/security/integrity/evm/evm_main.c
@@ -27,7 +27,8 @@
 int evm_initialized;
 
 static const char * const integrity_status_msg[] = {
-	"pass", "pass_immutable", "fail", "no_label", "no_xattrs", "unknown"
+	"pass", "pass_immutable", "fail", "fail_immutable", "no_label",
+	"no_xattrs", "unknown"
 };
 int evm_hmac_attrs;
 
@@ -155,7 +156,7 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 	enum integrity_status evm_status = INTEGRITY_PASS;
 	struct evm_digest digest;
 	struct inode *inode;
-	int rc, xattr_len;
+	int rc, xattr_len, evm_immutable = 0;
 
 	if (iint && (iint->evm_status == INTEGRITY_PASS ||
 		     iint->evm_status == INTEGRITY_PASS_IMMUTABLE))
@@ -200,8 +201,10 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		if (rc)
 			rc = -EINVAL;
 		break;
-	case EVM_IMA_XATTR_DIGSIG:
 	case EVM_XATTR_PORTABLE_DIGSIG:
+		evm_immutable = 1;
+		fallthrough;
+	case EVM_IMA_XATTR_DIGSIG:
 		/* accept xattr with non-empty signature field */
 		if (xattr_len <= sizeof(struct signature_v2_hdr)) {
 			evm_status = INTEGRITY_FAIL;
@@ -238,9 +241,12 @@ static enum integrity_status evm_verify_hmac(struct dentry *dentry,
 		break;
 	}
 
-	if (rc)
-		evm_status = (rc == -ENODATA) ?
-				INTEGRITY_NOXATTRS : INTEGRITY_FAIL;
+	if (rc) {
+		evm_status = INTEGRITY_NOXATTRS;
+		if (rc != -ENODATA)
+			evm_status = evm_immutable ?
+				     INTEGRITY_FAIL_IMMUTABLE : INTEGRITY_FAIL;
+	}
 out:
 	if (iint)
 		iint->evm_status = evm_status;
@@ -374,6 +380,14 @@ static int evm_protect_xattr(struct dentry *dentry, const char *xattr_name,
 out:
 	if (evm_ignore_error_safe(evm_status))
 		return 0;
+
+	/*
+	 * Writing other xattrs is safe for portable signatures, as portable
+	 * signatures are immutable and can never be updated.
+	 */
+	if (evm_status == INTEGRITY_FAIL_IMMUTABLE)
+		return 0;
+
 	if (evm_status != INTEGRITY_PASS)
 		integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
 				    dentry->d_name.name, "appraise_metadata",
@@ -534,8 +548,13 @@ int evm_inode_setattr(struct dentry *dentry, struct iattr *attr)
 	if (!(ia_valid & (ATTR_MODE | ATTR_UID | ATTR_GID)))
 		return 0;
 	evm_status = evm_verify_current_integrity(dentry);
+	/*
+	 * Writing attrs is safe for portable signatures, as portable signatures
+	 * are immutable and can never be updated.
+	 */
 	if ((evm_status == INTEGRITY_PASS) ||
 	    (evm_status == INTEGRITY_NOXATTRS) ||
+	    (evm_status == INTEGRITY_FAIL_IMMUTABLE) ||
 	    (evm_ignore_error_safe(evm_status)))
 		return 0;
 	integrity_audit_msg(AUDIT_INTEGRITY_METADATA, d_backing_inode(dentry),
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index d4b8db1acadd..24d59893aab0 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -416,6 +416,8 @@ int ima_appraise_measurement(enum ima_hooks func,
 	case INTEGRITY_NOLABEL:		/* No security.evm xattr. */
 		cause = "missing-HMAC";
 		goto out;
+	case INTEGRITY_FAIL_IMMUTABLE:
+		fallthrough;
 	case INTEGRITY_FAIL:		/* Invalid HMAC/signature. */
 		cause = "invalid-HMAC";
 		goto out;
-- 
2.26.2


  parent reply index

Thread overview: 40+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-07 10:52 [PATCH v5 00/12] evm: Improve usability of " Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 01/12] evm: Execute evm_inode_init_security() only when an HMAC key is loaded Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 02/12] evm: Load EVM key in ima_load_x509() to avoid appraisal Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 03/12] evm: Refuse EVM_ALLOW_METADATA_WRITES only if an HMAC key is loaded Roberto Sassu
2021-04-30 20:28   ` Mimi Zohar
2021-04-07 10:52 ` [PATCH v5 04/12] ima: Move ima_reset_appraise_flags() call to post hooks Roberto Sassu
2021-04-07 16:17   ` Casey Schaufler
2021-04-07 16:31     ` Roberto Sassu
2021-04-26 19:49     ` Mimi Zohar
2021-04-07 10:52 ` [PATCH v5 05/12] evm: Introduce evm_status_revalidate() Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 06/12] evm: Ignore INTEGRITY_NOLABEL/INTEGRITY_NOXATTRS if conditions are safe Roberto Sassu
2021-05-03  0:12   ` Mimi Zohar
2021-05-03  7:55     ` Roberto Sassu
2021-05-03 12:07       ` Mimi Zohar
2021-05-03 14:15       ` Roberto Sassu
2021-05-03 14:34         ` Mimi Zohar
2021-05-04 13:16           ` Roberto Sassu
2021-05-04 13:45             ` Mimi Zohar
2021-04-07 10:52 ` Roberto Sassu [this message]
2021-05-03  0:12   ` [PATCH v5 07/12] evm: Allow xattr/attr operations for portable signatures Mimi Zohar
2021-05-04 14:28     ` Roberto Sassu
2021-05-04 14:49       ` Mimi Zohar
2021-04-07 10:52 ` [PATCH v5 08/12] evm: Pass user namespace to set/remove xattr hooks Roberto Sassu
2021-04-07 12:06   ` Christian Brauner
2021-04-07 10:52 ` [PATCH v5 09/12] evm: Allow setxattr() and setattr() for unmodified metadata Roberto Sassu
2021-04-07 12:05   ` Christian Brauner
2021-04-07 15:23   ` kernel test robot
2021-04-07 18:14   ` kernel test robot
2021-04-07 19:28     ` [RESEND][PATCH " Roberto Sassu
2021-05-03 13:00   ` [PATCH " Mimi Zohar
2021-05-03 14:48     ` Roberto Sassu
2021-05-03 15:13       ` Mimi Zohar
2021-05-03 15:30         ` Roberto Sassu
2021-05-03 15:11     ` Roberto Sassu
2021-05-03 15:26       ` Mimi Zohar
2021-05-03 15:32         ` Roberto Sassu
2021-05-03 15:48           ` Mimi Zohar
2021-04-07 10:52 ` [PATCH v5 10/12] ima: Allow imasig requirement to be satisfied by EVM portable signatures Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 11/12] ima: Introduce template field evmsig and write to field sig as fallback Roberto Sassu
2021-04-07 10:52 ` [PATCH v5 12/12] ima: Don't remove security.ima if file must not be appraised Roberto Sassu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210407105252.30721-8-roberto.sassu@huawei.com \
    --to=roberto.sassu@huawei.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=mjg59@google.com \
    --cc=zohar@linux.ibm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-Security-Module Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-security-module/0 linux-security-module/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-security-module linux-security-module/ https://lore.kernel.org/linux-security-module \
		linux-security-module@vger.kernel.org
	public-inbox-index linux-security-module

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-security-module


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git