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

Commit Message

Deven Bowers Oct. 13, 2021, 7:06 p.m. UTC

From: Fan Wu <[e-mail]>

Add security_inode_setsecurity to fsverity signature verification.
This can let LSMs save the signature data and digest hashes provided
by fsverity.

Also changes the implementaion inside the hook function to let
multiple LSMs can add hooks.

Signed-off-by: Fan Wu <[e-mail]>
Signed-off-by: Deven Bowers <[e-mail]>
---
 fs/verity/open.c         | 12 ++++++++++++
 fs/verity/signature.c    |  5 ++++-
 include/linux/fsverity.h |  3 +++
 security/ipe/hooks.c     |  1 +
 security/security.c      |  6 +++---
 5 files changed, 23 insertions(+), 4 deletions(-)

Comments

Eric Biggers Oct. 13, 2021, 7:24 p.m. UTC | #1

On Wed, Oct 13, 2021 at 12:06:31PM -0700, [e-mail] wrote:
> From: Fan Wu <[e-mail]>
> 
> 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?

> 
> Also changes the implementaion inside the hook function to let
> multiple LSMs can add hooks.

Please split fs/verity/ changes and security/ changes into separate patches, if
possible.

> 
> Signed-off-by: Fan Wu <[e-mail]>
> Signed-off-by: Deven Bowers <[e-mail]>

> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
>  		fsverity_err(inode, "Error %d computing file digest", err);
>  		goto out;
>  	}
> +
> +	err = security_inode_setsecurity((struct inode *)inode,

If a non-const inode is needed, please propagate that into the callers rather
than randomly casting away the const.

> +					 FS_VERITY_DIGEST_SEC_NAME,
> +					 vi->file_digest,
> +					 vi->tree_params.hash_alg->digest_size,
> +					 0);

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.

> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
>  
>  	pr_debug("Valid signature for file digest %s:%*phN\n",
>  		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
> -	return 0;
> +	return security_inode_setsecurity((struct inode *)inode,

Likewise, please don't cast away const.

> +					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?  Can
you elaborate on your use case for fs-verity built-in signatures, and what the
LSM hook will do with them?

- Eric

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Deven Bowers Oct. 15, 2021, 7:25 p.m. UTC | #2

On 10/13/2021 12:24 PM, Eric Biggers wrote:
> On Wed, Oct 13, 2021 at 12:06:31PM -0700, [e-mail] wrote:
>> From: Fan Wu <[e-mail]>
>>
>> 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.

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.

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.
IPE does not want to recalculate a digest, that’s expensive and
doesn’t provide any value. IPE, in this case, treats this as a
buffer to compare the policy-provided one above to make a
policy decision about access to the resource.

>> Also changes the implementaion inside the hook function to let
>> multiple LSMs can add hooks.
> Please split fs/verity/ changes and security/ changes into separate patches, if
> possible.

Sorry, will do, not a problem.

>> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
>>   		fsverity_err(inode, "Error %d computing file digest", err);
>>   		goto out;
>>   	}
>> +
>> +	err = security_inode_setsecurity((struct inode *)inode,
> If a non-const inode is needed, please propagate that into the callers rather
> than randomly casting away the const.
>
>> +					 FS_VERITY_DIGEST_SEC_NAME,
>> +					 vi->file_digest,
>> +					 vi->tree_params.hash_alg->digest_size,
>> +					 0);
>> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
>>   
>>   	pr_debug("Valid signature for file digest %s:%*phN\n",
>>   		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
>> -	return 0;
>> +	return security_inode_setsecurity((struct inode *)inode,
>>
> Likewise, please don't cast away const.

Sorry, I should've caught these myself. I'll change
fsverity_create_info to accept the non-const inode, and
change fsverity_verify_signature to accept an additional inode
struct as the first arg instead of changing the fsverity_info
structure to have a non-const inode field.

>> +					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.

> and what the LSM hook will do with them?

At the moment, this will just signal to IPE that these fs-verity files were
enabled with a built-in signature as opposed to enabled without a signature.
In v7, it copies the signature data into IPE's LSM blob attached to the 
inode.
In v8+, I'm changing this to store “true” in IPE's LSM blob instead, as 
copying
the signature data is an unnecessary waste of space and point of 
failure. This
has a _slightly_ different functionality then fs.verity.require_signatures,
because even if someone were to disable the require signatures option, IPE
would still know if these files were signed or not and be able to make the
access control decision based IPE's policy.

Very concretely, this powers this kind of rule in IPE:

   op=EXECUTE fsverity_signature=TRUE action=ALLOW

if that fsverity_signature value in IPE’s LSM blob attached to the inode is
true, then fsverity_signature in IPE’s policy will evaluate to true and 
match
this rule. The inverse is also applicable.


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Eric Biggers Oct. 15, 2021, 8:11 p.m. UTC | #3

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, [e-mail] wrote:
> > > From: Fan Wu <[e-mail]>
> > > 
> > > 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.

> 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

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Roberto Sassu Oct. 20, 2021, 3:08 p.m. UTC | #4

> From: Eric Biggers [mailto:[e-mail]]
> 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,
> [e-mail] wrote:
> > > > From: Fan Wu <[e-mail]>
> > > >
> > > > 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

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/[e-mail]/

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

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Roberto Sassu Oct. 22, 2021, 4:31 p.m. UTC | #5

> From: Roberto Sassu [mailto:[e-mail]]
> Sent: Wednesday, October 20, 2021 5:09 PM
> > From: Eric Biggers [mailto:[e-mail]]
> > 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,
> > [e-mail] wrote:
> > > > > From: Fan Wu <[e-mail]>
> > > > >
> > > > > 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/[e-mail]/
> 
> 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

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Deven Bowers Oct. 26, 2021, 7:03 p.m. UTC | #6

On 10/22/2021 9:31 AM, Roberto Sassu wrote:
>> From: Roberto Sassu [mailto:[e-mail]]
>> Sent: Wednesday, October 20, 2021 5:09 PM
>>> From: Eric Biggers [mailto:[e-mail]]
>>> 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,
>>> [e-mail]  wrote:
>>>>>> From: Fan Wu<[e-mail]>
>>>>>>
>>>>>> 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).

I apologize for the delay in responses, but it looks like
you've figured it out.

This kind of thing is exactly what IPE's design is supposed to
solve, you have some other system which provides the
integrity mechanism and (optionally) determine if it is trusted or
not, and IPE can provide the policy aspect very easily to
make a set of system-wide requirements around your mechanism.

I'm very supportive of adding the functionality, but I wonder
if it makes more sense to have digilm's extension be a separate
key instead of tied to the fsverity_digest key - something like

    op=EXECUTE diglim_fsverity=TRUE action=DENY

that way the condition that enables this property can depend
on digilm in the build, and it separates the two systems'
integrations in a slightly more clean way.

As an aside, did you find it difficult to extend?

> 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/[e-mail]/
>>
>> Thanks
>>
>> Roberto
>>
>> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
>> Managing Director: Li Peng, Zhong Ronghua


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Deven Bowers Oct. 26, 2021, 7:03 p.m. UTC | #7

On 10/15/2021 1:11 PM, Eric Biggers wrote:

> 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,[e-mail]  wrote:
>>>> From: Fan Wu<[e-mail]>
>>>>
>>>> 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?
I’ve heard a lot of people use these terms in overloaded ways.

If we’re working with the definition of authenticity being
“the property that a resource was _actually_ sent/created by a
party”, and integrity being “the property that a resource was not
modified from a point of time”, then yes. Though the statement isn’t
false, though, because you’d need to prove integrity in the process of
proving authenticity.

If not, could you clarify what you mean by authenticity and integrity,
so that we can use consistent definitions?
> 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)?
We looked at extending IMA to cover our requirements extensively the 
past year
based on feedback the last time I posted these patches. We implemented a
prototype that had half of our requirements, but found it resulted in a
large change list that would result in a large amount of pain in respect
to maintenance, in addition to other more architectural concerns about the
implementation. We weren’t convinced it was the correct direction, for our
needs.

There was a presentation done at LSS 2021 around this prototype done by my
colleague, Fan, who authored this patch and implemented the aforementioned
prototype.

In general, IMA provides a whole suite of amazing functionality when it
comes to everything integrity, as the fs-verity documentation states
itself:

    IMA specifies a system-wide policy that specifies which
    files are hashed and what to do with those hashes, such
    as log them, authenticate them, or add them to a
    measurement list.

Instead, IPE provides a fine-tuned way to _only_ enforce an access control
policy to these files based on the defined trust requirements in the policy,
under various contexts, (you might have different requirements for what
executes in a general purpose, versus loadable kernel modules, for example).
It will never provide bother to log, measure, or revalidate these hashes 
because
that’s not its purpose. This is why it belongs at the LSM layer instead 
of the
integrity subsystem layer, as it is providing access control based on a 
policy,
versus providing deep integrations with the actual integrity claim.

IPE is trying to be agnostic to how precisely “trust” is provided, as
opposed to be deeply integrated into the mechanism that provides
“trust”.
>> 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.
Here’s a more complete example:

    policy_name=”fs-exec-only” policy_version=0.0.1
    DEFAULT action=ALLOW

    DEFAULT op=EXECUTE action=DENY
    op=EXECUTE fsverity_digest=<Digest> action=DENY
    op=EXECUTE fsverity_signature=TRUE action=ALLOW

Execution is prohibited unless it is a signed fs-verity file;
However, after one of those executables was signed and published,
an exploitable vulnerability in said executable was found, a new
version was published without that vulnerability. We need to
revoke trust for that executable since it could be used to exploit
the system, so the first rule prevents it from matching the second.
>> 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.
Oof. You’re completely right. The part I was missing is that as time 
goes on,
the secure status of these cryptographic algorithms will change, and 
then we’ll
need a way to migrate between algorithms. Additionally, tooling and the 
like will
likely need a way to identify this from the policy text without 
consulting anything
else. This is a major oversight for general use, the system that this 
was originally
designed for only had support for a subset of the sha2-family (all 
separate lengths)
so I hadn’t even considered it.

It's trivial to correct in a minimal amount of code, making the policy 
express the
digest like so:

    fsverity_digest=<algo>:<digest>

and change the argument passed to the LSM hook to accept a structure 
containing these
two fields.

> I'm not sure how these patches can be taken seriously when they're getting this
> sort of thing wrong.
That said, I, personally, hope that an honest mistake, in a series 
submitted as
an RFC submitted in good faith, is not a reason to discount an entire patch
series.

I hope you continue to provide feedback, as it is invaluable to making this
system better, and making me, personally, a better developer.
>>>> +					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).
I believe the difference in security models comes from this line
(emphasis, mine):

 > by checking whether _specific files_ have the fs-verity bit set or not.

IPE policy is written by a system author who owns the system, but may
not have 100% control over all of the application code running on the
system.  In the case of applications which are not aware of IPE, the policy
can still enforce that all of the code running on the system is trusted.

An example attack of what we're trying to mitigate:  A hostile actor
could downloads a binary off the internet with all required
dependencies into tmpfs and runs their malicious executable.

With us validating this information in the kernel, even if the attacker
downloaded their malicious executable to /tmp and executed it, it would
still fail to pass policy and be denied, as the kernel is the common
entrypoint across all executables.

Operationally, this _could_ be done by digest, but the policies would
quickly become gigantic on a cartoonish proportion, as you'll have to
authorize every single executable and dependency by digest - and
there would be a complicated update story as the policy would have to
be updated to onboard new digests.

By using signatures, we can prevent the policy update, and keep the
policy size small.

> 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


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Roberto Sassu Oct. 27, 2021, 8:41 a.m. UTC | #8

> From: Deven Bowers [mailto:[e-mail]]
> Sent: Tuesday, October 26, 2021 9:04 PM
> On 10/22/2021 9:31 AM, Roberto Sassu wrote:
> >> From: Roberto Sassu [mailto:[e-mail]]
> >> Sent: Wednesday, October 20, 2021 5:09 PM
> >>> From: Eric Biggers [mailto:[e-mail]]
> >>> 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,
> >>> [e-mail]  wrote:
> >>>>>> From: Fan Wu<[e-mail]>
> >>>>>>
> >>>>>> 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:079be6d88638e58141ee24bba89813917c44faa55ada4bf5d80335efe154
> 7803 /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).
> 
> I apologize for the delay in responses, but it looks like
> you've figured it out.

No problem.

> This kind of thing is exactly what IPE's design is supposed to
> solve, you have some other system which provides the
> integrity mechanism and (optionally) determine if it is trusted or
> not, and IPE can provide the policy aspect very easily to
> make a set of system-wide requirements around your mechanism.
> 
> I'm very supportive of adding the functionality, but I wonder
> if it makes more sense to have digilm's extension be a separate
> key instead of tied to the fsverity_digest key - something like
> 
>     op=EXECUTE diglim_fsverity=TRUE action=DENY
> 
> that way the condition that enables this property can depend
> on digilm in the build, and it separates the two systems'
> integrations in a slightly more clean way.

Yes, I agree. It is much more clean.

> As an aside, did you find it difficult to extend?

No, it was very straightforward.

Roberto

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

> > 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/[e-mail]/
> >>
> >> Thanks
> >>
> >> Roberto
> >>
> >> HUAWEI TECHNOLOGIES Duesseldorf GmbH, HRB 56063
> >> Managing Director: Li Peng, Zhong Ronghua

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Roberto Sassu Oct. 27, 2021, 9:34 a.m. UTC | #9

> From: Deven Bowers [mailto:[e-mail]]
> Sent: Tuesday, October 26, 2021 9:04 PM
> On 10/15/2021 1:11 PM, Eric Biggers wrote:
> 
> > 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,[e-mail]  wrote:
> >>>> From: Fan Wu<[e-mail]>
> >>>>
> >>>> 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?
> I’ve heard a lot of people use these terms in overloaded ways.
> 
> If we’re working with the definition of authenticity being
> “the property that a resource was _actually_ sent/created by a
> party”, and integrity being “the property that a resource was not
> modified from a point of time”, then yes. Though the statement isn’t
> false, though, because you’d need to prove integrity in the process of
> proving authenticity.
> 
> If not, could you clarify what you mean by authenticity and integrity,
> so that we can use consistent definitions?
> > 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)?
> We looked at extending IMA to cover our requirements extensively the
> past year
> based on feedback the last time I posted these patches. We implemented a
> prototype that had half of our requirements, but found it resulted in a
> large change list that would result in a large amount of pain in respect
> to maintenance, in addition to other more architectural concerns about the
> implementation. We weren’t convinced it was the correct direction, for our
> needs.
> 
> There was a presentation done at LSS 2021 around this prototype done by my
> colleague, Fan, who authored this patch and implemented the aforementioned
> prototype.
> 
> In general, IMA provides a whole suite of amazing functionality when it
> comes to everything integrity, as the fs-verity documentation states
> itself:
> 
>     IMA specifies a system-wide policy that specifies which
>     files are hashed and what to do with those hashes, such
>     as log them, authenticate them, or add them to a
>     measurement list.
> 
> Instead, IPE provides a fine-tuned way to _only_ enforce an access control
> policy to these files based on the defined trust requirements in the policy,
> under various contexts, (you might have different requirements for what
> executes in a general purpose, versus loadable kernel modules, for example).
> It will never provide bother to log, measure, or revalidate these hashes
> because
> that’s not its purpose. This is why it belongs at the LSM layer instead
> of the
> integrity subsystem layer, as it is providing access control based on a
> policy,
> versus providing deep integrations with the actual integrity claim.
> 
> IPE is trying to be agnostic to how precisely “trust” is provided, as
> opposed to be deeply integrated into the mechanism that provides
> “trust”.
> >> 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.
> Here’s a more complete example:
> 
>     policy_name=”fs-exec-only” policy_version=0.0.1
>     DEFAULT action=ALLOW
> 
>     DEFAULT op=EXECUTE action=DENY
>     op=EXECUTE fsverity_digest=<Digest> action=DENY
>     op=EXECUTE fsverity_signature=TRUE action=ALLOW
> 
> Execution is prohibited unless it is a signed fs-verity file;
> However, after one of those executables was signed and published,
> an exploitable vulnerability in said executable was found, a new
> version was published without that vulnerability. We need to
> revoke trust for that executable since it could be used to exploit
> the system, so the first rule prevents it from matching the second.

With DIGLIM, revocation will be completely transparent since
digests of files in the packages being removed will be also removed
from the kernel. The next time IPE will evaluate an old version of the
file (assuming that there was a copy somewhere) it will realize that
the digest is not known anymore and will deny access.

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.
> Oof. You’re completely right. The part I was missing is that as time
> goes on,
> the secure status of these cryptographic algorithms will change, and
> then we’ll
> need a way to migrate between algorithms. Additionally, tooling and the
> like will
> likely need a way to identify this from the policy text without
> consulting anything
> else. This is a major oversight for general use, the system that this
> was originally
> designed for only had support for a subset of the sha2-family (all
> separate lengths)
> so I hadn’t even considered it.
> 
> It's trivial to correct in a minimal amount of code, making the policy
> express the
> digest like so:
> 
>     fsverity_digest=<algo>:<digest>
> 
> and change the argument passed to the LSM hook to accept a structure
> containing these
> two fields.
> 
> > I'm not sure how these patches can be taken seriously when they're getting
> this
> > sort of thing wrong.
> That said, I, personally, hope that an honest mistake, in a series
> submitted as
> an RFC submitted in good faith, is not a reason to discount an entire patch
> series.
> 
> I hope you continue to provide feedback, as it is invaluable to making this
> system better, and making me, personally, a better developer.
> >>>> +					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).
> I believe the difference in security models comes from this line
> (emphasis, mine):
> 
>  > by checking whether _specific files_ have the fs-verity bit set or not.
> 
> IPE policy is written by a system author who owns the system, but may
> not have 100% control over all of the application code running on the
> system.  In the case of applications which are not aware of IPE, the policy
> can still enforce that all of the code running on the system is trusted.
> 
> An example attack of what we're trying to mitigate:  A hostile actor
> could downloads a binary off the internet with all required
> dependencies into tmpfs and runs their malicious executable.
> 
> With us validating this information in the kernel, even if the attacker
> downloaded their malicious executable to /tmp and executed it, it would
> still fail to pass policy and be denied, as the kernel is the common
> entrypoint across all executables.
> 
> Operationally, this _could_ be done by digest, but the policies would
> quickly become gigantic on a cartoonish proportion, as you'll have to
> authorize every single executable and dependency by digest - and
> there would be a complicated update story as the policy would have to
> be updated to onboard new digests.
> 
> By using signatures, we can prevent the policy update, and keep the
> policy size small.
> 
> > 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

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Eric Biggers Oct. 28, 2021, 3:48 a.m. UTC | #10

