linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 0/2] ima: untrusted filesystems
@ 2018-02-19 15:18 Mimi Zohar
  2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
  2018-02-19 15:18 ` [PATCH v1 2/2] fuse: define the filesystem as untrusted Mimi Zohar
  0 siblings, 2 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-02-19 15:18 UTC (permalink / raw)
  To: linux-integrity; +Cc: linux-security-module, linux-fsdevel, Mimi Zohar

Based on the mailing list discussions, it is clear that separating the
non-init unpriviliged, mounted untrusted filesystem from setuid
unprivileged or privileged mounted untrusted filesystems patches was
confusing.  I've combined the patches, commenting the code with an
explanation for the differentiation.

Instad of expliciting modifying the IMA policy to fail file signature
verfication for the setuid unprivileged or privileged mounted untrusted
filesystems cases, this patch set defines a builtin IMA policy named
"untrusted-fs".  No other IMA policy changes are required.

Mimi


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.

Mimi Zohar (2):
  ima: fail signature verification on untrusted filesystems
  fuse: define the filesystem as untrusted

 Documentation/admin-guide/kernel-parameters.txt |  6 +++++-
 fs/fuse/inode.c                                 |  1 +
 include/linux/fs.h                              |  1 +
 security/integrity/ima/ima_appraise.c           | 16 +++++++++++++++-
 security/integrity/ima/ima_policy.c             |  5 +++++
 security/integrity/integrity.h                  |  1 +
 6 files changed, 28 insertions(+), 2 deletions(-)

-- 
2.7.5

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

* [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-19 15:18 [PATCH v1 0/2] ima: untrusted filesystems Mimi Zohar
@ 2018-02-19 15:18 ` Mimi Zohar
  2018-02-19 21:47   ` Eric W. Biederman
                     ` (2 more replies)
  2018-02-19 15:18 ` [PATCH v1 2/2] fuse: define the filesystem as untrusted Mimi Zohar
  1 sibling, 3 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-02-19 15:18 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, Mimi Zohar, Miklos Szeredi,
	Seth Forshee, Eric W . Biederman, Dongsu Park, Alban Crequy,
	Serge E . Hallyn

Files on untrusted filesystems, such as fuse, can change at any time,
making the measurement(s) and by extension signature verification
meaningless.

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 differentiates between the new unprivileged non-init mounted
filesystems and everything else, by always failing file signature
verification on unprivileged non-init mounted untrusted filesystems, but
only failing everything else based on policy to avoid breaking existing
systems.

This patch defines a new sb->s_iflags option named SB_I_IMA_UNTRUSTED_FS
and a new builtin IMA policy named "untrusted_fs".

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

 Documentation/admin-guide/kernel-parameters.txt |  6 +++++-
 include/linux/fs.h                              |  1 +
 security/integrity/ima/ima_appraise.c           | 16 +++++++++++++++-
 security/integrity/ima/ima_policy.c             |  5 +++++
 security/integrity/integrity.h                  |  1 +
 5 files changed, 27 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index 90cefbddf1ed..f9eb24cea9a6 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -1522,7 +1522,7 @@
 
 	ima_policy=	[IMA]
 			The builtin policies to load during IMA setup.
-			Format: "tcb | appraise_tcb | secure_boot"
+			Format: "tcb | appraise_tcb | secure_boot | untrusted_fs"
 
 			The "tcb" policy measures all programs exec'd, files
 			mmap'd for exec, and all files opened with the read
@@ -1537,6 +1537,10 @@
 			of files (eg. kexec kernel image, kernel modules,
 			firmware, policy, etc) based on file signatures.
 
+			The "untrusted_fs" policy fails the file signature
+			verification on privileged mounted untrusted
+			filesystems.
+
 	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/include/linux/fs.h b/include/linux/fs.h
index 2a815560fda0..1d3fe0fe49ee 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1320,6 +1320,7 @@ 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_UNTRUSTED_FS	0x00000020 /* Kernel unaware of fs changes */
 
 /* Possible states of 'frozen' field */
 enum {
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index f2803a40ff82..ebfeec9b579f 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -292,7 +292,20 @@ int ima_appraise_measurement(enum ima_hooks func,
 	}
 
 out:
-	if (status != INTEGRITY_PASS) {
+	/*
+	 * Files on both privileged and unprivileged mounted untrusted
+	 * filesystems (eg. FUSE) should fail signature verification, but
+	 * this might break existing systems.  Differentiate between the
+	 * new unprivileged non-init mounted filesystems and everything else.
+	 */
+	if ((inode->i_sb->s_iflags & SB_I_IMA_UNTRUSTED_FS) &&
+	    ((inode->i_sb->s_user_ns != &init_user_ns) ||
+	     (iint->flags & IMA_FAIL_UNTRUSTED_FS))) {
+		status = INTEGRITY_FAIL;
+		cause = "untrusted-filesystem";
+		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)) {
@@ -309,6 +322,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 	} else {
 		ima_cache_flags(iint, func);
 	}
+
 	ima_set_cache_status(iint, func, status);
 	return status;
 }
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 915f5572c6ff..43fb05b9686d 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_untrusted_fs __initdata;
 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, "untrusted_fs") == 0)
+			ima_fail_untrusted_fs = true;
 	}
 
 	return 1;
@@ -385,6 +388,8 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
 		if (entry->action & IMA_APPRAISE) {
 			action |= get_subaction(entry, func);
 			action ^= IMA_HASH;
+			if (ima_fail_untrusted_fs)
+				action |= IMA_FAIL_UNTRUSTED_FS;
 		}
 
 		if (entry->action & IMA_DO_MASK)
diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
index 50a8e3365df7..f8fa60f560a6 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_UNTRUSTED_FS	0x10000000
 
 #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
 				 IMA_HASH | IMA_APPRAISE_SUBMASK)
-- 
2.7.5

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

* [PATCH v1 2/2] fuse: define the filesystem as untrusted
  2018-02-19 15:18 [PATCH v1 0/2] ima: untrusted filesystems Mimi Zohar
  2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
@ 2018-02-19 15:18 ` Mimi Zohar
  1 sibling, 0 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-02-19 15:18 UTC (permalink / raw)
  To: linux-integrity
  Cc: linux-security-module, linux-fsdevel, Mimi Zohar, Miklos Szeredi,
	Seth Forshee, Eric W . Biederman, Dongsu Park, Alban Crequy,
	Serge E. Hallyn

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_UNTRUSTED_FS 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 | 1 +
 1 file changed, 1 insertion(+)

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 624f18bbfd2b..2cbf01fc85a2 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1080,6 +1080,7 @@ 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_UNTRUSTED_FS;
 
 	file = fget(d.fd);
 	err = -EINVAL;
