[PATCH v3 0/4] unverifiable file signatures

[PATCH v3 0/4] unverifiable file signatures

[Mimi Zohar]

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

[PATCH v3 0/4] unverifiable file signatures

[Mimi Zohar]

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

[PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 3/4] ima: fail signature verification based on policy

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 3/4] ima: fail signature verification based on policy

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

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 <zohar@linux.vnet.ibm.com>
Cc: Miklos Szeredi <miklos@szeredi.hu>
Cc: Seth Forshee <seth.forshee@canonical.com>
Cc: Eric W. Biederman <ebiederm@xmission.com>
Cc: Dongsu Park <dongsu@kinvolk.io>
Cc: Alban Crequy <alban@kinvolk.io>
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

Re: [PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Serge E. Hallyn]

Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

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

One comment below though,

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

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

[PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Serge E. Hallyn]

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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

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

One comment below though,

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

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

Re: [PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Serge E. Hallyn]

Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

Acked-by: Serge 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

[PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Serge E. Hallyn]

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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

Acked-by: Serge 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

Re: [PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Serge E. Hallyn]

Quoting Serge E. Hallyn (serge@hallyn.com):
> Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> One comment below though,
> 
> > 
> > ---
> > 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 :)

[PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Serge E. Hallyn]

Quoting Serge E. Hallyn (serge at hallyn.com):
> 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 <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>
> 
> One comment below though,
> 
> > 
> > ---
> > 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

Re: [PATCH v3 3/4] ima: fail signature verification based on policy

[Serge E. Hallyn]

Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

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

but,

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

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

[PATCH v3 3/4] ima: fail signature verification based on policy

[Serge E. Hallyn]

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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> Cc: Serge E. Hallyn <serge@hallyn.com>

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

but,

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

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Serge E. Hallyn]

Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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?

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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Serge E. Hallyn]

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 <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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?

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

Re: [PATCH v3 3/4] ima: fail signature verification based on policy

[Mimi Zohar]

On Mon, 2018-03-12 at 14:28 -0500, Serge E. Hallyn wrote:
> Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>

Thanks!

> 
> but,
> 
> > 
> > ---
> > 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.

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

[PATCH v3 3/4] ima: fail signature verification based on policy

[Mimi Zohar]

On Mon, 2018-03-12 at 14:28 -0500, Serge E. Hallyn wrote:
> 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 <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > Cc: Serge E. Hallyn <serge@hallyn.com>
> 
> Acked-by: Serge Hallyn <serge@hallyn.com>

Thanks!

> 
> but,
> 
> > 
> > ---
> > 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.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stefan Berger]

On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
> Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
>> Cc: Miklos Szeredi <miklos@szeredi.hu>
>> Cc: Seth Forshee <seth.forshee@canonical.com>
>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>> Cc: Dongsu Park <dongsu@kinvolk.io>
>> Cc: Alban Crequy <alban@kinvolk.io>
>> 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?

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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stefan Berger]

On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
> 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 <zohar@linux.vnet.ibm.com>
>> Cc: Miklos Szeredi <miklos@szeredi.hu>
>> Cc: Seth Forshee <seth.forshee@canonical.com>
>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>> Cc: Dongsu Park <dongsu@kinvolk.io>
>> Cc: Alban Crequy <alban@kinvolk.io>
>> 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?

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

Re: [PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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;
>  }

[PATCH v3 1/4] ima: fail file signature verification on non-init mounted filesystems

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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;
>  }
--
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

Re: [PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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,

[PATCH v3 2/4] ima: re-evaluate files on privileged mounted filesystems

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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,
--
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

Re: [PATCH v3 3/4] ima: fail signature verification based on policy

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> 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=".

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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)

[PATCH v3 3/4] ima: fail signature verification based on policy

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> 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=".

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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)
--
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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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;

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
>
> Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> Cc: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Seth Forshee <seth.forshee@canonical.com>
> Cc: Eric W. Biederman <ebiederm@xmission.com>
> Cc: Dongsu Park <dongsu@kinvolk.io>
> Cc: Alban Crequy <alban@kinvolk.io>
> 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;
--
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

Re: [PATCH v3 0/4] unverifiable file signatures

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Overall ack on the way this is put together.

Eric

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

[PATCH v3 0/4] unverifiable file signatures

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

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

Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

Overall ack on the way this is put together.

Eric

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

Re: [PATCH v3 0/4] unverifiable file signatures

[Mimi Zohar]

On Tue, 2018-03-13 at 14:40 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Overall ack on the way this is put together.

Thanks you!

Mimi

[PATCH v3 0/4] unverifiable file signatures

[Mimi Zohar]

On Tue, 2018-03-13 at 14:40 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Overall ack on the way this is put together.

Thanks you!

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stef Bon]

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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stef Bon]

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Serge E. Hallyn]

Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
> On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
> >Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
> >>Cc: Miklos Szeredi <miklos@szeredi.hu>
> >>Cc: Seth Forshee <seth.forshee@canonical.com>
> >>Cc: Eric W. Biederman <ebiederm@xmission.com>
> >>Cc: Dongsu Park <dongsu@kinvolk.io>
> >>Cc: Alban Crequy <alban@kinvolk.io>
> >>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

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Serge E. Hallyn]

Quoting Stefan Berger (stefanb at linux.vnet.ibm.com):
> On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
> >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 <zohar@linux.vnet.ibm.com>
> >>Cc: Miklos Szeredi <miklos@szeredi.hu>
> >>Cc: Seth Forshee <seth.forshee@canonical.com>
> >>Cc: Eric W. Biederman <ebiederm@xmission.com>
> >>Cc: Dongsu Park <dongsu@kinvolk.io>
> >>Cc: Alban Crequy <alban@kinvolk.io>
> >>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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stefan Berger]

On 03/14/2018 10:27 AM, Serge E. Hallyn wrote:
> Quoting Stefan Berger (stefanb@linux.vnet.ibm.com):
>> On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
>>> Quoting Mimi Zohar (zohar@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 <zohar@linux.vnet.ibm.com>
>>>> Cc: Miklos Szeredi <miklos@szeredi.hu>
>>>> Cc: Seth Forshee <seth.forshee@canonical.com>
>>>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>>>> Cc: Dongsu Park <dongsu@kinvolk.io>
>>>> Cc: Alban Crequy <alban@kinvolk.io>
>>>> 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
>

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stefan Berger]

On 03/14/2018 10:27 AM, Serge E. Hallyn wrote:
> Quoting Stefan Berger (stefanb at linux.vnet.ibm.com):
>> On 03/12/2018 03:29 PM, Serge E. Hallyn wrote:
>>> 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 <zohar@linux.vnet.ibm.com>
>>>> Cc: Miklos Szeredi <miklos@szeredi.hu>
>>>> Cc: Seth Forshee <seth.forshee@canonical.com>
>>>> Cc: Eric W. Biederman <ebiederm@xmission.com>
>>>> Cc: Dongsu Park <dongsu@kinvolk.io>
>>>> Cc: Alban Crequy <alban@kinvolk.io>
>>>> 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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> 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

Which goes to why it is a flag that get's set.

All of this just needs a follow-up patch to update every filesystem
that does not meet ima's requirements.

Mimi I believe you said that the requirement is that all file changes
can be detected through the final __fput of a file that calls
ima_file_free.

Eric

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Mimi Zohar <zohar@linux.vnet.ibm.com> writes:

> 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

Which goes to why it is a flag that get's set.

