On Mon, Jun 12, 2023 at 9:09 PM Eric Biggers <ebiggers@xxxxxxxxxx> wrote:
>
> On Mon, Jun 12, 2023 at 12:28:17PM +0200, Alexander Larsson wrote:
> > +int ovl_validate_verity(struct ovl_fs *ofs,
> > + struct path *metapath,
> > + struct path *datapath)
> > +{
> > + u8 xattr_data[1+FS_VERITY_MAX_DIGEST_SIZE];
> > + u8 actual_digest[FS_VERITY_MAX_DIGEST_SIZE];
> > + enum hash_algo verity_algo;
> > + int xattr_len;
> > + int err;
> > +
> > + if (!ofs->config.verity ||
> > + /* Verity only works on regular files */
> > + !S_ISREG(d_inode(metapath->dentry)->i_mode))
> > + return 0;
> > +
> > + xattr_len = sizeof(xattr_data);
> > + err = ovl_get_verity_xattr(ofs, metapath, xattr_data, &xattr_len);
> > + if (err == -ENODATA) {
> > + if (ofs->config.require_verity) {
> > + pr_warn_ratelimited("metacopy file '%pd' has no overlay.verity xattr\n",
> > + metapath->dentry);
> > + return -EIO;
> > + }
> > + return 0;
> > + }
> > + if (err < 0)
> > + return err;
> > +
> > + err = ovl_ensure_verity_loaded(datapath);
> > + if (err < 0) {
> > + pr_warn_ratelimited("lower file '%pd' failed to load fs-verity info\n",
> > + datapath->dentry);
> > + return -EIO;
> > + }
> > +
> > + 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 (xattr_len != 1 + hash_digest_size[verity_algo] ||
> > + xattr_data[0] != (u8) verity_algo ||
> > + memcmp(xattr_data+1, actual_digest, xattr_len - 1) != 0) {
> > + pr_warn_ratelimited("lower file '%pd' has the wrong fs-verity digest\n",
> > + datapath->dentry);
> > + return -EIO;
> > + }
> > +
> > + return 0;
> > +}
>
> This means the overlayfs verity xattr contains the algorithm ID of the fsverity
> digest as a HASH_ALGO_* value.
>
> That works, but I think HASH_ALGO_* is somewhat of an IMA-ism. fsverity
> actually uses FS_VERITY_HASH_ALG_* everywhere else, including in the UAPI and in
> fsverity-utils which includes libfsverity
> (https://git.kernel.org/pub/scm/fs/fsverity/fsverity-utils.git/tree/include/libfsverity.h).
>
> I'm guessing that you used HASH_ALGO_* not because you really wanted to, but
> rather just because it's what fsverity_get_digest() happens to return, as IMA is
> currently its only user.
Yeah, that is exactly why.
> Can you consider
> https://lore.kernel.org/r/20230612190047.59755-1-ebiggers@xxxxxxxxxx which would
> make fsverity_get_digest() support both types of IDs? Then you can use
> FS_VERITY_HASH_ALG_*, which I think would make things slightly easier for you.
Sounds very good to me. I'll rebase the patchset on top of it. Not
sure how to best land this though, are you ok with this landing via
overlayfs-next?
--
=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
Alexander Larsson Red Hat, Inc
alexl@xxxxxxxxxx alexander.larsson@xxxxxxxxx
