Re: [RFC PATCH v2 0/2] ima: Fix detection of read/write violations on stacked filesystems

From: Amir Goldstein
Date: Tue Apr 23 2024 - 10:31:35 EST

Next message: André Draszik: "[PATCH 0/3] HSI0 support for Google Tensor gs101"
Previous message: Alan Maguire: "Re: [PATCH] libbpf: extending BTF_KIND_INIT to accommodate some unusual types"
In reply to: Roberto Sassu: "Re: [RFC PATCH v2 0/2] ima: Fix detection of read/write violations on stacked filesystems"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Tue, Apr 23, 2024 at 4:21 PM Roberto Sassu
<roberto.sassu@xxxxxxxxxxxxxxx> wrote:
>
> On Tue, 2024-04-23 at 09:02 +0300, Amir Goldstein wrote:
> > On Mon, Apr 22, 2024 at 6:07 PM Stefan Berger <stefanb@xxxxxxxxxxxxx> wrote:
> > >
> > > This series fixes the detection of read/write violations on stacked
> > > filesystems. To be able to access the relevant dentries necessary to
> > > detect files opened for writing on a stacked filesystem a new d_real_type
> > > D_REAL_FILEDATA is introduced that allows callers to access all relevant
> > > files involved in a stacked filesystem while traversing the layers.
> > >
> >
> > Stefan,
> >
> > Both Miklos and myself objected to this solution:
> > https://lore.kernel.org/linux-unionfs/CAJfpeguctirEYECoigcAsJwpGPCX2NyfMZ8H8GHGW-0UyKfjgg@xxxxxxxxxxxxxx/
> >
> > Not sure what you are hoping to achieve from re-posting the same solution.
> >
> > I stopped counting how many times I already argued that *all* IMA/EVM
> > assertions,
> > including rw-ro violations should be enforced only on the real inode.
>
> I have hopefully a better idea. We should detect violations at each
> level of the stack independently. And IMA should be invoked each time
> overlayfs uses an underlying layer.
>
> That is currently not easy, from the IMA policy perspective, because
> there are filesystem-specific rules, such as fsname= or fsuuid=. At the
> moment, I'm not planning to solve this, but I'm thinking to use for
> example FMODE_BACKING to ignore the filesystem-specific keywords and
> match the rule anyway.
>
> For now, I'm only addressing the call to underlying layers. To make
> sure that IMA evaluates every layer, I added a rule that checks the
> inode UID:
>
> measure fowner=2000 mask=MAY_READ
>
>
> I just investigated a bit, and I made some changes (for now, I'm just
> making it work, and you tell me what you think).

I did not examine this up close, but this seems like a change in the right
direction.
Will need Christian's approval that this does not break any assumptions
made on backing files.

Thanks,
Amir.