All of this just needs a follow-up patch to update every filesystem
that does not meet ima's requirements.

Mimi I believe you said that the requirement is that all file changes
can be detected through the final __fput of a file that calls
ima_file_free.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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
> 
> Which goes to why it is a flag that get's set.
> 
> All of this just needs a follow-up patch to update every filesystem
> that does not meet ima's requirements.

Currently files on remote filesystems are measured/appraised/audited
once.  With the new flags, our options would be to either fail the
signature verification or constantly re-measure/re-appraise files on
remote file systems.  Neither option seems like the right solution.

There's some very initial discussions on how to support file integrity
on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
on the fs-verity work being done.  From a very, very high level, IMA-
appraisal would verify the file signature, but leave the integrity
enforcement to the vfs/fs layer.  By integrating fs-verity or similar
proposal with IMA, measurements would be included in the measurement
list and keys used for file signature verification would use the same
existing IMA-appraisal infrastructure.

> Mimi I believe you said that the requirement is that all file changes
> can be detected through the final __fput of a file that calls
> ima_file_free.

Right, like for fuse, I don't believe this existing hook works for
remote filesystems.

Mimi

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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
> 
> Which goes to why it is a flag that get's set.
> 
> All of this just needs a follow-up patch to update every filesystem
> that does not meet ima's requirements.

Currently files on remote filesystems are measured/appraised/audited
once. ?With the new flags, our options would be to either fail the
signature verification or constantly re-measure/re-appraise files on
remote file systems. ?Neither option seems like the right solution.

There's some very initial discussions on how to support file integrity
on remote filesystems. ?Chuck Lever has some thoughts on piggy-backing 
on the fs-verity work being done. ?From a very, very high level, IMA-
appraisal would verify the file signature, but leave the integrity
enforcement to the vfs/fs layer. ?By integrating fs-verity or similar
proposal with IMA, measurements would be included in the measurement
list and keys used for file signature verification would use the same
existing IMA-appraisal infrastructure.

> Mimi I believe you said that the requirement is that all file changes
> can be detected through the final __fput of a file that calls
> ima_file_free.

Right, like for fuse, I don't believe this existing hook works for
remote filesystems.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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
> 
> Which goes to why it is a flag that get's set.
> 
> All of this just needs a follow-up patch to update every filesystem
> that does not meet ima's requirements.

Currently files on remote filesystems are measured/appraised/audited
once.  With the new flags, our options would be to either fail the
signature verification or constantly re-measure/re-appraise files on
remote file systems.  Neither option seems like the right solution.

There's some very initial discussions on how to support file integrity
on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
on the fs-verity work being done.  From a very, very high level, IMA-
appraisal would verify the file signature, but leave the integrity
enforcement to the vfs/fs layer.  By integrating fs-verity or similar
proposal with IMA, measurements would be included in the measurement
list and keys used for file signature verification would use the same
existing IMA-appraisal infrastructure.

> Mimi I believe you said that the requirement is that all file changes
> can be detected through the final __fput of a file that calls
> ima_file_free.

Right, like for fuse, I don't believe this existing hook works for
remote filesystems.

Mimi

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>> 
>>> 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
>> 
>> Which goes to why it is a flag that get's set.
>> 
>> All of this just needs a follow-up patch to update every filesystem
>> that does not meet ima's requirements.
> 
> Currently files on remote filesystems are measured/appraised/audited
> once.  With the new flags, our options would be to either fail the
> signature verification or constantly re-measure/re-appraise files on
> remote file systems.  Neither option seems like the right solution.

Being new to this game, I may be making a bad assumption, but I thought
that the (NFSv4) change attribute can be used to detect remote mutations
to a file's content or metadata, so that the appraisal could be cached
as long as that attribute does not change. At least for NFSv4, clients
assume their data cache is valid while the change attribute remains the
same.

NFSv4 (and SMB) also has a mechanism where a server guarantees it will
report any other clients that want to update a file. This is an NFSv4
read delegation or an SMB oplock. NFSv4 clients use this mechanism to
cache file data quite aggressively, and it could also be used to
preserve or cache audit state on remote files.


> There's some very initial discussions on how to support file integrity
> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
> on the fs-verity work being done.  From a very, very high level, IMA-
> appraisal would verify the file signature, but leave the integrity
> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
> proposal with IMA, measurements would be included in the measurement
> list and keys used for file signature verification would use the same
> existing IMA-appraisal infrastructure.

>> Mimi I believe you said that the requirement is that all file changes
>> can be detected through the final __fput of a file that calls
>> ima_file_free.
> 
> Right, like for fuse, I don't believe this existing hook works for
> remote filesystems.
> 
> Mimi

--
Chuck Lever

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> 
> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>> 
>>> 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
>> 
>> Which goes to why it is a flag that get's set.
>> 
>> All of this just needs a follow-up patch to update every filesystem
>> that does not meet ima's requirements.
> 
> Currently files on remote filesystems are measured/appraised/audited
> once.  With the new flags, our options would be to either fail the
> signature verification or constantly re-measure/re-appraise files on
> remote file systems.  Neither option seems like the right solution.

Being new to this game, I may be making a bad assumption, but I thought
that the (NFSv4) change attribute can be used to detect remote mutations
to a file's content or metadata, so that the appraisal could be cached
as long as that attribute does not change. At least for NFSv4, clients
assume their data cache is valid while the change attribute remains the
same.

NFSv4 (and SMB) also has a mechanism where a server guarantees it will
report any other clients that want to update a file. This is an NFSv4
read delegation or an SMB oplock. NFSv4 clients use this mechanism to
cache file data quite aggressively, and it could also be used to
preserve or cache audit state on remote files.


> There's some very initial discussions on how to support file integrity
> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
> on the fs-verity work being done.  From a very, very high level, IMA-
> appraisal would verify the file signature, but leave the integrity
> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
> proposal with IMA, measurements would be included in the measurement
> list and keys used for file signature verification would use the same
> existing IMA-appraisal infrastructure.

>> Mimi I believe you said that the requirement is that all file changes
>> can be detected through the final __fput of a file that calls
>> ima_file_free.
> 
> Right, like for fuse, I don't believe this existing hook works for
> remote filesystems.
> 
> Mimi

--
Chuck Lever



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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Chuck Lever <chuck.lever@oracle.com> writes:

>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> 
>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>> 
>>>> 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
>>> 
>>> Which goes to why it is a flag that get's set.
>>> 
>>> All of this just needs a follow-up patch to update every filesystem
>>> that does not meet ima's requirements.
>> 
>> Currently files on remote filesystems are measured/appraised/audited
>> once.  With the new flags, our options would be to either fail the
>> signature verification or constantly re-measure/re-appraise files on
>> remote file systems.  Neither option seems like the right solution.

They are measured/appraised/audited once, and you can not trust that at
all because you don't know when the files are going to be different.

So either keeping the filesystem out of the ima game or failing sounds
like the right answer to me.  At least until you can get better
information from the filesystem.

> Being new to this game, I may be making a bad assumption, but I thought
> that the (NFSv4) change attribute can be used to detect remote mutations
> to a file's content or metadata, so that the appraisal could be cached
> as long as that attribute does not change. At least for NFSv4, clients
> assume their data cache is valid while the change attribute remains the
> same.
>
> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
> report any other clients that want to update a file. This is an NFSv4
> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
> cache file data quite aggressively, and it could also be used to
> preserve or cache audit state on remote files.

