[PATCH] btrfs: send: add support for fs-verity

[PATCH] btrfs: send: add support for fs-verity

[Boris Burkov]

Preserve the fs-verity status of a btrfs file across send/recv.

There is no facility for installing the Merkle tree contents directly on
the receiving filesystem, so we package up the parameters used to enable
verity found in the verity descriptor. This gives the receive side
enough information to properly enable verity again. Note that this means
that receive will have to re-compute the whole Merkle tree, similar to
how compression worked before encoded_write.

Since the file becomes read-only after verity is enabled, it is
important that verity is added to the send stream after any file writes.
Therefore, when we process a verity item, merely note that it happened,
then actually create the command in the send stream during
'finish_inode_if_needed'.

This also creates V3 of the send stream format, without any format
changes besides adding the new commands and attributes.

Signed-off-by: Boris Burkov <boris@bur.io>
---
 fs/btrfs/send.c              | 97 ++++++++++++++++++++++++++++++++++++
 fs/btrfs/send.h              | 15 ++++--
 fs/verity/fsverity_private.h |  2 -
 include/linux/fsverity.h     |  3 ++
 4 files changed, 112 insertions(+), 5 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index e7671afcee4f..0ce243ea9c6d 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -15,6 +15,7 @@
 #include <linux/string.h>
 #include <linux/compat.h>
 #include <linux/crc32c.h>
+#include <linux/fsverity.h>
 
 #include "send.h"
 #include "ctree.h"
@@ -127,6 +128,7 @@ struct send_ctx {
 	bool cur_inode_new_gen;
 	bool cur_inode_deleted;
 	bool ignore_cur_inode;
+	bool cur_inode_needs_verity;
 
 	u64 send_progress;
 
@@ -624,6 +626,7 @@ static int tlv_put(struct send_ctx *sctx, u16 attr, const void *data, int len)
 		return tlv_put(sctx, attr, &__tmp, sizeof(__tmp));	\
 	}
 
+TLV_PUT_DEFINE_INT(8)
 TLV_PUT_DEFINE_INT(32)
 TLV_PUT_DEFINE_INT(64)
 
@@ -4886,6 +4889,80 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
 	return ret;
 }
 
+static int send_verity(struct send_ctx *sctx, struct fs_path *path,
+		       struct fsverity_descriptor *desc)
+{
+	int ret;
+
+	ret = begin_cmd(sctx, BTRFS_SEND_C_ENABLE_VERITY);
+	if (ret < 0)
+		goto out;
+
+	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, path);
+	TLV_PUT_U8(sctx, BTRFS_SEND_A_VERITY_ALGORITHM, desc->hash_algorithm);
+	TLV_PUT_U32(sctx, BTRFS_SEND_A_VERITY_BLOCK_SIZE, 1 << desc->log_blocksize);
+	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SALT_DATA, desc->salt, desc->salt_size);
+	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SIG_DATA, desc->signature, desc->sig_size);
+
+	ret = send_cmd(sctx);
+
+tlv_put_failure:
+out:
+	return ret;
+}
+
+static int process_new_verity(struct send_ctx *sctx)
+{
+	int ret = 0;
+	struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
+	struct inode *inode;
+	struct fsverity_descriptor *desc;
+	struct fs_path *p;
+
+	inode = btrfs_iget(fs_info->sb, sctx->cur_ino, sctx->send_root);
+	if (IS_ERR(inode))
+		return PTR_ERR(inode);
+
+	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, NULL, 0);
+	if (ret < 0)
+		goto iput;
+
+	if (ret > FS_VERITY_MAX_DESCRIPTOR_SIZE) {
+		ret = -EMSGSIZE;
+		goto iput;
+	}
+	desc = kmalloc(ret, GFP_KERNEL);
+	if (!desc) {
+		ret = -ENOMEM;
+		goto iput;
+	}
+
+	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, desc, ret);
+	if (ret < 0)
+		goto free_desc;
+
+	p = fs_path_alloc();
+	if (!p) {
+		ret = -ENOMEM;
+		goto free_desc;
+	}
+	ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, p);
+	if (ret < 0)
+		goto free_path;
+
+	ret = send_verity(sctx, p, desc);
+	if (ret < 0)
+		goto free_path;
+
+free_path:
+	fs_path_free(p);
+free_desc:
+	kfree(desc);
+iput:
+	iput(inode);
+	return ret;
+}
+
 static inline u64 max_send_read_size(const struct send_ctx *sctx)
 {
 	return sctx->send_max_size - SZ_16K;
@@ -6377,6 +6454,11 @@ static int finish_inode_if_needed(struct send_ctx *sctx, int at_end)
 		if (ret < 0)
 			goto out;
 	}
