linux-fscrypt.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] fsverity-utils: introduce libfsverity
@ 2020-05-15  4:10 Eric Biggers
  2020-05-15  4:10 ` [PATCH 1/3] Split up cmd_sign.c Eric Biggers
                   ` (3 more replies)
  0 siblings, 4 replies; 14+ messages in thread
From: Eric Biggers @ 2020-05-15  4:10 UTC (permalink / raw)
  To: linux-fscrypt, Jes Sorensen; +Cc: jsorensen, kernel-team

From the 'fsverity' program, split out a library 'libfsverity'.
Currently it supports computing file measurements ("digests"), and
signing those file measurements for use with the fs-verity builtin
signature verification feature.

Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
I made a lot of improvements; see patch 2 for details.

Jes, can you let me know whether this works for you?  Especially take a
close look at the API in libfsverity.h.

This patchset can also be found at branch "libfsverity" of
https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/fsverity-utils.git/

Eric Biggers (3):
  Split up cmd_sign.c
  Introduce libfsverity
  Add some basic test programs for libfsverity

 .gitignore                                |   9 +-
 Makefile                                  | 198 ++++++-
 cmd_sign.c                                | 635 ----------------------
 commands.h                                |  24 -
 util.h => common/common_defs.h            |  47 +-
 fsverity_uapi.h => common/fsverity_uapi.h |   0
 common/libfsverity.h                      | 132 +++++
 hash_algs.h                               |  68 ---
 lib/compute_digest.c                      | 243 +++++++++
 hash_algs.c => lib/hash_algs.c            | 126 +++--
 lib/lib_private.h                         |  83 +++
 lib/sign_digest.c                         | 395 ++++++++++++++
 lib/utils.c                               | 107 ++++
 cmd_enable.c => programs/cmd_enable.c     |  32 +-
 cmd_measure.c => programs/cmd_measure.c   |  12 +-
 programs/cmd_sign.c                       | 163 ++++++
 fsverity.c => programs/fsverity.c         |  52 +-
 programs/fsverity.h                       |  41 ++
 programs/test_compute_digest.c            |  54 ++
 programs/test_hash_algs.c                 |  27 +
 programs/test_sign_digest.c               |  44 ++
 util.c => programs/utils.c                |   7 +-
 programs/utils.h                          |  42 ++
 testdata/cert.pem                         |  31 ++
 testdata/file.sig                         | Bin 0 -> 708 bytes
 testdata/key.pem                          |  52 ++
 26 files changed, 1742 insertions(+), 882 deletions(-)
 delete mode 100644 cmd_sign.c
 delete mode 100644 commands.h
 rename util.h => common/common_defs.h (58%)
 rename fsverity_uapi.h => common/fsverity_uapi.h (100%)
 create mode 100644 common/libfsverity.h
 delete mode 100644 hash_algs.h
 create mode 100644 lib/compute_digest.c
 rename hash_algs.c => lib/hash_algs.c (54%)
 create mode 100644 lib/lib_private.h
 create mode 100644 lib/sign_digest.c
 create mode 100644 lib/utils.c
 rename cmd_enable.c => programs/cmd_enable.c (82%)
 rename cmd_measure.c => programs/cmd_measure.c (84%)
 create mode 100644 programs/cmd_sign.c
 rename fsverity.c => programs/fsverity.c (82%)
 create mode 100644 programs/fsverity.h
 create mode 100644 programs/test_compute_digest.c
 create mode 100644 programs/test_hash_algs.c
 create mode 100644 programs/test_sign_digest.c
 rename util.c => programs/utils.c (96%)
 create mode 100644 programs/utils.h
 create mode 100644 testdata/cert.pem
 create mode 100644 testdata/file.sig
 create mode 100644 testdata/key.pem

-- 
2.26.2


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/3] Split up cmd_sign.c
  2020-05-15  4:10 [PATCH 0/3] fsverity-utils: introduce libfsverity Eric Biggers