The file data invalid message, plus trusting the server, is what
would be needed for reliably caching the validity of the file.

>> There's some very initial discussions on how to support file integrity
>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>> on the fs-verity work being done.  From a very, very high level, IMA-
>> appraisal would verify the file signature, but leave the integrity
>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>> proposal with IMA, measurements would be included in the measurement
>> list and keys used for file signature verification would use the same
>> existing IMA-appraisal infrastructure.
>
>>> Mimi I believe you said that the requirement is that all file changes
>>> can be detected through the final __fput of a file that calls
>>> ima_file_free.
>> 
>> Right, like for fuse, I don't believe this existing hook works for
>> remote filesystems.

I am trying to understand things.

- I believe the beginning of any file write should invalidate the
  validity of the files cache.  IMA does something like that by looking
  at i_writecount.

- As I read the code ima_file_free triggers an update of the ima xattr
  and that update needs to wait until the file is quiescent.  AKA no
  more writers.  I am not certain how you get that in a remote
  filesystem.

  If the write is not coming from the local kernel I don't see how it
  makes any sense for IMA to actually update the xattr on write.

Eric

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Chuck Lever <chuck.lever@oracle.com> writes:

>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>> 
>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>> 
>>>> 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
>>> 
>>> Which goes to why it is a flag that get's set.
>>> 
>>> All of this just needs a follow-up patch to update every filesystem
>>> that does not meet ima's requirements.
>> 
>> Currently files on remote filesystems are measured/appraised/audited
>> once.  With the new flags, our options would be to either fail the
>> signature verification or constantly re-measure/re-appraise files on
>> remote file systems.  Neither option seems like the right solution.

They are measured/appraised/audited once, and you can not trust that at
all because you don't know when the files are going to be different.

So either keeping the filesystem out of the ima game or failing sounds
like the right answer to me.  At least until you can get better
information from the filesystem.

> Being new to this game, I may be making a bad assumption, but I thought
> that the (NFSv4) change attribute can be used to detect remote mutations
> to a file's content or metadata, so that the appraisal could be cached
> as long as that attribute does not change. At least for NFSv4, clients
> assume their data cache is valid while the change attribute remains the
> same.
>
> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
> report any other clients that want to update a file. This is an NFSv4
> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
> cache file data quite aggressively, and it could also be used to
> preserve or cache audit state on remote files.

The file data invalid message, plus trusting the server, is what
would be needed for reliably caching the validity of the file.

>> There's some very initial discussions on how to support file integrity
>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>> on the fs-verity work being done.  From a very, very high level, IMA-
>> appraisal would verify the file signature, but leave the integrity
>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>> proposal with IMA, measurements would be included in the measurement
>> list and keys used for file signature verification would use the same
>> existing IMA-appraisal infrastructure.
>
>>> Mimi I believe you said that the requirement is that all file changes
>>> can be detected through the final __fput of a file that calls
>>> ima_file_free.
>> 
>> Right, like for fuse, I don't believe this existing hook works for
>> remote filesystems.

I am trying to understand things.

- I believe the beginning of any file write should invalidate the
  validity of the files cache.  IMA does something like that by looking
  at i_writecount.

- As I read the code ima_file_free triggers an update of the ima xattr
  and that update needs to wait until the file is quiescent.  AKA no
  more writers.  I am not certain how you get that in a remote
  filesystem.

  If the write is not coming from the local kernel I don't see how it
  makes any sense for IMA to actually update the xattr on write.

Eric

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

> On Mar 14, 2018, at 3:46 PM, ebiederm@xmission.com wrote:
> 
> Chuck Lever <chuck.lever@oracle.com> writes:
> 
>>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> 
>>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>> 
>>>>> 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
>>>> 
>>>> Which goes to why it is a flag that get's set.
>>>> 
>>>> All of this just needs a follow-up patch to update every filesystem
>>>> that does not meet ima's requirements.
>>> 
>>> Currently files on remote filesystems are measured/appraised/audited
>>> once.  With the new flags, our options would be to either fail the
>>> signature verification or constantly re-measure/re-appraise files on
>>> remote file systems.  Neither option seems like the right solution.
> 
> They are measured/appraised/audited once, and you can not trust that at
> all because you don't know when the files are going to be different.
> 
> So either keeping the filesystem out of the ima game or failing sounds
> like the right answer to me.  At least until you can get better
> information from the filesystem.
> 
>> Being new to this game, I may be making a bad assumption, but I thought
>> that the (NFSv4) change attribute can be used to detect remote mutations
>> to a file's content or metadata, so that the appraisal could be cached
>> as long as that attribute does not change. At least for NFSv4, clients
>> assume their data cache is valid while the change attribute remains the
>> same.
>> 
>> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
>> report any other clients that want to update a file. This is an NFSv4
>> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
>> cache file data quite aggressively, and it could also be used to
>> preserve or cache audit state on remote files.
> 
> The file data invalid message, plus trusting the server, is what
> would be needed for reliably caching the validity of the file.

What establishes client trust in the server? I'm probably missing
something.

The NFS protocol can convey the contents of the file, it's attributes,
and the contents of the security.ima and security.evm xattrs. The xattrs
contain cryptographically signed integrity metadata, which I presume
cannot be altered undetectably either at rest or in transit. The client
has everything it needs to measure that file, doesn't it, as long as it
has the correct set of keys to verify the signatures?

Likely I am naive, but it seems to me a file server does not have to
participate in this process, other than to store and return IMA xattrs
along with the file content. All participating clients would need to
carry the same set of keys, however.

Please tell me if I'm hijacking the thread.


>>> There's some very initial discussions on how to support file integrity
>>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>>> on the fs-verity work being done.  From a very, very high level, IMA-
>>> appraisal would verify the file signature, but leave the integrity
>>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>>> proposal with IMA, measurements would be included in the measurement
>>> list and keys used for file signature verification would use the same
>>> existing IMA-appraisal infrastructure.
>> 
>>>> Mimi I believe you said that the requirement is that all file changes
>>>> can be detected through the final __fput of a file that calls
>>>> ima_file_free.
>>> 
>>> Right, like for fuse, I don't believe this existing hook works for
>>> remote filesystems.
> 
> I am trying to understand things.
> 
> - I believe the beginning of any file write should invalidate the
>  validity of the files cache.  IMA does something like that by looking
>  at i_writecount.
> 
> - As I read the code ima_file_free triggers an update of the ima xattr
>  and that update needs to wait until the file is quiescent.  AKA no
>  more writers.  I am not certain how you get that in a remote
>  filesystem.

With NFSv4, a read delegation is sufficient to guarantee that the
client is the only writer. The mechanism varies (or can be absent)
for other remote filesystem protocols. And, an NFSv4 server is not
obligated to always provide a delegation.

An NFSv4 client can also OPEN a file with share deny modes. That
would prevent other clients from accessing the file while the IMA
metadata was recomputed. Again, I believe something similar would
work for SMB3, but might not be applicable to other remote file-
system protocols (eg NFSv3 does not have all this magic).

However, computing a fresh IMA xattr would require access to the
whole file. For a large file, a client would have to read it from
the file server in its entirety, unless the file server offloads
this computation from the client somehow. The server would need to
wait until that client had CLOSEd the file to ensure that the
client had no more cached dirty data, and at that point the server
can itself guarantee there are no other remote accessors.