+	if (sctx->cur_inode_needs_verity) {
+		ret = process_new_verity(sctx);
+		if (ret < 0)
+			goto out;
+	}
 
 	ret = send_capabilities(sctx);
 	if (ret < 0)
@@ -6785,6 +6867,18 @@ static int changed_extent(struct send_ctx *sctx,
 	return ret;
 }
 
+static int changed_verity(struct send_ctx *sctx,
+			  enum btrfs_compare_tree_result result)
+{
+	int ret = 0;
+
+	if (!sctx->cur_inode_new_gen && !sctx->cur_inode_deleted) {
+		if (result == BTRFS_COMPARE_TREE_NEW)
+			sctx->cur_inode_needs_verity = true;
+	}
+	return ret;
+}
+
 static int dir_changed(struct send_ctx *sctx, u64 dir)
 {
 	u64 orig_gen, new_gen;
@@ -6939,6 +7033,9 @@ static int changed_cb(struct btrfs_path *left_path,
 			ret = changed_xattr(sctx, result);
 		else if (key->type == BTRFS_EXTENT_DATA_KEY)
 			ret = changed_extent(sctx, result);
+		else if (key->type == BTRFS_VERITY_DESC_ITEM_KEY &&
+			 key->offset == 0)
+			ret = changed_verity(sctx, result);
 	}
 
 out:
diff --git a/fs/btrfs/send.h b/fs/btrfs/send.h
index 4bb4e6a638cb..0a4537775e0c 100644
--- a/fs/btrfs/send.h
+++ b/fs/btrfs/send.h
@@ -92,8 +92,11 @@ enum btrfs_send_cmd {
 	BTRFS_SEND_C_ENCODED_WRITE	= 25,
 	BTRFS_SEND_C_MAX_V2		= 25,
 
+	/* Version 3 */
+	BTRFS_SEND_C_ENABLE_VERITY	= 26,
+	BTRFS_SEND_C_MAX_V3		= 26,
 	/* End */
-	BTRFS_SEND_C_MAX		= 25,
+	BTRFS_SEND_C_MAX		= 26,
 };
 
 /* attributes in send stream */
@@ -160,8 +163,14 @@ enum {
 	BTRFS_SEND_A_ENCRYPTION		= 31,
 	BTRFS_SEND_A_MAX_V2		= 31,
 
-	/* End */
-	BTRFS_SEND_A_MAX		= 31,
+	/* Version 3 */
+	BTRFS_SEND_A_VERITY_ALGORITHM	= 32,
+	BTRFS_SEND_A_VERITY_BLOCK_SIZE	= 33,
+	BTRFS_SEND_A_VERITY_SALT_DATA	= 34,
+	BTRFS_SEND_A_VERITY_SIG_DATA	= 35,
+	BTRFS_SEND_A_MAX_V3		= 35,
+
+	__BTRFS_SEND_A_MAX		= 35,
 };
 
 long btrfs_ioctl_send(struct inode *inode, struct btrfs_ioctl_send_args *arg);
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index 629785c95007..dbe1ce5b450a 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -70,8 +70,6 @@ struct fsverity_info {
 	const struct inode *inode;
 };
 
-/* Arbitrary limit to bound the kmalloc() size.  Can be changed. */
-#define FS_VERITY_MAX_DESCRIPTOR_SIZE	16384
 
 #define FS_VERITY_MAX_SIGNATURE_SIZE	(FS_VERITY_MAX_DESCRIPTOR_SIZE - \
 					 sizeof(struct fsverity_descriptor))
diff --git a/include/linux/fsverity.h b/include/linux/fsverity.h
index 7af030fa3c36..40f14e5fed9d 100644
--- a/include/linux/fsverity.h
+++ b/include/linux/fsverity.h
@@ -22,6 +22,9 @@
  */
 #define FS_VERITY_MAX_DIGEST_SIZE	SHA512_DIGEST_SIZE
 
