[fsverity-utils PATCH 0/2] Add libfsverity_enable() API

[fsverity-utils PATCH 0/2] Add libfsverity_enable() API

[Eric Biggers]

This patchset adds wrappers around FS_IOC_ENABLE_VERITY to the library
API, and makes the fsverity commands share code to parse the
libfsverity_merkle_tree_params.

This is my proposed alternative to Luca's patch
https://lkml.kernel.org/linux-fscrypt/20201113143527.1097499-1-luca.boccassi@gmail.com

Eric Biggers (2):
  lib: add libfsverity_enable() and libfsverity_enable_with_sig()
  programs/fsverity: share code to parse tree parameters

 include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
 lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
 programs/cmd_digest.c | 31 ++++-------------------------
 programs/cmd_enable.c | 34 ++++++++------------------------
 programs/cmd_sign.c   | 31 ++++-------------------------
 programs/fsverity.c   | 42 ++++++++++++++++++++++++++++++++++++----
 programs/fsverity.h   | 19 ++++++++++++++----
 7 files changed, 150 insertions(+), 88 deletions(-)
 create mode 100644 lib/enable.c

-- 
2.29.2

[fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
'struct libfsverity_merkle_tree_params' instead of
'struct fsverity_enable_arg'.  This is useful because it allows
libfsverity users to deal with one common struct.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
 lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
 programs/cmd_enable.c | 28 +++++++++++++++------------
 3 files changed, 97 insertions(+), 12 deletions(-)
 create mode 100644 lib/enable.c

diff --git a/include/libfsverity.h b/include/libfsverity.h
index 8f78a13..a8aecaf 100644
--- a/include/libfsverity.h
+++ b/include/libfsverity.h
@@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
 			const struct libfsverity_signature_params *sig_params,
 			uint8_t **sig_ret, size_t *sig_size_ret);
 