>  If the write is not coming from the local kernel I don't see how it
>  makes any sense for IMA to actually update the xattr on write.
> 
> Eric

--
Chuck Lever

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

> On Mar 14, 2018, at 3:46 PM, ebiederm at xmission.com wrote:
> 
> Chuck Lever <chuck.lever@oracle.com> writes:
> 
>>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>> 
>>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>> 
>>>>> 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
>>>> 
>>>> Which goes to why it is a flag that get's set.
>>>> 
>>>> All of this just needs a follow-up patch to update every filesystem
>>>> that does not meet ima's requirements.
>>> 
>>> Currently files on remote filesystems are measured/appraised/audited
>>> once.  With the new flags, our options would be to either fail the
>>> signature verification or constantly re-measure/re-appraise files on
>>> remote file systems.  Neither option seems like the right solution.
> 
> They are measured/appraised/audited once, and you can not trust that at
> all because you don't know when the files are going to be different.
> 
> So either keeping the filesystem out of the ima game or failing sounds
> like the right answer to me.  At least until you can get better
> information from the filesystem.
> 
>> Being new to this game, I may be making a bad assumption, but I thought
>> that the (NFSv4) change attribute can be used to detect remote mutations
>> to a file's content or metadata, so that the appraisal could be cached
>> as long as that attribute does not change. At least for NFSv4, clients
>> assume their data cache is valid while the change attribute remains the
>> same.
>> 
>> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
>> report any other clients that want to update a file. This is an NFSv4
>> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
>> cache file data quite aggressively, and it could also be used to
>> preserve or cache audit state on remote files.
> 
> The file data invalid message, plus trusting the server, is what
> would be needed for reliably caching the validity of the file.

What establishes client trust in the server? I'm probably missing
something.

The NFS protocol can convey the contents of the file, it's attributes,
and the contents of the security.ima and security.evm xattrs. The xattrs
contain cryptographically signed integrity metadata, which I presume
cannot be altered undetectably either at rest or in transit. The client
has everything it needs to measure that file, doesn't it, as long as it
has the correct set of keys to verify the signatures?

Likely I am naive, but it seems to me a file server does not have to
participate in this process, other than to store and return IMA xattrs
along with the file content. All participating clients would need to
carry the same set of keys, however.

Please tell me if I'm hijacking the thread.


>>> There's some very initial discussions on how to support file integrity
>>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>>> on the fs-verity work being done.  From a very, very high level, IMA-
>>> appraisal would verify the file signature, but leave the integrity
>>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>>> proposal with IMA, measurements would be included in the measurement
>>> list and keys used for file signature verification would use the same
>>> existing IMA-appraisal infrastructure.
>> 
>>>> Mimi I believe you said that the requirement is that all file changes
>>>> can be detected through the final __fput of a file that calls
>>>> ima_file_free.
>>> 
>>> Right, like for fuse, I don't believe this existing hook works for
>>> remote filesystems.
> 
> I am trying to understand things.
> 
> - I believe the beginning of any file write should invalidate the
>  validity of the files cache.  IMA does something like that by looking
>  at i_writecount.
> 
> - As I read the code ima_file_free triggers an update of the ima xattr
>  and that update needs to wait until the file is quiescent.  AKA no
>  more writers.  I am not certain how you get that in a remote
>  filesystem.

With NFSv4, a read delegation is sufficient to guarantee that the
client is the only writer. The mechanism varies (or can be absent)
for other remote filesystem protocols. And, an NFSv4 server is not
obligated to always provide a delegation.

An NFSv4 client can also OPEN a file with share deny modes. That
would prevent other clients from accessing the file while the IMA
metadata was recomputed. Again, I believe something similar would
work for SMB3, but might not be applicable to other remote file-
system protocols (eg NFSv3 does not have all this magic).

However, computing a fresh IMA xattr would require access to the
whole file. For a large file, a client would have to read it from
the file server in its entirety, unless the file server offloads
this computation from the client somehow. The server would need to
wait until that client had CLOSEd the file to ensure that the
client had no more cached dirty data, and at that point the server
can itself guarantee there are no other remote accessors.


>  If the write is not coming from the local kernel I don't see how it
>  makes any sense for IMA to actually update the xattr on write.
> 
> Eric

--
Chuck Lever



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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Chuck Lever <chuck.lever@oracle.com> writes:

>> On Mar 14, 2018, at 3:46 PM, ebiederm@xmission.com wrote:
>> 
>> Chuck Lever <chuck.lever@oracle.com> writes:
>> 
>>>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>>> 
>>>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>>> 
>>>>>> 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
>>>>> 
>>>>> Which goes to why it is a flag that get's set.
>>>>> 
>>>>> All of this just needs a follow-up patch to update every filesystem
>>>>> that does not meet ima's requirements.
>>>> 
>>>> Currently files on remote filesystems are measured/appraised/audited
>>>> once.  With the new flags, our options would be to either fail the
>>>> signature verification or constantly re-measure/re-appraise files on
>>>> remote file systems.  Neither option seems like the right solution.
>> 
>> They are measured/appraised/audited once, and you can not trust that at
>> all because you don't know when the files are going to be different.
>> 
>> So either keeping the filesystem out of the ima game or failing sounds
>> like the right answer to me.  At least until you can get better
>> information from the filesystem.
>> 
>>> Being new to this game, I may be making a bad assumption, but I thought
>>> that the (NFSv4) change attribute can be used to detect remote mutations
>>> to a file's content or metadata, so that the appraisal could be cached
>>> as long as that attribute does not change. At least for NFSv4, clients
>>> assume their data cache is valid while the change attribute remains the
>>> same.
>>> 
>>> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
>>> report any other clients that want to update a file. This is an NFSv4
>>> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
>>> cache file data quite aggressively, and it could also be used to
>>> preserve or cache audit state on remote files.
>> 
>> The file data invalid message, plus trusting the server, is what
>> would be needed for reliably caching the validity of the file.
>
> What establishes client trust in the server? I'm probably missing
> something.

In this case I mean trust as in the believe that the server is not
actively trying to subvert the guarantees that IMA is depending upon.