-- 
2.7.5

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
@ 2018-02-19 21:47   ` Eric W. Biederman
  2018-02-20  0:52     ` James Morris
  2018-02-19 22:50   ` kbuild test robot
  2018-02-19 23:36   ` kbuild test robot
  2 siblings, 1 reply; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-19 21:47 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: linux-integrity, linux-security-module, linux-fsdevel,
	Miklos Szeredi, Seth Forshee, Dongsu Park, Alban Crequy,
	Serge E . Hallyn

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

> Files on untrusted filesystems, such as fuse, can change at any time,
> making the measurement(s) and by extension signature verification
> meaningless.

Filesystems with servers?
Remote filesystems?
Perhaps unexpected changes.

Untrusted sounds a bit harsh, and I am not certain it quite captures
what you are looking to avoid.

Does proc count?
Does sysfs count?
Does nfs count?

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

There is one other way to mount a fuse filesystem and I feel it should
be considered int he mix.  Mounting a filesystem as root.  Where you
trust the server as much as you would trust an in kernel filesystem.  So
if the fuse protocol provides enough information it will be as safe to
trust as any other filesystem.

> This patch differentiates between the new unprivileged non-init mounted
> filesystems and everything else, by always failing file signature
> verification on unprivileged non-init mounted untrusted filesystems, but
> only failing everything else based on policy to avoid breaking existing
> systems.

Why the differentiation?

>From the previous conversation it feels like this is a differentiation
just because there is a chance of breaking existing users.

At the same time my understanding from the earlier conversation is that
you have no reason to suspect those users actually exist.

So I ask again.  Why differentiate?  Why have the complication?

There are two cases I can see for not trusting a filesystem here:

a) The protocol (like at least early nfs) does not provide enough
   information to know when the file changed on the server.  So caching
   an ima result can not be reliable.

b) The server is potentially hostile/malicious and may speak the
   protocol improperly in order to get around ima signatures (assuming
   the protocol is good enough).

Right now I am concerned the the patches are conflating those two cases.

Especially for cases where fuse is implementing a local filesystem that
root trusts (say ntfs-3g), and the mount is made by root (not
fusermount), I don't see an obvious reason for ima not to trust such
a filesystem.


> This patch defines a new sb->s_iflags option named SB_I_IMA_UNTRUSTED_FS
> and a new builtin IMA policy named "untrusted_fs".

This patch does make it clear what you are implementing but I am not yet
clear on the reasons for the various pieces.

Eric

>
> 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 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.
>
>  Documentation/admin-guide/kernel-parameters.txt |  6 +++++-
>  include/linux/fs.h                              |  1 +
>  security/integrity/ima/ima_appraise.c           | 16 +++++++++++++++-
>  security/integrity/ima/ima_policy.c             |  5 +++++
>  security/integrity/integrity.h                  |  1 +
>  5 files changed, 27 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index 90cefbddf1ed..f9eb24cea9a6 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -1522,7 +1522,7 @@
>  
>  	ima_policy=	[IMA]
>  			The builtin policies to load during IMA setup.
> -			Format: "tcb | appraise_tcb | secure_boot"
> +			Format: "tcb | appraise_tcb | secure_boot | untrusted_fs"
>  
>  			The "tcb" policy measures all programs exec'd, files
>  			mmap'd for exec, and all files opened with the read
> @@ -1537,6 +1537,10 @@
>  			of files (eg. kexec kernel image, kernel modules,
>  			firmware, policy, etc) based on file signatures.
>  
> +			The "untrusted_fs" policy fails the file signature
> +			verification on privileged mounted untrusted
> +			filesystems.
> +
>  	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/include/linux/fs.h b/include/linux/fs.h
> index 2a815560fda0..1d3fe0fe49ee 100644
> --- a/include/linux/fs.h
> +++ b/include/linux/fs.h
> @@ -1320,6 +1320,7 @@ 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_UNTRUSTED_FS	0x00000020 /* Kernel unaware of fs changes */
>  
>  /* Possible states of 'frozen' field */
>  enum {
> diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
> index f2803a40ff82..ebfeec9b579f 100644
> --- a/security/integrity/ima/ima_appraise.c
> +++ b/security/integrity/ima/ima_appraise.c
> @@ -292,7 +292,20 @@ int ima_appraise_measurement(enum ima_hooks func,
>  	}
>  
>  out:
> -	if (status != INTEGRITY_PASS) {
> +	/*
> +	 * Files on both privileged and unprivileged mounted untrusted
> +	 * filesystems (eg. FUSE) should fail signature verification, but
> +	 * this might break existing systems.  Differentiate between the
> +	 * new unprivileged non-init mounted filesystems and everything else.
> +	 */
> +	if ((inode->i_sb->s_iflags & SB_I_IMA_UNTRUSTED_FS) &&
> +	    ((inode->i_sb->s_user_ns != &init_user_ns) ||
> +	     (iint->flags & IMA_FAIL_UNTRUSTED_FS))) {
> +		status = INTEGRITY_FAIL;
> +		cause = "untrusted-filesystem";
> +		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)) {
> @@ -309,6 +322,7 @@ int ima_appraise_measurement(enum ima_hooks func,
>  	} else {
>  		ima_cache_flags(iint, func);
>  	}
> +
>  	ima_set_cache_status(iint, func, status);
>  	return status;
>  }
> diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
> index 915f5572c6ff..43fb05b9686d 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_untrusted_fs __initdata;
>  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, "untrusted_fs") == 0)
> +			ima_fail_untrusted_fs = true;
>  	}
>  
>  	return 1;
> @@ -385,6 +388,8 @@ int ima_match_policy(struct inode *inode, enum ima_hooks func, int mask,
>  		if (entry->action & IMA_APPRAISE) {
>  			action |= get_subaction(entry, func);
>  			action ^= IMA_HASH;
> +			if (ima_fail_untrusted_fs)
> +				action |= IMA_FAIL_UNTRUSTED_FS;
>  		}
>  
>  		if (entry->action & IMA_DO_MASK)
> diff --git a/security/integrity/integrity.h b/security/integrity/integrity.h
> index 50a8e3365df7..f8fa60f560a6 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_UNTRUSTED_FS	0x10000000
>  
>  #define IMA_DO_MASK		(IMA_MEASURE | IMA_APPRAISE | IMA_AUDIT | \
>  				 IMA_HASH | IMA_APPRAISE_SUBMASK)

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
  2018-02-19 21:47   ` Eric W. Biederman
@ 2018-02-19 22:50   ` kbuild test robot
  2018-02-19 23:36   ` kbuild test robot
  2 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2018-02-19 22:50 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kbuild-all, linux-integrity, linux-security-module,
	linux-fsdevel, Mimi Zohar, Miklos Szeredi, Seth Forshee,
	Eric W . Biederman, Dongsu Park, Alban Crequy, Serge E . Hallyn

[-- Attachment #1: Type: text/plain, Size: 1167 bytes --]

Hi Mimi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc2 next-20180219]
[cannot apply to integrity/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-untrusted-filesystems/20180220-040517
config: i386-randconfig-s1-201807 (attached as .config)
compiler: gcc-6 (Debian 6.4.0-9) 6.4.0 20171026
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x2afe73): Section mismatch in reference from the function ima_match_policy() to the variable .init.data:ima_fail_untrusted_fs
   The function ima_match_policy() references
   the variable __initdata ima_fail_untrusted_fs.
   This is often because ima_match_policy lacks a __initdata
   annotation or the annotation of ima_fail_untrusted_fs is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 25307 bytes --]

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
  2018-02-19 21:47   ` Eric W. Biederman
  2018-02-19 22:50   ` kbuild test robot
@ 2018-02-19 23:36   ` kbuild test robot
  2 siblings, 0 replies; 18+ messages in thread