@ 2020-05-15  4:10 ` Eric Biggers
  2020-05-21 15:26   ` Jes Sorensen
  2020-05-15  4:10 ` [PATCH 2/3] Introduce libfsverity Eric Biggers
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-15  4:10 UTC (permalink / raw)
  To: linux-fscrypt, Jes Sorensen; +Cc: jsorensen, kernel-team

From: Eric Biggers <ebiggers@google.com>

In preparation for moving most of the functionality of 'fsverity sign'
into a shared library, split up cmd_sign.c into three files:

- cmd_sign.c: the actual command
- compute_digest.c: computing the file measurement
- sign_digest.c: sign the file measurement

No "real" changes; this is just moving code around.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 Makefile             |   4 +-
 cmd_sign.c           | 481 +------------------------------------------
 lib/compute_digest.c | 184 +++++++++++++++++
 lib/sign_digest.c    | 304 +++++++++++++++++++++++++++
 sign.h               |  32 +++
 5 files changed, 523 insertions(+), 482 deletions(-)
 create mode 100644 lib/compute_digest.c
 create mode 100644 lib/sign_digest.c
 create mode 100644 sign.h

diff --git a/Makefile b/Makefile
index b9c09b9..c3b14a3 100644
--- a/Makefile
+++ b/Makefile
@@ -1,9 +1,9 @@
 EXE := fsverity
 CFLAGS := -O2 -Wall
-CPPFLAGS := -D_FILE_OFFSET_BITS=64
+CPPFLAGS := -D_FILE_OFFSET_BITS=64 -I.
 LDLIBS := -lcrypto
 DESTDIR := /usr/local
-SRC := $(wildcard *.c)
+SRC := $(wildcard *.c) $(wildcard lib/*.c)
 OBJ := $(SRC:.c=.o)
 HDRS := $(wildcard *.h)
 
diff --git a/cmd_sign.c b/cmd_sign.c
index 5a6deb7..5e3270d 100644
--- a/cmd_sign.c
+++ b/cmd_sign.c
@@ -9,337 +9,13 @@
 
 #include "commands.h"
 #include "fsverity_uapi.h"
-#include "hash_algs.h"
+#include "sign.h"
 
 #include <fcntl.h>
 #include <getopt.h>
-#include <limits.h>
-#include <openssl/bio.h>
-#include <openssl/err.h>
-#include <openssl/pem.h>
-#include <openssl/pkcs7.h>
 #include <stdlib.h>
 #include <string.h>
 
-/*
- * Merkle tree properties.  The file measurement is the hash of this structure
- * excluding the signature and with the sig_size field set to 0.
- */
-struct fsverity_descriptor {
-	__u8 version;		/* must be 1 */
-	__u8 hash_algorithm;	/* Merkle tree hash algorithm */
-	__u8 log_blocksize;	/* log2 of size of data and tree blocks */
-	__u8 salt_size;		/* size of salt in bytes; 0 if none */
-	__le32 sig_size;	/* size of signature in bytes; 0 if none */
-	__le64 data_size;	/* size of file the Merkle tree is built over */
-	__u8 root_hash[64];	/* Merkle tree root hash */
-	__u8 salt[32];		/* salt prepended to each hashed block */
-	__u8 __reserved[144];	/* must be 0's */
-	__u8 signature[];	/* optional PKCS#7 signature */
-};
-
-/*
- * Format in which verity file measurements are signed.  This is the same as
- * 'struct fsverity_digest', except here some magic bytes are prepended to
- * provide some context about what is being signed in case the same key is used
- * for non-fsverity purposes, and here the fields have fixed endianness.
- */
-struct fsverity_signed_digest {
-	char magic[8];			/* must be "FSVerity" */
-	__le16 digest_algorithm;
-	__le16 digest_size;
-	__u8 digest[];
-};
-
-static void __printf(1, 2) __cold
-error_msg_openssl(const char *format, ...)
-{
-	va_list va;
-
-	va_start(va, format);
-	do_error_msg(format, va, 0);
-	va_end(va);
-
-	if (ERR_peek_error() == 0)
-		return;
-
-	fprintf(stderr, "OpenSSL library errors:\n");
-	ERR_print_errors_fp(stderr);
-}
-
-/* Read a PEM PKCS#8 formatted private key */
-static EVP_PKEY *read_private_key(const char *keyfile)
-{
-	BIO *bio;
-	EVP_PKEY *pkey;
-
-	bio = BIO_new_file(keyfile, "r");
-	if (!bio) {
-		error_msg_openssl("can't open '%s' for reading", keyfile);
-		return NULL;
-	}
-
-	pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
-	if (!pkey) {
-		error_msg_openssl("Failed to parse private key file '%s'.\n"
-				  "       Note: it must be in PEM PKCS#8 format.",
-				  keyfile);
-	}
-	BIO_free(bio);
-	return pkey;
-}
-
-/* Read a PEM X.509 formatted certificate */
-static X509 *read_certificate(const char *certfile)
-{
-	BIO *bio;
-	X509 *cert;
-
-	bio = BIO_new_file(certfile, "r");
-	if (!bio) {
-		error_msg_openssl("can't open '%s' for reading", certfile);
-		return NULL;
-	}
-	cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
-	if (!cert) {
-		error_msg_openssl("Failed to parse X.509 certificate file '%s'.\n"
-				  "       Note: it must be in PEM format.",
-				  certfile);
-	}
-	BIO_free(bio);
-	return cert;
-}
-
-#ifdef OPENSSL_IS_BORINGSSL
-
-static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
-		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
-		       u8 **sig_ret, u32 *sig_size_ret)
-{
-	CBB out, outer_seq, wrapped_seq, seq, digest_algos_set, digest_algo,
-		null, content_info, issuer_and_serial, signer_infos,
-		signer_info, sign_algo, signature;
-	EVP_MD_CTX md_ctx;
-	u8 *name_der = NULL, *sig = NULL, *pkcs7_data = NULL;
-	size_t pkcs7_data_len, sig_len;
-	int name_der_len, sig_nid;
-	bool ok = false;
-
-	EVP_MD_CTX_init(&md_ctx);
-	BIGNUM *serial = ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), NULL);
-
-	if (!CBB_init(&out, 1024)) {
-		error_msg("out of memory");
-		goto out;
-	}
-
-	name_der_len = i2d_X509_NAME(X509_get_subject_name(cert), &name_der);
-	if (name_der_len < 0) {
-		error_msg_openssl("i2d_X509_NAME failed");
-		goto out;
-	}
-
-	if (!EVP_DigestSignInit(&md_ctx, NULL, md, NULL, pkey)) {
-		error_msg_openssl("EVP_DigestSignInit failed");
-		goto out;
-	}
-
-	sig_len = EVP_PKEY_size(pkey);
-	sig = xmalloc(sig_len);
-	if (!EVP_DigestSign(&md_ctx, sig, &sig_len, data_to_sign, data_size)) {
-		error_msg_openssl("EVP_DigestSign failed");
-		goto out;
-	}
-
-	sig_nid = EVP_PKEY_id(pkey);
-	/* To mirror OpenSSL behaviour, always use |NID_rsaEncryption| with RSA
-	 * rather than the combined hash+pkey NID. */
-	if (sig_nid != NID_rsaEncryption) {
-		OBJ_find_sigid_by_algs(&sig_nid, EVP_MD_type(md),
-				       EVP_PKEY_id(pkey));
-	}
-
-	// See https://tools.ietf.org/html/rfc2315#section-7
-	if (!CBB_add_asn1(&out, &outer_seq, CBS_ASN1_SEQUENCE) ||
-	    !OBJ_nid2cbb(&outer_seq, NID_pkcs7_signed) ||
-	    !CBB_add_asn1(&outer_seq, &wrapped_seq, CBS_ASN1_CONTEXT_SPECIFIC |
-			  CBS_ASN1_CONSTRUCTED | 0) ||
-	    // See https://tools.ietf.org/html/rfc2315#section-9.1
-	    !CBB_add_asn1(&wrapped_seq, &seq, CBS_ASN1_SEQUENCE) ||
-	    !CBB_add_asn1_uint64(&seq, 1 /* version */) ||
-	    !CBB_add_asn1(&seq, &digest_algos_set, CBS_ASN1_SET) ||
-	    !CBB_add_asn1(&digest_algos_set, &digest_algo, CBS_ASN1_SEQUENCE) ||
-	    !OBJ_nid2cbb(&digest_algo, EVP_MD_type(md)) ||
-	    !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
-	    !CBB_add_asn1(&seq, &content_info, CBS_ASN1_SEQUENCE) ||
-	    !OBJ_nid2cbb(&content_info, NID_pkcs7_data) ||
-	    !CBB_add_asn1(&seq, &signer_infos, CBS_ASN1_SET) ||
-	    !CBB_add_asn1(&signer_infos, &signer_info, CBS_ASN1_SEQUENCE) ||
-	    !CBB_add_asn1_uint64(&signer_info, 1 /* version */) ||
-	    !CBB_add_asn1(&signer_info, &issuer_and_serial,
-			  CBS_ASN1_SEQUENCE) ||
-	    !CBB_add_bytes(&issuer_and_serial, name_der, name_der_len) ||
-	    !BN_marshal_asn1(&issuer_and_serial, serial) ||
-	    !CBB_add_asn1(&signer_info, &digest_algo, CBS_ASN1_SEQUENCE) ||
-	    !OBJ_nid2cbb(&digest_algo, EVP_MD_type(md)) ||
-	    !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
-	    !CBB_add_asn1(&signer_info, &sign_algo, CBS_ASN1_SEQUENCE) ||
-	    !OBJ_nid2cbb(&sign_algo, sig_nid) ||
-	    !CBB_add_asn1(&sign_algo, &null, CBS_ASN1_NULL) ||
-	    !CBB_add_asn1(&signer_info, &signature, CBS_ASN1_OCTETSTRING) ||
-	    !CBB_add_bytes(&signature, sig, sig_len) ||
-	    !CBB_finish(&out, &pkcs7_data, &pkcs7_data_len)) {
-		error_msg_openssl("failed to construct PKCS#7 data");
-		goto out;
-	}
-
-	*sig_ret = xmemdup(pkcs7_data, pkcs7_data_len);
-	*sig_size_ret = pkcs7_data_len;
-	ok = true;
-out:
-	BN_free(serial);
-	EVP_MD_CTX_cleanup(&md_ctx);
-	CBB_cleanup(&out);
-	free(sig);
-	OPENSSL_free(name_der);
-	OPENSSL_free(pkcs7_data);
-	return ok;
-}
-
-#else /* OPENSSL_IS_BORINGSSL */
-
-static BIO *new_mem_buf(const void *buf, size_t size)
-{
-	BIO *bio;
-
-	ASSERT(size <= INT_MAX);
-	/*
-	 * Prior to OpenSSL 1.1.0, BIO_new_mem_buf() took a non-const pointer,
-	 * despite still marking the resulting bio as read-only.  So cast away
-	 * the const to avoid a compiler warning with older OpenSSL versions.
-	 */
-	bio = BIO_new_mem_buf((void *)buf, size);
-	if (!bio)
-		error_msg_openssl("out of memory");
-	return bio;
-}
-
-static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
-		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
-		       u8 **sig_ret, u32 *sig_size_ret)
-{
-	/*
-	 * PKCS#7 signing flags:
-	 *
-	 * - PKCS7_BINARY	signing binary data, so skip MIME translation
-	 *
-	 * - PKCS7_DETACHED	omit the signed data (include signature only)
-	 *
-	 * - PKCS7_NOATTR	omit extra authenticated attributes, such as
-	 *			SMIMECapabilities
-	 *
-	 * - PKCS7_NOCERTS	omit the signer's certificate
-	 *
-	 * - PKCS7_PARTIAL	PKCS7_sign() creates a handle only, then
-	 *			PKCS7_sign_add_signer() can add a signer later.
-	 *			This is necessary to change the message digest
-	 *			algorithm from the default of SHA-1.  Requires
-	 *			OpenSSL 1.0.0 or later.
-	 */
-	int pkcs7_flags = PKCS7_BINARY | PKCS7_DETACHED | PKCS7_NOATTR |
-			  PKCS7_NOCERTS | PKCS7_PARTIAL;
-	u8 *sig;
-	u32 sig_size;
-	BIO *bio = NULL;
-	PKCS7 *p7 = NULL;
-	bool ok = false;
-
-	bio = new_mem_buf(data_to_sign, data_size);
-	if (!bio)
-		goto out;
-
-	p7 = PKCS7_sign(NULL, NULL, NULL, bio, pkcs7_flags);
-	if (!p7) {
-		error_msg_openssl("failed to initialize PKCS#7 signature object");
-		goto out;
-	}
-
-	if (!PKCS7_sign_add_signer(p7, cert, pkey, md, pkcs7_flags)) {
-		error_msg_openssl("failed to add signer to PKCS#7 signature object");
-		goto out;
-	}
-
-	if (PKCS7_final(p7, bio, pkcs7_flags) != 1) {
-		error_msg_openssl("failed to finalize PKCS#7 signature");
-		goto out;
-	}
-
-	BIO_free(bio);
-	bio = BIO_new(BIO_s_mem());
-	if (!bio) {
-		error_msg_openssl("out of memory");
-		goto out;
-	}
-
-	if (i2d_PKCS7_bio(bio, p7) != 1) {
-		error_msg_openssl("failed to DER-encode PKCS#7 signature object");
-		goto out;
-	}
-
-	sig_size = BIO_get_mem_data(bio, &sig);
-	*sig_ret = xmemdup(sig, sig_size);
-	*sig_size_ret = sig_size;
-	ok = true;
-out:
-	PKCS7_free(p7);
-	BIO_free(bio);
-	return ok;
-}
-
-#endif /* !OPENSSL_IS_BORINGSSL */
-
-/*
- * Sign the specified @data_to_sign of length @data_size bytes using the private
- * key in @keyfile, the certificate in @certfile, and the hash algorithm
- * @hash_alg.  Returns the DER-formatted PKCS#7 signature in @sig_ret and
- * @sig_size_ret.
- */
-static bool sign_data(const void *data_to_sign, size_t data_size,
-		      const char *keyfile, const char *certfile,
-		      const struct fsverity_hash_alg *hash_alg,
-		      u8 **sig_ret, u32 *sig_size_ret)
-{
-	EVP_PKEY *pkey = NULL;
-	X509 *cert = NULL;
-	const EVP_MD *md;
-	bool ok = false;
-
-	pkey = read_private_key(keyfile);
-	if (!pkey)
-		goto out;
-
-	cert = read_certificate(certfile);
-	if (!cert)
-		goto out;
-
-	OpenSSL_add_all_digests();
-	md = EVP_get_digestbyname(hash_alg->name);
-	if (!md) {
-		fprintf(stderr,
-			"Warning: '%s' algorithm not found in OpenSSL library.\n"
-			"         Falling back to SHA-256 signature.\n",
-			hash_alg->name);
-		md = EVP_sha256();
-	}
-
-	ok = sign_pkcs7(data_to_sign, data_size, pkey, cert, md,
-			sig_ret, sig_size_ret);
-out:
-	EVP_PKEY_free(pkey);
-	X509_free(cert);
-	return ok;
-}
-
 static bool write_signature(const char *filename, const u8 *sig, u32 sig_size)
 {
 	struct filedes file;
@@ -352,161 +28,6 @@ static bool write_signature(const char *filename, const u8 *sig, u32 sig_size)
 	return ok;
 }
 
-#define FS_VERITY_MAX_LEVELS	64
-
-struct block_buffer {
-	u32 filled;
-	u8 *data;
-};
-
-/*
- * Hash a block, writing the result to the next level's pending block buffer.
- * Returns true if the next level's block became full, else false.
- */
-static bool hash_one_block(struct hash_ctx *hash, struct block_buffer *cur,
-			   u32 block_size, const u8 *salt, u32 salt_size)
-{
-	struct block_buffer *next = cur + 1;
-
-	/* Zero-pad the block if it's shorter than block_size. */
-	memset(&cur->data[cur->filled], 0, block_size - cur->filled);
-
-	hash_init(hash);
-	hash_update(hash, salt, salt_size);
-	hash_update(hash, cur->data, block_size);
-	hash_final(hash, &next->data[next->filled]);
-
-	next->filled += hash->alg->digest_size;
-	cur->filled = 0;
-
-	return next->filled + hash->alg->digest_size > block_size;
-}
-
-/*
- * Compute the file's Merkle tree root hash using the given hash algorithm,
- * block size, and salt.
- */
-static bool compute_root_hash(struct filedes *file, u64 file_size,
-			      struct hash_ctx *hash, u32 block_size,
-			      const u8 *salt, u32 salt_size, u8 *root_hash)
-{
-	const u32 hashes_per_block = block_size / hash->alg->digest_size;
-	const u32 padded_salt_size = roundup(salt_size, hash->alg->block_size);
-	u8 *padded_salt = xzalloc(padded_salt_size);
-	u64 blocks;
-	int num_levels = 0;
-	int level;
-	struct block_buffer _buffers[1 + FS_VERITY_MAX_LEVELS + 1] = {};
-	struct block_buffer *buffers = &_buffers[1];
-	u64 offset;
-	bool ok = false;
-
-	if (salt_size != 0)
-		memcpy(padded_salt, salt, salt_size);
-
-	/* Compute number of levels */
-	for (blocks = DIV_ROUND_UP(file_size, block_size); blocks > 1;
-	     blocks = DIV_ROUND_UP(blocks, hashes_per_block)) {
-		ASSERT(num_levels < FS_VERITY_MAX_LEVELS);
-		num_levels++;
-	}
-
-	/*
-	 * Allocate the block buffers.  Buffer "-1" is for data blocks.
-	 * Buffers 0 <= level < num_levels are for the actual tree levels.
-	 * Buffer 'num_levels' is for the root hash.
-	 */
-	for (level = -1; level < num_levels; level++)
-		buffers[level].data = xmalloc(block_size);
-	buffers[num_levels].data = root_hash;
-
-	/* Hash each data block, also hashing the tree blocks as they fill up */
-	for (offset = 0; offset < file_size; offset += block_size) {
-		buffers[-1].filled = min(block_size, file_size - offset);
-
-		if (!full_read(file, buffers[-1].data, buffers[-1].filled))
-			goto out;
-
-		level = -1;
-		while (hash_one_block(hash, &buffers[level], block_size,
-				      padded_salt, padded_salt_size)) {
-			level++;
-			ASSERT(level < num_levels);
-		}
-	}
-	/* Finish all nonempty pending tree blocks */
-	for (level = 0; level < num_levels; level++) {
-		if (buffers[level].filled != 0)
-			hash_one_block(hash, &buffers[level], block_size,
-				       padded_salt, padded_salt_size);
-	}
-
-	/* Root hash was filled by the last call to hash_one_block() */
-	ASSERT(buffers[num_levels].filled == hash->alg->digest_size);
-	ok = true;
-out:
-	for (level = -1; level < num_levels; level++)
-		free(buffers[level].data);
-	free(padded_salt);
-	return ok;
-}
-
-/*
- * Compute the fs-verity measurement of the given file.
- *
- * The fs-verity measurement is the hash of the fsverity_descriptor, which
- * contains the Merkle tree properties including the root hash.
- */
-static bool compute_file_measurement(const char *filename,
-				     const struct fsverity_hash_alg *hash_alg,
-				     u32 block_size, const u8 *salt,
-				     u32 salt_size, u8 *measurement)
-{
-	struct filedes file = { .fd = -1 };
-	struct hash_ctx *hash = hash_create(hash_alg);
-	u64 file_size;
-	struct fsverity_descriptor desc;
-	bool ok = false;
-
-	if (!open_file(&file, filename, O_RDONLY, 0))
-		goto out;
-
-	if (!get_file_size(&file, &file_size))
-		goto out;
-
-	memset(&desc, 0, sizeof(desc));
-	desc.version = 1;
-	desc.hash_algorithm = hash_alg - fsverity_hash_algs;
-
-	ASSERT(is_power_of_2(block_size));
-	desc.log_blocksize = ilog2(block_size);
-
-	if (salt_size != 0) {
-		if (salt_size > sizeof(desc.salt)) {
-			error_msg("Salt too long (got %u bytes; max is %zu bytes)",
-				  salt_size, sizeof(desc.salt));
-			goto out;
-		}
-		memcpy(desc.salt, salt, salt_size);
-		desc.salt_size = salt_size;
-	}
-
-	desc.data_size = cpu_to_le64(file_size);
-
-	/* Root hash of empty file is all 0's */
-	if (file_size != 0 &&
-	    !compute_root_hash(&file, file_size, hash, block_size, salt,
-			       salt_size, desc.root_hash))
-		goto out;
-
-	hash_full(hash, &desc, sizeof(desc), measurement);
-	ok = true;
-out:
-	filedes_close(&file);
-	hash_free(hash);
-	return ok;
-}
-
 enum {
 	OPT_HASH_ALG,
 	OPT_BLOCK_SIZE,
diff --git a/lib/compute_digest.c b/lib/compute_digest.c
new file mode 100644
index 0000000..b279d63
--- /dev/null
+++ b/lib/compute_digest.c
@@ -0,0 +1,184 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * compute_digest.c
+ *
+ * Copyright (C) 2018 Google LLC
+ */
+
+#include "sign.h"
+
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+
+#define FS_VERITY_MAX_LEVELS	64
+
+/*
+ * Merkle tree properties.  The file measurement is the hash of this structure
+ * excluding the signature and with the sig_size field set to 0.
+ */
+struct fsverity_descriptor {
+	__u8 version;		/* must be 1 */
+	__u8 hash_algorithm;	/* Merkle tree hash algorithm */
+	__u8 log_blocksize;	/* log2 of size of data and tree blocks */
+	__u8 salt_size;		/* size of salt in bytes; 0 if none */
+	__le32 sig_size;	/* size of signature in bytes; 0 if none */
+	__le64 data_size;	/* size of file the Merkle tree is built over */
+	__u8 root_hash[64];	/* Merkle tree root hash */
+	__u8 salt[32];		/* salt prepended to each hashed block */
+	__u8 __reserved[144];	/* must be 0's */
+	__u8 signature[];	/* optional PKCS#7 signature */
+};
+
+struct block_buffer {
+	u32 filled;
+	u8 *data;
+};
+
+/*
+ * Hash a block, writing the result to the next level's pending block buffer.
+ * Returns true if the next level's block became full, else false.
+ */
+static bool hash_one_block(struct hash_ctx *hash, struct block_buffer *cur,
+			   u32 block_size, const u8 *salt, u32 salt_size)
+{
+	struct block_buffer *next = cur + 1;
+
+	/* Zero-pad the block if it's shorter than block_size. */
+	memset(&cur->data[cur->filled], 0, block_size - cur->filled);
+
+	hash_init(hash);
+	hash_update(hash, salt, salt_size);
+	hash_update(hash, cur->data, block_size);
+	hash_final(hash, &next->data[next->filled]);
+
+	next->filled += hash->alg->digest_size;
+	cur->filled = 0;
+
+	return next->filled + hash->alg->digest_size > block_size;
+}
+
+/*
+ * Compute the file's Merkle tree root hash using the given hash algorithm,
+ * block size, and salt.
+ */
+static bool compute_root_hash(struct filedes *file, u64 file_size,
+			      struct hash_ctx *hash, u32 block_size,
+			      const u8 *salt, u32 salt_size, u8 *root_hash)
+{
+	const u32 hashes_per_block = block_size / hash->alg->digest_size;
+	const u32 padded_salt_size = roundup(salt_size, hash->alg->block_size);
+	u8 *padded_salt = xzalloc(padded_salt_size);
+	u64 blocks;
+	int num_levels = 0;
+	int level;
+	struct block_buffer _buffers[1 + FS_VERITY_MAX_LEVELS + 1] = {};
+	struct block_buffer *buffers = &_buffers[1];
+	u64 offset;
+	bool ok = false;
+
+	if (salt_size != 0)
+		memcpy(padded_salt, salt, salt_size);
+
+	/* Compute number of levels */
+	for (blocks = DIV_ROUND_UP(file_size, block_size); blocks > 1;
+	     blocks = DIV_ROUND_UP(blocks, hashes_per_block)) {
+		ASSERT(num_levels < FS_VERITY_MAX_LEVELS);
+		num_levels++;
+	}
+
+	/*
+	 * Allocate the block buffers.  Buffer "-1" is for data blocks.
+	 * Buffers 0 <= level < num_levels are for the actual tree levels.
+	 * Buffer 'num_levels' is for the root hash.
+	 */
+	for (level = -1; level < num_levels; level++)
+		buffers[level].data = xmalloc(block_size);
+	buffers[num_levels].data = root_hash;
+
+	/* Hash each data block, also hashing the tree blocks as they fill up */
+	for (offset = 0; offset < file_size; offset += block_size) {
+		buffers[-1].filled = min(block_size, file_size - offset);
+
+		if (!full_read(file, buffers[-1].data, buffers[-1].filled))
+			goto out;
+
+		level = -1;
+		while (hash_one_block(hash, &buffers[level], block_size,
+				      padded_salt, padded_salt_size)) {
+			level++;
+			ASSERT(level < num_levels);
+		}
+	}
+	/* Finish all nonempty pending tree blocks */
+	for (level = 0; level < num_levels; level++) {
+		if (buffers[level].filled != 0)
+			hash_one_block(hash, &buffers[level], block_size,
+				       padded_salt, padded_salt_size);
+	}
+
+	/* Root hash was filled by the last call to hash_one_block() */
+	ASSERT(buffers[num_levels].filled == hash->alg->digest_size);
+	ok = true;
+out:
+	for (level = -1; level < num_levels; level++)
+		free(buffers[level].data);
+	free(padded_salt);
+	return ok;
+}
+
+/*
+ * Compute the fs-verity measurement of the given file.
+ *
+ * The fs-verity measurement is the hash of the fsverity_descriptor, which
+ * contains the Merkle tree properties including the root hash.
+ */
+bool compute_file_measurement(const char *filename,
+			      const struct fsverity_hash_alg *hash_alg,
+			      u32 block_size, const u8 *salt,
+			      u32 salt_size, u8 *measurement)
+{
+	struct filedes file = { .fd = -1 };
+	struct hash_ctx *hash = hash_create(hash_alg);
+	u64 file_size;
+	struct fsverity_descriptor desc;
+	bool ok = false;
+
+	if (!open_file(&file, filename, O_RDONLY, 0))
+		goto out;
+
+	if (!get_file_size(&file, &file_size))
+		goto out;
+
+	memset(&desc, 0, sizeof(desc));
+	desc.version = 1;
+	desc.hash_algorithm = hash_alg - fsverity_hash_algs;
+
+	ASSERT(is_power_of_2(block_size));
+	desc.log_blocksize = ilog2(block_size);
+
+	if (salt_size != 0) {
+		if (salt_size > sizeof(desc.salt)) {
+			error_msg("Salt too long (got %u bytes; max is %zu bytes)",
+				  salt_size, sizeof(desc.salt));
+			goto out;
+		}
+		memcpy(desc.salt, salt, salt_size);
+		desc.salt_size = salt_size;
+	}
+
+	desc.data_size = cpu_to_le64(file_size);
+
+	/* Root hash of empty file is all 0's */
+	if (file_size != 0 &&
+	    !compute_root_hash(&file, file_size, hash, block_size, salt,
+			       salt_size, desc.root_hash))
+		goto out;
+
+	hash_full(hash, &desc, sizeof(desc), measurement);
+	ok = true;
+out:
+	filedes_close(&file);
+	hash_free(hash);
+	return ok;
+}
diff --git a/lib/sign_digest.c b/lib/sign_digest.c
new file mode 100644
index 0000000..d2b0d53
--- /dev/null
+++ b/lib/sign_digest.c
@@ -0,0 +1,304 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * sign_digest.c
+ *
+ * Copyright (C) 2018 Google LLC
+ */
+
+#include "hash_algs.h"
+#include "sign.h"
+
+#include <limits.h>
+#include <openssl/bio.h>
+#include <openssl/err.h>
+#include <openssl/pem.h>
+#include <openssl/pkcs7.h>
+
+static void __printf(1, 2) __cold
+error_msg_openssl(const char *format, ...)
+{
+	va_list va;
+
+	va_start(va, format);
+	do_error_msg(format, va, 0);
+	va_end(va);
+
+	if (ERR_peek_error() == 0)
+		return;
+
+	fprintf(stderr, "OpenSSL library errors:\n");
+	ERR_print_errors_fp(stderr);
+}
+
+/* Read a PEM PKCS#8 formatted private key */
+static EVP_PKEY *read_private_key(const char *keyfile)
+{
+	BIO *bio;
+	EVP_PKEY *pkey;
+
+	bio = BIO_new_file(keyfile, "r");
+	if (!bio) {
+		error_msg_openssl("can't open '%s' for reading", keyfile);
+		return NULL;
+	}
+
+	pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
+	if (!pkey) {
+		error_msg_openssl("Failed to parse private key file '%s'.\n"
+				  "       Note: it must be in PEM PKCS#8 format.",
+				  keyfile);
+	}
+	BIO_free(bio);
+	return pkey;
+}
+
+/* Read a PEM X.509 formatted certificate */
+static X509 *read_certificate(const char *certfile)
+{
+	BIO *bio;
+	X509 *cert;
+
+	bio = BIO_new_file(certfile, "r");
+	if (!bio) {
+		error_msg_openssl("can't open '%s' for reading", certfile);
+		return NULL;
+	}
+	cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
+	if (!cert) {
+		error_msg_openssl("Failed to parse X.509 certificate file '%s'.\n"
+				  "       Note: it must be in PEM format.",
+				  certfile);
+	}
+	BIO_free(bio);
+	return cert;
+}
+
+#ifdef OPENSSL_IS_BORINGSSL
+
+static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
+		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
+		       u8 **sig_ret, u32 *sig_size_ret)
+{
+	CBB out, outer_seq, wrapped_seq, seq, digest_algos_set, digest_algo,
+		null, content_info, issuer_and_serial, signer_infos,
+		signer_info, sign_algo, signature;
+	EVP_MD_CTX md_ctx;
+	u8 *name_der = NULL, *sig = NULL, *pkcs7_data = NULL;
+	size_t pkcs7_data_len, sig_len;
+	int name_der_len, sig_nid;
+	bool ok = false;
+
+	EVP_MD_CTX_init(&md_ctx);
+	BIGNUM *serial = ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), NULL);
+
+	if (!CBB_init(&out, 1024)) {
+		error_msg("out of memory");
+		goto out;
+	}
+
+	name_der_len = i2d_X509_NAME(X509_get_subject_name(cert), &name_der);
+	if (name_der_len < 0) {
+		error_msg_openssl("i2d_X509_NAME failed");
+		goto out;
+	}
+
+	if (!EVP_DigestSignInit(&md_ctx, NULL, md, NULL, pkey)) {
+		error_msg_openssl("EVP_DigestSignInit failed");
+		goto out;
+	}
+
+	sig_len = EVP_PKEY_size(pkey);
+	sig = xmalloc(sig_len);
+	if (!EVP_DigestSign(&md_ctx, sig, &sig_len, data_to_sign, data_size)) {
+		error_msg_openssl("EVP_DigestSign failed");
+		goto out;
+	}
+
+	sig_nid = EVP_PKEY_id(pkey);
+	/* To mirror OpenSSL behaviour, always use |NID_rsaEncryption| with RSA
+	 * rather than the combined hash+pkey NID. */
+	if (sig_nid != NID_rsaEncryption) {
+		OBJ_find_sigid_by_algs(&sig_nid, EVP_MD_type(md),
+				       EVP_PKEY_id(pkey));
+	}
+
+	// See https://tools.ietf.org/html/rfc2315#section-7
+	if (!CBB_add_asn1(&out, &outer_seq, CBS_ASN1_SEQUENCE) ||
+	    !OBJ_nid2cbb(&outer_seq, NID_pkcs7_signed) ||
+	    !CBB_add_asn1(&outer_seq, &wrapped_seq, CBS_ASN1_CONTEXT_SPECIFIC |
+			  CBS_ASN1_CONSTRUCTED | 0) ||
+	    // See https://tools.ietf.org/html/rfc2315#section-9.1
+	    !CBB_add_asn1(&wrapped_seq, &seq, CBS_ASN1_SEQUENCE) ||
+	    !CBB_add_asn1_uint64(&seq, 1 /* version */) ||
+	    !CBB_add_asn1(&seq, &digest_algos_set, CBS_ASN1_SET) ||
+	    !CBB_add_asn1(&digest_algos_set, &digest_algo, CBS_ASN1_SEQUENCE) ||
+	    !OBJ_nid2cbb(&digest_algo, EVP_MD_type(md)) ||
+	    !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
+	    !CBB_add_asn1(&seq, &content_info, CBS_ASN1_SEQUENCE) ||
+	    !OBJ_nid2cbb(&content_info, NID_pkcs7_data) ||
+	    !CBB_add_asn1(&seq, &signer_infos, CBS_ASN1_SET) ||
+	    !CBB_add_asn1(&signer_infos, &signer_info, CBS_ASN1_SEQUENCE) ||
+	    !CBB_add_asn1_uint64(&signer_info, 1 /* version */) ||
+	    !CBB_add_asn1(&signer_info, &issuer_and_serial,
+			  CBS_ASN1_SEQUENCE) ||
+	    !CBB_add_bytes(&issuer_and_serial, name_der, name_der_len) ||
+	    !BN_marshal_asn1(&issuer_and_serial, serial) ||
+	    !CBB_add_asn1(&signer_info, &digest_algo, CBS_ASN1_SEQUENCE) ||
+	    !OBJ_nid2cbb(&digest_algo, EVP_MD_type(md)) ||
+	    !CBB_add_asn1(&digest_algo, &null, CBS_ASN1_NULL) ||
+	    !CBB_add_asn1(&signer_info, &sign_algo, CBS_ASN1_SEQUENCE) ||
+	    !OBJ_nid2cbb(&sign_algo, sig_nid) ||
+	    !CBB_add_asn1(&sign_algo, &null, CBS_ASN1_NULL) ||
+	    !CBB_add_asn1(&signer_info, &signature, CBS_ASN1_OCTETSTRING) ||
+	    !CBB_add_bytes(&signature, sig, sig_len) ||
+	    !CBB_finish(&out, &pkcs7_data, &pkcs7_data_len)) {
+		error_msg_openssl("failed to construct PKCS#7 data");
+		goto out;
+	}
+
+	*sig_ret = xmemdup(pkcs7_data, pkcs7_data_len);
+	*sig_size_ret = pkcs7_data_len;
+	ok = true;
+out:
+	BN_free(serial);
+	EVP_MD_CTX_cleanup(&md_ctx);
+	CBB_cleanup(&out);
+	free(sig);
+	OPENSSL_free(name_der);
+	OPENSSL_free(pkcs7_data);
+	return ok;
+}
+
+#else /* OPENSSL_IS_BORINGSSL */
+
+static BIO *new_mem_buf(const void *buf, size_t size)
+{
+	BIO *bio;
+
+	ASSERT(size <= INT_MAX);
+	/*
+	 * Prior to OpenSSL 1.1.0, BIO_new_mem_buf() took a non-const pointer,
+	 * despite still marking the resulting bio as read-only.  So cast away
+	 * the const to avoid a compiler warning with older OpenSSL versions.
+	 */
+	bio = BIO_new_mem_buf((void *)buf, size);
+	if (!bio)
+		error_msg_openssl("out of memory");
+	return bio;
+}
+
+static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
+		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
+		       u8 **sig_ret, u32 *sig_size_ret)
+{
+	/*
+	 * PKCS#7 signing flags:
+	 *
+	 * - PKCS7_BINARY	signing binary data, so skip MIME translation
+	 *
+	 * - PKCS7_DETACHED	omit the signed data (include signature only)
+	 *
+	 * - PKCS7_NOATTR	omit extra authenticated attributes, such as
+	 *			SMIMECapabilities
+	 *
+	 * - PKCS7_NOCERTS	omit the signer's certificate
+	 *
+	 * - PKCS7_PARTIAL	PKCS7_sign() creates a handle only, then
+	 *			PKCS7_sign_add_signer() can add a signer later.
+	 *			This is necessary to change the message digest
+	 *			algorithm from the default of SHA-1.  Requires
+	 *			OpenSSL 1.0.0 or later.
+	 */
+	int pkcs7_flags = PKCS7_BINARY | PKCS7_DETACHED | PKCS7_NOATTR |
+			  PKCS7_NOCERTS | PKCS7_PARTIAL;
+	u8 *sig;
+	u32 sig_size;
+	BIO *bio = NULL;
+	PKCS7 *p7 = NULL;
+	bool ok = false;
+
+	bio = new_mem_buf(data_to_sign, data_size);
+	if (!bio)
+		goto out;
+
+	p7 = PKCS7_sign(NULL, NULL, NULL, bio, pkcs7_flags);
+	if (!p7) {
+		error_msg_openssl("failed to initialize PKCS#7 signature object");
+		goto out;
+	}
+
+	if (!PKCS7_sign_add_signer(p7, cert, pkey, md, pkcs7_flags)) {
+		error_msg_openssl("failed to add signer to PKCS#7 signature object");
+		goto out;
+	}
+
+	if (PKCS7_final(p7, bio, pkcs7_flags) != 1) {
+		error_msg_openssl("failed to finalize PKCS#7 signature");
+		goto out;
+	}
+
+	BIO_free(bio);
+	bio = BIO_new(BIO_s_mem());
+	if (!bio) {
+		error_msg_openssl("out of memory");
+		goto out;
+	}
+
+	if (i2d_PKCS7_bio(bio, p7) != 1) {
+		error_msg_openssl("failed to DER-encode PKCS#7 signature object");
+		goto out;
+	}
+
+	sig_size = BIO_get_mem_data(bio, &sig);
+	*sig_ret = xmemdup(sig, sig_size);
+	*sig_size_ret = sig_size;
+	ok = true;
+out:
+	PKCS7_free(p7);
+	BIO_free(bio);
+	return ok;
+}
+
+#endif /* !OPENSSL_IS_BORINGSSL */
+
+/*
+ * Sign the specified @data_to_sign of length @data_size bytes using the private
+ * key in @keyfile, the certificate in @certfile, and the hash algorithm
+ * @hash_alg.  Returns the DER-formatted PKCS#7 signature in @sig_ret and
+ * @sig_size_ret.
+ */
+bool sign_data(const void *data_to_sign, size_t data_size,
+	       const char *keyfile, const char *certfile,
+	       const struct fsverity_hash_alg *hash_alg,
+	       u8 **sig_ret, u32 *sig_size_ret)
+{
+	EVP_PKEY *pkey = NULL;
+	X509 *cert = NULL;
+	const EVP_MD *md;
+	bool ok = false;
+
+	pkey = read_private_key(keyfile);
+	if (!pkey)
+		goto out;
+
+	cert = read_certificate(certfile);
+	if (!cert)
+		goto out;
+
+	OpenSSL_add_all_digests();
+	md = EVP_get_digestbyname(hash_alg->name);
+	if (!md) {
+		fprintf(stderr,
+			"Warning: '%s' algorithm not found in OpenSSL library.\n"
+			"         Falling back to SHA-256 signature.\n",
+			hash_alg->name);
+		md = EVP_sha256();
+	}
+
+	ok = sign_pkcs7(data_to_sign, data_size, pkey, cert, md,
+			sig_ret, sig_size_ret);
+out:
+	EVP_PKEY_free(pkey);
+	X509_free(cert);
+	return ok;
+}
diff --git a/sign.h b/sign.h
new file mode 100644
index 0000000..f10ac91
--- /dev/null
+++ b/sign.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+#ifndef SIGN_H
+#define SIGN_H
+
+#include "hash_algs.h"
+
+#include <linux/types.h>
+
+/*
+ * Format in which verity file measurements are signed.  This is the same as
+ * 'struct fsverity_digest', except here some magic bytes are prepended to
+ * provide some context about what is being signed in case the same key is used
+ * for non-fsverity purposes, and here the fields have fixed endianness.
+ */
+struct fsverity_signed_digest {
+	char magic[8];			/* must be "FSVerity" */
+	__le16 digest_algorithm;
+	__le16 digest_size;
+	__u8 digest[];
+};
+
+bool compute_file_measurement(const char *filename,
+			      const struct fsverity_hash_alg *hash_alg,
+			      u32 block_size, const u8 *salt,
+			      u32 salt_size, u8 *measurement);
+
+bool sign_data(const void *data_to_sign, size_t data_size,
+	       const char *keyfile, const char *certfile,
+	       const struct fsverity_hash_alg *hash_alg,
+	       u8 **sig_ret, u32 *sig_size_ret);
+
+#endif /* SIGN_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/3] Introduce libfsverity
  2020-05-15  4:10 [PATCH 0/3] fsverity-utils: introduce libfsverity Eric Biggers
  2020-05-15  4:10 ` [PATCH 1/3] Split up cmd_sign.c Eric Biggers
@ 2020-05-15  4:10 ` Eric Biggers
  2020-05-21 15:24   ` Jes Sorensen
  2020-05-15  4:10 ` [PATCH 3/3] Add some basic test programs for libfsverity Eric Biggers
  2020-05-15 20:50 ` [PATCH 0/3] fsverity-utils: introduce libfsverity Jes Sorensen
  3 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-15  4:10 UTC (permalink / raw)
  To: linux-fscrypt, Jes Sorensen; +Cc: jsorensen, kernel-team

From: Eric Biggers <ebiggers@google.com>

From the 'fsverity' program, split out a library 'libfsverity'.
Currently it supports computing file measurements ("digests"), and
signing those file measurements for use with the fs-verity builtin
signature verification feature.

Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
I made a lot of improvements, e.g.:

- Separated library and program source into different directories.
- Drastically improved the Makefile.
- Added 'make check' target and rules to build test programs.
- In the shared lib, only export the functions intended to be public.
- Prefixed global functions with "libfsverity_" so that they don't cause
  conflicts when the library is built as a static library.
- Made library error messages be sent to a user-specified callback
  rather than always be printed to stderr.
- Keep showing OpenSSL error messages.
- Stopped abort()ing in library code, when possible.
- Made libfsverity_digest use native endianness.
- Moved file_size into the merkle_tree_params.
- Made libfsverity_hash_name() just return the static strings.
- Made some variables in the API uint32_t instead of uint16_t.
- Shared parse_hash_alg_option() between cmd_enable and cmd_sign.
- Lots of fixes.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 .gitignore                                |   9 +-
 Makefile                                  | 198 +++++++++++++++++++--
 commands.h                                |  24 ---
 util.h => common/common_defs.h            |  47 +----
 fsverity_uapi.h => common/fsverity_uapi.h |   0
 common/libfsverity.h                      | 132 ++++++++++++++
 hash_algs.h                               |  68 --------
 lib/compute_digest.c                      | 181 ++++++++++++-------
 hash_algs.c => lib/hash_algs.c            | 126 +++++++++-----
 lib/lib_private.h                         |  83 +++++++++
 lib/sign_digest.c                         | 201 ++++++++++++++++------
 lib/utils.c                               | 107 ++++++++++++
 cmd_enable.c => programs/cmd_enable.c     |  32 +---
 cmd_measure.c => programs/cmd_measure.c   |  12 +-
 cmd_sign.c => programs/cmd_sign.c         |  91 +++++-----
 fsverity.c => programs/fsverity.c         |  52 +++++-
 programs/fsverity.h                       |  41 +++++
 util.c => programs/utils.c                |   7 +-
 programs/utils.h                          |  42 +++++
 sign.h                                    |  32 ----
 20 files changed, 1048 insertions(+), 437 deletions(-)
 delete mode 100644 commands.h
 rename util.h => common/common_defs.h (58%)
 rename fsverity_uapi.h => common/fsverity_uapi.h (100%)
 create mode 100644 common/libfsverity.h
 delete mode 100644 hash_algs.h
 rename hash_algs.c => lib/hash_algs.c (54%)
 create mode 100644 lib/lib_private.h
 create mode 100644 lib/utils.c
 rename cmd_enable.c => programs/cmd_enable.c (82%)
 rename cmd_measure.c => programs/cmd_measure.c (84%)
 rename cmd_sign.c => programs/cmd_sign.c (54%)
 rename fsverity.c => programs/fsverity.c (82%)
 create mode 100644 programs/fsverity.h
 rename util.c => programs/utils.c (96%)
 create mode 100644 programs/utils.h
 delete mode 100644 sign.h

diff --git a/.gitignore b/.gitignore
index 95457ca..265306a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,5 +1,10 @@
-fsverity
+*.a
 *.o
-tags
+*.so
+*.so.*
+/.build-config
+/fsverity
+/test_*
 cscope.*
 ncscope.*
+tags
diff --git a/Makefile b/Makefile
index c3b14a3..1a7be53 100644
--- a/Makefile
+++ b/Makefile
@@ -1,22 +1,190 @@
-EXE := fsverity
-CFLAGS := -O2 -Wall
-CPPFLAGS := -D_FILE_OFFSET_BITS=64 -I.
-LDLIBS := -lcrypto
-DESTDIR := /usr/local
-SRC := $(wildcard *.c) $(wildcard lib/*.c)
-OBJ := $(SRC:.c=.o)
-HDRS := $(wildcard *.h)
+# SPDX-License-Identifier: GPL-2.0+
+#
+# Use 'make help' to list available targets.
+#
+# Define V=1 to enable "verbose" mode, showing all executed commands.
+#
+# Define USE_SHARED_LIB=1 to link the fsverity binary to the shared library
+# libfsverity.so rather than the static library libfsverity.a.
+#
+# Define PREFIX to override the installation prefix, like './configure --prefix'
+# in autotools-based projects (default: /usr/local)
+#
+# Define BINDIR to override where to install binaries, like './configure
+# --bindir' in autotools-based projects (default: PREFIX/bin)
+#
+# Define INCDIR to override where to install headers, like './configure
+# --includedir' in autotools-based projects (default: PREFIX/include)
+#
+# Define LIBDIR to override where to install libraries, like './configure
+# --libdir' in autotools-based projects (default: PREFIX/lib)
+#
+# Define DESTDIR to override the installation destination directory
+# (default: empty string)
+#
+# You can also specify custom CC, CFLAGS, CPPFLAGS, and/or LDFLAGS.
+#
+##############################################################################
 
-all:$(EXE)
+cc-option = $(shell if $(CC) $(1) -c -x c /dev/null -o /dev/null &>/dev/null; \
+	      then echo $(1); fi)
 
-$(EXE):$(OBJ)
+#### Common compiler flags.  You can add additional flags by defining CFLAGS
+#### and/or CPPFLAGS in the environment or on the 'make' command line.
 
-$(OBJ): %.o: %.c $(HDRS)
+override CFLAGS := -O2 -Wall -Wundef				\
+	$(call cc-option,-Wdeclaration-after-statement)		\
+	$(call cc-option,-Wmissing-prototypes)			\
+	$(call cc-option,-Wstrict-prototypes)			\
+	$(call cc-option,-Wvla)					\
+	$(call cc-option,-Wimplicit-fallthrough)		\
+	$(CFLAGS)
 
-clean:
-	rm -f $(EXE) $(OBJ)
+override CPPFLAGS := -D_FILE_OFFSET_BITS=64 $(CPPFLAGS)
+
+#### Other user settings
+
+ifneq ($(V),1)
+QUIET_CC        = @echo '  CC      ' $@;
+QUIET_CCLD      = @echo '  CCLD    ' $@;
+QUIET_AR        = @echo '  AR      ' $@;
+QUIET_LN        = @echo '  LN      ' $@;
+endif
+USE_SHARED_LIB  ?=
+PREFIX          ?= /usr/local
+BINDIR          ?= $(PREFIX)/bin
+INCDIR          ?= $(PREFIX)/include
+LIBDIR          ?= $(PREFIX)/lib
+DESTDIR         ?=
+
+# Rebuild if a user-specified setting that affects the build changed.
+.build-config: FORCE
+	@flags='$(CC):$(CFLAGS):$(CPPFLAGS):$(LDFLAGS):$(USE_SHARED_LIB)'; \
+	if [ "$$flags" != "`cat $@ 2>/dev/null`" ]; then		\
+		[ -e $@ ] && echo "Rebuilding due to new settings";	\
+		echo "$$flags" > $@;					\
+	fi
+
+#### Other variables
+
+DEFAULT_TARGETS :=
+COMMON_HEADERS  := $(wildcard common/*.h)
+LDLIBS          := -lcrypto
+
+##############################################################################
+
+#### Library
+
+SOVERSION       := 0
+LIB_CFLAGS      := $(CFLAGS) -fvisibility=hidden
+LIB_SRC         := $(wildcard lib/*.c)
+LIB_HEADERS     := $(wildcard lib/*.h) $(COMMON_HEADERS)
+STATIC_LIB_OBJ  := $(LIB_SRC:.c=.o)
+SHARED_LIB_OBJ  := $(LIB_SRC:.c=.shlib.o)
+
+# Compile static library object files
+$(STATIC_LIB_OBJ): %.o: %.c $(LIB_HEADERS) .build-config
+	$(QUIET_CC) $(CC) -o $@ -c $(CPPFLAGS) $(LIB_CFLAGS) $<
+
+# Compile shared library object files
+$(SHARED_LIB_OBJ): %.shlib.o: %.c $(LIB_HEADERS) .build-config
+	$(QUIET_CC) $(CC) -o $@ -c $(CPPFLAGS) $(LIB_CFLAGS) -fPIC $<
+
+# Create static library
+libfsverity.a:$(STATIC_LIB_OBJ)
+	$(QUIET_AR) $(AR) cr $@ $+
+
+DEFAULT_TARGETS += libfsverity.a
+
+# Create shared library
+libfsverity.so.$(SOVERSION):$(SHARED_LIB_OBJ)
+	$(QUIET_CCLD) $(CC) -o $@ -Wl,-soname=libfsverity.so.$(SOVERSION) \
+		-shared $+ $(LDFLAGS) $(LDLIBS)
+
+DEFAULT_TARGETS += libfsverity.so.$(SOVERSION)
+
+# Create the symlink libfsverity.so => libfsverity.so.$(SOVERSION)
+libfsverity.so:libfsverity.so.$(SOVERSION)
+	$(QUIET_LN) ln -sf $+ $@
+
+DEFAULT_TARGETS += libfsverity.so
+
+##############################################################################
+
+#### Programs
+
+ALL_PROG_SRC      := $(wildcard programs/*.c)
+ALL_PROG_OBJ      := $(ALL_PROG_SRC:.c=.o)
+ALL_PROG_HEADERS  := $(wildcard programs/*.h) $(COMMON_HEADERS)
+PROG_COMMON_SRC   := programs/utils.c
+PROG_COMMON_OBJ   := $(PROG_COMMON_SRC:.c=.o)
+FSVERITY_PROG_OBJ := $(PROG_COMMON_OBJ) \
+		     programs/cmd_enable.o \
+		     programs/cmd_measure.o \
+		     programs/cmd_sign.o \
+		     programs/fsverity.o
+TEST_PROG_SRC     := $(wildcard programs/test_*.c)
+TEST_PROGRAMS     := $(TEST_PROG_SRC:programs/%.c=%)
+
+# Compile program object files
+$(ALL_PROG_OBJ): %.o: %.c $(ALL_PROG_HEADERS) .build-config
+	$(QUIET_CC) $(CC) -o $@ -c $(CPPFLAGS) $(CFLAGS) $<
+
+# Link the fsverity program
+ifdef USE_SHARED_LIB
+fsverity: $(FSVERITY_PROG_OBJ) libfsverity.so
+	$(QUIET_CCLD) $(CC) -o $@ $(FSVERITY_PROG_OBJ) -L. -lfsverity
+else
+fsverity: $(FSVERITY_PROG_OBJ) libfsverity.a
+	$(QUIET_CCLD) $(CC) -o $@ $+ $(LDFLAGS) $(LDLIBS)
+endif
+
+DEFAULT_TARGETS += fsverity
+
+# Link the test programs
+$(TEST_PROGRAMS): %: programs/%.o $(PROG_COMMON_OBJ) libfsverity.a
+	$(QUIET_CCLD) $(CC) -o $@ $+ $(LDFLAGS) $(LDLIBS)
+
+##############################################################################
+
+all:$(DEFAULT_TARGETS)
+
+test_programs:$(TEST_PROGRAMS)
+
+check:test_programs
+	for prog in $(TEST_PROGRAMS); do \
+		./$$prog || exit 1; \
+	done
+	@echo "All tests passed!"
 
 install:all
-	install -Dm755 -t $(DESTDIR)/bin $(EXE)
+	install -d $(DESTDIR)$(LIBDIR) $(DESTDIR)$(INCDIR) $(DESTDIR)$(BINDIR)
+	install -m755 fsverity $(DESTDIR)$(BINDIR)
+	install -m644 libfsverity.a $(DESTDIR)$(LIBDIR)
+	install -m755 libfsverity.so.$(SOVERSION) $(DESTDIR)$(LIBDIR)
+	ln -sf libfsverity.so.$(SOVERSION) $(DESTDIR)$(LIBDIR)/libfsverity.so
+	install -m644 common/libfsverity.h $(DESTDIR)$(INCDIR)
+
+uninstall:
+	rm -f $(DESTDIR)$(BINDIR)/fsverity
+	rm -f $(DESTDIR)$(LIBDIR)/libfsverity.a
+	rm -f $(DESTDIR)$(LIBDIR)/libfsverity.so.$(SOVERSION)
+	rm -f $(DESTDIR)$(LIBDIR)/libfsverity.so
+	rm -f $(DESTDIR)$(INCDIR)/libfsverity.h
+
+help:
+	@echo "Available targets:"
+	@echo "------------------"
+	@for target in $(DEFAULT_TARGETS) $(TEST_PROGRAMS); do \
+		echo $$target; \
+	done
+
+clean:
+	rm -f $(DEFAULT_TARGETS) $(TEST_PROGRAMS) $(LIB_OBJS) $(ALL_PROG_OBJ) \
+		.build-config
+
+FORCE:
+
+.PHONY: all test_programs check install uninstall help clean FORCE
 
-.PHONY: all clean install
+.DEFAULT_GOAL = all
diff --git a/commands.h b/commands.h
deleted file mode 100644
index 9048601..0000000
--- a/commands.h
+++ /dev/null
@@ -1,24 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-#ifndef COMMANDS_H
-#define COMMANDS_H
-
-#include "util.h"
-
-#include <stdio.h>
-
-struct fsverity_command;
-
-void usage(const struct fsverity_command *cmd, FILE *fp);
-
-int fsverity_cmd_enable(const struct fsverity_command *cmd,
-			int argc, char *argv[]);
-int fsverity_cmd_measure(const struct fsverity_command *cmd,
-			 int argc, char *argv[]);
-int fsverity_cmd_sign(const struct fsverity_command *cmd,
-		      int argc, char *argv[]);
-
-bool parse_block_size_option(const char *arg, u32 *size_ptr);
-u32 get_default_block_size(void);
-bool parse_salt_option(const char *arg, u8 **salt_ptr, u32 *salt_size_ptr);
-
-#endif /* COMMANDS_H */
diff --git a/util.h b/common/common_defs.h
similarity index 58%
rename from util.h
rename to common/common_defs.h
index dfa10f2..e0625f1 100644
--- a/util.h
+++ b/common/common_defs.h
@@ -1,16 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 /*
- * Utility functions and macros for the 'fsverity' program
+ * Common definitions for libfsverity and the 'fsverity' program
  *
  * Copyright (C) 2018 Google LLC
  */
-#ifndef UTIL_H
-#define UTIL_H
+#ifndef COMMON_COMMON_DEFS_H
+#define COMMON_COMMON_DEFS_H
 
-#include <inttypes.h>
-#include <stdarg.h>
 #include <stdbool.h>
 #include <stddef.h>
+#include <stdint.h>
 
 typedef uint8_t u8;
 typedef uint16_t u16;
@@ -86,40 +85,4 @@ static inline int ilog2(unsigned long n)
 #  define le64_to_cpu(v)	(__builtin_bswap64((__force u64)(v)))
 #endif
 
-/* ========== Memory allocation ========== */
-
-void *xmalloc(size_t size);
-void *xzalloc(size_t size);
-void *xmemdup(const void *mem, size_t size);
-char *xstrdup(const char *s);
-
-/* ========== Error messages and assertions ========== */
-
-__cold void do_error_msg(const char *format, va_list va, int err);
-__printf(1, 2) __cold void error_msg(const char *format, ...);
-__printf(1, 2) __cold void error_msg_errno(const char *format, ...);
-__printf(1, 2) __cold __noreturn void fatal_error(const char *format, ...);
-__cold __noreturn void assertion_failed(const char *expr,
-					const char *file, int line);
-
-#define ASSERT(e) ({ if (!(e)) assertion_failed(#e, __FILE__, __LINE__); })
-
-/* ========== File utilities ========== */
-
-struct filedes {
-	int fd;
-	char *name;		/* filename, for logging or error messages */
-};
-
-bool open_file(struct filedes *file, const char *filename, int flags, int mode);
-bool get_file_size(struct filedes *file, u64 *size_ret);
-bool full_read(struct filedes *file, void *buf, size_t count);
-bool full_write(struct filedes *file, const void *buf, size_t count);
-bool filedes_close(struct filedes *file);
-
-/* ========== String utilities ========== */
-
-bool hex2bin(const char *hex, u8 *bin, size_t bin_len);
-void bin2hex(const u8 *bin, size_t bin_len, char *hex);
-
-#endif /* UTIL_H */
+#endif /* COMMON_COMMON_DEFS_H */
diff --git a/fsverity_uapi.h b/common/fsverity_uapi.h
similarity index 100%
rename from fsverity_uapi.h
rename to common/fsverity_uapi.h
diff --git a/common/libfsverity.h b/common/libfsverity.h
new file mode 100644
index 0000000..8462cc9
--- /dev/null
+++ b/common/libfsverity.h
@@ -0,0 +1,132 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * libfsverity API
+ *
+ * Copyright (C) 2018 Google LLC
+ * Copyright (C) 2020 Facebook
+ */
+
+#ifndef LIBFSVERITY_H
+#define LIBFSVERITY_H
+
+#include <errno.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;
+	uint64_t file_size;
+	const uint8_t *salt;
+	uint64_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;
+	uint64_t reserved[11];
+};
+
+/*
+ * libfsverity_read_fn_t - callback that incrementally provides a file's data
+ * @fd: the user-provided "file descriptor" (opaque to library)
+ * @buf: buffer into which to read the next chunk of the file's data
+ * @count: number of bytes to read in this chunk
+ *
+ * Must return 0 on success (all 'count' bytes read), or a negative errno value
+ * on failure.
+ */
+typedef int (*libfsverity_read_fn_t)(void *fd, void *buf, size_t count);
+
+/**
+ * libfsverity_compute_digest() - Compute digest of a file
+ *          An fsverity digest is the root of the Merkle tree of the file.
+ *          Not to be confused with a traditional file digests computed over
+ *          the entire file.
+ * @fd: context that will be passed to @read_fn
+ * @read_fn: a function that will read the data of the file
+ * @params: struct libfsverity_merkle_tree_params specifying hash algorithm,
+ *	    block size, version, and optional salt parameters.
+ *	    reserved parameters must be zero.
+ * @digest_ret: Pointer to pointer for computed digest.
+ *
+ * Returns:
+ * * 0 for success, -EINVAL for invalid input arguments, -ENOMEM if failed
+ *   to allocate memory, -EBADF if fd is invalid, and -EAGAIN if root hash
+ *   fails to compute.
+ * * digest_ret returns a pointer to the digest on success. The digest object
+ *   is allocated by libfsverity and must be freed by the caller.
+ */
+int
+libfsverity_compute_digest(void *fd, libfsverity_read_fn_t read_fn,
+			   const struct libfsverity_merkle_tree_params *params,
+			   struct libfsverity_digest **digest_ret);
+
+/**
+ * libfsverity_sign_digest() - Sign previously computed digest of a file
+ *          This is signature is used by the file system to validate the
+ *          signed file measurement against a public key loaded into the
+ *          .fs-verity kernel keyring, when CONFIG_FS_VERITY_BUILTIN_SIGNATURES
+ *          is enabled. The signature is formatted as PKCS#7 stored in DER
+ *          format. See Documentation/filesystems/fsverity.rst for further
+ *          details.
+ * @digest: pointer to previously computed digest
+ * @sig_params: struct libfsverity_signature_params providing filenames of
+ *          the keyfile and certificate file. Reserved parameters must be zero.
+ * @sig_ret: Pointer to pointer for signed digest
+ * @sig_size_ret: Pointer to size of signed return digest
+ *
+ * Return:
+ * * 0 for success, -EINVAL for invalid input arguments, -EAGAIN if key or
+ *   certificate files fail to read, or if signing the digest fails.
+ * * sig_ret returns a pointer to the signed digest on success. This object
+ *   is allocated by libfsverity_sign_digest and must be freed by the caller.
+ * * sig_size_ret returns the size of the signed digest on success.
+ */
+int
+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_find_hash_alg_by_name() - Find hash algorithm by name
+ * @name: Pointer to name of hash algorithm
+ *
+ * Return: The hash algorithm number, or zero if not found.
+ */
+uint32_t libfsverity_find_hash_alg_by_name(const char *name);
+
+/**
+ * libfsverity_digest_size() - Return size of digest for a given algorithm
+ * @alg_num: Number of hash algorithm
+ *
+ * Return: size of digest in bytes, or -1 if algorithm is unknown.
+ */
+int libfsverity_digest_size(uint32_t alg_num);
+
+/**
+ * libfsverity_hash_name() - Find name of hash algorithm by number
+ * @alg_num: Number of hash algorithm
+ *
+ * Return: The name of the hash algorithm, or NULL if algorithm is unknown.
+ */
+const char *libfsverity_hash_name(uint32_t alg_num);
+
+/**
+ * libfsverity_set_error_callback() - Set callback to handle error messages
+ * @cb: the callback function
+ */
+void libfsverity_set_error_callback(void (*cb)(const char *msg));
+
+#endif /* LIBFSVERITY_H */
diff --git a/hash_algs.h b/hash_algs.h
deleted file mode 100644
index 913c918..0000000
--- a/hash_algs.h
+++ /dev/null
@@ -1,68 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-#ifndef HASH_ALGS_H
-#define HASH_ALGS_H
-
-#include "util.h"
-
-#include <stdio.h>
-
-struct fsverity_hash_alg {
-	const char *name;
-	unsigned int digest_size;
-	unsigned int block_size;
-	struct hash_ctx *(*create_ctx)(const struct fsverity_hash_alg *alg);
-};
-
-extern const struct fsverity_hash_alg fsverity_hash_algs[];
-
-struct hash_ctx {
-	const struct fsverity_hash_alg *alg;
-	void (*init)(struct hash_ctx *ctx);
-	void (*update)(struct hash_ctx *ctx, const void *data, size_t size);
-	void (*final)(struct hash_ctx *ctx, u8 *out);
-	void (*free)(struct hash_ctx *ctx);
-};
-
-const struct fsverity_hash_alg *find_hash_alg_by_name(const char *name);
-const struct fsverity_hash_alg *find_hash_alg_by_num(unsigned int num);
-void show_all_hash_algs(FILE *fp);
-
-/* The hash algorithm that fsverity-utils 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
-
-static inline struct hash_ctx *hash_create(const struct fsverity_hash_alg *alg)
-{
-	return alg->create_ctx(alg);
-}
-
-static inline void hash_init(struct hash_ctx *ctx)
-{
-	ctx->init(ctx);
-}
-
-static inline void hash_update(struct hash_ctx *ctx,
-			       const void *data, size_t size)
-{
-	ctx->update(ctx, data, size);
-}
-
-static inline void hash_final(struct hash_ctx *ctx, u8 *digest)
-{
-	ctx->final(ctx, digest);
-}
-
-static inline void hash_free(struct hash_ctx *ctx)
-{
-	if (ctx)
-		ctx->free(ctx);
-}
-
-void hash_full(struct hash_ctx *ctx, const void *data, size_t size, u8 *digest);
-
-#endif /* HASH_ALGS_H */
diff --git a/lib/compute_digest.c b/lib/compute_digest.c
index b279d63..13998bb 100644
--- a/lib/compute_digest.c
+++ b/lib/compute_digest.c
@@ -1,13 +1,13 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * compute_digest.c
+ * Implementation of libfsverity_compute_digest().
  *
  * Copyright (C) 2018 Google LLC