One such guarantee is that if data is dropped from the page cache and
reread it will be the same data (unless the server let's you know).
AKA IMA needs to trust that the cache coherency protocol is implemented
honestly.

Except for the case of something like fusermount that is the standard
assumption today.  That the filesystem is not actively trying to trip
up the kernel.

> The NFS protocol can convey the contents of the file, it's attributes,
> and the contents of the security.ima and security.evm xattrs. The xattrs
> contain cryptographically signed integrity metadata, which I presume
> cannot be altered undetectably either at rest or in transit. The client
> has everything it needs to measure that file, doesn't it, as long as it
> has the correct set of keys to verify the signatures?
>
> Likely I am naive, but it seems to me a file server does not have to
> participate in this process, other than to store and return IMA xattrs
> along with the file content. All participating clients would need to
> carry the same set of keys, however.
>
> Please tell me if I'm hijacking the thread.

Unless something brings us to non-consensus about the patches to merge
we are good.  I think this is an area that need some discussion.

The big big thing right now, as I understand it, is these mechanisms that
nfs uses to keep the cache in sync are not clearly reflected in the vfs
in a way that ima can take advantage of them.


Please note I am stretching what we can do with the vfs in the kernel,
but working on unprivileged fuse mounts.  This has me asking all are
our kernel mechanisms ok if the server is actively hostile.  What
happens if the server on the first read returns an innocuous file that
matches it's ima xattr, but on the next read of the file returns an evil
trojan horse.  Or what if the server implements a cache coherency
protocol but lies and does not report all of the changes to a file.

Which is what got this conversation started in the first place
discovering that unprivileged fuse+xattrs is not something people have
looked at closely.

A side effect of the conversation is realizing that remote filesystems
have many of the same issues, but we can trust that the remote
filesystems are not actively hostile.  They just happen to not maintain
all of the same invariants as local filesystems (like all modifications
go through the local kernel).  Which leads to the implication that we
need some of these mechanisms on filesystems like nfs as well.

>>>> There's some very initial discussions on how to support file integrity
>>>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>>>> on the fs-verity work being done.  From a very, very high level, IMA-
>>>> appraisal would verify the file signature, but leave the integrity
>>>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>>>> proposal with IMA, measurements would be included in the measurement
>>>> list and keys used for file signature verification would use the same
>>>> existing IMA-appraisal infrastructure.
>>> 
>>>>> Mimi I believe you said that the requirement is that all file changes
>>>>> can be detected through the final __fput of a file that calls
>>>>> ima_file_free.
>>>> 
>>>> Right, like for fuse, I don't believe this existing hook works for
>>>> remote filesystems.
>> 
>> I am trying to understand things.
>> 
>> - I believe the beginning of any file write should invalidate the
>>  validity of the files cache.  IMA does something like that by looking
>>  at i_writecount.
>> 
>> - As I read the code ima_file_free triggers an update of the ima xattr
>>  and that update needs to wait until the file is quiescent.  AKA no
>>  more writers.  I am not certain how you get that in a remote
>>  filesystem.
>
> With NFSv4, a read delegation is sufficient to guarantee that the
> client is the only writer. The mechanism varies (or can be absent)
> for other remote filesystem protocols. And, an NFSv4 server is not
> obligated to always provide a delegation.
>
> An NFSv4 client can also OPEN a file with share deny modes. That
> would prevent other clients from accessing the file while the IMA
> metadata was recomputed. Again, I believe something similar would
> work for SMB3, but might not be applicable to other remote file-
> system protocols (eg NFSv3 does not have all this magic).
>
> However, computing a fresh IMA xattr would require access to the
> whole file. For a large file, a client would have to read it from
> the file server in its entirety, unless the file server offloads
> this computation from the client somehow. The server would need to
> wait until that client had CLOSEd the file to ensure that the
> client had no more cached dirty data, and at that point the server
> can itself guarantee there are no other remote accessors.

Which sounds to me like all of this is implementable if desired,
but that ima is not currently tied into these mechanisms.  Which
I expect is the next step.

Eric

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Eric W. Biederman]

Chuck Lever <chuck.lever@oracle.com> writes:

>> On Mar 14, 2018, at 3:46 PM, ebiederm at xmission.com wrote:
>> 
>> Chuck Lever <chuck.lever@oracle.com> writes:
>> 
>>>> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
>>>> 
>>>> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
>>>>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>>>>> 
>>>>>> 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
>>>>> 
>>>>> Which goes to why it is a flag that get's set.
>>>>> 
>>>>> All of this just needs a follow-up patch to update every filesystem
>>>>> that does not meet ima's requirements.
>>>> 
>>>> Currently files on remote filesystems are measured/appraised/audited
>>>> once.  With the new flags, our options would be to either fail the
>>>> signature verification or constantly re-measure/re-appraise files on
>>>> remote file systems.  Neither option seems like the right solution.
>> 
>> They are measured/appraised/audited once, and you can not trust that at
>> all because you don't know when the files are going to be different.
>> 
>> So either keeping the filesystem out of the ima game or failing sounds
>> like the right answer to me.  At least until you can get better
>> information from the filesystem.
>> 
>>> Being new to this game, I may be making a bad assumption, but I thought
>>> that the (NFSv4) change attribute can be used to detect remote mutations
>>> to a file's content or metadata, so that the appraisal could be cached
>>> as long as that attribute does not change. At least for NFSv4, clients
>>> assume their data cache is valid while the change attribute remains the
>>> same.
>>> 
>>> NFSv4 (and SMB) also has a mechanism where a server guarantees it will
>>> report any other clients that want to update a file. This is an NFSv4
>>> read delegation or an SMB oplock. NFSv4 clients use this mechanism to
>>> cache file data quite aggressively, and it could also be used to
>>> preserve or cache audit state on remote files.
>> 
>> The file data invalid message, plus trusting the server, is what
>> would be needed for reliably caching the validity of the file.
>
> What establishes client trust in the server? I'm probably missing
> something.

In this case I mean trust as in the believe that the server is not
actively trying to subvert the guarantees that IMA is depending upon.

One such guarantee is that if data is dropped from the page cache and
reread it will be the same data (unless the server let's you know).
AKA IMA needs to trust that the cache coherency protocol is implemented
honestly.

Except for the case of something like fusermount that is the standard
assumption today.  That the filesystem is not actively trying to trip
up the kernel.

> The NFS protocol can convey the contents of the file, it's attributes,
> and the contents of the security.ima and security.evm xattrs. The xattrs
> contain cryptographically signed integrity metadata, which I presume
> cannot be altered undetectably either at rest or in transit. The client
> has everything it needs to measure that file, doesn't it, as long as it
> has the correct set of keys to verify the signatures?
>
> Likely I am naive, but it seems to me a file server does not have to
> participate in this process, other than to store and return IMA xattrs
> along with the file content. All participating clients would need to
> carry the same set of keys, however.
>
> Please tell me if I'm hijacking the thread.

Unless something brings us to non-consensus about the patches to merge
we are good.  I think this is an area that need some discussion.

The big big thing right now, as I understand it, is these mechanisms that
nfs uses to keep the cache in sync are not clearly reflected in the vfs
in a way that ima can take advantage of them.


Please note I am stretching what we can do with the vfs in the kernel,
but working on unprivileged fuse mounts.  This has me asking all are
our kernel mechanisms ok if the server is actively hostile.  What
happens if the server on the first read returns an innocuous file that
matches it's ima xattr, but on the next read of the file returns an evil
trojan horse.  Or what if the server implements a cache coherency
protocol but lies and does not report all of the changes to a file.

Which is what got this conversation started in the first place
discovering that unprivileged fuse+xattrs is not something people have
looked at closely.

A side effect of the conversation is realizing that remote filesystems
have many of the same issues, but we can trust that the remote
filesystems are not actively hostile.  They just happen to not maintain
all of the same invariants as local filesystems (like all modifications
go through the local kernel).  Which leads to the implication that we
need some of these mechanisms on filesystems like nfs as well.

>>>> There's some very initial discussions on how to support file integrity
>>>> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
>>>> on the fs-verity work being done.  From a very, very high level, IMA-
>>>> appraisal would verify the file signature, but leave the integrity
>>>> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
>>>> proposal with IMA, measurements would be included in the measurement
>>>> list and keys used for file signature verification would use the same
>>>> existing IMA-appraisal infrastructure.
>>> 
>>>>> Mimi I believe you said that the requirement is that all file changes
>>>>> can be detected through the final __fput of a file that calls
>>>>> ima_file_free.
>>>> 
>>>> Right, like for fuse, I don't believe this existing hook works for
>>>> remote filesystems.
>> 
>> I am trying to understand things.
>> 
>> - I believe the beginning of any file write should invalidate the
>>  validity of the files cache.  IMA does something like that by looking
>>  at i_writecount.
>> 
>> - As I read the code ima_file_free triggers an update of the ima xattr
>>  and that update needs to wait until the file is quiescent.  AKA no
>>  more writers.  I am not certain how you get that in a remote
>>  filesystem.
>
> With NFSv4, a read delegation is sufficient to guarantee that the
> client is the only writer. The mechanism varies (or can be absent)
> for other remote filesystem protocols. And, an NFSv4 server is not
> obligated to always provide a delegation.
>
> An NFSv4 client can also OPEN a file with share deny modes. That
> would prevent other clients from accessing the file while the IMA
> metadata was recomputed. Again, I believe something similar would
> work for SMB3, but might not be applicable to other remote file-
> system protocols (eg NFSv3 does not have all this magic).
>
> However, computing a fresh IMA xattr would require access to the
> whole file. For a large file, a client would have to read it from
> the file server in its entirety, unless the file server offloads
> this computation from the client somehow. The server would need to
> wait until that client had CLOSEd the file to ensure that the
> client had no more cached dirty data, and at that point the server
> can itself guarantee there are no other remote accessors.

Which sounds to me like all of this is implementable if desired,
but that ima is not currently tied into these mechanisms.  Which
I expect is the next step.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Michael Halcrow]