>
> diff --git a/fs/backing-file.c b/fs/backing-file.c
> index 740185198db3..8016f62cf770 100644
> --- a/fs/backing-file.c
> +++ b/fs/backing-file.c
> @@ -12,6 +12,7 @@
> #include <linux/backing-file.h>
> #include <linux/splice.h>
> #include <linux/mm.h>
> +#include <linux/security.h>
>
> #include "internal.h"
>
> @@ -40,12 +41,16 @@ struct file *backing_file_open(const struct path
> *user_path, int flags,
> if (IS_ERR(f))
> return f;
>
> + f->f_mode |= OPEN_FMODE(flags);
> +
> path_get(user_path);
> *backing_file_user_path(f) = *user_path;
> error = vfs_open(real_path, f);
> if (error) {
> fput(f);
> f = ERR_PTR(error);
> + } else {
> + security_file_post_open(f, ACC_MODE(flags));
> }
>
> return f;
>
>
> Setup:
>
> # mount -t overlay -olowerdir=a,upperdir=b,workdir=c overlay d
>
> open is a tool with the following syntax:
>
> open <path> <perm>
>
> It performs the open, and waits for user input before closing the file.
>
>
>
> ToMToU (Time of Measurement - Time of Use):
>
> Same fs (overlayfs)
>
> # /root/open /root/test-dir/d/test-file r (terminal 1)
> # /root/open /root/test-dir/d/test-file w (terminal 2)
>
> This works:
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 cef529d5d1032ffb6d3e2154664c83ba18cf2576 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 test-file
> 10 694277487b9753db78446192231b59b7be7c03ad ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/d/test-file
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 test-file
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 /root/test-dir/d/test-file
>
> This is the result of calling IMA at both layers, and the violation of
> course happens twice.
>
> This is also confirmed in the logs:
>
> Apr 23 14:52:45 fedora audit[994]: INTEGRITY_PCR pid=994 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=ToMToU comm="open" name="test-file" dev="sda3" ino=995512 res=1 errno=0
> Apr 23 14:52:45 fedora audit[994]: INTEGRITY_PCR pid=994 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=ToMToU comm="open" name="/root/test-dir/d/test-file" dev="overlay" ino=995512 res=1 errno=0
>
>
> Different fs (overlayfs, btrfs)
>
> # /root/open /root/test-dir/d/test-file r (terminal 1)
> # /root/open /root/test-dir/b/test-file w (terminal 2)
>
> Again, this works despite the read is in overlayfs, and the write is in
> btrfs:
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 cef529d5d1032ffb6d3e2154664c83ba18cf2576 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 test-file
> 10 694277487b9753db78446192231b59b7be7c03ad ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/d/test-file
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 /root/test-dir/b/test-file
>
> The difference from the previous example is that now there is only one
> violation, which is detected only in the upper layer. The logs have:
>
> Apr 23 15:01:15 fedora audit[985]: INTEGRITY_PCR pid=985 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=ToMToU comm="open" name="/root/test-dir/b/test-file" dev="sda3" ino=995512 res=1 errno=0
>
>
> Different fs (btrfs, overlayfs)
>
> # /root/open /root/test-dir/b/test-file r (terminal 2)
> # /root/open /root/test-dir/d/test-file w (terminal 1)
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 d7a692e19158820d2755542a8d31b49ac7ac2729 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/b/test-file
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 test-file
>
> Works too. There is only one measurement, since that is done only for
> the upper layer.
>
> Apr 23 15:05:40 fedora audit[982]: INTEGRITY_PCR pid=982 uid=0 auid=0 ses=1 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=ToMToU comm="open" name="test-file" dev="sda3" ino=995512 res=1 errno=0
>
>
>
> Open writers
>
> Same fs (overlayfs)
>
> # /root/open /root/test-dir/d/test-file w (terminal 1)
> # /root/open /root/test-dir/d/test-file r (terminal 2)
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 test-file
> 10 cef529d5d1032ffb6d3e2154664c83ba18cf2576 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 test-file
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 /root/test-dir/d/test-file
> 10 694277487b9753db78446192231b59b7be7c03ad ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/d/test-file
>
> Apr 23 15:10:46 fedora audit[983]: INTEGRITY_PCR pid=983 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=open_writers comm="open" name="test-file" dev="sda3" ino=995512 res=1 errno=0
> Apr 23 15:10:46 fedora audit[983]: INTEGRITY_PCR pid=983 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=open_writers comm="open" name="/root/test-dir/d/test-file" dev="overlay" ino=995512 res=1 errno=0
>
>
> Different fs (overlayfs, btrfs)
>
> # /root/open /root/test-dir/d/test-file w (terminal 1)
> # /root/open /root/test-dir/b/test-file r (terminal 2)
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 /root/test-dir/b/test-file
> 10 d7a692e19158820d2755542a8d31b49ac7ac2729 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/b/test-file
>
> Apr 23 15:12:58 fedora audit[984]: INTEGRITY_PCR pid=984 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=open_writers comm="open" name="/root/test-dir/b/test-file" dev="sda3" ino=995512 res=1 errno=0
>
>
> Different fs (btrfs, overlayfs)
>
> # /root/open /root/test-dir/b/test-file w (terminal 1)
> # /root/open /root/test-dir/d/test-file r (terminal 2)
>
> 10 35435d0858d895b90097306171a2e5fcc7f5da9e ima-ng sha256:0e4acf326a82c6bded9d86f48d272d7a036b6490081bb6466ecc2a0e416b244a boot_aggregate
> 10 0000000000000000000000000000000000000000 ima-ng sha256:0000000000000000000000000000000000000000000000000000000000000000 test-file
> 10 cef529d5d1032ffb6d3e2154664c83ba18cf2576 ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 test-file
> 10 694277487b9753db78446192231b59b7be7c03ad ima-ng sha256:f2ca1bb6c7e907d06dafe4687e579fce76b37e4e93b7605022da52e6ccc26fd2 /root/test-dir/d/test-file
>
> Apr 23 15:16:37 fedora audit[983]: INTEGRITY_PCR pid=983 uid=0 auid=0 ses=3 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 op=invalid_pcr cause=open_writers comm="open" name="test-file" dev="sda3" ino=995512 res=1 errno=0
>
> Roberto
>
> > I know this does not work - so you should find out why it does not work and fix
> > the problem.
> >
> > Enforcing IMA/EVM on the overlayfs inode layer is just the wrong way IMO.
> > Not once have I heard an argument from IMA/EVM developers why it is really
> > needed to enforce IMA/EVM on the overlayfs inode layer and not on the
> > real inode.
> > I am sorry that we are failing to communicate on this matter, but I am not
> > sure how else I can help.
> >
> > Thanks,
> > Amir.
>

Next message: André Draszik: "[PATCH 0/3] HSI0 support for Google Tensor gs101"
Previous message: Alan Maguire: "Re: [PATCH] libbpf: extending BTF_KIND_INIT to accommodate some unusual types"
In reply to: Roberto Sassu: "Re: [RFC PATCH v2 0/2] ima: Fix detection of read/write violations on stacked filesystems"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]