| Message ID | 20220906065157.10662-2-pankaj.gupta@nxp.com |
|---|---|
| State | New |
| Headers | show |
| Series | HW BOUND KEY as TRUSTED KEY | expand |
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
> -----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 --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:
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(+)