From: kbuild test robot @ 2018-02-19 23:36 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: kbuild-all, linux-integrity, linux-security-module,
	linux-fsdevel, Mimi Zohar, Miklos Szeredi, Seth Forshee,
	Eric W . Biederman, Dongsu Park, Alban Crequy, Serge E . Hallyn

[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]

Hi Mimi,

I love your patch! Perhaps something to improve:

[auto build test WARNING on linus/master]
[also build test WARNING on v4.16-rc2 next-20180219]
[cannot apply to integrity/next]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Mimi-Zohar/ima-untrusted-filesystems/20180220-040517
config: x86_64-rhel (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=x86_64 

All warnings (new ones prefixed by >>):

>> WARNING: vmlinux.o(.text+0x3b320a): Section mismatch in reference from the function ima_match_policy() to the variable .init.data:init_keyring
   The function ima_match_policy() references
   the variable __initdata init_keyring.
   This is often because ima_match_policy lacks a __initdata
   annotation or the annotation of init_keyring is wrong.

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 40638 bytes --]

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-19 21:47   ` Eric W. Biederman
@ 2018-02-20  0:52     ` James Morris
  2018-02-20  2:02       ` Eric W. Biederman
  0 siblings, 1 reply; 18+ messages in thread
From: James Morris @ 2018-02-20  0:52 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Mimi Zohar, linux-integrity, linux-security-module,
	linux-fsdevel, Miklos Szeredi, Seth Forshee, Dongsu Park,
	Alban Crequy, Serge E . Hallyn

On Mon, 19 Feb 2018, Eric W. Biederman wrote:

> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > Files on untrusted filesystems, such as fuse, can change at any time,
> > making the measurement(s) and by extension signature verification
> > meaningless.
> 
> Filesystems with servers?
> Remote filesystems?
> Perhaps unexpected changes.
> 
> Untrusted sounds a bit harsh, and I am not certain it quite captures
> what you are looking to avoid.

Right -- I think whether you trust a filesystem or not depends on how much 
assurance you have in your specific configuration, rather than whether you 
think the filesystem can be manipulated or not.

There is a difference between:

  - This fs has no way to communicate a change to IMA, and;

  - This fs could be malicious.

In the latter case, I suggest that any fs could be malicious if the 
overall security policy / settings are inadequate for the threat model, or 
if there are vulnerabilities which allow such security to be bypassed.

Whether a user trusts FUSE on their particular system should be a policy 
decision on the part of the user.  The kernel should not be deciding what 
is trusted or not trusted here.



-- 
James Morris
<jmorris@namei.org>

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-20  0:52     ` James Morris
@ 2018-02-20  2:02       ` Eric W. Biederman
  2018-02-20 14:02         ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-20  2:02 UTC (permalink / raw)
  To: James Morris
  Cc: Mimi Zohar, linux-integrity, linux-security-module,
	linux-fsdevel, Miklos Szeredi, Seth Forshee, Dongsu Park,
	Alban Crequy, Serge E . Hallyn

James Morris <jmorris@namei.org> writes:

> On Mon, 19 Feb 2018, Eric W. Biederman wrote:
>
>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>> 
>> > Files on untrusted filesystems, such as fuse, can change at any time,
>> > making the measurement(s) and by extension signature verification
>> > meaningless.
>> 
>> Filesystems with servers?
>> Remote filesystems?
>> Perhaps unexpected changes.
>> 
>> Untrusted sounds a bit harsh, and I am not certain it quite captures
>> what you are looking to avoid.
>
> Right -- I think whether you trust a filesystem or not depends on how much 
> assurance you have in your specific configuration, rather than whether you 
> think the filesystem can be manipulated or not.
>
> There is a difference between:
>
>   - This fs has no way to communicate a change to IMA, and;
>
>   - This fs could be malicious.
>
> In the latter case, I suggest that any fs could be malicious if the 
> overall security policy / settings are inadequate for the threat model, or 
> if there are vulnerabilities which allow such security to be bypassed.
>
> Whether a user trusts FUSE on their particular system should be a policy 
> decision on the part of the user.  The kernel should not be deciding what 
> is trusted or not trusted here.

I believe there has been a good techincal argument made that fuse
mounted by an malicious user can defeat the protections ima is trying to
provide.

In particular the file could change after the signature of the file has
been verified without ima being alerted.

As such I think it is very reasonable for ima when a fuse filesystem has
been mounted by an unprivileged user to report that it can not verify
signatures, because IMA can not verify signatures in a meaningful way.

Now that might be better called SB_I_IMA_UNVERIFIABLE_SIGNATURES.

We may want to complement that flag with SB_I_UNTRUSTED_MOUNTER.
For the times when it is not the global root user who mounts
the filesystem.

So I do think when both conditions are true there very much is a case
for the kernel saying realizing it would be stupid to trust sigantures
it can not reliably verify.

On the flip side when it really is a trusted mounter, and it is in a
configuration that IMA has a reasonable expectation of seeing all of
the changes it would be nice if we can say please trust this mount.

It would also be nice if I could provide all of this information at
mount time (when I am the global root) with mount options.  So I don't
need to update all of my tooling to know how to update ima policy when I
am mounting a filesystem.

Eric

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-20  2:02       ` Eric W. Biederman
@ 2018-02-20 14:02         ` Mimi Zohar
  2018-02-20 20:16           ` Serge E. Hallyn
  2018-02-21 22:53           ` Eric W. Biederman
  0 siblings, 2 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-02-20 14:02 UTC (permalink / raw)
  To: Eric W. Biederman, James Morris
  Cc: linux-integrity, linux-security-module, linux-fsdevel,
	Miklos Szeredi, Seth Forshee, Dongsu Park, Alban Crequy,
	Serge E . Hallyn

On Mon, 2018-02-19 at 20:02 -0600, Eric W. Biederman wrote:
> James Morris <jmorris@namei.org> writes:
> 
> > On Mon, 19 Feb 2018, Eric W. Biederman wrote:
> >
> >> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> >> 
> >> > Files on untrusted filesystems, such as fuse, can change at any time,
> >> > making the measurement(s) and by extension signature verification
> >> > meaningless.
> >> 
> >> Filesystems with servers?
> >> Remote filesystems?
> >> Perhaps unexpected changes.
> >> 
> >> Untrusted sounds a bit harsh, and I am not certain it quite captures
> >> what you are looking to avoid.
> >
> > Right -- I think whether you trust a filesystem or not depends on how much 
> > assurance you have in your specific configuration, rather than whether you 
> > think the filesystem can be manipulated or not.
> >
> > There is a difference between:
> >
> >   - This fs has no way to communicate a change to IMA, and;
> >
> >   - This fs could be malicious.
> >
> > In the latter case, I suggest that any fs could be malicious if the 
> > overall security policy / settings are inadequate for the threat model, or 
> > if there are vulnerabilities which allow such security to be bypassed.
> >
> > Whether a user trusts FUSE on their particular system should be a policy 
> > decision on the part of the user.  The kernel should not be deciding what 
> > is trusted or not trusted here.
> 
> I believe there has been a good techincal argument made that fuse
> mounted by an malicious user can defeat the protections ima is trying to
> provide.
> 
> In particular the file could change after the signature of the file has
> been verified without ima being alerted.
> 
> As such I think it is very reasonable for ima when a fuse filesystem has
> been mounted by an unprivileged user to report that it can not verify
> signatures, because IMA can not verify signatures in a meaningful way.
> 
> Now that might be better called SB_I_IMA_UNVERIFIABLE_SIGNATURES.

The file signatures are always unverifiable, whether it is mounted by
root or an unprivileged user.  This flag would always be set.

> We may want to complement that flag with SB_I_UNTRUSTED_MOUNTER.
> For the times when it is not the global root user who mounts
> the filesystem.

Ok

> So I do think when both conditions are true there very much is a case
> for the kernel saying realizing it would be stupid to trust sigantures
> it can not reliably verify.

Agreed

> On the flip side when it really is a trusted mounter, and it is in a
> configuration that IMA has a reasonable expectation of seeing all of
> the changes it would be nice if we can say please trust this mount.

IMA has no way of detecting file change.  This was one of the reasons
for the original patch set's not using the cached IMA results.

Even in the case of a trusted mounter and not using IMA cached
results, there are no guarantees that the data read to calculate the
file hash, will be the same as what is subsequently read.  In some
environments this might be an acceptable risk, while in others not.

> It would also be nice if I could provide all of this information at
> mount time (when I am the global root) with mount options.  So I don't
> need to update all of my tooling to know how to update ima policy when I
> am mounting a filesystem.

The latest version of this patch relies on a builtin IMA policy to set
a flag.  No other changes are required to the IMA policy.  This
builtin policy could be used for environments not willing to accept
the default unverifiable signature risk.

Mimi

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-20 14:02         ` Mimi Zohar
@ 2018-02-20 20:16           ` Serge E. Hallyn
  2018-02-21 14:46             ` Mimi Zohar
  2018-02-21 22:53           ` Eric W. Biederman
  1 sibling, 1 reply; 18+ messages in thread
From: Serge E. Hallyn @ 2018-02-20 20:16 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Eric W. Biederman, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy, Serge E . Hallyn

Quoting Mimi Zohar (zohar@linux.vnet.ibm.com):
> On Mon, 2018-02-19 at 20:02 -0600, Eric W. Biederman wrote:
> > James Morris <jmorris@namei.org> writes:
> > 
> > > On Mon, 19 Feb 2018, Eric W. Biederman wrote:
> > >
> > >> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> > >> 
> > >> > Files on untrusted filesystems, such as fuse, can change at any time,
> > >> > making the measurement(s) and by extension signature verification
> > >> > meaningless.
> > >> 
> > >> Filesystems with servers?
> > >> Remote filesystems?
> > >> Perhaps unexpected changes.
> > >> 
> > >> Untrusted sounds a bit harsh, and I am not certain it quite captures
> > >> what you are looking to avoid.
> > >
> > > Right -- I think whether you trust a filesystem or not depends on how much 
> > > assurance you have in your specific configuration, rather than whether you 
> > > think the filesystem can be manipulated or not.
> > >
> > > There is a difference between:
> > >
> > >   - This fs has no way to communicate a change to IMA, and;
> > >
> > >   - This fs could be malicious.
> > >
> > > In the latter case, I suggest that any fs could be malicious if the 
> > > overall security policy / settings are inadequate for the threat model, or 
> > > if there are vulnerabilities which allow such security to be bypassed.
> > >
> > > Whether a user trusts FUSE on their particular system should be a policy 
> > > decision on the part of the user.  The kernel should not be deciding what 
> > > is trusted or not trusted here.
> > 
> > I believe there has been a good techincal argument made that fuse
> > mounted by an malicious user can defeat the protections ima is trying to
> > provide.
> > 
> > In particular the file could change after the signature of the file has
> > been verified without ima being alerted.
> > 
> > As such I think it is very reasonable for ima when a fuse filesystem has
> > been mounted by an unprivileged user to report that it can not verify
> > signatures, because IMA can not verify signatures in a meaningful way.
> > 
> > Now that might be better called SB_I_IMA_UNVERIFIABLE_SIGNATURES.
> 
> The file signatures are always unverifiable, whether it is mounted by
> root or an unprivileged user. �This flag would always be set.
> 
> > We may want to complement that flag with SB_I_UNTRUSTED_MOUNTER.
> > For the times when it is not the global root user who mounts
> > the filesystem.
> 
> Ok
> 
> > So I do think when both conditions are true there very much is a case
> > for the kernel saying realizing it would be stupid to trust sigantures
> > it can not reliably verify.
> 
> Agreed
> 
> > On the flip side when it really is a trusted mounter, and it is in a
> > configuration that IMA has a reasonable expectation of seeing all of
> > the changes it would be nice if we can say please trust this mount.
> 
> IMA has no way of detecting file change. �This was one of the reasons
> for the original patch set's not using the cached IMA results.
> 
> Even in the case of a trusted mounter and not using IMA cached
> results, there are no guarantees that the data read to calculate the
> file hash, will be the same as what is subsequently read. �In some
> environments this might be an acceptable risk, while in others not.

So for the cases where it's not, there should be an IMA option or policy
to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?

> > It would also be nice if I could provide all of this information at
> > mount time (when I am the global root) with mount options.  So I don't
> > need to update all of my tooling to know how to update ima policy when I
> > am mounting a filesystem.
> 
> The latest version of this patch relies on a builtin IMA policy to set
> a flag. �No other changes are required to the IMA policy. �This
> builtin policy could be used for environments not willing to accept
> the default unverifiable signature risk.

iiuc that sounds good.

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-20 20:16           ` Serge E. Hallyn
@ 2018-02-21 14:46             ` Mimi Zohar
  2018-02-21 22:46               ` Eric W. Biederman
  0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2018-02-21 14:46 UTC (permalink / raw)
  To: Serge E. Hallyn
  Cc: Eric W. Biederman, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

