[PATCH v3 0/4] unverifiable file signatures

For local filesystems, the kernel prevents files being executed from
being modified.  With IMA-measurement enabled, the kernel also emits
audit "time of measure, time of use" messages for files opened for
read, and subsequently opened for write.

Files on fuse are initially measured, appraised, and audited.  Although
the file data can change dynamically any time, making re-measuring,
re-appraising, or re-auditing pointless, this patch set attempts to
differentiate between unprivileged non-init root and privileged
mounted fuse filesystems.

This patch set addresses three different scenarios:
- Unprivileged non-init root mounted fuse filesystems are untrusted.
  Signature verification should always fail and re-measuring,
  re-appraising, re-auditing files makes no sense.

  Always enabled.

- For privileged mounted filesystems in a "secure" environment, with a
  correctly enforced security policy, which is willing to assume the
  inherent risk of specific fuse filesystems, it is reasonable to
  re-measure, re-appraise, and re-audit files.

  Enabled by default to prevent breaking existing systems.

- Privileged mounted filesystems unwilling to assume the risks and
  prefers to securely fail safe.

  Enabled based on policy.

Changelog v3:
- Fix SB_IMA_UNVERIFIABLE_SIGNATURE & SB_I_UNTRUSTED_MOUNTER test.
- Rename the builtin policy name to "fail_securely".

Mimi

Mimi Zohar (4):
  ima: fail file signature verification on non-init mounted filesystems
  ima: re-evaluate files on privileged mounted filesystems
  ima: fail signature verification based on policy
  fuse: define the filesystem as untrusted

 Documentation/admin-guide/kernel-parameters.txt |  8 +++++++-
 fs/fuse/inode.c                                 |  3 +++
 include/linux/fs.h                              |  2 ++
 security/integrity/ima/ima_appraise.c           | 16 +++++++++++++++-
 security/integrity/ima/ima_main.c               | 14 ++++++++++++--
 security/integrity/ima/ima_policy.c             |  5 +++++
 security/integrity/integrity.h                  |  1 +
 7 files changed, 45 insertions(+), 4 deletions(-)

-- 
2.7.5

--
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

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch addresses the new unprivileged non-init mounted filesystems,
which are untrusted, by failing the signature verification.

This patch defines two new flags SB_I_IMA_UNVERIFIABLE_SIGNATURE and
SB_I_UNTRUSTED_MOUNTER.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

---
Changelog v3:
- Fix SB_IMA_UNVERIFIABLE_SIGNATURE & SB_I_UNTRUSTED_MOUNTER test.

Changelog v2:
- Limit patch to non-init mounted filesystems.
- Define 2 sb->s_iflags

