[RFC 11/12] keys/mktme: Add a new key service type for memory encryption keys
Huang, Kai
kai.huang at intel.com
Mon Sep 10 03:29:29 UTC 2018
> -----Original Message-----
> From: keyrings-owner at vger.kernel.org [mailto:keyrings-
> owner at vger.kernel.org] On Behalf Of Alison Schofield
> Sent: Saturday, September 8, 2018 10:39 AM
> To: dhowells at redhat.com; tglx at linutronix.de
> Cc: Huang, Kai <kai.huang at intel.com>; Nakajima, Jun
> <jun.nakajima at intel.com>; Shutemov, Kirill <kirill.shutemov at intel.com>;
> Hansen, Dave <dave.hansen at intel.com>; Sakkinen, Jarkko
> <jarkko.sakkinen at intel.com>; jmorris at namei.org; keyrings at vger.kernel.org;
> linux-security-module at vger.kernel.org; mingo at redhat.com; hpa at zytor.com;
> x86 at kernel.org; linux-mm at kvack.org
> Subject: [RFC 11/12] keys/mktme: Add a new key service type for memory
> encryption keys
>
> MKTME (Multi-Key Total Memory Encryption) is a technology that allows
> transparent memory encryption in upcoming Intel platforms. MKTME will
> support mulitple encryption domains, each having their own key. The main use
> case for the feature is virtual machine isolation. The API needs the flexibility to
> work for a wide range of uses.
>
> The MKTME key service type manages the addition and removal of the memory
> encryption keys. It maps software keys to hardware keyids and programs the
> hardware with the user requested encryption options.
>
> The only supported encryption algorithm is AES-XTS 128.
>
> The MKTME key service is half of the MKTME API level solution. It pairs with a
> new memory encryption system call: encrypt_mprotect() that uses the keys to
> encrypt memory.
>
> See Documentation/x86/mktme-keys.txt
>
> Signed-off-by: Alison Schofield <alison.schofield at intel.com>
> ---
> arch/x86/Kconfig | 1 +
> include/keys/mktme-type.h | 28 +++++
> security/keys/Kconfig | 11 ++
> security/keys/Makefile | 1 +
> security/keys/mktme_keys.c | 278
> +++++++++++++++++++++++++++++++++++++++++++++
> 5 files changed, 319 insertions(+)
> create mode 100644 include/keys/mktme-type.h create mode 100644
> security/keys/mktme_keys.c
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig index
> 023a22568c06..50d8aa6a58e9 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1527,6 +1527,7 @@ config X86_INTEL_MKTME
> bool "Intel Multi-Key Total Memory Encryption"
> select DYNAMIC_PHYSICAL_MASK
> select PAGE_EXTENSION
> + select MKTME_KEYS
> depends on X86_64 && CPU_SUP_INTEL
> ---help---
> Say yes to enable support for Multi-Key Total Memory Encryption.
> diff --git a/include/keys/mktme-type.h b/include/keys/mktme-type.h new file
> mode 100644 index 000000000000..bebe74cb2b51
> --- /dev/null
> +++ b/include/keys/mktme-type.h
> @@ -0,0 +1,28 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +
> +/*
> + * Key service for Multi-KEY Total Memory Encryption */
> +
> +#ifndef _KEYS_MKTME_TYPE_H
> +#define _KEYS_MKTME_TYPE_H
> +
> +#include <linux/key.h>
> +
> +/*
> + * The AES-XTS 128 encryption algorithm requires 128 bits for each
> + * user supplied option: userkey=, tweak=, entropy=.
> + */
> +#define MKTME_AES_XTS_SIZE 16
> +
> +enum mktme_alg {
> + MKTME_ALG_AES_XTS_128,
> +};
> +
> +const char *const mktme_alg_names[] = {
> + [MKTME_ALG_AES_XTS_128] = "aes_xts_128",
> +};
> +
> +extern struct key_type key_type_mktme;
> +
> +#endif /* _KEYS_MKTME_TYPE_H */
> diff --git a/security/keys/Kconfig b/security/keys/Kconfig index
> 6462e6654ccf..c36972113e67 100644
> --- a/security/keys/Kconfig
> +++ b/security/keys/Kconfig
> @@ -101,3 +101,14 @@ config KEY_DH_OPERATIONS
> in the kernel.
>
> If you are unsure as to whether this is required, answer N.
> +
> +config MKTME_KEYS
> + bool "Multi-Key Total Memory Encryption Keys"
> + depends on KEYS && X86_INTEL_MKTME
> + help
> + This option provides support for Multi-Key Total Memory
> + Encryption (MKTME) on Intel platforms offering the feature.
> + MKTME allows userspace to manage the hardware encryption
> + keys through the kernel key services.
> +
> + If you are unsure as to whether this is required, answer N.
> diff --git a/security/keys/Makefile b/security/keys/Makefile index
> ef1581b337a3..2d9f9a82cb8a 100644
> --- a/security/keys/Makefile
> +++ b/security/keys/Makefile
> @@ -29,3 +29,4 @@ obj-$(CONFIG_KEY_DH_OPERATIONS) += dh.o
> obj-$(CONFIG_BIG_KEYS) += big_key.o
> obj-$(CONFIG_TRUSTED_KEYS) += trusted.o
> obj-$(CONFIG_ENCRYPTED_KEYS) += encrypted-keys/
> +obj-$(CONFIG_MKTME_KEYS) += mktme_keys.o
> diff --git a/security/keys/mktme_keys.c b/security/keys/mktme_keys.c new file
> mode 100644 index 000000000000..dcbce7194647
> --- /dev/null
> +++ b/security/keys/mktme_keys.c
> @@ -0,0 +1,278 @@
> +// SPDX-License-Identifier: GPL-3.0
> +
> +/* Documentation/x86/mktme-keys.txt */
> +
> +#include <linux/cred.h>
> +#include <linux/cpu.h>
> +#include <linux/err.h>
> +#include <linux/init.h>
> +#include <linux/key.h>
> +#include <linux/key-type.h>
> +#include <linux/init.h>
> +#include <linux/parser.h>
> +#include <linux/slab.h>
> +#include <linux/string.h>
> +#include <asm/intel_pconfig.h>
> +#include <asm/mktme.h>
> +#include <keys/mktme-type.h>
> +#include <keys/user-type.h>
> +
> +#include "internal.h"
> +
> +struct kmem_cache *mktme_prog_cache; /* hardware programming
> struct */
> +cpumask_var_t mktme_cpumask; /* one cpu per pkg to program
> keys */
Oh the 'mktme_cpumask' is here. Sorry I didn't notice when replying to your patch 10. :)
But I think you can just move what you did in patch 10 here and leave intel_pconfig.h unchanged. It's much clearer.
> +
> +static const char * const mktme_program_err[] = {
> + "KeyID was successfully programmed", /* 0 */
> + "Invalid KeyID programming command", /* 1 */
> + "Insufficient entropy", /* 2 */
> + "KeyID not valid", /* 3 */
> + "Invalid encryption algorithm chosen", /* 4 */
> + "Failure to access key table", /* 5 */
> +};
> +
> +/* If a key is available, program and add the key to the software map.
> +*/ static int mktme_program_key(key_serial_t serial,
> + struct mktme_key_program *kprog) {
> + int keyid, ret;
> +
> + keyid = mktme_map_get_free_keyid();
> + if (keyid == 0)
> + return -EDQUOT;
> +
> + kprog->keyid = keyid;
> + ret = mktme_key_program(kprog, mktme_cpumask);
> + if (ret == MKTME_PROG_SUCCESS)
> + mktme_map_set_keyid(keyid, serial);
> + else
> + pr_debug("mktme: %s [%d]\n", mktme_program_err[ret], ret);
> +
> + return ret;
> +}
> +
> +enum mktme_opt_id {
> + OPT_ERROR = -1,
> + OPT_USERKEY,
> + OPT_TWEAK,
> + OPT_ENTROPY,
> + OPT_ALGORITHM,
> +};
> +
> +static const match_table_t mktme_token = {
> + {OPT_USERKEY, "userkey=%s"},
> + {OPT_TWEAK, "tweak=%s"},
> + {OPT_ENTROPY, "entropy=%s"},
> + {OPT_ALGORITHM, "algorithm=%s"},
> + {OPT_ERROR, NULL}
> +
> +};
> +
> +/*
> + * Algorithm AES-XTS 128 is the only supported encryption algorithm.
> + * CPU Generated Key: requires user supplied entropy and accepts no
> + * other options.
> + * User Supplied Key: requires user supplied tweak key and accepts
> + * no other options.
> + */
> +static int mktme_check_options(struct mktme_key_program *kprog,
> + unsigned long token_mask)
> +{
> + if (!token_mask)
> + return -EINVAL;
> +
> + kprog->keyid_ctrl |= MKTME_AES_XTS_128;
> +
> + if (!test_bit(OPT_USERKEY, &token_mask)) {
> + if ((!test_bit(OPT_ENTROPY, &token_mask)) ||
> + (test_bit(OPT_TWEAK, &token_mask)))
> + return -EINVAL;
> +
> + kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_RANDOM;
> + }
> + if (test_bit(OPT_USERKEY, &token_mask)) {
> + if ((test_bit(OPT_ENTROPY, &token_mask)) ||
> + (!test_bit(OPT_TWEAK, &token_mask)))
> + return -EINVAL;
> +
> + kprog->keyid_ctrl |= MKTME_KEYID_SET_KEY_DIRECT;
> + }
> + return 0;
> +}
> +
> +/*
> + * Parse the options and begin to fill in the key programming struct kprog.
> + * Check the lengths of incoming data and push data directly into kprog fields.
> + */
> +static int mktme_get_options(char *options, struct mktme_key_program
> +*kprog) {
> + int len = MKTME_AES_XTS_SIZE / 2;
> + substring_t args[MAX_OPT_ARGS];
> + unsigned long token_mask = 0;
> + enum mktme_alg alg;
> + char *p = options;
> + int ret, token;
> +
> + while ((p = strsep(&options, " \t"))) {
> + if (*p == '\0' || *p == ' ' || *p == '\t')
> + continue;
> + token = match_token(p, mktme_token, args);
> + if (test_and_set_bit(token, &token_mask))
> + return -EINVAL;
> +
> + switch (token) {
> + case OPT_USERKEY:
> + if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> + return -EINVAL;
> + ret = hex2bin(kprog->key_field_1, args[0].from, len);
> + if (ret < 0)
> + return -EINVAL;
> + break;
> +
> + case OPT_TWEAK:
> + if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> + return -EINVAL;
> + ret = hex2bin(kprog->key_field_2, args[0].from, len);
> + if (ret < 0)
> + return -EINVAL;
> + break;
> +
> + case OPT_ENTROPY:
> + if (strlen(args[0].from) != MKTME_AES_XTS_SIZE)
> + return -EINVAL;
> + /* Applied to both CPU-generated data and tweak keys
> */
> + ret = hex2bin(kprog->key_field_1, args[0].from, len);
> + ret = hex2bin(kprog->key_field_2, args[0].from, len);
> + if (ret < 0)
> + return -EINVAL;
> + break;
I replied w/ some comments in patch 1 (Document part). Do you have any particular reason to introduce OPT_ENTROPY, while we can simply use OPT_USERKEY and OPT_TWEAK?
Actually I think it might be better that we disallow or ignore OPT_USERKEY, OPT_TWEAK (or OPT_ENTROPY in your patch). Please see my reply to your patch 1.
> +
> + case OPT_ALGORITHM:
> + alg = match_string(mktme_alg_names,
> + ARRAY_SIZE(mktme_alg_names),
> + args[0].from);
> + if (alg != MKTME_ALG_AES_XTS_128)
> + return -EINVAL;
> + break;
> +
> + default:
> + return -EINVAL;
> + }
> + }
> + return mktme_check_options(kprog, token_mask); }
> +
> +/* Key Service Command: Creates a software key and programs hardware */
> +int mktme_instantiate(struct key *key, struct key_preparsed_payload
> +*prep) {
> + struct mktme_key_program *kprog = NULL;
> + size_t datalen = prep->datalen;
> + char *options;
> + int ret = 0;
> +
> + if (!capable(CAP_SYS_RESOURCE) && !capable(CAP_SYS_ADMIN))
> + return -EACCES;
> +
> + if (datalen <= 0 || datalen > 1024 || !prep->data)
> + return -EINVAL;
> +
> + options = kmemdup(prep->data, datalen + 1, GFP_KERNEL);
> + if (!options)
> + return -ENOMEM;
> +
> + options[datalen] = '\0';
> +
> + kprog = kmem_cache_zalloc(mktme_prog_cache, GFP_KERNEL);
> + if (!kprog) {
> + kzfree(options);
> + return -ENOMEM;
> + }
> + ret = mktme_get_options(options, kprog);
> + if (ret < 0)
> + goto out;
> +
> + mktme_map_lock();
> + ret = mktme_program_key(key->serial, kprog);
> + mktme_map_unlock();
> +out:
> + kzfree(options);
> + kmem_cache_free(mktme_prog_cache, kprog);
> + return ret;
> +}
> +
> +struct key_type key_type_mktme = {
> + .name = "mktme",
> + .instantiate = mktme_instantiate,
> + .describe = user_describe,
> +};
> +
> +/*
> + * Build mktme_cpumask to include one cpu per physical package.
> + * The mask is used in mktme_key_program() when the hardware key
> + * table is programmed on a per package basis.
> + */
> +static int mktme_build_cpumask(void)
> +{
> + int online_cpu, mktme_cpu;
> + int online_pkgid, mktme_pkgid = -1;
> +
> + if (!zalloc_cpumask_var(&mktme_cpumask, GFP_KERNEL))
> + return -ENOMEM;
> +
> + for_each_online_cpu(online_cpu) {
> + online_pkgid = topology_physical_package_id(online_cpu);
> +
> + for_each_cpu(mktme_cpu, mktme_cpumask) {
> + mktme_pkgid =
> topology_physical_package_id(mktme_cpu);
> + if (mktme_pkgid == online_pkgid)
> + break;
> + }
> + if (mktme_pkgid != online_pkgid)
> + cpumask_set_cpu(online_cpu, mktme_cpumask);
> + }
Could we use 'for_each_online_node', 'cpumask_first/next', etc to simplify the logic?
> + return 0;
> +}
> +
> +/*
> + * Allocate the global key map structure based on the available keyids
> + * at boot time. Create a cache and a cpu_mask to use for programming
> + * the hardware. Initialize the encrypt_count array to track VMA's per
> + * keyid. Once all that succeeds, register the 'mktme' key type.
> + */
> +static int __init init_mktme(void)
> +{
> + int ret;
> +
> + /* Verify keys are present */
> + if (!(mktme_nr_keyids > 0))
> + return -EINVAL;
> +
> + if (!mktme_map_alloc())
> + return -ENOMEM;
> +
> + mktme_prog_cache = KMEM_CACHE(mktme_key_program,
> SLAB_PANIC);
> + if (!mktme_prog_cache)
> + goto free_map;
> +
> + if (vma_alloc_encrypt_array() < 0)
> + goto free_cache;
I think it's better to move 'vma_alloc_encrypt_array' part to this patch. Please see my reply to your patch 7.
I also think we should avoid adding new staff to arch/x86/include/asm/mktme.h since it is included by some basic page table manipulation header files. Adding mm related structure to asm/mktme.h sometimes may fail to compile kernel in my experience.
Thanks,
-Kai
> +
> + if (mktme_build_cpumask() < 0)
> + goto free_array;
> +
> + ret = register_key_type(&key_type_mktme);
> + if (!ret)
> + return ret;
> +
> + free_cpumask_var(mktme_cpumask);
> +free_array:
> + vma_free_encrypt_array();
> +free_cache:
> + kmem_cache_destroy(mktme_prog_cache);
> +free_map:
> + mktme_map_free();
> +
> + return -ENOMEM;
> +}
> +
> +late_initcall(init_mktme);
> --
> 2.14.1
More information about the Linux-security-module-archive
mailing list