From 5a9b3d82b90470861fb81fe6e460a6ba37a3a647 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Nov 2024 20:22:42 -0500 Subject: [PATCH 1/5] lib: Add lcfs_fd_measure_fsverity Our history with fsverity APIs is a bit messy. For now historical reasons lcfs_fd_get_fsverity tries to query the kernel (via ioctl) but will silently fall back to userspace computation - which is sometimes desirable, other times not. We also have lcfs_fd_compute_fsverity which is unconditionally userspace. However some cases actually really want to require the fd to have fsverity - so add an API to do that. Signed-off-by: Colin Walters --- libcomposefs/lcfs-writer.c | 39 ++++++++++++++++++++++++-------------- libcomposefs/lcfs-writer.h | 1 + tests/test-lcfs.c | 15 +++++++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c index f7560c6..81826fd 100644 --- a/libcomposefs/lcfs-writer.c +++ b/libcomposefs/lcfs-writer.c @@ -564,10 +564,9 @@ int lcfs_compute_fsverity_from_fd(uint8_t *digest, int fd) return lcfs_compute_fsverity_from_content(digest, &_fd, fsverity_read_cb); } -// Given a file descriptor, first query the kernel for its fsverity digest. If -// it is not available in the kernel, perform an in-memory computation. The file -// position will always be reset to zero if needed. -int lcfs_fd_get_fsverity(uint8_t *digest, int fd) +// Given a file descriptor, query the kernel for its fsverity digest. It +// is an error if fsverity is not enabled. +int lcfs_fd_measure_fsverity(uint8_t *digest, int fd) { char buf[sizeof(struct fsverity_digest) + MAX_DIGEST_SIZE]; struct fsverity_digest *fsv = (struct fsverity_digest *)&buf; @@ -577,16 +576,6 @@ int lcfs_fd_get_fsverity(uint8_t *digest, int fd) fsv->digest_size = MAX_DIGEST_SIZE; int res = ioctl(fd, FS_IOC_MEASURE_VERITY, fsv); if (res == -1) { - // Under this condition, the file didn't have fsverity enabled or the - // kernel doesn't support it at all. We need to compute it in the current process. - if (errno == ENODATA || errno == EOPNOTSUPP || errno == ENOTTY) { - // For consistency ensure we start from the beginning. We could - // avoid this by using pread() in the future. - if (lseek(fd, 0, SEEK_SET) < 0) - return -errno; - return lcfs_compute_fsverity_from_fd(digest, fd); - } - // In this case, we found an unexpected error return -errno; } // The file has fsverity enabled, but with an unexpected different algorithm (e.g. sha512). @@ -600,6 +589,28 @@ int lcfs_fd_get_fsverity(uint8_t *digest, int fd) return 0; } +// Given a file descriptor, first query the kernel for its fsverity digest. If +// it is not available in the kernel, perform an in-memory computation. The file +// position will always be reset to zero if needed. +int lcfs_fd_get_fsverity(uint8_t *digest, int fd) +{ + int res = lcfs_fd_measure_fsverity(digest, fd); + if (res == 0) { + return 0; + } + // Under this condition, the file didn't have fsverity enabled or the + // kernel doesn't support it at all. We need to compute it in the current process. + if (errno == ENODATA || errno == EOPNOTSUPP || errno == ENOTTY) { + // For consistency ensure we start from the beginning. We could + // avoid this by using pread() in the future. + if (lseek(fd, 0, SEEK_SET) < 0) + return -errno; + return lcfs_compute_fsverity_from_fd(digest, fd); + } + // In this case, we found an unexpected error + return -errno; +} + int lcfs_compute_fsverity_from_data(uint8_t *digest, uint8_t *data, size_t data_len) { FsVerityContext *ctx; diff --git a/libcomposefs/lcfs-writer.h b/libcomposefs/lcfs-writer.h index 212164a..a68564f 100644 --- a/libcomposefs/lcfs-writer.h +++ b/libcomposefs/lcfs-writer.h @@ -182,6 +182,7 @@ LCFS_EXTERN int lcfs_compute_fsverity_from_content(uint8_t *digest, void *file, LCFS_EXTERN int lcfs_compute_fsverity_from_fd(uint8_t *digest, int fd); LCFS_EXTERN int lcfs_compute_fsverity_from_data(uint8_t *digest, uint8_t *data, size_t data_len); +LCFS_EXTERN int lcfs_fd_measure_fsverity(uint8_t *digest, int fd); LCFS_EXTERN int lcfs_fd_get_fsverity(uint8_t *digest, int fd); LCFS_EXTERN int lcfs_node_set_from_content(struct lcfs_node_s *node, int dirfd, diff --git a/tests/test-lcfs.c b/tests/test-lcfs.c index 9f1ac1d..e8d5f7f 100644 --- a/tests/test-lcfs.c +++ b/tests/test-lcfs.c @@ -3,6 +3,7 @@ #include "lcfs-writer.h" #include +#include #include static inline void lcfs_node_unrefp(struct lcfs_node_s **nodep) @@ -75,8 +76,22 @@ static void test_add_uninitialized_child(void) assert(errno == EINVAL); } +// Verifies that lcfs_fd_measure_fsverity fails on a fd without fsverity +static void test_no_verity(void) +{ + char buf[] = "/tmp/test-verity.XXXXXX"; + int tmpfd = mkstemp(buf); + assert(tmpfd > 0); + + uint8_t digest[LCFS_DIGEST_SIZE]; + int r = lcfs_fd_require_fsverity(digest, tmpfd); + assert(r != 0); + close(tmpfd); +} + int main(int argc, char **argv) { test_basic(); + test_no_verity(); test_add_uninitialized_child(); } From 19bca5538e4600c04e1f301ab1ef19a5ba3fb935 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Nov 2024 20:28:32 -0500 Subject: [PATCH 2/5] writer: Canonicalize no-verity errno to -ENOVERITY This is what we do elsewhere. Signed-off-by: Colin Walters --- libcomposefs/lcfs-writer.c | 4 ++++ tests/test-lcfs.c | 5 ++++- 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c index 81826fd..18c20f8 100644 --- a/libcomposefs/lcfs-writer.c +++ b/libcomposefs/lcfs-writer.c @@ -576,6 +576,10 @@ int lcfs_fd_measure_fsverity(uint8_t *digest, int fd) fsv->digest_size = MAX_DIGEST_SIZE; int res = ioctl(fd, FS_IOC_MEASURE_VERITY, fsv); if (res == -1) { + if (errno == ENODATA || errno == EOPNOTSUPP || errno == ENOTTY) { + // Canonicalize errno + errno = ENOVERITY; + } return -errno; } // The file has fsverity enabled, but with an unexpected different algorithm (e.g. sha512). diff --git a/tests/test-lcfs.c b/tests/test-lcfs.c index e8d5f7f..59f0c13 100644 --- a/tests/test-lcfs.c +++ b/tests/test-lcfs.c @@ -2,6 +2,7 @@ #define _GNU_SOURCE #include "lcfs-writer.h" +#include "lcfs-mount.h" #include #include #include @@ -84,8 +85,10 @@ static void test_no_verity(void) assert(tmpfd > 0); uint8_t digest[LCFS_DIGEST_SIZE]; - int r = lcfs_fd_require_fsverity(digest, tmpfd); + int r = lcfs_fd_measure_fsverity(digest, tmpfd); + int errsv = errno; assert(r != 0); + assert(errsv == ENOVERITY); close(tmpfd); } From 52d98a4ee06879df91ebe4620ea2ed632dbbb8b2 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 5 Nov 2024 20:29:08 -0500 Subject: [PATCH 3/5] lib/mount: Use lcfs_fd_measure_fsverity This is ensuring we have our fsverity ioctl parsing code in one place. Signed-off-by: Colin Walters --- libcomposefs/lcfs-mount.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/libcomposefs/lcfs-mount.c b/libcomposefs/lcfs-mount.c index 916d984..780d6d1 100644 --- a/libcomposefs/lcfs-mount.c +++ b/libcomposefs/lcfs-mount.c @@ -210,20 +210,15 @@ static errint_t lcfs_validate_mount_options(struct lcfs_mount_state_s *state) static errint_t lcfs_validate_verity_fd(struct lcfs_mount_state_s *state) { - char buf[sizeof(struct fsverity_digest) + MAX_DIGEST_SIZE]; - struct fsverity_digest *fsv = (struct fsverity_digest *)&buf; int res; if (state->expected_digest_len != 0) { - fsv->digest_size = MAX_DIGEST_SIZE; - res = ioctl(state->fd, FS_IOC_MEASURE_VERITY, fsv); - if (res == -1) { - if (errno == ENODATA || errno == EOPNOTSUPP || errno == ENOTTY) - return -ENOVERITY; - return -errno; + uint8_t found_digest[LCFS_DIGEST_SIZE]; + res = lcfs_fd_measure_fsverity(found_digest, state->fd); + if (res < 0) { + return res; } - if (fsv->digest_size != state->expected_digest_len || - memcmp(state->expected_digest, fsv->digest, fsv->digest_size) != 0) + if (memcmp(state->expected_digest, found_digest, LCFS_DIGEST_SIZE) != 0) return -EWRONGVERITY; } From d771778e05a8afb6dc22f56601bcc436b26dde7e Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 6 Nov 2024 08:24:57 -0500 Subject: [PATCH 4/5] tests: Handle ENOSYS in qemu on cross-arch emulation We may get ENOSYS from qemu userspace emulation not implementing the ioctl. Signed-off-by: Colin Walters --- .github/workflows/test.yaml | 2 +- tests/test-lcfs.c | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yaml b/.github/workflows/test.yaml index 01119ba..51c9c69 100644 --- a/.github/workflows/test.yaml +++ b/.github/workflows/test.yaml @@ -141,7 +141,7 @@ jobs: ./hacking/installdeps.sh meson setup build --werror meson compile -C build - meson test -C build --timeout-multiplier 10 + env CFS_TEST_ARCH_EMULATION=${{ matrix.arch }} meson test -C build --timeout-multiplier 10 - name: Upload log uses: actions/upload-artifact@v4 if: always() diff --git a/tests/test-lcfs.c b/tests/test-lcfs.c index 59f0c13..527271d 100644 --- a/tests/test-lcfs.c +++ b/tests/test-lcfs.c @@ -88,7 +88,9 @@ static void test_no_verity(void) int r = lcfs_fd_measure_fsverity(digest, tmpfd); int errsv = errno; assert(r != 0); - assert(errsv == ENOVERITY); + // We may get ENOSYS from qemu userspace emulation not implementing the ioctl + if (getenv("CFS_TEST_ARCH_EMULATION") == NULL) + assert(errsv == ENOVERITY); close(tmpfd); } From 69078b9f8216d97a0637dc09a73f4dd7c1648ab4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Fri, 15 Nov 2024 15:59:06 +0000 Subject: [PATCH 5/5] writer: Use a union for stack buffer to ensure alignment Adapted from an equivalent patch by Simon for ostree: https://github.com/ostreedev/ostree/pull/3340/commits/67ed2acad47642c5253d66aa70b5035687ed5728 Reported-by: Simon McVittie Signed-off-by: Colin Walters --- libcomposefs/lcfs-writer.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/libcomposefs/lcfs-writer.c b/libcomposefs/lcfs-writer.c index 18c20f8..f6e202e 100644 --- a/libcomposefs/lcfs-writer.c +++ b/libcomposefs/lcfs-writer.c @@ -568,13 +568,15 @@ int lcfs_compute_fsverity_from_fd(uint8_t *digest, int fd) // is an error if fsverity is not enabled. int lcfs_fd_measure_fsverity(uint8_t *digest, int fd) { - char buf[sizeof(struct fsverity_digest) + MAX_DIGEST_SIZE]; - struct fsverity_digest *fsv = (struct fsverity_digest *)&buf; + union { + struct fsverity_digest fsv; + char buf[sizeof(struct fsverity_digest) + MAX_DIGEST_SIZE]; + } result; // First, ask the kernel if the file already has fsverity; if so we just return // that. - fsv->digest_size = MAX_DIGEST_SIZE; - int res = ioctl(fd, FS_IOC_MEASURE_VERITY, fsv); + result.fsv.digest_size = MAX_DIGEST_SIZE; + int res = ioctl(fd, FS_IOC_MEASURE_VERITY, &result); if (res == -1) { if (errno == ENODATA || errno == EOPNOTSUPP || errno == ENOTTY) { // Canonicalize errno @@ -584,11 +586,11 @@ int lcfs_fd_measure_fsverity(uint8_t *digest, int fd) } // The file has fsverity enabled, but with an unexpected different algorithm (e.g. sha512). // This is going to be a weird corner case. For now, we error out. - if (fsv->digest_size != LCFS_DIGEST_SIZE) { + if (result.fsv.digest_size != LCFS_DIGEST_SIZE) { return -EWRONGVERITY; } - memcpy(digest, buf + sizeof(struct fsverity_digest), LCFS_DIGEST_SIZE); + memcpy(digest, result.buf + sizeof(struct fsverity_digest), LCFS_DIGEST_SIZE); return 0; }