Re: [PATCH 4/9] Move hash algorithm code to shared library

[Eric Biggers <ebiggers@kernel.org>]

On Thu, Mar 12, 2020 at 05:47:53PM -0400, Jes Sorensen wrote:
> diff --git a/libfsverity.h b/libfsverity.h
> index 396a6ee..318dcd7 100644
> --- a/libfsverity.h
> +++ b/libfsverity.h
> @@ -18,6 +18,9 @@
>  #define FS_VERITY_HASH_ALG_SHA256       1
>  #define FS_VERITY_HASH_ALG_SHA512       2
>  
> +/* The hash algorithm that fsverity-utils assumes when none is specified */
> +#define FS_VERITY_HASH_ALG_DEFAULT	FS_VERITY_HASH_ALG_SHA256
> +
>  struct libfsverity_merkle_tree_params {
>  	uint16_t version;
>  	uint16_t hash_algorithm;	/* Matches the digest_algorithm type */
> @@ -27,6 +30,12 @@ struct libfsverity_merkle_tree_params {
>  	uint64_t reserved[11];
>  };
>  
> +/*
> + * Largest digest size among all hash algorithms supported by fs-verity.
> + * This can be increased if needed.
> + */
> +#define FS_VERITY_MAX_DIGEST_SIZE	64
> +
>  struct libfsverity_digest {
>  	char magic[8];			/* must be "FSVerity" */
>  	uint16_t digest_algorithm;
> @@ -57,9 +66,22 @@ struct fsverity_descriptor {
>  	uint8_t signature[];	/* optional PKCS#7 signature */
>  };
>  
> +struct fsverity_hash_alg {
> +	const char *name;
> +	unsigned int digest_size;
> +	unsigned int block_size;
> +	uint16_t hash_num;
> +	struct hash_ctx *(*create_ctx)(const struct fsverity_hash_alg *alg);
> +};
> +

It's still a bit weird to have struct fsverity_hash_alg as part of the library
API, since the .create_ctx() member is for internal library use only.  We at
least need to clearly comment this:

	struct fsverity_hash_alg {
		const char *name;
		unsigned int digest_size;
		unsigned int block_size;
		uint16_t hash_num;

		/* for library-internal use only */
		struct hash_ctx *(*create_ctx)(const struct fsverity_hash_alg *alg);
	};

But ideally there would be nothing library-internal in the API at all.

- Eric