Linux-FSCrypt Archive on lore.kernel.org
 help / color / Atom feed
From: Jes Sorensen <jes.sorensen@gmail.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: linux-fscrypt@vger.kernel.org, kernel-team@fb.com,
	Jes Sorensen <jsorensen@fb.com>
Subject: Re: [PATCH 0/7] Split fsverity-utils into a shared library
Date: Wed, 19 Feb 2020 18:49:07 -0500
Message-ID: <479b0fff-6af2-32e6-a645-03fcfc65ad59@gmail.com> (raw)
In-Reply-To: <20200214203510.GA1985@gmail.com>

Hi Eric,

On 2/14/20 3:35 PM, Eric Biggers wrote:
> Well, this might be a legitimate use case then.  We need to define the library
> interface as simply as possible, though, so that we can maintain this code in
> the future without breaking users.  I suggest starting with something along the
> lines of:
> 
> #ifndef _LIBFSVERITY_H
> #define _LIBFSVERITY_H
> 
> #include <stddef.h>
> #include <stdint.h>
> 
> #define FS_VERITY_HASH_ALG_SHA256       1
> #define FS_VERITY_HASH_ALG_SHA512       2
> 
> struct libfsverity_merkle_tree_params {
> 	uint32_t version;
> 	uint32_t hash_algorithm;
> 	uint32_t block_size;
> 	uint32_t salt_size;
> 	const uint8_t *salt;
> 	size_t reserved[11];
> };
> 
> struct libfsverity_digest {
> 	uint16_t digest_algorithm;
> 	uint16_t digest_size;
> 	uint8_t digest[];
> };
> 
> struct libfsverity_signature_params {
> 	const char *keyfile;
> 	const char *certfile;
> 	size_t reserved[11];
> };

This looks reasonable to me - I would do the reserved fields as void *
or uint32_t, but that is a detail.

> int libfsverity_compute_digest(int fd,
> 			       const struct libfsverity_merkle_tree_params *params,
> 			       struct libfsverity_digest **digest_ret);
> 
> int libfsverity_sign_digest(const struct libfsverity_digest *digest,
> 			    const struct libfsverity_signature_params *sig_params,
> 			    void **sig_ret, size_t *sig_size_ret);
> 
> #endif /* _LIBFSVERITY_H */

Looks good too, I deliberately named the functions as fsverity, but
happy to prepend them with 'lib'. Didn't want to have a clash with
'sign_hash' as a function is actually named in a related library.

> I.e.:
> 
> - The stuff in util.h and hash_algs.h isn't exposed to library users.
> - Then names of all library functions and structs are appropriately prefixed
>   and avoid collisions with the kernel header.
> - Only signing functionality is included.
> - There are reserved fields, so we can add more parameters later.

I was debating whether to expect the library to do the open or have the
caller be responsible for that. Given we have to play the song and dance
with the signing key and certificate filenames, it's a little quirky,
but we're passing those to libopenssl so no way to really get around it.

> Before committing to any stable API, it would also be helpful to see the RPM
> patches to see what it actually does.

Absolutely, I wanted to have us agree on the strategy first before
taking it to the next step.

I'll take a stab at this.

> We'd also need to follow shared library best practices like compiling with
> -fvisibility=hidden and marking the API functions explicitly with
> __attribute__((visibility("default"))), and setting the 'soname' like
> -Wl,-soname=libfsverity.so.0.
> 
> Also, is the GPLv2+ license okay for the use case?

Personally I only care about linking it into rpm, which is GPL v2, so
from my perspective, that is sufficient. I am also fine making it LGPL,
but given it's your code I am stealing, I cannot make that call.

Cheers,
Jes



  reply index

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-11  0:00 Jes Sorensen
2020-02-11  0:00 ` [PATCH 1/7] Build basic " Jes Sorensen
2020-02-11  0:00 ` [PATCH 2/7] Restructure fsverity_cmd_sign for shared libraries Jes Sorensen
2020-02-11  0:00 ` [PATCH 3/7] Make fsverity_cmd_measure() a library function Jes Sorensen
2020-02-11  0:00 ` [PATCH 4/7] Make fsverity_cmd_enable a library call() Jes Sorensen
2020-02-11  0:00 ` [PATCH 5/7] Rename commands.h to fsverity.h Jes Sorensen
2020-02-11  0:00 ` [PATCH 6/7] Move cmdline helper functions to fsverity.c Jes Sorensen
2020-02-11  0:00 ` [PATCH 7/7] cmd_sign: fsverity_cmd_sign() into two functions Jes Sorensen
2020-02-11 19:22 ` [PATCH 0/7] Split fsverity-utils into a shared library Eric Biggers
2020-02-11 22:09   ` Jes Sorensen
2020-02-11 23:14     ` Eric Biggers
2020-02-11 23:35       ` Jes Sorensen
2020-02-14 20:35         ` Eric Biggers
2020-02-19 23:49           ` Jes Sorensen [this message]
2020-07-30 17:52             ` Eric Biggers
2020-07-31 17:40               ` Jes Sorensen
2020-07-31 17:47                 ` Chris Mason
2020-07-31 19:14                   ` Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=479b0fff-6af2-32e6-a645-03fcfc65ad59@gmail.com \
    --to=jes.sorensen@gmail.com \
    --cc=ebiggers@kernel.org \
    --cc=jsorensen@fb.com \
    --cc=kernel-team@fb.com \
    --cc=linux-fscrypt@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Linux-FSCrypt Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-fscrypt/0 linux-fscrypt/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-fscrypt linux-fscrypt/ https://lore.kernel.org/linux-fscrypt \
		linux-fscrypt@vger.kernel.org
	public-inbox-index linux-fscrypt

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-fscrypt


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git