Re: [PATCH v3 3/4] ovl: Validate verity xattr when resolving lowerdata

[Eric Biggers <ebiggers@kernel.org>]

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.

Can you consider
https://lore.kernel.org/r/20230612190047.59755-1-ebiggers@kernel.org 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.

- Eric