diff mbox series

[3/9] Move fsverity_descriptor definition to libfsverity.h

Message ID 20200312214758.343212-4-Jes.Sorensen@gmail.com (mailing list archive)
State Superseded
Headers show
Series Split fsverity-utils into a shared library | expand

Commit Message

Jes Sorensen March 12, 2020, 9:47 p.m. UTC 
From: Jes Sorensen <jsorensen@fb.com>

Signed-off-by: Jes Sorensen <jsorensen@fb.com>
---
 cmd_sign.c    | 19 +------------------
 libfsverity.h | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 19 deletions(-)

Comments

Eric Biggers March 22, 2020, 4:57 a.m. UTC | #1 
On Thu, Mar 12, 2020 at 05:47:52PM -0400, Jes Sorensen wrote:
> From: Jes Sorensen <jsorensen@fb.com>
> 
> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
> ---
>  cmd_sign.c    | 19 +------------------
>  libfsverity.h | 26 +++++++++++++++++++++++++-
>  2 files changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/cmd_sign.c b/cmd_sign.c
> index dcc44f8..1792084 100644
> --- a/cmd_sign.c
> +++ b/cmd_sign.c
> @@ -20,26 +20,9 @@
>  #include <unistd.h>
>  
>  #include "commands.h"
> -#include "fsverity_uapi.h"
> +#include "libfsverity.h"
>  #include "hash_algs.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
> diff --git a/libfsverity.h b/libfsverity.h
> index ceebae1..396a6ee 100644
> --- a/libfsverity.h
> +++ b/libfsverity.h
> @@ -13,13 +13,14 @@
>  
>  #include <stddef.h>
>  #include <stdint.h>
> +#include <linux/types.h>
>  
>  #define FS_VERITY_HASH_ALG_SHA256       1
>  #define FS_VERITY_HASH_ALG_SHA512       2
>  
>  struct libfsverity_merkle_tree_params {
>  	uint16_t version;
> -	uint16_t hash_algorithm;
> +	uint16_t hash_algorithm;	/* Matches the digest_algorithm type */
>  	uint32_t block_size;
>  	uint32_t salt_size;
>  	const uint8_t *salt;
> @@ -27,6 +28,7 @@ struct libfsverity_merkle_tree_params {
>  };
>  
>  struct libfsverity_digest {
> +	char magic[8];			/* must be "FSVerity" */
>  	uint16_t digest_algorithm;
>  	uint16_t digest_size;
>  	uint8_t digest[];
> @@ -38,4 +40,26 @@ struct libfsverity_signature_params {
>  	uint64_t reserved[11];
>  };
>  
> +/*
> + * 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 {
> +	uint8_t version;	/* must be 1 */
> +	uint8_t hash_algorithm;	/* Merkle tree hash algorithm */
> +	uint8_t log_blocksize;	/* log2 of size of data and tree blocks */
> +	uint8_t 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 */
> +	uint8_t root_hash[64];	/* Merkle tree root hash */
> +	uint8_t salt[32];	/* salt prepended to each hashed block */
> +	uint8_t __reserved[144];/* must be 0's */
> +	uint8_t signature[];	/* optional PKCS#7 signature */
> +};
> +

I thought there was no need for this to be part of the library API?

- Eric
Jes Sorensen April 21, 2020, 4:07 p.m. UTC | #2 
On 3/22/20 12:57 AM, Eric Biggers wrote:
> On Thu, Mar 12, 2020 at 05:47:52PM -0400, Jes Sorensen wrote:
>> From: Jes Sorensen <jsorensen@fb.com>
>>
>> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
>> ---
>>  cmd_sign.c    | 19 +------------------
>>  libfsverity.h | 26 +++++++++++++++++++++++++-
>>  2 files changed, 26 insertions(+), 19 deletions(-)
>>
>> diff --git a/cmd_sign.c b/cmd_sign.c
>> index dcc44f8..1792084 100644
>> --- a/cmd_sign.c
>> +++ b/cmd_sign.c
>> @@ -20,26 +20,9 @@
>>  #include <unistd.h>
>>  
>>  #include "commands.h"
>> -#include "fsverity_uapi.h"
>> +#include "libfsverity.h"
>>  #include "hash_algs.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
>> diff --git a/libfsverity.h b/libfsverity.h
>> index ceebae1..396a6ee 100644
>> --- a/libfsverity.h
>> +++ b/libfsverity.h
>> @@ -13,13 +13,14 @@
>>  
>>  #include <stddef.h>
>>  #include <stdint.h>
>> +#include <linux/types.h>
>>  
>>  #define FS_VERITY_HASH_ALG_SHA256       1
>>  #define FS_VERITY_HASH_ALG_SHA512       2
>>  
>>  struct libfsverity_merkle_tree_params {
>>  	uint16_t version;
>> -	uint16_t hash_algorithm;
>> +	uint16_t hash_algorithm;	/* Matches the digest_algorithm type */
>>  	uint32_t block_size;
>>  	uint32_t salt_size;
>>  	const uint8_t *salt;
>> @@ -27,6 +28,7 @@ struct libfsverity_merkle_tree_params {
>>  };
>>  
>>  struct libfsverity_digest {
>> +	char magic[8];			/* must be "FSVerity" */
>>  	uint16_t digest_algorithm;
>>  	uint16_t digest_size;
>>  	uint8_t digest[];
>> @@ -38,4 +40,26 @@ struct libfsverity_signature_params {
>>  	uint64_t reserved[11];
>>  };
>>  
>> +/*
>> + * 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 {
>> +	uint8_t version;	/* must be 1 */
>> +	uint8_t hash_algorithm;	/* Merkle tree hash algorithm */
>> +	uint8_t log_blocksize;	/* log2 of size of data and tree blocks */
>> +	uint8_t 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 */
>> +	uint8_t root_hash[64];	/* Merkle tree root hash */
>> +	uint8_t salt[32];	/* salt prepended to each hashed block */
>> +	uint8_t __reserved[144];/* must be 0's */
>> +	uint8_t signature[];	/* optional PKCS#7 signature */
>> +};
>> +
> 
> I thought there was no need for this to be part of the library API?

Hi Eric,

Been busy working on RPM support, but looking at this again now. Given
that the fsverity signature is a hash of the descriptor, I don't see how
we can avoid this?

Cheers,
Jes
Eric Biggers April 21, 2020, 4:16 p.m. UTC | #3 
On Tue, Apr 21, 2020 at 12:07:07PM -0400, Jes Sorensen wrote:
> On 3/22/20 12:57 AM, Eric Biggers wrote:
> > On Thu, Mar 12, 2020 at 05:47:52PM -0400, Jes Sorensen wrote:
> >> From: Jes Sorensen <jsorensen@fb.com>
> >>
> >> Signed-off-by: Jes Sorensen <jsorensen@fb.com>
> >> ---
> >>  cmd_sign.c    | 19 +------------------
> >>  libfsverity.h | 26 +++++++++++++++++++++++++-
> >>  2 files changed, 26 insertions(+), 19 deletions(-)
> >>
> >> diff --git a/cmd_sign.c b/cmd_sign.c
> >> index dcc44f8..1792084 100644
> >> --- a/cmd_sign.c
> >> +++ b/cmd_sign.c
> >> @@ -20,26 +20,9 @@
> >>  #include <unistd.h>
> >>  
> >>  #include "commands.h"
> >> -#include "fsverity_uapi.h"
> >> +#include "libfsverity.h"
> >>  #include "hash_algs.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
> >> diff --git a/libfsverity.h b/libfsverity.h
> >> index ceebae1..396a6ee 100644
> >> --- a/libfsverity.h
> >> +++ b/libfsverity.h
> >> @@ -13,13 +13,14 @@
> >>  
> >>  #include <stddef.h>
> >>  #include <stdint.h>
> >> +#include <linux/types.h>
> >>  
> >>  #define FS_VERITY_HASH_ALG_SHA256       1
> >>  #define FS_VERITY_HASH_ALG_SHA512       2
> >>  
> >>  struct libfsverity_merkle_tree_params {
> >>  	uint16_t version;
> >> -	uint16_t hash_algorithm;
> >> +	uint16_t hash_algorithm;	/* Matches the digest_algorithm type */
> >>  	uint32_t block_size;
> >>  	uint32_t salt_size;
> >>  	const uint8_t *salt;
> >> @@ -27,6 +28,7 @@ struct libfsverity_merkle_tree_params {
> >>  };
> >>  
> >>  struct libfsverity_digest {
> >> +	char magic[8];			/* must be "FSVerity" */
> >>  	uint16_t digest_algorithm;
> >>  	uint16_t digest_size;
> >>  	uint8_t digest[];
> >> @@ -38,4 +40,26 @@ struct libfsverity_signature_params {
> >>  	uint64_t reserved[11];
> >>  };
> >>  
> >> +/*
> >> + * 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 {
> >> +	uint8_t version;	/* must be 1 */
> >> +	uint8_t hash_algorithm;	/* Merkle tree hash algorithm */
> >> +	uint8_t log_blocksize;	/* log2 of size of data and tree blocks */
> >> +	uint8_t 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 */
> >> +	uint8_t root_hash[64];	/* Merkle tree root hash */
> >> +	uint8_t salt[32];	/* salt prepended to each hashed block */
> >> +	uint8_t __reserved[144];/* must be 0's */
> >> +	uint8_t signature[];	/* optional PKCS#7 signature */
> >> +};
> >> +
> > 
> > I thought there was no need for this to be part of the library API?
> 
> Hi Eric,
> 
> Been busy working on RPM support, but looking at this again now. Given
> that the fsverity signature is a hash of the descriptor, I don't see how
> we can avoid this?
> 

struct fsverity_descriptor isn't signed directly; it's hashed as an intermediate
step in libfsverity_compute_digest().  So why would the library user need the
definition of 'struct fsverity_descriptor'?

- Eric
Jes Sorensen April 21, 2020, 4:20 p.m. UTC | #4 
On 4/21/20 12:16 PM, Eric Biggers wrote:
> On Tue, Apr 21, 2020 at 12:07:07PM -0400, Jes Sorensen wrote:
>> On 3/22/20 12:57 AM, Eric Biggers wrote:
>>> I thought there was no need for this to be part of the library API?
>>
>> Hi Eric,
>>
>> Been busy working on RPM support, but looking at this again now. Given
>> that the fsverity signature is a hash of the descriptor, I don't see how
>> we can avoid this?
>>
> 
> struct fsverity_descriptor isn't signed directly; it's hashed as an intermediate
> step in libfsverity_compute_digest().  So why would the library user need the
> definition of 'struct fsverity_descriptor'?

Hi Eric,

You're right, I actually moved it to libfsverity_private.h already, but
it's in the new patches I am working on.

I pushed it all to git.kernel.org, but I still need to address some of
the issues you responded about. I'll post an update to this when I have
worked through your list of comments. Most noticeable is that I had to
rework the read API to make it work with RPM, but you can find my
current tree here (libfsverity branch):
https://git.kernel.org/pub/scm/linux/kernel/git/jes/fsverity-utils.git/

Current RPM work is here:
https://github.com/jessorensen/rpm/tree/rpm-fsverity

Cheers,
Jes
diff mbox series

Patch

diff --git a/cmd_sign.c b/cmd_sign.c
index dcc44f8..1792084 100644
--- a/cmd_sign.c
+++ b/cmd_sign.c
@@ -20,26 +20,9 @@ 
 #include <unistd.h>
 
 #include "commands.h"
-#include "fsverity_uapi.h"
+#include "libfsverity.h"
 #include "hash_algs.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
diff --git a/libfsverity.h b/libfsverity.h
index ceebae1..396a6ee 100644
--- a/libfsverity.h
+++ b/libfsverity.h
@@ -13,13 +13,14 @@ 
 
 #include <stddef.h>
 #include <stdint.h>
+#include <linux/types.h>
 
 #define FS_VERITY_HASH_ALG_SHA256       1
 #define FS_VERITY_HASH_ALG_SHA512       2
 
 struct libfsverity_merkle_tree_params {
 	uint16_t version;
-	uint16_t hash_algorithm;
+	uint16_t hash_algorithm;	/* Matches the digest_algorithm type */
 	uint32_t block_size;
 	uint32_t salt_size;
 	const uint8_t *salt;
@@ -27,6 +28,7 @@  struct libfsverity_merkle_tree_params {
 };
 
 struct libfsverity_digest {
+	char magic[8];			/* must be "FSVerity" */
 	uint16_t digest_algorithm;
 	uint16_t digest_size;
 	uint8_t digest[];
@@ -38,4 +40,26 @@  struct libfsverity_signature_params {
 	uint64_t reserved[11];
 };
 
+/*
+ * 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 {
+	uint8_t version;	/* must be 1 */
+	uint8_t hash_algorithm;	/* Merkle tree hash algorithm */
+	uint8_t log_blocksize;	/* log2 of size of data and tree blocks */
+	uint8_t 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 */
+	uint8_t root_hash[64];	/* Merkle tree root hash */
+	uint8_t salt[32];	/* salt prepended to each hashed block */
+	uint8_t __reserved[144];/* must be 0's */
+	uint8_t signature[];	/* optional PKCS#7 signature */
+};
+
+int
+libfsverity_compute_digest(int fd,
+			   const struct libfsverity_merkle_tree_params *params,
+			   struct libfsverity_digest **digest_ret);
+
 #endif