Changelog v1:
- Merged the unprivileged and privileged patches.
- Dropped IMA fsname support.
- Introduced a new IMA builtin policy named "untrusted_fs".
- Replaced fs_type flag with sb->s_iflags flag.

 include/linux/fs.h                    |  2 ++
 security/integrity/ima/ima_appraise.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..4e1c76af7b68 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -1320,6 +1320,8 @@ extern int send_sigurg(struct fown_struct *fown);
 
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
+#define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
+#define SB_I_UNTRUSTED_MOUNTER		0x00000040
 
 /* Possible states of 'frozen' field */
 enum {

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1b177461f20e..4bafb397ee91 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -302,7 +302,19 @@ int ima_appraise_measurement(enum ima_hooks func,
 	}
 
 out:
-	if (status != INTEGRITY_PASS) {
+	/*
+	 * File signatures on some filesystems can not be properly verified.
+	 * On these filesytems, that are mounted by an untrusted mounter,
+	 * fail the file signature verification.
+	 */
+	if ((inode->i_sb->s_iflags &
+	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) ==
+	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) {
+		status = INTEGRITY_FAIL;
+		cause = "unverifiable-signature";
+		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,
+				    op, cause, rc, 0);
+	} else if (status != INTEGRITY_PASS) {
 		if ((ima_appraise & IMA_APPRAISE_FIX) &&
 		    (!xattr_value ||
 		     xattr_value->type != EVM_IMA_XATTR_DIGSIG)) {

@@ -319,6 +331,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	} else {
 		ima_cache_flags(iint, func);
 	}
+
 	ima_set_cache_status(iint, func, status);
 	return status;
 }

-- 
2.7.5

--
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

This patch addresses the fuse privileged mounted filesystems in a "secure"
environment, with a correctly enforced security policy, which is willing
to assume the inherent risk of specific fuse filesystems that are well
defined and properly implemented.

As there is no way for the kernel to detect file changes, the kernel
ignores the cached file integrity results and re-measures, re-appraises,
and re-audits the file.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>
---
 security/integrity/ima/ima_main.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index a5d225ffc388..f550f25294a3 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

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

@@ -230,9 +231,17 @@ static int process_measurement(struct file *file, const struct cred *cred,
 				 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 */
+	/*
+	 * Re-evaulate the file if either the xattr has changed or the
+	 * kernel has no way of detecting file change on the filesystem.
+	 * (Limited to privileged mounted filesystems.)
+	 */
+	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) ||
+	    ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
+	     !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER))) {
 		iint->flags &= ~IMA_DONE_MASK;
+		iint->measured_pcrs = 0;
+	}
 
 	/* Determine if already appraised/measured based on bitmask
 	 * (IMA_MEASURE, IMA_MEASURED, IMA_XXXX_APPRAISE, IMA_XXXX_APPRAISED,

-- 
2.7.5

--
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

This patch addresses the fuse privileged mounted filesystems in
environments which are unwilling to accept the risk of trusting the
signature verification and want to always fail safe, but are for example
using a pre-built kernel.

This patch defines a new builtin policy named "fail_securely", which can
be specified on the boot command line as an argument to "ima_policy=".

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

---
Changelog v3:
- Rename the builtin policy name

Changelog v2:
- address the fail safe environement

 Documentation/admin-guide/kernel-parameters.txt |  8 +++++++-
 security/integrity/ima/ima_appraise.c           | 11 ++++++-----
 security/integrity/ima/ima_main.c               |  3 ++-
 security/integrity/ima/ima_policy.c             |  5 +++++
 security/integrity/integrity.h                  |  1 +
 5 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..2cc17dc7ab84 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt

@@ -1525,7 +1525,8 @@
 
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
-			Format: "tcb | appraise_tcb | secure_boot"
+			Format: "tcb | appraise_tcb | secure_boot |
+				 fail_securely"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read

@@ -1540,6 +1541,11 @@
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
 
+			The "fail_securely" policy forces file signature
+			verification failure also on privileged mounted
+			filesystems with the SB_I_UNVERIFIABLE_SIGNATURE
+			flag.
+
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4bafb397ee91..3034935e1eb3 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -304,12 +304,13 @@ int ima_appraise_measurement(enum ima_hooks func,
 out:
 	/*
 	 * File signatures on some filesystems can not be properly verified.
-	 * On these filesytems, that are mounted by an untrusted mounter,
-	 * fail the file signature verification.
+	 * On these filesytems, that are mounted by an untrusted mounter or
+	 * for systems not willing to accept the risk, fail the file signature
+	 * verification.
 	 */
-	if ((inode->i_sb->s_iflags &
-	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) ==
-	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) {
+	if ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
+	    ((inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) ||
+	     (iint->flags & IMA_FAIL_UNVERIFIABLE_SIGS))) {
 		status = INTEGRITY_FAIL;
 		cause = "unverifiable-signature";
 		integrity_audit_msg(AUDIT_INTEGRITY_DATA, inode, filename,

diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index f550f25294a3..5d122daf5c8a 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c

@@ -238,7 +238,8 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	 */
 	if (test_and_clear_bit(IMA_CHANGE_XATTR, &iint->atomic_flags) ||
 	    ((inode->i_sb->s_iflags & SB_I_IMA_UNVERIFIABLE_SIGNATURE) &&
-	     !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER))) {
+	     !(inode->i_sb->s_iflags & SB_I_UNTRUSTED_MOUNTER) &&
+	     !(action & IMA_FAIL_UNVERIFIABLE_SIGS))) {
 		iint->flags &= ~IMA_DONE_MASK;
 		iint->measured_pcrs = 0;
 	}

diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index e3da29af2c16..36f9570941c1 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c

@@ -188,6 +188,7 @@ __setup("ima_tcb", default_measure_policy_setup);
 
 static bool ima_use_appraise_tcb __initdata;
 static bool ima_use_secure_boot __initdata;
+static bool ima_fail_unverifiable_sigs __ro_after_init;
 static int __init policy_setup(char *str)
 {
 	char *p;

@@ -201,6 +202,8 @@ static int __init policy_setup(char *str)
 			ima_use_appraise_tcb = true;
 		else if (strcmp(p, "secure_boot") == 0)
 			ima_use_secure_boot = true;
+		else if (strcmp(p, "fail_securely") == 0)
+			ima_fail_unverifiable_sigs = true;
 	}
 
 	return 1;

