diff mbox series

[RFC,HBK:,1/8] keys-trusted: new cmd line option added

Message ID 20220906065157.10662-2-pankaj.gupta@nxp.com
State New
Headers show
Series HW BOUND KEY as TRUSTED KEY | expand

Commit Message

Pankaj Gupta Sept. 6, 2022, 6:51 a.m. UTC
Two changes are done:
- new cmd line option "hw" needs to be suffix, to generate the
  hw bound key.
  for ex:
   $:> keyctl add trusted <KEYNAME> 'new 32 hw' @s
   $:> keyctl add trusted <KEYNAME> 'load $(cat <KEY_BLOB_FILE_NAME>) hw' @s

- For "new", generating the hw bounded trusted key, updating the input key
  length as part of seal operation as well.

Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
---
 include/keys/trusted-type.h               |  2 ++
 security/keys/trusted-keys/trusted_caam.c |  6 ++++++
 security/keys/trusted-keys/trusted_core.c | 14 ++++++++++++++
 3 files changed, 22 insertions(+)

Comments

Ben Boeckel Sept. 6, 2022, 1:01 p.m. UTC | #1
On Tue, Sep 06, 2022 at 12:21:50 +0530, Pankaj Gupta wrote:
> Two changes are done:
> - new cmd line option "hw" needs to be suffix, to generate the
>   hw bound key.
>   for ex:
>    $:> keyctl add trusted <KEYNAME> 'new 32 hw' @s
>    $:> keyctl add trusted <KEYNAME> 'load $(cat <KEY_BLOB_FILE_NAME>) hw' @s
> 
> - For "new", generating the hw bounded trusted key, updating the input key
>   length as part of seal operation as well.
> 
> Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> ---
>  include/keys/trusted-type.h               |  2 ++
>  security/keys/trusted-keys/trusted_caam.c |  6 ++++++
>  security/keys/trusted-keys/trusted_core.c | 14 ++++++++++++++
>  3 files changed, 22 insertions(+)
> 
> diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> index 4eb64548a74f..064266b936c7 100644
> --- a/include/keys/trusted-type.h
> +++ b/include/keys/trusted-type.h
> @@ -22,6 +22,7 @@
>  #define MAX_BLOB_SIZE			512
>  #define MAX_PCRINFO_SIZE		64
>  #define MAX_DIGEST_SIZE			64
> +#define HW_BOUND_KEY                    1
>  
>  struct trusted_key_payload {
>  	struct rcu_head rcu;
> @@ -29,6 +30,7 @@ struct trusted_key_payload {
>  	unsigned int blob_len;
>  	unsigned char migratable;
>  	unsigned char old_format;
> +	unsigned char is_hw_bound;
>  	unsigned char key[MAX_KEY_SIZE + 1];
>  	unsigned char blob[MAX_BLOB_SIZE];
>  };
> diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
> index e3415c520c0a..fceb9a271c4d 100644
> --- a/security/keys/trusted-keys/trusted_caam.c
> +++ b/security/keys/trusted-keys/trusted_caam.c
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0-only
>  /*
>   * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
> + * Copyright 2022 NXP, Pankaj Gupta <pankaj.gupta@nxp.com>
>   */
>  
>  #include <keys/trusted_caam.h>
> @@ -23,6 +24,7 @@ static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
>  		.input  = p->key,  .input_len   = p->key_len,
>  		.output = p->blob, .output_len  = MAX_BLOB_SIZE,
>  		.key_mod = KEYMOD, .key_mod_len = sizeof(KEYMOD) - 1,
> +		.is_hw_bound = p->is_hw_bound,
>  	};
>  
>  	ret = caam_encap_blob(blobifier, &info);
> @@ -30,6 +32,9 @@ static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
>  		return ret;
>  
>  	p->blob_len = info.output_len;
> +	if (p->is_hw_bound)
> +		p->key_len = info.input_len;
> +
>  	return 0;
>  }
>  
> @@ -40,6 +45,7 @@ static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
>  		.input   = p->blob,  .input_len  = p->blob_len,
>  		.output  = p->key,   .output_len = MAX_KEY_SIZE,
>  		.key_mod = KEYMOD,  .key_mod_len = sizeof(KEYMOD) - 1,
> +		.is_hw_bound = p->is_hw_bound,
>  	};
>  
>  	ret = caam_decap_blob(blobifier, &info);
> diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
> index c6fc50d67214..7f7cc2551b92 100644
> --- a/security/keys/trusted-keys/trusted_core.c
> +++ b/security/keys/trusted-keys/trusted_core.c
> @@ -79,6 +79,8 @@ static int datablob_parse(char **datablob, struct trusted_key_payload *p)
>  	int key_cmd;
>  	char *c;
>  
> +	p->is_hw_bound = !HW_BOUND_KEY;

