Re: [fsverity-utils RFC PATCH] Add libfsverity_enable() API

[Luca Boccassi <Luca.Boccassi@microsoft.com>]

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

On Fri, 2020-11-13 at 16:21 -0800, Eric Biggers wrote:
> On Fri, Nov 13, 2020 at 02:35:27PM +0000, luca.boccassi@gmail.com wrote:
> > From: Luca Boccassi <luca.boccassi@microsoft.com>
> > 
> > Factor out the 'fsverity enable' implementation in the library, to
> > give users a shortcut for reading signatures and enabling a file
> > with the default parameters.
> > 
> > Signed-off-by: Luca Boccassi <luca.boccassi@microsoft.com>
> > ---
> > Marked as RFC to get guidance on how to deal with helper functions
> > duplication, that right now are part of the "programs" utility objects
> > and not usable from the "library" objects.
> > There's dozens of different ways to handle this, all equally valid, so
> > it's down to the preference of the maintainer (eg: new common helpers,
> > include helpers at build time, further splits of sources, etc).
> > Please provide a preference and I'll follow up.
> > 
> >  common/common_defs.h  |   9 ++
> >  include/libfsverity.h |  21 +++++
> >  lib/enable.c          | 191 ++++++++++++++++++++++++++++++++++++++++++
> >  programs/cmd_enable.c |  66 ++-------------
> >  programs/fsverity.h   |   9 --
> >  5 files changed, 227 insertions(+), 69 deletions(-)
> >  create mode 100644 lib/enable.c
> > 
> > diff --git a/common/common_defs.h b/common/common_defs.h
> > index 279385a..871db2c 100644
> > --- a/common/common_defs.h
> > +++ b/common/common_defs.h
> > @@ -90,4 +90,13 @@ static inline int ilog2(unsigned long n)
> >  #  define le64_to_cpu(v)	(__builtin_bswap64((__force u64)(v)))
> >  #endif
> >  
> > +/* The hash algorithm that 'fsverity' assumes when none is specified */
> > +#define FS_VERITY_HASH_ALG_DEFAULT	FS_VERITY_HASH_ALG_SHA256
> > +
> > +/*
> > + * Largest digest size among all hash algorithms supported by fs-verity.
> > + * This can be increased if needed.
> > + */
> > +#define FS_VERITY_MAX_DIGEST_SIZE	64
> > +
> >  #endif /* COMMON_COMMON_DEFS_H */
> > diff --git a/include/libfsverity.h b/include/libfsverity.h
> > index 8f78a13..8d1f93b 100644
> > --- a/include/libfsverity.h
> > +++ b/include/libfsverity.h
> > @@ -112,6 +112,27 @@ 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
> > + *          An fsverity_digest (also called a "file measurement") is the root of
> > + *          a file's Merkle tree.  Not to be confused with a traditional file
> > + *          digest computed over the entire file.
> > + * @file: path to the file to enable
> > + * @signature: (optional) path to signature for @file
> > + * @params: struct libfsverity_merkle_tree_params specifying the fs-verity
> > + *	    version, the hash algorithm, the block size, and
> > + *	    optionally a salt.  Reserved fields must be zero.
> > + *      All fields bar the version are optional, and defaults will be used
> > + *      if set to zero.
> > + *
> > + * Returns:
> > + * * 0 for success, -EINVAL for invalid input arguments, or a generic error
> > + *   if the FS_IOC_ENABLE_VERITY ioctl fails.
> > + */
> > +int
> > +libfsverity_enable(const char *file, const char *signature,
> > +			struct libfsverity_merkle_tree_params *params);
> > +
> >  /**
> >   * libfsverity_find_hash_alg_by_name() - Find hash algorithm by name
> >   * @name: Pointer to name of hash algorithm
> 
> Hi Luca, can you consider
> https://lkml.kernel.org/linux-fscrypt/20201114001529.185751-1-ebiggers@kernel.org/T/#u
> instead?
> 
> It's somewhat useful to have a wrapper around FS_IOC_ENABLE_VERITY that takes
> 'struct libfsverity_merkle_tree_params', so that library users can deal with one
> common struct.  (And I took advantage of that to simplify the code that parses
> the parameters.)
> 
> But I think we should keep it as a thin wrapper, and not have file path
> parameters or set defaults in the libfsverity_merkle_tree_params.  The library
> user is better suited to deal with those, like they already do for
> libfsverity_compute_digest().
> 
> - Eric

Hi,

Sure, no problem, we can drop this, thanks for sending those series.
Left a comment in the other thread.

-- 
Kind regards,
Luca Boccassi

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