+/* Arbitrary limit to bound the kmalloc() size.  Can be changed. */
+#define FS_VERITY_MAX_DESCRIPTOR_SIZE	16384
+
 /* Verity operations for filesystems */
 struct fsverity_operations {
 
-- 
2.37.0

Re: [PATCH] btrfs: send: add support for fs-verity

[David Sterba]

On Wed, Jul 27, 2022 at 04:46:22PM -0700, Boris Burkov wrote:
> Preserve the fs-verity status of a btrfs file across send/recv.
> 
> There is no facility for installing the Merkle tree contents directly on
> the receiving filesystem, so we package up the parameters used to enable
> verity found in the verity descriptor. This gives the receive side
> enough information to properly enable verity again. Note that this means
> that receive will have to re-compute the whole Merkle tree, similar to
> how compression worked before encoded_write.
> 
> Since the file becomes read-only after verity is enabled, it is
> important that verity is added to the send stream after any file writes.
> Therefore, when we process a verity item, merely note that it happened,
> then actually create the command in the send stream during
> 'finish_inode_if_needed'.
> 
> This also creates V3 of the send stream format, without any format
> changes besides adding the new commands and attributes.
> 
> Signed-off-by: Boris Burkov <boris@bur.io>
> ---
> @@ -4886,6 +4889,80 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
>  	return ret;
>  }
>  
> +static int send_verity(struct send_ctx *sctx, struct fs_path *path,
> +		       struct fsverity_descriptor *desc)
> +{
> +	int ret;
> +
> +	ret = begin_cmd(sctx, BTRFS_SEND_C_ENABLE_VERITY);
> +	if (ret < 0)
> +		goto out;
> +
> +	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, path);
> +	TLV_PUT_U8(sctx, BTRFS_SEND_A_VERITY_ALGORITHM, desc->hash_algorithm);
> +	TLV_PUT_U32(sctx, BTRFS_SEND_A_VERITY_BLOCK_SIZE, 1 << desc->log_blocksize);

bitshifts should be done on unsigned types

	1U << desc->log_blocksize

> +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SALT_DATA, desc->salt, desc->salt_size);
> +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SIG_DATA, desc->signature, desc->sig_size);
> +
> +	ret = send_cmd(sctx);
> +
> +tlv_put_failure:
> +out:
> +	return ret;
> +}
> +
> +static int process_new_verity(struct send_ctx *sctx)
> +{
> +	int ret = 0;
> +	struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
> +	struct inode *inode;
> +	struct fsverity_descriptor *desc;
> +	struct fs_path *p;
> +
> +	inode = btrfs_iget(fs_info->sb, sctx->cur_ino, sctx->send_root);
> +	if (IS_ERR(inode))
> +		return PTR_ERR(inode);
> +
> +	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, NULL, 0);
> +	if (ret < 0)
> +		goto iput;
> +
> +	if (ret > FS_VERITY_MAX_DESCRIPTOR_SIZE) {
> +		ret = -EMSGSIZE;
> +		goto iput;
> +	}
> +	desc = kmalloc(ret, GFP_KERNEL);

I think that once there's a file with verity record there will be more
so it would make sense to cache the allocated memory, which means to
allocate the full size.

FS_VERITY_MAX_DESCRIPTOR_SIZE is 16K so this should be allocated by
kvmalloc, like we have for other data during send.

