Received-SPF: pass (google.com: domain of linux-kernel+bounces-65351-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: corbet@lwn.net,
	zohar@linux.ibm.com,
	dmitry.kasatkin@gmail.com,
	eric.snowberg@oracle.com,
	paul@paul-moore.com,
	jmorris@namei.org,
	serge@hallyn.com
Cc: linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org,
	linux-integrity@vger.kernel.org,
	linux-security-module@vger.kernel.org,
	wufan@linux.microsoft.com,
	pbrobinson@gmail.com,
	zbyszek@in.waw.pl,
	hch@lst.de,
	mjg59@srcf.ucam.org,
	pmatilai@redhat.com,
	jannh@google.com,
	dhowells@redhat.com,
	jikos@kernel.org,
	mkoutny@suse.com,
	ppavlu@suse.com,
	petr.vorel@gmail.com,
	petrtesarik@huaweicloud.com,
	mzerqung@0pointer.de,
	kgold@linux.ibm.com,
	Roberto Sassu <roberto.sassu@huawei.com>
Subject: [RFC][PATCH 8/8] ima: Detect if digest cache changed since last measurement/appraisal
Date: Wed, 14 Feb 2024 15:35:24 +0100
Message-Id: <20240214143525.2205481-9-roberto.sassu@huaweicloud.com>
In-Reply-To: <20240214143525.2205481-1-roberto.sassu@huaweicloud.com>
References: <20240214143525.2205481-1-roberto.sassu@huaweicloud.com>
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: Roberto Sassu <roberto.sassu@huawei.com>

IMA invalidates the cached verification result on file content/metadata
update, so that the file is evaluated again at next access.

While until now checking modifications on the file was sufficient to
determine if the cached verification result is still valid, that no longer
applies if that verification result was obtained with digest caches.

In that case, it is also necessary to check modifications on the digest
lists and on the security.digest_list xattr of the files for which digest
caches are used.

The digest_cache LSM offers the digest_cache_changed() function, which
tells if a file would use a different digest cache than the one passed as
argument. digest_cache_get() might return a different digest cache if the
digest list was modified/deleted/renamed or the security.digest_list xattr
was modified.

Hold a digest cache reference in the IMA integrity metadata, when using it
for measurement/appraisal. At every file access, check if that reference is
still actual by passing it to digest_cache_changed(). If not, reset the
integrity status and do the verification again.

Finally, move the digest_cache_put() call from process_measurement() to
ima_iint_free(), unless the digest cache changed. In that case, still
release the reference in process_measurement().

Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 security/integrity/ima/ima.h      |  1 +
 security/integrity/ima/ima_iint.c |  3 +++
 security/integrity/ima/ima_main.c | 22 ++++++++++++++++++----
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 36faf2bc81b0..c25bde918cd5 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -192,6 +192,7 @@ struct ima_iint_cache {
 	enum integrity_status ima_read_status:4;
 	enum integrity_status ima_creds_status:4;
 	struct ima_digest_data *ima_hash;
+	struct digest_cache *digest_cache;
 };
 
 extern struct lsm_blob_sizes ima_blob_sizes;
diff --git a/security/integrity/ima/ima_iint.c b/security/integrity/ima/ima_iint.c
index b4f476fae437..fd369809809f 100644
--- a/security/integrity/ima/ima_iint.c
+++ b/security/integrity/ima/ima_iint.c
@@ -68,6 +68,7 @@ static void ima_iint_init_always(struct ima_iint_cache *iint,
 	iint->ima_read_status = INTEGRITY_UNKNOWN;
 	iint->ima_creds_status = INTEGRITY_UNKNOWN;
 	iint->measured_pcrs = 0;
+	iint->digest_cache = NULL;
 	mutex_init(&iint->mutex);
 	ima_iint_lockdep_annotate(iint, inode, nested);
 }
@@ -75,6 +76,8 @@ static void ima_iint_init_always(struct ima_iint_cache *iint,
 static void ima_iint_free(struct ima_iint_cache *iint)
 {
 	kfree(iint->ima_hash);
+	if (iint->digest_cache)
+		digest_cache_put(iint->digest_cache);
 	mutex_destroy(&iint->mutex);
 	kmem_cache_free(ima_iint_cache, iint);
 }
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a66522a22cbc..e1b2f5737753 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -301,6 +301,15 @@ static int process_measurement(struct file *file, const struct cred *cred,
 		}
 	}
 
+	/* Check if digest cache changed since last measurement/appraisal. */
+	if (iint->digest_cache &&
+	    digest_cache_changed(inode, iint->digest_cache)) {
+		iint->flags &= ~IMA_DONE_MASK;
+		iint->measured_pcrs = 0;
+		digest_cache_put(iint->digest_cache);
+		iint->digest_cache = NULL;
+	}
+
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
 	 *  IMA_AUDIT, IMA_AUDITED)
@@ -371,8 +380,15 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	 * Since we allow IMA policy rules without func=, we have to enforce
 	 * this restriction here.
 	 */
-	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK)
-		digest_cache = digest_cache_get(file_dentry(file));
+	if (rc == 0 && policy_mask && func != DIGEST_LIST_CHECK) {
+		if (!iint->digest_cache) {
+			/* Released by ima_iint_free(). */
+			digest_cache = digest_cache_get(file_dentry(file));
+			iint->digest_cache = digest_cache;
+		} else {
+			digest_cache = iint->digest_cache;
+		}
+	}
 
 	if (digest_cache) {
 		found = digest_cache_lookup(file_dentry(file), digest_cache,
@@ -386,8 +402,6 @@ static int process_measurement(struct file *file, const struct cred *cred,
 			if (verif_mask_ptr)
 				allow_mask = policy_mask & *verif_mask_ptr;
 		}
-
-		digest_cache_put(digest_cache);
 	}
 
 	if (action & IMA_MEASURE)
-- 
2.34.1