Linux-FSCrypt Archive on lore.kernel.org
 help / color / Atom feed
From: Eric Biggers <ebiggers@kernel.org>
To: Jes Sorensen <jes.sorensen@gmail.com>
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: Fri, 14 Feb 2020 12:35:11 -0800
Message-ID: <20200214203510.GA1985@gmail.com> (raw)
In-Reply-To: <c39f57d5-c9a4-5fbb-3ce3-cd21e90ef921@gmail.com>

Hi Jes,

On Tue, Feb 11, 2020 at 06:35:45PM -0500, Jes Sorensen wrote:
> On 2/11/20 6:14 PM, Eric Biggers wrote:
> > On Tue, Feb 11, 2020 at 05:09:22PM -0500, Jes Sorensen wrote:
> >> On 2/11/20 2:22 PM, Eric Biggers wrote:
> >>> Hi Jes,
> >> So I basically want to be able to carry verity signatures in RPM as RPM
> >> internal data, similar to how it supports IMA signatures. I want to be
> >> able to install those without relying on post-install scripts and
> >> signature files being distributed as actual files that gets installed,
> >> just to have to remove them. This is how IMA support is integrated into
> >> RPM as well.
> >>
> >> Right now the RPM approach for signatures involves two steps, a build
> >> digest phase, and a sign the digest phase.
> >>
> >> The reason I included enable and measure was for completeness. I don't
> >> care wildly about those.
> > 
> > So the signing happens when the RPM is built, not when it's installed?  Are you
> > sure you actually need a library and not just 'fsverity sign' called from a
> > build script?
> 
> So the way RPM is handling these is to calculate the digest in one
> place, and sign it in another. Basically the signing is a second step,
> post build, using the rpmsign command. Shelling out is not a good fit
> for this model.
> 
> >>> Separately, before you start building something around fs-verity's builtin
> >>> signature verification support, have you also considered adding support for
> >>> fs-verity to IMA?  I.e., using the fs-verity hashing mechanism with the IMA
> >>> signature mechanism.  The IMA maintainer has been expressed interested in that.
> >>> If rpm already supports IMA signatures, maybe that way would be a better fit?
> >>
> >> I looked at IMA and it is overly complex. It is not obvious to me how
> >> you would get around that without the full complexity of IMA? The beauty
> >> of fsverity's approach is the simplicity of relying on just the kernel
> >> keyring for validation of the signature. If you have explicit
> >> suggestions, I am certainly happy to look at it.
> > 
> > fs-verity's builtin signature verification feature is simple, but does it
> > actually do what you need?  Note that unlike IMA, it doesn't provide an
> > in-kernel policy about which files have to have signatures and which don't.
> > I.e., to get any authenticity guarantee, before using any files that are
> > supposed to be protected by fs-verity, userspace has to manually check whether
> > the fs-verity bit is actually set.  Is that part of your design?
> 
> Totally aware of this, and it fits the model I am looking at.
> 

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];
};

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 */



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.

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

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?

- Eric

  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 [this message]
2020-02-19 23:49           ` Jes Sorensen
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=20200214203510.GA1985@gmail.com \
    --to=ebiggers@kernel.org \
    --cc=jes.sorensen@gmail.com \
    --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