| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <DU2PR04MB86303F82A10F5989DE0A0F7395419@DU2PR04MB8630.eurprd04.prod.outlook.com>
Date: Wed, 7 Sep 2022 07:22:51 +0000
From: Pankaj Gupta <pankaj.gupta@....com>
To: Ben Boeckel <me@...boeckel.net>
CC: "jarkko@...nel.org" <jarkko@...nel.org>,
"a.fatoum@...gutronix.de" <a.fatoum@...gutronix.de>,
"Jason@...c4.com" <Jason@...c4.com>,
"jejb@...ux.ibm.com" <jejb@...ux.ibm.com>,
"zohar@...ux.ibm.com" <zohar@...ux.ibm.com>,
"dhowells@...hat.com" <dhowells@...hat.com>,
"sumit.garg@...aro.org" <sumit.garg@...aro.org>,
"david@...ma-star.at" <david@...ma-star.at>,
"michael@...le.cc" <michael@...le.cc>,
"john.ernberg@...ia.se" <john.ernberg@...ia.se>,
"jmorris@...ei.org" <jmorris@...ei.org>,
"serge@...lyn.com" <serge@...lyn.com>,
"herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
"davem@...emloft.net" <davem@...emloft.net>,
"j.luebbe@...gutronix.de" <j.luebbe@...gutronix.de>,
"ebiggers@...nel.org" <ebiggers@...nel.org>,
"richard@....at" <richard@....at>,
"keyrings@...r.kernel.org" <keyrings@...r.kernel.org>,
"linux-crypto@...r.kernel.org" <linux-crypto@...r.kernel.org>,
"linux-integrity@...r.kernel.org" <linux-integrity@...r.kernel.org>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-security-module@...r.kernel.org"
<linux-security-module@...r.kernel.org>,
Sahil Malhotra <sahil.malhotra@....com>,
Kshitiz Varshney <kshitiz.varshney@....com>,
Horia Geanta <horia.geanta@....com>,
Varun Sethi <V.Sethi@....com>
Subject: RE: [EXT] Re: [RFC PATCH HBK: 1/8] keys-trusted: new cmd line option
added
> -----Original Message-----
> From: Ben Boeckel <me@...boeckel.net>
> Sent: Tuesday, September 6, 2022 6:32 PM
> To: Pankaj Gupta <pankaj.gupta@....com>
> Cc: jarkko@...nel.org; a.fatoum@...gutronix.de; Jason@...c4.com;
> jejb@...ux.ibm.com; zohar@...ux.ibm.com; dhowells@...hat.com;
> sumit.garg@...aro.org; david@...ma-star.at; michael@...le.cc;
> john.ernberg@...ia.se; jmorris@...ei.org; serge@...lyn.com;
> herbert@...dor.apana.org.au; davem@...emloft.net;
> j.luebbe@...gutronix.de; ebiggers@...nel.org; richard@....at;
> keyrings@...r.kernel.org; linux-crypto@...r.kernel.org; linux-
> integrity@...r.kernel.org; linux-kernel@...r.kernel.org; linux-security-
> module@...r.kernel.org; Sahil Malhotra <sahil.malhotra@....com>; Kshitiz
> Varshney <kshitiz.varshney@....com>; Horia Geanta
> <horia.geanta@....com>; Varun Sethi <V.Sethi@....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@....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@...gutronix.de>
> > + * Copyright 2022 NXP, Pankaj Gupta <pankaj.gupta@....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
Powered by blists - more mailing lists