> > > On the flip side when it really is a trusted mounter, and it is in a
> > > configuration that IMA has a reasonable expectation of seeing all of
> > > the changes it would be nice if we can say please trust this mount.
> > 
> > IMA has no way of detecting file change.  This was one of the reasons
> > for the original patch set's not using the cached IMA results.
> > 
> > Even in the case of a trusted mounter and not using IMA cached
> > results, there are no guarantees that the data read to calculate the
> > file hash, will be the same as what is subsequently read.  In some
> > environments this might be an acceptable risk, while in others not.
> 
> So for the cases where it's not, there should be an IMA option or policy
> to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
> trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
> SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?

Right.  To summarize, we've identified 3 scenarios:
1. Fail signature verification on unprivileged non-init root mounted
file systems.

flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES and SB_I_UNTRUSTED_MOUNTER
(always enabled)

2. Permit signature verification on privileged file system mounts in a
secure system environment.  Willing to accept the risk.  Does not rely
on cached integrity results, but forces re-evaluation.

flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES, not SB_I_UNTRUSTED_MOUNTER or
IMA_FAIL_UNVERIFICABLE_SIGNATURES (default behavior)

3. Fail signature verification also on privileged file system mounts.
Fail safe, unwilling to accept the risk.

flags:
SB_I_IMA_UNVERIFIABLE_SIGNATURES and IMA_FAIL_UNVERIFIABLE_SIGNATURES

