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

[Eric Biggers <ebiggers@kernel.org>]

On Mon, Oct 26, 2020 at 06:17:29PM +0000, 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[])

Since it can be more than one file now:

/* Compute the fs-verity measurement of the given file(s), for offline signing */

> +	for (int i = 0; i < argc; i++) {
> +		struct filedes file = { .fd = -1 };
> +		struct fsverity_signed_digest *d = NULL;
> +		struct libfsverity_digest *digest = NULL;
> +		char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + sizeof(struct fsverity_signed_digest) * 2 + 1];
> +
> +		if (!open_file(&file, argv[i], O_RDONLY, 0))
> +			goto out_err;
> +
> +		if (!get_file_size(&file, &tree_params.file_size))
> +			goto out_err;

'file' doesn't get closed on error.  Making it back to the outer scope would fix
that.

> +		if (compact)
> +			printf("%s\n", digest_hex);
> +		else
> +			printf("%s:%s %s\n",
> +				libfsverity_get_hash_name(tree_params.hash_algorithm),
> +				digest_hex, argv[i]);

I don't think the hash algorithm should be printed in the
'!compact && for_builtin_sig' case, since it's already included in the struct
that gets hex-encoded.  I.e.

		else if (for_builtin_sig)
			printf("%s %s\n", digest_hex, argv[i]);

> diff --git a/programs/fsverity.c b/programs/fsverity.c
> index 95f6964..c7c4f75 100644
> --- a/programs/fsverity.c
> +++ b/programs/fsverity.c
> @@ -21,6 +21,14 @@ static const struct fsverity_command {
>  	const char *usage_str;
>  } fsverity_commands[] = {
>  	{
> +		.name = "digest",
> +		.func = fsverity_cmd_digest,
> +		.short_desc = "Compute and print hex-encoded fs-verity digest of a file, for offline signing",

Likewise, since this can now accept multiple files:

"Compute the fs-verity measurement of the given file(s), for offline signing"

(I don't think that "printed as hex" needs to be explicitly mentioned here.)

- Eric