| Message ID | 1519053483-18396-2-git-send-email-zohar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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)
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
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
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> 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
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
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.
> > > 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
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
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
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
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
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
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 >
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
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)
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(-)