On Mon, 1 Jun 2020 at 07:41, Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> wrote: > > On Wed, May 06, 2020 at 03:10:14PM +0530, Sumit Garg wrote: > > Current trusted keys framework is tightly coupled to use TPM device as > > an underlying implementation which makes it difficult for implementations > > like Trusted Execution Environment (TEE) etc. to provide trusked keys > > support in case platform doesn't posses a TPM device. > > > > So this patch tries to add generic trusted keys framework where underlying > > implemtations like TPM, TEE etc. could be easily plugged-in. > > > > Suggested-by: Jarkko Sakkinen <jarkko.sakki...@linux.intel.com> > > Signed-off-by: Sumit Garg <sumit.g...@linaro.org> > > --- > > include/keys/trusted-type.h | 45 ++++ > > include/keys/trusted_tpm.h | 15 -- > > security/keys/trusted-keys/Makefile | 1 + > > security/keys/trusted-keys/trusted_common.c | 333 > > +++++++++++++++++++++++++++ > > security/keys/trusted-keys/trusted_tpm1.c | 335 > > +++++----------------------- > > 5 files changed, 437 insertions(+), 292 deletions(-) > > create mode 100644 security/keys/trusted-keys/trusted_common.c > > > > diff --git a/include/keys/trusted-type.h b/include/keys/trusted-type.h > > index a94c03a..5559010 100644 > > --- a/include/keys/trusted-type.h > > +++ b/include/keys/trusted-type.h > > @@ -40,6 +40,51 @@ struct trusted_key_options { > > uint32_t policyhandle; > > }; > > > > +struct trusted_key_ops { > > + /* > > + * flag to indicate if trusted key implementation supports migration > > + * or not. > > + */ > > + unsigned char migratable; > > + > > + /* trusted key init */ > > + int (*init)(void); > > + > > + /* seal a trusted key */ > > + int (*seal)(struct trusted_key_payload *p, char *datablob); > > + > > + /* unseal a trusted key */ > > + int (*unseal)(struct trusted_key_payload *p, char *datablob); > > + > > + /* get random trusted key */ > > + int (*get_random)(unsigned char *key, size_t key_len); > > + > > + /* trusted key cleanup */ > > + void (*cleanup)(void); > > +}; > > + > > extern struct key_type key_type_trusted; > > +#if defined(CONFIG_TCG_TPM) > > +extern struct trusted_key_ops tpm_trusted_key_ops; > > +#endif > > + > > +#define TRUSTED_DEBUG 0 > > + > > +#if TRUSTED_DEBUG > > +static inline void dump_payload(struct trusted_key_payload *p) > > +{ > > + pr_info("trusted_key: key_len %d\n", p->key_len); > > + print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE, > > + 16, 1, p->key, p->key_len, 0); > > + pr_info("trusted_key: bloblen %d\n", p->blob_len); > > + print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE, > > + 16, 1, p->blob, p->blob_len, 0); > > + pr_info("trusted_key: migratable %d\n", p->migratable); > > +} > > +#else > > +static inline void dump_payload(struct trusted_key_payload *p) > > +{ > > +} > > +#endif > > > > #endif /* _KEYS_TRUSTED_TYPE_H */ > > diff --git a/include/keys/trusted_tpm.h b/include/keys/trusted_tpm.h > > index a56d8e1..5753231 100644 > > --- a/include/keys/trusted_tpm.h > > +++ b/include/keys/trusted_tpm.h > > @@ -60,17 +60,6 @@ static inline void dump_options(struct > > trusted_key_options *o) > > 16, 1, o->pcrinfo, o->pcrinfo_len, 0); > > } > > > > -static inline void dump_payload(struct trusted_key_payload *p) > > -{ > > - pr_info("trusted_key: key_len %d\n", p->key_len); > > - print_hex_dump(KERN_INFO, "key ", DUMP_PREFIX_NONE, > > - 16, 1, p->key, p->key_len, 0); > > - pr_info("trusted_key: bloblen %d\n", p->blob_len); > > - print_hex_dump(KERN_INFO, "blob ", DUMP_PREFIX_NONE, > > - 16, 1, p->blob, p->blob_len, 0); > > - pr_info("trusted_key: migratable %d\n", p->migratable); > > -} > > - > > static inline void dump_sess(struct osapsess *s) > > { > > print_hex_dump(KERN_INFO, "trusted-key: handle ", DUMP_PREFIX_NONE, > > @@ -96,10 +85,6 @@ static inline void dump_options(struct > > trusted_key_options *o) > > { > > } > > > > -static inline void dump_payload(struct trusted_key_payload *p) > > -{ > > -} > > - > > static inline void dump_sess(struct osapsess *s) > > { > > } > > diff --git a/security/keys/trusted-keys/Makefile > > b/security/keys/trusted-keys/Makefile > > index 7b73ceb..2b1085b 100644 > > --- a/security/keys/trusted-keys/Makefile > > +++ b/security/keys/trusted-keys/Makefile > > @@ -4,5 +4,6 @@ > > # > > > > obj-$(CONFIG_TRUSTED_KEYS) += trusted.o > > +trusted-y += trusted_common.o > > trusted-y += trusted_tpm1.o > > trusted-y += trusted_tpm2.o > > diff --git a/security/keys/trusted-keys/trusted_common.c > > b/security/keys/trusted-keys/trusted_common.c > > new file mode 100644 > > index 0000000..9bfd081 > > --- /dev/null > > +++ b/security/keys/trusted-keys/trusted_common.c > > @@ -0,0 +1,333 @@ > > +// SPDX-License-Identifier: GPL-2.0-only > > +/* > > + * Copyright (C) 2010 IBM Corporation > > + * Copyright (c) 2019, Linaro Limited > > + * > > + * Author: > > + * David Safford <saff...@us.ibm.com> > > + * Added generic trusted key framework: Sumit Garg <sumit.g...@linaro.org> > > + * > > + * See Documentation/security/keys/trusted-encrypted.rst > > + */ > > + > > +#include <keys/user-type.h> > > +#include <keys/trusted-type.h> > > +#include <linux/capability.h> > > +#include <linux/err.h> > > +#include <linux/init.h> > > +#include <linux/key-type.h> > > +#include <linux/module.h> > > +#include <linux/parser.h> > > +#include <linux/rcupdate.h> > > +#include <linux/slab.h> > > +#include <linux/string.h> > > +#include <linux/uaccess.h> > > + > > +static struct trusted_key_ops *available_tk_ops[] = { > > +#if defined(CONFIG_TCG_TPM) > > + &tpm_trusted_key_ops, > > +#endif > > +}; > > This, I think is wrong. You should have a compile time flag for TPM e.g. > CONFIG_TRUSTED_TPM, not this dynamic mess. >
The whole idea to have it dynamic was to have a common trusted keys module which could support both TPM and TEE implementation depending on hardware. I guess it may be useful in scenarios where a particular hardware supports a TPM chip while other doesn't but both need to run a common kernel image. > Please make the whole choice compile time, not run-time. > > > +static struct trusted_key_ops *tk_ops; > > + > > +enum { > > + Opt_err, > > + Opt_new, Opt_load, Opt_update, > > +}; > > + > > +static const match_table_t key_tokens = { > > + {Opt_new, "new"}, > > + {Opt_load, "load"}, > > + {Opt_update, "update"}, > > + {Opt_err, NULL} > > +}; > > + > > +/* > > + * datablob_parse - parse the keyctl data and fill in the > > + * payload structure > > + * > > + * On success returns 0, otherwise -EINVAL. > > + */ > > +static int datablob_parse(char *datablob, struct trusted_key_payload *p) > > +{ > > + substring_t args[MAX_OPT_ARGS]; > > + long keylen; > > + int ret = -EINVAL; > > + int key_cmd; > > + char *c; > > + > > + /* main command */ > > + c = strsep(&datablob, " \t"); > > + if (!c) > > + return -EINVAL; > > + key_cmd = match_token(c, key_tokens, args); > > + switch (key_cmd) { > > + case Opt_new: > > + /* first argument is key size */ > > + c = strsep(&datablob, " \t"); > > + if (!c) > > + return -EINVAL; > > + ret = kstrtol(c, 10, &keylen); > > + if (ret < 0 || keylen < MIN_KEY_SIZE || keylen > MAX_KEY_SIZE) > > + return -EINVAL; > > + p->key_len = keylen; > > + ret = Opt_new; > > + break; > > + case Opt_load: > > + /* first argument is sealed blob */ > > + c = strsep(&datablob, " \t"); > > + if (!c) > > + return -EINVAL; > > + p->blob_len = strlen(c) / 2; > > + if (p->blob_len > MAX_BLOB_SIZE) > > + return -EINVAL; > > + ret = hex2bin(p->blob, c, p->blob_len); > > + if (ret < 0) > > + return -EINVAL; > > + ret = Opt_load; > > + break; > > + case Opt_update: > > + ret = Opt_update; > > + break; > > + case Opt_err: > > + return -EINVAL; > > + } > > + return ret; > > +} > > + > > +static struct trusted_key_payload *trusted_payload_alloc(struct key *key) > > +{ > > + struct trusted_key_payload *p = NULL; > > + int ret; > > + > > + ret = key_payload_reserve(key, sizeof(*p)); > > + if (ret < 0) > > + return p; > > + p = kzalloc(sizeof(*p), GFP_KERNEL); > > + > > + p->migratable = tk_ops->migratable; > > + > > + return p; > > +} > > + > > +/* > > + * trusted_instantiate - create a new trusted key > > + * > > + * Unseal an existing trusted blob or, for a new key, get a > > + * random key, then seal and create a trusted key-type key, > > + * adding it to the specified keyring. > > + * > > + * On success, return 0. Otherwise return errno. > > + */ > > +static int trusted_instantiate(struct key *key, > > + struct key_preparsed_payload *prep) > > +{ > > + struct trusted_key_payload *payload = NULL; > > + size_t datalen = prep->datalen; > > + char *datablob; > > + int ret = 0; > > + int key_cmd; > > + size_t key_len; > > + > > + if (datalen <= 0 || datalen > 32767 || !prep->data) > > + return -EINVAL; > > + > > + datablob = kmalloc(datalen + 1, GFP_KERNEL); > > + if (!datablob) > > + return -ENOMEM; > > + memcpy(datablob, prep->data, datalen); > > + datablob[datalen] = '\0'; > > + > > + payload = trusted_payload_alloc(key); > > + if (!payload) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + key_cmd = datablob_parse(datablob, payload); > > + if (key_cmd < 0) { > > + ret = key_cmd; > > + goto out; > > + } > > + > > + dump_payload(payload); > > + > > + switch (key_cmd) { > > + case Opt_load: > > + ret = tk_ops->unseal(payload, datablob); > > + dump_payload(payload); > > + if (ret < 0) > > + pr_info("trusted_key: key_unseal failed (%d)\n", ret); > > + break; > > + case Opt_new: > > + key_len = payload->key_len; > > + ret = tk_ops->get_random(payload->key, key_len); > > + if (ret != key_len) { > > + pr_info("trusted_key: key_create failed (%d)\n", ret); > > + goto out; > > + } > > + > > + ret = tk_ops->seal(payload, datablob); > > + if (ret < 0) > > + pr_info("trusted_key: key_seal failed (%d)\n", ret); > > + break; > > + default: > > + ret = -EINVAL; > > + } > > +out: > > + kzfree(datablob); > > + if (!ret) > > + rcu_assign_keypointer(key, payload); > > + else > > + kzfree(payload); > > + return ret; > > +} > > + > > +static void trusted_rcu_free(struct rcu_head *rcu) > > +{ > > + struct trusted_key_payload *p; > > + > > + p = container_of(rcu, struct trusted_key_payload, rcu); > > + kzfree(p); > > +} > > + > > +/* > > + * trusted_update - reseal an existing key with new PCR values > > + */ > > +static int trusted_update(struct key *key, struct key_preparsed_payload > > *prep) > > +{ > > + struct trusted_key_payload *p; > > + struct trusted_key_payload *new_p; > > + size_t datalen = prep->datalen; > > + char *datablob; > > + int ret = 0; > > + > > + if (key_is_negative(key)) > > + return -ENOKEY; > > + p = key->payload.data[0]; > > + if (!p->migratable) > > + return -EPERM; > > + if (datalen <= 0 || datalen > 32767 || !prep->data) > > + return -EINVAL; > > + > > + datablob = kmalloc(datalen + 1, GFP_KERNEL); > > + if (!datablob) > > + return -ENOMEM; > > + > > + new_p = trusted_payload_alloc(key); > > + if (!new_p) { > > + ret = -ENOMEM; > > + goto out; > > + } > > + > > + memcpy(datablob, prep->data, datalen); > > + datablob[datalen] = '\0'; > > + ret = datablob_parse(datablob, new_p); > > + if (ret != Opt_update) { > > + ret = -EINVAL; > > + kzfree(new_p); > > + goto out; > > + } > > + > > + /* copy old key values, and reseal with new pcrs */ > > + new_p->migratable = p->migratable; > > + new_p->key_len = p->key_len; > > + memcpy(new_p->key, p->key, p->key_len); > > + dump_payload(p); > > + dump_payload(new_p); > > + > > + ret = tk_ops->seal(new_p, datablob); > > + if (ret < 0) { > > + pr_info("trusted_key: key_seal failed (%d)\n", ret); > > + kzfree(new_p); > > + goto out; > > + } > > + > > + rcu_assign_keypointer(key, new_p); > > + call_rcu(&p->rcu, trusted_rcu_free); > > +out: > > + kzfree(datablob); > > + return ret; > > +} > > + > > +/* > > + * trusted_read - copy the sealed blob data to userspace in hex. > > + * On success, return to userspace the trusted key datablob size. > > + */ > > +static long trusted_read(const struct key *key, char *buffer, > > + size_t buflen) > > +{ > > + const struct trusted_key_payload *p; > > + char *bufp; > > + int i; > > + > > + p = dereference_key_locked(key); > > + if (!p) > > + return -EINVAL; > > + > > + if (buffer && buflen >= 2 * p->blob_len) { > > + bufp = buffer; > > + for (i = 0; i < p->blob_len; i++) > > + bufp = hex_byte_pack(bufp, p->blob[i]); > > + } > > + return 2 * p->blob_len; > > +} > > + > > +/* > > + * trusted_destroy - clear and free the key's payload > > + */ > > +static void trusted_destroy(struct key *key) > > +{ > > + kzfree(key->payload.data[0]); > > +} > > + > > +struct key_type key_type_trusted = { > > + .name = "trusted", > > + .instantiate = trusted_instantiate, > > + .update = trusted_update, > > + .destroy = trusted_destroy, > > + .describe = user_describe, > > + .read = trusted_read, > > +}; > > +EXPORT_SYMBOL_GPL(key_type_trusted); > > + > > +static int __init init_trusted(void) > > +{ > > + int i, ret = 0; > > + > > + for (i = 0; i < sizeof(available_tk_ops); i++) { > > + tk_ops = available_tk_ops[i]; > > + > > + if (!(tk_ops && tk_ops->init && tk_ops->seal && > > + tk_ops->unseal && tk_ops->get_random)) > > + continue; > > This check should not exist as there is no legit case for any of these > callbacks missing. Please remove it. Okay. > > > + > > + ret = tk_ops->init(); > > + if (ret) { > > + if (tk_ops->cleanup) > > + tk_ops->cleanup(); > > Why is clean up called? What is "clean up"? Init should take care clean > up its dirt if it fails. Please remove the calll to clean up from here. Makes sense, will remove it. -Sumit > > /Jarkko