Enabled by specifying "ima_policy=unverifiable_sigs" on the boot
command line.

Mimi

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 14:46             ` Mimi Zohar
@ 2018-02-21 22:46               ` Eric W. Biederman
  2018-02-21 22:57                 ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-21 22:46 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Serge E. Hallyn, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

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

>> > > On the flip side when it really is a trusted mounter, and it is in a
>> > > configuration that IMA has a reasonable expectation of seeing all of
>> > > the changes it would be nice if we can say please trust this mount.
>> > 
>> > IMA has no way of detecting file change.  This was one of the reasons
>> > for the original patch set's not using the cached IMA results.
>> > 
>> > Even in the case of a trusted mounter and not using IMA cached
>> > results, there are no guarantees that the data read to calculate the
>> > file hash, will be the same as what is subsequently read.  In some
>> > environments this might be an acceptable risk, while in others not.
>> 
>> So for the cases where it's not, there should be an IMA option or policy
>> to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
>> trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
>> SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?
>
> Right.  To summarize, we've identified 3 scenarios:
> 1. Fail signature verification on unprivileged non-init root mounted
> file systems.
>
> flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES and SB_I_UNTRUSTED_MOUNTER
> (always enabled)
>
> 2. Permit signature verification on privileged file system mounts in a
> secure system environment.  Willing to accept the risk.  Does not rely
> on cached integrity results, but forces re-evaluation.
>
> flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES, not SB_I_UNTRUSTED_MOUNTER or
> IMA_FAIL_UNVERIFICABLE_SIGNATURES (default behavior)
>
> 3. Fail signature verification also on privileged file system mounts.
> Fail safe, unwilling to accept the risk.
>
> flags:
> SB_I_IMA_UNVERIFIABLE_SIGNATURES and IMA_FAIL_UNVERIFIABLE_SIGNATURES
>
> Enabled by specifying "ima_policy=unverifiable_sigs" on the boot
> command line.

