public inbox for linux-integrity@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] ima: define a new policy option named "force"
@ 2018-01-10 14:13 Alban Crequy
  2018-01-10 14:39 ` Mimi Zohar
  0 siblings, 1 reply; 9+ messages in thread
From: Alban Crequy @ 2018-01-10 14:13 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-integrity, Seth Forshee, Eric W . Biederman,
	dmitry.kasatkin, Sascha Hauer, Alban Crequy, dongsu

> There are times instead of relying on previously cached status
> information we want to force the file to be re-measured, re-appraised,
> and re-audited.
>
> This patch defines a new policy option named "force", which forces
> files to be re-measured, re-appraised or re-audited.
>
> Signed-off-by: Mimi Zohar <zohar@xxxxxxxxxxxxxxxxxx>

Tested-by: Alban Crequy <alban@kinvolk.io>


tl;dr: without the patch, the measurements don't get all the changes
on FUSE. With the patch and when enabling the "force" option,
ascii_runtime_measurements gets the updated measurements.


Longer explanation:

The test I did was using a patched version of the memfs FUSE driver
[1][2] and two very simple "hello-world" programs [4] (prog1 prints
"hello world: 1" and prog2 prints "hello world: 2").

I copy prog1 and prog2 in the fuse-memfs mount point, execute them and
check the sha1 hash in
"/sys/kernel/security/ima/ascii_runtime_measurements".

My patch on the memfs FUSE driver added a backdoor command to serve
prog1 when the kernel asks for prog2 or vice-versa. In this way, I can
exec prog1 and get it to print "hello world: 2" without ever replacing
the file via the VFS, so the kernel is not aware of the change.

The test was done using Dongsu's branch "fuse-userns-v5-2" [3],
including both this new force option and Sascha's patch ("ima: Use
i_version only when filesystem supports it").


Step by step test procedure:

1.  Mount the memfs FUSE using [2]:
rm -f  /tmp/memfs-switch* ; memfs -L DEBUG  /mnt/memfs

2. Copy prog1 and prog2 using [4]
cp prog1 /mnt/memfs/prog1
cp prog2 /mnt/memfs/prog2

3. Lookup the files and let the FUSE driver to keep the handles open:
dd if=/mnt/memfs/prog1 bs=1 | (read -n 1 x ; sleep 3600 ) &
dd if=/mnt/memfs/prog2 bs=1 | (read -n 1 x ; sleep 3600 ) &

4. Check the 2 programs work correctly:
$ /mnt/memfs/prog1
hello world: 1
$ /mnt/memfs/prog2
hello world: 2

5.  Check the measurements for prog1 and prog2:
$ sudo cat /sys/kernel/security/ima/ascii_runtime_measurements|grep
/mnt/memfs/prog
10 7ac5aed52061cb09120e977c6d04ee5c7b11c371 ima-ng
sha1:ac14c9268cd2811f7a5adea17b27d84f50e1122c /mnt/memfs/prog1
10 9acc17a9a32aec4a676b8f6558e17a3d6c9a78e6 ima-ng
sha1:799cb5d1e06d5c37ae7a76ba25ecd1bd01476383 /mnt/memfs/prog2

6. Use the backdoor command in my patched memfs to redirect file
operations on file handle 3 to file handle 2:
rm -f  /tmp/memfs-switch* ; touch /tmp/memfs-switch-3-2

7. Check how the FUSE driver serves different content for the files:
$ /mnt/memfs/prog1
hello world: 2
$ /mnt/memfs/prog2
hello world: 2

8. Check the measurements:
sudo cat /sys/kernel/security/ima/ascii_runtime_measurements|grep
/mnt/memfs/prog

Without the patches, on a vanilla kernel, there are no new
measurements, despite the FUSE driver having served different
executables. Same thing with the patch but without enabling the new
force option.

However, with the "force" option enabled, I can see additional
measurements for prog1 and prog2 with the hashes reversed when the
FUSE driver served the alternative content.


[1] https://github.com/bbengfort/memfs
[2] https://github.com/kinvolk/memfs/commits/alban/switch-files
[3] https://github.com/kinvolk/linux/commits/dongsu/fuse-userns-v5-2
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

^ permalink raw reply	[flat|nested] 9+ messages in thread
* [PATCH] ima: define a new policy option named "force"
@ 2017-12-08 18:12 Mimi Zohar
  2017-12-10 22:07 ` James Morris
  0 siblings, 1 reply; 9+ messages in thread
From: Mimi Zohar @ 2017-12-08 18:12 UTC (permalink / raw)
  To: linux-integrity; +Cc: Seth Forshee, Eric W. Biederman, Dmitry Kasatkin

There are times instead of relying on previously cached status
information we want to force the file to be re-measured, re-appraised,
and re-audited.