+/**
+ * libfsverity_enable() - Enable fs-verity on a file
+ * @fd: read-only file descriptor to the file
+ * @params: pointer to the Merkle tree parameters
+ *
+ * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
+ *
+ * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
+ *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
+ *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
+ *	   the possible error codes from FS_IOC_ENABLE_VERITY.
+ */
+int
+libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
+
+/**
+ * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
+ * @fd: read-only file descriptor to the file
+ * @params: pointer to the Merkle tree parameters
+ * @sig: pointer to the file's signature
+ * @sig_size: size of the file's signature in bytes
+ *
+ * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
+ * singature created with libfsverity_sign_digest()) to associate with the file.
+ * This is only needed if the in-kernel signature verification support is being
+ * used; it is not needed if signatures are being verified in userspace.
+ *
+ * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
+ *
+ * Return: See libfsverity_enable().
+ */
+int
+libfsverity_enable_with_sig(int fd,
+			    const struct libfsverity_merkle_tree_params *params,
+			    const uint8_t *sig, size_t sig_size);
+
 /**
  * libfsverity_find_hash_alg_by_name() - Find hash algorithm by name
  * @name: Pointer to name of hash algorithm
diff --git a/lib/enable.c b/lib/enable.c
new file mode 100644
index 0000000..dd77292
--- /dev/null
+++ b/lib/enable.c
@@ -0,0 +1,45 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Implementation of libfsverity_enable() and libfsverity_enable_with_sig().
+ *
+ * Copyright 2020 Google LLC
+ *
+ * Use of this source code is governed by an MIT-style
+ * license that can be found in the LICENSE file or at
+ * https://opensource.org/licenses/MIT.
+ */
+
+#include "lib_private.h"
+
+#include <sys/ioctl.h>
+
+LIBEXPORT int
+libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params)
+{
+	return libfsverity_enable_with_sig(fd, params, NULL, 0);
+}
+
+LIBEXPORT int
+libfsverity_enable_with_sig(int fd,
+			    const struct libfsverity_merkle_tree_params *params,
+			    const uint8_t *sig, size_t sig_size)
+{
+	struct fsverity_enable_arg arg = {};
+
+	if (!params) {
+		libfsverity_error_msg("missing required parameters for enable");
+		return -EINVAL;
+	}
+
+	arg.version = 1;
+	arg.hash_algorithm = params->hash_algorithm;
+	arg.block_size = params->block_size;
+	arg.salt_size = params->salt_size;
+	arg.salt_ptr = (uintptr_t)params->salt;
+	arg.sig_size = sig_size;
+	arg.sig_ptr = (uintptr_t)sig;
+
+	if (ioctl(fd, FS_IOC_ENABLE_VERITY, &arg) != 0)
+		return -errno;
+	return 0;
+}
diff --git a/programs/cmd_enable.c b/programs/cmd_enable.c
index d90d208..48d33c2 100644
--- a/programs/cmd_enable.c
+++ b/programs/cmd_enable.c
@@ -68,9 +68,10 @@ static const struct option longopts[] = {
 int fsverity_cmd_enable(const struct fsverity_command *cmd,
 			int argc, char *argv[])
 {
-	struct fsverity_enable_arg arg = { .version = 1 };
+	struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
 	u8 *salt = NULL;
 	u8 *sig = NULL;
+	u32 sig_size = 0;
 	struct filedes file;
 	int status;
 	int c;
@@ -78,26 +79,28 @@ int fsverity_cmd_enable(const struct fsverity_command *cmd,
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_HASH_ALG:
-			if (!parse_hash_alg_option(optarg, &arg.hash_algorithm))
+			if (!parse_hash_alg_option(optarg,
+						   &tree_params.hash_algorithm))
 				goto out_usage;
 			break;
 		case OPT_BLOCK_SIZE:
-			if (!parse_block_size_option(optarg, &arg.block_size))
+			if (!parse_block_size_option(optarg,
+						     &tree_params.block_size))
 				goto out_usage;
 			break;
 		case OPT_SALT:
-			if (!parse_salt_option(optarg, &salt, &arg.salt_size))
+			if (!parse_salt_option(optarg, &salt,
+					       &tree_params.salt_size))
 				goto out_usage;
-			arg.salt_ptr = (uintptr_t)salt;
+			tree_params.salt = salt;
 			break;
 		case OPT_SIGNATURE:
 			if (sig != NULL) {
 				error_msg("--signature can only be specified once");
 				goto out_usage;
 			}
-			if (!read_signature(optarg, &sig, &arg.sig_size))
+			if (!read_signature(optarg, &sig, &sig_size))
 				goto out_err;
-			arg.sig_ptr = (uintptr_t)sig;
 			break;
 		default:
 			goto out_usage;
@@ -110,15 +113,16 @@ int fsverity_cmd_enable(const struct fsverity_command *cmd,
 	if (argc != 1)
 		goto out_usage;
 
-	if (arg.hash_algorithm == 0)
-		arg.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
+	if (tree_params.hash_algorithm == 0)
+		tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
 
-	if (arg.block_size == 0)
-		arg.block_size = get_default_block_size();
+	if (tree_params.block_size == 0)
+		tree_params.block_size = get_default_block_size();
 
 	if (!open_file(&file, argv[0], O_RDONLY, 0))
 		goto out_err;
-	if (ioctl(file.fd, FS_IOC_ENABLE_VERITY, &arg) != 0) {
+
+	if (libfsverity_enable_with_sig(file.fd, &tree_params, sig, sig_size)) {
 		error_msg_errno("FS_IOC_ENABLE_VERITY failed on '%s'",
 				file.name);
 		filedes_close(&file);
-- 
2.29.2

[fsverity-utils PATCH 2/2] programs/fsverity: share code to parse tree parameters

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

The "digest", "enable", and "sign" commands all parse the --hash-alg,
--block-size, and --salt options and initialize a struct
libfsverity_merkle_tree_params, so share the code that does this.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 programs/cmd_digest.c | 31 ++++---------------------------
 programs/cmd_enable.c | 30 ++++--------------------------
 programs/cmd_sign.c   | 31 ++++---------------------------
 programs/fsverity.c   | 42 ++++++++++++++++++++++++++++++++++++++----
 programs/fsverity.h   | 19 +++++++++++++++----
 5 files changed, 65 insertions(+), 88 deletions(-)

diff --git a/programs/cmd_digest.c b/programs/cmd_digest.c
index 180f438..e420d17 100644
--- a/programs/cmd_digest.c
+++ b/programs/cmd_digest.c
@@ -14,14 +14,6 @@
 #include <fcntl.h>
 #include <getopt.h>
 
-enum {
-	OPT_HASH_ALG,
-	OPT_BLOCK_SIZE,
-	OPT_SALT,
-	OPT_COMPACT,
-	OPT_FOR_BUILTIN_SIG,
-};
-
 static const struct option longopts[] = {
 	{"hash-alg",		required_argument, NULL, OPT_HASH_ALG},
 	{"block-size",		required_argument, NULL, OPT_BLOCK_SIZE},
@@ -44,9 +36,8 @@ struct fsverity_signed_digest {
 int fsverity_cmd_digest(const struct fsverity_command *cmd,
 		      int argc, char *argv[])
 {
-	u8 *salt = NULL;
 	struct filedes file = { .fd = -1 };
-	struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
+	struct libfsverity_merkle_tree_params tree_params = {};
 	bool compact = false, for_builtin_sig = false;
 	int status;
 	int c;
@@ -54,20 +45,10 @@ int fsverity_cmd_digest(const struct fsverity_command *cmd,
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_HASH_ALG:
-			if (!parse_hash_alg_option(optarg,
-						   &tree_params.hash_algorithm))
-				goto out_usage;
-			break;
 		case OPT_BLOCK_SIZE:
-			if (!parse_block_size_option(optarg,
-						     &tree_params.block_size))
-				goto out_usage;
-			break;
 		case OPT_SALT:
-			if (!parse_salt_option(optarg, &salt,
-					       &tree_params.salt_size))
+			if (!parse_tree_param(c, optarg, &tree_params))
 				goto out_usage;
-			tree_params.salt = salt;
 			break;
 		case OPT_COMPACT:
 			compact = true;
@@ -86,11 +67,7 @@ int fsverity_cmd_digest(const struct fsverity_command *cmd,
 	if (argc < 1)
 		goto out_usage;
 
-	if (tree_params.hash_algorithm == 0)
-		tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
-
-	if (tree_params.block_size == 0)
-		tree_params.block_size = get_default_block_size();
+	finalize_tree_params(&tree_params);
 
 	for (int i = 0; i < argc; i++) {
 		struct fsverity_signed_digest *d = NULL;
@@ -146,7 +123,7 @@ int fsverity_cmd_digest(const struct fsverity_command *cmd,
 	}
 	status = 0;
 out:
-	free(salt);
+	destroy_tree_params(&tree_params);
 	return status;
 
 out_err:
diff --git a/programs/cmd_enable.c b/programs/cmd_enable.c
index 48d33c2..3c722e5 100644
--- a/programs/cmd_enable.c
+++ b/programs/cmd_enable.c
@@ -49,13 +49,6 @@ out:
 	return ok;
 }
 
-enum {
-	OPT_HASH_ALG,
-	OPT_BLOCK_SIZE,
-	OPT_SALT,
-	OPT_SIGNATURE,
-};
-
 static const struct option longopts[] = {
 	{"hash-alg",	required_argument, NULL, OPT_HASH_ALG},
 	{"block-size",	required_argument, NULL, OPT_BLOCK_SIZE},
@@ -68,8 +61,7 @@ static const struct option longopts[] = {
 int fsverity_cmd_enable(const struct fsverity_command *cmd,
 			int argc, char *argv[])
 {
-	struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
-	u8 *salt = NULL;
+	struct libfsverity_merkle_tree_params tree_params = {};
 	u8 *sig = NULL;
 	u32 sig_size = 0;
 	struct filedes file;
@@ -79,20 +71,10 @@ int fsverity_cmd_enable(const struct fsverity_command *cmd,
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_HASH_ALG:
-			if (!parse_hash_alg_option(optarg,
-						   &tree_params.hash_algorithm))
-				goto out_usage;
-			break;
 		case OPT_BLOCK_SIZE:
-			if (!parse_block_size_option(optarg,
-						     &tree_params.block_size))
-				goto out_usage;
-			break;
 		case OPT_SALT:
-			if (!parse_salt_option(optarg, &salt,
-					       &tree_params.salt_size))
+			if (!parse_tree_param(c, optarg, &tree_params))
 				goto out_usage;
-			tree_params.salt = salt;
 			break;
 		case OPT_SIGNATURE:
 			if (sig != NULL) {
@@ -113,11 +95,7 @@ int fsverity_cmd_enable(const struct fsverity_command *cmd,
 	if (argc != 1)
 		goto out_usage;
 
-	if (tree_params.hash_algorithm == 0)
-		tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
-
-	if (tree_params.block_size == 0)
-		tree_params.block_size = get_default_block_size();
+	finalize_tree_params(&tree_params);
 
 	if (!open_file(&file, argv[0], O_RDONLY, 0))
 		goto out_err;
@@ -133,7 +111,7 @@ int fsverity_cmd_enable(const struct fsverity_command *cmd,
 
 	status = 0;
 out:
-	free(salt);
+	destroy_tree_params(&tree_params);
 	free(sig);
 	return status;
 
diff --git a/programs/cmd_sign.c b/programs/cmd_sign.c
index 580e4df..fb17b8a 100644
--- a/programs/cmd_sign.c
+++ b/programs/cmd_sign.c
@@ -26,14 +26,6 @@ static bool write_signature(const char *filename, const u8 *sig, u32 sig_size)
 	return ok;
 }
 
-enum {
-	OPT_HASH_ALG,
-	OPT_BLOCK_SIZE,
-	OPT_SALT,
-	OPT_KEY,
-	OPT_CERT,
-};
-
 static const struct option longopts[] = {
 	{"hash-alg",	required_argument, NULL, OPT_HASH_ALG},
 	{"block-size",	required_argument, NULL, OPT_BLOCK_SIZE},
@@ -48,8 +40,7 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 		      int argc, char *argv[])
 {
 	struct filedes file = { .fd = -1 };
-	u8 *salt = NULL;
-	struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
+	struct libfsverity_merkle_tree_params tree_params = {};
 	struct libfsverity_signature_params sig_params = {};
 	struct libfsverity_digest *digest = NULL;
 	char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + 1];
@@ -61,20 +52,10 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_HASH_ALG:
-			if (!parse_hash_alg_option(optarg,
-						   &tree_params.hash_algorithm))
-				goto out_usage;
-			break;
 		case OPT_BLOCK_SIZE:
-			if (!parse_block_size_option(optarg,
-						     &tree_params.block_size))
-				goto out_usage;
-			break;
 		case OPT_SALT:
-			if (!parse_salt_option(optarg, &salt,
-					       &tree_params.salt_size))
+			if (!parse_tree_param(c, optarg, &tree_params))
 				goto out_usage;
-			tree_params.salt = salt;
 			break;
 		case OPT_KEY:
 			if (sig_params.keyfile != NULL) {
@@ -101,11 +82,7 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 	if (argc != 2)
 		goto out_usage;
 
-	if (tree_params.hash_algorithm == 0)
-		tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
-
-	if (tree_params.block_size == 0)
-		tree_params.block_size = get_default_block_size();
+	finalize_tree_params(&tree_params);
 
 	if (sig_params.keyfile == NULL) {
 		error_msg("Missing --key argument");
@@ -143,7 +120,7 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 	status = 0;
 out:
 	filedes_close(&file);
-	free(salt);
+	destroy_tree_params(&tree_params);
 	free(digest);
 	free(sig);
 	return status;
diff --git a/programs/fsverity.c b/programs/fsverity.c
index 4a2f8df..052a640 100644
--- a/programs/fsverity.c
+++ b/programs/fsverity.c
@@ -134,7 +134,7 @@ static const struct fsverity_command *find_command(const char *name)
 	return NULL;
 }
 
-bool parse_hash_alg_option(const char *arg, u32 *alg_ptr)
+static bool parse_hash_alg_option(const char *arg, u32 *alg_ptr)
 {
 	char *end;
 	unsigned long n = strtoul(arg, &end, 10);
@@ -159,7 +159,7 @@ bool parse_hash_alg_option(const char *arg, u32 *alg_ptr)
 	return false;
 }
 
-bool parse_block_size_option(const char *arg, u32 *size_ptr)
+static bool parse_block_size_option(const char *arg, u32 *size_ptr)
 {
 	char *end;
 	unsigned long n = strtoul(arg, &end, 10);
@@ -177,7 +177,8 @@ bool parse_block_size_option(const char *arg, u32 *size_ptr)
 	return true;
 }
 
-bool parse_salt_option(const char *arg, u8 **salt_ptr, u32 *salt_size_ptr)
+static bool parse_salt_option(const char *arg, u8 **salt_ptr,
+			      u32 *salt_size_ptr)
 {
 	if (*salt_ptr != NULL) {
 		error_msg("--salt can only be specified once");
@@ -192,7 +193,23 @@ bool parse_salt_option(const char *arg, u8 **salt_ptr, u32 *salt_size_ptr)
 	return true;
 }
 
-u32 get_default_block_size(void)
+bool parse_tree_param(int opt_char, const char *arg,
+		      struct libfsverity_merkle_tree_params *params)
+{
+	switch (opt_char) {
+	case OPT_HASH_ALG:
+		return parse_hash_alg_option(arg, &params->hash_algorithm);
+	case OPT_BLOCK_SIZE:
+		return parse_block_size_option(arg, &params->block_size);
+	case OPT_SALT:
+		return parse_salt_option(arg, (u8 **)&params->salt,
+					 &params->salt_size);
+	default:
+		ASSERT(0);
+	}
+}
+
+static u32 get_default_block_size(void)
 {
 	long n = sysconf(_SC_PAGESIZE);
 
@@ -205,6 +222,23 @@ u32 get_default_block_size(void)
 	return n;
 }
 
+void finalize_tree_params(struct libfsverity_merkle_tree_params *params)
+{
+	params->version = 1;
+
+	if (params->hash_algorithm == 0)
+		params->hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
+
+	if (params->block_size == 0)
+		params->block_size = get_default_block_size();
+}
+
+void destroy_tree_params(struct libfsverity_merkle_tree_params *params)
+{
+	free((u8 *)params->salt);
+	memset(params, 0, sizeof(*params));
+}
+
 int main(int argc, char *argv[])
 {
 	const struct fsverity_command *cmd;
diff --git a/programs/fsverity.h b/programs/fsverity.h
index 669fef2..51bba32 100644
--- a/programs/fsverity.h
+++ b/programs/fsverity.h
@@ -23,6 +23,17 @@
  */
 #define FS_VERITY_MAX_DIGEST_SIZE	64
 
+enum {
+	OPT_BLOCK_SIZE,
+	OPT_CERT,
+	OPT_COMPACT,
+	OPT_FOR_BUILTIN_SIG,
+	OPT_HASH_ALG,
+	OPT_KEY,
+	OPT_SALT,
+	OPT_SIGNATURE,
+};
+
 struct fsverity_command;
 
 /* cmd_digest.c */
@@ -43,9 +54,9 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 
 /* fsverity.c */
 void usage(const struct fsverity_command *cmd, FILE *fp);
-bool parse_hash_alg_option(const char *arg, u32 *alg_ptr);
-bool parse_block_size_option(const char *arg, u32 *size_ptr);
-bool parse_salt_option(const char *arg, u8 **salt_ptr, u32 *salt_size_ptr);
-u32 get_default_block_size(void);
+bool parse_tree_param(int opt_char, const char *arg,
+		      struct libfsverity_merkle_tree_params *params);
+void finalize_tree_params(struct libfsverity_merkle_tree_params *params);
+void destroy_tree_params(struct libfsverity_merkle_tree_params *params);
 
 #endif /* PROGRAMS_FSVERITY_H */
-- 
2.29.2

Re: [fsverity-utils PATCH 2/2] programs/fsverity: share code to parse tree parameters

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 832 bytes --]

On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> The "digest", "enable", and "sign" commands all parse the --hash-alg,
> --block-size, and --salt options and initialize a struct
> libfsverity_merkle_tree_params, so share the code that does this.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  programs/cmd_digest.c | 31 ++++---------------------------
>  programs/cmd_enable.c | 30 ++++--------------------------
>  programs/cmd_sign.c   | 31 ++++---------------------------
>  programs/fsverity.c   | 42 ++++++++++++++++++++++++++++++++++++++----
>  programs/fsverity.h   | 19 +++++++++++++++----
>  5 files changed, 65 insertions(+), 88 deletions(-)

Acked-by: Luca Boccassi <luca.boccassi@microsoft.com>

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 5441 bytes --]

On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
> 'struct libfsverity_merkle_tree_params' instead of
> 'struct fsverity_enable_arg'.  This is useful because it allows
> libfsverity users to deal with one common struct.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
>  lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
>  programs/cmd_enable.c | 28 +++++++++++++++------------
>  3 files changed, 97 insertions(+), 12 deletions(-)
>  create mode 100644 lib/enable.c
> 
> diff --git a/include/libfsverity.h b/include/libfsverity.h
> index 8f78a13..a8aecaf 100644
> --- a/include/libfsverity.h
> +++ b/include/libfsverity.h
> @@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
>  			const struct libfsverity_signature_params *sig_params,
>  			uint8_t **sig_ret, size_t *sig_size_ret);
>  
> +/**
> + * libfsverity_enable() - Enable fs-verity on a file
> + * @fd: read-only file descriptor to the file
> + * @params: pointer to the Merkle tree parameters
> + *
> + * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
> + *
> + * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
> + *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
> + *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
> + *	   the possible error codes from FS_IOC_ENABLE_VERITY.
> + */
> +int
> +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
> +
> +/**
> + * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
> + * @fd: read-only file descriptor to the file
> + * @params: pointer to the Merkle tree parameters
> + * @sig: pointer to the file's signature
> + * @sig_size: size of the file's signature in bytes
> + *
> + * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
> + * singature created with libfsverity_sign_digest()) to associate with the file.
> + * This is only needed if the in-kernel signature verification support is being
> + * used; it is not needed if signatures are being verified in userspace.
> + *
> + * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
> + *
> + * Return: See libfsverity_enable().
> + */
> +int
> +libfsverity_enable_with_sig(int fd,
> +			    const struct libfsverity_merkle_tree_params *params,
> +			    const uint8_t *sig, size_t sig_size);
> +

I don't have a strong preference either way, but any specific reason
for a separate function rather than treating sig == NULL and sig_size
== 0 as a signature-less enable? For clients deploying files, it would
appear easier to me to just use empty parameters to choose between
signed/not signed, without having to also change which API to call. But
maybe there's some use case I'm missing where it's better to be
explicit.

>  /**
>   * libfsverity_find_hash_alg_by_name() - Find hash algorithm by name
>   * @name: Pointer to name of hash algorithm
> diff --git a/lib/enable.c b/lib/enable.c
> new file mode 100644
> index 0000000..dd77292
> --- /dev/null
> +++ b/lib/enable.c
> @@ -0,0 +1,45 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Implementation of libfsverity_enable() and libfsverity_enable_with_sig().
> + *
> + * Copyright 2020 Google LLC
> + *
> + * Use of this source code is governed by an MIT-style
> + * license that can be found in the LICENSE file or at
> + * https://opensource.org/licenses/MIT.
> + */
> +
> +#include "lib_private.h"
> +
> +#include <sys/ioctl.h>
> +
> +LIBEXPORT int
> +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params)
> +{
> +	return libfsverity_enable_with_sig(fd, params, NULL, 0);
> +}
> +
> +LIBEXPORT int
> +libfsverity_enable_with_sig(int fd,
> +			    const struct libfsverity_merkle_tree_params *params,
> +			    const uint8_t *sig, size_t sig_size)
> +{
> +	struct fsverity_enable_arg arg = {};
> +
> +	if (!params) {
> +		libfsverity_error_msg("missing required parameters for enable");
> +		return -EINVAL;
> +	}
> +
> +	arg.version = 1;
> +	arg.hash_algorithm = params->hash_algorithm;
> +	arg.block_size = params->block_size;
> +	arg.salt_size = params->salt_size;
> +	arg.salt_ptr = (uintptr_t)params->salt;
> +	arg.sig_size = sig_size;
> +	arg.sig_ptr = (uintptr_t)sig;
> +
> +	if (ioctl(fd, FS_IOC_ENABLE_VERITY, &arg) != 0)
> +		return -errno;
> +	return 0;
> +}

I'm ok with leaving file handling to clients - after all, depending on
infrastructure/bindings/helper libs/whatnot, file handling might vary
wildly.

But could we at least provide a default for block size and hash algo,
if none are specified?

While file handling is a generic concept and expected to be a solved
problem for most programs, figuring out what the default block size
should be or what hash algorithm to use is, are fs-verity specific
concepts that most clients (at least those that I deal with) wouldn't
care about in any way outside of this use, and would want it to "just
work".

Apart from these 2 comments, I ran a quick test of the 2 new series,
and everything works as expected. Thanks!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Eric Biggers]

On Mon, Nov 16, 2020 at 11:52:57AM +0000, Luca Boccassi wrote:
> On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
> > 'struct libfsverity_merkle_tree_params' instead of
> > 'struct fsverity_enable_arg'.  This is useful because it allows
> > libfsverity users to deal with one common struct.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > ---
> >  include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
> >  lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
> >  programs/cmd_enable.c | 28 +++++++++++++++------------
> >  3 files changed, 97 insertions(+), 12 deletions(-)
> >  create mode 100644 lib/enable.c
> > 
> > diff --git a/include/libfsverity.h b/include/libfsverity.h
> > index 8f78a13..a8aecaf 100644
> > --- a/include/libfsverity.h
> > +++ b/include/libfsverity.h
> > @@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
> >  			const struct libfsverity_signature_params *sig_params,
> >  			uint8_t **sig_ret, size_t *sig_size_ret);
> >  
> > +/**
> > + * libfsverity_enable() - Enable fs-verity on a file
> > + * @fd: read-only file descriptor to the file
> > + * @params: pointer to the Merkle tree parameters
> > + *
> > + * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
> > + *
> > + * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
> > + *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
> > + *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
> > + *	   the possible error codes from FS_IOC_ENABLE_VERITY.
> > + */
> > +int
> > +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
> > +
> > +/**
> > + * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
> > + * @fd: read-only file descriptor to the file
> > + * @params: pointer to the Merkle tree parameters
> > + * @sig: pointer to the file's signature
> > + * @sig_size: size of the file's signature in bytes
> > + *
> > + * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
> > + * singature created with libfsverity_sign_digest()) to associate with the file.
> > + * This is only needed if the in-kernel signature verification support is being
> > + * used; it is not needed if signatures are being verified in userspace.
> > + *
> > + * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
> > + *
> > + * Return: See libfsverity_enable().
> > + */
> > +int
> > +libfsverity_enable_with_sig(int fd,
> > +			    const struct libfsverity_merkle_tree_params *params,
> > +			    const uint8_t *sig, size_t sig_size);
> > +
> 
> I don't have a strong preference either way, but any specific reason
> for a separate function rather than treating sig == NULL and sig_size
> == 0 as a signature-less enable? For clients deploying files, it would
> appear easier to me to just use empty parameters to choose between
> signed/not signed, without having to also change which API to call. But
> maybe there's some use case I'm missing where it's better to be
> explicit.

libfsverity_enable_with_sig() works since that; it allows sig == NULL and
sig_size == 0.

The reason I don't want the regular libfsverity_enable() to take the signature
parameters is that I'd like to encourage people to do userspace signature
verification instead.  I want to avoid implying that the in-kernel signature
verification is something that everyone should use.  Same reason I didn't want
'fsverity digest' to output fsverity_formatted_digest by default.

> > +LIBEXPORT int
> > +libfsverity_enable_with_sig(int fd,
> > +			    const struct libfsverity_merkle_tree_params *params,
> > +			    const uint8_t *sig, size_t sig_size)
> > +{
> > +	struct fsverity_enable_arg arg = {};
> > +
> > +	if (!params) {
> > +		libfsverity_error_msg("missing required parameters for enable");
> > +		return -EINVAL;
> > +	}
> > +
> > +	arg.version = 1;
> > +	arg.hash_algorithm = params->hash_algorithm;
> > +	arg.block_size = params->block_size;
> > +	arg.salt_size = params->salt_size;
> > +	arg.salt_ptr = (uintptr_t)params->salt;
> > +	arg.sig_size = sig_size;
> > +	arg.sig_ptr = (uintptr_t)sig;
> > +
> > +	if (ioctl(fd, FS_IOC_ENABLE_VERITY, &arg) != 0)
> > +		return -errno;
> > +	return 0;
> > +}
> 
> I'm ok with leaving file handling to clients - after all, depending on
> infrastructure/bindings/helper libs/whatnot, file handling might vary
> wildly.
> 
> But could we at least provide a default for block size and hash algo,
> if none are specified?
> 
> While file handling is a generic concept and expected to be a solved
> problem for most programs, figuring out what the default block size
> should be or what hash algorithm to use is, are fs-verity specific
> concepts that most clients (at least those that I deal with) wouldn't
> care about in any way outside of this use, and would want it to "just
> work".
> 
> Apart from these 2 comments, I ran a quick test of the 2 new series,
> and everything works as expected. Thanks!

First, providing defaults in libfsverity_enable() doesn't make sense unless the
same defaults are provided in libfsverity_compute_digest() too.

Second, PAGE_SIZE is a bad default block size.  It really should be a fixed
value, like 4096, so that e.g. if you sign files on both x86 and PowerPC, or
sign on x86 and enable on PowerPC, you get the same results.

So at least we shouldn't provide defaults unless it's done right.

I suppose it's probably not too late to change the default for the fsverity
program, though.  I doubt anyone is using it with a non-4K PAGE_SIZE yet.

Would it work for you if both libfsverity_compute_digest() and
libfsverity_enable() defaulted to SHA-256 and 4096?

- Eric

Re: [fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 6675 bytes --]

On Mon, 2020-11-16 at 09:41 -0800, Eric Biggers wrote:
> On Mon, Nov 16, 2020 at 11:52:57AM +0000, Luca Boccassi wrote:
> > On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> > > From: Eric Biggers <ebiggers@google.com>
> > > 
> > > Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
> > > 'struct libfsverity_merkle_tree_params' instead of
> > > 'struct fsverity_enable_arg'.  This is useful because it allows
> > > libfsverity users to deal with one common struct.
> > > 
> > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > ---
> > >  include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
> > >  lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
> > >  programs/cmd_enable.c | 28 +++++++++++++++------------
> > >  3 files changed, 97 insertions(+), 12 deletions(-)
> > >  create mode 100644 lib/enable.c
> > > 
> > > diff --git a/include/libfsverity.h b/include/libfsverity.h
> > > index 8f78a13..a8aecaf 100644
> > > --- a/include/libfsverity.h
> > > +++ b/include/libfsverity.h
> > > @@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
> > >  			const struct libfsverity_signature_params *sig_params,
> > >  			uint8_t **sig_ret, size_t *sig_size_ret);
> > >  
> > > +/**
> > > + * libfsverity_enable() - Enable fs-verity on a file
> > > + * @fd: read-only file descriptor to the file
> > > + * @params: pointer to the Merkle tree parameters
> > > + *
> > > + * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
> > > + *
> > > + * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
> > > + *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
> > > + *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
> > > + *	   the possible error codes from FS_IOC_ENABLE_VERITY.
> > > + */
> > > +int
> > > +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
> > > +
> > > +/**
> > > + * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
> > > + * @fd: read-only file descriptor to the file
> > > + * @params: pointer to the Merkle tree parameters
> > > + * @sig: pointer to the file's signature
> > > + * @sig_size: size of the file's signature in bytes
> > > + *
> > > + * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
> > > + * singature created with libfsverity_sign_digest()) to associate with the file.
> > > + * This is only needed if the in-kernel signature verification support is being
> > > + * used; it is not needed if signatures are being verified in userspace.
> > > + *
> > > + * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
> > > + *
> > > + * Return: See libfsverity_enable().
> > > + */
> > > +int
> > > +libfsverity_enable_with_sig(int fd,
> > > +			    const struct libfsverity_merkle_tree_params *params,
> > > +			    const uint8_t *sig, size_t sig_size);
> > > +
> > 
> > I don't have a strong preference either way, but any specific reason
> > for a separate function rather than treating sig == NULL and sig_size
> > == 0 as a signature-less enable? For clients deploying files, it would
> > appear easier to me to just use empty parameters to choose between
> > signed/not signed, without having to also change which API to call. But
> > maybe there's some use case I'm missing where it's better to be
> > explicit.
> 
> libfsverity_enable_with_sig() works since that; it allows sig == NULL and
> sig_size == 0.
> 
> The reason I don't want the regular libfsverity_enable() to take the signature
> parameters is that I'd like to encourage people to do userspace signature
> verification instead.  I want to avoid implying that the in-kernel signature
> verification is something that everyone should use.  Same reason I didn't want
> 'fsverity digest' to output fsverity_formatted_digest by default.

Ok, I understand - makes sense to me, thanks.

Maybe it's worth documenting in the the header description of the API
that empty/null values are accepted and will result in enabling without
signature check?

> > > +LIBEXPORT int
> > > +libfsverity_enable_with_sig(int fd,
> > > +			    const struct libfsverity_merkle_tree_params *params,
> > > +			    const uint8_t *sig, size_t sig_size)
> > > +{
> > > +	struct fsverity_enable_arg arg = {};
> > > +
> > > +	if (!params) {
> > > +		libfsverity_error_msg("missing required parameters for enable");
> > > +		return -EINVAL;
> > > +	}
> > > +
> > > +	arg.version = 1;
> > > +	arg.hash_algorithm = params->hash_algorithm;
> > > +	arg.block_size = params->block_size;
> > > +	arg.salt_size = params->salt_size;
> > > +	arg.salt_ptr = (uintptr_t)params->salt;
> > > +	arg.sig_size = sig_size;
> > > +	arg.sig_ptr = (uintptr_t)sig;
> > > +
> > > +	if (ioctl(fd, FS_IOC_ENABLE_VERITY, &arg) != 0)
> > > +		return -errno;
> > > +	return 0;
> > > +}
> > 
> > I'm ok with leaving file handling to clients - after all, depending on
> > infrastructure/bindings/helper libs/whatnot, file handling might vary
> > wildly.
> > 
> > But could we at least provide a default for block size and hash algo,
> > if none are specified?
> > 
> > While file handling is a generic concept and expected to be a solved
> > problem for most programs, figuring out what the default block size
> > should be or what hash algorithm to use is, are fs-verity specific
> > concepts that most clients (at least those that I deal with) wouldn't
> > care about in any way outside of this use, and would want it to "just
> > work".
> > 
> > Apart from these 2 comments, I ran a quick test of the 2 new series,
> > and everything works as expected. Thanks!
> 
> First, providing defaults in libfsverity_enable() doesn't make sense unless the
> same defaults are provided in libfsverity_compute_digest() too.
> 
> Second, PAGE_SIZE is a bad default block size.  It really should be a fixed
> value, like 4096, so that e.g. if you sign files on both x86 and PowerPC, or
> sign on x86 and enable on PowerPC, you get the same results.
> 
> So at least we shouldn't provide defaults unless it's done right.
> 
> I suppose it's probably not too late to change the default for the fsverity
> program, though.  I doubt anyone is using it with a non-4K PAGE_SIZE yet.
> 
> Would it work for you if both libfsverity_compute_digest() and
> libfsverity_enable() defaulted to SHA-256 and 4096?
> 
> - Eric

Yes, using those defaults in the functions would work perfectly fine
for me, thank you!

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Eric Biggers]

On Mon, Nov 16, 2020 at 05:50:45PM +0000, Luca Boccassi wrote:
> On Mon, 2020-11-16 at 09:41 -0800, Eric Biggers wrote:
> > On Mon, Nov 16, 2020 at 11:52:57AM +0000, Luca Boccassi wrote:
> > > On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> > > > From: Eric Biggers <ebiggers@google.com>
> > > > 
> > > > Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
> > > > 'struct libfsverity_merkle_tree_params' instead of
> > > > 'struct fsverity_enable_arg'.  This is useful because it allows
> > > > libfsverity users to deal with one common struct.
> > > > 
> > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > ---
> > > >  include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
> > > >  lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > >  programs/cmd_enable.c | 28 +++++++++++++++------------
> > > >  3 files changed, 97 insertions(+), 12 deletions(-)
> > > >  create mode 100644 lib/enable.c
> > > > 
> > > > diff --git a/include/libfsverity.h b/include/libfsverity.h
> > > > index 8f78a13..a8aecaf 100644
> > > > --- a/include/libfsverity.h
> > > > +++ b/include/libfsverity.h
> > > > @@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
> > > >  			const struct libfsverity_signature_params *sig_params,
> > > >  			uint8_t **sig_ret, size_t *sig_size_ret);
> > > >  
> > > > +/**
> > > > + * libfsverity_enable() - Enable fs-verity on a file
> > > > + * @fd: read-only file descriptor to the file
> > > > + * @params: pointer to the Merkle tree parameters
> > > > + *
> > > > + * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
> > > > + *
> > > > + * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
> > > > + *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
> > > > + *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
> > > > + *	   the possible error codes from FS_IOC_ENABLE_VERITY.
> > > > + */
> > > > +int
> > > > +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
> > > > +
> > > > +/**
> > > > + * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
> > > > + * @fd: read-only file descriptor to the file
> > > > + * @params: pointer to the Merkle tree parameters
> > > > + * @sig: pointer to the file's signature
> > > > + * @sig_size: size of the file's signature in bytes
> > > > + *
> > > > + * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
> > > > + * singature created with libfsverity_sign_digest()) to associate with the file.
> > > > + * This is only needed if the in-kernel signature verification support is being
> > > > + * used; it is not needed if signatures are being verified in userspace.
> > > > + *
> > > > + * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
> > > > + *
> > > > + * Return: See libfsverity_enable().
> > > > + */
> > > > +int
> > > > +libfsverity_enable_with_sig(int fd,
> > > > +			    const struct libfsverity_merkle_tree_params *params,
> > > > +			    const uint8_t *sig, size_t sig_size);
> > > > +
> > > 
> > > I don't have a strong preference either way, but any specific reason
> > > for a separate function rather than treating sig == NULL and sig_size
> > > == 0 as a signature-less enable? For clients deploying files, it would
> > > appear easier to me to just use empty parameters to choose between
> > > signed/not signed, without having to also change which API to call. But
> > > maybe there's some use case I'm missing where it's better to be
> > > explicit.
> > 
> > libfsverity_enable_with_sig() works since that; it allows sig == NULL and
> > sig_size == 0.
> > 
> > The reason I don't want the regular libfsverity_enable() to take the signature
> > parameters is that I'd like to encourage people to do userspace signature
> > verification instead.  I want to avoid implying that the in-kernel signature
> > verification is something that everyone should use.  Same reason I didn't want
> > 'fsverity digest' to output fsverity_formatted_digest by default.
> 
> Ok, I understand - makes sense to me, thanks.
> 
> Maybe it's worth documenting in the the header description of the API
> that empty/null values are accepted and will result in enabling without
> signature check?
> 

It's already there:

 * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().

Re: [fsverity-utils PATCH 1/2] lib: add libfsverity_enable() and libfsverity_enable_with_sig()

[Luca Boccassi]

[-- Attachment #1: Type: text/plain, Size: 4878 bytes --]

On Mon, 2020-11-16 at 10:42 -0800, Eric Biggers wrote:
> On Mon, Nov 16, 2020 at 05:50:45PM +0000, Luca Boccassi wrote:
> > On Mon, 2020-11-16 at 09:41 -0800, Eric Biggers wrote:
> > > On Mon, Nov 16, 2020 at 11:52:57AM +0000, Luca Boccassi wrote:
> > > > On Fri, 2020-11-13 at 16:15 -0800, Eric Biggers wrote:
> > > > > From: Eric Biggers <ebiggers@google.com>
> > > > > 
> > > > > Add convenience functions that wrap FS_IOC_ENABLE_VERITY but take a
> > > > > 'struct libfsverity_merkle_tree_params' instead of
> > > > > 'struct fsverity_enable_arg'.  This is useful because it allows
> > > > > libfsverity users to deal with one common struct.
> > > > > 
> > > > > Signed-off-by: Eric Biggers <ebiggers@google.com>
> > > > > ---
> > > > >  include/libfsverity.h | 36 ++++++++++++++++++++++++++++++++++
> > > > >  lib/enable.c          | 45 +++++++++++++++++++++++++++++++++++++++++++
> > > > >  programs/cmd_enable.c | 28 +++++++++++++++------------
> > > > >  3 files changed, 97 insertions(+), 12 deletions(-)
> > > > >  create mode 100644 lib/enable.c
> > > > > 
> > > > > diff --git a/include/libfsverity.h b/include/libfsverity.h
> > > > > index 8f78a13..a8aecaf 100644
> > > > > --- a/include/libfsverity.h
> > > > > +++ b/include/libfsverity.h
> > > > > @@ -112,6 +112,42 @@ libfsverity_sign_digest(const struct libfsverity_digest *digest,
> > > > >  			const struct libfsverity_signature_params *sig_params,
> > > > >  			uint8_t **sig_ret, size_t *sig_size_ret);
> > > > >  
> > > > > +/**
> > > > > + * libfsverity_enable() - Enable fs-verity on a file
> > > > > + * @fd: read-only file descriptor to the file
> > > > > + * @params: pointer to the Merkle tree parameters
> > > > > + *
> > > > > + * This is a simple wrapper around the FS_IOC_ENABLE_VERITY ioctl.
> > > > > + *
> > > > > + * Return: 0 on success, -EINVAL for invalid arguments, or a negative errno
> > > > > + *	   value from the FS_IOC_ENABLE_VERITY ioctl.  See
> > > > > + *	   Documentation/filesystems/fsverity.rst in the kernel source tree for
> > > > > + *	   the possible error codes from FS_IOC_ENABLE_VERITY.
> > > > > + */
> > > > > +int
> > > > > +libfsverity_enable(int fd, const struct libfsverity_merkle_tree_params *params);
> > > > > +
> > > > > +/**
> > > > > + * libfsverity_enable_with_sig() - Enable fs-verity on a file, with a signature
> > > > > + * @fd: read-only file descriptor to the file
> > > > > + * @params: pointer to the Merkle tree parameters
> > > > > + * @sig: pointer to the file's signature
> > > > > + * @sig_size: size of the file's signature in bytes
> > > > > + *
> > > > > + * Like libfsverity_enable(), but allows specifying a built-in signature (i.e. a
> > > > > + * singature created with libfsverity_sign_digest()) to associate with the file.
> > > > > + * This is only needed if the in-kernel signature verification support is being
> > > > > + * used; it is not needed if signatures are being verified in userspace.
> > > > > + *
> > > > > + * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().
> > > > > + *
> > > > > + * Return: See libfsverity_enable().
> > > > > + */
> > > > > +int
> > > > > +libfsverity_enable_with_sig(int fd,
> > > > > +			    const struct libfsverity_merkle_tree_params *params,
> > > > > +			    const uint8_t *sig, size_t sig_size);
> > > > > +
> > > > 
> > > > I don't have a strong preference either way, but any specific reason
> > > > for a separate function rather than treating sig == NULL and sig_size
> > > > == 0 as a signature-less enable? For clients deploying files, it would
> > > > appear easier to me to just use empty parameters to choose between
> > > > signed/not signed, without having to also change which API to call. But
> > > > maybe there's some use case I'm missing where it's better to be
> > > > explicit.
> > > 
> > > libfsverity_enable_with_sig() works since that; it allows sig == NULL and
> > > sig_size == 0.
> > > 
> > > The reason I don't want the regular libfsverity_enable() to take the signature
> > > parameters is that I'd like to encourage people to do userspace signature
> > > verification instead.  I want to avoid implying that the in-kernel signature
> > > verification is something that everyone should use.  Same reason I didn't want
> > > 'fsverity digest' to output fsverity_formatted_digest by default.
> > 
> > Ok, I understand - makes sense to me, thanks.
> > 
> > Maybe it's worth documenting in the the header description of the API
> > that empty/null values are accepted and will result in enabling without
> > signature check?
> > 
> 
> It's already there:
> 
>  * If @sig is NULL and @sig_size is 0, this is the same as libfsverity_enable().

Ah of course, sorry, right under my nose and still missed it :-)

-- 
Kind regards,
Luca Boccassi

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]