There is another scenaro.
4. Permit signature verification on out of kernel but otherwise fully
   capable and trusted filesystems.

Fuse has a mode where it appears to be cache coherent, and guaranteed to
be local. AKA when fuse block is used and FUSE_WRITEBACK_CACHE is set.
That configuratioin plus the the allow_other mount option appear to
signal a fuse mount that can be reasonably be trusted as much as an
in-kernel block based filesystem.

That is a mode someone might use to mount exFat or ntfs-3g.

As all writes come from the kernel, and it is safe to have a write-back
cache I believe ima can reasonably verify signatures.  There may be
something technical like the need to verify i_version in this case,
but for purposes of argument let's say fuse has implemented all of the
necessary technical details.

In that case we have a case where it is reasonable to say that
SB_I_IMA_UNVERIFIABLE_SIGNATURES would be incorrect to set on a fuse
filesystem.

Mimi do you agree or am I missing something?

Eric

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-20 14:02         ` Mimi Zohar
  2018-02-20 20:16           ` Serge E. Hallyn
@ 2018-02-21 22:53           ` Eric W. Biederman
  2018-02-21 23:03             ` Mimi Zohar
  1 sibling, 1 reply; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-21 22:53 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: James Morris, linux-integrity, linux-security-module,
	linux-fsdevel, Miklos Szeredi, Seth Forshee, Dongsu Park,
	Alban Crequy, Serge E . Hallyn

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

> On Mon, 2018-02-19 at 20:02 -0600, Eric W. Biederman wrote:
>> It would also be nice if I could provide all of this information at
>> mount time (when I am the global root) with mount options.  So I don't
>> need to update all of my tooling to know how to update ima policy when I
>> am mounting a filesystem.
>
> The latest version of this patch relies on a builtin IMA policy to set
> a flag.  No other changes are required to the IMA policy.  This
> builtin policy could be used for environments not willing to accept
> the default unverifiable signature risk.

I still remain puzzled by this.  Why is the default to accept the risk?

Eric

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 22:46               ` Eric W. Biederman
@ 2018-02-21 22:57                 ` Mimi Zohar
  2018-02-21 23:12                   ` Eric W. Biederman
  0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2018-02-21 22:57 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

On Wed, 2018-02-21 at 16:46 -0600, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> >> > > On the flip side when it really is a trusted mounter, and it is in a
> >> > > configuration that IMA has a reasonable expectation of seeing all of
> >> > > the changes it would be nice if we can say please trust this mount.
> >> > 
> >> > IMA has no way of detecting file change.  This was one of the reasons
> >> > for the original patch set's not using the cached IMA results.
> >> > 
> >> > Even in the case of a trusted mounter and not using IMA cached
> >> > results, there are no guarantees that the data read to calculate the
> >> > file hash, will be the same as what is subsequently read.  In some
> >> > environments this might be an acceptable risk, while in others not.
> >> 
> >> So for the cases where it's not, there should be an IMA option or policy
> >> to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
> >> trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
> >> SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?
> >
> > Right.  To summarize, we've identified 3 scenarios:
> > 1. Fail signature verification on unprivileged non-init root mounted
> > file systems.
> >
> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES and SB_I_UNTRUSTED_MOUNTER
> > (always enabled)
> >
> > 2. Permit signature verification on privileged file system mounts in a
> > secure system environment.  Willing to accept the risk.  Does not rely
> > on cached integrity results, but forces re-evaluation.
> >
> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES, not SB_I_UNTRUSTED_MOUNTER or
> > IMA_FAIL_UNVERIFICABLE_SIGNATURES (default behavior)
> >
> > 3. Fail signature verification also on privileged file system mounts.
> > Fail safe, unwilling to accept the risk.
> >
> > flags:
> > SB_I_IMA_UNVERIFIABLE_SIGNATURES and IMA_FAIL_UNVERIFIABLE_SIGNATURES
> >
> > Enabled by specifying "ima_policy=unverifiable_sigs" on the boot
> > command line.
> 
> There is another scenaro.
> 4. Permit signature verification on out of kernel but otherwise fully
>    capable and trusted filesystems.
> 
> Fuse has a mode where it appears to be cache coherent, and guaranteed to
> be local. AKA when fuse block is used and FUSE_WRITEBACK_CACHE is set.
> That configuratioin plus the the allow_other mount option appear to
> signal a fuse mount that can be reasonably be trusted as much as an
> in-kernel block based filesystem.
> 
> That is a mode someone might use to mount exFat or ntfs-3g.
> 
> As all writes come from the kernel, and it is safe to have a write-back
> cache I believe ima can reasonably verify signatures.  There may be
> something technical like the need to verify i_version in this case,
> but for purposes of argument let's say fuse has implemented all of the
> necessary technical details.
> 
> In that case we have a case where it is reasonable to say that
> SB_I_IMA_UNVERIFIABLE_SIGNATURES would be incorrect to set on a fuse
> filesystem.
> 
> Mimi do you agree or am I missing something?

This simply sounds like a performance improvement to the second
scenario, where instead of *always* forcing re-validation, it checks
the i_version.  Perhaps based on a different flag.

Mimi 

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 22:53           ` Eric W. Biederman
@ 2018-02-21 23:03             ` Mimi Zohar
  0 siblings, 0 replies; 18+ messages in thread
From: Mimi Zohar @ 2018-02-21 23:03 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: James Morris, linux-integrity, linux-security-module,
	linux-fsdevel, Miklos Szeredi, Seth Forshee, Dongsu Park,
	Alban Crequy, Serge E . Hallyn

On Wed, 2018-02-21 at 16:53 -0600, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > On Mon, 2018-02-19 at 20:02 -0600, Eric W. Biederman wrote:
> >> It would also be nice if I could provide all of this information at
> >> mount time (when I am the global root) with mount options.  So I don't
> >> need to update all of my tooling to know how to update ima policy when I
> >> am mounting a filesystem.
> >
> > The latest version of this patch relies on a builtin IMA policy to set
> > a flag.  No other changes are required to the IMA policy.  This
> > builtin policy could be used for environments not willing to accept
> > the default unverifiable signature risk.
> 
> I still remain puzzled by this.  Why is the default to accept the risk?

Accepting the risk is option 2, the privileged mount scenario.  It
requires re-evaluating the cached info.

Mimi

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 22:57                 ` Mimi Zohar
@ 2018-02-21 23:12                   ` Eric W. Biederman
  2018-02-21 23:32                     ` Mimi Zohar
  0 siblings, 1 reply; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-21 23:12 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Serge E. Hallyn, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

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