On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote:
> In this case I mean trust as in the believe that the server is not
> actively trying to subvert the guarantees that IMA is depending
> upon.

The fs-verity adversarial model we're targeting includes a malicious
layer backing the local file system inode's page cache.

For example, the malicious layer can be a drive controller with
trojaned firmware or a compromised remote server.

> What happens if the server on the first read returns an innocuous
> file that matches it's ima xattr, but on the next read of the file
> returns an evil trojan horse.  Or what if the server implements a
> cache coherency protocol but lies and does not report all of the
> changes to a file.

fs-verity validates each page in the inode's page cache against the
inode's root-of-trust every time the page is read into the cache.

The signature mechanism on the root-of-trust must bind the inode
identity.

The initial version of fs-verity supports Secure Boot, and the TCB
validates each inode identity in the mount during the boot process.
fs-verity does this via an ioctl that provisions the fs-verity
measurement, causing the inode (along with its immutable measurement)
to be pinned by the superblock for the life of the mount.  This meets
requirements for the Android platform's use case, which only needs to
cover about 15 APKs during boot.

This doesn't scale.  Future work will include integration with PKCS7
and/or IMA.  Signatures will be dynamically verifiable against a key
or keys in the TCB, and I'd like to establish a safe way to avoid
having to pin inodes and their measurements.

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Michael Halcrow]

On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote:
> In this case I mean trust as in the believe that the server is not
> actively trying to subvert the guarantees that IMA is depending
> upon.

The fs-verity adversarial model we're targeting includes a malicious
layer backing the local file system inode's page cache.

For example, the malicious layer can be a drive controller with
trojaned firmware or a compromised remote server.

> What happens if the server on the first read returns an innocuous
> file that matches it's ima xattr, but on the next read of the file
> returns an evil trojan horse.  Or what if the server implements a
> cache coherency protocol but lies and does not report all of the
> changes to a file.

fs-verity validates each page in the inode's page cache against the
inode's root-of-trust every time the page is read into the cache.

The signature mechanism on the root-of-trust must bind the inode
identity.

The initial version of fs-verity supports Secure Boot, and the TCB
validates each inode identity in the mount during the boot process.
fs-verity does this via an ioctl that provisions the fs-verity
measurement, causing the inode (along with its immutable measurement)
to be pinned by the superblock for the life of the mount.  This meets
requirements for the Android platform's use case, which only needs to
cover about 15 APKs during boot.

This doesn't scale.  Future work will include integration with PKCS7
and/or IMA.  Signatures will be dynamically verifiable against a key
or keys in the TCB, and I'd like to establish a safe way to avoid
having to pin inodes and their measurements.
--
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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stef Bon]

2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>:
>>
>> Please tell me if I'm hijacking the thread.
>
> Unless something brings us to non-consensus about the patches to merge
> we are good.  I think this is an area that need some discussion.
>
> The big big thing right now, as I understand it, is these mechanisms that
> nfs uses to keep the cache in sync are not clearly reflected in the vfs
> in a way that ima can take advantage of them.
>
>

Chuck you mean fschange notifications methods like
NT_TRANSACT_NOTIFY_CHANGE for cifs. I believe that NFS4 has something simular.
These mechanism will inform the client when a file in a watched
directory is changed.
This is not yet supported in Linux (these methods are not triggered
any way when setting a watch using inotify for exmple).
There was support with dnotify
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393).

But these methods are triggered by the user and not the VFS/kernel and
therefore cannot garantee that all files on the client
are the same as on the server.
This also counts for a read delegation with nfs and methods like
leases in a client server environment.

Stef

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Stef Bon]

2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>:
>>
>> Please tell me if I'm hijacking the thread.
>
> Unless something brings us to non-consensus about the patches to merge
> we are good.  I think this is an area that need some discussion.
>
> The big big thing right now, as I understand it, is these mechanisms that
> nfs uses to keep the cache in sync are not clearly reflected in the vfs
> in a way that ima can take advantage of them.
>
>

Chuck you mean fschange notifications methods like
NT_TRANSACT_NOTIFY_CHANGE for cifs. I believe that NFS4 has something simular.
These mechanism will inform the client when a file in a watched
directory is changed.
This is not yet supported in Linux (these methods are not triggered
any way when setting a watch using inotify for exmple).
There was support with dnotify
(https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393).

But these methods are triggered by the user and not the VFS/kernel and
therefore cannot garantee that all files on the client
are the same as on the server.
This also counts for a read delegation with nfs and methods like
leases in a client server environment.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

Hi Stef-

> On Mar 15, 2018, at 6:07 AM, Stef Bon <stefbon@gmail.com> wrote:
> 
> 2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>:
>>> 
>>> Please tell me if I'm hijacking the thread.
>> 
>> Unless something brings us to non-consensus about the patches to merge
>> we are good.  I think this is an area that need some discussion.
>> 
>> The big big thing right now, as I understand it, is these mechanisms that
>> nfs uses to keep the cache in sync are not clearly reflected in the vfs
>> in a way that ima can take advantage of them.
>> 
>> 
> 
> Chuck you mean fschange notifications methods like
> NT_TRANSACT_NOTIFY_CHANGE for cifs.

No, I don't mean notification. That's a different mechanism entirely.

NFSv4 delegations are similar to SMB oplocks. It's a protocol
guarantee that the server will tell the client that holds a delegation
when another client wants conflicting access to a file. The client
then has an opportunity to update the file with anything it has cached
and then the client releases the delegation. Servers have the option
of granting a delegation for a file when it is OPENed.

NFSv4 OPEN with share deny is similar to the way that SMB does OPENs.
When a user OPENs a file this way, no other user or client is allowed
to access it until the user CLOSEs the file.


> I believe that NFS4 has something simular.
> These mechanism will inform the client when a file in a watched
> directory is changed.
> This is not yet supported in Linux (these methods are not triggered
> any way when setting a watch using inotify for exmple).
> There was support with dnotify
> (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393).
> 
> But these methods are triggered by the user and not the VFS/kernel and
> therefore cannot garantee that all files on the client
> are the same as on the server.
> This also counts for a read delegation with nfs and methods like
> leases in a client server environment.
> 
> Stef