This patch defines a new policy option named "force", which forces
files to be re-measured, re-appraised or re-audited.

Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
---
 Documentation/ABI/testing/ima_policy |  2 +-
 security/integrity/ima/ima_main.c    | 22 ++++++++++++++++++++--
 security/integrity/ima/ima_policy.c  |  8 +++++++-
 security/integrity/integrity.h       |  1 +
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/Documentation/ABI/testing/ima_policy b/Documentation/ABI/testing/ima_policy
index 2028f2d093b2..b0e8143c681f 100644
--- a/Documentation/ABI/testing/ima_policy
+++ b/Documentation/ABI/testing/ima_policy
@@ -24,7 +24,7 @@ Description:
 				[euid=] [fowner=]]
 			lsm:	[[subj_user=] [subj_role=] [subj_type=]
 				 [obj_user=] [obj_role=] [obj_type=]]
-			option:	[[appraise_type=]] [permit_directio]
+			option:	[[appraise_type=]] [permit_directio] [force]
 
 		base: 	func:= [BPRM_CHECK][MMAP_CHECK][FILE_CHECK][MODULE_CHECK]
 				[FIRMWARE_CHECK]
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 4dce3626dd4d..2a483184bc9a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -228,9 +228,27 @@ static int process_measurement(struct file *file, char *buf, loff_t size,
 				 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 */
+	/*
+	 * Reset the measure, appraise and audit cached flags either if
+	 * ima_inode_setxattr was called or based on policy, forcing
+	 * the file to be re-evaluated.
+	 */
+	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags)) {
 		iint->flags &= ~IMA_DONE_MASK;
+	} else if (action & IMA_FORCE) {
+		if (action & IMA_MEASURE) {
+			iint->measured_pcrs = 0;
+			iint->flags &=
+			    ~(IMA_COLLECTED | IMA_MEASURE | IMA_MEASURED);
+		}
+		if (action & IMA_APPRAISE)
+			iint->flags &=
+			    ~(IMA_COLLECTED | IMA_APPRAISE | IMA_APPRAISED |
+			      IMA_APPRAISE_SUBMASK | IMA_APPRAISED_SUBMASK);
+		if (action & IMA_AUDIT)
+			iint->flags &=
+			    ~(IMA_COLLECTED | IMA_AUDIT | IMA_AUDITED);
+	}
 
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 93dcf1bf92a8..878ae1a06e1e 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -533,7 +533,7 @@ enum {
 	Opt_uid_gt, Opt_euid_gt, Opt_fowner_gt,
 	Opt_uid_lt, Opt_euid_lt, Opt_fowner_lt,
 	Opt_appraise_type, Opt_permit_directio,
-	Opt_pcr
+	Opt_pcr, Opt_force
 };
 
 static match_table_t policy_tokens = {
@@ -566,6 +566,7 @@ static match_table_t policy_tokens = {
 	{Opt_appraise_type, "appraise_type=%s"},
 	{Opt_permit_directio, "permit_directio"},
 	{Opt_pcr, "pcr=%s"},
+	{Opt_force, "force"},
 	{Opt_err, NULL}
 };
 
@@ -895,6 +896,9 @@ static int ima_parse_rule(char *rule, struct ima_rule_entry *entry)
 				entry->flags |= IMA_PCR;
 
 			break;
+		case Opt_force:
+			entry->flags |= IMA_FORCE;
+			break;
 		case Opt_err:
 			ima_log_string(ab, "UNKNOWN", p);
 			result = -EINVAL;
@@ -1168,6 +1172,8 @@ int ima_policy_show(struct seq_file *m, void *v)
 		seq_puts(m, "appraise_type=imasig ");
 	if (entry->flags & IMA_PERMIT_DIRECTIO)
 		seq_puts(m, "permit_directio ");
+	if (entry->flags & IMA_FORCE)
+		seq_puts(m, "force ");
 	rcu_read_unlock();
 	seq_puts(m, "\n");
 	return 0;
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 50a8e3365df7..4e16b1212d0f 100644
--- a/security/integrity/integrity.h
+++ b/security/integrity/integrity.h
@@ -35,6 +35,7 @@
 #define IMA_PERMIT_DIRECTIO	0x02000000
 #define IMA_NEW_FILE		0x04000000
 #define EVM_IMMUTABLE_DIGSIG	0x08000000
+#define IMA_FORCE		0x10000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.7.4

^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2018-01-11 13:59 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-10 14:13 [PATCH] ima: define a new policy option named "force" Alban Crequy
2018-01-10 14:39 ` Mimi Zohar
2018-01-10 14:44   ` Seth Forshee
2018-01-10 14:48     ` Mimi Zohar
2018-01-11 13:59       ` Alban Crequy
  -- strict thread matches above, loose matches on Subject: below --
2017-12-08 18:12 Mimi Zohar
2017-12-10 22:07 ` James Morris
2017-12-11 13:12   ` Mimi Zohar
2017-12-11 13:30     ` Seth Forshee

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox