Re:[RFC 1/1] Add dm verity root hash pkcs7 sig validation.

2019-06-03 Thread jaskarankhurana




On Tue, 21 May 2019, Milan Broz wrote:

On 20/05/2019 23:54, Jaskaran Khurana wrote:

Adds in-kernel pkcs7 signature checking for the roothash of
the dm-verity hash tree.


Adds DM_VERITY_VERIFY_ROOTHASH_SIG_FORCE: roothash signature *must* be
specified for all dm verity volumes and verification must succeed prior
to creation of device mapper block device.


I am not sure this is a good idea. If I understand it correctly, this will
block creating another dm-verity mappings without PKCS7 signature, and these
are used in many other environments and applications that could possibly
run on that system later.

(But I have no idea how to solve it better though :-)

...


+   /* Root hash signature is  a optional parameter*/
+   r = verity_verify_root_hash(root_hash_digest_to_validate,
+   strlen(root_hash_digest_to_validate),
+   verify_args.sig,
+   verify_args.sig_size);
+   if (r < 0) {
+   ti->error = "Root hash verification failed";
+   goto bad;
+   }


You are sending the PKCS7 signature as a (quite large) binary blob inside the 
mapping table.

I am not sure if it is possible here (I guess so), but why not put this it 
kernel keyring
and then just reference it from mapping table?
(We use kernel keyring in libcryptsetup already for dm-crypt.)

It will also solve an issue in userspace patch, when you are reading the 
signature
file too late (devices can be suspended in that moment, so I would prefer to 
download
sig file to keyring in advance, and then just reference it in mapping table).

(I guess you will send merge request for veritysetup userspace part later.)


I have made the changes for passing the signature bytes using the keyring 
and I had sent an updated patch for the same last week. I have given a 
link to the veritysetup changes which I used to test this in the patch and I will 
cleanup and send those for review next.


Please take a look and provide code review feedback for the kernel 
changes.




Milan


Regards,
Jaskaran


Re: [RFC 1/1] Add dm verity root hash pkcs7 sig validation.

2019-05-20 Thread Singh, Balbir



On 5/21/19 7:54 AM, Jaskaran Khurana wrote:
> Adds in-kernel pkcs7 signature checking for the roothash of
> the dm-verity hash tree.
> 
> The verification is to support cases where the roothash is not secured by
> Trusted Boot, UEFI Secureboot or similar technologies.
> One of the use cases for this is for dm-verity volumes mounted after boot,
> the root hash provided during the creation of the dm-verity volume has to
> be secure and thus in-kernel validation implemented here will be used
> before we trust the root hash and allow the block device to be created.
> 
The first patch was your cover letter, I'd suggest name it that way in
the subject.

> The signature being provided for verification must verify the root hash and 
> must be trusted by the builtin keyring for verification to succeed.
> 
> Adds DM_VERITY_VERIFY_ROOTHASH_SIG: roothash verification
> against the roothash signature file *if* specified, if signature file is
> specified verification must succeed prior to creation of device mapper 
> block device.
> 
> Adds DM_VERITY_VERIFY_ROOTHASH_SIG_FORCE: roothash signature *must* be
> specified for all dm verity volumes and verification must succeed prior
> to creation of device mapper block device.
> 
> Signed-off-by: Jaskaran Khurana 
> ---
>  drivers/md/Kconfig|  23 ++
>  drivers/md/Makefile   |   2 +-
>  drivers/md/dm-verity-target.c |  44 --
>  drivers/md/dm-verity-verify-sig.c | 129 ++
>  drivers/md/dm-verity-verify-sig.h |  32 
>  5 files changed, 222 insertions(+), 8 deletions(-)
>  create mode 100644 drivers/md/dm-verity-verify-sig.c
>  create mode 100644 drivers/md/dm-verity-verify-sig.h
> 
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index db269a348b20..da4115753f25 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -489,6 +489,29 @@ config DM_VERITY
>  
> If unsure, say N.
>  
> +config DM_VERITY_VERIFY_ROOTHASH_SIG
> + def_bool n
> + bool "Verity data device root hash signature verification support"
> + depends on DM_VERITY
> + select SYSTEM_DATA_VERIFICATION
> +   help
> +   The device mapper target created by DM-VERITY can be validated if the
> +   pre-generated tree of cryptographic checksums passed has a pkcs#7
> +   signature file that can validate the roothash of the tree.
> +
> +   If unsure, say N.
> +
> +config DM_VERITY_VERIFY_ROOTHASH_SIG_FORCE
> + def_bool n
> + bool "Forces all dm verity data device root hash should be signed"
> + depends on DM_VERITY_VERIFY_ROOTHASH_SIG
> +   help
> +   The device mapper target created by DM-VERITY will succeed only if the
> +   pre-generated tree of cryptographic checksums passed also has a pkcs#7
> +   signature file that can validate the roothash of the tree.
> +
> +   If unsure, say N.
> +
>  config DM_VERITY_FEC
>   bool "Verity forward error correction support"
>   depends on DM_VERITY
> diff --git a/drivers/md/Makefile b/drivers/md/Makefile
> index be7a6eb92abc..8a8c142bcfe1 100644
> --- a/drivers/md/Makefile
> +++ b/drivers/md/Makefile
> @@ -61,7 +61,7 @@ obj-$(CONFIG_DM_LOG_USERSPACE)  += dm-log-userspace.o
>  obj-$(CONFIG_DM_ZERO)+= dm-zero.o
>  obj-$(CONFIG_DM_RAID)+= dm-raid.o
>  obj-$(CONFIG_DM_THIN_PROVISIONING)   += dm-thin-pool.o
> -obj-$(CONFIG_DM_VERITY)  += dm-verity.o
> +obj-$(CONFIG_DM_VERITY)  += dm-verity.o dm-verity-verify-sig.o
>  obj-$(CONFIG_DM_CACHE)   += dm-cache.o
>  obj-$(CONFIG_DM_CACHE_SMQ)   += dm-cache-smq.o
>  obj-$(CONFIG_DM_ERA) += dm-era.o
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index f4c31ffaa88e..53aebfa8bc38 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -16,7 +16,7 @@
>  
>  #include "dm-verity.h"
>  #include "dm-verity-fec.h"
> -
> +#include "dm-verity-verify-sig.h"
>  #include 
>  #include 
>  
> @@ -34,7 +34,11 @@
>  #define DM_VERITY_OPT_IGN_ZEROES "ignore_zero_blocks"
>  #define DM_VERITY_OPT_AT_MOST_ONCE   "check_at_most_once"
>  
> -#define DM_VERITY_OPTS_MAX   (2 + DM_VERITY_OPTS_FEC)
> +#define DM_VERITY_OPTS_MAX   (2 + DM_VERITY_OPTS_FEC + \
> +  DM_VERITY_ROOT_HASH_VERIFICATION_OPTS)
> +
> +#define DM_VERITY_MANDATORY_ARGS10
> +
>  
>  static unsigned dm_verity_prefetch_cluster = DM_VERITY_DEFAULT_PREFETCH_SIZE;
>  
> @@ -855,7 +859,8 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
>   return r;
>  }
>  
> -static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v)
> +static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
> +  struct dm_verity_sig_opts *verify_args)
>  {
>   int r;
>   unsigned argc;
> @@ -904,6 +909,15 @@ static int verity_parse_opt_args(struct dm_arg_set *as, 
> struct