--
Chuck Lever

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Chuck Lever]

Hi Stef-

> On Mar 15, 2018, at 6:07 AM, Stef Bon <stefbon@gmail.com> wrote:
> 
> 2018-03-14 22:42 GMT+01:00 Eric W. Biederman <ebiederm@xmission.com>:
>>> 
>>> Please tell me if I'm hijacking the thread.
>> 
>> Unless something brings us to non-consensus about the patches to merge
>> we are good.  I think this is an area that need some discussion.
>> 
>> The big big thing right now, as I understand it, is these mechanisms that
>> nfs uses to keep the cache in sync are not clearly reflected in the vfs
>> in a way that ima can take advantage of them.
>> 
>> 
> 
> Chuck you mean fschange notifications methods like
> NT_TRANSACT_NOTIFY_CHANGE for cifs.

No, I don't mean notification. That's a different mechanism entirely.

NFSv4 delegations are similar to SMB oplocks. It's a protocol
guarantee that the server will tell the client that holds a delegation
when another client wants conflicting access to a file. The client
then has an opportunity to update the file with anything it has cached
and then the client releases the delegation. Servers have the option
of granting a delegation for a file when it is OPENed.

NFSv4 OPEN with share deny is similar to the way that SMB does OPENs.
When a user OPENs a file this way, no other user or client is allowed
to access it until the user CLOSEs the file.


