RE: [RFC PATCH v7 12/16] fsverity|security: add security hooks to fsverity digest and signature

Roberto Sassu
2021-10-22T16:31:13Z

> From: Roberto Sassu [mailto:roberto.sassu@huawei.com]
> Sent: Wednesday, October 20, 2021 5:09 PM
> > From: Eric Biggers [mailto:ebiggers@kernel.org]
> > Sent: Friday, October 15, 2021 10:11 PM
> > On Fri, Oct 15, 2021 at 12:25:53PM -0700, Deven Bowers wrote:
> > >
> > > On 10/13/2021 12:24 PM, Eric Biggers wrote:
> > > > On Wed, Oct 13, 2021 at 12:06:31PM -0700,
> > deven.desai@linux.microsoft.com wrote:
> > > > > From: Fan Wu <wufan@linux.microsoft.com>
> > > > >
> > > > > Add security_inode_setsecurity to fsverity signature verification.
> > > > > This can let LSMs save the signature data and digest hashes provided
> > > > > by fsverity.
> > > > Can you elaborate on why LSMs need this information?
> > >
> > > The proposed LSM (IPE) of this series will be the only one to need
> > > this information at the  moment. IPE’s goal is to have provide
> > > trust-based access control. Trust and Integrity are tied together,
> > > as you cannot prove trust without proving integrity.
> >
> > I think you mean authenticity, not integrity?
> >
> > Also how does this differ from IMA? I know that IMA doesn't support fs-verity
> > file hashes, but that could be changed. Why not extend IMA to cover your use
> > case(s)?
> >
> > > IPE needs the digest information to be able to compare a digest
> > > provided by the policy author, against the digest calculated by
> > > fsverity to make a decision on whether that specific file, represented
> > > by the digest is authorized for the actions specified in the policy.
> > >
> > > A more concrete example, if an IPE policy author writes:
> > >
> > >     op=EXECUTE fsverity_digest=<HexDigest > action=DENY
> > >
> > > IPE takes the digest provided by this security hook, stores it
> > > in IPE's security blob on the inode. If this file is later
> > > executed, IPE compares the digest stored in the LSM blob,
> > > provided by this hook, against <HexDigest> in the policy, if
> > > it matches, it denies the access, performing a revocation
> > > of that file.
> >
> > Do you have a better example? This one is pretty useless since one can get
> > around it just by executing a file that doesn't have fs-verity enabled.
>
> I was wondering if the following use case can be supported:
> allow the execution of files protected with fsverity if the root
> digest is found among reference values (instead of providing
> them one by one in the policy).
>
> Something like:
>
> op=EXECUTE fsverity_digest=diglim action=ALLOW

Looks like it works. I modified IPE to query the root digest
of an fsverity-protected file in DIGLIM.

# cat ipe-policy
policy_name="AllowFSVerityKmodules" policy_version=0.0.1
DEFAULT action=ALLOW
DEFAULT op=KMODULE action=DENY
op=KMODULE fsverity_digest=diglim action=ALLOW

IPE setup:
# cat ipe-policy.p7s > /sys/kernel/security/ipe/new_policy
# echo -n 1 > /sys/kernel/security/ipe/policies/AllowFSVerityKmodules/active
# echo 1 > /sys/kernel/security/ipe/enforce

IPE denies loading of kernel modules not protected by fsverity:
# insmod /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko
insmod: ERROR: could not insert module /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko: Permission denied

Protect fat.ko with fsverity:
# cp /lib/modules/5.15.0-rc1+/kernel/fs/fat/fat.ko /fsverity
# fsverity enable /fsverity/fat.ko
# fsverity measure /fsverity/fat.ko
sha256:079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 /fsverity/fat.ko

IPE still denies the loading of fat.ko (root digest not uploaded to the kernel):
# insmod /fsverity/fat.ko
insmod: ERROR: could not insert module /fsverity/fat.ko: Permission denied