@@ -390,6 +393,8 @@ int ima_match_policy(struct inode *inode, const struct cred *cred, u32 secid,
 		if (entry->action & IMA_APPRAISE) {
 			action |= get_subaction(entry, func);
 			action ^= IMA_HASH;
+			if (ima_fail_unverifiable_sigs)
+				action |= IMA_FAIL_UNVERIFIABLE_SIGS;
 		}
 
 		if (entry->action & IMA_DO_MASK)

diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 843ae23ba0ac..8224880935e0 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_FAIL_UNVERIFIABLE_SIGS	0x10000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)

-- 
2.7.5

--
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

Files on FUSE can change at any point in time without IMA being able
to detect it.  The file data read for the file signature verification
could be totally different from what is subsequently read, making the
signature verification useless.

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when
appropriate sets the SB_I_UNTRUSTED_MOUNTER flag.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>
---
 fs/fuse/inode.c | 3 +++
 1 file changed, 3 insertions(+)

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

@@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
 	sb->s_maxbytes = MAX_LFS_FILESIZE;
 	sb->s_time_gran = 1;
 	sb->s_export_op = &fuse_export_operations;
+	sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE;
+	if (sb->s_user_ns != &init_user_ns)
+		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
 
 	file = fget(d.fd);
 	err = -EINVAL;

-- 
2.7.5

--
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 Mimi Zohar (zohar at linux.vnet.ibm.com):

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch addresses the new unprivileged non-init mounted filesystems,
which are untrusted, by failing the signature verification.

This patch defines two new flags SB_I_IMA_UNVERIFIABLE_SIGNATURE and
SB_I_UNTRUSTED_MOUNTER.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

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

One comment below though,

Heh, this is misleading combination of parentheses and indentation :)
I would recommend using a temporary variable like:

	cmpflags = SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER;
	if ((inode->i_sb->s_iflags & cmpflags) == cmpflags) {

or maybe a helper function.

--
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 Mimi Zohar (zohar at linux.vnet.ibm.com):

This patch addresses the fuse privileged mounted filesystems in a "secure"
environment, with a correctly enforced security policy, which is willing
to assume the inherent risk of specific fuse filesystems that are well
defined and properly implemented.

As there is no way for the kernel to detect file changes, the kernel
ignores the cached file integrity results and re-measures, re-appraises,
and re-audits the file.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

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

--
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 Serge E. Hallyn (serge at hallyn.com):

Quoting Mimi Zohar (zohar at linux.vnet.ibm.com):

quoted

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch addresses the new unprivileged non-init mounted filesystems,
which are untrusted, by failing the signature verification.

This patch defines two new flags SB_I_IMA_UNVERIFIABLE_SIGNATURE and
SB_I_UNTRUSTED_MOUNTER.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

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

One comment below though,

quoted

---
Changelog v3:
- Fix SB_IMA_UNVERIFIABLE_SIGNATURE & SB_I_UNTRUSTED_MOUNTER test.

Changelog v2:
- Limit patch to non-init mounted filesystems.
- Define 2 sb->s_iflags

Changelog v1:
- Merged the unprivileged and privileged patches.
- Dropped IMA fsname support.
- Introduced a new IMA builtin policy named "untrusted_fs".
- Replaced fs_type flag with sb->s_iflags flag.

 include/linux/fs.h                    |  2 ++
 security/integrity/ima/ima_appraise.c | 15 ++++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..4e1c76af7b68 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h

@@ -1320,6 +1320,8 @@ extern int send_sigurg(struct fown_struct *fown);
 
 /* sb->s_iflags to limit user namespace mounts */
 #define SB_I_USERNS_VISIBLE		0x00000010 /* fstype already mounted */
+#define SB_I_IMA_UNVERIFIABLE_SIGNATURE	0x00000020
+#define SB_I_UNTRUSTED_MOUNTER		0x00000040
 
 /* Possible states of 'frozen' field */
 enum {

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 1b177461f20e..4bafb397ee91 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -302,7 +302,19 @@ int ima_appraise_measurement(enum ima_hooks func,
 	}
 
 out:
-	if (status != INTEGRITY_PASS) {
+	/*
+	 * File signatures on some filesystems can not be properly verified.
+	 * On these filesytems, that are mounted by an untrusted mounter,
+	 * fail the file signature verification.
+	 */
+	if ((inode->i_sb->s_iflags &
+	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) ==
+	    (SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER)) {

Heh, this is misleading combination of parentheses and indentation :)
I would recommend using a temporary variable like:

	cmpflags = SB_I_IMA_UNVERIFIABLE_SIGNATURE | SB_I_UNTRUSTED_MOUNTER;
	if ((inode->i_sb->s_iflags & cmpflags) == cmpflags) {

or maybe a helper function.

Never mind, I see it's going away two patches later :)
--
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 Mimi Zohar (zohar at linux.vnet.ibm.com):

This patch addresses the fuse privileged mounted filesystems in
environments which are unwilling to accept the risk of trusting the
signature verification and want to always fail safe, but are for example
using a pre-built kernel.

This patch defines a new builtin policy named "fail_securely", which can
be specified on the boot command line as an argument to "ima_policy=".

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

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

but,

How about "When such filesystems are mounted by an untrusted mounter or
on a system not willing to accept such a risk, ..." ?

(also filesytems is misspelled :)

+	 * for systems not willing to accept the risk, fail the file signature
+	 * verification.
 	 */

--
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 Mimi Zohar (zohar at linux.vnet.ibm.com):

Files on FUSE can change at any point in time without IMA being able
to detect it.  The file data read for the file signature verification
could be totally different from what is subsequently read, making the
signature verification useless.

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when
appropriate sets the SB_I_UNTRUSTED_MOUNTER flag.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>

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

Of course when IMA namespacing hits, you'll want to compare the
sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns
right?

--
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, 2018-03-12 at 14:28 -0500, Serge E. Hallyn wrote:

Quoting Mimi Zohar (zohar at linux.vnet.ibm.com):

quoted

This patch addresses the fuse privileged mounted filesystems in
environments which are unwilling to accept the risk of trusting the
signature verification and want to always fail safe, but are for example
using a pre-built kernel.

This patch defines a new builtin policy named "fail_securely", which can
be specified on the boot command line as an argument to "ima_policy=".

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: Serge E. Hallyn <serge@hallyn.com>

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

Thanks!

but,

quoted

---
Changelog v3:
- Rename the builtin policy name

Changelog v2:
- address the fail safe environement

 Documentation/admin-guide/kernel-parameters.txt |  8 +++++++-
 security/integrity/ima/ima_appraise.c           | 11 ++++++-----
 security/integrity/ima/ima_main.c               |  3 ++-
 security/integrity/ima/ima_policy.c             |  5 +++++
 security/integrity/integrity.h                  |  1 +
 5 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 1d1d53f85ddd..2cc17dc7ab84 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt

@@ -1525,7 +1525,8 @@
 
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
-			Format: "tcb | appraise_tcb | secure_boot"
+			Format: "tcb | appraise_tcb | secure_boot |
+				 fail_securely"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read

@@ -1540,6 +1541,11 @@
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
 
+			The "fail_securely" policy forces file signature
+			verification failure also on privileged mounted
+			filesystems with the SB_I_UNVERIFIABLE_SIGNATURE
+			flag.
+
 	ima_tcb		[IMA] Deprecated.  Use ima_policy= instead.
 			Load a policy which meets the needs of the Trusted
 			Computing Base.  This means IMA will measure all

diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 4bafb397ee91..3034935e1eb3 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c

@@ -304,12 +304,13 @@ int ima_appraise_measurement(enum ima_hooks func,
 out:
 	/*
 	 * File signatures on some filesystems can not be properly verified.
-	 * On these filesytems, that are mounted by an untrusted mounter,
-	 * fail the file signature verification.
+	 * On these filesytems, that are mounted by an untrusted mounter or

How about "When such filesystems are mounted by an untrusted mounter or
on a system not willing to accept such a risk, ..." ?

(also filesytems is misspelled :)

It definitely sounds better.

quoted

+	 * for systems not willing to accept the risk, fail the file signature
+	 * verification.
 	 */

--
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 03/12/2018 03:29 PM, Serge E. Hallyn wrote:

Quoting Mimi Zohar (zohar at linux.vnet.ibm.com):

quoted

Files on FUSE can change at any point in time without IMA being able
to detect it.  The file data read for the file signature verification
could be totally different from what is subsequently read, making the
signature verification useless.

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when
appropriate sets the SB_I_UNTRUSTED_MOUNTER flag.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>

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

Of course when IMA namespacing hits, you'll want to compare the
sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns
right?

I suppose this would be the only way to enable 'trusted mounters' within 
IMA namespaces. Maybe there could be an additional capability gate that 
would allow one to be a 'trusted mounter' then?

quoted

---
  fs/fuse/inode.c | 3 +++
  1 file changed, 3 insertions(+)

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

@@ -1080,6 +1080,9 @@ static int fuse_fill_super(struct super_block *sb, void *data, int silent)
  	sb->s_maxbytes = MAX_LFS_FILESIZE;
  	sb->s_time_gran = 1;
  	sb->s_export_op = &fuse_export_operations;
+	sb->s_iflags |= SB_I_IMA_UNVERIFIABLE_SIGNATURE;
+	if (sb->s_user_ns != &init_user_ns)
+		sb->s_iflags |= SB_I_UNTRUSTED_MOUNTER;
  
  	file = fget(d.fd);
  	err = -EINVAL;

-- 
2.7.5

--
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

I do not have any comments about the patches but a question.
I completely agree that the files can change without the VFS knowing
about it, but isn't that in general the case with filesystems with a
backend shared with others (network fs's?).

Stef
--
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 Wed, 2018-03-14 at 08:52 +0100, Stef Bon wrote:

I do not have any comments about the patches but a question.
I completely agree that the files can change without the VFS knowing
about it, but isn't that in general the case with filesystems with a
backend shared with others (network fs's?).

Right, the problem is not limited to fuse, but needs to be addressed
before unprivileged fuse mounts are upstreamed.

Alban's response to this question:
https://marc.info/?l=linux-kernel&m=151784020321045&w=2

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

Quoting Stefan Berger (stefanb at linux.vnet.ibm.com):

On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:

quoted

Quoting Mimi Zohar (zohar at linux.vnet.ibm.com):

quoted

Files on FUSE can change at any point in time without IMA being able
to detect it.  The file data read for the file signature verification
could be totally different from what is subsequently read, making the
signature verification useless.

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when
appropriate sets the SB_I_UNTRUSTED_MOUNTER flag.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>

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

Of course when IMA namespacing hits, you'll want to compare the
sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns
right?

I suppose this would be the only way to enable 'trusted mounters'
within IMA namespaces. Maybe there could be an additional capability
gate that would allow one to be a 'trusted mounter' then?

Wouldn't CAP_SYS_ADMIN to the ima_ns->user_ns suffice?

I personally think CAP_INTEGRITY would make sense, but right
now CAP_SYS_ADMIN seems to suffice so it wouldn't make sense to
raise the bar there unless we raise it for all of IMA configuration.

-serge
--
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 03/14/2018 10:27 AM, Serge E. Hallyn wrote:

Quoting Stefan Berger (stefanb at linux.vnet.ibm.com):

quoted

On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:

quoted

Quoting Mimi Zohar (zohar at linux.vnet.ibm.com):

quoted

Files on FUSE can change at any point in time without IMA being able
to detect it.  The file data read for the file signature verification
could be totally different from what is subsequently read, making the
signature verification useless.

FUSE can be mounted by unprivileged users either today with fusermount
installed with setuid, or soon with the upcoming patches to allow FUSE
mounts in a non-init user namespace.

This patch sets the SB_I_IMA_UNVERIFIABLE_SIGNATURE flag and when
appropriate sets the SB_I_UNTRUSTED_MOUNTER flag.

Signed-off-by: Mimi Zohar <redacted>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <redacted>
Cc: Eric W. Biederman <redacted>
Cc: Dongsu Park <redacted>
Cc: Alban Crequy <redacted>
Cc: "Serge E. Hallyn" <serge@hallyn.com>

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

Of course when IMA namespacing hits, you'll want to compare the
sb->s_user_ns to the (~handwaving~) user_ns owning the ima ns
right?

I suppose this would be the only way to enable 'trusted mounters'
within IMA namespaces. Maybe there could be an additional capability
gate that would allow one to be a 'trusted mounter' then?

Wouldn't CAP_SYS_ADMIN to the ima_ns->user_ns suffice?

I personally think CAP_INTEGRITY would make sense, but right
now CAP_SYS_ADMIN seems to suffice so it wouldn't make sense to
raise the bar there unless we raise it for all of IMA configuration.

So for IMA namespacing we may want to avoid CAP_SYS_ADMIN and introduce 
one or more capabilities to:

- set security xattrs from inside the container (when building the 
container for example, maybe also during runtime)
- access IMA's securityfs entries 
(https://elixir.bootlin.com/linux/latest/source/security/integrity/ima/ima_fs.c#L391) 
from inside the container for reading/writing the policy  (during run-time)
- then possibly mounting a trusted filesystem via fuse

    Stefan

-serge

--
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