> I believe that NFS4 has something simular.
> These mechanism will inform the client when a file in a watched
> directory is changed.
> This is not yet supported in Linux (these methods are not triggered
> any way when setting a watch using inotify for exmple).
> There was support with dnotify
> (https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/fs/cifs/cifssmb.c#n6393).
> 
> But these methods are triggered by the user and not the VFS/kernel and
> therefore cannot garantee that all files on the client
> are the same as on the server.
> This also counts for a read delegation with nfs and methods like
> leases in a client server environment.
> 
> Stef

--
Chuck Lever



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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 15:53 -0700, Michael Halcrow wrote:
> On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote:
> > In this case I mean trust as in the believe that the server is not
> > actively trying to subvert the guarantees that IMA is depending
> > upon.
> 
> The fs-verity adversarial model we're targeting includes a malicious
> layer backing the local file system inode's page cache.
> 
> For example, the malicious layer can be a drive controller with
> trojaned firmware or a compromised remote server.
> 
> > What happens if the server on the first read returns an innocuous
> > file that matches it's ima xattr, but on the next read of the file
> > returns an evil trojan horse.  Or what if the server implements a
> > cache coherency protocol but lies and does not report all of the
> > changes to a file.
> 
> fs-verity validates each page in the inode's page cache against the
> inode's root-of-trust every time the page is read into the cache.
> 
> The signature mechanism on the root-of-trust must bind the inode
> identity.
> 
> The initial version of fs-verity supports Secure Boot, and the TCB
> validates each inode identity in the mount during the boot process.
> fs-verity does this via an ioctl that provisions the fs-verity
> measurement, causing the inode (along with its immutable measurement)
> to be pinned by the superblock for the life of the mount.  This meets
> requirements for the Android platform's use case, which only needs to
> cover about 15 APKs during boot.
> 
> This doesn't scale.  Future work will include integration with PKCS7
> and/or IMA.  Signatures will be dynamically verifiable against a key
> or keys in the TCB, and I'd like to establish a safe way to avoid
> having to pin inodes and their measurements.

Next steps should also include identifying methods for storing and
transporting the Merkle tree signature without having to read the
entire file for remote filesystems.

With this fs support, signed files on file systems mounted with the
 SB_I_IMA_UNVERIFIABLE_SIGNATURE flag would then be verifiable.  

Mimi

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 15:53 -0700, Michael Halcrow wrote:
> On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote:
> > In this case I mean trust as in the believe that the server is not
> > actively trying to subvert the guarantees that IMA is depending
> > upon.
> 
> The fs-verity adversarial model we're targeting includes a malicious
> layer backing the local file system inode's page cache.
> 
> For example, the malicious layer can be a drive controller with
> trojaned firmware or a compromised remote server.
> 
> > What happens if the server on the first read returns an innocuous
> > file that matches it's ima xattr, but on the next read of the file
> > returns an evil trojan horse.  Or what if the server implements a
> > cache coherency protocol but lies and does not report all of the
> > changes to a file.
> 
> fs-verity validates each page in the inode's page cache against the
> inode's root-of-trust every time the page is read into the cache.
> 
> The signature mechanism on the root-of-trust must bind the inode
> identity.
> 
> The initial version of fs-verity supports Secure Boot, and the TCB
> validates each inode identity in the mount during the boot process.
> fs-verity does this via an ioctl that provisions the fs-verity
> measurement, causing the inode (along with its immutable measurement)
> to be pinned by the superblock for the life of the mount.  This meets
> requirements for the Android platform's use case, which only needs to
> cover about 15 APKs during boot.
> 
> This doesn't scale.  Future work will include integration with PKCS7
> and/or IMA.  Signatures will be dynamically verifiable against a key
> or keys in the TCB, and I'd like to establish a safe way to avoid
> having to pin inodes and their measurements.

Next steps should also include identifying methods for storing and
transporting the Merkle tree signature without having to read the
entire file for remote filesystems.

With this fs support, signed files on file systems mounted with the
?SB_I_IMA_UNVERIFIABLE_SIGNATURE flag would then be verifiable. ?

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 15:53 -0700, Michael Halcrow wrote:
> On Wed, Mar 14, 2018 at 04:42:51PM -0500, Eric W. Biederman wrote:
> > In this case I mean trust as in the believe that the server is not
> > actively trying to subvert the guarantees that IMA is depending
> > upon.
> 
> The fs-verity adversarial model we're targeting includes a malicious
> layer backing the local file system inode's page cache.
> 
> For example, the malicious layer can be a drive controller with
> trojaned firmware or a compromised remote server.
> 
> > What happens if the server on the first read returns an innocuous
> > file that matches it's ima xattr, but on the next read of the file
> > returns an evil trojan horse.  Or what if the server implements a
> > cache coherency protocol but lies and does not report all of the
> > changes to a file.
> 
> fs-verity validates each page in the inode's page cache against the
> inode's root-of-trust every time the page is read into the cache.
> 
> The signature mechanism on the root-of-trust must bind the inode
> identity.
> 
> The initial version of fs-verity supports Secure Boot, and the TCB
> validates each inode identity in the mount during the boot process.
> fs-verity does this via an ioctl that provisions the fs-verity
> measurement, causing the inode (along with its immutable measurement)
> to be pinned by the superblock for the life of the mount.  This meets
> requirements for the Android platform's use case, which only needs to
> cover about 15 APKs during boot.
> 
> This doesn't scale.  Future work will include integration with PKCS7
> and/or IMA.  Signatures will be dynamically verifiable against a key
> or keys in the TCB, and I'd like to establish a safe way to avoid
> having to pin inodes and their measurements.

Next steps should also include identifying methods for storing and
transporting the Merkle tree signature without having to read the
entire file for remote filesystems.

With this fs support, signed files on file systems mounted with the
 SB_I_IMA_UNVERIFIABLE_SIGNATURE flag would then be verifiable.  

Mimi

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 14:46 -0500, Eric W. Biederman wrote:
> Chuck Lever <chuck.lever@oracle.com> writes:
> 
> >> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> 
> >> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
> >>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> >>> 
> >>>> 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
> >>> 
> >>> Which goes to why it is a flag that get's set.
> >>> 
> >>> All of this just needs a follow-up patch to update every filesystem
> >>> that does not meet ima's requirements.
> >> 
> >> Currently files on remote filesystems are measured/appraised/audited
> >> once.  With the new flags, our options would be to either fail the
> >> signature verification or constantly re-measure/re-appraise files on
> >> remote file systems.  Neither option seems like the right solution.
> 
> They are measured/appraised/audited once, and you can not trust that at
> all because you don't know when the files are going to be different.
> 
> So either keeping the filesystem out of the ima game or failing sounds
> like the right answer to me.  At least until you can get better
> information from the filesystem.

Agreed

> 
> > Being new to this game, I may be making a bad assumption, but I thought
> > that the (NFSv4) change attribute can be used to detect remote mutations
> > to a file's content or metadata, so that the appraisal could be cached
> > as long as that attribute does not change. At least for NFSv4, clients
> > assume their data cache is valid while the change attribute remains the
> > same.
> >
> > NFSv4 (and SMB) also has a mechanism where a server guarantees it will
> > report any other clients that want to update a file. This is an NFSv4
> > read delegation or an SMB oplock. NFSv4 clients use this mechanism to
> > cache file data quite aggressively, and it could also be used to
> > preserve or cache audit state on remote files.
> 
> The file data invalid message, plus trusting the server, is what
> would be needed for reliably caching the validity of the file.
> 
> >> There's some very initial discussions on how to support file integrity
> >> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
> >> on the fs-verity work being done.  From a very, very high level, IMA-
> >> appraisal would verify the file signature, but leave the integrity
> >> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
> >> proposal with IMA, measurements would be included in the measurement
> >> list and keys used for file signature verification would use the same
> >> existing IMA-appraisal infrastructure.
> >
> >>> Mimi I believe you said that the requirement is that all file changes
> >>> can be detected through the final __fput of a file that calls
> >>> ima_file_free.
> >> 
> >> Right, like for fuse, I don't believe this existing hook works for
> >> remote filesystems.
> 
> I am trying to understand things.
> 
> - I believe the beginning of any file write should invalidate the
>   validity of the files cache.  IMA does something like that by looking
>   at i_writecount.

Unless we're willing to recalculate the file hash and update the
security xattr each time the file changes, invalidating the IMA cache
on the first write would result in any subsequent file opens failing.

> - As I read the code ima_file_free triggers an update of the ima xattr
>   and that update needs to wait until the file is quiescent.  AKA no
>   more writers.  I am not certain how you get that in a remote
>   filesystem.
> 
>   If the write is not coming from the local kernel I don't see how it
>   makes any sense for IMA to actually update the xattr on write.

Agreed, it doesn't.

Mimi

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

On Wed, 2018-03-14 at 14:46 -0500, Eric W. Biederman wrote:
> Chuck Lever <chuck.lever@oracle.com> writes:
> 
> >> On Mar 14, 2018, at 1:50 PM, Mimi Zohar <zohar@linux.vnet.ibm.com> wrote:
> >> 
> >> On Wed, 2018-03-14 at 11:17 -0500, Eric W. Biederman wrote:
> >>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> >>> 
> >>>> 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
> >>> 
> >>> Which goes to why it is a flag that get's set.
> >>> 
> >>> All of this just needs a follow-up patch to update every filesystem
> >>> that does not meet ima's requirements.
> >> 
> >> Currently files on remote filesystems are measured/appraised/audited
> >> once.  With the new flags, our options would be to either fail the
> >> signature verification or constantly re-measure/re-appraise files on
> >> remote file systems.  Neither option seems like the right solution.
> 
> They are measured/appraised/audited once, and you can not trust that at
> all because you don't know when the files are going to be different.
> 
> So either keeping the filesystem out of the ima game or failing sounds
> like the right answer to me.  At least until you can get better
> information from the filesystem.

Agreed

> 
> > Being new to this game, I may be making a bad assumption, but I thought
> > that the (NFSv4) change attribute can be used to detect remote mutations
> > to a file's content or metadata, so that the appraisal could be cached
> > as long as that attribute does not change. At least for NFSv4, clients
> > assume their data cache is valid while the change attribute remains the
> > same.
> >
> > NFSv4 (and SMB) also has a mechanism where a server guarantees it will
> > report any other clients that want to update a file. This is an NFSv4
> > read delegation or an SMB oplock. NFSv4 clients use this mechanism to
> > cache file data quite aggressively, and it could also be used to
> > preserve or cache audit state on remote files.
> 
> The file data invalid message, plus trusting the server, is what
> would be needed for reliably caching the validity of the file.
> 
> >> There's some very initial discussions on how to support file integrity
> >> on remote filesystems.  Chuck Lever has some thoughts on piggy-backing 
> >> on the fs-verity work being done.  From a very, very high level, IMA-
> >> appraisal would verify the file signature, but leave the integrity
> >> enforcement to the vfs/fs layer.  By integrating fs-verity or similar
> >> proposal with IMA, measurements would be included in the measurement
> >> list and keys used for file signature verification would use the same
> >> existing IMA-appraisal infrastructure.
> >
> >>> Mimi I believe you said that the requirement is that all file changes
> >>> can be detected through the final __fput of a file that calls
> >>> ima_file_free.
> >> 
> >> Right, like for fuse, I don't believe this existing hook works for
> >> remote filesystems.
> 
> I am trying to understand things.
> 
> - I believe the beginning of any file write should invalidate the
>   validity of the files cache.  IMA does something like that by looking
>   at i_writecount.

Unless we're willing to recalculate the file hash and update the
security xattr each time the file changes, invalidating the IMA cache
on the first write would result in any subsequent file opens failing.

> - As I read the code ima_file_free triggers an update of the ima xattr
>   and that update needs to wait until the file is quiescent.  AKA no
>   more writers.  I am not certain how you get that in a remote
>   filesystem.
> 
>   If the write is not coming from the local kernel I don't see how it
>   makes any sense for IMA to actually update the xattr on write.

Agreed, it doesn't.

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

Hi Miklos,

On Tue, 2018-03-13 at 14:32 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

There's been a number of changes since the original version of this
patch set.  I would appreciate your Ack for this version?

Thanks,

Mimi


> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > 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;
> 

[PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

Hi Miklos,

On Tue, 2018-03-13 at 14:32 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

There's been a number of changes since the original version of this
patch set. ?I would appreciate your Ack for this version?

Thanks,

Mimi


> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > 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;
> 

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

Re: [PATCH v3 4/4] fuse: define the filesystem as untrusted

[Mimi Zohar]

Hi Miklos,

On Tue, 2018-03-13 at 14:32 -0500, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > 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.
> 
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>

There's been a number of changes since the original version of this
patch set.  I would appreciate your Ack for this version?

Thanks,

Mimi


> >
> > Signed-off-by: Mimi Zohar <zohar@linux.vnet.ibm.com>
> > Cc: Miklos Szeredi <miklos@szeredi.hu>
> > Cc: Seth Forshee <seth.forshee@canonical.com>
> > Cc: Eric W. Biederman <ebiederm@xmission.com>
> > Cc: Dongsu Park <dongsu@kinvolk.io>
> > Cc: Alban Crequy <alban@kinvolk.io>
> > 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;
>