On Wed, Apr 26, 2023 at 11:47 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Thu, Apr 20, 2023 at 09:44:04AM +0200, Alexander Larsson wrote:
> > + err = fsverity_get_digest(d_inode(datapath->dentry), actual_digest, &verity_algo);
> > + if (err < 0) {
> > + pr_warn_ratelimited("lower file '%pd' has no fs-verity digest\n", datapath->dentry);
> > + return -EIO;
> > + }
> > +
> > + if (digest_len != hash_digest_size[verity_algo] ||
> > + memcmp(required_digest, actual_digest, digest_len) != 0) {
> > + pr_warn_ratelimited("lower file '%pd' has the wrong fs-verity digest\n",
> > + datapath->dentry);
> > + return -EIO;
> > + }
> > +
> > + return 0;
>
> This is incorrect because the digest algorithm is not being compared.
This is actually an interesting question. How much are things weakened
by comparing the digest size, but not comparing the digest type. Like,
suppose the xattr has a sha256 digest (32 bytes), how likely is there
to be another new supported verity algorithm of the same digest size
where you can force it to produce matching digests?
I ask because ideally we want to minimize the size of the xattrs,
since they are stored for each file, and not having to specify the
type for each saves space. Currently the only two supported algorithms
(sha256 and sha512) are different sizes, so we essentially compare
type by comparing the size.
I see three options here:
1) Only compare digest + size (like now)
2) Assume size 32 means sha256, and 64 means sha512 and validate that
3) Use more space in the xattr to store an algorithm type
Maybe alternative 2 is the best option on balance, less extensible,
but safe and uses least space.
Opinions?
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Alexander Larsson Red Hat, Inc
alexl@xxxxxxxxxx alexander.larsson@xxxxxxxxx