> +	if (!desc) {
> +		ret = -ENOMEM;
> +		goto iput;
> +	}
> +
> +	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, desc, ret);
> +	if (ret < 0)
> +		goto free_desc;
> +
> +	p = fs_path_alloc();
> +	if (!p) {
> +		ret = -ENOMEM;
> +		goto free_desc;
> +	}
> +	ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, p);
> +	if (ret < 0)
> +		goto free_path;
> +
> +	ret = send_verity(sctx, p, desc);
> +	if (ret < 0)
> +		goto free_path;
> +
> +free_path:
> +	fs_path_free(p);
> +free_desc:
> +	kfree(desc);
> +iput:
> +	iput(inode);
> +	return ret;
> +}
> +
>  static inline u64 max_send_read_size(const struct send_ctx *sctx)
>  {
>  	return sctx->send_max_size - SZ_16K;
> --- a/fs/btrfs/send.h
> +++ b/fs/btrfs/send.h
> @@ -92,8 +92,11 @@ enum btrfs_send_cmd {
>  	BTRFS_SEND_C_ENCODED_WRITE	= 25,
>  	BTRFS_SEND_C_MAX_V2		= 25,
>  
> +	/* Version 3 */
> +	BTRFS_SEND_C_ENABLE_VERITY	= 26,

I find the name confusing, it sounds like enabling it for the whole
filesystem, but it affects only one file.

> +	BTRFS_SEND_C_MAX_V3		= 26,
>  	/* End */
> -	BTRFS_SEND_C_MAX		= 25,
> +	BTRFS_SEND_C_MAX		= 26,
>  };
>  
>  /* attributes in send stream */
> @@ -160,8 +163,14 @@ enum {
>  	BTRFS_SEND_A_ENCRYPTION		= 31,
>  	BTRFS_SEND_A_MAX_V2		= 31,
>  
> -	/* End */
> -	BTRFS_SEND_A_MAX		= 31,
> +	/* Version 3 */
> +	BTRFS_SEND_A_VERITY_ALGORITHM	= 32,
> +	BTRFS_SEND_A_VERITY_BLOCK_SIZE	= 33,
> +	BTRFS_SEND_A_VERITY_SALT_DATA	= 34,
> +	BTRFS_SEND_A_VERITY_SIG_DATA	= 35,
> +	BTRFS_SEND_A_MAX_V3		= 35,
> +
> +	__BTRFS_SEND_A_MAX		= 35,
>  };

Re: [PATCH] btrfs: send: add support for fs-verity

[Boris Burkov]

On Thu, Jul 28, 2022 at 01:43:32PM +0200, David Sterba wrote:
> On Wed, Jul 27, 2022 at 04:46:22PM -0700, Boris Burkov wrote:
> > Preserve the fs-verity status of a btrfs file across send/recv.
> > 
> > There is no facility for installing the Merkle tree contents directly on
> > the receiving filesystem, so we package up the parameters used to enable
> > verity found in the verity descriptor. This gives the receive side
> > enough information to properly enable verity again. Note that this means
> > that receive will have to re-compute the whole Merkle tree, similar to
> > how compression worked before encoded_write.
> > 
> > Since the file becomes read-only after verity is enabled, it is
> > important that verity is added to the send stream after any file writes.
> > Therefore, when we process a verity item, merely note that it happened,
> > then actually create the command in the send stream during
> > 'finish_inode_if_needed'.
> > 
> > This also creates V3 of the send stream format, without any format
> > changes besides adding the new commands and attributes.
> > 
> > Signed-off-by: Boris Burkov <boris@bur.io>
> > ---
> > @@ -4886,6 +4889,80 @@ static int process_all_new_xattrs(struct send_ctx *sctx)
> >  	return ret;
> >  }
> >  
> > +static int send_verity(struct send_ctx *sctx, struct fs_path *path,
> > +		       struct fsverity_descriptor *desc)
> > +{
> > +	int ret;
> > +
> > +	ret = begin_cmd(sctx, BTRFS_SEND_C_ENABLE_VERITY);
> > +	if (ret < 0)
> > +		goto out;
> > +
> > +	TLV_PUT_PATH(sctx, BTRFS_SEND_A_PATH, path);
> > +	TLV_PUT_U8(sctx, BTRFS_SEND_A_VERITY_ALGORITHM, desc->hash_algorithm);
> > +	TLV_PUT_U32(sctx, BTRFS_SEND_A_VERITY_BLOCK_SIZE, 1 << desc->log_blocksize);
> 
> bitshifts should be done on unsigned types
> 
> 	1U << desc->log_blocksize
> 
> > +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SALT_DATA, desc->salt, desc->salt_size);
> > +	TLV_PUT(sctx, BTRFS_SEND_A_VERITY_SIG_DATA, desc->signature, desc->sig_size);
> > +
> > +	ret = send_cmd(sctx);
> > +
> > +tlv_put_failure:
> > +out:
> > +	return ret;
> > +}
> > +
> > +static int process_new_verity(struct send_ctx *sctx)
> > +{
> > +	int ret = 0;
> > +	struct btrfs_fs_info *fs_info = sctx->send_root->fs_info;
> > +	struct inode *inode;
> > +	struct fsverity_descriptor *desc;
> > +	struct fs_path *p;
> > +
> > +	inode = btrfs_iget(fs_info->sb, sctx->cur_ino, sctx->send_root);
> > +	if (IS_ERR(inode))
> > +		return PTR_ERR(inode);
> > +
> > +	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, NULL, 0);
> > +	if (ret < 0)
> > +		goto iput;
> > +
> > +	if (ret > FS_VERITY_MAX_DESCRIPTOR_SIZE) {
> > +		ret = -EMSGSIZE;
> > +		goto iput;
> > +	}
> > +	desc = kmalloc(ret, GFP_KERNEL);
> 
> I think that once there's a file with verity record there will be more
> so it would make sense to cache the allocated memory, which means to
> allocate the full size.
> 
> FS_VERITY_MAX_DESCRIPTOR_SIZE is 16K so this should be allocated by
> kvmalloc, like we have for other data during send.
> 
> > +	if (!desc) {
> > +		ret = -ENOMEM;
> > +		goto iput;
> > +	}
> > +
> > +	ret = fs_info->sb->s_vop->get_verity_descriptor(inode, desc, ret);
> > +	if (ret < 0)
> > +		goto free_desc;
> > +
> > +	p = fs_path_alloc();
> > +	if (!p) {
> > +		ret = -ENOMEM;
> > +		goto free_desc;
> > +	}
> > +	ret = get_cur_path(sctx, sctx->cur_ino, sctx->cur_inode_gen, p);
> > +	if (ret < 0)
> > +		goto free_path;
> > +
> > +	ret = send_verity(sctx, p, desc);
> > +	if (ret < 0)
> > +		goto free_path;
> > +
> > +free_path:
> > +	fs_path_free(p);
> > +free_desc:
> > +	kfree(desc);
> > +iput:
> > +	iput(inode);
> > +	return ret;
> > +}
> > +
> >  static inline u64 max_send_read_size(const struct send_ctx *sctx)
> >  {
> >  	return sctx->send_max_size - SZ_16K;
> > --- a/fs/btrfs/send.h
> > +++ b/fs/btrfs/send.h
> > @@ -92,8 +92,11 @@ enum btrfs_send_cmd {
> >  	BTRFS_SEND_C_ENCODED_WRITE	= 25,
> >  	BTRFS_SEND_C_MAX_V2		= 25,
> >  
> > +	/* Version 3 */
> > +	BTRFS_SEND_C_ENABLE_VERITY	= 26,
> 
> I find the name confusing, it sounds like enabling it for the whole
> filesystem, but it affects only one file.

Your point about fs-wide vs file specific makes sense,  but on the other
hand, that is the name of the ioctl and the cli command.

Would you prefer VERITY_ENABLE? or just VERITY? I am having trouble
thinking of a better name, but I also don't mind if you think of one
and rename it.

> 
> > +	BTRFS_SEND_C_MAX_V3		= 26,
> >  	/* End */
> > -	BTRFS_SEND_C_MAX		= 25,
> > +	BTRFS_SEND_C_MAX		= 26,
> >  };
> >  
> >  /* attributes in send stream */
> > @@ -160,8 +163,14 @@ enum {
> >  	BTRFS_SEND_A_ENCRYPTION		= 31,
> >  	BTRFS_SEND_A_MAX_V2		= 31,
> >  
> > -	/* End */
> > -	BTRFS_SEND_A_MAX		= 31,
> > +	/* Version 3 */
> > +	BTRFS_SEND_A_VERITY_ALGORITHM	= 32,
> > +	BTRFS_SEND_A_VERITY_BLOCK_SIZE	= 33,
> > +	BTRFS_SEND_A_VERITY_SALT_DATA	= 34,
> > +	BTRFS_SEND_A_VERITY_SIG_DATA	= 35,
> > +	BTRFS_SEND_A_MAX_V3		= 35,
> > +
> > +	__BTRFS_SEND_A_MAX		= 35,
> >  };