Generate a digest list with the root digest above and upload it to the kernel:
# ./compact_gen -i 079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 -a sha256 -d test -s -t file -f
# echo $PWD/test/0-file_list-compact-079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe1547803 > /sys/kernel/security/integrity/diglim/digest_list_add

IPE allows the loading of fat.ko:
# insmod /fsverity/fat.ko
#

Regarding authenticity, not shown in this demo, IPE will also
ensure that the root digest is signed (diglim_digest_get_info()
reports this information).

Roberto

HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
Managing Director: Li Peng, Zhong Ronghua

> DIGLIM is a component I'm working on that generically
> stores digests. The current use case is to store file digests
> from RPMTAG_FILEDIGESTS and use them with IMA, but
> the fsverity use case could be easily supported (if the root
> digest is stored in the RPM header).
>
> DIGLIM also tells whether or not the signature of the source
> containing file digests (or fsverity digests) is valid (the signature
> of the RPM header is taken from RPMTAG_RSAHEADER).
>
> The memory occupation is relatively small for executables
> and shared libraries. I published a demo for Fedora and
> openSUSE some time ago:
>
> https://lore.kernel.org/linux-
> integrity/48cd737c504d45208377daa27d625531@huawei.com/
>
> Thanks
>
> Roberto
>
> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> Managing Director: Li Peng, Zhong Ronghua
>
> > > This brings me to your next comment:
> > >
> > > > The digest isn't meaningful without knowing the hash algorithm it uses.
> > > It's available here, but you aren't passing it to this function.
> > >
> > > The digest is meaningful without the algorithm in this case.
> >
> > No, it's not.
> >
> > Digests are meaningless without knowing what algorithm they were created
> > with.
> >
> > If your security policy is something like "Trust the file with digest $foo" and
> > multiple hash algorithms are possible, then the alorithm intended to be used
> > needs to be explicitly specified. Otherwise any algorithm with the same length
> > digest will be accepted. That's a fatal flaw if any of these algorithms is
> > cryptographically broken or was never intended to be a cryptographic
> algorithm
> > in the first place (e.g., a non-cryptographic checksum).
> >
> > Cryptosystems always need to specify the crypto algorithm(s) used; the
> > adversary
> > must not be allowed to choose the algorithms.
> >
> > I'm not sure how these patches can be taken seriously when they're getting
> this
> > sort of thing wrong.
> >
> > > > > +
> FS_VERITY_SIGNATURE_SEC_NAME,
> > > > > + signature, sig_size, 0);
> > > > This is only for fs-verity built-in signatures which aren't the only way to do
> > > > signatures with fs-verity. Are you sure this is what you're looking for?
> > >
> > > Could you elaborate on the other signature types that can be used
> > > with fs-verity? I’m 99% sure this is what I’m looking for as this
> > > is a signature validated in the kernel against the fs-verity keyring
> > > as part of the “fsverity enable” utility.
> > >
> > > It's important that the signature is validated in the kernel, as
> > > userspace is considered untrusted until the signature is validated
> > > for this case.
> > >
> > > > Can you elaborate on your use case for fs-verity built-in signatures,
> > > Sure, signatures, like digests, also provide a way to prove integrity,
> > > and the trust component comes from the validation against the keyring,
> > > as opposed to a fixed value in IPE’s policy. The use case for fs-verity
> > > built-in signatures is that we have a rw ext4 filesystem that has some
> > > executable files, and we want to have a execution policy (through IPE)
> > > that only _trusted_ executables can run. Perf is important here, hence
> > > fs-verity.
> >
> > Most users of fs-verity built-in signatures have actually been enforcing their
> > security policy in userspace, by checking whether specific files have the
> > fs-verity bit set or not. Such users could just store and verify signatures in
> > userspace instead, without any kernel involvement. So that's what I've been
> > recommending (with limited success, unfortunately).
> >
> > If you really do need in-kernel signature verification, then that may be a
> > legitimate use case for the fs-verity built-in signatures, although I do wonder
> > why you aren't using IMA and its signature mechanism instead.
> >
> > - Eric