From: Deven Bowers [mailto:deven.desai@linux.microsoft.com]
Sent: Friday, October 15, 2021 9:26 PM
On 10/13/2021 12:24 PM, Eric Biggers wrote:

quoted

On Wed, Oct 13, 2021 at 12:06:31PM -0700,

deven.desai@linux.microsoft.com wrote:

quoted

quoted

From: Fan Wu <redacted>

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.

quoted

quoted

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.

quoted

quoted

@@ -177,6 +178,17 @@ struct fsverity_info *fsverity_create_info(const

struct inode *inode,

quoted

quoted

  		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.

quoted

+					 FS_VERITY_DIGEST_SEC_NAME,
+					 vi->file_digest,
+					 vi->tree_params.hash_alg-

digest_size,

quoted

+					 0);

@@ -84,7 +85,9 @@ int fsverity_verify_signature(const struct fsverity_info

*vi,

quoted

quoted

  	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.

quoted

quoted

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

quoted

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.

quoted

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.