On Tue, Oct 26, 2021 at 12:03:53PM -0700, Deven Bowers wrote:
> > > 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?
> I’ve heard a lot of people use these terms in overloaded ways.
> 
> If we’re working with the definition of authenticity being
> “the property that a resource was _actually_ sent/created by a
> party”, and integrity being “the property that a resource was not
> modified from a point of time”, then yes. Though the statement isn’t
> false, though, because you’d need to prove integrity in the process of
> proving authenticity.
> 
> If not, could you clarify what you mean by authenticity and integrity,
> so that we can use consistent definitions?

In cryptography, integrity normally means knowing whether data has been
non-maliciously changed, while authenticity means knowing whether data is from a
particular source, which implies knowing whether it has been changed at all
(whether maliciously or not).  Consider that there are "Message Authentication
Codes" (MACs) and "Authenticated Encryption", not "Message Integrity Codes" and
"Intact Encryption".

Unfortunately lots of people do overload "integrity" to mean authenticity, so
you're not alone.  But it's confusing, so if you're going to do that then please
make sure to clearly explain what you mean.

> > 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)?
> We looked at extending IMA to cover our requirements extensively the past
> year
> based on feedback the last time I posted these patches. We implemented a
> prototype that had half of our requirements, but found it resulted in a
> large change list that would result in a large amount of pain in respect
> to maintenance, in addition to other more architectural concerns about the
> implementation. We weren’t convinced it was the correct direction, for our
> needs.
> 
> There was a presentation done at LSS 2021 around this prototype done by my
> colleague, Fan, who authored this patch and implemented the aforementioned
> prototype.
> 
> In general, IMA provides a whole suite of amazing functionality when it
> comes to everything integrity, as the fs-verity documentation states
> itself:
> 
>    IMA specifies a system-wide policy that specifies which
>    files are hashed and what to do with those hashes, such
>    as log them, authenticate them, or add them to a
>    measurement list.
> 
> Instead, IPE provides a fine-tuned way to _only_ enforce an access control
> policy to these files based on the defined trust requirements in the policy,
> under various contexts, (you might have different requirements for what
> executes in a general purpose, versus loadable kernel modules, for example).
> It will never provide bother to log, measure, or revalidate these hashes
> because
> that’s not its purpose. This is why it belongs at the LSM layer instead of
> the
> integrity subsystem layer, as it is providing access control based on a
> policy,
> versus providing deep integrations with the actual integrity claim.
> 
> IPE is trying to be agnostic to how precisely “trust” is provided, as
> opposed to be deeply integrated into the mechanism that provides
> “trust”.

IMA doesn't require logging or "measuring" hashes, though.  Those are just some
of its supported features.  And I thought the IMA developers were planning to
add support for fs-verity hashes, and that it wouldn't require an entirely new
architecture to do so.

Anyway, while it does sound to me like you're duplicating IMA, I don't really
have a horse in this race, and I defer to the IMA developers on this.  I trust
that you've been engaging with them?  This patchset isn't even Cc'ed to
linux-integrity, so it's unclear that's been happening.

- Eric

--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Deven Bowers Oct. 28, 2021, 6:11 p.m. UTC | #11

On 10/27/2021 8:48 PM, Eric Biggers wrote:
> On Tue, Oct 26, 2021 at 12:03:53PM -0700, Deven Bowers wrote:
>>>> 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?
>> I’ve heard a lot of people use these terms in overloaded ways.
>>
>> If we’re working with the definition of authenticity being
>> “the property that a resource was _actually_ sent/created by a
>> party”, and integrity being “the property that a resource was not
>> modified from a point of time”, then yes. Though the statement isn’t
>> false, though, because you’d need to prove integrity in the process of
>> proving authenticity.
>>
>> If not, could you clarify what you mean by authenticity and integrity,
>> so that we can use consistent definitions?
> In cryptography, integrity normally means knowing whether data has been
> non-maliciously changed, while authenticity means knowing whether data is from a
> particular source, which implies knowing whether it has been changed at all
> (whether maliciously or not).  Consider that there are "Message Authentication
> Codes" (MACs) and "Authenticated Encryption", not "Message Integrity Codes" and
> "Intact Encryption".
>
> Unfortunately lots of people do overload "integrity" to mean authenticity, so
> you're not alone.  But it's confusing, so if you're going to do that then please
> make sure to clearly explain what you mean.
>
>>> 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)?
>> We looked at extending IMA to cover our requirements extensively the past
>> year
>> based on feedback the last time I posted these patches. We implemented a
>> prototype that had half of our requirements, but found it resulted in a
>> large change list that would result in a large amount of pain in respect
>> to maintenance, in addition to other more architectural concerns about the
>> implementation. We weren’t convinced it was the correct direction, for our
>> needs.
>>
>> There was a presentation done at LSS 2021 around this prototype done by my
>> colleague, Fan, who authored this patch and implemented the aforementioned
>> prototype.
>>
>> In general, IMA provides a whole suite of amazing functionality when it
>> comes to everything integrity, as the fs-verity documentation states
>> itself:
>>
>>     IMA specifies a system-wide policy that specifies which
>>     files are hashed and what to do with those hashes, such
>>     as log them, authenticate them, or add them to a
>>     measurement list.
>>
>> Instead, IPE provides a fine-tuned way to _only_ enforce an access control
>> policy to these files based on the defined trust requirements in the policy,
>> under various contexts, (you might have different requirements for what
>> executes in a general purpose, versus loadable kernel modules, for example).
>> It will never provide bother to log, measure, or revalidate these hashes
>> because
>> that’s not its purpose. This is why it belongs at the LSM layer instead of
>> the
>> integrity subsystem layer, as it is providing access control based on a
>> policy,
>> versus providing deep integrations with the actual integrity claim.
>>
>> IPE is trying to be agnostic to how precisely “trust” is provided, as
>> opposed to be deeply integrated into the mechanism that provides
>> “trust”.
> IMA doesn't require logging or "measuring" hashes, though.  Those are just some
> of its supported features.  And I thought the IMA developers were planning to
> add support for fs-verity hashes, and that it wouldn't require an entirely new
> architecture to do so.
>
> Anyway, while it does sound to me like you're duplicating IMA, I don't really
> have a horse in this race, and I defer to the IMA developers on this.  I trust
> that you've been engaging with them?  This patchset isn't even Cc'ed to
> linux-integrity, so it's unclear that's been happening.
That was entirely my mistake. Mimi and the linux-integrity list was CC'd 
on previous
versions (Roberto actually added the list to his responses) - when I was 
reconstructing
the To: line with get-maintainers.pl, the list didn't pop up and I did 
not remember to
add it manually. I've corrected my mailing script to re-add them again.


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Roberto Sassu Nov. 3, 2021, 12:28 p.m. UTC | #12

> From: Deven Bowers [mailto:[e-mail]]
> Sent: Friday, October 15, 2021 9:26 PM
> On 10/13/2021 12:24 PM, Eric Biggers wrote:
> > On Wed, Oct 13, 2021 at 12:06:31PM -0700,
> [e-mail] wrote:
> >> From: Fan Wu <[e-mail]>
> >>
> >> 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 wanted to go back on this question.

It seems, at least for fsverity, that you could obtain the
root digest at run-time, without storing it in a security blob.

I thought I should use fsverity_get_info() but the fsverity_info
structure is not exported (it is defined in fs/verity/fsverity_private.h).

Then, I defined a new function, fsverity_get_file_digest() to copy
the file_digest member of fsverity_info to a buffer and to pass
the associated hash algorithm.

With that, the code of evaluate() for DIGLIM becomes:

        info = fsverity_get_info(file_inode(ctx->file));
        if (info)
                ret = fsverity_get_file_digest(info, buffer, sizeof(buffer), &algo);

        if (!strcmp(expect->data, "diglim") && ret > 0) {
                ret = diglim_digest_get_info(buffer, algo, COMPACT_FILE, &modifiers, &actions);
                if (!ret)
                        return true;
        }

