[RFC PATCH v3 0/2] ima,fuse: introduce new fs flag FS_IMA_NO_CACHE

This patchset v3 introduces a new fs flag FS_IMA_NO_CACHE and uses it in
FUSE. This forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

There was a previous attempt (unmerged) with a IMA option named "force" and using
that option for FUSE filesystems. These patches use a different approach
so that the IMA subsystem does not need to know about FUSE.
- https://www.spinics.net/lists/linux-integrity/msg00948.html
- https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1584131.html

Changes since v1: https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1587390.html
- include linux-fsdevel mailing list in cc
- mark patch as RFC
- based on next-integrity, without other unmerged FUSE / IMA patches

Changes since v2: https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1587678.html
- rename flag to FS_IMA_NO_CACHE
- split patch into 2

The patchset is also available in our github repo:
  https://github.com/kinvolk/linux/tree/alban/fuse-flag-ima-nocache-v3


Alban Crequy (2):
  fuse: introduce new fs_type flag FS_IMA_NO_CACHE
  ima: force re-appraisal on filesystems with FS_IMA_NO_CACHE

 fs/fuse/inode.c                   |  2 +-
 include/linux/fs.h                |  1 +
 security/integrity/ima/ima_main.c | 24 ++++++++++++++++++++++--
 3 files changed, 24 insertions(+), 3 deletions(-)

-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Alban Crequy <redacted>

This new fs_type flag FS_IMA_NO_CACHE means files should be re-measured,
re-appraised and re-audited each time. Cached integrity results should
not be used.

It is useful in FUSE because the userspace FUSE process can change the
underlying files at any time without notifying the kernel.

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <redacted>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Seth Forshee <redacted>
Cc: Christoph Hellwig <hch@infradead.org>
Tested-by: Dongsu Park <redacted>
Signed-off-by: Alban Crequy <redacted>
---
 fs/fuse/inode.c    | 2 +-
 include/linux/fs.h | 1 +
 2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 624f18bbfd2b..0a9e516461d5 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c

@@ -1205,7 +1205,7 @@ static void fuse_kill_sb_anon(struct super_block *sb)
 static struct file_system_type fuse_fs_type = {
 	.owner		= THIS_MODULE,
 	.name		= "fuse",
-	.fs_flags	= FS_HAS_SUBTYPE,
+	.fs_flags	= FS_HAS_SUBTYPE | FS_IMA_NO_CACHE,
 	.mount		= fuse_mount,
 	.kill_sb	= fuse_kill_sb_anon,
 };

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 511fbaabf624..ced841ba6701 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -2075,6 +2075,7 @@ struct file_system_type {
 #define FS_BINARY_MOUNTDATA	2
 #define FS_HAS_SUBTYPE		4
 #define FS_USERNS_MOUNT		8	/* Can be mounted by userns root */
+#define FS_IMA_NO_CACHE		16	/* Force IMA to re-measure, re-appraise, re-audit files */
 #define FS_RENAME_DOES_D_MOVE	32768	/* FS will handle d_move() during rename() internally. */
 	struct dentry *(*mount) (struct file_system_type *, int,
 		       const char *, void *);

-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

From: Alban Crequy <redacted>

This patch forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

How to test this:

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 the branch "alban/fuse-flag-ima-nocache-v3" [3].

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 [...] ima-ng sha1:ac14c9268cd2[...] /mnt/memfs/prog1
10 [...] ima-ng sha1:799cb5d1e06d[...] /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 patch, there are no new measurements, despite the FUSE
driver having served different executables.

With the patch, 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/alban/fuse-flag-ima-nocache-v3
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <redacted>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Seth Forshee <redacted>
Cc: Christoph Hellwig <hch@infradead.org>
Tested-by: Dongsu Park <redacted>
Signed-off-by: Alban Crequy <redacted>
---
 security/integrity/ima/ima_main.c | 24 ++++++++++++++++++++++--
 1 file changed, 22 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 6d78cb26784d..8870a7bbe9b9 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -24,6 +24,7 @@
 #include <linux/slab.h>
 #include <linux/xattr.h>
 #include <linux/ima.h>
+#include <linux/fs.h>
 
 #include "ima.h"
 

@@ -228,9 +229,28 @@ 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 filesystem feature flag
+	 * 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 (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
+		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,

-- 
2.13.6

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Mon, Jan 22, 2018 at 05:24:52PM +0100, Alban Crequy wrote:

From: Alban Crequy <redacted>

This patch forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

How to test this:

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 the branch "alban/fuse-flag-ima-nocache-v3" [3].

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 [...] ima-ng sha1:ac14c9268cd2[...] /mnt/memfs/prog1
10 [...] ima-ng sha1:799cb5d1e06d[...] /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 patch, there are no new measurements, despite the FUSE
driver having served different executables.

With the patch, 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/alban/fuse-flag-ima-nocache-v3
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <redacted>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Seth Forshee <redacted>
Cc: Christoph Hellwig <hch@infradead.org>
Tested-by: Dongsu Park <redacted>
Signed-off-by: Alban Crequy <redacted>

I like this approach as it doesn't require any IMA policy changes to be
effective. I'm not sure about the flag name (in my mind it would
describe the fs property and not be specifically giving instructions to
IMA), but if others are happy with it I won't complain.

Acked-by: Seth Forshee <redacted>

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoting Alban Crequy (alban.crequy at gmail.com):

From: Alban Crequy <redacted>

This patch forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

How to test this:

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 the branch "alban/fuse-flag-ima-nocache-v3" [3].

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 [...] ima-ng sha1:ac14c9268cd2[...] /mnt/memfs/prog1
10 [...] ima-ng sha1:799cb5d1e06d[...] /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 patch, there are no new measurements, despite the FUSE
driver having served different executables.

With the patch, 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/alban/fuse-flag-ima-nocache-v3
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <redacted>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>

Acked-by: Serge Hallyn <serge@hallyn.com>

to both.

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Quoting Seth Forshee (seth.forshee at canonical.com):

On Mon, Jan 22, 2018 at 05:24:52PM +0100, Alban Crequy wrote:

quoted

From: Alban Crequy <redacted>

This patch forces files to be re-measured, re-appraised and re-audited
on file systems with the feature flag FS_IMA_NO_CACHE. In that way,
cached integrity results won't be used.

How to test this:

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 the branch "alban/fuse-flag-ima-nocache-v3" [3].

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 [...] ima-ng sha1:ac14c9268cd2[...] /mnt/memfs/prog1
10 [...] ima-ng sha1:799cb5d1e06d[...] /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 patch, there are no new measurements, despite the FUSE
driver having served different executables.

With the patch, 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/alban/fuse-flag-ima-nocache-v3
[4] https://github.com/kinvolk/fuse-userns-patches/commit/cf1f5750cab0

Cc: linux-kernel at vger.kernel.org
Cc: linux-integrity at vger.kernel.org
Cc: linux-security-module at vger.kernel.org
Cc: linux-fsdevel at vger.kernel.org
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>
Cc: Mimi Zohar <redacted>
Cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
Cc: James Morris <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
Cc: Seth Forshee <redacted>
Cc: Christoph Hellwig <hch@infradead.org>
Tested-by: Dongsu Park <redacted>
Signed-off-by: Alban Crequy <redacted>

I like this approach as it doesn't require any IMA policy changes to be
effective. I'm not sure about the flag name (in my mind it would
describe the fs property and not be specifically giving instructions to
IMA), but if others are happy with it I won't complain.