This seems…backwards to me.

> @@ -94,6 +96,12 @@ static int datablob_parse(char **datablob, struct trusted_key_payload *p)
>  		if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
>  			return -EINVAL;
>  		p->key_len = keylen;
> +		/* second argument is to determine if tied to HW */
> +		c = strsep(datablob, " \t");
> +		if (c) {
> +			if (strcmp(c, "hw") == 0)
> +				p->is_hw_bound = HW_BOUND_KEY;
> +		}

Userspace documentation is missing for this new field. Must it always be
second or is it "any following argument"? For example, let's say we have
another flag like this for "FIPS" (or whatever). It'd be nice if these
all worked:

    'new 32 fips hw'
    'new 32 fips'
    'new 32 hw fips'
    'new 32 hw'

> @@ -107,6 +115,12 @@ static int datablob_parse(char **datablob, struct trusted_key_payload *p)
>  		ret = hex2bin(p->blob, c, p->blob_len);
>  		if (ret < 0)
>  			return -EINVAL;
> +		/* second argument is to determine if tied to HW */
> +		c = strsep(datablob, " \t");
> +		if (c) {
> +			if (strcmp(c, "hw") == 0)
> +				p->is_hw_bound = HW_BOUND_KEY;
> +		}

Same here.

--Ben
Pankaj Gupta Sept. 7, 2022, 7:22 a.m. UTC | #2
> -----Original Message-----
> From: Ben Boeckel <me@benboeckel.net>
> Sent: Tuesday, September 6, 2022 6:32 PM
> To: Pankaj Gupta <pankaj.gupta@nxp.com>
> Cc: jarkko@kernel.org; a.fatoum@pengutronix.de; Jason@zx2c4.com;
> jejb@linux.ibm.com; zohar@linux.ibm.com; dhowells@redhat.com;
> sumit.garg@linaro.org; david@sigma-star.at; michael@walle.cc;
> john.ernberg@actia.se; jmorris@namei.org; serge@hallyn.com;
> herbert@gondor.apana.org.au; davem@davemloft.net;
> j.luebbe@pengutronix.de; ebiggers@kernel.org; richard@nod.at;
> keyrings@vger.kernel.org; linux-crypto@vger.kernel.org; linux-
> integrity@vger.kernel.org; linux-kernel@vger.kernel.org; linux-security-
> module@vger.kernel.org; Sahil Malhotra <sahil.malhotra@nxp.com>; Kshitiz
> Varshney <kshitiz.varshney@nxp.com>; Horia Geanta
> <horia.geanta@nxp.com>; Varun Sethi <V.Sethi@nxp.com>
> Subject: [EXT] Re: [RFC PATCH HBK: 1/8] keys-trusted: new cmd line option
> added
> 
> Caution: EXT Email
> 
> On Tue, Sep 06, 2022 at 12:21:50 +0530, Pankaj Gupta wrote:
> > Two changes are done:
> > - new cmd line option "hw" needs to be suffix, to generate the
> >   hw bound key.
> >   for ex:
> >    $:> keyctl add trusted <KEYNAME> 'new 32 hw' @s
> >    $:> keyctl add trusted <KEYNAME> 'load $(cat <KEY_BLOB_FILE_NAME>)
> > hw' @s
> >
> > - For "new", generating the hw bounded trusted key, updating the input
> key
> >   length as part of seal operation as well.
> >
> > Signed-off-by: Pankaj Gupta <pankaj.gupta@nxp.com>
> > ---
> >  include/keys/trusted-type.h               |  2 ++
> >  security/keys/trusted-keys/trusted_caam.c |  6 ++++++
> > security/keys/trusted-keys/trusted_core.c | 14 ++++++++++++++
> >  3 files changed, 22 insertions(+)
> >
> > diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
> > index 4eb64548a74f..064266b936c7 100644
> > --- a/include/keys/trusted-type.h
> > +++ b/include/keys/trusted-type.h
> > @@ -22,6 +22,7 @@
> >  #define MAX_BLOB_SIZE                        512
> >  #define MAX_PCRINFO_SIZE             64
> >  #define MAX_DIGEST_SIZE                      64
> > +#define HW_BOUND_KEY                    1
> >
> >  struct trusted_key_payload {
> >       struct rcu_head rcu;
> > @@ -29,6 +30,7 @@ struct trusted_key_payload {
> >       unsigned int blob_len;
> >       unsigned char migratable;
> >       unsigned char old_format;
> > +     unsigned char is_hw_bound;
> >       unsigned char key[MAX_KEY_SIZE + 1];
> >       unsigned char blob[MAX_BLOB_SIZE];  }; diff --git
> > a/security/keys/trusted-keys/trusted_caam.c
> > b/security/keys/trusted-keys/trusted_caam.c
> > index e3415c520c0a..fceb9a271c4d 100644
> > --- a/security/keys/trusted-keys/trusted_caam.c
> > +++ b/security/keys/trusted-keys/trusted_caam.c
> > @@ -1,6 +1,7 @@
> >  // SPDX-License-Identifier: GPL-2.0-only
> >  /*
> >   * Copyright (C) 2021 Pengutronix, Ahmad Fatoum
> > <kernel@pengutronix.de>
> > + * Copyright 2022 NXP, Pankaj Gupta <pankaj.gupta@nxp.com>
> >   */
> >
> >  #include <keys/trusted_caam.h>
> > @@ -23,6 +24,7 @@ static int trusted_caam_seal(struct
> trusted_key_payload *p, char *datablob)
> >               .input  = p->key,  .input_len   = p->key_len,
> >               .output = p->blob, .output_len  = MAX_BLOB_SIZE,
> >               .key_mod = KEYMOD, .key_mod_len = sizeof(KEYMOD) - 1,
> > +             .is_hw_bound = p->is_hw_bound,
> >       };
> >
> >       ret = caam_encap_blob(blobifier, &info); @@ -30,6 +32,9 @@
> > static int trusted_caam_seal(struct trusted_key_payload *p, char
> *datablob)
> >               return ret;
> >
> >       p->blob_len = info.output_len;
> > +     if (p->is_hw_bound)
> > +             p->key_len = info.input_len;
> > +
> >       return 0;
> >  }
> >
> > @@ -40,6 +45,7 @@ static int trusted_caam_unseal(struct
> trusted_key_payload *p, char *datablob)
> >               .input   = p->blob,  .input_len  = p->blob_len,
> >               .output  = p->key,   .output_len = MAX_KEY_SIZE,
> >               .key_mod = KEYMOD,  .key_mod_len = sizeof(KEYMOD) - 1,
> > +             .is_hw_bound = p->is_hw_bound,
> >       };
> >
> >       ret = caam_decap_blob(blobifier, &info); diff --git
> > a/security/keys/trusted-keys/trusted_core.c
> > b/security/keys/trusted-keys/trusted_core.c
> > index c6fc50d67214..7f7cc2551b92 100644
> > --- a/security/keys/trusted-keys/trusted_core.c
> > +++ b/security/keys/trusted-keys/trusted_core.c
> > @@ -79,6 +79,8 @@ static int datablob_parse(char **datablob, struct
> trusted_key_payload *p)
> >       int key_cmd;
> >       char *c;
> >
> > +     p->is_hw_bound = !HW_BOUND_KEY;
> 
> This seems…backwards to me.
>
Initialized it to be a plain key & not a HW bounded key.
 
> > @@ -94,6 +96,12 @@ static int datablob_parse(char **datablob, struct
> trusted_key_payload *p)
> >               if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
> >                       return -EINVAL;
> >               p->key_len = keylen;
> > +             /* second argument is to determine if tied to HW */
> > +             c = strsep(datablob, " \t");
> > +             if (c) {
> > +                     if (strcmp(c, "hw") == 0)
> > +                             p->is_hw_bound = HW_BOUND_KEY;
> > +             }
> 
> Userspace documentation is missing for this new field. Must it always be
> second or is it "any following argument"? For example, let's say we have
> another flag like this for "FIPS" (or whatever). It'd be nice if these all worked:
> 
>     'new 32 fips hw'
>     'new 32 fips'
>     'new 32 hw fips'
>     'new 32 hw'
> 
Will consider this, in the next version of this patch set.

> > @@ -107,6 +115,12 @@ static int datablob_parse(char **datablob, struct
> trusted_key_payload *p)
> >               ret = hex2bin(p->blob, c, p->blob_len);
> >               if (ret < 0)
> >                       return -EINVAL;
> > +             /* second argument is to determine if tied to HW */
> > +             c = strsep(datablob, " \t");
> > +             if (c) {
> > +                     if (strcmp(c, "hw") == 0)
> > +                             p->is_hw_bound = HW_BOUND_KEY;
> > +             }
> 
> Same here.
> 
> --Ben
diff mbox series

