Re: [fsverity-utils PATCH] Add digest sub command

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Oct 22, 2020 at 06:21:55PM +0100, luca.boccassi@gmail.com wrote:
> +/* Compute a file's fs-verity measurement, then print it in hex format. */
> +int fsverity_cmd_digest(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_digest *digest = NULL;
> +	struct fsverity_signed_digest *d = NULL;
> +	char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + sizeof(struct fsverity_signed_digest) * 2 + 1];
> +    bool compact = false;
> +	int status;
> +	int c;
> +
> +	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))
> +				goto out_usage;
> +			tree_params.salt = salt;
> +			break;
> +		case OPT_COMPACT:
> +			compact = true;
> +			break;
> +		default:
> +			goto out_usage;
> +		}
> +	}
> +
> +	argv += optind;
> +	argc -= optind;
> +
> +	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();
> +
> +	if (!open_file(&file, argv[0], O_RDONLY, 0))
> +		goto out_err;
> +
> +	if (!get_file_size(&file, &tree_params.file_size))
> +		goto out_err;
> +
> +	if (libfsverity_compute_digest(&file, read_callback,
> +				       &tree_params, &digest) != 0) {
> +		error_msg("failed to compute digest");
> +		goto out_err;
> +	}
> +
> +	ASSERT(digest->digest_size <= FS_VERITY_MAX_DIGEST_SIZE);
> +
> +	d = xzalloc(sizeof(*d) + digest->digest_size);
> +	if (!d)
> +		goto out_err;
> +	memcpy(d->magic, "FSVerity", 8);
> +	d->digest_algorithm = cpu_to_le16(digest->digest_algorithm);
> +	d->digest_size = cpu_to_le16(digest->digest_size);
> +	memcpy(d->digest, digest->digest, digest->digest_size);
> +
> +	bin2hex((const u8 *)d, sizeof(*d) + digest->digest_size, digest_hex);
> +
> +	if (compact)
> +		printf("%s", digest_hex);
> +	else
> +		printf("File '%s' (%s:%s)\n", argv[0],
> +			   libfsverity_get_hash_name(tree_params.hash_algorithm),
> +			   digest_hex);

Can you make this command format its output in the same way as
'fsverity measure' by default, and put the 'struct fsverity_signed_digest'
formatted output behind an option, like --for-builtin-sig?

The 'struct fsverity_signed_digest' is specific to the builtin (in-kernel)
signature support, which isn't the only way to use fs-verity.  The signature
verification can also be done in userspace, which is more flexible.  (And you
should consider doing it that way, if you haven't already.  I'm not sure exactly
what your use case is.)

So when possible, I'd like to have the default be the basic fs-verity feature.
If someone then specifically wants to use the builtin signature support on top
of that, as opposed to using fs-verity in another way, then they can provide the
option they need to do that.

Separately, it would also be nice to share more code with cmd_sign.c, as they
both have to parse a lot of the same options.  Maybe it doesn't work out,
though.

- Eric