+ * Copyright (C) 2020 Facebook
  */
 
-#include "sign.h"
+#include "lib_private.h"
 
-#include <fcntl.h>
 #include <stdlib.h>
 #include <string.h>
 
@@ -47,10 +47,10 @@ static bool hash_one_block(struct hash_ctx *hash, struct block_buffer *cur,
 	/* Zero-pad the block if it's shorter than block_size. */
 	memset(&cur->data[cur->filled], 0, block_size - cur->filled);
 
-	hash_init(hash);
-	hash_update(hash, salt, salt_size);
-	hash_update(hash, cur->data, block_size);
-	hash_final(hash, &next->data[next->filled]);
+	libfsverity_hash_init(hash);
+	libfsverity_hash_update(hash, salt, salt_size);
+	libfsverity_hash_update(hash, cur->data, block_size);
+	libfsverity_hash_final(hash, &next->data[next->filled]);
 
 	next->filled += hash->alg->digest_size;
 	cur->filled = 0;
@@ -62,28 +62,42 @@ static bool hash_one_block(struct hash_ctx *hash, struct block_buffer *cur,
  * Compute the file's Merkle tree root hash using the given hash algorithm,
  * block size, and salt.
  */
-static bool compute_root_hash(struct filedes *file, u64 file_size,
-			      struct hash_ctx *hash, u32 block_size,
-			      const u8 *salt, u32 salt_size, u8 *root_hash)
+static int compute_root_hash(void *fd, libfsverity_read_fn_t read_fn,
+			     u64 file_size, struct hash_ctx *hash,
+			     u32 block_size, const u8 *salt, u32 salt_size,
+			     u8 *root_hash)
 {
 	const u32 hashes_per_block = block_size / hash->alg->digest_size;
 	const u32 padded_salt_size = roundup(salt_size, hash->alg->block_size);
-	u8 *padded_salt = xzalloc(padded_salt_size);
+	u8 *padded_salt = NULL;
 	u64 blocks;
 	int num_levels = 0;
 	int level;
 	struct block_buffer _buffers[1 + FS_VERITY_MAX_LEVELS + 1] = {};
 	struct block_buffer *buffers = &_buffers[1];
 	u64 offset;
-	bool ok = false;
+	int err = 0;
 
-	if (salt_size != 0)
+	/* Root hash of empty file is all 0's */
+	if (file_size == 0) {
+		memset(root_hash, 0, hash->alg->digest_size);
+		return 0;
+	}
+
+	if (salt_size != 0) {
+		padded_salt = libfsverity_zalloc(padded_salt_size);
+		if (!padded_salt)
+			return -ENOMEM;
 		memcpy(padded_salt, salt, salt_size);
+	}
 
 	/* Compute number of levels */
 	for (blocks = DIV_ROUND_UP(file_size, block_size); blocks > 1;
 	     blocks = DIV_ROUND_UP(blocks, hashes_per_block)) {
-		ASSERT(num_levels < FS_VERITY_MAX_LEVELS);
+		if (WARN_ON(num_levels >= FS_VERITY_MAX_LEVELS)) {
+			err = -EINVAL;
+			goto out;
+		}
 		num_levels++;
 	}
 
@@ -92,22 +106,33 @@ static bool compute_root_hash(struct filedes *file, u64 file_size,
 	 * Buffers 0 <= level < num_levels are for the actual tree levels.
 	 * Buffer 'num_levels' is for the root hash.
 	 */
-	for (level = -1; level < num_levels; level++)
-		buffers[level].data = xmalloc(block_size);
+	for (level = -1; level < num_levels; level++) {
+		buffers[level].data = libfsverity_zalloc(block_size);
+		if (!buffers[level].data) {
+			err = -ENOMEM;
+			goto out;
+		}
+	}
 	buffers[num_levels].data = root_hash;
 
 	/* Hash each data block, also hashing the tree blocks as they fill up */
 	for (offset = 0; offset < file_size; offset += block_size) {
 		buffers[-1].filled = min(block_size, file_size - offset);
 
-		if (!full_read(file, buffers[-1].data, buffers[-1].filled))
+		err = read_fn(fd, buffers[-1].data, buffers[-1].filled);
+		if (err) {
+			libfsverity_error_msg("error reading file");
 			goto out;
+		}
 
 		level = -1;
 		while (hash_one_block(hash, &buffers[level], block_size,
 				      padded_salt, padded_salt_size)) {
 			level++;
-			ASSERT(level < num_levels);
+			if (WARN_ON(level >= num_levels)) {
+				err = -EINVAL;
+				goto out;
+			}
 		}
 	}
 	/* Finish all nonempty pending tree blocks */
@@ -118,67 +143,101 @@ static bool compute_root_hash(struct filedes *file, u64 file_size,
 	}
 
 	/* Root hash was filled by the last call to hash_one_block() */
-	ASSERT(buffers[num_levels].filled == hash->alg->digest_size);
-	ok = true;
+	if (WARN_ON(buffers[num_levels].filled != hash->alg->digest_size)) {
+		err = -EINVAL;
+		goto out;
+	}
+	err = 0;
 out:
 	for (level = -1; level < num_levels; level++)
 		free(buffers[level].data);
 	free(padded_salt);
-	return ok;
+	return err;
 }
 
-/*
- * Compute the fs-verity measurement of the given file.
- *
- * The fs-verity measurement is the hash of the fsverity_descriptor, which
- * contains the Merkle tree properties including the root hash.
- */
-bool compute_file_measurement(const char *filename,
-			      const struct fsverity_hash_alg *hash_alg,
-			      u32 block_size, const u8 *salt,
-			      u32 salt_size, u8 *measurement)
+LIBEXPORT int
+libfsverity_compute_digest(void *fd, libfsverity_read_fn_t read_fn,
+			   const struct libfsverity_merkle_tree_params *params,
+			   struct libfsverity_digest **digest_ret)
 {
-	struct filedes file = { .fd = -1 };
-	struct hash_ctx *hash = hash_create(hash_alg);
-	u64 file_size;
+	const struct fsverity_hash_alg *hash_alg;
+	struct hash_ctx *hash = NULL;
+	struct libfsverity_digest *digest = NULL;
 	struct fsverity_descriptor desc;
-	bool ok = false;
+	int i;
+	int err;
 
-	if (!open_file(&file, filename, O_RDONLY, 0))
+	if (!read_fn || !params || !digest_ret) {
+		libfsverity_error_msg("missing required parameters for compute_digest");
+		return -EINVAL;
+	}
+	if (params->version != 1) {
+		libfsverity_error_msg("unsupported version (%u)",
+				      params->version);
+		return -EINVAL;
+	}
+	if (!is_power_of_2(params->block_size)) {
+		libfsverity_error_msg("unsupported block size (%u)",
+				      params->block_size);
+		return -EINVAL;
+	}
+	if (params->salt_size > sizeof(desc.salt)) {
+		libfsverity_error_msg("unsupported salt size (%u)",
+				      params->salt_size);
+		return -EINVAL;
+	}
+	if (params->salt_size && !params->salt)  {
+		libfsverity_error_msg("salt_size specified, but salt is NULL");
+		return -EINVAL;
+	}
+	for (i = 0; i < ARRAY_SIZE(params->reserved); i++) {
+		if (params->reserved[i]) {
+			libfsverity_error_msg("reserved bits set in merkle_tree_params");
+			return -EINVAL;
+		}
+	}
+
+	hash_alg = libfsverity_find_hash_alg_by_num(params->hash_algorithm);
+	if (!hash_alg) {
+		libfsverity_error_msg("unknown hash algorithm: %u",
+				      params->hash_algorithm);
+		return -EINVAL;
+	}
+
+	err = -ENOMEM;
+	hash = hash_alg->create_ctx(hash_alg);
+	if (!hash)
 		goto out;
 
-	if (!get_file_size(&file, &file_size))
+	err = -ENOMEM;
+	digest = libfsverity_zalloc(sizeof(*digest) + hash_alg->digest_size);
+	if (!digest)
 		goto out;
+	digest->digest_algorithm = hash_alg - libfsverity_hash_algs;
+	digest->digest_size = hash_alg->digest_size;
 
 	memset(&desc, 0, sizeof(desc));
 	desc.version = 1;
-	desc.hash_algorithm = hash_alg - fsverity_hash_algs;
-
-	ASSERT(is_power_of_2(block_size));
-	desc.log_blocksize = ilog2(block_size);
-
-	if (salt_size != 0) {
-		if (salt_size > sizeof(desc.salt)) {
-			error_msg("Salt too long (got %u bytes; max is %zu bytes)",
-				  salt_size, sizeof(desc.salt));
-			goto out;
-		}
-		memcpy(desc.salt, salt, salt_size);
-		desc.salt_size = salt_size;
+	desc.hash_algorithm = params->hash_algorithm;
+	desc.log_blocksize = ilog2(params->block_size);
+	desc.data_size = cpu_to_le64(params->file_size);
+	if (params->salt_size != 0) {
+		memcpy(desc.salt, params->salt, params->salt_size);
+		desc.salt_size = params->salt_size;
 	}
 
-	desc.data_size = cpu_to_le64(file_size);
-
-	/* Root hash of empty file is all 0's */
-	if (file_size != 0 &&
-	    !compute_root_hash(&file, file_size, hash, block_size, salt,
-			       salt_size, desc.root_hash))
+	err = compute_root_hash(fd, read_fn, params->file_size, hash,
+				params->block_size, params->salt,
+				params->salt_size, desc.root_hash);
+	if (err)
 		goto out;
 
-	hash_full(hash, &desc, sizeof(desc), measurement);
-	ok = true;
+	libfsverity_hash_full(hash, &desc, sizeof(desc), digest->digest);
 out:
-	filedes_close(&file);
-	hash_free(hash);
-	return ok;
+	libfsverity_free_hash_ctx(hash);
+	if (err)
+		free(digest);
+	else
+		*digest_ret = digest;
+	return err;
 }
diff --git a/hash_algs.c b/lib/hash_algs.c
similarity index 54%
rename from hash_algs.c
rename to lib/hash_algs.c
index bb153de..0e50259 100644
--- a/hash_algs.c
+++ b/lib/hash_algs.c
@@ -7,8 +7,7 @@
  * Written by Eric Biggers.
  */
 
-#include "fsverity_uapi.h"
-#include "hash_algs.h"
+#include "lib_private.h"
 
 #include <openssl/evp.h>
 #include <stdlib.h>
@@ -25,29 +24,29 @@ struct openssl_hash_ctx {
 static void openssl_digest_init(struct hash_ctx *_ctx)
 {
 	struct openssl_hash_ctx *ctx = (void *)_ctx;
+	int ret;
 
-	if (EVP_DigestInit_ex(ctx->md_ctx, ctx->md, NULL) != 1)
-		fatal_error("EVP_DigestInit_ex() failed for algorithm '%s'",
-			    ctx->base.alg->name);
+	ret = EVP_DigestInit_ex(ctx->md_ctx, ctx->md, NULL);
+	BUG_ON(ret != 1);
 }
 
 static void openssl_digest_update(struct hash_ctx *_ctx,
 				  const void *data, size_t size)
 {
 	struct openssl_hash_ctx *ctx = (void *)_ctx;
+	int ret;
 
-	if (EVP_DigestUpdate(ctx->md_ctx, data, size) != 1)
-		fatal_error("EVP_DigestUpdate() failed for algorithm '%s'",
-			    ctx->base.alg->name);
+	ret = EVP_DigestUpdate(ctx->md_ctx, data, size);
+	BUG_ON(ret != 1);
 }
 
 static void openssl_digest_final(struct hash_ctx *_ctx, u8 *digest)
 {
 	struct openssl_hash_ctx *ctx = (void *)_ctx;
+	int ret;
 
-	if (EVP_DigestFinal_ex(ctx->md_ctx, digest, NULL) != 1)
-		fatal_error("EVP_DigestFinal_ex() failed for algorithm '%s'",
-			    ctx->base.alg->name);
+	ret = EVP_DigestFinal_ex(ctx->md_ctx, digest, NULL);
+	BUG_ON(ret != 1);
 }
 
 static void openssl_digest_ctx_free(struct hash_ctx *_ctx)
@@ -68,7 +67,10 @@ openssl_digest_ctx_create(const struct fsverity_hash_alg *alg, const EVP_MD *md)
 {
 	struct openssl_hash_ctx *ctx;
 
-	ctx = xzalloc(sizeof(*ctx));
+	ctx = libfsverity_zalloc(sizeof(*ctx));
+	if (!ctx)
+		return NULL;
+
 	ctx->base.alg = alg;
 	ctx->base.init = openssl_digest_init;
 	ctx->base.update = openssl_digest_update;
@@ -80,13 +82,22 @@ openssl_digest_ctx_create(const struct fsverity_hash_alg *alg, const EVP_MD *md)
 	 * with older OpenSSL versions.
 	 */
 	ctx->md_ctx = EVP_MD_CTX_create();
-	if (!ctx->md_ctx)
-		fatal_error("out of memory");
+	if (!ctx->md_ctx) {
+		libfsverity_error_msg("failed to allocate EVP_MD_CTX");
+		goto err1;
+	}
 
 	ctx->md = md;
-	ASSERT(EVP_MD_size(md) == alg->digest_size);
+	if (WARN_ON(EVP_MD_size(md) != alg->digest_size))
+		goto err2;
 
 	return &ctx->base;
+
+err2:
+	EVP_MD_CTX_destroy(ctx->md_ctx);
+err1:
+	free(ctx);
+	return NULL;
 }
 
 static struct hash_ctx *create_sha256_ctx(const struct fsverity_hash_alg *alg)
@@ -99,9 +110,42 @@ static struct hash_ctx *create_sha512_ctx(const struct fsverity_hash_alg *alg)
 	return openssl_digest_ctx_create(alg, EVP_sha512());
 }
 
+/* ========== Hash utilities ========== */
+
+void libfsverity_hash_init(struct hash_ctx *ctx)
+{
+	ctx->init(ctx);
+}
+
+void libfsverity_hash_update(struct hash_ctx *ctx, const void *data,
+			     size_t size)
+{
+	ctx->update(ctx, data, size);
+}
+
+void libfsverity_hash_final(struct hash_ctx *ctx, u8 *digest)
+{
+	ctx->final(ctx, digest);
+}
+
+/* ->init(), ->update(), and ->final() all in one step */
+void libfsverity_hash_full(struct hash_ctx *ctx, const void *data, size_t size,
+			   u8 *digest)
+{
+	libfsverity_hash_init(ctx);
+	libfsverity_hash_update(ctx, data, size);
+	libfsverity_hash_final(ctx, digest);
+}
+
+void libfsverity_free_hash_ctx(struct hash_ctx *ctx)
+{
+	if (ctx)
+		ctx->free(ctx);
+}
+
 /* ========== Hash algorithm definitions ========== */
 
-const struct fsverity_hash_alg fsverity_hash_algs[] = {
+const struct fsverity_hash_alg libfsverity_hash_algs[] = {
 	[FS_VERITY_HASH_ALG_SHA256] = {
 		.name = "sha256",
 		.digest_size = 32,
@@ -116,48 +160,42 @@ const struct fsverity_hash_alg fsverity_hash_algs[] = {
 	},
 };
 
-const struct fsverity_hash_alg *find_hash_alg_by_name(const char *name)
+LIBEXPORT u32
+libfsverity_find_hash_alg_by_name(const char *name)
 {
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(fsverity_hash_algs); i++) {
-		if (fsverity_hash_algs[i].name &&
-		    !strcmp(name, fsverity_hash_algs[i].name))
-			return &fsverity_hash_algs[i];
+	for (i = 0; i < ARRAY_SIZE(libfsverity_hash_algs); i++) {
+		if (libfsverity_hash_algs[i].name &&
+		    !strcmp(name, libfsverity_hash_algs[i].name))
+			return i;
 	}
-	error_msg("unknown hash algorithm: '%s'", name);
-	fputs("Available hash algorithms: ", stderr);
-	show_all_hash_algs(stderr);
-	putc('\n', stderr);
-	return NULL;
+	return 0;
 }
 
-const struct fsverity_hash_alg *find_hash_alg_by_num(unsigned int num)
+const struct fsverity_hash_alg *libfsverity_find_hash_alg_by_num(u32 alg_num)
 {
-	if (num < ARRAY_SIZE(fsverity_hash_algs) &&
-	    fsverity_hash_algs[num].name)
-		return &fsverity_hash_algs[num];
+	if (alg_num < ARRAY_SIZE(libfsverity_hash_algs) &&
+	    libfsverity_hash_algs[alg_num].name)
+		return &libfsverity_hash_algs[alg_num];
 
 	return NULL;
 }
 
-void show_all_hash_algs(FILE *fp)
+LIBEXPORT int
+libfsverity_digest_size(u32 alg_num)
 {
-	int i;
-	const char *sep = "";
+	const struct fsverity_hash_alg *alg =
+		libfsverity_find_hash_alg_by_num(alg_num);
 
-	for (i = 0; i < ARRAY_SIZE(fsverity_hash_algs); i++) {
-		if (fsverity_hash_algs[i].name) {
-			fprintf(fp, "%s%s", sep, fsverity_hash_algs[i].name);
-			sep = ", ";
-		}
-	}
+	return alg ? alg->digest_size : -1;
 }
 
-/* ->init(), ->update(), and ->final() all in one step */
-void hash_full(struct hash_ctx *ctx, const void *data, size_t size, u8 *digest)
+LIBEXPORT const char *
+libfsverity_hash_name(u32 alg_num)
 {
-	hash_init(ctx);
-	hash_update(ctx, data, size);
-	hash_final(ctx, digest);
+	const struct fsverity_hash_alg *alg =
+		libfsverity_find_hash_alg_by_num(alg_num);
+
+	return alg ? alg->name : NULL;
 }
diff --git a/lib/lib_private.h b/lib/lib_private.h
new file mode 100644
index 0000000..50d5f09
--- /dev/null
+++ b/lib/lib_private.h
@@ -0,0 +1,83 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Private header for libfsverity
+ */
+#ifndef LIB_LIB_PRIVATE_H
+#define LIB_LIB_PRIVATE_H
+
+#include "../common/libfsverity.h"
+#include "../common/common_defs.h"
+#include "../common/fsverity_uapi.h"
+
+#include <stdarg.h>
+
+#define LIBEXPORT	__attribute__((visibility("default")))
+
+/* hash_algs.c */
+
+struct fsverity_hash_alg {
+	const char *name;
+	unsigned int digest_size;
+	unsigned int block_size;
+	struct hash_ctx *(*create_ctx)(const struct fsverity_hash_alg *alg);
+};
+
+extern const struct fsverity_hash_alg libfsverity_hash_algs[];
+
+const struct fsverity_hash_alg *libfsverity_find_hash_alg_by_num(u32 alg_num);
+
+struct hash_ctx {
+	const struct fsverity_hash_alg *alg;
+	void (*init)(struct hash_ctx *ctx);
+	void (*update)(struct hash_ctx *ctx, const void *data, size_t size);
+	void (*final)(struct hash_ctx *ctx, u8 *out);
+	void (*free)(struct hash_ctx *ctx);
+};
+
+void libfsverity_hash_init(struct hash_ctx *ctx);
+void libfsverity_hash_update(struct hash_ctx *ctx, const void *data,
+			     size_t size);
+void libfsverity_hash_final(struct hash_ctx *ctx, u8 *digest);
+void libfsverity_hash_full(struct hash_ctx *ctx, const void *data, size_t size,
+			   u8 *digest);
+void libfsverity_free_hash_ctx(struct hash_ctx *ctx);
+
+/* utils.c */
+
+void *libfsverity_zalloc(size_t size);
+void *libfsverity_memdup(const void *mem, size_t size);
+
+__cold void
+libfsverity_do_error_msg(const char *format, va_list va, int err);
+
+__printf(1, 2) __cold void
+libfsverity_error_msg(const char *format, ...);
+
+__printf(1, 2) __cold void
+libfsverity_error_msg_errno(const char *format, ...);
+
+__cold void
+libfsverity_warn_on(const char *condition, const char *file, int line);
+
+#define WARN_ON(condition)						\
+({									\
+	bool c = (condition);						\
+									\
+	if (c)								\
+		libfsverity_warn_on(#condition, __FILE__, __LINE__);	\
+	c;								\
+})
+
+__cold void
+libfsverity_bug_on(const char *condition, const char *file, int line);
+
+#define BUG_ON(condition)						\
+({									\
+	bool c = (condition);						\
+									\
+	if (c)								\
+		libfsverity_bug_on(#condition, __FILE__, __LINE__);	\
+	c;								\
+})
+
+#endif /* LIB_LIB_PRIVATE_H */
diff --git a/lib/sign_digest.c b/lib/sign_digest.c
index d2b0d53..d856392 100644
--- a/lib/sign_digest.c
+++ b/lib/sign_digest.c
@@ -1,45 +1,68 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
- * sign_digest.c
+ * Implementation of libfsverity_sign_digest().
  *
  * Copyright (C) 2018 Google LLC
+ * Copyright (C) 2020 Facebook
  */
 
-#include "hash_algs.h"
-#include "sign.h"
+#include "lib_private.h"
 
 #include <limits.h>
 #include <openssl/bio.h>
 #include <openssl/err.h>
 #include <openssl/pem.h>
 #include <openssl/pkcs7.h>
+#include <string.h>
+
+/*
+ * Format in which verity file measurements are signed.  This is the same as
+ * 'struct fsverity_digest', except here some magic bytes are prepended to
+ * provide some context about what is being signed in case the same key is used
+ * for non-fsverity purposes, and here the fields have fixed endianness.
+ */
+struct fsverity_signed_digest {
+	char magic[8];			/* must be "FSVerity" */
+	__le16 digest_algorithm;
+	__le16 digest_size;
+	__u8 digest[];
+};
+
+static int print_openssl_err_cb(const char *str, size_t len, void *u)
+{
+	libfsverity_error_msg("%s", str);
+	return 1;
+}
 
 static void __printf(1, 2) __cold
 error_msg_openssl(const char *format, ...)
 {
+	int saved_errno = errno;
 	va_list va;
 
 	va_start(va, format);
-	do_error_msg(format, va, 0);
+	libfsverity_do_error_msg(format, va, 0);
 	va_end(va);
 
 	if (ERR_peek_error() == 0)
 		return;
 
-	fprintf(stderr, "OpenSSL library errors:\n");
-	ERR_print_errors_fp(stderr);
+	libfsverity_error_msg("OpenSSL library errors:");
+	ERR_print_errors_cb(print_openssl_err_cb, NULL);
+	errno = saved_errno;
 }
 
 /* Read a PEM PKCS#8 formatted private key */
-static EVP_PKEY *read_private_key(const char *keyfile)
+static int read_private_key(const char *keyfile, EVP_PKEY **pkey_ret)
 {
 	BIO *bio;
 	EVP_PKEY *pkey;
+	int err;
 
 	bio = BIO_new_file(keyfile, "r");
 	if (!bio) {
 		error_msg_openssl("can't open '%s' for reading", keyfile);
-		return NULL;
+		return -errno;
 	}
 
 	pkey = PEM_read_bio_PrivateKey(bio, NULL, NULL, NULL);
@@ -47,38 +70,50 @@ static EVP_PKEY *read_private_key(const char *keyfile)
 		error_msg_openssl("Failed to parse private key file '%s'.\n"
 				  "       Note: it must be in PEM PKCS#8 format.",
 				  keyfile);
+		err = -EINVAL;
+		goto out;
 	}
+	*pkey_ret = pkey;
+	err = 0;
+out:
 	BIO_free(bio);
-	return pkey;
+	return err;
 }
 
 /* Read a PEM X.509 formatted certificate */
-static X509 *read_certificate(const char *certfile)
+static int read_certificate(const char *certfile, X509 **cert_ret)
 {
 	BIO *bio;
 	X509 *cert;
+	int err;
 
 	bio = BIO_new_file(certfile, "r");
 	if (!bio) {
 		error_msg_openssl("can't open '%s' for reading", certfile);
-		return NULL;
+		return -errno;
 	}
 	cert = PEM_read_bio_X509(bio, NULL, NULL, NULL);
 	if (!cert) {
 		error_msg_openssl("Failed to parse X.509 certificate file '%s'.\n"
 				  "       Note: it must be in PEM format.",
 				  certfile);
+		err = -EINVAL;
+		goto out;
 	}
+	*cert_ret = cert;
+	err = 0;
+out:
 	BIO_free(bio);
-	return cert;
+	return err;
 }
 
 #ifdef OPENSSL_IS_BORINGSSL
 
-static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
-		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
-		       u8 **sig_ret, u32 *sig_size_ret)
+static int sign_pkcs7(const void *data_to_sign, size_t data_size,
+		      EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
+		      u8 **sig_ret, size_t *sig_size_ret)
 {
+	BIGNUM *serial;
 	CBB out, outer_seq, wrapped_seq, seq, digest_algos_set, digest_algo,
 		null, content_info, issuer_and_serial, signer_infos,
 		signer_info, sign_algo, signature;
@@ -86,31 +121,39 @@ static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
 	u8 *name_der = NULL, *sig = NULL, *pkcs7_data = NULL;
 	size_t pkcs7_data_len, sig_len;
 	int name_der_len, sig_nid;
-	bool ok = false;
+	int err;
 
 	EVP_MD_CTX_init(&md_ctx);
-	BIGNUM *serial = ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), NULL);
+	serial = ASN1_INTEGER_to_BN(X509_get_serialNumber(cert), NULL);
 
 	if (!CBB_init(&out, 1024)) {
-		error_msg("out of memory");
+		error_msg_openssl("out of memory");
+		err = -ENOMEM;
 		goto out;
 	}
 
 	name_der_len = i2d_X509_NAME(X509_get_subject_name(cert), &name_der);
 	if (name_der_len < 0) {
 		error_msg_openssl("i2d_X509_NAME failed");
+		err = -EINVAL;
 		goto out;
 	}
 
 	if (!EVP_DigestSignInit(&md_ctx, NULL, md, NULL, pkey)) {
 		error_msg_openssl("EVP_DigestSignInit failed");
+		err = -EINVAL;
 		goto out;
 	}
 
 	sig_len = EVP_PKEY_size(pkey);
-	sig = xmalloc(sig_len);
+	sig = libfsverity_zalloc(sig_len);
+	if (!sig) {
+		err = -ENOMEM;
+		goto out;
+	}
 	if (!EVP_DigestSign(&md_ctx, sig, &sig_len, data_to_sign, data_size)) {
 		error_msg_openssl("EVP_DigestSign failed");
+		err = -EINVAL;
 		goto out;
 	}
 
@@ -153,12 +196,17 @@ static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
 	    !CBB_add_bytes(&signature, sig, sig_len) ||
 	    !CBB_finish(&out, &pkcs7_data, &pkcs7_data_len)) {
 		error_msg_openssl("failed to construct PKCS#7 data");
+		err = -EINVAL;
 		goto out;
 	}
 
-	*sig_ret = xmemdup(pkcs7_data, pkcs7_data_len);
+	*sig_ret = libfsverity_memdup(pkcs7_data, pkcs7_data_len);
+	if (!*sig_ret) {
+		err = -ENOMEM;
+		goto out;
+	}
 	*sig_size_ret = pkcs7_data_len;
-	ok = true;
+	err = 0;
 out:
 	BN_free(serial);
 	EVP_MD_CTX_cleanup(&md_ctx);
@@ -166,7 +214,7 @@ out:
 	free(sig);
 	OPENSSL_free(name_der);
 	OPENSSL_free(pkcs7_data);
-	return ok;
+	return err;
 }
 
 #else /* OPENSSL_IS_BORINGSSL */
@@ -175,7 +223,9 @@ static BIO *new_mem_buf(const void *buf, size_t size)
 {
 	BIO *bio;
 
-	ASSERT(size <= INT_MAX);
+	if (WARN_ON(size > INT_MAX))
+		return NULL;
+
 	/*
 	 * Prior to OpenSSL 1.1.0, BIO_new_mem_buf() took a non-const pointer,
 	 * despite still marking the resulting bio as read-only.  So cast away
@@ -187,9 +237,9 @@ static BIO *new_mem_buf(const void *buf, size_t size)
 	return bio;
 }
 
-static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
-		       EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
-		       u8 **sig_ret, u32 *sig_size_ret)
+static int sign_pkcs7(const void *data_to_sign, size_t data_size,
+		      EVP_PKEY *pkey, X509 *cert, const EVP_MD *md,
+		      u8 **sig_ret, size_t *sig_size_ret)
 {
 	/*
 	 * PKCS#7 signing flags:
@@ -215,25 +265,30 @@ static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
 	u32 sig_size;
 	BIO *bio = NULL;
 	PKCS7 *p7 = NULL;
-	bool ok = false;
+	int err;
 
 	bio = new_mem_buf(data_to_sign, data_size);
-	if (!bio)
+	if (!bio) {
+		err = -ENOMEM;
 		goto out;
+	}
 
 	p7 = PKCS7_sign(NULL, NULL, NULL, bio, pkcs7_flags);
 	if (!p7) {
 		error_msg_openssl("failed to initialize PKCS#7 signature object");
+		err = -EINVAL;
 		goto out;
 	}
 
 	if (!PKCS7_sign_add_signer(p7, cert, pkey, md, pkcs7_flags)) {
 		error_msg_openssl("failed to add signer to PKCS#7 signature object");
+		err = -EINVAL;
 		goto out;
 	}
 
 	if (PKCS7_final(p7, bio, pkcs7_flags) != 1) {
 		error_msg_openssl("failed to finalize PKCS#7 signature");
+		err = -EINVAL;
 		goto out;
 	}
 
@@ -241,64 +296,100 @@ static bool sign_pkcs7(const void *data_to_sign, size_t data_size,
 	bio = BIO_new(BIO_s_mem());
 	if (!bio) {
 		error_msg_openssl("out of memory");
+		err = -ENOMEM;
 		goto out;
 	}
 
 	if (i2d_PKCS7_bio(bio, p7) != 1) {
 		error_msg_openssl("failed to DER-encode PKCS#7 signature object");
+		err = -EINVAL;
 		goto out;
 	}
 
 	sig_size = BIO_get_mem_data(bio, &sig);
-	*sig_ret = xmemdup(sig, sig_size);
+	*sig_ret = libfsverity_memdup(sig, sig_size);
+	if (!*sig_ret) {
+		err = -ENOMEM;
+		goto out;
+	}
 	*sig_size_ret = sig_size;
-	ok = true;
+	err = 0;
 out:
 	PKCS7_free(p7);
 	BIO_free(bio);
-	return ok;
+	return err;
 }
 
 #endif /* !OPENSSL_IS_BORINGSSL */
 
-/*
- * Sign the specified @data_to_sign of length @data_size bytes using the private
- * key in @keyfile, the certificate in @certfile, and the hash algorithm
- * @hash_alg.  Returns the DER-formatted PKCS#7 signature in @sig_ret and
- * @sig_size_ret.
- */
-bool sign_data(const void *data_to_sign, size_t data_size,
-	       const char *keyfile, const char *certfile,
-	       const struct fsverity_hash_alg *hash_alg,
-	       u8 **sig_ret, u32 *sig_size_ret)
+LIBEXPORT int
+libfsverity_sign_digest(const struct libfsverity_digest *digest,
+			const struct libfsverity_signature_params *sig_params,
+			u8 **sig_ret, size_t *sig_size_ret)
 {
+	int i;
+	const struct fsverity_hash_alg *hash_alg;
 	EVP_PKEY *pkey = NULL;
 	X509 *cert = NULL;
 	const EVP_MD *md;
-	bool ok = false;
+	struct fsverity_signed_digest *d = NULL;
+	int err;
+
+	if (!digest || !sig_params || !sig_ret || !sig_size_ret)  {
+		libfsverity_error_msg("missing required parameters for sign_digest");
+		return -EINVAL;
+	}
+
+	if (!sig_params->keyfile || !sig_params->certfile) {
+		libfsverity_error_msg("keyfile and certfile must be specified");
+		return -EINVAL;
+	}
+
+	for (i = 0; i < ARRAY_SIZE(sig_params->reserved); i++) {
+		if (sig_params->reserved[i]) {
+			libfsverity_error_msg("reserved bits set in signature_params");
+			return -EINVAL;
+		}
+	}
+
+	hash_alg = libfsverity_find_hash_alg_by_num(digest->digest_algorithm);
+	if (!hash_alg || digest->digest_size != hash_alg->digest_size) {
+		libfsverity_error_msg("malformed fsverity digest");
+		return -EINVAL;
+	}
 
-	pkey = read_private_key(keyfile);
-	if (!pkey)
+	err = read_private_key(sig_params->keyfile, &pkey);
+	if (err)
 		goto out;
 
-	cert = read_certificate(certfile);
-	if (!cert)
+	err = read_certificate(sig_params->certfile, &cert);
+	if (err)
 		goto out;
 
 	OpenSSL_add_all_digests();
 	md = EVP_get_digestbyname(hash_alg->name);
 	if (!md) {
-		fprintf(stderr,
-			"Warning: '%s' algorithm not found in OpenSSL library.\n"
-			"         Falling back to SHA-256 signature.\n",
-			hash_alg->name);
-		md = EVP_sha256();
+		libfsverity_error_msg("'%s' algorithm not found in OpenSSL library",
+				      hash_alg->name);
+		err = -ENOPKG;
+		goto out;
 	}
 
-	ok = sign_pkcs7(data_to_sign, data_size, pkey, cert, md,
-			sig_ret, sig_size_ret);
-out:
+	d = libfsverity_zalloc(sizeof(*d) + digest->digest_size);
+	if (!d) {
+		err = -ENOMEM;
+		goto out;
+	}
+	memcpy(d->magic, "FSVerity", 8);
+	d->digest_algorithm = cpu_to_le16(hash_alg - libfsverity_hash_algs);
+	d->digest_size = cpu_to_le16(digest->digest_size);
+	memcpy(d->digest, digest->digest, digest->digest_size);
+
+	err = sign_pkcs7(d, sizeof(*d) + digest->digest_size,
+			 pkey, cert, md, sig_ret, sig_size_ret);
+ out:
 	EVP_PKEY_free(pkey);
 	X509_free(cert);
-	return ok;
+	free(d);
+	return err;
 }
diff --git a/lib/utils.c b/lib/utils.c
new file mode 100644
index 0000000..0684526
--- /dev/null
+++ b/lib/utils.c
@@ -0,0 +1,107 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Utility functions for libfsverity
+ *
+ * Copyright 2020 Google LLC
+ */
+
+#define _GNU_SOURCE /* for asprintf() */
+
+#include "lib_private.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+static void *xmalloc(size_t size)
+{
+	void *p = malloc(size);
+
+	if (!p)
+		libfsverity_error_msg("out of memory");
+	return p;
+}
+
+void *libfsverity_zalloc(size_t size)
+{
+	void *p = xmalloc(size);
+
+	if (!p)
+		return NULL;
+	return memset(p, 0, size);
+}
+
+void *libfsverity_memdup(const void *mem, size_t size)
+{
+	void *p = xmalloc(size);
+
+	if (!p)
+		return NULL;
+	return memcpy(p, mem, size);
+}
+
+static void (*libfsverity_error_cb)(const char *msg);
+
+LIBEXPORT void libfsverity_set_error_callback(void (*cb)(const char *msg))
+{
+	libfsverity_error_cb = cb;
+}
+
+void libfsverity_do_error_msg(const char *format, va_list va, int err)
+{
+	int saved_errno = errno;
+	char *msg = NULL;
+
+	if (!libfsverity_error_cb)
+		return;
+
+	if (vasprintf(&msg, format, va) < 0)
+		goto out;
+
+	if (err) {
+		char *msg2 = NULL;
+		char errbuf[64];
+
+		if (asprintf(&msg2, "%s: %s", msg,
+			     strerror_r(err, errbuf, sizeof(errbuf))) < 0)
+			goto out2;
+		free(msg);
+		msg = msg2;
+	}
+	(*libfsverity_error_cb)(msg);
+out2:
+	free(msg);
+out:
+	errno = saved_errno;
+}
+
+void libfsverity_error_msg(const char *format, ...)
+{
+	va_list va;
+
+	va_start(va, format);
+	libfsverity_do_error_msg(format, va, 0);
+	va_end(va);
+}
+
+void libfsverity_error_msg_errno(const char *format, ...)
+{
+	va_list va;
+
+	va_start(va, format);
+	libfsverity_do_error_msg(format, va, errno);
+	va_end(va);
+}
+
+void libfsverity_warn_on(const char *condition, const char *file, int line)
+{
+	fprintf(stderr, "libfsverity internal error! %s at %s:%d\n",
+		condition, file, line);
+}
+
+void libfsverity_bug_on(const char *condition, const char *file, int line)
+{
+	fprintf(stderr, "libfsverity internal error! %s at %s:%d.\n"
+		"Non-recoverable, aborting program.\n", condition, file, line);
+	abort();
+}
diff --git a/cmd_enable.c b/programs/cmd_enable.c
similarity index 82%
rename from cmd_enable.c
rename to programs/cmd_enable.c
index cc22f17..36e21a4 100644
--- a/cmd_enable.c
+++ b/programs/cmd_enable.c
@@ -7,43 +7,13 @@
  * Written by Eric Biggers.
  */
 
-#include "commands.h"
-#include "fsverity_uapi.h"
-#include "hash_algs.h"
+#include "fsverity.h"
 
 #include <fcntl.h>
 #include <getopt.h>
 #include <limits.h>
-#include <stdlib.h>
-#include <string.h>
 #include <sys/ioctl.h>
 
-static bool parse_hash_alg_option(const char *arg, u32 *alg_ptr)
-{
-	char *end;
-	unsigned long n = strtoul(arg, &end, 10);
-	const struct fsverity_hash_alg *alg;
-
-	if (*alg_ptr != 0) {
-		error_msg("--hash-alg can only be specified once");
-		return false;
-	}
-
-	/* Specified by number? */
-	if (n > 0 && n < INT32_MAX && *end == '\0') {
-		*alg_ptr = n;
-		return true;
-	}
-
-	/* Specified by name? */
-	alg = find_hash_alg_by_name(arg);
-	if (alg != NULL) {
-		*alg_ptr = alg - fsverity_hash_algs;
-		return true;
-	}
-	return false;
-}
-
 static bool read_signature(const char *filename, u8 **sig_ret,
 			   u32 *sig_size_ret)
 {
diff --git a/cmd_measure.c b/programs/cmd_measure.c
similarity index 84%
rename from cmd_measure.c
rename to programs/cmd_measure.c
index e8218ab..8676177 100644
--- a/cmd_measure.c
+++ b/programs/cmd_measure.c
@@ -7,12 +7,9 @@
  * Written by Eric Biggers.
  */
 
-#include "commands.h"
-#include "fsverity_uapi.h"
-#include "hash_algs.h"
+#include "fsverity.h"
 
 #include <fcntl.h>
-#include <stdlib.h>
 #include <sys/ioctl.h>
 
 /* Display the measurement of the given verity file(s). */
@@ -22,7 +19,6 @@ int fsverity_cmd_measure(const struct fsverity_command *cmd,
 	struct fsverity_digest *d = NULL;
 	struct filedes file;
 	char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + 1];
-	const struct fsverity_hash_alg *hash_alg;
 	char _hash_alg_name[32];
 	const char *hash_alg_name;
 	int status;
@@ -48,10 +44,8 @@ int fsverity_cmd_measure(const struct fsverity_command *cmd,
 
 		ASSERT(d->digest_size <= FS_VERITY_MAX_DIGEST_SIZE);
 		bin2hex(d->digest, d->digest_size, digest_hex);
-		hash_alg = find_hash_alg_by_num(d->digest_algorithm);
-		if (hash_alg) {
-			hash_alg_name = hash_alg->name;
-		} else {
+		hash_alg_name = libfsverity_hash_name(d->digest_algorithm);
+		if (!hash_alg_name) {
 			sprintf(_hash_alg_name, "ALG_%u", d->digest_algorithm);
 			hash_alg_name = _hash_alg_name;
 		}
diff --git a/cmd_sign.c b/programs/cmd_sign.c
similarity index 54%
rename from cmd_sign.c
rename to programs/cmd_sign.c
index 5e3270d..8999a13 100644
--- a/cmd_sign.c
+++ b/programs/cmd_sign.c
@@ -7,14 +7,10 @@
  * Written by Eric Biggers.
  */
 
-#include "commands.h"
-#include "fsverity_uapi.h"
-#include "sign.h"
+#include "fsverity.h"
 
 #include <fcntl.h>
 #include <getopt.h>
-#include <stdlib.h>
-#include <string.h>
 
 static bool write_signature(const char *filename, const u8 *sig, u32 sig_size)
 {
@@ -45,55 +41,60 @@ static const struct option longopts[] = {
 	{NULL, 0, NULL, 0}
 };
 
+static int read_callback(void *file, void *buf, size_t count)
+{
+	errno = 0;
+	if (!full_read(file, buf, count))
+		return errno ? -errno : -EIO;
+	return 0;
+}
+
 /* Sign a file for fs-verity by computing its measurement, then signing it. */
 int fsverity_cmd_sign(const struct fsverity_command *cmd,
 		      int argc, char *argv[])
 {
-	const struct fsverity_hash_alg *hash_alg = NULL;
-	u32 block_size = 0;
+	struct filedes file = { .fd = -1 };
 	u8 *salt = NULL;
-	u32 salt_size = 0;
-	const char *keyfile = NULL;
-	const char *certfile = NULL;
-	struct fsverity_signed_digest *digest = NULL;
+	struct libfsverity_merkle_tree_params tree_params = { .version = 1 };
+	struct libfsverity_signature_params sig_params = { NULL };
+	struct libfsverity_digest *digest = NULL;
 	char digest_hex[FS_VERITY_MAX_DIGEST_SIZE * 2 + 1];
 	u8 *sig = NULL;
-	u32 sig_size;
+	size_t sig_size;
 	int status;
 	int c;
 
 	while ((c = getopt_long(argc, argv, "", longopts, NULL)) != -1) {
 		switch (c) {
 		case OPT_HASH_ALG:
-			if (hash_alg != NULL) {
-				error_msg("--hash-alg can only be specified once");
-				goto out_usage;
-			}
-			hash_alg = find_hash_alg_by_name(optarg);
-			if (hash_alg == NULL)
+			if (!parse_hash_alg_option(optarg,
+						   &tree_params.hash_algorithm))
 				goto out_usage;
 			break;
 		case OPT_BLOCK_SIZE:
-			if (!parse_block_size_option(optarg, &block_size))
+			if (!parse_block_size_option(optarg,
+						     &tree_params.block_size))
 				goto out_usage;
 			break;
 		case OPT_SALT:
-			if (!parse_salt_option(optarg, &salt, &salt_size))
+			if (!parse_salt_option(optarg, &salt,
+					       &tree_params.salt_size))
 				goto out_usage;
+			tree_params.salt = salt;
 			break;
 		case OPT_KEY:
-			if (keyfile != NULL) {
+			if (sig_params.keyfile != NULL) {
 				error_msg("--key can only be specified once");
 				goto out_usage;
 			}
-			keyfile = optarg;
+			sig_params.keyfile = optarg;
 			break;
 		case OPT_CERT:
-			if (certfile != NULL) {
+			if (sig_params.certfile != NULL) {
 				error_msg("--cert can only be specified once");
 				goto out_usage;
 			}
-			certfile = optarg;
+			sig_params.certfile = optarg;
 			break;
 		default:
 			goto out_usage;
@@ -106,40 +107,46 @@ int fsverity_cmd_sign(const struct fsverity_command *cmd,
 	if (argc != 2)
 		goto out_usage;
 
-	if (hash_alg == NULL)
-		hash_alg = &fsverity_hash_algs[FS_VERITY_HASH_ALG_DEFAULT];
+	if (tree_params.hash_algorithm == 0)
+		tree_params.hash_algorithm = FS_VERITY_HASH_ALG_DEFAULT;
 
-	if (block_size == 0)
-		block_size = get_default_block_size();
+	if (tree_params.block_size == 0)
+		tree_params.block_size = get_default_block_size();
 
-	if (keyfile == NULL) {
+	if (sig_params.keyfile == NULL) {
 		error_msg("Missing --key argument");
 		goto out_usage;
 	}
-	if (certfile == NULL)
-		certfile = keyfile;
+	if (sig_params.certfile == NULL)
+		sig_params.certfile = sig_params.keyfile;
 
-	digest = xzalloc(sizeof(*digest) + hash_alg->digest_size);
-	memcpy(digest->magic, "FSVerity", 8);
-	digest->digest_algorithm = cpu_to_le16(hash_alg - fsverity_hash_algs);
-	digest->digest_size = cpu_to_le16(hash_alg->digest_size);
+	if (!open_file(&file, argv[0], O_RDONLY, 0))
+		goto out_err;
 
-	if (!compute_file_measurement(argv[0], hash_alg, block_size,
-				      salt, salt_size, digest->digest))
+	if (!get_file_size(&file, &tree_params.file_size))
 		goto out_err;
 
-	if (!sign_data(digest, sizeof(*digest) + hash_alg->digest_size,
-		       keyfile, certfile, hash_alg, &sig, &sig_size))
+	if (libfsverity_compute_digest(&file, read_callback,
+				       &tree_params, &digest) != 0) {
+		error_msg("failed to compute digest");
 		goto out_err;
+	}
+
+	if (libfsverity_sign_digest(digest, &sig_params,
+				    &sig, &sig_size) != 0) {
+		error_msg("failed to sign digest");
+		goto out_err;
+	}
 
 	if (!write_signature(argv[1], sig, sig_size))
 		goto out_err;
 
-	bin2hex(digest->digest, hash_alg->digest_size, digest_hex);
-	printf("Signed file '%s' (%s:%s)\n", argv[0], hash_alg->name,
-	       digest_hex);
+	bin2hex(digest->digest, digest->digest_size, digest_hex);
+	printf("Signed file '%s' (%s:%s)\n", argv[0],
+	       libfsverity_hash_name(tree_params.hash_algorithm), digest_hex);
 	status = 0;
 out:
+	filedes_close(&file);
 	free(salt);
 	free(digest);
 	free(sig);
diff --git a/fsverity.c b/programs/fsverity.c
similarity index 82%
rename from fsverity.c
rename to programs/fsverity.c
index 8d94c98..8b7f268 100644
--- a/fsverity.c
+++ b/programs/fsverity.c
@@ -7,12 +7,9 @@
  * Written by Eric Biggers.
  */
 
-#include "commands.h"
-#include "hash_algs.h"
+#include "fsverity.h"
 
 #include <limits.h>
-#include <stdlib.h>
-#include <string.h>
 #include <unistd.h>
 
 static const struct fsverity_command {
@@ -47,6 +44,17 @@ static const struct fsverity_command {
 	}
 };
 
+static void show_all_hash_algs(FILE *fp)
+{
+	u32 alg_num = 1;
+	const char *name;
+
+	fprintf(fp, "Available hash algorithms:");
+	while ((name = libfsverity_hash_name(alg_num++)) != NULL)
+		fprintf(fp, " %s", name);
+	putc('\n', fp);
+}
+
 static void usage_all(FILE *fp)
 {
 	int i;
@@ -59,10 +67,8 @@ static void usage_all(FILE *fp)
 "  Standard options:\n"
 "    fsverity --help\n"
 "    fsverity --version\n"
-"\n"
-"Available hash algorithms: ", fp);
+"\n", fp);
 	show_all_hash_algs(fp);
-	putc('\n', fp);
 }
 
 static void usage_cmd(const struct fsverity_command *cmd, FILE *fp)
@@ -127,6 +133,31 @@ static const struct fsverity_command *find_command(const char *name)
 	return NULL;
 }
 
+bool parse_hash_alg_option(const char *arg, u32 *alg_ptr)
+{
+	char *end;
+	unsigned long n = strtoul(arg, &end, 10);
+
+	if (*alg_ptr != 0) {
+		error_msg("--hash-alg can only be specified once");
+		return false;
+	}
+
+	/* Specified by number? */
+	if (n > 0 && n < INT32_MAX && *end == '\0') {
+		*alg_ptr = n;
+		return true;
+	}
+
+	/* Specified by name? */
+	*alg_ptr = libfsverity_find_hash_alg_by_name(arg);
+	if (*alg_ptr)
+		return true;
+	error_msg("unknown hash algorithm: '%s'", arg);
+	show_all_hash_algs(stderr);
+	return false;
+}
+
 bool parse_block_size_option(const char *arg, u32 *size_ptr)
 {
 	char *end;
@@ -173,10 +204,17 @@ u32 get_default_block_size(void)
 	return n;
 }
 
+static void print_libfsverity_error(const char *msg)
+{
+	error_msg("%s", msg);
+}
+
 int main(int argc, char *argv[])
 {
 	const struct fsverity_command *cmd;
 
+	libfsverity_set_error_callback(print_libfsverity_error);
+
 	if (argc < 2) {
 		error_msg("no command specified");
 		usage_all(stderr);
diff --git a/programs/fsverity.h b/programs/fsverity.h
new file mode 100644
index 0000000..f690528
--- /dev/null
+++ b/programs/fsverity.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Private header for the 'fsverity' program
+ */
+#ifndef PROGRAMS_FSVERITY_H
+#define PROGRAMS_FSVERITY_H
+
+#include "utils.h"
+#include "../common/fsverity_uapi.h"
+
+/* 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
+
+struct fsverity_command;
+
+/* cmd_enable.c */
+int fsverity_cmd_enable(const struct fsverity_command *cmd,
+			int argc, char *argv[]);
+
+/* cmd_measure.c */
+int fsverity_cmd_measure(const struct fsverity_command *cmd,
+			 int argc, char *argv[]);
+
+/* cmd_sign.c */
+int fsverity_cmd_sign(const struct fsverity_command *cmd,
+		      int argc, char *argv[]);
+
+/* fsverity.c */
+void usage(const struct fsverity_command *cmd, FILE *fp);
+bool parse_hash_alg_option(const char *arg, u32 *alg_ptr);
+bool parse_block_size_option(const char *arg, u32 *size_ptr);
+bool parse_salt_option(const char *arg, u8 **salt_ptr, u32 *salt_size_ptr);
+u32 get_default_block_size(void);
+
+#endif /* PROGRAMS_FSVERITY_H */
diff --git a/util.c b/programs/utils.c
similarity index 96%
rename from util.c
rename to programs/utils.c
index 2951729..be68a74 100644
--- a/util.c
+++ b/programs/utils.c
@@ -7,15 +7,12 @@
  * Written by Eric Biggers.
  */
 
-#include "util.h"
+#include "utils.h"
 
 #include <errno.h>
 #include <fcntl.h>
 #include <limits.h>
 #include <stdarg.h>
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
 #include <sys/stat.h>
 #include <unistd.h>
 
@@ -47,7 +44,7 @@ char *xstrdup(const char *s)
 
 /* ========== Error messages and assertions ========== */
 
-void do_error_msg(const char *format, va_list va, int err)
+static void do_error_msg(const char *format, va_list va, int err)
 {
 	fputs("ERROR: ", stderr);
 	vfprintf(stderr, format, va);
diff --git a/programs/utils.h b/programs/utils.h
new file mode 100644
index 0000000..8916527
--- /dev/null
+++ b/programs/utils.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0+ */
+/*
+ * Utility functions for programs
+ */
+#ifndef PROGRAMS_UTILS_H
+#define PROGRAMS_UTILS_H
+
+#include "../common/libfsverity.h"
+#include "../common/common_defs.h"
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+void *xmalloc(size_t size);
+void *xzalloc(size_t size);
+void *xmemdup(const void *mem, size_t size);
+char *xstrdup(const char *s);
+
+__printf(1, 2) __cold void error_msg(const char *format, ...);
+__printf(1, 2) __cold void error_msg_errno(const char *format, ...);
+__printf(1, 2) __cold __noreturn void fatal_error(const char *format, ...);
+__cold __noreturn void assertion_failed(const char *expr,
+					const char *file, int line);
+
+#define ASSERT(e) ({ if (!(e)) assertion_failed(#e, __FILE__, __LINE__); })
+
+struct filedes {
+	int fd;
+	char *name;		/* filename, for logging or error messages */
+};
+
+bool open_file(struct filedes *file, const char *filename, int flags, int mode);
+bool get_file_size(struct filedes *file, u64 *size_ret);
+bool full_read(struct filedes *file, void *buf, size_t count);
+bool full_write(struct filedes *file, const void *buf, size_t count);
+bool filedes_close(struct filedes *file);
+
+bool hex2bin(const char *hex, u8 *bin, size_t bin_len);
+void bin2hex(const u8 *bin, size_t bin_len, char *hex);
+
+#endif /* PROGRAMS_UTILS_H */
diff --git a/sign.h b/sign.h
deleted file mode 100644
index f10ac91..0000000
--- a/sign.h
+++ /dev/null
@@ -1,32 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-#ifndef SIGN_H
-#define SIGN_H
-
-#include "hash_algs.h"
-
-#include <linux/types.h>
-
-/*
- * Format in which verity file measurements are signed.  This is the same as
- * 'struct fsverity_digest', except here some magic bytes are prepended to
- * provide some context about what is being signed in case the same key is used
- * for non-fsverity purposes, and here the fields have fixed endianness.
- */
-struct fsverity_signed_digest {
-	char magic[8];			/* must be "FSVerity" */
-	__le16 digest_algorithm;
-	__le16 digest_size;
-	__u8 digest[];
-};
-
-bool compute_file_measurement(const char *filename,
-			      const struct fsverity_hash_alg *hash_alg,
-			      u32 block_size, const u8 *salt,
-			      u32 salt_size, u8 *measurement);
-
-bool sign_data(const void *data_to_sign, size_t data_size,
-	       const char *keyfile, const char *certfile,
-	       const struct fsverity_hash_alg *hash_alg,
-	       u8 **sig_ret, u32 *sig_size_ret);
-
-#endif /* SIGN_H */
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/3] Add some basic test programs for libfsverity
  2020-05-15  4:10 [PATCH 0/3] fsverity-utils: introduce libfsverity Eric Biggers
  2020-05-15  4:10 ` [PATCH 1/3] Split up cmd_sign.c Eric Biggers
  2020-05-15  4:10 ` [PATCH 2/3] Introduce libfsverity Eric Biggers
@ 2020-05-15  4:10 ` Eric Biggers
  2020-05-21 15:29   ` Jes Sorensen
  2020-05-15 20:50 ` [PATCH 0/3] fsverity-utils: introduce libfsverity Jes Sorensen
  3 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-15  4:10 UTC (permalink / raw)
  To: linux-fscrypt, Jes Sorensen; +Cc: jsorensen, kernel-team

From: Eric Biggers <ebiggers@google.com>

Add three test programs: 'test_hash_algs', 'test_compute_digest', and
'test_sign_digest'.  Nothing fancy yet, just some basic tests to test
each library function.

With the new Makefile, these get run by 'make check'.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 programs/test_compute_digest.c |  54 +++++++++++++++++++++++++++++++++
 programs/test_hash_algs.c      |  27 +++++++++++++++++
 programs/test_sign_digest.c    |  44 +++++++++++++++++++++++++++
 testdata/cert.pem              |  31 +++++++++++++++++++
 testdata/file.sig              | Bin 0 -> 708 bytes
 testdata/key.pem               |  52 +++++++++++++++++++++++++++++++
 6 files changed, 208 insertions(+)
 create mode 100644 programs/test_compute_digest.c
 create mode 100644 programs/test_hash_algs.c
 create mode 100644 programs/test_sign_digest.c
 create mode 100644 testdata/cert.pem
 create mode 100644 testdata/file.sig
 create mode 100644 testdata/key.pem

diff --git a/programs/test_compute_digest.c b/programs/test_compute_digest.c
new file mode 100644
index 0000000..5d00576
--- /dev/null
+++ b/programs/test_compute_digest.c
@@ -0,0 +1,54 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "utils.h"
+
+struct file {
+	u8 *data;
+	size_t size;
+	size_t offset;
+};
+
+static int read_fn(void *fd, void *buf, size_t count)
+{
+	struct file *f = fd;
+
+	ASSERT(count <= f->size - f->offset);
+	memcpy(buf, &f->data[f->offset], count);
+	f->offset += count;
+	return 0;
+}
+
+int main(void)
+{
+	struct file f = { .size = 1000000 };
+	size_t i;
+	const struct libfsverity_merkle_tree_params params = {
+		.version = 1,
+		.hash_algorithm = FS_VERITY_HASH_ALG_SHA256,
+		.block_size = 4096,
+		.salt_size = 4,
+		.salt = (u8 *)"abcd",
+		.file_size = f.size,
+	};
+	struct libfsverity_digest *d;
+	static const u8 expected_digest[32] =
+		"\x91\x79\x00\xb0\xd2\x99\x45\x4a\xa3\x04\xd5\xde\xbc\x6f\x39"
+		"\xe4\xaf\x7b\x5a\xbe\x33\xbd\xbc\x56\x8d\x5d\x8f\x1e\x5c\x4d"
+		"\x86\x52";
+	int err;
+
+	f.data = xmalloc(f.size);
+	for (i = 0; i < f.size; i++)
+		f.data[i] = (i % 11) + (i % 439) + (i % 1103);
+
+	err = libfsverity_compute_digest(&f, read_fn, &params, &d);
+	ASSERT(err == 0);
+
+	ASSERT(d->digest_algorithm == FS_VERITY_HASH_ALG_SHA256);
+	ASSERT(d->digest_size == 32);
+	ASSERT(!memcmp(d->digest, expected_digest, 32));
+
+	free(f.data);
+	free(d);
+	printf("test_compute_digest passed\n");
+	return 0;
+}
diff --git a/programs/test_hash_algs.c b/programs/test_hash_algs.c
new file mode 100644
index 0000000..b7db2ac
--- /dev/null
+++ b/programs/test_hash_algs.c
@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "utils.h"
+
+int main(void)
+{
+	ASSERT(libfsverity_digest_size(0) == -1);
+	ASSERT(libfsverity_hash_name(0) == NULL);
+	ASSERT(libfsverity_find_hash_alg_by_name("bad") == 0);
+
+	ASSERT(libfsverity_digest_size(100) == -1);
+	ASSERT(libfsverity_hash_name(100) == NULL);
+
+	ASSERT(libfsverity_digest_size(FS_VERITY_HASH_ALG_SHA256) == 32);
+	ASSERT(!strcmp("sha256",
+		       libfsverity_hash_name(FS_VERITY_HASH_ALG_SHA256)));
+	ASSERT(libfsverity_find_hash_alg_by_name("sha256") ==
+	       FS_VERITY_HASH_ALG_SHA256);
+
+	ASSERT(libfsverity_digest_size(FS_VERITY_HASH_ALG_SHA512) == 64);
+	ASSERT(!strcmp("sha512",
+		       libfsverity_hash_name(FS_VERITY_HASH_ALG_SHA512)));
+	ASSERT(libfsverity_find_hash_alg_by_name("sha512") ==
+	       FS_VERITY_HASH_ALG_SHA512);
+
+	printf("test_hash_algs passed\n");
+	return 0;
+}
diff --git a/programs/test_sign_digest.c b/programs/test_sign_digest.c
new file mode 100644
index 0000000..0a97865
--- /dev/null
+++ b/programs/test_sign_digest.c
@@ -0,0 +1,44 @@
+// SPDX-License-Identifier: GPL-2.0+
+#include "utils.h"
+
+#include <fcntl.h>
+
+int main(void)
+{
+	struct libfsverity_digest *d = xzalloc(sizeof(*d) + 32);
+	u8 *sig;
+	size_t sig_size;
+	struct libfsverity_signature_params params = {
+		.keyfile = "testdata/key.pem",
+		.certfile = "testdata/cert.pem",
+	};
+	struct filedes file;
+	u8 *expected_sig;
+	u64 expected_sig_size;
+	int err;
+
+	d->digest_algorithm = FS_VERITY_HASH_ALG_SHA256;
+	d->digest_size = 32;
+	memcpy(d->digest,
+	       "\x91\x79\x00\xb0\xd2\x99\x45\x4a\xa3\x04\xd5\xde\xbc\x6f\x39"
+	       "\xe4\xaf\x7b\x5a\xbe\x33\xbd\xbc\x56\x8d\x5d\x8f\x1e\x5c\x4d"
+	       "\x86\x52", 32);
+
+	err = libfsverity_sign_digest(d, &params, &sig, &sig_size);
+	ASSERT(err == 0);
+
+	ASSERT(open_file(&file, "testdata/file.sig", O_RDONLY, 0));
+	ASSERT(get_file_size(&file, &expected_sig_size));
+	ASSERT(sig_size == expected_sig_size);
+	expected_sig = xmalloc(sig_size);
+	ASSERT(full_read(&file, expected_sig, sig_size));
+	ASSERT(!memcmp(sig, expected_sig, sig_size));
+
+	free(d);
+	free(sig);
+	free(expected_sig);
+	filedes_close(&file);
+	printf("test_sign_digest passed\n");
+	return 0;
+}
+
diff --git a/testdata/cert.pem b/testdata/cert.pem
new file mode 100644
index 0000000..c63b965
--- /dev/null
+++ b/testdata/cert.pem
@@ -0,0 +1,31 @@
+-----BEGIN CERTIFICATE-----
+MIIFazCCA1OgAwIBAgIUYaRYcyZGDIv9fIxx/RoJwQu23+owDQYJKoZIhvcNAQEL
+BQAwRTELMAkGA1UEBhMCQVUxEzARBgNVBAgMClNvbWUtU3RhdGUxITAfBgNVBAoM
+GEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZDAeFw0yMDA1MTUwMjUyMzFaFw0yMDA2
+MTQwMjUyMzFaMEUxCzAJBgNVBAYTAkFVMRMwEQYDVQQIDApTb21lLVN0YXRlMSEw
+HwYDVQQKDBhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwggIiMA0GCSqGSIb3DQEB
+AQUAA4ICDwAwggIKAoICAQC5lXk9otBz/VM/tbvBBK6sK//HE+q3ctY4+fPVv3Ob
+D1YNYuWRD59U+7K8fVUfZlXyjgxt3n4VPzkLgRSr/w5YUTa5NEOVJIltKT4ugswL
+5oY8eRVSuIr1O+vTbu+EpUk3DhTaFkalVzwspwipBeiVTDO9kh0NAueRk2HyLJte
+IoPyfzSCKxg9sED6/WtLFqrhDb5+1qeGoMNGM66ueWXKX0QjomMEODGXC04ypIY3
+sTwB+sYhZUe3YRpY0HyaVNh/6cxCxSiKr2jkC5UL+ry+46EJerNZbKmeqqyqmhXh
+P2cHv8MO91zdH1xbXUUenLcdpK/0oq+/sTAVV1/qPvnAofpN8tdZdrH65JD753jt
+s+lH/f0iGuKAb9ZpLOTM2d3wjY13OcHElj0zu2Usw9PXQpTK/DYlbcapOI4NTVyU
+NpK3yP4i5dPnkHoxpjLWz75Eq6gP9ZXohGq3YG0LxtvELWfaFmpzEUTlD59hJJOZ
+ELGxAXzsxLbelX/EmpX+GIqnFBpdMIuPO4HEfJwD5IcdeHqGl2iH9LIqsY5DcGj4
+hnqYplIYYk5mWgbhkexRbJIGNdn8WyXlraVp2MoSr3p7xJbmo0qYRRtt3kQShzDG
+0FrZX7wqxemc/g/hr+g2xqMQj0nYLehDeodqxA2n9grDUr4AdgQyxXDMUkGZdrg3
+cwIDAQABo1MwUTAdBgNVHQ4EFgQUxpaj6YjgLFyh9UVM71hf18cb/hkwHwYDVR0j
+BBgwFoAUxpaj6YjgLFyh9UVM71hf18cb/hkwDwYDVR0TAQH/BAUwAwEB/zANBgkq
+hkiG9w0BAQsFAAOCAgEAQnXMCgI+eSjK+l3nrpE+TRrXZhHeB0aT3gilVtBqFM95
+GxkLzOgJnW4SU+BCKTiGjwhCEXQiFj6UNDrI7vaNzmurI370uqmC9/pwKn4/L27V
+ToqLHk5d8kmvjSJyfgY/9H73srzHjNcqLG3uy+JP3/fIzaUzy7x9OUJtzdH19zic
+b61kCbqe6TJrlpL0Y50FY91QTzupsIS9IsAAeYXrJiEwpkXv/O9c51swtGZmQhVD
+TDn4B2aKOHecR+gKZQbPcuwTCbNLDRRPT4q0IM9yKjUxIM8vkAaxlrW3O7fgqUV0
+GU3/i0zZugq2XEludF3VelrqMUhSMqaREAtRUe3ufipwEsovDa43Hr9P6bINAfU3
+92Kv6adgeKZc4DmuEg6sFje/ET85teioHtwmjviJu6vnkbZg7x+IU01Y6YHboz/z
+hTAjz9g6owgdsbTG9nptvgJllY83zBtnAGOqhJLNVZ+TC3pbbKht/7sT+s+WP2+K
+81oZ3gmxIr6myMVyR5CCt+FNJ1hFxYNBJDao35iiZLxDe2s5nMJKHYezUY60ujqT
+Ljv3Ku4uAk5PgXltZnWGz152ntjopA0gbnlU4f+SgnmPoBFcvn36BcUQWQbTDqmh
+h+Y0OaXR3x/27M/qkPBov4IAfoCkWeF7i02wxwtdTLiSF7OjTDkQXtZemUzN5+w=
+-----END CERTIFICATE-----
diff --git a/testdata/file.sig b/testdata/file.sig
new file mode 100644
index 0000000000000000000000000000000000000000..1ba61f8c939c8d2616953f4ac8681f9f5988cd0c
GIT binary patch
literal 708
zcmXqLVmiRasnzDu_MMlJooPW6(?)|PrnO9rjE4LMylk8aZ61uN%q&cdtPBR+2!)J>
zO-vm?g)KmZ2C)XNhTI06Y|No7Y{E>Ap@zZ+f*=kD4_9!0ZmMo@Nn%N=p`w92NRW$1
z!ZWWVwJ0yOL?JvgB|WpGSRtUKQo*Mrg-IlFNkp-l8&CJ&nx4YHQk)05x7~k*?h2rz
zpw49kI*O%<iGg!!lb*qm@M8gu7e2ppaAveD+qdta)zR|kQ_GGWju6_iS3RtEj?1rg
z+gxO3AC%f_zvR$9_K?K_FQ$C6T*1iT$$!u=sI$GcweNhG#bo1Se$U@W|7bs-u~RuT
zU#P+L)aCq#yX-Ijxbv4q?@y!phckhfsuNNst$+A+ht(TN#?A&FrAehGYl}Ah^IBOZ
zFE&9<L+zgUob1vD12=Q7t%Wh|Dwziqrj)&ZH>2X4fms>ft>ndJ&RWZ5o8OfE)I0Q6
z-6Gz&UN@n8PH&~n8Ron1h2-iB%AcH>;*)fTRg+QP_VW6LCgwlyW$PTeS?E-7>6iaL
z`wr!rH%<%BaBj{tc){-6eZF`F`)q})$1~qD6-lRB$+bHdidb<+X(vwQV3}Su^X6(f
zPpcJEKiBT9e|SqKy31_Uw;f-4<ko4+GgXEu*XL=hf6>FLe&@{^S^gbv_r9sUnz_05
zVb_<#Cv7_lu6LcxI#s-4=G-6GXIH=4{&L>-9s1c7rOM^Mp141CdtB$ba}wiu{eSzy
zrl_1d5~eTyPr8;jtJdhvzwQ6NulgwBQD%Nu&%M{<u5A|Y`bI`wR&hz|jMq*sYZKKM
zMjjTp@bXEI;OeZ`aosaDGEOglf9yz&iu@AKoXSX^%f{~*Z+*~rF5Ehk$7`w6*%N68
hQ#Gt}UVr&ozToz@w0R5m{&x=h$?<v4+YgS@jRE2!E@}V(

literal 0
HcmV?d00001

diff --git a/testdata/key.pem b/testdata/key.pem
new file mode 100644
index 0000000..e76db4c
--- /dev/null
+++ b/testdata/key.pem
@@ -0,0 +1,52 @@
+-----BEGIN PRIVATE KEY-----
+MIIJQQIBADANBgkqhkiG9w0BAQEFAASCCSswggknAgEAAoICAQC5lXk9otBz/VM/
+tbvBBK6sK//HE+q3ctY4+fPVv3ObD1YNYuWRD59U+7K8fVUfZlXyjgxt3n4VPzkL
+gRSr/w5YUTa5NEOVJIltKT4ugswL5oY8eRVSuIr1O+vTbu+EpUk3DhTaFkalVzws
+pwipBeiVTDO9kh0NAueRk2HyLJteIoPyfzSCKxg9sED6/WtLFqrhDb5+1qeGoMNG
+M66ueWXKX0QjomMEODGXC04ypIY3sTwB+sYhZUe3YRpY0HyaVNh/6cxCxSiKr2jk
+C5UL+ry+46EJerNZbKmeqqyqmhXhP2cHv8MO91zdH1xbXUUenLcdpK/0oq+/sTAV
+V1/qPvnAofpN8tdZdrH65JD753jts+lH/f0iGuKAb9ZpLOTM2d3wjY13OcHElj0z
+u2Usw9PXQpTK/DYlbcapOI4NTVyUNpK3yP4i5dPnkHoxpjLWz75Eq6gP9ZXohGq3
+YG0LxtvELWfaFmpzEUTlD59hJJOZELGxAXzsxLbelX/EmpX+GIqnFBpdMIuPO4HE
+fJwD5IcdeHqGl2iH9LIqsY5DcGj4hnqYplIYYk5mWgbhkexRbJIGNdn8WyXlraVp
+2MoSr3p7xJbmo0qYRRtt3kQShzDG0FrZX7wqxemc/g/hr+g2xqMQj0nYLehDeodq
+xA2n9grDUr4AdgQyxXDMUkGZdrg3cwIDAQABAoICACla0aWWfnUaYk60JJ6ieHoN
+Y/XszkUK5gnUSS28d/p5tGdPPnDQ1mSNogq2sx1IJKbkWIizJ818RS33GbAqKfws
+PNGQf+7gMW+N3TloFCgiuo8HPGUukmiLbcWz1tPsMSB/ls3yYNO/WL1qi1d+5ZE/
+Zdg8kxSvLQMXoJ/iqMyVTGnhRsYq7D/y4sgLaLlW18VG1shU9QffEyS1p5thmfk6
+uWhna0EpdIOAFXDbkL0gVYrrYvNWKmEG1mQsMVgCyCvY4ZePb7VX2TvYCOKegSjY
+eK4wFX8746Bj0A5EP9Pt2Pu1E7ZmEN+FeYMyiZCEw5lrdXpCNn+08E4RJmKAng6Z
+LXu7FeBwOffsEcYDk1IPtT4JAkpEO0ennjyY2X+bWu6faaSGqoTDRm0igL/2qeDK
+xJCCOTPDePSDKkVFaimBncQlz4SSVS7WhqqofFMBIPBtZ6oe0c+0ZeKoaH2ZX7jU
+q2DTsRbUKeRerjOlAg5WxQoLxTmLl4t7aHjm4HVafEZHqja5szyNLy13qJZyo7+x
+R8kDRkVdui7XKdC9ajCMAsVcen4NyrvNqQHSoocgqmSuD3d5S/eZnE0cSGGX3Nae
+pb0bAFthQdPq2qfZDDL/cRpe+0FAsOtXG66yEcQjZdsdNerU+pSi/sgArJkNj/U1
+Dqd0LCPOOFiLa/f9LO6hAoIBAQD2HHn2Ghl7W1j4t806i6tfDNocgUCaO+2wjaOY
+cNrg0WiJP24oaRmXew1Le0q5NhX8wHgGZAvVOh0rkcX+sUBoSURzWFK5cavxIGmL
+XqW5wHFS9ahS3mQ3I0zEbueYPyFKEg2a2aFqqq8g3XRqgZtc4kfg4GUbIoltey4z
+uQlp3tIpbRjD9w2cYF9bU9xEgdp7PU5TDmYDiRetSd0HxwD2osj40sWsEbV9UN+4
+xIh6mUwn1ZcJmt7NzuHwyXGs63EhSQoxQwS/vAt0pIKwAlyn06//GBFZGhpnpjCT
+flN9vQPaqJZA93kugsBEt3fX+oGbFTVHgEYsyMTEzH2by6c1AoIBAQDBCmg0ziga
+2WZJE0YARf/gek3MKZNG6qjahuRynpGdtgqa6cSgnRzntDCEkLhU6KZ5+SYJNp0x
+9EBkO7o8YOGWfGUtnccqdbSCt14959Vqn+kcrKnadjFiSVOAGc1728YUjd3jYGS8
+ZXkGe3xXhmXiB1mv8ekKrGY2mmx7mBiZ40gNGJNg1gboenffMeP5l7JjaO7ew5QY
+A3swJkBHNIyGCdntlmrlw1vxT7bmDQoHVLKdQ+VFJFvUVyY40nVkeH1/9WB215Lu
+hzDTzn5Q+Z19pjEouohsJDY6k2FbOyaT0c/KI1NDlNZmUpWcoqf7IIusg1eCCGCH
+nXafifGxv7EHAoIBAAehemaXCJM6kdekW0ila/rWeyzHFSmzEfuXaKshVKgD1inr
+PY8jMxfvSMo+WGLFuojLru0DzRofYygmrOzosgaJvwWUh3wYeixPxPX9SUYpIVph
+I4buPk03Wvn8NlISIwYY6TMT7F1STXvHYgSrYBXRLklaq8fbmkc6uoQACLqvnfSK
+3Wm2D0X59vrt7rZxEEUh8XvBxof1iDZnQ+Mp2G3NPk34uwhKxEXObCFedpzWg/X4
+OWai1qWq9HZyyIOECU3u5dIBMfR/8Br9vs+WQykw9xQBuwf4NzlffcIU+KG9apEt
+CPuasLcwdqWqypx3t+0HC0/cOlDJKNCxRnO+LMECggEABzLoJ+/4NugclGUPmzsB
+C9IDzLVQNLjTizK0mkGnlIYRZy2Ik6TISyvBE3CCL0htzOapsHZE7nP5YsOHcnD6
+eK4y57yWjNLO5IEKFqzqnItSGiumOetmdA/f+Ur9Cr1raaDQwYX6u7vdA4zfWjQ8
+4Gz9vz36PtenCCpCGWnWoQaEzVg5Rsc0gr7ucXhe1BQAJwzmu4/3md2nXmhOxVkE
+VItRgTa2zdK3PwyF+ZZK5XMXJh4+EpIEiqqlVkEi95g2terkqgnoBNUt0PhGZaap
+ZOIpuycZp07CZvTQEKLoEWMlqJggpsiKJk62HZ1DPm48RzausL63Otd4cQKn7MUF
+SQKCAQAHbuzSCvI1YUnFiXtfNLIXdfTV1UJ+y9Mmb3/Lz/wvfbGiAC+TrJ4sdA7R
+k4riwnrv3F3kMop1gxQk1iWSt6qPg2tvw6ZoBcnzE6qJ2/xSdIXgrpV4kCrxAVYz
+vYt9KMWp17mgk4BYbnYU/1U1hZVowlhyx+lzsZtaRRnySvxeEcCO+eR6jTKAXWfH
+rVlbGuuBcTQ/Sz+Jw7mIM3gcI+fE9FyRznSjgVnsXqIsV9bntqkEUFb8VfunR2g5
+6eMoP8XSQdTcEvtKNJ2hrr9fkRABE4L/EeaMT4XOcYyNN9nPka+uj2nRoB9Vsb25
+x55d2r87sh6ScDvl57EIav09Y6W/
+-----END PRIVATE KEY-----
-- 
2.26.2


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] fsverity-utils: introduce libfsverity
  2020-05-15  4:10 [PATCH 0/3] fsverity-utils: introduce libfsverity Eric Biggers
                   ` (2 preceding siblings ...)
  2020-05-15  4:10 ` [PATCH 3/3] Add some basic test programs for libfsverity Eric Biggers
@ 2020-05-15 20:50 ` Jes Sorensen
  2020-05-20  3:06   ` Eric Biggers
  3 siblings, 1 reply; 14+ messages in thread
From: Jes Sorensen @ 2020-05-15 20:50 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt, Jes Sorensen; +Cc: jsorensen, kernel-team

On 5/15/20 12:10 AM, Eric Biggers wrote:
> From the 'fsverity' program, split out a library 'libfsverity'.
> Currently it supports computing file measurements ("digests"), and
> signing those file measurements for use with the fs-verity builtin
> signature verification feature.
> 
> Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
> I made a lot of improvements; see patch 2 for details.
> 
> Jes, can you let me know whether this works for you?  Especially take a
> close look at the API in libfsverity.h.

Hi Eric,

Thanks for looking at this. I have gone through this and managed to get
my RPM code to work with it. I will push the updated code to my rpm
github repo shortly. I have two fixes for the Makefile I will send to
you in a separate email.

One comment I have is that you changed the size of version and
hash_algorithm to 32 bit in struct libfsverity_merkle_tree_params, but
the kernel API only takes 8 bit values anyway. I had them at 16 bit to
handle the struct padding, but if anything it seems to make more sense
to make them 8 bit and pad the struct?

struct libfsverity_merkle_tree_params {
        uint32_t version;
        uint32_t hash_algorithm;

That said, not a big deal.

Cheers,
Jes

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] fsverity-utils: introduce libfsverity
  2020-05-15 20:50 ` [PATCH 0/3] fsverity-utils: introduce libfsverity Jes Sorensen
@ 2020-05-20  3:06   ` Eric Biggers
  2020-05-20 13:26     ` Jes Sorensen
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-20  3:06 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-fscrypt, Jes Sorensen, jsorensen, kernel-team

On Fri, May 15, 2020 at 04:50:49PM -0400, Jes Sorensen wrote:
> On 5/15/20 12:10 AM, Eric Biggers wrote:
> > From the 'fsverity' program, split out a library 'libfsverity'.
> > Currently it supports computing file measurements ("digests"), and
> > signing those file measurements for use with the fs-verity builtin
> > signature verification feature.
> > 
> > Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
> > I made a lot of improvements; see patch 2 for details.
> > 
> > Jes, can you let me know whether this works for you?  Especially take a
> > close look at the API in libfsverity.h.
> 
> Hi Eric,
> 
> Thanks for looking at this. I have gone through this and managed to get
> my RPM code to work with it. I will push the updated code to my rpm
> github repo shortly. I have two fixes for the Makefile I will send to
> you in a separate email.
> 
> One comment I have is that you changed the size of version and
> hash_algorithm to 32 bit in struct libfsverity_merkle_tree_params, but
> the kernel API only takes 8 bit values anyway. I had them at 16 bit to
> handle the struct padding, but if anything it seems to make more sense
> to make them 8 bit and pad the struct?
> 
> struct libfsverity_merkle_tree_params {
>         uint32_t version;
>         uint32_t hash_algorithm;
> 
> That said, not a big deal.
> 

Well, they're 32-bit in 'struct fsverity_enable_arg' (the argument to
FS_IOC_ENABLE_VERITY), but 8-bit in 'struct fsverity_descriptor'.
The reason for the difference is that 'struct fsverity_enable_arg' is just an
in-memory structure for the ioctl, so there was no reason not to use larger
fields.  But fsverity_descriptor is stored on-disk and hashed, and it has to
have a specific byte order, so just using 8-bit fields for it seemed best.

'struct libfsverity_merkle_tree_params' is just an in-memory structure too, so I
think going with the 32-bits convention makes sense.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/3] fsverity-utils: introduce libfsverity
  2020-05-20  3:06   ` Eric Biggers
@ 2020-05-20 13:26     ` Jes Sorensen
  0 siblings, 0 replies; 14+ messages in thread
From: Jes Sorensen @ 2020-05-20 13:26 UTC (permalink / raw)
  To: Eric Biggers, Jes Sorensen; +Cc: linux-fscrypt, Jes Sorensen, kernel-team

On 5/19/20 11:06 PM, Eric Biggers wrote:
> On Fri, May 15, 2020 at 04:50:49PM -0400, Jes Sorensen wrote:
>> On 5/15/20 12:10 AM, Eric Biggers wrote:
>>> From the 'fsverity' program, split out a library 'libfsverity'.
>>> Currently it supports computing file measurements ("digests"), and
>>> signing those file measurements for use with the fs-verity builtin
>>> signature verification feature.
>>>
>>> Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
>>> I made a lot of improvements; see patch 2 for details.
>>>
>>> Jes, can you let me know whether this works for you?  Especially take a
>>> close look at the API in libfsverity.h.
>>
>> Hi Eric,
>>
>> Thanks for looking at this. I have gone through this and managed to get
>> my RPM code to work with it. I will push the updated code to my rpm
>> github repo shortly. I have two fixes for the Makefile I will send to
>> you in a separate email.
>>
>> One comment I have is that you changed the size of version and
>> hash_algorithm to 32 bit in struct libfsverity_merkle_tree_params, but
>> the kernel API only takes 8 bit values anyway. I had them at 16 bit to
>> handle the struct padding, but if anything it seems to make more sense
>> to make them 8 bit and pad the struct?
>>
>> struct libfsverity_merkle_tree_params {
>>         uint32_t version;
>>         uint32_t hash_algorithm;
>>
>> That said, not a big deal.
>>
> 
> Well, they're 32-bit in 'struct fsverity_enable_arg' (the argument to
> FS_IOC_ENABLE_VERITY), but 8-bit in 'struct fsverity_descriptor'.
> The reason for the difference is that 'struct fsverity_enable_arg' is just an
> in-memory structure for the ioctl, so there was no reason not to use larger
> fields.  But fsverity_descriptor is stored on-disk and hashed, and it has to
> have a specific byte order, so just using 8-bit fields for it seemed best.
> 
> 'struct libfsverity_merkle_tree_params' is just an in-memory structure too, so I
> think going with the 32-bits convention makes sense.

OK, thanks for the explanation, it's not a big deal going one way or the
other.

Cheers,
Jes


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] Introduce libfsverity
  2020-05-15  4:10 ` [PATCH 2/3] Introduce libfsverity Eric Biggers
@ 2020-05-21 15:24   ` Jes Sorensen
  2020-05-21 16:08     ` Eric Biggers
  0 siblings, 1 reply; 14+ messages in thread
From: Jes Sorensen @ 2020-05-21 15:24 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt; +Cc: jsorensen, kernel-team

On 5/15/20 12:10 AM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> From the 'fsverity' program, split out a library 'libfsverity'.
> Currently it supports computing file measurements ("digests"), and
> signing those file measurements for use with the fs-verity builtin
> signature verification feature.
> 
> Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
> I made a lot of improvements, e.g.:
> 
> - Separated library and program source into different directories.
> - Drastically improved the Makefile.
> - Added 'make check' target and rules to build test programs.
> - In the shared lib, only export the functions intended to be public.
> - Prefixed global functions with "libfsverity_" so that they don't cause
>   conflicts when the library is built as a static library.
> - Made library error messages be sent to a user-specified callback
>   rather than always be printed to stderr.
> - Keep showing OpenSSL error messages.
> - Stopped abort()ing in library code, when possible.
> - Made libfsverity_digest use native endianness.
> - Moved file_size into the merkle_tree_params.
> - Made libfsverity_hash_name() just return the static strings.
> - Made some variables in the API uint32_t instead of uint16_t.
> - Shared parse_hash_alg_option() between cmd_enable and cmd_sign.
> - Lots of fixes.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Eric,

Here is a more detailed review. The code as we have it seems to work for
me, but there are some issues that I think would be right to address:

I appreciate that you improved the error return values from the original
true/false/assert handling.

As much as I hate typedefs, I also like the introduction of
libfsverity_read_fn_t as function pointers are somewhat annoying to deal
with.

My biggest objection is the export of kernel datatypes to userland and I
really don't think using u32/u16/u8 internally in the library adds any
value. I had explicitly converted it to uint32_t/uint16_t/uint8_t in my
version.

max() defined in common/utils.h is not used by anything, so lets get rid
of it.

I also wonder if we should introduce an
libfsverity_get_digest_size(alg_nr) function? It would be useful for a
caller trying to allocate buffers to store them in, to be able to do
this prior to calculating the first digest.

> diff --git a/lib/compute_digest.c b/lib/compute_digest.c
> index b279d63..13998bb 100644
> --- a/lib/compute_digest.c
> +++ b/lib/compute_digest.c
> @@ -1,13 +1,13 @@
... snip ...
> -const struct fsverity_hash_alg *find_hash_alg_by_name(const char *name)
> +LIBEXPORT u32
> +libfsverity_find_hash_alg_by_name(const char *name)

This export of u32 here is problematic.

> diff --git a/lib/sign_digest.c b/lib/sign_digest.c
> index d2b0d53..d856392 100644
> --- a/lib/sign_digest.c
> +++ b/lib/sign_digest.c
> @@ -1,45 +1,68 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  /*
> - * sign_digest.c
> + * Implementation of libfsverity_sign_digest().
>   *
>   * Copyright (C) 2018 Google LLC
> + * Copyright (C) 2020 Facebook
>   */
>  
> -#include "hash_algs.h"
> -#include "sign.h"
> +#include "lib_private.h"
>  
>  #include <limits.h>
>  #include <openssl/bio.h>
>  #include <openssl/err.h>
>  #include <openssl/pem.h>
>  #include <openssl/pkcs7.h>
> +#include <string.h>
> +
> +/*
> + * Format in which verity file measurements are signed.  This is the same as
> + * 'struct fsverity_digest', except here some magic bytes are prepended to
> + * provide some context about what is being signed in case the same key is used
> + * for non-fsverity purposes, and here the fields have fixed endianness.
> + */
> +struct fsverity_signed_digest {
> +	char magic[8];			/* must be "FSVerity" */
> +	__le16 digest_algorithm;
> +	__le16 digest_size;
> +	__u8 digest[];
> +};

I don't really understand why you prefer to manage two versions of the
digest, ie. libfsverity_digest and libfsverity_signed_digest, but it's
not a big deal.

> diff --git a/lib/utils.c b/lib/utils.c
> new file mode 100644
> index 0000000..0684526
> --- /dev/null
> +++ b/lib/utils.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * Utility functions for libfsverity
> + *
> + * Copyright 2020 Google LLC
> + */
> +
> +#define _GNU_SOURCE /* for asprintf() */
> +
> +#include "lib_private.h"
> +
> +#include <stdio.h>
> +#include <stdlib.h>
> +#include <string.h>
> +
> +static void *xmalloc(size_t size)
> +{
> +	void *p = malloc(size);
> +
> +	if (!p)
> +		libfsverity_error_msg("out of memory");
> +	return p;
> +}
> +
> +void *libfsverity_zalloc(size_t size)
> +{
> +	void *p = xmalloc(size);
> +
> +	if (!p)
> +		return NULL;
> +	return memset(p, 0, size);
> +}

I suggest we get rid of xmalloc() and libfsverity_zalloc(). libc
provides perfectly good malloc() and calloc() functions, and printing an
out of memory error in a generic location doesn't tell us where the
error happened. If anything those error messages should be printed by
the calling function IMO.

Reviewed-by: Jes Sorensen <jsorensen@fb.com>

Cheers,
Jes

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/3] Split up cmd_sign.c
  2020-05-15  4:10 ` [PATCH 1/3] Split up cmd_sign.c Eric Biggers
@ 2020-05-21 15:26   ` Jes Sorensen
  0 siblings, 0 replies; 14+ messages in thread
From: Jes Sorensen @ 2020-05-21 15:26 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt; +Cc: jsorensen, kernel-team

On 5/15/20 12:10 AM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> In preparation for moving most of the functionality of 'fsverity sign'
> into a shared library, split up cmd_sign.c into three files:
> 
> - cmd_sign.c: the actual command
> - compute_digest.c: computing the file measurement
> - sign_digest.c: sign the file measurement
> 
> No "real" changes; this is just moving code around.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  Makefile             |   4 +-
>  cmd_sign.c           | 481 +------------------------------------------
>  lib/compute_digest.c | 184 +++++++++++++++++
>  lib/sign_digest.c    | 304 +++++++++++++++++++++++++++
>  sign.h               |  32 +++
>  5 files changed, 523 insertions(+), 482 deletions(-)
>  create mode 100644 lib/compute_digest.c
>  create mode 100644 lib/sign_digest.c
>  create mode 100644 sign.h

I don't really have specific comments to this one, except for the
u8/u16/u32 datatype issue I mentioned in the the 2/3 patch.

Reviewed-by: Jes Sorensen <jsorensen@fb.com>

Thanks,
Jes


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 3/3] Add some basic test programs for libfsverity
  2020-05-15  4:10 ` [PATCH 3/3] Add some basic test programs for libfsverity Eric Biggers
@ 2020-05-21 15:29   ` Jes Sorensen
  0 siblings, 0 replies; 14+ messages in thread
From: Jes Sorensen @ 2020-05-21 15:29 UTC (permalink / raw)
  To: Eric Biggers, linux-fscrypt; +Cc: jsorensen, kernel-team

On 5/15/20 12:10 AM, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Add three test programs: 'test_hash_algs', 'test_compute_digest', and
> 'test_sign_digest'.  Nothing fancy yet, just some basic tests to test
> each library function.
> 
> With the new Makefile, these get run by 'make check'.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  programs/test_compute_digest.c |  54 +++++++++++++++++++++++++++++++++
>  programs/test_hash_algs.c      |  27 +++++++++++++++++
>  programs/test_sign_digest.c    |  44 +++++++++++++++++++++++++++
>  testdata/cert.pem              |  31 +++++++++++++++++++
>  testdata/file.sig              | Bin 0 -> 708 bytes
>  testdata/key.pem               |  52 +++++++++++++++++++++++++++++++
>  6 files changed, 208 insertions(+)
>  create mode 100644 programs/test_compute_digest.c
>  create mode 100644 programs/test_hash_algs.c
>  create mode 100644 programs/test_sign_digest.c
>  create mode 100644 testdata/cert.pem
>  create mode 100644 testdata/file.sig
>  create mode 100644 testdata/key.pem
> 
> diff --git a/programs/test_compute_digest.c b/programs/test_compute_digest.c
> new file mode 100644
> index 0000000..5d00576
> --- /dev/null
> +++ b/programs/test_compute_digest.c
> @@ -0,0 +1,54 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +#include "utils.h"
> +
> +struct file {
> +	u8 *data;
> +	size_t size;
> +	size_t offset;
> +};

The only issue I have here is the use of u8/u16/u32 in userland.

Reviewed-by: Jes Sorensen <jsorensen@fb.com>

Cheers,
Jes


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] Introduce libfsverity
  2020-05-21 15:24   ` Jes Sorensen
@ 2020-05-21 16:08     ` Eric Biggers
  2020-05-21 16:45       ` Jes Sorensen
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-21 16:08 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-fscrypt, jsorensen, kernel-team

On Thu, May 21, 2020 at 11:24:34AM -0400, Jes Sorensen wrote:
> On 5/15/20 12:10 AM, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > From the 'fsverity' program, split out a library 'libfsverity'.
> > Currently it supports computing file measurements ("digests"), and
> > signing those file measurements for use with the fs-verity builtin
> > signature verification feature.
> > 
> > Rewritten from patches by Jes Sorensen <jsorensen@fb.com>.
> > I made a lot of improvements, e.g.:
> > 
> > - Separated library and program source into different directories.
> > - Drastically improved the Makefile.
> > - Added 'make check' target and rules to build test programs.
> > - In the shared lib, only export the functions intended to be public.
> > - Prefixed global functions with "libfsverity_" so that they don't cause
> >   conflicts when the library is built as a static library.
> > - Made library error messages be sent to a user-specified callback
> >   rather than always be printed to stderr.
> > - Keep showing OpenSSL error messages.
> > - Stopped abort()ing in library code, when possible.
> > - Made libfsverity_digest use native endianness.
> > - Moved file_size into the merkle_tree_params.
> > - Made libfsverity_hash_name() just return the static strings.
> > - Made some variables in the API uint32_t instead of uint16_t.
> > - Shared parse_hash_alg_option() between cmd_enable and cmd_sign.
> > - Lots of fixes.
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Eric,
> 
> Here is a more detailed review. The code as we have it seems to work for
> me, but there are some issues that I think would be right to address:

Thanks for the feedback!

> 
> I appreciate that you improved the error return values from the original
> true/false/assert handling.
> 
> As much as I hate typedefs, I also like the introduction of
> libfsverity_read_fn_t as function pointers are somewhat annoying to deal
> with.
> 
> My biggest objection is the export of kernel datatypes to userland and I
> really don't think using u32/u16/u8 internally in the library adds any
> value. I had explicitly converted it to uint32_t/uint16_t/uint8_t in my
> version.

I prefer u64/u32/u16/u8 since they're shorter and easier to read, and it's the
same convention that is used in the kernel code (which is where the other half
of fs-verity is).

Note that these types aren't "exported" to or from anywhere but rather are just
typedefs in common/common_defs.h.  It's just a particular convention.

Also, fsverity-utils is already using this convention prior to this patchset.
If we did decide to change it, then we should change it in all the code, not
just in certain places.

> 
> max() defined in common/utils.h is not used by anything, so lets get rid
> of it.

Yes, I'll do that.

> 
> I also wonder if we should introduce an
> libfsverity_get_digest_size(alg_nr) function? It would be useful for a
> caller trying to allocate buffers to store them in, to be able to do
> this prior to calculating the first digest.

That already exists; it's called libfsverity_digest_size().

Would it be clearer if we renamed:

	libfsverity_digest_size() => libfsverity_get_digest_size()
	libfsverity_hash_name() => libfsverity_get_hash_name()

> > diff --git a/lib/compute_digest.c b/lib/compute_digest.c
> > index b279d63..13998bb 100644
> > --- a/lib/compute_digest.c
> > +++ b/lib/compute_digest.c
> > @@ -1,13 +1,13 @@
> ... snip ...
> > -const struct fsverity_hash_alg *find_hash_alg_by_name(const char *name)
> > +LIBEXPORT u32
> > +libfsverity_find_hash_alg_by_name(const char *name)
> 
> This export of u32 here is problematic.
> 

It's not "exported"; this is a .c file.  As long as we use the stdint.h name in
libfsverity.h (to avoid polluting the library user's namespace), it is okay.
u32 and uint32_t are compatible; they're just different names for the same type.

> > diff --git a/lib/sign_digest.c b/lib/sign_digest.c
> > index d2b0d53..d856392 100644
> > --- a/lib/sign_digest.c
> > +++ b/lib/sign_digest.c
> > @@ -1,45 +1,68 @@
> >  // SPDX-License-Identifier: GPL-2.0+
> >  /*
> > - * sign_digest.c
> > + * Implementation of libfsverity_sign_digest().
> >   *
> >   * Copyright (C) 2018 Google LLC
> > + * Copyright (C) 2020 Facebook
> >   */
> >  
> > -#include "hash_algs.h"
> > -#include "sign.h"
> > +#include "lib_private.h"
> >  
> >  #include <limits.h>
> >  #include <openssl/bio.h>
> >  #include <openssl/err.h>
> >  #include <openssl/pem.h>
> >  #include <openssl/pkcs7.h>
> > +#include <string.h>
> > +
> > +/*
> > + * Format in which verity file measurements are signed.  This is the same as
> > + * 'struct fsverity_digest', except here some magic bytes are prepended to
> > + * provide some context about what is being signed in case the same key is used
> > + * for non-fsverity purposes, and here the fields have fixed endianness.
> > + */
> > +struct fsverity_signed_digest {
> > +	char magic[8];			/* must be "FSVerity" */
> > +	__le16 digest_algorithm;
> > +	__le16 digest_size;
> > +	__u8 digest[];
> > +};
> 
> I don't really understand why you prefer to manage two versions of the
> digest, ie. libfsverity_digest and libfsverity_signed_digest, but it's
> not a big deal.

Because fsverity_signed_digest has a specific endianness, people will access the
fields directly and forget to do the needed endianness conventions -- thus
producing code that doesn't work on big endian systems.  Using a
native-endianness type for the intermediate struct avoids that pitfall.

I think keeping the byte order handling internal to the library is preferable.

> 
> > diff --git a/lib/utils.c b/lib/utils.c
> > new file mode 100644
> > index 0000000..0684526
> > --- /dev/null
> > +++ b/lib/utils.c
> > @@ -0,0 +1,107 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +/*
> > + * Utility functions for libfsverity
> > + *
> > + * Copyright 2020 Google LLC
> > + */
> > +
> > +#define _GNU_SOURCE /* for asprintf() */
> > +
> > +#include "lib_private.h"
> > +
> > +#include <stdio.h>
> > +#include <stdlib.h>
> > +#include <string.h>
> > +
> > +static void *xmalloc(size_t size)
> > +{
> > +	void *p = malloc(size);
> > +
> > +	if (!p)
> > +		libfsverity_error_msg("out of memory");
> > +	return p;
> > +}
> > +
> > +void *libfsverity_zalloc(size_t size)
> > +{
> > +	void *p = xmalloc(size);
> > +
> > +	if (!p)
> > +		return NULL;
> > +	return memset(p, 0, size);
> > +}
> 
> I suggest we get rid of xmalloc() and libfsverity_zalloc(). libc
> provides perfectly good malloc() and calloc() functions, and printing an
> out of memory error in a generic location doesn't tell us where the
> error happened. If anything those error messages should be printed by
> the calling function IMO.
> 

Maybe.  I'm not sure knowing the specific allocation sites would be useful
enough to make all the callers handle printing the error message (which is
easily forgotten about).  We could also add the allocation size that failed to
the message here.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] Introduce libfsverity
  2020-05-21 16:08     ` Eric Biggers
@ 2020-05-21 16:45       ` Jes Sorensen
  2020-05-21 16:59         ` Eric Biggers
  0 siblings, 1 reply; 14+ messages in thread
From: Jes Sorensen @ 2020-05-21 16:45 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-fscrypt, jsorensen, kernel-team

On 5/21/20 12:08 PM, Eric Biggers wrote:
> On Thu, May 21, 2020 at 11:24:34AM -0400, Jes Sorensen wrote:

>> Eric,
>>
>> Here is a more detailed review. The code as we have it seems to work for
>> me, but there are some issues that I think would be right to address:
> 
> Thanks for the feedback!
> 
>>
>> I appreciate that you improved the error return values from the original
>> true/false/assert handling.
>>
>> As much as I hate typedefs, I also like the introduction of
>> libfsverity_read_fn_t as function pointers are somewhat annoying to deal
>> with.
>>
>> My biggest objection is the export of kernel datatypes to userland and I
>> really don't think using u32/u16/u8 internally in the library adds any
>> value. I had explicitly converted it to uint32_t/uint16_t/uint8_t in my
>> version.
> 
> I prefer u64/u32/u16/u8 since they're shorter and easier to read, and it's the
> same convention that is used in the kernel code (which is where the other half
> of fs-verity is).

I like them too, but I tend to live in kernel space.

> Note that these types aren't "exported" to or from anywhere but rather are just
> typedefs in common/common_defs.h.  It's just a particular convention.
> 
> Also, fsverity-utils is already using this convention prior to this patchset.
> If we did decide to change it, then we should change it in all the code, not
> just in certain places.

I thought I did it everywhere in my patch set?

>> I also wonder if we should introduce an
>> libfsverity_get_digest_size(alg_nr) function? It would be useful for a
>> caller trying to allocate buffers to store them in, to be able to do
>> this prior to calculating the first digest.
> 
> That already exists; it's called libfsverity_digest_size().
> 
> Would it be clearer if we renamed:
> 
> 	libfsverity_digest_size() => libfsverity_get_digest_size()
> 	libfsverity_hash_name() => libfsverity_get_hash_name()

Oh I missed you added that. Probably a good idea to rename them for
consistency.

>>> diff --git a/lib/compute_digest.c b/lib/compute_digest.c
>>> index b279d63..13998bb 100644
>>> --- a/lib/compute_digest.c
>>> +++ b/lib/compute_digest.c
>>> @@ -1,13 +1,13 @@
>> ... snip ...
>>> -const struct fsverity_hash_alg *find_hash_alg_by_name(const char *name)
>>> +LIBEXPORT u32
>>> +libfsverity_find_hash_alg_by_name(const char *name)
>>
>> This export of u32 here is problematic.
> 
> It's not "exported"; this is a .c file.  As long as we use the stdint.h name in
> libfsverity.h (to avoid polluting the library user's namespace), it is okay.
> u32 and uint32_t are compatible; they're just different names for the same type.

I would still keep it consistent avoid relying on assumptions that types
are identical.

>>> +struct fsverity_signed_digest {
>>> +	char magic[8];			/* must be "FSVerity" */
>>> +	__le16 digest_algorithm;
>>> +	__le16 digest_size;
>>> +	__u8 digest[];
>>> +};
>>
>> I don't really understand why you prefer to manage two versions of the
>> digest, ie. libfsverity_digest and libfsverity_signed_digest, but it's
>> not a big deal.
> 
> Because fsverity_signed_digest has a specific endianness, people will access the
> fields directly and forget to do the needed endianness conventions -- thus
> producing code that doesn't work on big endian systems.  Using a
> native-endianness type for the intermediate struct avoids that pitfall.
> 
> I think keeping the byte order handling internal to the library is preferable.

Fair enough

>>> +static void *xmalloc(size_t size)
>>> +{
>>> +	void *p = malloc(size);
>>> +
>>> +	if (!p)
>>> +		libfsverity_error_msg("out of memory");
>>> +	return p;
>>> +}
>>> +
>>> +void *libfsverity_zalloc(size_t size)
>>> +{
>>> +	void *p = xmalloc(size);
>>> +
>>> +	if (!p)
>>> +		return NULL;
>>> +	return memset(p, 0, size);
>>> +}
>>
>> I suggest we get rid of xmalloc() and libfsverity_zalloc(). libc
>> provides perfectly good malloc() and calloc() functions, and printing an
>> out of memory error in a generic location doesn't tell us where the
>> error happened. If anything those error messages should be printed by
>> the calling function IMO.
>>
> 
> Maybe.  I'm not sure knowing the specific allocation sites would be useful
> enough to make all the callers handle printing the error message (which is
> easily forgotten about).  We could also add the allocation size that failed to
> the message here.

My point is mostly at this point, this just adds code obfuscation rather
than adding real value. I can see how it was useful during initial
development.

Cheers,
Jes



^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] Introduce libfsverity
  2020-05-21 16:45       ` Jes Sorensen
@ 2020-05-21 16:59         ` Eric Biggers
  2020-05-21 17:13           ` Jes Sorensen
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2020-05-21 16:59 UTC (permalink / raw)
  To: Jes Sorensen; +Cc: linux-fscrypt, jsorensen, kernel-team

On Thu, May 21, 2020 at 12:45:57PM -0400, Jes Sorensen wrote:
> >> My biggest objection is the export of kernel datatypes to userland and I
> >> really don't think using u32/u16/u8 internally in the library adds any
> >> value. I had explicitly converted it to uint32_t/uint16_t/uint8_t in my
> >> version.
> > 
> > I prefer u64/u32/u16/u8 since they're shorter and easier to read, and it's the
> > same convention that is used in the kernel code (which is where the other half
> > of fs-verity is).
> 
> I like them too, but I tend to live in kernel space.
> 
> > Note that these types aren't "exported" to or from anywhere but rather are just
> > typedefs in common/common_defs.h.  It's just a particular convention.
> > 
> > Also, fsverity-utils is already using this convention prior to this patchset.
> > If we did decide to change it, then we should change it in all the code, not
> > just in certain places.
> 
> I thought I did it everywhere in my patch set?

No, they were still left in various places.

> 
> >> I also wonder if we should introduce an
> >> libfsverity_get_digest_size(alg_nr) function? It would be useful for a
> >> caller trying to allocate buffers to store them in, to be able to do
> >> this prior to calculating the first digest.
> > 
> > That already exists; it's called libfsverity_digest_size().
> > 
> > Would it be clearer if we renamed:
> > 
> > 	libfsverity_digest_size() => libfsverity_get_digest_size()
> > 	libfsverity_hash_name() => libfsverity_get_hash_name()
> 
> Oh I missed you added that. Probably a good idea to rename them for
> consistency.

libfsverity_digest_size() was actually in your patchset too.

I'll add the "get" to the names so that all function names start with a verb.

> >>> +static void *xmalloc(size_t size)
> >>> +{
> >>> +	void *p = malloc(size);
> >>> +
> >>> +	if (!p)
> >>> +		libfsverity_error_msg("out of memory");
> >>> +	return p;
> >>> +}
> >>> +
> >>> +void *libfsverity_zalloc(size_t size)
> >>> +{
> >>> +	void *p = xmalloc(size);
> >>> +
> >>> +	if (!p)
> >>> +		return NULL;
> >>> +	return memset(p, 0, size);
> >>> +}
> >>
> >> I suggest we get rid of xmalloc() and libfsverity_zalloc(). libc
> >> provides perfectly good malloc() and calloc() functions, and printing an
> >> out of memory error in a generic location doesn't tell us where the
> >> error happened. If anything those error messages should be printed by
> >> the calling function IMO.
> >>
> > 
> > Maybe.  I'm not sure knowing the specific allocation sites would be useful
> > enough to make all the callers handle printing the error message (which is
> > easily forgotten about).  We could also add the allocation size that failed to
> > the message here.
> 
> My point is mostly at this point, this just adds code obfuscation rather
> than adding real value. I can see how it was useful during initial
> development.

It's helpful to eliminate the need for callers to print the error message.

We also still need libfsverity_memdup() anyway, unless we hard-code
malloc() + memcpy().

I also had in mind that we'd follow the (increasingly recommended) practice of
initializing all heap memory.  This can be done by only providing allocation
functions that initialize memory.

I'll think about it.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 2/3] Introduce libfsverity
  2020-05-21 16:59         ` Eric Biggers
@ 2020-05-21 17:13           ` Jes Sorensen
  0 siblings, 0 replies; 14+ messages in thread
From: Jes Sorensen @ 2020-05-21 17:13 UTC (permalink / raw)
  To: Eric Biggers; +Cc: linux-fscrypt, jsorensen, kernel-team

On 5/21/20 12:59 PM, Eric Biggers wrote:
> On Thu, May 21, 2020 at 12:45:57PM -0400, Jes Sorensen wrote:
>>>> My biggest objection is the export of kernel datatypes to userland and I
>>>> really don't think using u32/u16/u8 internally in the library adds any
>>>> value. I had explicitly converted it to uint32_t/uint16_t/uint8_t in my
>>>> version.
>>>
>>> I prefer u64/u32/u16/u8 since they're shorter and easier to read, and it's the
>>> same convention that is used in the kernel code (which is where the other half
>>> of fs-verity is).
>>
>> I like them too, but I tend to live in kernel space.
>>
>>> Note that these types aren't "exported" to or from anywhere but rather are just
>>> typedefs in common/common_defs.h.  It's just a particular convention.
>>>
>>> Also, fsverity-utils is already using this convention prior to this patchset.
>>> If we did decide to change it, then we should change it in all the code, not
>>> just in certain places.
>>
>> I thought I did it everywhere in my patch set?
> 
> No, they were still left in various places.

I see, I thought I had only left the __u32 ones in place, but just goes
to show I didn't do my job properly.

>>>> I also wonder if we should introduce an
>>>> libfsverity_get_digest_size(alg_nr) function? It would be useful for a
>>>> caller trying to allocate buffers to store them in, to be able to do
>>>> this prior to calculating the first digest.
>>>
>>> That already exists; it's called libfsverity_digest_size().
>>>
>>> Would it be clearer if we renamed:
>>>
>>> 	libfsverity_digest_size() => libfsverity_get_digest_size()
>>> 	libfsverity_hash_name() => libfsverity_get_hash_name()
>>
>> Oh I missed you added that. Probably a good idea to rename them for
>> consistency.
> 
> libfsverity_digest_size() was actually in your patchset too.

Whoops egg on face :)

> I'll add the "get" to the names so that all function names start with a verb.

Sounds good!

>>>>> +static void *xmalloc(size_t size)
>>>>> +{
>>>>> +	void *p = malloc(size);
>>>>> +
>>>>> +	if (!p)
>>>>> +		libfsverity_error_msg("out of memory");
>>>>> +	return p;
>>>>> +}
>>>>> +
>>>>> +void *libfsverity_zalloc(size_t size)
>>>>> +{
>>>>> +	void *p = xmalloc(size);
>>>>> +
>>>>> +	if (!p)
>>>>> +		return NULL;
>>>>> +	return memset(p, 0, size);
>>>>> +}
>>>>
>>>> I suggest we get rid of xmalloc() and libfsverity_zalloc(). libc
>>>> provides perfectly good malloc() and calloc() functions, and printing an
>>>> out of memory error in a generic location doesn't tell us where the
>>>> error happened. If anything those error messages should be printed by
>>>> the calling function IMO.
>>>>
>>>
>>> Maybe.  I'm not sure knowing the specific allocation sites would be useful
>>> enough to make all the callers handle printing the error message (which is
>>> easily forgotten about).  We could also add the allocation size that failed to
>>> the message here.
>>
>> My point is mostly at this point, this just adds code obfuscation rather
>> than adding real value. I can see how it was useful during initial
>> development.
> 
> It's helpful to eliminate the need for callers to print the error message.
> 
> We also still need libfsverity_memdup() anyway, unless we hard-code
> malloc() + memcpy().
> 
> I also had in mind that we'd follow the (increasingly recommended) practice of
> initializing all heap memory.  This can be done by only providing allocation
> functions that initialize memory.
> 
> I'll think about it.

It's not a showstopper, my primary interest is a working release :)

Cheers,
Jes



^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2020-05-21 17:13 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-15  4:10 [PATCH 0/3] fsverity-utils: introduce libfsverity Eric Biggers
2020-05-15  4:10 ` [PATCH 1/3] Split up cmd_sign.c Eric Biggers
2020-05-21 15:26   ` Jes Sorensen
2020-05-15  4:10 ` [PATCH 2/3] Introduce libfsverity Eric Biggers
2020-05-21 15:24   ` Jes Sorensen
2020-05-21 16:08     ` Eric Biggers
2020-05-21 16:45       ` Jes Sorensen
2020-05-21 16:59         ` Eric Biggers
2020-05-21 17:13           ` Jes Sorensen
2020-05-15  4:10 ` [PATCH 3/3] Add some basic test programs for libfsverity Eric Biggers
2020-05-21 15:29   ` Jes Sorensen
2020-05-15 20:50 ` [PATCH 0/3] fsverity-utils: introduce libfsverity Jes Sorensen
2020-05-20  3:06   ` Eric Biggers
2020-05-20 13:26     ` Jes Sorensen

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).