Roberto

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

> 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.
> 
> 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.
> IPE does not want to recalculate a digest, that’s expensive and
> doesn’t provide any value. IPE, in this case, treats this as a
> buffer to compare the policy-provided one above to make a
> policy decision about access to the resource.
> 
> >> Also changes the implementaion inside the hook function to let
> >> multiple LSMs can add hooks.
> > Please split fs/verity/ changes and security/ changes into separate patches, if
> > possible.
> 
> Sorry, will do, not a problem.
> 
> >> @@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const
> struct inode *inode,
> >>   		fsverity_err(inode, "Error %d computing file digest", err);
> >>   		goto out;
> >>   	}
> >> +
> >> +	err = security_inode_setsecurity((struct inode *)inode,
> > If a non-const inode is needed, please propagate that into the callers rather
> > than randomly casting away the const.
> >
> >> +					 FS_VERITY_DIGEST_SEC_NAME,
> >> +					 vi->file_digest,
> >> +					 vi->tree_params.hash_alg-
> >digest_size,
> >> +					 0);
> >> @@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info
> *vi,
> >>
> >>   	pr_debug("Valid signature for file digest %s:%*phN\n",
> >>   		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
> >> -	return 0;
> >> +	return security_inode_setsecurity((struct inode *)inode,
> >>
> > Likewise, please don't cast away const.
> 
> Sorry, I should've caught these myself. I'll change
> fsverity_create_info to accept the non-const inode, and
> change fsverity_verify_signature to accept an additional inode
> struct as the first arg instead of changing the fsverity_info
> structure to have a non-const inode field.
> 
> >> +					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.
> 
> > and what the LSM hook will do with them?
> 
> At the moment, this will just signal to IPE that these fs-verity files were
> enabled with a built-in signature as opposed to enabled without a signature.
> In v7, it copies the signature data into IPE's LSM blob attached to the
> inode.
> In v8+, I'm changing this to store “true” in IPE's LSM blob instead, as
> copying
> the signature data is an unnecessary waste of space and point of
> failure. This
> has a _slightly_ different functionality then fs.verity.require_signatures,
> because even if someone were to disable the require signatures option, IPE
> would still know if these files were signed or not and be able to make the
> access control decision based IPE's policy.
> 
> Very concretely, this powers this kind of rule in IPE:
> 
>    op=EXECUTE fsverity_signature=TRUE action=ALLOW
> 
> if that fsverity_signature value in IPE’s LSM blob attached to the inode is
> true, then fsverity_signature in IPE’s policy will evaluate to true and
> match
> this rule. The inverse is also applicable.


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Deven Bowers Nov. 4, 2021, 5:12 p.m. UTC | #13

On 11/3/2021 5:28 AM, Roberto Sassu wrote:
>> From: Deven Bowers [mailto:[e-mail]]
>> Sent: Friday, October 15, 2021 9:26 PM
>> On 10/13/2021 12:24 PM, Eric Biggers wrote:
>>> On Wed, Oct 13, 2021 at 12:06:31PM -0700,
>> [e-mail] wrote:
>>>> From: Fan Wu <[e-mail]>
>>>>
>>>> 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 wanted to go back on this question.
>
> It seems, at least for fsverity, that you could obtain the
> root digest at run-time, without storing it in a security blob.
>
> I thought I should use fsverity_get_info() but the fsverity_info
> structure is not exported (it is defined in fs/verity/fsverity_private.h).
>
> Then, I defined a new function, fsverity_get_file_digest() to copy
> the file_digest member of fsverity_info to a buffer and to pass
> the associated hash algorithm.
>
> With that, the code of evaluate() for DIGLIM becomes:
>
>          info = fsverity_get_info(file_inode(ctx->file));
>          if (info)
>                  ret = fsverity_get_file_digest(info, buffer, sizeof(buffer), &algo);
>
>          if (!strcmp(expect->data, "diglim") && ret > 0) {
>                  ret = diglim_digest_get_info(buffer, algo, COMPACT_FILE, &modifiers, &actions);
>                  if (!ret)
>                          return true;
>          }
This would work with the digest with a bit more code in fs-verity. It
also has benefits if there are other callers who want this information.

I was planning on grouping the digest and signature into 
apublic_key_signature
structure in v8 to pass the digest, digest algorithm,digest size, signature
and signature size (as opposed to defining a new structfor this purpose),
reducing the number of LSM hook calls down to one functionin fsverity.

I think both approaches have merit. fsverity_get_file_digest is more useful
if there are callers outside of LSMs that want this information. The LSM 
hook
is cleaner if only LSMs want this information.

At least, at the moment, it seems like it's only IPE who wants this 
information,
and it's not like it won't be able to change later if the need arises, 
as this
is all implementation details that wouldn't effect the end-user.

I'll defer to Eric - his opinion holds the most weight, as fsverity would be
the main code affected in either case.


--
dm-devel mailing list
[e-mail]
https://listman.redhat.com/mailman/listinfo/dm-devel

Patch

diff --git a/fs/verity/open.c b/fs/verity/open.c
index 92df87f5fa38..1f36dae01c22 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -7,6 +7,7 @@ 
 
 #include "fsverity_private.h"
 
+#include <linux/security.h>
 #include <linux/slab.h>
 
 static struct kmem_cache *fsverity_info_cachep;
@@ -177,6 +178,17 @@  struct fsverity_info *fsverity_create_info(const struct inode *inode,
 		fsverity_err(inode, "Error %d computing file digest", err);
 		goto out;
 	}
+
+	err = security_inode_setsecurity((struct inode *)inode,
+					 FS_VERITY_DIGEST_SEC_NAME,
+					 vi->file_digest,
+					 vi->tree_params.hash_alg->digest_size,
+					 0);
+	if (err) {
+		fsverity_err(inode, "Error %d inode setsecurity hook", err);
+		goto out;
+	}
+
 	pr_debug("Computed file digest: %s:%*phN\n",
 		 vi->tree_params.hash_alg->name,
 		 vi->tree_params.digest_size, vi->file_digest);
diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index 143a530a8008..20e585d5fa6d 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -9,6 +9,7 @@ 
 
 #include <linux/cred.h>
 #include <linux/key.h>
+#include <linux/security.h>
 #include <linux/slab.h>
 #include <linux/verification.h>
 
@@ -84,7 +85,9 @@  int fsverity_verify_signature(const struct fsverity_info *vi,
 
 	pr_debug("Valid signature for file digest %s:%*phN\n",
 		 hash_alg->name, hash_alg->digest_size, vi->file_digest);
-	return 0;
+	return security_inode_setsecurity((struct inode *)inode,
+					FS_VERITY_SIGNATURE_SEC_NAME,
+					signature, sig_size, 0);
 }
 
 #ifdef CONFIG_SYSCTL
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index b568b3c7d095..dfd7b5a85c67 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -233,4 +233,7 @@  static inline bool fsverity_active(const struct inode *inode)
 	return fsverity_get_info(inode) != NULL;
 }
 
+#define FS_VERITY_SIGNATURE_SEC_NAME "fsverity.verity-sig"
+#define FS_VERITY_DIGEST_SEC_NAME "fsverity.verity-digest"
+
 #endif	/* _LINUX_FSVERITY_H */
diff --git a/security/ipe/hooks.c b/security/ipe/hooks.c
index 470fb48e490c..d76e60a3f511 100644
--- a/security/ipe/hooks.c
+++ b/security/ipe/hooks.c
@@ -232,6 +232,7 @@  void ipe_bdev_free_security(struct block_device *bdev)
 	struct ipe_bdev *blob = ipe_bdev(bdev);
 
 	kfree(blob->sigdata);
+	kfree(blob->hash);
 }
 
 /**
diff --git a/security/security.c b/security/security.c
index d7ac9f01500b..81751a91f438 100644
--- a/security/security.c
+++ b/security/security.c
@@ -1462,7 +1462,7 @@  int security_inode_getsecurity(struct user_namespace *mnt_userns,
 int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
 {
 	struct security_hook_list *hp;
-	int rc;
+	int rc = LSM_RET_DEFAULT(inode_setsecurity);
 
 	if (unlikely(IS_PRIVATE(inode)))
 		return LSM_RET_DEFAULT(inode_setsecurity);
@@ -1472,10 +1472,10 @@  int security_inode_setsecurity(struct inode *inode, const char *name, const void
 	hlist_for_each_entry(hp, &security_hook_heads.inode_setsecurity, list) {
 		rc = hp->hook.inode_setsecurity(inode, name, value, size,
 								flags);
-		if (rc != LSM_RET_DEFAULT(inode_setsecurity))
+		if (rc && rc != LSM_RET_DEFAULT(inode_setsecurity))
 			return rc;
 	}
-	return LSM_RET_DEFAULT(inode_setsecurity);
+	return rc;
 }
 
 int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size)