| Message ID | 1518615315-7162-2-git-send-email-zohar@linux.vnet.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > 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 always fails the file signature verification on unprivileged > and untrusted filesystems. To also fail file signature verification on Why only untrusted? Fuse could cause the same issue if it just messes up when mounted from init userns right? > privileged, untrusted filesystems requires a custom policy. (I'm not saying you shouldn't do this, but) does this mean that a container whose rootfs is fuse-mounted by the unprivileged user cannot possibly use IMA? Good thing we can partially work around that by intercepting real mount calls with Tycho's new patchset :) > (This patch is based on Alban Crequy's use of fs_flags and patch > description.) > > 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> > --- > include/linux/fs.h | 1 + > security/integrity/ima/ima_appraise.c | 10 +++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 2a815560fda0..faffe4aab43d 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2069,6 +2069,7 @@ struct file_system_type { > #define FS_BINARY_MOUNTDATA 2 > #define FS_HAS_SUBTYPE 4 > #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ > +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ > #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ > struct dentry *(*mount) (struct file_system_type *, int, > const char *, void *); > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index f2803a40ff82..af8add31fe26 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, > } > > out: > - if (status != INTEGRITY_PASS) { > + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ > + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && > + (inode->i_sb->s_user_ns != &init_user_ns)) { > + 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 +316,7 @@ int ima_appraise_measurement(enum ima_hooks func, > } else { > ima_cache_flags(iint, func); > } > + > ima_set_cache_status(iint, func, status); > return status; > } > -- > 2.7.5
On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > 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 always fails the file signature verification on unprivileged > > and untrusted filesystems. To also fail file signature verification on > > Why only untrusted? Fuse could cause the same issue if it just > messes up when mounted from init userns right? Right, whether it is an unprivileged mount or not, fuse can return whatever it wants, whenever it wants. IMA can calculate the file hash based based on what it reads, but fuse can return whatever it wants on subsequent reads. Refer to the discussion with Linus - http://kernsec.org/pipermail/linu x-security-module-archive/2018-February/005200.html > > privileged, untrusted filesystems requires a custom policy. > > (I'm not saying you shouldn't do this, but) does this mean that > a container whose rootfs is fuse-mounted by the unprivileged user > cannot possibly use IMA? How would you suggest to differentiate between your unprivileged fuse mounts from unintended, unintended malicious ones? The remaining patches are policy based. > Good thing we can partially work around that by intercepting real > mount calls with Tycho's new patchset :) Can you provide a little more details? thanks, Mimi > > > (This patch is based on Alban Crequy's use of fs_flags and patch > > description.) > > > > 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> > > --- > > include/linux/fs.h | 1 + > > security/integrity/ima/ima_appraise.c | 10 +++++++++- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 2a815560fda0..faffe4aab43d 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -2069,6 +2069,7 @@ struct file_system_type { > > #define FS_BINARY_MOUNTDATA 2 > > #define FS_HAS_SUBTYPE 4 > > #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ > > +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ > > #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ > > struct dentry *(*mount) (struct file_system_type *, int, > > const char *, void *); > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > > index f2803a40ff82..af8add31fe26 100644 > > --- a/security/integrity/ima/ima_appraise.c > > +++ b/security/integrity/ima/ima_appraise.c > > @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, > > } > > > > out: > > - if (status != INTEGRITY_PASS) { > > + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ > > + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && > > + (inode->i_sb->s_user_ns != &init_user_ns)) { > > + 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 +316,7 @@ int ima_appraise_measurement(enum ima_hooks func, > > } else { > > ima_cache_flags(iint, func); > > } > > + > > ima_set_cache_status(iint, func, status); > > return status; > > } > > -- > > 2.7.5 >
Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > 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 always fails the file signature verification on unprivileged > > > and untrusted filesystems. To also fail file signature verification on > > > > Why only untrusted? Fuse could cause the same issue if it just > > messes up when mounted from init userns right? > > Right, whether it is an unprivileged mount or not, fuse can return > whatever it wants, whenever it wants. IMA can calculate the file hash > based based on what it reads, but fuse can return whatever it wants on > subsequent reads. Ok but your patch seems to let privileged fuse mounts slide? (see below) > Refer to the discussion with Linus - http://kernsec.org/pipermail/linu > x-security-module-archive/2018-February/005200.html > > > > privileged, untrusted filesystems requires a custom policy. > > > > (I'm not saying you shouldn't do this, but) does this mean that > > a container whose rootfs is fuse-mounted by the unprivileged user > > cannot possibly use IMA? > > How would you suggest to differentiate between your unprivileged fuse > mounts from unintended, unintended malicious ones? I wouldn't. > The remaining patches are policy based. > > > Good thing we can partially work around that by intercepting real > > mount calls with Tycho's new patchset :) > > Can you provide a little more details? It would allow a container runtime to intercept mount(2) and perform a real mount on the user's behalf. Assuming the runtime is privileged, of course, otherwise it would intercept and do a fuse mount which is no help here :) https://lkml.org/lkml/2018/2/4/28 > thanks, > > Mimi > > > > > > (This patch is based on Alban Crequy's use of fs_flags and patch > > > description.) > > > > > > 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> > > > --- > > > include/linux/fs.h | 1 + > > > security/integrity/ima/ima_appraise.c | 10 +++++++++- > > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > > index 2a815560fda0..faffe4aab43d 100644 > > > --- a/include/linux/fs.h > > > +++ b/include/linux/fs.h > > > @@ -2069,6 +2069,7 @@ struct file_system_type { > > > #define FS_BINARY_MOUNTDATA 2 > > > #define FS_HAS_SUBTYPE 4 > > > #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ > > > +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ > > > #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ > > > struct dentry *(*mount) (struct file_system_type *, int, > > > const char *, void *); > > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > > > index f2803a40ff82..af8add31fe26 100644 > > > --- a/security/integrity/ima/ima_appraise.c > > > +++ b/security/integrity/ima/ima_appraise.c > > > @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, > > > } > > > > > > out: > > > - if (status != INTEGRITY_PASS) { > > > + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ > > > + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && > > > + (inode->i_sb->s_user_ns != &init_user_ns)) { ^ if inode->i_sb->s_user_ns == &init_user_ns you don't fail. > > > + 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 +316,7 @@ int ima_appraise_measurement(enum ima_hooks func, > > > } else { > > > ima_cache_flags(iint, func); > > > } > > > + > > > ima_set_cache_status(iint, func, status); > > > return status; > > > } > > > -- > > > 2.7.5 > >
On Wed, 2018-02-14 at 09:16 -0600, Serge E. Hallyn wrote: > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > 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 always fails the file signature verification on unprivileged > > > > and untrusted filesystems. To also fail file signature verification on > > > > > > Why only untrusted? Fuse could cause the same issue if it just > > > messes up when mounted from init userns right? > > > > Right, whether it is an unprivileged mount or not, fuse can return > > whatever it wants, whenever it wants. IMA can calculate the file hash > > based based on what it reads, but fuse can return whatever it wants on > > subsequent reads. > > Ok but your patch seems to let privileged fuse mounts slide? (see below) Unprivileged fuse mounts hasn't been upstreamed yet, so we wouldn't be breaking existing userspace. > > > Refer to the discussion with Linus - http://kernsec.org/pipermail/linu > > x-security-module-archive/2018-February/005200.html > > > > > > privileged, untrusted filesystems requires a custom policy. > > > > > > (I'm not saying you shouldn't do this, but) does this mean that > > > a container whose rootfs is fuse-mounted by the unprivileged user > > > cannot possibly use IMA? > > > > How would you suggest to differentiate between your unprivileged fuse > > mounts from unintended, unintended malicious ones? > > I wouldn't. What happened to the requirement that systems should be "fail-safe"? Ok, hard coding this rule probably is not a good idea. For those wanting to take this liability on their systems, we can make this configurable, like for privileged fuse mounts. Unlike for privileged fuse mounts, the builtin policies, I think, should be fail safe and include the "fail" rule for unprivileged fuse mounts. > > > The remaining patches are policy based. > > > > > Good thing we can partially work around that by intercepting real > > > mount calls with Tycho's new patchset :) > > > > Can you provide a little more details? > > It would allow a container runtime to intercept mount(2) and perform > a real mount on the user's behalf. Assuming the runtime is privileged, > of course, otherwise it would intercept and do a fuse mount which is > no help here :) > > https://lkml.org/lkml/2018/2/4/28 thanks! Mimi
Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > On Wed, 2018-02-14 at 09:16 -0600, Serge E. Hallyn wrote: > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > > 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 always fails the file signature verification on unprivileged > > > > > and untrusted filesystems. To also fail file signature verification on > > > > > > > > Why only untrusted? Fuse could cause the same issue if it just > > > > messes up when mounted from init userns right? > > > > > > Right, whether it is an unprivileged mount or not, fuse can return > > > whatever it wants, whenever it wants. IMA can calculate the file hash > > > based based on what it reads, but fuse can return whatever it wants on > > > subsequent reads. > > > > Ok but your patch seems to let privileged fuse mounts slide? (see below) > > Unprivileged fuse mounts hasn't been upstreamed yet, so we wouldn't be > breaking existing userspace. I don't think I'm being clear. In your patch it looks like you mark unprivileged FUSE mounts as INTEGRITY_FAIL. I agree you should do that. But you skip the FS_UNTRUSTED check for privileged FUSE mounts. I'm asking why that's ok. > > > Refer to the discussion with Linus - http://kernsec.org/pipermail/linu > > > x-security-module-archive/2018-February/005200.html > > > > > > > > privileged, untrusted filesystems requires a custom policy. > > > > > > > > (I'm not saying you shouldn't do this, but) does this mean that > > > > a container whose rootfs is fuse-mounted by the unprivileged user > > > > cannot possibly use IMA? > > > > > > How would you suggest to differentiate between your unprivileged fuse > > > mounts from unintended, unintended malicious ones? > > > > I wouldn't. > > What happened to the requirement that systems should be "fail-safe"? My point was - I was asking whether there was any way to have IMA be meaningful with such containers, not saying I had any ideas, and certainly not saying that just because you can't detect it means you should allow it in all cases. It's too bad that it has this effect, but I agree with your patch. I only didn't ack it because you're skipping the check for privileged mounts which seems wrong. -serge
On Wed, 2018-02-14 at 09:42 -0600, Serge E. Hallyn wrote: > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > On Wed, 2018-02-14 at 09:16 -0600, Serge E. Hallyn wrote: > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > > > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > > > 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 always fails the file signature verification on unprivileged > > > > > > and untrusted filesystems. To also fail file signature verification on > > > > > > > > > > Why only untrusted? Fuse could cause the same issue if it just > > > > > messes up when mounted from init userns right? > > > > > > > > Right, whether it is an unprivileged mount or not, fuse can return > > > > whatever it wants, whenever it wants. IMA can calculate the file hash > > > > based based on what it reads, but fuse can return whatever it wants on > > > > subsequent reads. > > > > > > Ok but your patch seems to let privileged fuse mounts slide? (see below) > > > > Unprivileged fuse mounts hasn't been upstreamed yet, so we wouldn't be > > breaking existing userspace. > > I don't think I'm being clear. > > In your patch it looks like you mark unprivileged FUSE mounts as > INTEGRITY_FAIL. I agree you should do that. But you skip the > FS_UNTRUSTED check for privileged FUSE mounts. I'm asking why > that's ok. > > > > > Refer to the discussion with Linus - http://kernsec.org/pipermail/linu > > > > x-security-module-archive/2018-February/005200.html > > > > > > > > > > privileged, untrusted filesystems requires a custom policy. > > > > > > > > > > (I'm not saying you shouldn't do this, but) does this mean that > > > > > a container whose rootfs is fuse-mounted by the unprivileged user > > > > > cannot possibly use IMA? > > > > > > > > How would you suggest to differentiate between your unprivileged fuse > > > > mounts from unintended, unintended malicious ones? > > > > > > I wouldn't. > > > > What happened to the requirement that systems should be "fail-safe"? > > My point was - I was asking whether there was any way to have IMA be > meaningful with such containers, not saying I had any ideas, and > certainly not saying that just because you can't detect it means you > should allow it in all cases. It's too bad that it has this effect, > but I agree with your patch. > > I only didn't ack it because you're skipping the check for privileged > mounts which seems wrong. Oh! That is based on Linus' "request" not to break userspace. Mimi
Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > On Wed, 2018-02-14 at 09:42 -0600, Serge E. Hallyn wrote: > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > On Wed, 2018-02-14 at 09:16 -0600, Serge E. Hallyn wrote: > > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > > On Wed, 2018-02-14 at 08:49 -0600, Serge E. Hallyn wrote: > > > > > > Quoting Mimi Zohar (zohar@linux.vnet.ibm.com): > > > > > > > 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 always fails the file signature verification on unprivileged > > > > > > > and untrusted filesystems. To also fail file signature verification on > > > > > > > > > > > > Why only untrusted? Fuse could cause the same issue if it just > > > > > > messes up when mounted from init userns right? > > > > > > > > > > Right, whether it is an unprivileged mount or not, fuse can return > > > > > whatever it wants, whenever it wants. IMA can calculate the file hash > > > > > based based on what it reads, but fuse can return whatever it wants on > > > > > subsequent reads. > > > > > > > > Ok but your patch seems to let privileged fuse mounts slide? (see below) > > > > > > Unprivileged fuse mounts hasn't been upstreamed yet, so we wouldn't be > > > breaking existing userspace. > > > > I don't think I'm being clear. > > > > In your patch it looks like you mark unprivileged FUSE mounts as > > INTEGRITY_FAIL. I agree you should do that. But you skip the > > FS_UNTRUSTED check for privileged FUSE mounts. I'm asking why > > that's ok. > > > > > > > Refer to the discussion with Linus - http://kernsec.org/pipermail/linu > > > > > x-security-module-archive/2018-February/005200.html > > > > > > > > > > > > privileged, untrusted filesystems requires a custom policy. > > > > > > > > > > > > (I'm not saying you shouldn't do this, but) does this mean that > > > > > > a container whose rootfs is fuse-mounted by the unprivileged user > > > > > > cannot possibly use IMA? > > > > > > > > > > How would you suggest to differentiate between your unprivileged fuse > > > > > mounts from unintended, unintended malicious ones? > > > > > > > > I wouldn't. > > > > > > What happened to the requirement that systems should be "fail-safe"? > > > > My point was - I was asking whether there was any way to have IMA be > > meaningful with such containers, not saying I had any ideas, and > > certainly not saying that just because you can't detect it means you > > should allow it in all cases. It's too bad that it has this effect, > > but I agree with your patch. > > > > I only didn't ack it because you're skipping the check for privileged > > mounts which seems wrong. > > Oh! That is based on Linus' "request" not to break userspace. Oh, yeah, I guess that would do it :) It seems so wrong that it's probably worth putting a comment above that exception. So probably not worth much but Acked-by: Serge Hallyn <serge@hallyn.com> thanks, -serge
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. > > 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 always fails the file signature verification on unprivileged > and untrusted filesystems. To also fail file signature verification on > privileged, untrusted filesystems requires a custom policy. > > (This patch is based on Alban Crequy's use of fs_flags and patch > description.) This would be much better done based on a flag in s_iflags and then the mounts that need this can set this. That new flag can perhaps be called SB_I_IMA_FAIL. Among other things that should allow the policy of when to set this to be set in fuse where it is obvious rather than in an magic location in IMA. 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> > --- > include/linux/fs.h | 1 + > security/integrity/ima/ima_appraise.c | 10 +++++++++- > 2 files changed, 10 insertions(+), 1 deletion(-) > > diff --git a/include/linux/fs.h b/include/linux/fs.h > index 2a815560fda0..faffe4aab43d 100644 > --- a/include/linux/fs.h > +++ b/include/linux/fs.h > @@ -2069,6 +2069,7 @@ struct file_system_type { > #define FS_BINARY_MOUNTDATA 2 > #define FS_HAS_SUBTYPE 4 > #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ > +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ > #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ > struct dentry *(*mount) (struct file_system_type *, int, > const char *, void *); > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > index f2803a40ff82..af8add31fe26 100644 > --- a/security/integrity/ima/ima_appraise.c > +++ b/security/integrity/ima/ima_appraise.c > @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, > } > > out: > - if (status != INTEGRITY_PASS) { > + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ > + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && > + (inode->i_sb->s_user_ns != &init_user_ns)) { > + 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 +316,7 @@ int ima_appraise_measurement(enum ima_hooks func, > } else { > ima_cache_flags(iint, func); > } > + > ima_set_cache_status(iint, func, status); > return status; > }
On Wed, 2018-02-14 at 17:57 -0600, 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. > > > > 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 always fails the file signature verification on unprivileged > > and untrusted filesystems. To also fail file signature verification on > > privileged, untrusted filesystems requires a custom policy. > > > > (This patch is based on Alban Crequy's use of fs_flags and patch > > description.) > > This would be much better done based on a flag in s_iflags and then the > mounts that need this can set this. That new flag can perhaps be called > SB_I_IMA_FAIL. > > Among other things that should allow the policy of when to set this to > be set in fuse where it is obvious rather than in an magic location in > IMA. Using s_iflags instead of fs_flags is fine, but I'm not sure how this affects the IMA policy. This patch set assumes only unprivileged, untrusted filesytems can automatically fail file signature verification (2nd patch), as that hasn't yet been upstreamed and won't break userspace. Based on policy, IMA should additionally be able to fail the signature verification for files on privileged, untrusted filesystems. Mimi > 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> > > --- > > include/linux/fs.h | 1 + > > security/integrity/ima/ima_appraise.c | 10 +++++++++- > > 2 files changed, 10 insertions(+), 1 deletion(-) > > > > diff --git a/include/linux/fs.h b/include/linux/fs.h > > index 2a815560fda0..faffe4aab43d 100644 > > --- a/include/linux/fs.h > > +++ b/include/linux/fs.h > > @@ -2069,6 +2069,7 @@ struct file_system_type { > > #define FS_BINARY_MOUNTDATA 2 > > #define FS_HAS_SUBTYPE 4 > > #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ > > +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ > > #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ > > struct dentry *(*mount) (struct file_system_type *, int, > > const char *, void *); > > diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c > > index f2803a40ff82..af8add31fe26 100644 > > --- a/security/integrity/ima/ima_appraise.c > > +++ b/security/integrity/ima/ima_appraise.c > > @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, > > } > > > > out: > > - if (status != INTEGRITY_PASS) { > > + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ > > + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && > > + (inode->i_sb->s_user_ns != &init_user_ns)) { > > + 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 +316,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/include/linux/fs.h b/include/linux/fs.h index 2a815560fda0..faffe4aab43d 100644 --- a/include/linux/fs.h +++ b/include/linux/fs.h @@ -2069,6 +2069,7 @@ struct file_system_type { #define FS_BINARY_MOUNTDATA 2 #define FS_HAS_SUBTYPE 4 #define FS_USERNS_MOUNT 8 /* Can be mounted by userns root */ +#define FS_UNTRUSTED 16 /* Defined filesystem as untrusted */ #define FS_RENAME_DOES_D_MOVE 32768 /* FS will handle d_move() during rename() internally. */ struct dentry *(*mount) (struct file_system_type *, int, const char *, void *); diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c index f2803a40ff82..af8add31fe26 100644 --- a/security/integrity/ima/ima_appraise.c +++ b/security/integrity/ima/ima_appraise.c @@ -292,7 +292,14 @@ int ima_appraise_measurement(enum ima_hooks func, } out: - if (status != INTEGRITY_PASS) { + /* Fail untrusted and unpriviliged filesystems (eg FUSE) */ + if ((inode->i_sb->s_type->fs_flags & FS_UNTRUSTED) && + (inode->i_sb->s_user_ns != &init_user_ns)) { + 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 +316,7 @@ int ima_appraise_measurement(enum ima_hooks func, } else { ima_cache_flags(iint, func); } + ima_set_cache_status(iint, func, status); return status; }
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 always fails the file signature verification on unprivileged and untrusted filesystems. To also fail file signature verification on privileged, untrusted filesystems requires a custom policy. (This patch is based on Alban Crequy's use of fs_flags and patch description.) 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> --- include/linux/fs.h | 1 + security/integrity/ima/ima_appraise.c | 10 +++++++++- 2 files changed, 10 insertions(+), 1 deletion(-)