linux-kernel - Re: [RFC][PATCH 5/5] shmem: Add fsverity support

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YZKvXK+vX/we4GCD@gmail.com>
Date:   Mon, 15 Nov 2021 11:05:00 -0800
From:   Eric Biggers <ebiggers@...nel.org>
To:     Roberto Sassu <roberto.sassu@...wei.com>
Cc:     "tytso@....edu" <tytso@....edu>, "corbet@....net" <corbet@....net>,
        "viro@...iv.linux.org.uk" <viro@...iv.linux.org.uk>,
        "hughd@...gle.com" <hughd@...gle.com>,
        "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        "linux-fscrypt@...r.kernel.org" <linux-fscrypt@...r.kernel.org>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC][PATCH 5/5] shmem: Add fsverity support

On Mon, Nov 15, 2021 at 08:49:41AM +0000, Roberto Sassu wrote:
> > From: Eric Biggers [mailto:ebiggers@...nel.org]
> > Sent: Friday, November 12, 2021 8:12 PM
> > On Fri, Nov 12, 2021 at 01:44:11PM +0100, Roberto Sassu wrote:
> > > Make the necessary modifications to support fsverity in tmpfs.
> > >
> > > First, implement the fsverity operations (in a similar way of f2fs). These
> > > operations make use of shmem_read_mapping_page() instead of
> > > read_mapping_page() to handle the case where the page has been swapped
> > out.
> > > The fsverity descriptor is placed at the end of the file and its location
> > > is stored in an xattr.
> > >
> > > Second, implement the ioctl operations to enable, measure and read fsverity
> > > metadata.
> > >
> > > Lastly, add calls to fsverity functions, to ensure that fsverity-relevant
> > > operations are checked and handled by fsverity (file open, attr set, inode
> > > evict).
> > >
> > > Fsverity support can be enabled through the kernel configuration and
> > > remains enabled by default for every tmpfs filesystem instantiated (there
> > > should be no overhead, unless fsverity is enabled for a file).
> > >
> > > Signed-off-by: Roberto Sassu <roberto.sassu@...wei.com>
> > 
> > I don't see how this makes sense at all.  The point of fs-verity is to avoid
> > having to hash the whole file when verifying it.  However, obviously the whole
> > file still has to be hashed to build the Merkle tree in the first place.  That
> > makes sense for a persistent filesystem where a file can be written once and
> > verified many times.  I don't see how it makes sense for tmpfs, where files have
> > to be re-created on every boot.  You might as well just hash the whole file.
> 
> The point of adding fsverity support for tmpfs was to being able to do
> integrity enforcement with just one mechanism, given that I was
> planning to do integrity verification with reference values loaded
> to the kernel with DIGLIM [1].
> 
> With an LSM such as IPE [2], integrity verification would consist in
> querying the fsverity digest with DIGLIM and allowing the operation
> if the digest was found. With fsverity support in tmpfs, this can be
> done from the very beginning of the boot process.
> 
> Using regular file digests would be also possible but this requires
> loading with DIGLIM both fsverity and non-fsverity reference values.
> It would also require two separate mechanisms for calculating
> the file digest depending on the filesystem. It could be done, but
> I thought it was easier to add support for fsverity in tmpfs.
> 
> > Also, you didn't implement actually verifying the data (by calling
> > fsverity_verify_page()), so this patch doesn't really do anything anyway.
> 
> Yes, at the end I didn't add it. Probably the only place where
> calling fsverity_verify_page() would make sense is when a page
> is swapped in (assuming that the swap device is untrusted).
> 
> I tried to add a call in shmem_swapin_page() but fsverity complained
> due to the fact that the page was already up to date, and also
> rejected the page. I will check it better.
> 

It sounds like you really only care about calculating fs-verity file digests.
That's just an algorithm for hashing a file, so it could just be implemented in
generic code that operates on any file on any filesystem, like how IMA
implemennts full file hashing for any file.  There isn't a need for any special
filesystem support to do this.

- Eric