Patch

diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h
index 4eb64548a74f..064266b936c7 100644
--- a/include/keys/trusted-type.h
+++ b/include/keys/trusted-type.h
@@ -22,6 +22,7 @@ 
 #define MAX_BLOB_SIZE			512
 #define MAX_PCRINFO_SIZE		64
 #define MAX_DIGEST_SIZE			64
+#define HW_BOUND_KEY                    1
 
 struct trusted_key_payload {
 	struct rcu_head rcu;
@@ -29,6 +30,7 @@  struct trusted_key_payload {
 	unsigned int blob_len;
 	unsigned char migratable;
 	unsigned char old_format;
+	unsigned char is_hw_bound;
 	unsigned char key[MAX_KEY_SIZE + 1];
 	unsigned char blob[MAX_BLOB_SIZE];
 };
diff --git a/security/keys/trusted-keys/trusted_caam.c b/security/keys/trusted-keys/trusted_caam.c
index e3415c520c0a..fceb9a271c4d 100644
--- a/security/keys/trusted-keys/trusted_caam.c
+++ b/security/keys/trusted-keys/trusted_caam.c
@@ -1,6 +1,7 @@ 
 // SPDX-License-Identifier: GPL-2.0-only
 /*
  * Copyright (C) 2021 Pengutronix, Ahmad Fatoum <kernel@pengutronix.de>
+ * Copyright 2022 NXP, Pankaj Gupta <pankaj.gupta@nxp.com>
  */
 
 #include <keys/trusted_caam.h>
@@ -23,6 +24,7 @@  static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
 		.input  = p->key,  .input_len   = p->key_len,
 		.output = p->blob, .output_len  = MAX_BLOB_SIZE,
 		.key_mod = KEYMOD, .key_mod_len = sizeof(KEYMOD) - 1,
+		.is_hw_bound = p->is_hw_bound,
 	};
 
 	ret = caam_encap_blob(blobifier, &info);
@@ -30,6 +32,9 @@  static int trusted_caam_seal(struct trusted_key_payload *p, char *datablob)
 		return ret;
 
 	p->blob_len = info.output_len;
+	if (p->is_hw_bound)
+		p->key_len = info.input_len;
+
 	return 0;
 }
 
@@ -40,6 +45,7 @@  static int trusted_caam_unseal(struct trusted_key_payload *p, char *datablob)
 		.input   = p->blob,  .input_len  = p->blob_len,
 		.output  = p->key,   .output_len = MAX_KEY_SIZE,
 		.key_mod = KEYMOD,  .key_mod_len = sizeof(KEYMOD) - 1,
+		.is_hw_bound = p->is_hw_bound,
 	};
 
 	ret = caam_decap_blob(blobifier, &info);
diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c
index c6fc50d67214..7f7cc2551b92 100644
--- a/security/keys/trusted-keys/trusted_core.c
+++ b/security/keys/trusted-keys/trusted_core.c
@@ -79,6 +79,8 @@  static int datablob_parse(char **datablob, struct trusted_key_payload *p)
 	int key_cmd;
 	char *c;
 
+	p->is_hw_bound = !HW_BOUND_KEY;
+
 	/* main command */
 	c = strsep(datablob, " \t");
 	if (!c)
@@ -94,6 +96,12 @@  static int datablob_parse(char **datablob, struct trusted_key_payload *p)
 		if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE)
 			return -EINVAL;
 		p->key_len = keylen;
+		/* second argument is to determine if tied to HW */
+		c = strsep(datablob, " \t");
+		if (c) {
+			if (strcmp(c, "hw") == 0)
+				p->is_hw_bound = HW_BOUND_KEY;
+		}
 		ret = Opt_new;
 		break;
 	case Opt_load:
@@ -107,6 +115,12 @@  static int datablob_parse(char **datablob, struct trusted_key_payload *p)
 		ret = hex2bin(p->blob, c, p->blob_len);
 		if (ret < 0)
 			return -EINVAL;
+		/* second argument is to determine if tied to HW */
+		c = strsep(datablob, " \t");
+		if (c) {
+			if (strcmp(c, "hw") == 0)
+				p->is_hw_bound = HW_BOUND_KEY;
+		}
 		ret = Opt_load;
 		break;
 	case Opt_update: