[This patchset applies to v5.11-rc3]
Add an ioctl FS_IOC_READ_VERITY_METADATA which allows reading verity
metadata from a file that has fs-verity enabled, including:
- The Merkle tree
- The fsverity_descriptor (not including the signature if present)
- The built-in signature, if present
This ioctl has similar semantics to pread(). It is passed the type of
metadata to read (one of the above three), and a buffer, offset, and
size. It returns the number of bytes read or an error.
This ioctl doesn't make any assumption about where the metadata is
stored on-disk. It does assume the metadata is in a stable format, but
that's basically already the case:
- The Merkle tree and fsverity_descriptor are defined by how fs-verity
file digests are computed; see the "File digest computation" section
of Documentation/filesystems/fsverity.rst. Technically, the way in
which the levels of the tree are ordered relative to each other wasn't
previously specified, but it's logical to put the root level first.
- The built-in signature is the value passed to FS_IOC_ENABLE_VERITY.
This ioctl is useful because it allows writing a server program that
takes a verity file and serves it to a client program, such that the
client can do its own fs-verity compatible verification of the file.
This only makes sense if the client doesn't trust the server and if the
server needs to provide the storage for the client.
More concretely, there is interest in using this ability in Android to
export APK files (which are protected by fs-verity) to "protected VMs".
This would use Protected KVM (https://lwn.net/Articles/836693), which
provides an isolated execution environment without having to trust the
traditional "host". A "guest" VM can boot from a signed image and
perform specific tasks in a minimum trusted environment using files that
have fs-verity enabled on the host, without trusting the host or
requiring that the guest has its own trusted storage.
Technically, it would be possible to duplicate the metadata and store it
in separate files for serving. However, that would be less efficient
and would require extra care in userspace to maintain file consistency.
In addition to the above, the ability to read the built-in signatures is
useful because it allows a system that is using the in-kernel signature
verification to migrate to userspace signature verification.
This patchset has been tested by new xfstests which call this new ioctl
via a new subcommand for the 'fsverity' program from fsverity-utils.
Eric Biggers (6):
fs-verity: factor out fsverity_get_descriptor()
fs-verity: don't pass whole descriptor to fsverity_verify_signature()
fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl
fs-verity: support reading Merkle tree with ioctl
fs-verity: support reading descriptor with ioctl
fs-verity: support reading signature with ioctl
Documentation/filesystems/fsverity.rst | 76 ++++++++++
fs/ext4/ioctl.c | 7 +
fs/f2fs/file.c | 11 ++
fs/verity/Makefile | 1 +
fs/verity/fsverity_private.h | 13 +-
fs/verity/open.c | 133 +++++++++++------
fs/verity/read_metadata.c | 195 +++++++++++++++++++++++++
fs/verity/signature.c | 20 +--
include/linux/fsverity.h | 12 ++
include/uapi/linux/fsverity.h | 14 ++
10 files changed, 417 insertions(+), 65 deletions(-)
create mode 100644 fs/verity/read_metadata.c
base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
--
2.30.0
From: Eric Biggers <[email protected]>
Now that fsverity_get_descriptor() validates the sig_size field,
fsverity_verify_signature() doesn't need to do it.
Just change the prototype of fsverity_verify_signature() to take the
signature directly rather than take a fsverity_descriptor.
Signed-off-by: Eric Biggers <[email protected]>
---
fs/verity/fsverity_private.h | 6 ++----
fs/verity/open.c | 3 ++-
fs/verity/signature.c | 20 ++++++--------------
3 files changed, 10 insertions(+), 19 deletions(-)
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index 6c9caccc06021..a7920434bae50 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -140,15 +140,13 @@ void __init fsverity_exit_info_cache(void);
#ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
int fsverity_verify_signature(const struct fsverity_info *vi,
- const struct fsverity_descriptor *desc,
- size_t desc_size);
+ const u8 *signature, size_t sig_size);
int __init fsverity_init_signature(void);
#else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */
static inline int
fsverity_verify_signature(const struct fsverity_info *vi,
- const struct fsverity_descriptor *desc,
- size_t desc_size)
+ const u8 *signature, size_t sig_size)
{
return 0;
}
diff --git a/fs/verity/open.c b/fs/verity/open.c
index a987bb785e9b0..60ff8af7219fe 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -181,7 +181,8 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
vi->tree_params.hash_alg->name,
vi->tree_params.digest_size, vi->file_digest);
- err = fsverity_verify_signature(vi, desc, desc_size);
+ err = fsverity_verify_signature(vi, desc->signature,
+ le32_to_cpu(desc->sig_size));
out:
if (err) {
fsverity_free_info(vi);
diff --git a/fs/verity/signature.c b/fs/verity/signature.c
index 012468eda2a78..143a530a80088 100644
--- a/fs/verity/signature.c
+++ b/fs/verity/signature.c
@@ -29,21 +29,19 @@ static struct key *fsverity_keyring;
/**
* fsverity_verify_signature() - check a verity file's signature
* @vi: the file's fsverity_info
- * @desc: the file's fsverity_descriptor
- * @desc_size: size of @desc
+ * @signature: the file's built-in signature
+ * @sig_size: size of signature in bytes, or 0 if no signature
*
- * If the file's fs-verity descriptor includes a signature of the file digest,
- * verify it against the certificates in the fs-verity keyring.
+ * If the file includes a signature of its fs-verity file digest, verify it
+ * against the certificates in the fs-verity keyring.
*
* Return: 0 on success (signature valid or not required); -errno on failure
*/
int fsverity_verify_signature(const struct fsverity_info *vi,
- const struct fsverity_descriptor *desc,
- size_t desc_size)
+ const u8 *signature, size_t sig_size)
{
const struct inode *inode = vi->inode;
const struct fsverity_hash_alg *hash_alg = vi->tree_params.hash_alg;
- const u32 sig_size = le32_to_cpu(desc->sig_size);
struct fsverity_formatted_digest *d;
int err;
@@ -56,11 +54,6 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
return 0;
}
- if (sig_size > desc_size - sizeof(*desc)) {
- fsverity_err(inode, "Signature overflows verity descriptor");
- return -EBADMSG;
- }
-
d = kzalloc(sizeof(*d) + hash_alg->digest_size, GFP_KERNEL);
if (!d)
return -ENOMEM;
@@ -70,8 +63,7 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
memcpy(d->digest, vi->file_digest, hash_alg->digest_size);
err = verify_pkcs7_signature(d, sizeof(*d) + hash_alg->digest_size,
- desc->signature, sig_size,
- fsverity_keyring,
+ signature, sig_size, fsverity_keyring,
VERIFYING_UNSPECIFIED_SIGNATURE,
NULL, NULL);
kfree(d);
--
2.30.0
From: Eric Biggers <[email protected]>
Add support for FS_VERITY_METADATA_TYPE_SIGNATURE to
FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
retrieve the built-in signature (if present) of a verity file for
serving to a client which implements fs-verity compatible verification.
See the patch which introduced FS_IOC_READ_VERITY_METADATA for more
details.
The ability for userspace to read the built-in signatures is also useful
because it allows a system that is using the in-kernel signature
verification to migrate to userspace signature verification.
This has been tested using a new xfstest which calls this ioctl via a
new subcommand for the 'fsverity' program from fsverity-utils.
Signed-off-by: Eric Biggers <[email protected]>
---
Documentation/filesystems/fsverity.rst | 9 +++++++-
fs/verity/read_metadata.c | 30 ++++++++++++++++++++++++++
include/uapi/linux/fsverity.h | 1 +
3 files changed, 39 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 6dc5772037ef9..1d831e3cbcb33 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -236,6 +236,7 @@ This ioctl takes in a pointer to the following structure::
#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
#define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
+ #define FS_VERITY_METADATA_TYPE_SIGNATURE 3
struct fsverity_read_metadata_arg {
__u64 metadata_type;
@@ -256,6 +257,10 @@ This ioctl takes in a pointer to the following structure::
- ``FS_VERITY_METADATA_TYPE_DESCRIPTOR`` reads the fs-verity
descriptor. See `fs-verity descriptor`_.
+- ``FS_VERITY_METADATA_TYPE_SIGNATURE`` reads the signature which was
+ passed to FS_IOC_ENABLE_VERITY, if any. See `Built-in signature
+ verification`_.
+
The semantics are similar to those of ``pread()``. ``offset``
specifies the offset in bytes into the metadata item to read from, and
``length`` specifies the maximum number of bytes to read from the
@@ -279,7 +284,9 @@ FS_IOC_READ_VERITY_METADATA can fail with the following errors:
- ``EINTR``: the ioctl was interrupted before any data was read
- ``EINVAL``: reserved fields were set, or ``offset + length``
overflowed
-- ``ENODATA``: the file is not a verity file
+- ``ENODATA``: the file is not a verity file, or
+ FS_VERITY_METADATA_TYPE_SIGNATURE was requested but the file doesn't
+ have a built-in signature
- ``ENOTTY``: this type of filesystem does not implement fs-verity, or
this ioctl is not yet implemented on it
- ``EOPNOTSUPP``: the kernel was not configured with fs-verity
diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
index 2dea6dd3bb05a..7e2d0c7bdf0de 100644
--- a/fs/verity/read_metadata.c
+++ b/fs/verity/read_metadata.c
@@ -114,6 +114,34 @@ static int fsverity_read_descriptor(struct inode *inode,
kfree(desc);
return res;
}
+
+static int fsverity_read_signature(struct inode *inode,
+ void __user *buf, u64 offset, int length)
+{
+ struct fsverity_descriptor *desc;
+ size_t desc_size;
+ int res;
+
+ res = fsverity_get_descriptor(inode, &desc, &desc_size);
+ if (res)
+ return res;
+
+ if (desc->sig_size == 0) {
+ res = -ENODATA;
+ goto out;
+ }
+
+ /*
+ * Include only the signature. Note that fsverity_get_descriptor()
+ * already verified that sig_size is in-bounds.
+ */
+ res = fsverity_read_buffer(buf, offset, length, desc->signature,
+ le32_to_cpu(desc->sig_size));
+out:
+ kfree(desc);
+ return res;
+}
+
/**
* fsverity_ioctl_read_metadata() - read verity metadata from a file
* @filp: file to read the metadata from
@@ -158,6 +186,8 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
length);
case FS_VERITY_METADATA_TYPE_DESCRIPTOR:
return fsverity_read_descriptor(inode, buf, arg.offset, length);
+ case FS_VERITY_METADATA_TYPE_SIGNATURE:
+ return fsverity_read_signature(inode, buf, arg.offset, length);
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
index 41abc283dbccb..15384e22e331e 100644
--- a/include/uapi/linux/fsverity.h
+++ b/include/uapi/linux/fsverity.h
@@ -85,6 +85,7 @@ struct fsverity_formatted_digest {
#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
#define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
+#define FS_VERITY_METADATA_TYPE_SIGNATURE 3
struct fsverity_read_metadata_arg {
__u64 metadata_type;
--
2.30.0
From: Eric Biggers <[email protected]>
Add support for FS_VERITY_METADATA_TYPE_MERKLE_TREE to
FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
retrieve the Merkle tree of a verity file for serving to a client which
implements fs-verity compatible verification. See the patch which
introduced FS_IOC_READ_VERITY_METADATA for more details.
This has been tested using a new xfstest which calls this ioctl via a
new subcommand for the 'fsverity' program from fsverity-utils.
Signed-off-by: Eric Biggers <[email protected]>
---
Documentation/filesystems/fsverity.rst | 10 +++-
fs/verity/read_metadata.c | 70 ++++++++++++++++++++++++++
include/uapi/linux/fsverity.h | 2 +
3 files changed, 81 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 9ef7a7de60085..50b47a6d9ea11 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -234,6 +234,8 @@ need this ioctl.
This ioctl takes in a pointer to the following structure::
+ #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
+
struct fsverity_read_metadata_arg {
__u64 metadata_type;
__u64 offset;
@@ -242,7 +244,13 @@ This ioctl takes in a pointer to the following structure::
__u64 __reserved;
};
-``metadata_type`` specifies the type of metadata to read.
+``metadata_type`` specifies the type of metadata to read:
+
+- ``FS_VERITY_METADATA_TYPE_MERKLE_TREE`` reads the blocks of the
+ Merkle tree. The blocks are returned in order from the root level
+ to the leaf level. Within each level, the blocks are returned in
+ the same order that their hashes are themselves hashed.
+ See `Merkle tree`_ for more information.
The semantics are similar to those of ``pread()``. ``offset``
specifies the offset in bytes into the metadata item to read from, and
diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
index 43be990fd53e4..0f8ad2991cf90 100644
--- a/fs/verity/read_metadata.c
+++ b/fs/verity/read_metadata.c
@@ -7,8 +7,75 @@
#include "fsverity_private.h"
+#include <linux/backing-dev.h>
+#include <linux/highmem.h>
+#include <linux/sched/signal.h>
#include <linux/uaccess.h>
+static int fsverity_read_merkle_tree(struct inode *inode,
+ const struct fsverity_info *vi,
+ void __user *buf, u64 offset, int length)
+{
+ const struct fsverity_operations *vops = inode->i_sb->s_vop;
+ u64 end_offset;
+ unsigned int offs_in_page;
+ pgoff_t index, last_index;
+ int retval = 0;
+ int err = 0;
+
+ end_offset = min(offset + length, vi->tree_params.tree_size);
+ if (offset >= end_offset)
+ return 0;
+ offs_in_page = offset_in_page(offset);
+ last_index = (end_offset - 1) >> PAGE_SHIFT;
+
+ /*
+ * Iterate through each Merkle tree page in the requested range and copy
+ * the requested portion to userspace. Note that the Merkle tree block
+ * size isn't important here, as we are returning a byte stream; i.e.,
+ * we can just work with pages even if the tree block size != PAGE_SIZE.
+ */
+ for (index = offset >> PAGE_SHIFT; index <= last_index; index++) {
+ unsigned long num_ra_pages =
+ min_t(unsigned long, last_index - index + 1,
+ inode->i_sb->s_bdi->io_pages);
+ unsigned int bytes_to_copy = min_t(u64, end_offset - offset,
+ PAGE_SIZE - offs_in_page);
+ struct page *page;
+ const void *virt;
+
+ page = vops->read_merkle_tree_page(inode, index, num_ra_pages);
+ if (IS_ERR(page)) {
+ err = PTR_ERR(page);
+ fsverity_err(inode,
+ "Error %d reading Merkle tree page %lu",
+ err, index);
+ break;
+ }
+
+ virt = kmap(page);
+ if (copy_to_user(buf, virt + offs_in_page, bytes_to_copy)) {
+ kunmap(page);
+ put_page(page);
+ err = -EFAULT;
+ break;
+ }
+ kunmap(page);
+ put_page(page);
+
+ retval += bytes_to_copy;
+ buf += bytes_to_copy;
+ offset += bytes_to_copy;
+
+ if (fatal_signal_pending(current)) {
+ err = -EINTR;
+ break;
+ }
+ cond_resched();
+ offs_in_page = 0;
+ }
+ return retval ? retval : err;
+}
/**
* fsverity_ioctl_read_metadata() - read verity metadata from a file
* @filp: file to read the metadata from
@@ -48,6 +115,9 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
buf = u64_to_user_ptr(arg.buf_ptr);
switch (arg.metadata_type) {
+ case FS_VERITY_METADATA_TYPE_MERKLE_TREE:
+ return fsverity_read_merkle_tree(inode, vi, buf, arg.offset,
+ length);
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
index e062751294d01..94003b153cb3d 100644
--- a/include/uapi/linux/fsverity.h
+++ b/include/uapi/linux/fsverity.h
@@ -83,6 +83,8 @@ struct fsverity_formatted_digest {
__u8 digest[];
};
+#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
+
struct fsverity_read_metadata_arg {
__u64 metadata_type;
__u64 offset;
--
2.30.0
From: Eric Biggers <[email protected]>
The FS_IOC_READ_VERITY_METADATA ioctl will need to return the fs-verity
descriptor (and signature) to userspace.
There are a few ways we could implement this:
- Save a copy of the descriptor (and signature) in the fsverity_info
struct that hangs off of the in-memory inode. However, this would
waste memory since most of the time it wouldn't be needed.
- Regenerate the descriptor from the merkle_tree_params in the
fsverity_info. However, this wouldn't work for the signature, nor for
the salt which the merkle_tree_params only contains indirectly as part
of the 'hashstate'. It would also be error-prone.
- Just get them from the filesystem again. The disadvantage is that in
general we can't trust that they haven't been maliciously changed
since the file has opened. However, the use cases for
FS_IOC_READ_VERITY_METADATA don't require that it verifies the chain
of trust. So this is okay as long as we do some basic validation.
In preparation for implementing the third option, factor out a helper
function fsverity_get_descriptor() which gets the descriptor (and
appended signature) from the filesystem and does some basic validation.
As part of this, start checking the sig_size field for overflow.
Currently fsverity_verify_signature() does this. But the new ioctl will
need this too, so do it earlier.
Signed-off-by: Eric Biggers <[email protected]>
---
fs/verity/fsverity_private.h | 7 +-
fs/verity/open.c | 130 +++++++++++++++++++++++------------
2 files changed, 91 insertions(+), 46 deletions(-)
diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
index 6413d28664d6d..6c9caccc06021 100644
--- a/fs/verity/fsverity_private.h
+++ b/fs/verity/fsverity_private.h
@@ -122,12 +122,17 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
const u8 *salt, size_t salt_size);
struct fsverity_info *fsverity_create_info(const struct inode *inode,
- void *desc, size_t desc_size);
+ struct fsverity_descriptor *desc,
+ size_t desc_size);
void fsverity_set_info(struct inode *inode, struct fsverity_info *vi);
void fsverity_free_info(struct fsverity_info *vi);
+int fsverity_get_descriptor(struct inode *inode,
+ struct fsverity_descriptor **desc_ret,
+ size_t *desc_size_ret);
+
int __init fsverity_init_info_cache(void);
void __init fsverity_exit_info_cache(void);
diff --git a/fs/verity/open.c b/fs/verity/open.c
index 228d0eca3e2e5..a987bb785e9b0 100644
--- a/fs/verity/open.c
+++ b/fs/verity/open.c
@@ -142,45 +142,17 @@ static int compute_file_digest(struct fsverity_hash_alg *hash_alg,
}
/*
- * Validate the given fsverity_descriptor and create a new fsverity_info from
- * it. The signature (if present) is also checked.
+ * Create a new fsverity_info from the given fsverity_descriptor (with optional
+ * appended signature), and check the signature if present. The
+ * fsverity_descriptor must have already undergone basic validation.
*/
struct fsverity_info *fsverity_create_info(const struct inode *inode,
- void *_desc, size_t desc_size)
+ struct fsverity_descriptor *desc,
+ size_t desc_size)
{
- struct fsverity_descriptor *desc = _desc;
struct fsverity_info *vi;
int err;
- if (desc_size < sizeof(*desc)) {
- fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
- desc_size);
- return ERR_PTR(-EINVAL);
- }
-
- if (desc->version != 1) {
- fsverity_err(inode, "Unrecognized descriptor version: %u",
- desc->version);
- return ERR_PTR(-EINVAL);
- }
-
- if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
- fsverity_err(inode, "Reserved bits set in descriptor");
- return ERR_PTR(-EINVAL);
- }
-
- if (desc->salt_size > sizeof(desc->salt)) {
- fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
- return ERR_PTR(-EINVAL);
- }
-
- if (le64_to_cpu(desc->data_size) != inode->i_size) {
- fsverity_err(inode,
- "Wrong data_size: %llu (desc) != %lld (inode)",
- le64_to_cpu(desc->data_size), inode->i_size);
- return ERR_PTR(-EINVAL);
- }
-
vi = kmem_cache_zalloc(fsverity_info_cachep, GFP_KERNEL);
if (!vi)
return ERR_PTR(-ENOMEM);
@@ -245,15 +217,57 @@ void fsverity_free_info(struct fsverity_info *vi)
kmem_cache_free(fsverity_info_cachep, vi);
}
-/* Ensure the inode has an ->i_verity_info */
-static int ensure_verity_info(struct inode *inode)
+static bool validate_fsverity_descriptor(struct inode *inode,
+ const struct fsverity_descriptor *desc,
+ size_t desc_size)
{
- struct fsverity_info *vi = fsverity_get_info(inode);
- struct fsverity_descriptor *desc;
- int res;
+ if (desc_size < sizeof(*desc)) {
+ fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
+ desc_size);
+ return false;
+ }
- if (vi)
- return 0;
+ if (desc->version != 1) {
+ fsverity_err(inode, "Unrecognized descriptor version: %u",
+ desc->version);
+ return false;
+ }
+
+ if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
+ fsverity_err(inode, "Reserved bits set in descriptor");
+ return false;
+ }
+
+ if (desc->salt_size > sizeof(desc->salt)) {
+ fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
+ return false;
+ }
+
+ if (le64_to_cpu(desc->data_size) != inode->i_size) {
+ fsverity_err(inode,
+ "Wrong data_size: %llu (desc) != %lld (inode)",
+ le64_to_cpu(desc->data_size), inode->i_size);
+ return false;
+ }
+
+ if (le32_to_cpu(desc->sig_size) > desc_size - sizeof(*desc)) {
+ fsverity_err(inode, "Signature overflows verity descriptor");
+ return false;
+ }
+
+ return true;
+}
+
+/*
+ * Read the inode's fsverity_descriptor (with optional appended signature) from
+ * the filesystem, and do basic validation of it.
+ */
+int fsverity_get_descriptor(struct inode *inode,
+ struct fsverity_descriptor **desc_ret,
+ size_t *desc_size_ret)
+{
+ int res;
+ struct fsverity_descriptor *desc;
res = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0);
if (res < 0) {
@@ -272,20 +286,46 @@ static int ensure_verity_info(struct inode *inode)
res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, res);
if (res < 0) {
fsverity_err(inode, "Error %d reading verity descriptor", res);
- goto out_free_desc;
+ kfree(desc);
+ return res;
+ }
+
+ if (!validate_fsverity_descriptor(inode, desc, res)) {
+ kfree(desc);
+ return -EINVAL;
}
- vi = fsverity_create_info(inode, desc, res);
+ *desc_ret = desc;
+ *desc_size_ret = res;
+ return 0;
+}
+
+/* Ensure the inode has an ->i_verity_info */
+static int ensure_verity_info(struct inode *inode)
+{
+ struct fsverity_info *vi = fsverity_get_info(inode);
+ struct fsverity_descriptor *desc;
+ size_t desc_size;
+ int err;
+
+ if (vi)
+ return 0;
+
+ err = fsverity_get_descriptor(inode, &desc, &desc_size);
+ if (err)
+ return err;
+
+ vi = fsverity_create_info(inode, desc, desc_size);
if (IS_ERR(vi)) {
- res = PTR_ERR(vi);
+ err = PTR_ERR(vi);
goto out_free_desc;
}
fsverity_set_info(inode, vi);
- res = 0;
+ err = 0;
out_free_desc:
kfree(desc);
- return res;
+ return err;
}
/**
--
2.30.0
From: Eric Biggers <[email protected]>
Add support for FS_VERITY_METADATA_TYPE_DESCRIPTOR to
FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
retrieve the fs-verity descriptor of a file for serving to a client
which implements fs-verity compatible verification. See the patch which
introduced FS_IOC_READ_VERITY_METADATA for more details.
"fs-verity descriptor" here means only the part that userspace cares
about because it is hashed to produce the file digest. It doesn't
include the signature which ext4 and f2fs append to the
fsverity_descriptor struct when storing it on-disk, since that way of
storing the signature is an implementation detail. The next patch adds
a separate metadata_type value for retrieving the signature separately.
This has been tested using a new xfstest which calls this ioctl via a
new subcommand for the 'fsverity' program from fsverity-utils.
Signed-off-by: Eric Biggers <[email protected]>
---
Documentation/filesystems/fsverity.rst | 4 +++
fs/verity/read_metadata.c | 40 ++++++++++++++++++++++++++
include/uapi/linux/fsverity.h | 1 +
3 files changed, 45 insertions(+)
diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
index 50b47a6d9ea11..6dc5772037ef9 100644
--- a/Documentation/filesystems/fsverity.rst
+++ b/Documentation/filesystems/fsverity.rst
@@ -235,6 +235,7 @@ need this ioctl.
This ioctl takes in a pointer to the following structure::
#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
+ #define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
struct fsverity_read_metadata_arg {
__u64 metadata_type;
@@ -252,6 +253,9 @@ This ioctl takes in a pointer to the following structure::
the same order that their hashes are themselves hashed.
See `Merkle tree`_ for more information.
+- ``FS_VERITY_METADATA_TYPE_DESCRIPTOR`` reads the fs-verity
+ descriptor. See `fs-verity descriptor`_.
+
The semantics are similar to those of ``pread()``. ``offset``
specifies the offset in bytes into the metadata item to read from, and
``length`` specifies the maximum number of bytes to read from the
diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
index 0f8ad2991cf90..2dea6dd3bb05a 100644
--- a/fs/verity/read_metadata.c
+++ b/fs/verity/read_metadata.c
@@ -76,6 +76,44 @@ static int fsverity_read_merkle_tree(struct inode *inode,
}
return retval ? retval : err;
}
+
+/* Copy the requested portion of the buffer to userspace. */
+static int fsverity_read_buffer(void __user *dst, u64 offset, int length,
+ const void *src, size_t src_length)
+{
+ if (offset >= src_length)
+ return 0;
+ src += offset;
+ src_length -= offset;
+
+ length = min_t(size_t, length, src_length);
+
+ if (copy_to_user(dst, src, length))
+ return -EFAULT;
+
+ return length;
+}
+
+static int fsverity_read_descriptor(struct inode *inode,
+ void __user *buf, u64 offset, int length)
+{
+ struct fsverity_descriptor *desc;
+ size_t desc_size;
+ int res;
+
+ res = fsverity_get_descriptor(inode, &desc, &desc_size);
+ if (res)
+ return res;
+
+ /* don't include the signature */
+ desc_size = offsetof(struct fsverity_descriptor, signature);
+ desc->sig_size = 0;
+
+ res = fsverity_read_buffer(buf, offset, length, desc, desc_size);
+
+ kfree(desc);
+ return res;
+}
/**
* fsverity_ioctl_read_metadata() - read verity metadata from a file
* @filp: file to read the metadata from
@@ -118,6 +156,8 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
case FS_VERITY_METADATA_TYPE_MERKLE_TREE:
return fsverity_read_merkle_tree(inode, vi, buf, arg.offset,
length);
+ case FS_VERITY_METADATA_TYPE_DESCRIPTOR:
+ return fsverity_read_descriptor(inode, buf, arg.offset, length);
default:
return -EINVAL;
}
diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
index 94003b153cb3d..41abc283dbccb 100644
--- a/include/uapi/linux/fsverity.h
+++ b/include/uapi/linux/fsverity.h
@@ -84,6 +84,7 @@ struct fsverity_formatted_digest {
};
#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
+#define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
struct fsverity_read_metadata_arg {
__u64 metadata_type;
--
2.30.0
LGTM. Thanks!
Reviewed-by: Victor Hsieh <[email protected]>
On Fri, Jan 15, 2021 at 10:19 AM Eric Biggers <[email protected]> wrote:
>
> [This patchset applies to v5.11-rc3]
>
> Add an ioctl FS_IOC_READ_VERITY_METADATA which allows reading verity
> metadata from a file that has fs-verity enabled, including:
>
> - The Merkle tree
> - The fsverity_descriptor (not including the signature if present)
> - The built-in signature, if present
>
> This ioctl has similar semantics to pread(). It is passed the type of
> metadata to read (one of the above three), and a buffer, offset, and
> size. It returns the number of bytes read or an error.
>
> This ioctl doesn't make any assumption about where the metadata is
> stored on-disk. It does assume the metadata is in a stable format, but
> that's basically already the case:
>
> - The Merkle tree and fsverity_descriptor are defined by how fs-verity
> file digests are computed; see the "File digest computation" section
> of Documentation/filesystems/fsverity.rst. Technically, the way in
> which the levels of the tree are ordered relative to each other wasn't
> previously specified, but it's logical to put the root level first.
>
> - The built-in signature is the value passed to FS_IOC_ENABLE_VERITY.
>
> This ioctl is useful because it allows writing a server program that
> takes a verity file and serves it to a client program, such that the
> client can do its own fs-verity compatible verification of the file.
> This only makes sense if the client doesn't trust the server and if the
> server needs to provide the storage for the client.
>
> More concretely, there is interest in using this ability in Android to
> export APK files (which are protected by fs-verity) to "protected VMs".
> This would use Protected KVM (https://lwn.net/Articles/836693), which
> provides an isolated execution environment without having to trust the
> traditional "host". A "guest" VM can boot from a signed image and
> perform specific tasks in a minimum trusted environment using files that
> have fs-verity enabled on the host, without trusting the host or
> requiring that the guest has its own trusted storage.
>
> Technically, it would be possible to duplicate the metadata and store it
> in separate files for serving. However, that would be less efficient
> and would require extra care in userspace to maintain file consistency.
>
> In addition to the above, the ability to read the built-in signatures is
> useful because it allows a system that is using the in-kernel signature
> verification to migrate to userspace signature verification.
>
> This patchset has been tested by new xfstests which call this new ioctl
> via a new subcommand for the 'fsverity' program from fsverity-utils.
>
> Eric Biggers (6):
> fs-verity: factor out fsverity_get_descriptor()
> fs-verity: don't pass whole descriptor to fsverity_verify_signature()
> fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl
> fs-verity: support reading Merkle tree with ioctl
> fs-verity: support reading descriptor with ioctl
> fs-verity: support reading signature with ioctl
>
> Documentation/filesystems/fsverity.rst | 76 ++++++++++
> fs/ext4/ioctl.c | 7 +
> fs/f2fs/file.c | 11 ++
> fs/verity/Makefile | 1 +
> fs/verity/fsverity_private.h | 13 +-
> fs/verity/open.c | 133 +++++++++++------
> fs/verity/read_metadata.c | 195 +++++++++++++++++++++++++
> fs/verity/signature.c | 20 +--
> include/linux/fsverity.h | 12 ++
> include/uapi/linux/fsverity.h | 14 ++
> 10 files changed, 417 insertions(+), 65 deletions(-)
> create mode 100644 fs/verity/read_metadata.c
>
>
> base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
> --
> 2.30.0
>
On Fri, Jan 22, 2021 at 03:26:48PM -0800, Victor Hsieh wrote:
> LGTM. Thanks!
>
> Reviewed-by: Victor Hsieh <[email protected]>
>
> On Fri, Jan 15, 2021 at 10:19 AM Eric Biggers <[email protected]> wrote:
> >
> > [This patchset applies to v5.11-rc3]
> >
> > Add an ioctl FS_IOC_READ_VERITY_METADATA which allows reading verity
> > metadata from a file that has fs-verity enabled, including:
Thanks Victor. Does anyone else have comments on this patchset?
- Eric
On 01/15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Now that fsverity_get_descriptor() validates the sig_size field,
> fsverity_verify_signature() doesn't need to do it.
>
> Just change the prototype of fsverity_verify_signature() to take the
> signature directly rather than take a fsverity_descriptor.
>
> Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
> ---
> fs/verity/fsverity_private.h | 6 ++----
> fs/verity/open.c | 3 ++-
> fs/verity/signature.c | 20 ++++++--------------
> 3 files changed, 10 insertions(+), 19 deletions(-)
>
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index 6c9caccc06021..a7920434bae50 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -140,15 +140,13 @@ void __init fsverity_exit_info_cache(void);
>
> #ifdef CONFIG_FS_VERITY_BUILTIN_SIGNATURES
> int fsverity_verify_signature(const struct fsverity_info *vi,
> - const struct fsverity_descriptor *desc,
> - size_t desc_size);
> + const u8 *signature, size_t sig_size);
>
> int __init fsverity_init_signature(void);
> #else /* !CONFIG_FS_VERITY_BUILTIN_SIGNATURES */
> static inline int
> fsverity_verify_signature(const struct fsverity_info *vi,
> - const struct fsverity_descriptor *desc,
> - size_t desc_size)
> + const u8 *signature, size_t sig_size)
> {
> return 0;
> }
> diff --git a/fs/verity/open.c b/fs/verity/open.c
> index a987bb785e9b0..60ff8af7219fe 100644
> --- a/fs/verity/open.c
> +++ b/fs/verity/open.c
> @@ -181,7 +181,8 @@ struct fsverity_info *fsverity_create_info(const struct inode *inode,
> vi->tree_params.hash_alg->name,
> vi->tree_params.digest_size, vi->file_digest);
>
> - err = fsverity_verify_signature(vi, desc, desc_size);
> + err = fsverity_verify_signature(vi, desc->signature,
> + le32_to_cpu(desc->sig_size));
> out:
> if (err) {
> fsverity_free_info(vi);
> diff --git a/fs/verity/signature.c b/fs/verity/signature.c
> index 012468eda2a78..143a530a80088 100644
> --- a/fs/verity/signature.c
> +++ b/fs/verity/signature.c
> @@ -29,21 +29,19 @@ static struct key *fsverity_keyring;
> /**
> * fsverity_verify_signature() - check a verity file's signature
> * @vi: the file's fsverity_info
> - * @desc: the file's fsverity_descriptor
> - * @desc_size: size of @desc
> + * @signature: the file's built-in signature
> + * @sig_size: size of signature in bytes, or 0 if no signature
> *
> - * If the file's fs-verity descriptor includes a signature of the file digest,
> - * verify it against the certificates in the fs-verity keyring.
> + * If the file includes a signature of its fs-verity file digest, verify it
> + * against the certificates in the fs-verity keyring.
> *
> * Return: 0 on success (signature valid or not required); -errno on failure
> */
> int fsverity_verify_signature(const struct fsverity_info *vi,
> - const struct fsverity_descriptor *desc,
> - size_t desc_size)
> + const u8 *signature, size_t sig_size)
> {
> const struct inode *inode = vi->inode;
> const struct fsverity_hash_alg *hash_alg = vi->tree_params.hash_alg;
> - const u32 sig_size = le32_to_cpu(desc->sig_size);
> struct fsverity_formatted_digest *d;
> int err;
>
> @@ -56,11 +54,6 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
> return 0;
> }
>
> - if (sig_size > desc_size - sizeof(*desc)) {
> - fsverity_err(inode, "Signature overflows verity descriptor");
> - return -EBADMSG;
> - }
> -
> d = kzalloc(sizeof(*d) + hash_alg->digest_size, GFP_KERNEL);
> if (!d)
> return -ENOMEM;
> @@ -70,8 +63,7 @@ int fsverity_verify_signature(const struct fsverity_info *vi,
> memcpy(d->digest, vi->file_digest, hash_alg->digest_size);
>
> err = verify_pkcs7_signature(d, sizeof(*d) + hash_alg->digest_size,
> - desc->signature, sig_size,
> - fsverity_keyring,
> + signature, sig_size, fsverity_keyring,
> VERIFYING_UNSPECIFIED_SIGNATURE,
> NULL, NULL);
> kfree(d);
> --
> 2.30.0
On 01/15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> The FS_IOC_READ_VERITY_METADATA ioctl will need to return the fs-verity
> descriptor (and signature) to userspace.
>
> There are a few ways we could implement this:
>
> - Save a copy of the descriptor (and signature) in the fsverity_info
> struct that hangs off of the in-memory inode. However, this would
> waste memory since most of the time it wouldn't be needed.
>
> - Regenerate the descriptor from the merkle_tree_params in the
> fsverity_info. However, this wouldn't work for the signature, nor for
> the salt which the merkle_tree_params only contains indirectly as part
> of the 'hashstate'. It would also be error-prone.
>
> - Just get them from the filesystem again. The disadvantage is that in
> general we can't trust that they haven't been maliciously changed
> since the file has opened. However, the use cases for
> FS_IOC_READ_VERITY_METADATA don't require that it verifies the chain
> of trust. So this is okay as long as we do some basic validation.
>
> In preparation for implementing the third option, factor out a helper
> function fsverity_get_descriptor() which gets the descriptor (and
> appended signature) from the filesystem and does some basic validation.
>
> As part of this, start checking the sig_size field for overflow.
> Currently fsverity_verify_signature() does this. But the new ioctl will
> need this too, so do it earlier.
>
> Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
> ---
> fs/verity/fsverity_private.h | 7 +-
> fs/verity/open.c | 130 +++++++++++++++++++++++------------
> 2 files changed, 91 insertions(+), 46 deletions(-)
>
> diff --git a/fs/verity/fsverity_private.h b/fs/verity/fsverity_private.h
> index 6413d28664d6d..6c9caccc06021 100644
> --- a/fs/verity/fsverity_private.h
> +++ b/fs/verity/fsverity_private.h
> @@ -122,12 +122,17 @@ int fsverity_init_merkle_tree_params(struct merkle_tree_params *params,
> const u8 *salt, size_t salt_size);
>
> struct fsverity_info *fsverity_create_info(const struct inode *inode,
> - void *desc, size_t desc_size);
> + struct fsverity_descriptor *desc,
> + size_t desc_size);
>
> void fsverity_set_info(struct inode *inode, struct fsverity_info *vi);
>
> void fsverity_free_info(struct fsverity_info *vi);
>
> +int fsverity_get_descriptor(struct inode *inode,
> + struct fsverity_descriptor **desc_ret,
> + size_t *desc_size_ret);
> +
> int __init fsverity_init_info_cache(void);
> void __init fsverity_exit_info_cache(void);
>
> diff --git a/fs/verity/open.c b/fs/verity/open.c
> index 228d0eca3e2e5..a987bb785e9b0 100644
> --- a/fs/verity/open.c
> +++ b/fs/verity/open.c
> @@ -142,45 +142,17 @@ static int compute_file_digest(struct fsverity_hash_alg *hash_alg,
> }
>
> /*
> - * Validate the given fsverity_descriptor and create a new fsverity_info from
> - * it. The signature (if present) is also checked.
> + * Create a new fsverity_info from the given fsverity_descriptor (with optional
> + * appended signature), and check the signature if present. The
> + * fsverity_descriptor must have already undergone basic validation.
> */
> struct fsverity_info *fsverity_create_info(const struct inode *inode,
> - void *_desc, size_t desc_size)
> + struct fsverity_descriptor *desc,
> + size_t desc_size)
> {
> - struct fsverity_descriptor *desc = _desc;
> struct fsverity_info *vi;
> int err;
>
> - if (desc_size < sizeof(*desc)) {
> - fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
> - desc_size);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - if (desc->version != 1) {
> - fsverity_err(inode, "Unrecognized descriptor version: %u",
> - desc->version);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
> - fsverity_err(inode, "Reserved bits set in descriptor");
> - return ERR_PTR(-EINVAL);
> - }
> -
> - if (desc->salt_size > sizeof(desc->salt)) {
> - fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
> - return ERR_PTR(-EINVAL);
> - }
> -
> - if (le64_to_cpu(desc->data_size) != inode->i_size) {
> - fsverity_err(inode,
> - "Wrong data_size: %llu (desc) != %lld (inode)",
> - le64_to_cpu(desc->data_size), inode->i_size);
> - return ERR_PTR(-EINVAL);
> - }
> -
> vi = kmem_cache_zalloc(fsverity_info_cachep, GFP_KERNEL);
> if (!vi)
> return ERR_PTR(-ENOMEM);
> @@ -245,15 +217,57 @@ void fsverity_free_info(struct fsverity_info *vi)
> kmem_cache_free(fsverity_info_cachep, vi);
> }
>
> -/* Ensure the inode has an ->i_verity_info */
> -static int ensure_verity_info(struct inode *inode)
> +static bool validate_fsverity_descriptor(struct inode *inode,
> + const struct fsverity_descriptor *desc,
> + size_t desc_size)
> {
> - struct fsverity_info *vi = fsverity_get_info(inode);
> - struct fsverity_descriptor *desc;
> - int res;
> + if (desc_size < sizeof(*desc)) {
> + fsverity_err(inode, "Unrecognized descriptor size: %zu bytes",
> + desc_size);
> + return false;
> + }
>
> - if (vi)
> - return 0;
> + if (desc->version != 1) {
> + fsverity_err(inode, "Unrecognized descriptor version: %u",
> + desc->version);
> + return false;
> + }
> +
> + if (memchr_inv(desc->__reserved, 0, sizeof(desc->__reserved))) {
> + fsverity_err(inode, "Reserved bits set in descriptor");
> + return false;
> + }
> +
> + if (desc->salt_size > sizeof(desc->salt)) {
> + fsverity_err(inode, "Invalid salt_size: %u", desc->salt_size);
> + return false;
> + }
> +
> + if (le64_to_cpu(desc->data_size) != inode->i_size) {
> + fsverity_err(inode,
> + "Wrong data_size: %llu (desc) != %lld (inode)",
> + le64_to_cpu(desc->data_size), inode->i_size);
> + return false;
> + }
> +
> + if (le32_to_cpu(desc->sig_size) > desc_size - sizeof(*desc)) {
> + fsverity_err(inode, "Signature overflows verity descriptor");
> + return false;
> + }
> +
> + return true;
> +}
> +
> +/*
> + * Read the inode's fsverity_descriptor (with optional appended signature) from
> + * the filesystem, and do basic validation of it.
> + */
> +int fsverity_get_descriptor(struct inode *inode,
> + struct fsverity_descriptor **desc_ret,
> + size_t *desc_size_ret)
> +{
> + int res;
> + struct fsverity_descriptor *desc;
>
> res = inode->i_sb->s_vop->get_verity_descriptor(inode, NULL, 0);
> if (res < 0) {
> @@ -272,20 +286,46 @@ static int ensure_verity_info(struct inode *inode)
> res = inode->i_sb->s_vop->get_verity_descriptor(inode, desc, res);
> if (res < 0) {
> fsverity_err(inode, "Error %d reading verity descriptor", res);
> - goto out_free_desc;
> + kfree(desc);
> + return res;
> + }
> +
> + if (!validate_fsverity_descriptor(inode, desc, res)) {
> + kfree(desc);
> + return -EINVAL;
> }
>
> - vi = fsverity_create_info(inode, desc, res);
> + *desc_ret = desc;
> + *desc_size_ret = res;
> + return 0;
> +}
> +
> +/* Ensure the inode has an ->i_verity_info */
> +static int ensure_verity_info(struct inode *inode)
> +{
> + struct fsverity_info *vi = fsverity_get_info(inode);
> + struct fsverity_descriptor *desc;
> + size_t desc_size;
> + int err;
> +
> + if (vi)
> + return 0;
> +
> + err = fsverity_get_descriptor(inode, &desc, &desc_size);
> + if (err)
> + return err;
> +
> + vi = fsverity_create_info(inode, desc, desc_size);
> if (IS_ERR(vi)) {
> - res = PTR_ERR(vi);
> + err = PTR_ERR(vi);
> goto out_free_desc;
> }
>
> fsverity_set_info(inode, vi);
> - res = 0;
> + err = 0;
> out_free_desc:
> kfree(desc);
> - return res;
> + return err;
> }
>
> /**
> --
> 2.30.0
On 01/15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Add support for FS_VERITY_METADATA_TYPE_MERKLE_TREE to
> FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
> retrieve the Merkle tree of a verity file for serving to a client which
> implements fs-verity compatible verification. See the patch which
> introduced FS_IOC_READ_VERITY_METADATA for more details.
>
> This has been tested using a new xfstest which calls this ioctl via a
> new subcommand for the 'fsverity' program from fsverity-utils.
>
> Signed-off-by: Eric Biggers <[email protected]>
> ---
> Documentation/filesystems/fsverity.rst | 10 +++-
> fs/verity/read_metadata.c | 70 ++++++++++++++++++++++++++
> include/uapi/linux/fsverity.h | 2 +
> 3 files changed, 81 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 9ef7a7de60085..50b47a6d9ea11 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -234,6 +234,8 @@ need this ioctl.
>
> This ioctl takes in a pointer to the following structure::
>
> + #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> +
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> __u64 offset;
> @@ -242,7 +244,13 @@ This ioctl takes in a pointer to the following structure::
> __u64 __reserved;
> };
>
> -``metadata_type`` specifies the type of metadata to read.
> +``metadata_type`` specifies the type of metadata to read:
> +
> +- ``FS_VERITY_METADATA_TYPE_MERKLE_TREE`` reads the blocks of the
> + Merkle tree. The blocks are returned in order from the root level
> + to the leaf level. Within each level, the blocks are returned in
> + the same order that their hashes are themselves hashed.
> + See `Merkle tree`_ for more information.
>
> The semantics are similar to those of ``pread()``. ``offset``
> specifies the offset in bytes into the metadata item to read from, and
> diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
> index 43be990fd53e4..0f8ad2991cf90 100644
> --- a/fs/verity/read_metadata.c
> +++ b/fs/verity/read_metadata.c
> @@ -7,8 +7,75 @@
>
> #include "fsverity_private.h"
>
> +#include <linux/backing-dev.h>
> +#include <linux/highmem.h>
> +#include <linux/sched/signal.h>
> #include <linux/uaccess.h>
>
> +static int fsverity_read_merkle_tree(struct inode *inode,
> + const struct fsverity_info *vi,
> + void __user *buf, u64 offset, int length)
> +{
> + const struct fsverity_operations *vops = inode->i_sb->s_vop;
> + u64 end_offset;
> + unsigned int offs_in_page;
> + pgoff_t index, last_index;
> + int retval = 0;
> + int err = 0;
> +
> + end_offset = min(offset + length, vi->tree_params.tree_size);
> + if (offset >= end_offset)
> + return 0;
> + offs_in_page = offset_in_page(offset);
> + last_index = (end_offset - 1) >> PAGE_SHIFT;
> +
> + /*
> + * Iterate through each Merkle tree page in the requested range and copy
> + * the requested portion to userspace. Note that the Merkle tree block
> + * size isn't important here, as we are returning a byte stream; i.e.,
> + * we can just work with pages even if the tree block size != PAGE_SIZE.
> + */
> + for (index = offset >> PAGE_SHIFT; index <= last_index; index++) {
> + unsigned long num_ra_pages =
> + min_t(unsigned long, last_index - index + 1,
> + inode->i_sb->s_bdi->io_pages);
> + unsigned int bytes_to_copy = min_t(u64, end_offset - offset,
> + PAGE_SIZE - offs_in_page);
> + struct page *page;
> + const void *virt;
> +
> + page = vops->read_merkle_tree_page(inode, index, num_ra_pages);
> + if (IS_ERR(page)) {
> + err = PTR_ERR(page);
> + fsverity_err(inode,
> + "Error %d reading Merkle tree page %lu",
> + err, index);
> + break;
> + }
> +
> + virt = kmap(page);
> + if (copy_to_user(buf, virt + offs_in_page, bytes_to_copy)) {
> + kunmap(page);
> + put_page(page);
> + err = -EFAULT;
> + break;
> + }
> + kunmap(page);
> + put_page(page);
> +
> + retval += bytes_to_copy;
> + buf += bytes_to_copy;
> + offset += bytes_to_copy;
> +
> + if (fatal_signal_pending(current)) {
> + err = -EINTR;
> + break;
> + }
> + cond_resched();
> + offs_in_page = 0;
> + }
Minor thought:
How about invalidating or truncating merkel tree pages?
Reviewed-by: Jaegeuk Kim <[email protected]>
> + return retval ? retval : err;
> +}
> /**
> * fsverity_ioctl_read_metadata() - read verity metadata from a file
> * @filp: file to read the metadata from
> @@ -48,6 +115,9 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
> buf = u64_to_user_ptr(arg.buf_ptr);
>
> switch (arg.metadata_type) {
> + case FS_VERITY_METADATA_TYPE_MERKLE_TREE:
> + return fsverity_read_merkle_tree(inode, vi, buf, arg.offset,
> + length);
> default:
> return -EINVAL;
> }
> diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
> index e062751294d01..94003b153cb3d 100644
> --- a/include/uapi/linux/fsverity.h
> +++ b/include/uapi/linux/fsverity.h
> @@ -83,6 +83,8 @@ struct fsverity_formatted_digest {
> __u8 digest[];
> };
>
> +#define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> +
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> __u64 offset;
> --
> 2.30.0
On 01/15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Add support for FS_VERITY_METADATA_TYPE_SIGNATURE to
> FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
> retrieve the built-in signature (if present) of a verity file for
> serving to a client which implements fs-verity compatible verification.
> See the patch which introduced FS_IOC_READ_VERITY_METADATA for more
> details.
>
> The ability for userspace to read the built-in signatures is also useful
> because it allows a system that is using the in-kernel signature
> verification to migrate to userspace signature verification.
>
> This has been tested using a new xfstest which calls this ioctl via a
> new subcommand for the 'fsverity' program from fsverity-utils.
>
> Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/filesystems/fsverity.rst | 9 +++++++-
> fs/verity/read_metadata.c | 30 ++++++++++++++++++++++++++
> include/uapi/linux/fsverity.h | 1 +
> 3 files changed, 39 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 6dc5772037ef9..1d831e3cbcb33 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -236,6 +236,7 @@ This ioctl takes in a pointer to the following structure::
>
> #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> #define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
> + #define FS_VERITY_METADATA_TYPE_SIGNATURE 3
>
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> @@ -256,6 +257,10 @@ This ioctl takes in a pointer to the following structure::
> - ``FS_VERITY_METADATA_TYPE_DESCRIPTOR`` reads the fs-verity
> descriptor. See `fs-verity descriptor`_.
>
> +- ``FS_VERITY_METADATA_TYPE_SIGNATURE`` reads the signature which was
> + passed to FS_IOC_ENABLE_VERITY, if any. See `Built-in signature
> + verification`_.
> +
> The semantics are similar to those of ``pread()``. ``offset``
> specifies the offset in bytes into the metadata item to read from, and
> ``length`` specifies the maximum number of bytes to read from the
> @@ -279,7 +284,9 @@ FS_IOC_READ_VERITY_METADATA can fail with the following errors:
> - ``EINTR``: the ioctl was interrupted before any data was read
> - ``EINVAL``: reserved fields were set, or ``offset + length``
> overflowed
> -- ``ENODATA``: the file is not a verity file
> +- ``ENODATA``: the file is not a verity file, or
> + FS_VERITY_METADATA_TYPE_SIGNATURE was requested but the file doesn't
> + have a built-in signature
> - ``ENOTTY``: this type of filesystem does not implement fs-verity, or
> this ioctl is not yet implemented on it
> - ``EOPNOTSUPP``: the kernel was not configured with fs-verity
> diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
> index 2dea6dd3bb05a..7e2d0c7bdf0de 100644
> --- a/fs/verity/read_metadata.c
> +++ b/fs/verity/read_metadata.c
> @@ -114,6 +114,34 @@ static int fsverity_read_descriptor(struct inode *inode,
> kfree(desc);
> return res;
> }
> +
> +static int fsverity_read_signature(struct inode *inode,
> + void __user *buf, u64 offset, int length)
> +{
> + struct fsverity_descriptor *desc;
> + size_t desc_size;
> + int res;
> +
> + res = fsverity_get_descriptor(inode, &desc, &desc_size);
> + if (res)
> + return res;
> +
> + if (desc->sig_size == 0) {
> + res = -ENODATA;
> + goto out;
> + }
> +
> + /*
> + * Include only the signature. Note that fsverity_get_descriptor()
> + * already verified that sig_size is in-bounds.
> + */
> + res = fsverity_read_buffer(buf, offset, length, desc->signature,
> + le32_to_cpu(desc->sig_size));
> +out:
> + kfree(desc);
> + return res;
> +}
> +
> /**
> * fsverity_ioctl_read_metadata() - read verity metadata from a file
> * @filp: file to read the metadata from
> @@ -158,6 +186,8 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
> length);
> case FS_VERITY_METADATA_TYPE_DESCRIPTOR:
> return fsverity_read_descriptor(inode, buf, arg.offset, length);
> + case FS_VERITY_METADATA_TYPE_SIGNATURE:
> + return fsverity_read_signature(inode, buf, arg.offset, length);
> default:
> return -EINVAL;
> }
> diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
> index 41abc283dbccb..15384e22e331e 100644
> --- a/include/uapi/linux/fsverity.h
> +++ b/include/uapi/linux/fsverity.h
> @@ -85,6 +85,7 @@ struct fsverity_formatted_digest {
>
> #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> #define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
> +#define FS_VERITY_METADATA_TYPE_SIGNATURE 3
>
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> --
> 2.30.0
On 01/15, Eric Biggers wrote:
> From: Eric Biggers <[email protected]>
>
> Add support for FS_VERITY_METADATA_TYPE_DESCRIPTOR to
> FS_IOC_READ_VERITY_METADATA. This allows a userspace server program to
> retrieve the fs-verity descriptor of a file for serving to a client
> which implements fs-verity compatible verification. See the patch which
> introduced FS_IOC_READ_VERITY_METADATA for more details.
>
> "fs-verity descriptor" here means only the part that userspace cares
> about because it is hashed to produce the file digest. It doesn't
> include the signature which ext4 and f2fs append to the
> fsverity_descriptor struct when storing it on-disk, since that way of
> storing the signature is an implementation detail. The next patch adds
> a separate metadata_type value for retrieving the signature separately.
>
> This has been tested using a new xfstest which calls this ioctl via a
> new subcommand for the 'fsverity' program from fsverity-utils.
>
> Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Jaegeuk Kim <[email protected]>
> ---
> Documentation/filesystems/fsverity.rst | 4 +++
> fs/verity/read_metadata.c | 40 ++++++++++++++++++++++++++
> include/uapi/linux/fsverity.h | 1 +
> 3 files changed, 45 insertions(+)
>
> diff --git a/Documentation/filesystems/fsverity.rst b/Documentation/filesystems/fsverity.rst
> index 50b47a6d9ea11..6dc5772037ef9 100644
> --- a/Documentation/filesystems/fsverity.rst
> +++ b/Documentation/filesystems/fsverity.rst
> @@ -235,6 +235,7 @@ need this ioctl.
> This ioctl takes in a pointer to the following structure::
>
> #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> + #define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
>
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> @@ -252,6 +253,9 @@ This ioctl takes in a pointer to the following structure::
> the same order that their hashes are themselves hashed.
> See `Merkle tree`_ for more information.
>
> +- ``FS_VERITY_METADATA_TYPE_DESCRIPTOR`` reads the fs-verity
> + descriptor. See `fs-verity descriptor`_.
> +
> The semantics are similar to those of ``pread()``. ``offset``
> specifies the offset in bytes into the metadata item to read from, and
> ``length`` specifies the maximum number of bytes to read from the
> diff --git a/fs/verity/read_metadata.c b/fs/verity/read_metadata.c
> index 0f8ad2991cf90..2dea6dd3bb05a 100644
> --- a/fs/verity/read_metadata.c
> +++ b/fs/verity/read_metadata.c
> @@ -76,6 +76,44 @@ static int fsverity_read_merkle_tree(struct inode *inode,
> }
> return retval ? retval : err;
> }
> +
> +/* Copy the requested portion of the buffer to userspace. */
> +static int fsverity_read_buffer(void __user *dst, u64 offset, int length,
> + const void *src, size_t src_length)
> +{
> + if (offset >= src_length)
> + return 0;
> + src += offset;
> + src_length -= offset;
> +
> + length = min_t(size_t, length, src_length);
> +
> + if (copy_to_user(dst, src, length))
> + return -EFAULT;
> +
> + return length;
> +}
> +
> +static int fsverity_read_descriptor(struct inode *inode,
> + void __user *buf, u64 offset, int length)
> +{
> + struct fsverity_descriptor *desc;
> + size_t desc_size;
> + int res;
> +
> + res = fsverity_get_descriptor(inode, &desc, &desc_size);
> + if (res)
> + return res;
> +
> + /* don't include the signature */
> + desc_size = offsetof(struct fsverity_descriptor, signature);
> + desc->sig_size = 0;
> +
> + res = fsverity_read_buffer(buf, offset, length, desc, desc_size);
> +
> + kfree(desc);
> + return res;
> +}
> /**
> * fsverity_ioctl_read_metadata() - read verity metadata from a file
> * @filp: file to read the metadata from
> @@ -118,6 +156,8 @@ int fsverity_ioctl_read_metadata(struct file *filp, const void __user *uarg)
> case FS_VERITY_METADATA_TYPE_MERKLE_TREE:
> return fsverity_read_merkle_tree(inode, vi, buf, arg.offset,
> length);
> + case FS_VERITY_METADATA_TYPE_DESCRIPTOR:
> + return fsverity_read_descriptor(inode, buf, arg.offset, length);
> default:
> return -EINVAL;
> }
> diff --git a/include/uapi/linux/fsverity.h b/include/uapi/linux/fsverity.h
> index 94003b153cb3d..41abc283dbccb 100644
> --- a/include/uapi/linux/fsverity.h
> +++ b/include/uapi/linux/fsverity.h
> @@ -84,6 +84,7 @@ struct fsverity_formatted_digest {
> };
>
> #define FS_VERITY_METADATA_TYPE_MERKLE_TREE 1
> +#define FS_VERITY_METADATA_TYPE_DESCRIPTOR 2
>
> struct fsverity_read_metadata_arg {
> __u64 metadata_type;
> --
> 2.30.0
On Wed, Jan 27, 2021 at 05:10:39PM -0800, Jaegeuk Kim wrote:
>
> Minor thought:
> How about invalidating or truncating merkel tree pages?
>
> Reviewed-by: Jaegeuk Kim <[email protected]>
>
Removing them from the page cache after the read, you mean? I'm not sure we can
assume that users of this ioctl would want the pages to *not* be cached, any
more than we could assume that for any regular read(). I think we should just
leave the pages cached (like a regular read) and not do anything special. Like
other pagecache pages, the kernel will evict the Merkle tree pages eventually if
they aren't being accessed anymore and memory needs to be reclaimed.
- Eric
On Fri, Jan 15, 2021 at 10:22 AM Eric Biggers <[email protected]> wrote:
>
> From: Eric Biggers <[email protected]>
>
> Now that fsverity_get_descriptor() validates the sig_size field,
> fsverity_verify_signature() doesn't need to do it.
>
> Just change the prototype of fsverity_verify_signature() to take the
> signature directly rather than take a fsverity_descriptor.
>
> Signed-off-by: Eric Biggers <[email protected]>
Reviewed-by: Amy Parker <[email protected]>
On Fri, Jan 15, 2021 at 10:18:13AM -0800, Eric Biggers wrote:
> [This patchset applies to v5.11-rc3]
>
> Add an ioctl FS_IOC_READ_VERITY_METADATA which allows reading verity
> metadata from a file that has fs-verity enabled, including:
>
> - The Merkle tree
> - The fsverity_descriptor (not including the signature if present)
> - The built-in signature, if present
>
> This ioctl has similar semantics to pread(). It is passed the type of
> metadata to read (one of the above three), and a buffer, offset, and
> size. It returns the number of bytes read or an error.
>
> This ioctl doesn't make any assumption about where the metadata is
> stored on-disk. It does assume the metadata is in a stable format, but
> that's basically already the case:
>
> - The Merkle tree and fsverity_descriptor are defined by how fs-verity
> file digests are computed; see the "File digest computation" section
> of Documentation/filesystems/fsverity.rst. Technically, the way in
> which the levels of the tree are ordered relative to each other wasn't
> previously specified, but it's logical to put the root level first.
>
> - The built-in signature is the value passed to FS_IOC_ENABLE_VERITY.
>
> This ioctl is useful because it allows writing a server program that
> takes a verity file and serves it to a client program, such that the
> client can do its own fs-verity compatible verification of the file.
> This only makes sense if the client doesn't trust the server and if the
> server needs to provide the storage for the client.
>
> More concretely, there is interest in using this ability in Android to
> export APK files (which are protected by fs-verity) to "protected VMs".
> This would use Protected KVM (https://lwn.net/Articles/836693), which
> provides an isolated execution environment without having to trust the
> traditional "host". A "guest" VM can boot from a signed image and
> perform specific tasks in a minimum trusted environment using files that
> have fs-verity enabled on the host, without trusting the host or
> requiring that the guest has its own trusted storage.
>
> Technically, it would be possible to duplicate the metadata and store it
> in separate files for serving. However, that would be less efficient
> and would require extra care in userspace to maintain file consistency.
>
> In addition to the above, the ability to read the built-in signatures is
> useful because it allows a system that is using the in-kernel signature
> verification to migrate to userspace signature verification.
>
> This patchset has been tested by new xfstests which call this new ioctl
> via a new subcommand for the 'fsverity' program from fsverity-utils.
>
> Eric Biggers (6):
> fs-verity: factor out fsverity_get_descriptor()
> fs-verity: don't pass whole descriptor to fsverity_verify_signature()
> fs-verity: add FS_IOC_READ_VERITY_METADATA ioctl
> fs-verity: support reading Merkle tree with ioctl
> fs-verity: support reading descriptor with ioctl
> fs-verity: support reading signature with ioctl
>
> Documentation/filesystems/fsverity.rst | 76 ++++++++++
> fs/ext4/ioctl.c | 7 +
> fs/f2fs/file.c | 11 ++
> fs/verity/Makefile | 1 +
> fs/verity/fsverity_private.h | 13 +-
> fs/verity/open.c | 133 +++++++++++------
> fs/verity/read_metadata.c | 195 +++++++++++++++++++++++++
> fs/verity/signature.c | 20 +--
> include/linux/fsverity.h | 12 ++
> include/uapi/linux/fsverity.h | 14 ++
> 10 files changed, 417 insertions(+), 65 deletions(-)
> create mode 100644 fs/verity/read_metadata.c
All applied to fscrypt.git#fsverity for 5.12.
- Eric