Purely internal name at least, so if we come up with a better name
we can trivially change it.
--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

quoted

@@ -228,9 +229,28 @@ 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 filesystem feature flag
+	 * 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 (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
+		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);
+	}

Alban, I don't know what I was thinking, but this can be simplified
like for the IMA_CHANGE_XATTR case. ?Except in the IMA_CHANGE_XATTR
case, "measured_pcrs" was already reset, whereas in this case
"measured_pcrs" needs to be reset.

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Alban,

On Thu, 2018-01-25 at 06:56 -0500, Mimi Zohar wrote:

quoted

quoted

@@ -228,9 +229,28 @@ 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 filesystem feature flag
+	 * 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 (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
+		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);
+	}

Alban, I don't know what I was thinking, but this can be simplified
like for the IMA_CHANGE_XATTR case. ?Except in the IMA_CHANGE_XATTR
case, "measured_pcrs" was already reset, whereas in this case
"measured_pcrs" needs to be reset.

Did you get a chance to make the change and test it?

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi Mimi,

On Mon, Jan 29, 2018 at 5:33 PM, Mimi Zohar [off-list ref] wrote:

Hi Alban,

On Thu, 2018-01-25 at 06:56 -0500, Mimi Zohar wrote:

quoted

quoted

quoted

@@ -228,9 +229,28 @@ 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 filesystem feature flag
+  * 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 (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
+         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);
+ }

Alban, I don't know what I was thinking, but this can be simplified
like for the IMA_CHANGE_XATTR case.  Except in the IMA_CHANGE_XATTR
case, "measured_pcrs" was already reset, whereas in this case
"measured_pcrs" needs to be reset.

Did you get a chance to make the change and test it?

Alban has been on holidays, so he will be back on Wednesday or so.
So I'll try to understand what you meant in the last email.

As IMA_DONE_MASK contains all other bitmasks, it's possible to
optimize the code like this:

        if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags)) {
                iint->flags &= ~IMA_DONE_MASK;
        } else if (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
                iint->flags &= ~IMA_DONE_MASK;
                if (action & IMA_MEASURE)
                        iint->measured_pcrs = 0;
        }

Is that what you want to see? Please let me know if it's not.
Tomorrow I will try to test with a new patch.

Thanks,
Dongsu

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Hi,

On Mon, Jan 29, 2018 at 6:40 PM, Dongsu Park [off-list ref] wrote:

On Mon, Jan 29, 2018 at 5:33 PM, Mimi Zohar [off-list ref] wrote:

quoted

On Thu, 2018-01-25 at 06:56 -0500, Mimi Zohar wrote:

...

quoted

Did you get a chance to make the change and test it?

Alban has been on holidays, so he will be back on Wednesday or so.
So I'll try to understand what you meant in the last email.

As IMA_DONE_MASK contains all other bitmasks, it's possible to
optimize the code like this:

        if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags)) {
                iint->flags &= ~IMA_DONE_MASK;
        } else if (inode->i_sb->s_type->fs_flags & FS_IMA_NO_CACHE) {
                iint->flags &= ~IMA_DONE_MASK;
                if (action & IMA_MEASURE)
                        iint->measured_pcrs = 0;
        }

Is that what you want to see? Please let me know if it's not.
Tomorrow I will try to test with a new patch.

Today I created a new patch, and tested it. It worked fine.
So I've just sent a new patchset v4. Please see:
https://www.mail-archive.com/linux-kernel at vger.kernel.org/msg1598387.html

Thanks,
Dongsu

Thanks,
Dongsu

quoted

Mimi

--
To unsubscribe from this list: send the line "unsubscribe linux-security-module" in
the body of a message to majordomo at vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html