> On Wed, 2018-02-21 at 16:46 -0600, Eric W. Biederman wrote:
>> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
>> 
>> >> > > On the flip side when it really is a trusted mounter, and it is in a
>> >> > > configuration that IMA has a reasonable expectation of seeing all of
>> >> > > the changes it would be nice if we can say please trust this mount.
>> >> > 
>> >> > IMA has no way of detecting file change.  This was one of the reasons
>> >> > for the original patch set's not using the cached IMA results.
>> >> > 
>> >> > Even in the case of a trusted mounter and not using IMA cached
>> >> > results, there are no guarantees that the data read to calculate the
>> >> > file hash, will be the same as what is subsequently read.  In some
>> >> > environments this might be an acceptable risk, while in others not.
>> >> 
>> >> So for the cases where it's not, there should be an IMA option or policy
>> >> to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
>> >> trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
>> >> SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?
>> >
>> > Right.  To summarize, we've identified 3 scenarios:
>> > 1. Fail signature verification on unprivileged non-init root mounted
>> > file systems.
>> >
>> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES and SB_I_UNTRUSTED_MOUNTER
>> > (always enabled)
>> >
>> > 2. Permit signature verification on privileged file system mounts in a
>> > secure system environment.  Willing to accept the risk.  Does not rely
>> > on cached integrity results, but forces re-evaluation.
>> >
>> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES, not SB_I_UNTRUSTED_MOUNTER or
>> > IMA_FAIL_UNVERIFICABLE_SIGNATURES (default behavior)
>> >
>> > 3. Fail signature verification also on privileged file system mounts.
>> > Fail safe, unwilling to accept the risk.
>> >
>> > flags:
>> > SB_I_IMA_UNVERIFIABLE_SIGNATURES and IMA_FAIL_UNVERIFIABLE_SIGNATURES
>> >
>> > Enabled by specifying "ima_policy=unverifiable_sigs" on the boot
>> > command line.
>> 
>> There is another scenaro.
>> 4. Permit signature verification on out of kernel but otherwise fully
>>    capable and trusted filesystems.
>> 
>> Fuse has a mode where it appears to be cache coherent, and guaranteed to
>> be local. AKA when fuse block is used and FUSE_WRITEBACK_CACHE is set.
>> That configuratioin plus the the allow_other mount option appear to
>> signal a fuse mount that can be reasonably be trusted as much as an
>> in-kernel block based filesystem.
>> 
>> That is a mode someone might use to mount exFat or ntfs-3g.
>> 
>> As all writes come from the kernel, and it is safe to have a write-back
>> cache I believe ima can reasonably verify signatures.  There may be
>> something technical like the need to verify i_version in this case,
>> but for purposes of argument let's say fuse has implemented all of the
>> necessary technical details.
>> 
>> In that case we have a case where it is reasonable to say that
>> SB_I_IMA_UNVERIFIABLE_SIGNATURES would be incorrect to set on a fuse
>> filesystem.
>> 
>> Mimi do you agree or am I missing something?
>
> This simply sounds like a performance improvement to the second
> scenario, where instead of *always* forcing re-validation, it checks
> the i_version.  Perhaps based on a different flag.

As I understand the second scenario SB_I_IMA_UNVERIFIABLE_SIGNATURES
is set, which implies that the filesystem is lacking something for IMA
to reliably know when a file has changed.  AKA a technical deficiency.

The fourth scenario is the case when SB_I_IMA_UNVERIFIABLE_SIGNATURES
can be legitimately be cleared, because the filesystem provides all
of the necessary support for IMA to reliably know when a file has
changed.

My point is that cases exists or it is straight forward to implemented
in fuse.


I add the fourth case so that we can get a solid definition of
SB_I_IMA_UNVERIFIABLE_SIGNATURES.

Eric

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 23:12                   ` Eric W. Biederman
@ 2018-02-21 23:32                     ` Mimi Zohar
  2018-02-27  2:12                       ` Eric W. Biederman
  0 siblings, 1 reply; 18+ messages in thread
From: Mimi Zohar @ 2018-02-21 23:32 UTC (permalink / raw)
  To: Eric W. Biederman
  Cc: Serge E. Hallyn, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

On Wed, 2018-02-21 at 17:12 -0600, Eric W. Biederman wrote:
> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> 
> > On Wed, 2018-02-21 at 16:46 -0600, Eric W. Biederman wrote:
> >> Mimi Zohar <zohar@linux.vnet.ibm.com> writes:
> >> 
> >> >> > > On the flip side when it really is a trusted mounter, and it is in a
> >> >> > > configuration that IMA has a reasonable expectation of seeing all of
> >> >> > > the changes it would be nice if we can say please trust this mount.
> >> >> > 
> >> >> > IMA has no way of detecting file change.  This was one of the reasons
> >> >> > for the original patch set's not using the cached IMA results.
> >> >> > 
> >> >> > Even in the case of a trusted mounter and not using IMA cached
> >> >> > results, there are no guarantees that the data read to calculate the
> >> >> > file hash, will be the same as what is subsequently read.  In some
> >> >> > environments this might be an acceptable risk, while in others not.
> >> >> 
> >> >> So for the cases where it's not, there should be an IMA option or policy
> >> >> to say any SB_I_IMA_UNVERIFIABLE_SIGNATURES mounts should be not
> >> >> trusted, with the default being both SB_I_IMA_UNVERIFIABLE_SIGNATURES and
> >> >> SB_I_UNTRUSTED_MOUNTER must be true to not trust, right?
> >> >
> >> > Right.  To summarize, we've identified 3 scenarios:
> >> > 1. Fail signature verification on unprivileged non-init root mounted
> >> > file systems.
> >> >
> >> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES and SB_I_UNTRUSTED_MOUNTER
> >> > (always enabled)
> >> >
> >> > 2. Permit signature verification on privileged file system mounts in a
> >> > secure system environment.  Willing to accept the risk.  Does not rely
> >> > on cached integrity results, but forces re-evaluation.
> >> >
> >> > flags: SB_I_IMA_UNVERIFIABLE_SIGNATURES, not SB_I_UNTRUSTED_MOUNTER or
> >> > IMA_FAIL_UNVERIFICABLE_SIGNATURES (default behavior)
> >> >
> >> > 3. Fail signature verification also on privileged file system mounts.
> >> > Fail safe, unwilling to accept the risk.
> >> >
> >> > flags:
> >> > SB_I_IMA_UNVERIFIABLE_SIGNATURES and IMA_FAIL_UNVERIFIABLE_SIGNATURES
> >> >
> >> > Enabled by specifying "ima_policy=unverifiable_sigs" on the boot
> >> > command line.
> >> 
> >> There is another scenaro.
> >> 4. Permit signature verification on out of kernel but otherwise fully
> >>    capable and trusted filesystems.
> >> 
> >> Fuse has a mode where it appears to be cache coherent, and guaranteed to
> >> be local. AKA when fuse block is used and FUSE_WRITEBACK_CACHE is set.
> >> That configuratioin plus the the allow_other mount option appear to
> >> signal a fuse mount that can be reasonably be trusted as much as an
> >> in-kernel block based filesystem.
> >> 
> >> That is a mode someone might use to mount exFat or ntfs-3g.
> >> 
> >> As all writes come from the kernel, and it is safe to have a write-back
> >> cache I believe ima can reasonably verify signatures.  There may be
> >> something technical like the need to verify i_version in this case,
> >> but for purposes of argument let's say fuse has implemented all of the
> >> necessary technical details.
> >> 
> >> In that case we have a case where it is reasonable to say that
> >> SB_I_IMA_UNVERIFIABLE_SIGNATURES would be incorrect to set on a fuse
> >> filesystem.
> >> 
> >> Mimi do you agree or am I missing something?
> >
> > This simply sounds like a performance improvement to the second
> > scenario, where instead of *always* forcing re-validation, it checks
> > the i_version.  Perhaps based on a different flag.
> 
> As I understand the second scenario SB_I_IMA_UNVERIFIABLE_SIGNATURES
> is set, which implies that the filesystem is lacking something for IMA
> to reliably know when a file has changed.  AKA a technical deficiency.
> 
> The fourth scenario is the case when SB_I_IMA_UNVERIFIABLE_SIGNATURES
> can be legitimately be cleared, because the filesystem provides all
> of the necessary support for IMA to reliably know when a file has
> changed.

The information might be there, but IMA currently detects a file
change and resets the flags only when the last writer calls
__fput().  Any other time, new support would be needed.

Mimi

> My point is that cases exists or it is straight forward to implemented
> in fuse.
> 
> 
> I add the fourth case so that we can get a solid definition of
> SB_I_IMA_UNVERIFIABLE_SIGNATURES.
> 
> Eric
> 

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

* Re: [PATCH v1 1/2] ima: fail signature verification on untrusted filesystems
  2018-02-21 23:32                     ` Mimi Zohar
@ 2018-02-27  2:12                       ` Eric W. Biederman
  0 siblings, 0 replies; 18+ messages in thread
From: Eric W. Biederman @ 2018-02-27  2:12 UTC (permalink / raw)
  To: Mimi Zohar
  Cc: Serge E. Hallyn, James Morris, linux-integrity,
	linux-security-module, linux-fsdevel, Miklos Szeredi,
	Seth Forshee, Dongsu Park, Alban Crequy

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

> On Wed, 2018-02-21 at 17:12 -0600, Eric W. Biederman wrote:
>> 
>> As I understand the second scenario SB_I_IMA_UNVERIFIABLE_SIGNATURES
>> is set, which implies that the filesystem is lacking something for IMA
>> to reliably know when a file has changed.  AKA a technical deficiency.
>> 
>> The fourth scenario is the case when SB_I_IMA_UNVERIFIABLE_SIGNATURES
>> can be legitimately be cleared, because the filesystem provides all
>> of the necessary support for IMA to reliably know when a file has
>> changed.
>
> The information might be there, but IMA currently detects a file
> change and resets the flags only when the last writer calls
> __fput().  Any other time, new support would be needed.

My point was only that for local NTFS or local exFAT with a quality
and trusted fuse implementation they should be as safe in this regard
as any other filesystem.   So in theory we could have fuse implementing
this level of filesystem as well.  Not that I suggest we try for that
out of the gate.

Thank you very much for the clarification about the last fput that helps
me understand SB_I_IMA_UNVERIFIEDABLE_SIGNATURES much better.

Eric

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

end of thread, other threads:[~2018-02-27  2:13 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-02-19 15:18 [PATCH v1 0/2] ima: untrusted filesystems Mimi Zohar
2018-02-19 15:18 ` [PATCH v1 1/2] ima: fail signature verification on " Mimi Zohar
2018-02-19 21:47   ` Eric W. Biederman
2018-02-20  0:52     ` James Morris
2018-02-20  2:02       ` Eric W. Biederman
2018-02-20 14:02         ` Mimi Zohar
2018-02-20 20:16           ` Serge E. Hallyn
2018-02-21 14:46             ` Mimi Zohar
2018-02-21 22:46               ` Eric W. Biederman
2018-02-21 22:57                 ` Mimi Zohar
2018-02-21 23:12                   ` Eric W. Biederman
2018-02-21 23:32                     ` Mimi Zohar
2018-02-27  2:12                       ` Eric W. Biederman
2018-02-21 22:53           ` Eric W. Biederman
2018-02-21 23:03             ` Mimi Zohar
2018-02-19 22:50   ` kbuild test robot
2018-02-19 23:36   ` kbuild test robot
2018-02-19 15:18 ` [PATCH v1 2/2] fuse: define the filesystem as untrusted Mimi Zohar

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).