From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS, UNWANTED_LANGUAGE_BODY,URIBL_BLOCKED autolearn=unavailable autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id C2771C433DF for ; Tue, 13 Oct 2020 11:02:12 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 7549A21D7F for ; Tue, 13 Oct 2020 11:02:12 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="Y6h6Uijg" Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2387804AbgJMLCM (ORCPT ); Tue, 13 Oct 2020 07:02:12 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:60498 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730076AbgJMLCK (ORCPT ); Tue, 13 Oct 2020 07:02:10 -0400 Received: from mail-lf1-x143.google.com (mail-lf1-x143.google.com [IPv6:2a00:1450:4864:20::143]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 5D080C0613D6 for ; Tue, 13 Oct 2020 04:02:09 -0700 (PDT) Received: by mail-lf1-x143.google.com with SMTP id c141so14987310lfg.5 for ; Tue, 13 Oct 2020 04:02:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=z+F5TIpvBycYxYvhVh3hVnn+ZbSdcvydLVz/6hEafIw=; b=Y6h6Uijgr2v4ijkpkhMVQoLSbpCR26gaKYCeqb+KSsw9ucdgLSAHMwA4i4b/ZtAgxI RhGOD6MM/WnLFw3RKDdawIpXbAYG5X9L2klsC6gnCtYi2tbxb05PPurw9XOYdKgIXdj+ SNrQp3Cs/XIVDx7q7zLcN7CY/EUaekaJXdKPP7GTdQN+cvIIrUzVsDo9sahJVOrF/GWO wvaAhmetMoazaFpJ1IFveB+Dibb/wvFeJALDyGQLcWL6SK4HQ/mXVC+LMjfGNqhMJrbf IH3NWM03q74RB5Y1lMW2sjrdfaOJR62pZ5qrdTp+ktECtOHMNnCiwC52BsIzsRDmjsmC LCVg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=z+F5TIpvBycYxYvhVh3hVnn+ZbSdcvydLVz/6hEafIw=; b=bxQIWFUEPjZBcoSFYj1kpwucsGeaeb6lW2gMGNM5cSRZr5KLA9FmzffmopYEyrjILl gHhbgHj9hzekqj/JGMgfKqPK0WBYeYgsDJNwA+yCRHERYpGkWJx7jK5WA0wNkZEkkcad EeqWQ5ngSRHlvgt0iqJoG7u/qvG8dFtlqVzkJmqh+yPjDjIk70TqSTfUABedDQNL/OuS UgOcxNQc+RRl5u3vmodfuiChKd7ArrvBT30BLT5FA+ulq0j46GMB0BRn6JN1euMKlxii 5xJtcreiP0UZXFupg3VI9IQ2udFnNE7/CHf1xmcTCTm0UA384Bvgd1OQH6AFIvf3XMqp IszQ== X-Gm-Message-State: AOAM531ZHuXNiMubsc5XQO0VpoK4meJD7yOmTcDSeBhgwFgFtsarC2DG /yCVLuAs7+4qFQJY9Fplv+y13+b/BWoSGSJdTkJHxQ== X-Google-Smtp-Source: ABdhPJz6HrmxHG8/P4Okqhij9ZwOKw4XPANKC2O6p5RY+QZHHTpRvs/6HcY6xF0zAP4ns2TIwaZ5rZeBey1CWdRLDrw= X-Received: by 2002:a19:5cb:: with SMTP id 194mr6949698lff.205.1602586927502; Tue, 13 Oct 2020 04:02:07 -0700 (PDT) MIME-Version: 1.0 References: <1602065268-26017-1-git-send-email-sumit.garg@linaro.org> <1602065268-26017-3-git-send-email-sumit.garg@linaro.org> <20201013015230.GA45644@linux.intel.com> In-Reply-To: <20201013015230.GA45644@linux.intel.com> From: Sumit Garg Date: Tue, 13 Oct 2020 16:31:56 +0530 Message-ID: Subject: Re: [PATCH v7 2/4] KEYS: trusted: Introduce TEE based Trusted Keys To: Jarkko Sakkinen Cc: Mimi Zohar , James Bottomley , David Howells , Jens Wiklander , Jonathan Corbet , James Morris , "Serge E. Hallyn" , Casey Schaufler , Janne Karhunen , Daniel Thompson , Markus Wamser , Luke Hinds , "open list:ASYMMETRIC KEYS" , linux-integrity@vger.kernel.org, linux-security-module@vger.kernel.org, Linux Doc Mailing List , Linux Kernel Mailing List , linux-arm-kernel , op-tee@lists.trustedfirmware.org Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-doc@vger.kernel.org On Tue, 13 Oct 2020 at 07:22, Jarkko Sakkinen wrote: > > On Wed, Oct 07, 2020 at 03:37:46PM +0530, Sumit Garg wrote: > > Add support for TEE based trusted keys where TEE provides the functionality > > to seal and unseal trusted keys using hardware unique key. > > > > Refer to Documentation/tee.txt for detailed information about TEE. > > > > Signed-off-by: Sumit Garg > > --- > > include/keys/trusted_tee.h | 55 ++++++ > > security/keys/trusted-keys/Makefile | 1 + > > security/keys/trusted-keys/trusted_core.c | 4 + > > security/keys/trusted-keys/trusted_tee.c | 278 ++++++++++++++++++++++++++++++ > > 4 files changed, 338 insertions(+) > > create mode 100644 include/keys/trusted_tee.h > > create mode 100644 security/keys/trusted-keys/trusted_tee.c > > > > diff --git a/include/keys/trusted_tee.h b/include/keys/trusted_tee.h > > new file mode 100644 > > index 0000000..2e2bb15 > > --- /dev/null > > +++ b/include/keys/trusted_tee.h > > @@ -0,0 +1,55 @@ > > +/* SPDX-License-Identifier: GPL-2.0 */ > > +/* > > + * Copyright (C) 2019-2020 Linaro Ltd. > > + * > > + * Author: > > + * Sumit Garg > > + */ > > + > > +#ifndef __TEE_TRUSTED_KEY_H > > +#define __TEE_TRUSTED_KEY_H > > + > > +#include > > + > > +#define DRIVER_NAME "tee-trusted-key" > > + > > +/* > > + * Get random data for symmetric key > > + * > > + * [out] memref[0] Random data > > + */ > > +#define TA_CMD_GET_RANDOM 0x0 > > + > > +/* > > + * Seal trusted key using hardware unique key > > + * > > + * [in] memref[0] Plain key > > + * [out] memref[1] Sealed key datablob > > + */ > > +#define TA_CMD_SEAL 0x1 > > + > > +/* > > + * Unseal trusted key using hardware unique key > > + * > > + * [in] memref[0] Sealed key datablob > > + * [out] memref[1] Plain key > > + */ > > +#define TA_CMD_UNSEAL 0x2 > > + > > +/** > > + * struct trusted_key_private - TEE Trusted key private data > > + * @dev: TEE based Trusted key device. > > + * @ctx: TEE context handler. > > + * @session_id: Trusted key TA session identifier. > > + * @shm_pool: Memory pool shared with TEE device. > > + */ > > +struct trusted_key_private { > > + struct device *dev; > > + struct tee_context *ctx; > > + u32 session_id; > > + struct tee_shm *shm_pool; > > +}; > > + > > +extern struct trusted_key_ops tee_trusted_key_ops; > > + > > +#endif > > diff --git a/security/keys/trusted-keys/Makefile b/security/keys/trusted-keys/Makefile > > index 49e3bcf..012dd78 100644 > > --- a/security/keys/trusted-keys/Makefile > > +++ b/security/keys/trusted-keys/Makefile > > @@ -7,3 +7,4 @@ obj-$(CONFIG_TRUSTED_KEYS) += trusted.o > > trusted-y += trusted_core.o > > trusted-y += trusted_tpm1.o > > trusted-y += trusted_tpm2.o > > +trusted-y += trusted_tee.o > > diff --git a/security/keys/trusted-keys/trusted_core.c b/security/keys/trusted-keys/trusted_core.c > > index 71a5e27..74a3d80 100644 > > --- a/security/keys/trusted-keys/trusted_core.c > > +++ b/security/keys/trusted-keys/trusted_core.c > > @@ -8,6 +8,7 @@ > > > > #include > > #include > > +#include > > #include > > #include > > #include > > @@ -28,6 +29,9 @@ static struct trusted_key_source trusted_key_sources[] = { > > #if defined(CONFIG_TCG_TPM) > > { "tpm", &tpm_trusted_key_ops }, > > #endif > > +#if defined(CONFIG_TEE) > > + { "tee", &tee_trusted_key_ops }, > > +#endif > > }; > > static struct trusted_key_ops *trusted_key_ops; > > > > diff --git a/security/keys/trusted-keys/trusted_tee.c b/security/keys/trusted-keys/trusted_tee.c > > new file mode 100644 > > index 0000000..b414d52 > > --- /dev/null > > +++ b/security/keys/trusted-keys/trusted_tee.c > > @@ -0,0 +1,278 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2019-2020 Linaro Ltd. > > + * > > + * Author: > > + * Sumit Garg > > + */ > > + > > +#include > > +#include > > +#include > > +#include > > +#include > > + > > +#include > > +#include > > + > > +static struct trusted_key_private pvt_data; > > + > > +/* > > + * Have the TEE seal(encrypt) the symmetric key > > + */ > > +static int tee_trusted_seal(struct trusted_key_payload *p, char *datablob) > > Use trusted_tee_* prefix. > Okay, so will also switch to trusted_tpm_* prefix in patch #1 too. > > +{ > > + int ret = 0; > > "int ret;" > > It is never used uninitialized. > Ack. > > + struct tee_ioctl_invoke_arg inv_arg; > > + struct tee_param param[4]; > > + struct tee_shm *reg_shm_in = NULL, *reg_shm_out = NULL; > > + > > + memset(&inv_arg, 0, sizeof(inv_arg)); > > + memset(¶m, 0, sizeof(param)); > > + > > + reg_shm_in = tee_shm_register(pvt_data.ctx, (unsigned long)p->key, > > + p->key_len, TEE_SHM_DMA_BUF | > > + TEE_SHM_KERNEL_MAPPED); > > + if (IS_ERR(reg_shm_in)) { > > + dev_err(pvt_data.dev, "key shm register failed\n"); > > + return PTR_ERR(reg_shm_in); > > + } > > + > > + reg_shm_out = tee_shm_register(pvt_data.ctx, (unsigned long)p->blob, > > + sizeof(p->blob), TEE_SHM_DMA_BUF | > > + TEE_SHM_KERNEL_MAPPED); > > + if (IS_ERR(reg_shm_out)) { > > + dev_err(pvt_data.dev, "blob shm register failed\n"); > > + ret = PTR_ERR(reg_shm_out); > > + goto out; > > + } > > + > > + inv_arg.func = TA_CMD_SEAL; > > + inv_arg.session = pvt_data.session_id; > > + inv_arg.num_params = 4; > > + > > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT; > > + param[0].u.memref.shm = reg_shm_in; > > + param[0].u.memref.size = p->key_len; > > + param[0].u.memref.shm_offs = 0; > > + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT; > > + param[1].u.memref.shm = reg_shm_out; > > + param[1].u.memref.size = sizeof(p->blob); > > + param[1].u.memref.shm_offs = 0; > > + > > + ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param); > > + if ((ret < 0) || (inv_arg.ret != 0)) { > > + dev_err(pvt_data.dev, "TA_CMD_SEAL invoke err: %x\n", > > + inv_arg.ret); > > + ret = -EFAULT; > > + } else { > > + p->blob_len = param[1].u.memref.size; > > + } > > + > > +out: > > + if (reg_shm_out) > > + tee_shm_free(reg_shm_out); > > + if (reg_shm_in) > > + tee_shm_free(reg_shm_in); > > + > > + return ret; > > +} > > + > > +/* > > + * Have the TEE unseal(decrypt) the symmetric key > > + */ > > +static int tee_trusted_unseal(struct trusted_key_payload *p, char *datablob) > > +{ > > + int ret = 0; > > Ditto. > Ack. > > + struct tee_ioctl_invoke_arg inv_arg; > > + struct tee_param param[4]; > > + struct tee_shm *reg_shm_in = NULL, *reg_shm_out = NULL; > > + > > + memset(&inv_arg, 0, sizeof(inv_arg)); > > + memset(¶m, 0, sizeof(param)); > > + > > + reg_shm_in = tee_shm_register(pvt_data.ctx, (unsigned long)p->blob, > > + p->blob_len, TEE_SHM_DMA_BUF | > > + TEE_SHM_KERNEL_MAPPED); > > + if (IS_ERR(reg_shm_in)) { > > + dev_err(pvt_data.dev, "blob shm register failed\n"); > > + return PTR_ERR(reg_shm_in); > > + } > > + > > + reg_shm_out = tee_shm_register(pvt_data.ctx, (unsigned long)p->key, > > + sizeof(p->key), TEE_SHM_DMA_BUF | > > + TEE_SHM_KERNEL_MAPPED); > > + if (IS_ERR(reg_shm_out)) { > > + dev_err(pvt_data.dev, "key shm register failed\n"); > > + ret = PTR_ERR(reg_shm_out); > > + goto out; > > + } > > + > > + inv_arg.func = TA_CMD_UNSEAL; > > + inv_arg.session = pvt_data.session_id; > > + inv_arg.num_params = 4; > > + > > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_INPUT; > > + param[0].u.memref.shm = reg_shm_in; > > + param[0].u.memref.size = p->blob_len; > > + param[0].u.memref.shm_offs = 0; > > + param[1].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT; > > + param[1].u.memref.shm = reg_shm_out; > > + param[1].u.memref.size = sizeof(p->key); > > + param[1].u.memref.shm_offs = 0; > > + > > + ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param); > > + if ((ret < 0) || (inv_arg.ret != 0)) { > > + dev_err(pvt_data.dev, "TA_CMD_UNSEAL invoke err: %x\n", > > + inv_arg.ret); > > + ret = -EFAULT; > > + } else { > > + p->key_len = param[1].u.memref.size; > > + } > > + > > +out: > > + if (reg_shm_out) > > + tee_shm_free(reg_shm_out); > > + if (reg_shm_in) > > + tee_shm_free(reg_shm_in); > > + > > + return ret; > > +} > > + > > +/* > > + * Have the TEE generate random symmetric key > > + */ > > +static int tee_trusted_get_random(unsigned char *key, size_t key_len) > > +{ > > + int ret = 0; > > Ditto. > Ack. > > + struct tee_ioctl_invoke_arg inv_arg; > > + struct tee_param param[4]; > > + struct tee_shm *reg_shm = NULL; > > + > > + memset(&inv_arg, 0, sizeof(inv_arg)); > > + memset(¶m, 0, sizeof(param)); > > + > > + reg_shm = tee_shm_register(pvt_data.ctx, (unsigned long)key, key_len, > > + TEE_SHM_DMA_BUF | TEE_SHM_KERNEL_MAPPED); > > + if (IS_ERR(reg_shm)) { > > + dev_err(pvt_data.dev, "key shm register failed\n"); > > + return PTR_ERR(reg_shm); > > + } > > + > > + inv_arg.func = TA_CMD_GET_RANDOM; > > + inv_arg.session = pvt_data.session_id; > > + inv_arg.num_params = 4; > > + > > + param[0].attr = TEE_IOCTL_PARAM_ATTR_TYPE_MEMREF_OUTPUT; > > + param[0].u.memref.shm = reg_shm; > > + param[0].u.memref.size = key_len; > > + param[0].u.memref.shm_offs = 0; > > + > > + ret = tee_client_invoke_func(pvt_data.ctx, &inv_arg, param); > > + if ((ret < 0) || (inv_arg.ret != 0)) { > > + dev_err(pvt_data.dev, "TA_CMD_GET_RANDOM invoke err: %x\n", > > + inv_arg.ret); > > + ret = -EFAULT; > > + } else { > > + ret = param[0].u.memref.size; > > + } > > + > > + tee_shm_free(reg_shm); > > + > > + return ret; > > +} > > + > > +static int optee_ctx_match(struct tee_ioctl_version_data *ver, const void *data) > > +{ > > + if (ver->impl_id == TEE_IMPL_ID_OPTEE) > > + return 1; > > + else > > + return 0; > > +} > > + > > +static int trusted_key_probe(struct device *dev) > > +{ > > + struct tee_client_device *rng_device = to_tee_client_device(dev); > > + int ret = 0, err = -ENODEV; > > + struct tee_ioctl_open_session_arg sess_arg; > > Ditto. I'm not sure why you need both 'ret' and 'err'. > Okay, will use 'ret' only. > > + > > + memset(&sess_arg, 0, sizeof(sess_arg)); > > + > > + pvt_data.ctx = tee_client_open_context(NULL, optee_ctx_match, NULL, > > + NULL); > > + if (IS_ERR(pvt_data.ctx)) > > + return -ENODEV; > > + > > + memcpy(sess_arg.uuid, rng_device->id.uuid.b, TEE_IOCTL_UUID_LEN); > > + sess_arg.clnt_login = TEE_IOCTL_LOGIN_REE_KERNEL; > > + sess_arg.num_params = 0; > > + > > + ret = tee_client_open_session(pvt_data.ctx, &sess_arg, NULL); > > + if ((ret < 0) || (sess_arg.ret != 0)) { > > + dev_err(dev, "tee_client_open_session failed, err: %x\n", > > + sess_arg.ret); > > + err = -EINVAL; > > Couldn't you just overwrite 'ret'? > Ack. -Sumit > > + goto out_ctx; > > + } > > + pvt_data.session_id = sess_arg.session; > > + > > + ret = register_key_type(&key_type_trusted); > > + if (ret < 0) > > + goto out_sess; > > + > > + pvt_data.dev = dev; > > + > > + return 0; > > + > > +out_sess: > > + tee_client_close_session(pvt_data.ctx, pvt_data.session_id); > > +out_ctx: > > + tee_client_close_context(pvt_data.ctx); > > + > > + return err; > > +} > > + > > +static int trusted_key_remove(struct device *dev) > > +{ > > + unregister_key_type(&key_type_trusted); > > + tee_client_close_session(pvt_data.ctx, pvt_data.session_id); > > + tee_client_close_context(pvt_data.ctx); > > + > > + return 0; > > +} > > + > > +static const struct tee_client_device_id trusted_key_id_table[] = { > > + {UUID_INIT(0xf04a0fe7, 0x1f5d, 0x4b9b, > > + 0xab, 0xf7, 0x61, 0x9b, 0x85, 0xb4, 0xce, 0x8c)}, > > + {} > > +}; > > +MODULE_DEVICE_TABLE(tee, trusted_key_id_table); > > + > > +static struct tee_client_driver trusted_key_driver = { > > + .id_table = trusted_key_id_table, > > + .driver = { > > + .name = DRIVER_NAME, > > + .bus = &tee_bus_type, > > + .probe = trusted_key_probe, > > + .remove = trusted_key_remove, > > + }, > > +}; > > + > > +static int __init init_tee_trusted(void) > > +{ > > + return driver_register(&trusted_key_driver.driver); > > +} > > + > > +static void __exit exit_tee_trusted(void) > > +{ > > + driver_unregister(&trusted_key_driver.driver); > > +} > > + > > +struct trusted_key_ops tee_trusted_key_ops = { > > + .migratable = 0, /* non-migratable */ > > + .init = init_tee_trusted, > > + .seal = tee_trusted_seal, > > + .unseal = tee_trusted_unseal, > > + .get_random = tee_trusted_get_random, > > + .exit = exit_tee_trusted, > > +}; > > -- > > 2.7.4 > > > > /Jarkko