| Message ID | 20190103143227.9138-2-jlee@suse.com (mailing list archive) |
|---|---|
| State | Changes Requested, archived |
| Headers | show |
| Series | Encryption and authentication for hibernate snapshot image | expand |
Am Donnerstag, 3. Januar 2019, 15:32:23 CET schrieb Lee, Chun-Yi: Hi Chun, > This patch adds a snapshot keys handler for using the key retention > service api to create keys for snapshot image encryption and > authentication. > > This handler uses TPM trusted key as the snapshot master key, and the > encryption key and authentication key are derived from the snapshot > key. The user defined key can also be used as the snapshot master key > , but user must be aware that the security of user key relies on user > space. > > The name of snapshot key is fixed to "swsusp-kmk". User should use > the keyctl tool to load the key blob to root's user keyring. e.g. > > # /bin/keyctl add trusted swsusp-kmk "load `cat swsusp-kmk.blob`" @u > > or create a new user key. e.g. > > # /bin/keyctl add user swsusp-kmk password @u > > Then the disk_kmk sysfs file can be used to trigger the initialization > of snapshot key: > > # echo 1 > /sys/power/disk_kmk > > After the initialization be triggered, the secret in the payload of > swsusp-key will be copied by hibernation and be erased. Then user can > use keyctl to remove swsusp-kmk key from root's keyring. > > If user does not trigger the initialization by disk_kmk file after > swsusp-kmk be loaded to kernel. Then the snapshot key will be > initialled when hibernation be triggered. > > v2: > - Fixed bug of trusted_key_init's return value. > - Fixed wording in Kconfig > - Removed VLA usage > - Removed the checking of capability for writing disk_kmk. > - Fixed Kconfig, select trusted key. > - Add memory barrier before setting key initialized flag. > > Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Chen Yu <yu.c.chen@intel.com> > Cc: Oliver Neukum <oneukum@suse.com> > Cc: Ryan Chen <yu.chen.surf@gmail.com> > Cc: David Howells <dhowells@redhat.com> > Cc: Giovanni Gherdovich <ggherdovich@suse.cz> > Cc: Randy Dunlap <rdunlap@infradead.org> > Cc: Jann Horn <jannh@google.com> > Cc: Andy Lutomirski <luto@kernel.org> > Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> > --- > kernel/power/Kconfig | 14 +++ > kernel/power/Makefile | 1 + > kernel/power/hibernate.c | 33 ++++++ > kernel/power/power.h | 16 +++ > kernel/power/snapshot_key.c | 245 > ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 309 > insertions(+) > create mode 100644 kernel/power/snapshot_key.c > > diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig > index f8fe57d1022e..506a3c5a7a0d 100644 > --- a/kernel/power/Kconfig > +++ b/kernel/power/Kconfig > @@ -76,6 +76,20 @@ config HIBERNATION > > For more information take a look at > <file:Documentation/power/swsusp.txt>. > > +config HIBERNATION_ENC_AUTH > + bool "Hibernation encryption and authentication" > + depends on HIBERNATION > + select TRUSTED_KEYS > + select CRYPTO_AES > + select CRYPTO_HMAC > + select CRYPTO_SHA512 > + help > + This option will encrypt and authenticate the memory snapshot image > + of hibernation. It prevents that the snapshot image be arbitrarily > + modified. A user can use the TPM's trusted key or user defined key > + as the master key of hibernation. The TPM trusted key depends on TPM. > + The security of user defined key relies on user space. > + > config ARCH_SAVE_PAGE_KEYS > bool > > diff --git a/kernel/power/Makefile b/kernel/power/Makefile > index e7e47d9be1e5..d949adbaf580 100644 > --- a/kernel/power/Makefile > +++ b/kernel/power/Makefile > @@ -11,6 +11,7 @@ obj-$(CONFIG_FREEZER) += process.o > obj-$(CONFIG_SUSPEND) += suspend.o > obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o > obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o > +obj-$(CONFIG_HIBERNATION_ENC_AUTH) += snapshot_key.o > obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o > obj-$(CONFIG_PM_WAKELOCKS) += wakelock.o > > diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c > index abef759de7c8..ecc31e8e40d0 100644 > --- a/kernel/power/hibernate.c > +++ b/kernel/power/hibernate.c > @@ -1034,6 +1034,36 @@ static ssize_t disk_store(struct kobject *kobj, > struct kobj_attribute *attr, > > power_attr(disk); > > +#ifdef CONFIG_HIBERNATION_ENC_AUTH > +static ssize_t disk_kmk_show(struct kobject *kobj, struct kobj_attribute > *attr, + char *buf) > +{ > + if (snapshot_key_initialized()) > + return sprintf(buf, "initialized\n"); > + else > + return sprintf(buf, "uninitialized\n"); > +} > + > +static ssize_t disk_kmk_store(struct kobject *kobj, struct kobj_attribute > *attr, + const char *buf, size_t n) > +{ > + int error = 0; > + char *p; > + int len; > + > + p = memchr(buf, '\n', n); > + len = p ? p - buf : n; > + if (strncmp(buf, "1", len)) > + return -EINVAL; > + > + error = snapshot_key_init(); > + > + return error ? error : n; > +} > + > +power_attr(disk_kmk); > +#endif /* !CONFIG_HIBERNATION_ENC_AUTH */ > + > static ssize_t resume_show(struct kobject *kobj, struct kobj_attribute > *attr, char *buf) > { > @@ -1138,6 +1168,9 @@ power_attr(reserved_size); > > static struct attribute * g[] = { > &disk_attr.attr, > +#ifdef CONFIG_HIBERNATION_ENC_AUTH > + &disk_kmk_attr.attr, > +#endif > &resume_offset_attr.attr, > &resume_attr.attr, > &image_size_attr.attr, > diff --git a/kernel/power/power.h b/kernel/power/power.h > index 9e58bdc8a562..fe2dfa0d4d36 100644 > --- a/kernel/power/power.h > +++ b/kernel/power/power.h > @@ -4,6 +4,12 @@ > #include <linux/utsname.h> > #include <linux/freezer.h> > #include <linux/compiler.h> > +#include <crypto/sha.h> > + > +/* The max size of encrypted key blob */ > +#define KEY_BLOB_BUFF_LEN 512 > +#define SNAPSHOT_KEY_SIZE SHA512_DIGEST_SIZE > +#define DERIVED_KEY_SIZE SHA512_DIGEST_SIZE > > struct swsusp_info { > struct new_utsname uts; > @@ -20,6 +26,16 @@ struct swsusp_info { > extern void __init hibernate_reserved_size_init(void); > extern void __init hibernate_image_size_init(void); > > +#ifdef CONFIG_HIBERNATION_ENC_AUTH > +/* kernel/power/snapshot_key.c */ > +extern int snapshot_key_init(void); > +extern bool snapshot_key_initialized(void); > +extern int snapshot_get_auth_key(u8 *auth_key, bool may_sleep); > +extern int snapshot_get_enc_key(u8 *enc_key, bool may_sleep); > +#else > +static inline int snapshot_key_init(void) { return 0; } > +#endif /* !CONFIG_HIBERNATION_ENC_AUTH */ > + > #ifdef CONFIG_ARCH_HIBERNATION_HEADER > /* Maximum size of architecture specific data in a hibernation header */ > #define MAX_ARCH_HEADER_SIZE (sizeof(struct new_utsname) + 4) > diff --git a/kernel/power/snapshot_key.c b/kernel/power/snapshot_key.c > new file mode 100644 > index 000000000000..3a569b505d8d > --- /dev/null > +++ b/kernel/power/snapshot_key.c > @@ -0,0 +1,245 @@ > +// SPDX-License-Identifier: GPL-2.0 > + > +/* snapshot keys handler > + * > + * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com> > + * > + * This program is free software; you can redistribute it and/or > + * modify it under the terms of the GNU General Public Licence > + * as published by the Free Software Foundation; either version > + * 2 of the Licence, or (at your option) any later version. > + */ > +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt > + > +#include <linux/cred.h> > +#include <linux/key-type.h> > +#include <linux/slab.h> > +#include <crypto/hash.h> > +#include <crypto/sha.h> > +#include <keys/trusted-type.h> > +#include <keys/user-type.h> > + > +#include "power.h" > + > +static const char hash_alg[] = "sha512"; > +static struct crypto_shash *hash_tfm; > + > +/* The master key of snapshot */ > +static struct snapshot_key { > + const char *key_name; > + bool initialized; > + unsigned int key_len; > + u8 key[SNAPSHOT_KEY_SIZE]; > +} skey = { > + .key_name = "swsusp-kmk", > +}; > + > +static int calc_hash(u8 *digest, const u8 *buf, unsigned int buflen, > + bool may_sleep) > +{ > + struct shash_desc *desc; > + int err; > + > + desc = kzalloc(sizeof(struct shash_desc) + > + crypto_shash_descsize(hash_tfm), > + may_sleep ? GFP_KERNEL : GFP_ATOMIC); Why not using SHASH_DESC_ON_STACK? > + if (!desc) > + return -ENOMEM; > + > + desc->tfm = hash_tfm; > + desc->flags = may_sleep ? CRYPTO_TFM_REQ_MAY_SLEEP : 0; > + err = crypto_shash_digest(desc, buf, buflen, digest); > + shash_desc_zero(desc); > + kzfree(desc); > + > + return err; > +} > + > +static int calc_key_hash(u8 *key, unsigned int key_len, const char *salt, > + u8 *hash, bool may_sleep) > +{ > + unsigned int salted_buf_len; > + u8 *salted_buf; > + int ret; > + > + if (!key || !hash_tfm || !hash) > + return -EINVAL; > + > + salted_buf_len = strlen(salt) + 1 + SNAPSHOT_KEY_SIZE; strlen on binary data? I guess that will not work. May I suggest to hand down the length of salt to this function? > + salted_buf = kzalloc(salted_buf_len, > + may_sleep ? GFP_KERNEL : GFP_ATOMIC); > + if (!salted_buf) > + return -ENOMEM; > + > + strcpy(salted_buf, salt); > + memcpy(salted_buf + strlen(salted_buf) + 1, key, key_len); > + > + ret = calc_hash(hash, salted_buf, salted_buf_len, may_sleep); > + memzero_explicit(salted_buf, salted_buf_len); > + kzfree(salted_buf); > + > + return ret; > +} This function looks very much like a key derivation. May I strongly propose to use an official KDF type like SP800-108 or HKDF? You find the counter-KDF according to SP800-108 in security/keys/dh.c (search for functions *kdf*). Or we may start pulling in KDF support into the kernel crypto API via the patches along the line of [1]. [1] http://www.chronox.de/kdf.html > + > +/* Derive authentication/encryption key */ > +static int get_derived_key(u8 *derived_key, const char *derived_type_str, > + bool may_sleep) > +{ > + int ret; > + > + if (!skey.initialized || !hash_tfm) > + return -EINVAL; > + > + ret = calc_key_hash(skey.key, skey.key_len, derived_type_str, > + derived_key, may_sleep); > + > + return ret; > +} > + > +int snapshot_get_auth_key(u8 *auth_key, bool may_sleep) > +{ > + return get_derived_key(auth_key, "AUTH_KEY", may_sleep); > +} > + > +int snapshot_get_enc_key(u8 *enc_key, bool may_sleep) > +{ > + return get_derived_key(enc_key, "ENC_KEY", may_sleep); > +} > + > +bool snapshot_key_initialized(void) > +{ > + return skey.initialized; > +} > + > +static bool invalid_key(u8 *key, unsigned int key_len) > +{ > + int i; > + > + if (!key || !key_len) > + return true; > + > + if (key_len > SNAPSHOT_KEY_SIZE) { > + pr_warn("Size of swsusp key more than: %d.\n", > + SNAPSHOT_KEY_SIZE); > + return true; > + } > + > + /* zero keyblob is invalid key */ > + for (i = 0; i < key_len; i++) { > + if (key[i] != 0) > + return false; > + } > + pr_warn("The swsusp key should not be zero.\n"); > + > + return true; > +} > + > +static int trusted_key_init(void) > +{ > + struct trusted_key_payload *tkp; > + struct key *key; > + int err = 0; > + > + pr_debug("%s\n", __func__); > + > + /* find out swsusp-key */ > + key = request_key(&key_type_trusted, skey.key_name, NULL); > + if (IS_ERR(key)) { > + pr_err("Request key error: %ld\n", PTR_ERR(key)); > + err = PTR_ERR(key); > + return err; > + } > + > + down_write(&key->sem); > + tkp = key->payload.data[0]; > + if (invalid_key(tkp->key, tkp->key_len)) { > + err = -EINVAL; > + goto key_invalid; > + } > + skey.key_len = tkp->key_len; > + memcpy(skey.key, tkp->key, tkp->key_len); > + /* burn the original key contents */ > + memzero_explicit(tkp->key, tkp->key_len); > + > +key_invalid: > + up_write(&key->sem); > + key_put(key); > + > + return err; > +} > + > +static int user_key_init(void) This function and trusted_key_init look very similar - could they be collapsed into one function? > +{ > + struct user_key_payload *ukp; > + struct key *key; > + int err = 0; > + > + pr_debug("%s\n", __func__); > + > + /* find out swsusp-key */ > + key = request_key(&key_type_user, skey.key_name, NULL); > + if (IS_ERR(key)) { > + pr_err("Request key error: %ld\n", PTR_ERR(key)); > + err = PTR_ERR(key); > + return err; > + } > + > + down_write(&key->sem); > + ukp = user_key_payload_locked(key); > + if (!ukp) { > + /* key was revoked before we acquired its semaphore */ > + err = -EKEYREVOKED; > + goto key_invalid; > + } > + if (invalid_key(ukp->data, ukp->datalen)) { > + err = -EINVAL; > + goto key_invalid; > + } > + skey.key_len = ukp->datalen; > + memcpy(skey.key, ukp->data, ukp->datalen); Where would skey.key be destroyed again? > + /* burn the original key contents */ > + memzero_explicit(ukp->data, ukp->datalen); > + > +key_invalid: > + up_write(&key->sem); > + key_put(key); > + > + return err; > +} > + > +/* this function may sleeps */ > +int snapshot_key_init(void) > +{ > + int err; > + > + pr_debug("%s\n", __func__); > + > + if (skey.initialized) > + return 0; > + > + hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC); > + if (IS_ERR(hash_tfm)) { > + pr_err("Can't allocate %s transform: %ld\n", > + hash_alg, PTR_ERR(hash_tfm)); > + return PTR_ERR(hash_tfm); > + } > + > + err = trusted_key_init(); > + if (err) > + err = user_key_init(); > + if (err) > + goto key_fail; > + > + barrier(); > + skey.initialized = true; > + > + pr_info("Snapshot key is initialled.\n"); > + > + return 0; > + > +key_fail: > + crypto_free_shash(hash_tfm); > + hash_tfm = NULL; > + > + return err; > +} Ciao Stephan
Am Sonntag, 6. Januar 2019, 09:01:27 CET schrieb Stephan Mueller: Hi, > > + memcpy(skey.key, ukp->data, ukp->datalen); > > Where would skey.key be destroyed again? Now I see it - it is in patch 4. Please disregard my comment. Ciao Stephan
Hi Stephan, First, thanks for your review! On Sun, Jan 06, 2019 at 09:01:27AM +0100, Stephan Mueller wrote: > Am Donnerstag, 3. Januar 2019, 15:32:23 CET schrieb Lee, Chun-Yi: > > Hi Chun, > > > This patch adds a snapshot keys handler for using the key retention > > service api to create keys for snapshot image encryption and > > authentication. > > > > This handler uses TPM trusted key as the snapshot master key, and the > > encryption key and authentication key are derived from the snapshot > > key. The user defined key can also be used as the snapshot master key > > , but user must be aware that the security of user key relies on user > > space. > > [...snip] > > +static int calc_hash(u8 *digest, const u8 *buf, unsigned int buflen, > > + bool may_sleep) > > +{ > > + struct shash_desc *desc; > > + int err; > > + > > + desc = kzalloc(sizeof(struct shash_desc) + > > + crypto_shash_descsize(hash_tfm), > > + may_sleep ? GFP_KERNEL : GFP_ATOMIC); > > Why not using SHASH_DESC_ON_STACK? > Because security concern and bad runtime performance. Please looking at c2cd0b08e1e patch for hibernation. And reference: https://lore.kernel.org/lkml/CA+55aFzCG-zNmZwX4A2FQpadafLfEzK6CC=qPXydAacU1RqZWA@mail.gmail.com/T/#u https://lwn.net/Articles/749064/ > > + if (!desc) > > + return -ENOMEM; > > + > > + desc->tfm = hash_tfm; > > + desc->flags = may_sleep ? CRYPTO_TFM_REQ_MAY_SLEEP : 0; > > + err = crypto_shash_digest(desc, buf, buflen, digest); > > + shash_desc_zero(desc); > > + kzfree(desc); > > + > > + return err; > > +} > > + > > +static int calc_key_hash(u8 *key, unsigned int key_len, const char *salt, > > + u8 *hash, bool may_sleep) > > +{ > > + unsigned int salted_buf_len; > > + u8 *salted_buf; > > + int ret; > > + > > + if (!key || !hash_tfm || !hash) > > + return -EINVAL; > > + > > + salted_buf_len = strlen(salt) + 1 + SNAPSHOT_KEY_SIZE; > > strlen on binary data? I guess that will not work. May I suggest to hand down > the length of salt to this function? > hm... The salt is actually a "salt string" that's gave from snapshot_get_auth_key() or snapshot_get_enc_key(). So I use strlen() here. I will change the name to salt_string to avoid confusion. > > + salted_buf = kzalloc(salted_buf_len, > > + may_sleep ? GFP_KERNEL : GFP_ATOMIC); > > + if (!salted_buf) > > + return -ENOMEM; > > + > > + strcpy(salted_buf, salt); > > + memcpy(salted_buf + strlen(salted_buf) + 1, key, key_len); > > + > > + ret = calc_hash(hash, salted_buf, salted_buf_len, may_sleep); > > + memzero_explicit(salted_buf, salted_buf_len); > > + kzfree(salted_buf); > > + > > + return ret; > > +} > > This function looks very much like a key derivation. May I strongly propose to Actually key derivation function is modified from the get_derived_key() from the encrypted.c file in encrypted key. > use an official KDF type like SP800-108 or HKDF? > > You find the counter-KDF according to SP800-108 in security/keys/dh.c (search > for functions *kdf*). > > Or we may start pulling in KDF support into the kernel crypto API via the > patches along the line of [1]. > > [1] http://www.chronox.de/kdf.html > Thanks for your suggestion. I didn't touch any key derivation standard before. I will study it. But I still want to use my original function currently. Because the same logic is also used in trusted key. I will happy to move to SP800-108 or HKDF when it's available in kernel. > > + > > +/* Derive authentication/encryption key */ > > +static int get_derived_key(u8 *derived_key, const char *derived_type_str, > > + bool may_sleep) [...snip] > > +static int trusted_key_init(void) > > +{ > > + struct trusted_key_payload *tkp; > > + struct key *key; > > + int err = 0; > > + > > + pr_debug("%s\n", __func__); > > + > > + /* find out swsusp-key */ > > + key = request_key(&key_type_trusted, skey.key_name, NULL); > > + if (IS_ERR(key)) { > > + pr_err("Request key error: %ld\n", PTR_ERR(key)); > > + err = PTR_ERR(key); > > + return err; > > + } > > + > > + down_write(&key->sem); > > + tkp = key->payload.data[0]; > > + if (invalid_key(tkp->key, tkp->key_len)) { > > + err = -EINVAL; > > + goto key_invalid; > > + } > > + skey.key_len = tkp->key_len; > > + memcpy(skey.key, tkp->key, tkp->key_len); > > + /* burn the original key contents */ > > + memzero_explicit(tkp->key, tkp->key_len); > > + > > +key_invalid: > > + up_write(&key->sem); > > + key_put(key); > > + > > + return err; > > +} > > + > > +static int user_key_init(void) > > This function and trusted_key_init look very similar - could they be collapsed > into one function? > The data structure is different between trusted key with user key. I will try to extract the duplicate part but may not collapse into one. > > +{ > > + struct user_key_payload *ukp; > > + struct key *key; > > + int err = 0; > > + > > + pr_debug("%s\n", __func__); > > + > > + /* find out swsusp-key */ > > + key = request_key(&key_type_user, skey.key_name, NULL); > > + if (IS_ERR(key)) { > > + pr_err("Request key error: %ld\n", PTR_ERR(key)); > > + err = PTR_ERR(key); > > + return err; > > + } > > + > > + down_write(&key->sem); > > + ukp = user_key_payload_locked(key); > > + if (!ukp) { > > + /* key was revoked before we acquired its semaphore */ > > + err = -EKEYREVOKED; > > + goto key_invalid; > > + } > > + if (invalid_key(ukp->data, ukp->datalen)) { > > + err = -EINVAL; > > + goto key_invalid; > > + } > > + skey.key_len = ukp->datalen; > > + memcpy(skey.key, ukp->data, ukp->datalen); > > Where would skey.key be destroyed again? > Yes, you saw it in later patch. Thanks a lot! Joey Lee
Am Montag, 7. Januar 2019, 16:33:27 CET schrieb joeyli: Hi Herbert, > > > use an official KDF type like SP800-108 or HKDF? > > > > You find the counter-KDF according to SP800-108 in security/keys/dh.c > > (search for functions *kdf*). > > > > Or we may start pulling in KDF support into the kernel crypto API via the > > patches along the line of [1]. > > > > [1] http://www.chronox.de/kdf.html > > Thanks for your suggestion. I didn't touch any key derivation standard > before. I will study it. > > But I still want to use my original function currently. Because the same > logic is also used in trusted key. I will happy to move to SP800-108 or > HKDF when it's available in kernel. Would it make sense to polish these mentioned KDF patches and add them to the kernel crypto API? The sprawl of key derivation logic here and there which seemingly does not comply to any standard and thus possibly have issues should be prevented and cleaned up. Ciao Stephan
On Mon, Jan 07, 2019 at 04:52:00PM +0100, Stephan Mueller wrote: > > Would it make sense to polish these mentioned KDF patches and add them to the > kernel crypto API? The sprawl of key derivation logic here and there which > seemingly does not comply to any standard and thus possibly have issues should > be prevented and cleaned up. Are we going to have multiple implementations for the same KDF? If not then the crypto API is not a good fit. To consolidate multiple implementations of the same KDF, simply provide helpers for them. Cheers,
Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: Hi Herbert, > Are we going to have multiple implementations for the same KDF? > If not then the crypto API is not a good fit. To consolidate > multiple implementations of the same KDF, simply provide helpers > for them. It is unlikely to have multiple implementations of a KDF. However, KDFs relate to hashes like block chaining modes to raw block ciphers. Thus a KDF can be applied with different hashes. My idea was to add template support to RNGs (because KDFs are effectively a type of RNG since they produce an arbitrary output from a fixed input). The KDFs would be a template wrapping hashes. For example, the CTR-KDF from SP800-108 could be instantiated like kdf-ctr(sha256). Ciao Stephan
> On Jan 7, 2019, at 11:09 PM, Stephan Mueller <smueller@chronox.de> wrote: > > Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: > > Hi Herbert, > >> Are we going to have multiple implementations for the same KDF? >> If not then the crypto API is not a good fit. To consolidate >> multiple implementations of the same KDF, simply provide helpers >> for them. > > It is unlikely to have multiple implementations of a KDF. However, KDFs relate > to hashes like block chaining modes to raw block ciphers. Thus a KDF can be > applied with different hashes. > > My idea was to add template support to RNGs (because KDFs are effectively a > type of RNG since they produce an arbitrary output from a fixed input). The > KDFs would be a template wrapping hashes. For example, the CTR-KDF from > SP800-108 could be instantiated like kdf-ctr(sha256). > > I think that, if the crypto API is going to grow a KDF facility, it should be done right. Have a key type or flag or whatever that says “this key may *only* be used to derive keys using such-and-such algorithm”, and have a helper to derive a key. That helper should take some useful parameters and mix them in: - What type of key is being derived? ECDSA signing key? HMAC key? AES key? - Can user code access the derived key? - What is the key’s purpose? “Encrypt and authenticate a hibernation image” would be a purpose. - Number of bytes. All of these parameters should be mixed in to the key derivation. Also, an AE key, even for AES+HMAC, should be just one derived key. If you need 512 bits, ask for a 512-bit key, not two 256-bit keys.
On Tue, 2019-01-08 at 15:54 -0800, Andy Lutomirski wrote: > > On Jan 7, 2019, at 11:09 PM, Stephan Mueller <smueller@chronox.de> > > wrote: > > > > Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: > > > > Hi Herbert, > > > > > Are we going to have multiple implementations for the same KDF? > > > If not then the crypto API is not a good fit. To consolidate > > > multiple implementations of the same KDF, simply provide helpers > > > for them. > > > > It is unlikely to have multiple implementations of a KDF. However, > > KDFs relate to hashes like block chaining modes to raw block > > ciphers. Thus a KDF can be applied with different hashes. > > > > My idea was to add template support to RNGs (because KDFs are > > effectively a type of RNG since they produce an arbitrary output > > from a fixed input). The KDFs would be a template wrapping hashes. > > For example, the CTR-KDF from SP800-108 could be instantiated like > > kdf-ctr(sha256). > > > > > > I think that, if the crypto API is going to grow a KDF facility, it > should be done right. Have a key type or flag or whatever that says > “this key may *only* be used to derive keys using such-and-such > algorithm”, and have a helper to derive a key. That helper should > take some useful parameters and mix them in: > > - What type of key is being derived? ECDSA signing key? HMAC > key? AES key? > > - Can user code access the derived key? > > - What is the key’s purpose? “Encrypt and authenticate a hibernation > image” would be a purpose. > > - Number of bytes. > > All of these parameters should be mixed in to the key derivation. > > Also, an AE key, even for AES+HMAC, should be just one derived > key. If you need 512 bits, ask for a 512-bit key, not two 256-bit > keys. Actually, it would be enormously helpful if we could reuse these pieces for the TPM as well. It has two KDFs: KDFa, which is the CTR-KDF from SP800-108 and KDFe which is the SP800-56A KDF for elliptic curve one pass Diffie Hellman, so if we're going to do the former, I'd really like the latter as well. The way the TPM parametrises input to both KDFs is (hashAlg, key, label, contextU, contextV, bits) Where hashAlg = the hash algorithm used as the PRF key = the input parameter of variable bit size or the x co-ordinate of the shared point label = An ASCII string representing the use contextU = public input U contextV = public input V bits = number of output bits Is that a good enough parametrisation (not the only way you distinguish uses is with the label, which is not recoverable)? ContextU and ContextV are simply concatenated to form the full Context of SP800-108, but we tend to need two separate inputs (for KDFe they're the public x co-ordinates of the points of the two inputs to ECDH for instance; in KDFa they're usually the local and remote nonces). The labels for TPM usage are things like "INTEGRITY" for HMAC keys or "CFB" when generating an aes128-cfb session key. For KDFe, the tpm seems to like the label "SECRET". Although the TPM specifies fixed short strings for the label, nothing prevents them being longer like the purpose you state above (essentially we could mix purpose, use and key type into the label and the contexts). From the point of view of accelerators, the only thing you really need to know is the hash algorthim (PRF), because everything else above is an input to the function, so I suppose it makes sense to name them as kdf-X(PRF) where X would be ctr or ecdh and PRF would be a hash. James
[Adding Jarkko because this stuff relates to the TPM.] On Tue, Jan 8, 2019 at 4:44 PM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > On Tue, 2019-01-08 at 15:54 -0800, Andy Lutomirski wrote: > > > On Jan 7, 2019, at 11:09 PM, Stephan Mueller <smueller@chronox.de> > > > wrote: > > > > > > Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: > > > > > > Hi Herbert, > > > > > > > Are we going to have multiple implementations for the same KDF? > > > > If not then the crypto API is not a good fit. To consolidate > > > > multiple implementations of the same KDF, simply provide helpers > > > > for them. > > > > > > It is unlikely to have multiple implementations of a KDF. However, > > > KDFs relate to hashes like block chaining modes to raw block > > > ciphers. Thus a KDF can be applied with different hashes. > > > > > > My idea was to add template support to RNGs (because KDFs are > > > effectively a type of RNG since they produce an arbitrary output > > > from a fixed input). The KDFs would be a template wrapping hashes. > > > For example, the CTR-KDF from SP800-108 could be instantiated like > > > kdf-ctr(sha256). > > > > > > > > > > I think that, if the crypto API is going to grow a KDF facility, it > > should be done right. Have a key type or flag or whatever that says > > “this key may *only* be used to derive keys using such-and-such > > algorithm”, and have a helper to derive a key. That helper should > > take some useful parameters and mix them in: > > > > - What type of key is being derived? ECDSA signing key? HMAC > > key? AES key? > > > > - Can user code access the derived key? > > > > - What is the key’s purpose? “Encrypt and authenticate a hibernation > > image” would be a purpose. > > > > - Number of bytes. > > > > All of these parameters should be mixed in to the key derivation. > > > > Also, an AE key, even for AES+HMAC, should be just one derived > > key. If you need 512 bits, ask for a 512-bit key, not two 256-bit > > keys. > > Actually, it would be enormously helpful if we could reuse these pieces > for the TPM as well. It has two KDFs: KDFa, which is the CTR-KDF from > SP800-108 and KDFe which is the SP800-56A KDF for elliptic curve one > pass Diffie Hellman, so if we're going to do the former, I'd really > like the latter as well. > > The way the TPM parametrises input to both KDFs is > > (hashAlg, key, label, contextU, contextV, bits) > > Where > > hashAlg = the hash algorithm used as the PRF > key = the input parameter of variable bit size or > the x co-ordinate of the shared point > label = An ASCII string representing the use > contextU = public input U > contextV = public input V > bits = number of output bits > > Is that a good enough parametrisation (not the only way you distinguish > uses is with the label, which is not recoverable)? ContextU and > ContextV are simply concatenated to form the full Context of SP800-108, > but we tend to need two separate inputs (for KDFe they're the public x > co-ordinates of the points of the two inputs to ECDH for instance; in > KDFa they're usually the local and remote nonces). > > The labels for TPM usage are things like "INTEGRITY" for HMAC keys or > "CFB" when generating an aes128-cfb session key. For KDFe, the tpm > seems to like the label "SECRET". Although the TPM specifies fixed > short strings for the label, nothing prevents them being longer like > the purpose you state above (essentially we could mix purpose, use and > key type into the label and the contexts). > That really ought to cover anything the kernel needs. But can you explain what's up with with KDFe? The obvious searches end up with just warnings that the US currently has no government :( Anyway, if we're talking about the TPM, it seems like the entire "trusted key" mechanism in the kernel is missing the point. If I want to encrypt something like a hibernation image on a machine with a TPM, it makes essentially no sense to me that we would get a key with a known raw value that is merely TPM-backed (i.e. the "trusted key") and use that to decrypt the image. The right way to do it is to the use the TPM as it was intended to be used: generate a single-use key that protects the hibernation image and seal *that* directly on the TPM, such that it can only be unsealed with appropriate PCR values. Heck, we could even use one of the fancy NV counters such that we *can't* decrypt the image later on. And using HMAC or any AE construction the normal way is also wrong -- we should *hash* the image and sign the hash directly on the TPM so that the restore code can validate the PCR values that were in place when the hibernation image was created. [0] In other words, I think that a kernel-based encrypted hibernation mechanism should create an image like this: - wrapped key - instructions, if needed, for unwrapping - hash of the entire image except the hash and signature fields - signature of the hash and the remainder is a regular hiberation image that is encrypted against the key. No AE is needed -- just encryption. And there's no trampoline, no weird per-page hashing, etc. Of course, this also means that someone needs to audit the hibernation restore code to make sure that there's no way for a malicious image to gain code execution over the restoring kernel before the verification even runs. Or some much more complicated hash can be used that supports incremental verification. (Also, do we have a sensible story of how the TPM interacts with hibernation at all? Presumably we should at least try to replay the PCR operations that have occurred so that we can massage the PCRs into the same state post-hibernation. Also, do we have any way for the kernel to sign something with the TPM along with an attestation that the signature was requested *by the kernel*? Something like a sub-hierarchy of keys that the kernel explicitly prevents userspace from accessing?) [0] If you take some data, run it through an authenticated encryption algorithm, and sign (key, nonce, tag), I think you're operating outside of the accepted security definitions if you expect this to guarantee that the data wasn't tampered with. I'm reasonably confident that there are quite a few excellent AE algorithms that completely fail if used this like this. In fact, pretty much all of the modern fast ones probably fail. AE is for when the key is *secret*.
Am Mittwoch, 9. Januar 2019, 00:54:22 CET schrieb Andy Lutomirski: Hi Andy, > > I think that, if the crypto API is going to grow a KDF facility, it should > be done right. Have a key type or flag or whatever that says “this key may > *only* be used to derive keys using such-and-such algorithm”, and have a > helper to derive a key. That helper should take some useful parameters and > mix them in: > > - What type of key is being derived? ECDSA signing key? HMAC key? AES > key? > > - Can user code access the derived key? > > - What is the key’s purpose? “Encrypt and authenticate a hibernation image” > would be a purpose. > > - Number of bytes. > > All of these parameters should be mixed in to the key derivation. > > Also, an AE key, even for AES+HMAC, should be just one derived key. If you > need 512 bits, ask for a 512-bit key, not two 256-bit keys. I concur with your requirements. However, is the kernel crypto API the right place to enforce such policies? To me, the kernel crypto API is a tinker-toy set of ciphers. The real policy enforcer would or should be the keyring facility. Thus, may I propose to: - implement the cryptographic primitive of the KDF in the kernel crypto API - implement the policy system how to use the KDF in the keyring facility Ciao Stephan
Am Mittwoch, 9. Januar 2019, 01:44:31 CET schrieb James Bottomley: Hi James, > Actually, it would be enormously helpful if we could reuse these pieces > for the TPM as well. Could you please help me understand whether the KDFs in TPM are directly usable as a standalone cipher primitive or does it go together with additional key generation operations? > It has two KDFs: KDFa, which is the CTR-KDF from > SP800-108 and KDFe which is the SP800-56A KDF for elliptic curve one > pass Diffie Hellman, so if we're going to do the former, I'd really > like the latter as well. > > The way the TPM parametrises input to both KDFs is > > (hashAlg, key, label, contextU, contextV, bits) > > Where > > hashAlg = the hash algorithm used as the PRF > key = the input parameter of variable bit size or > the x co-ordinate of the shared point > label = An ASCII string representing the use > contextU = public input U > contextV = public input V > bits = number of output bits When implementing KDFs as an extension of the kernel crypto API's RNG facility we currently have to handle the limitiation of the existing API. The label/ context data (and when considering RFC 5869 HKDF requring IKM, salt and additional information as input) currently cannot directly be communicated through the API. The issue is that the RNG facility currently has the following prototype defined: int (*generate)(struct crypto_rng *tfm, const u8 *src, unsigned int slen, u8 *dst, unsigned int dlen); The src pointer would need to take the label/context data. Would it be appropriate, to implement a type cast to a structure from the u8 pointer? E.g. for the aforementioned label/context data, we could define something like struct crypto_kdf_ctr { char *label; size_t label_len; u8 *contextU; size_t contextU_len; u8 *contextV; size_t contextV_len; }; And the implementation of the generate function for CTR KDF would then have a type cast along the following lines: if (slen != sizeof(struct crypto_kdf_ctr)) return -EINVAL; const struct crypto_kdf_ctr *kdf_ctr_input = (struct crypto_kdf_ctr *)src; For different KDFs, different structs would be needed. Ciao Stephan
On Tue, 2019-01-08 at 17:43 -0800, Andy Lutomirski wrote: > [Adding Jarkko because this stuff relates to the TPM.] > > On Tue, Jan 8, 2019 at 4:44 PM James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: > > > > On Tue, 2019-01-08 at 15:54 -0800, Andy Lutomirski wrote: > > > > On Jan 7, 2019, at 11:09 PM, Stephan Mueller <smueller@chronox. > > > > de> > > > > wrote: > > > > > > > > Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: > > > > > > > > Hi Herbert, > > > > > > > > > Are we going to have multiple implementations for the same > > > > > KDF? If not then the crypto API is not a good fit. To > > > > > consolidate multiple implementations of the same KDF, simply > > > > > provide helpers for them. > > > > > > > > It is unlikely to have multiple implementations of a KDF. > > > > However, KDFs relate to hashes like block chaining modes to raw > > > > block ciphers. Thus a KDF can be applied with different hashes. > > > > > > > > My idea was to add template support to RNGs (because KDFs are > > > > effectively a type of RNG since they produce an arbitrary > > > > output from a fixed input). The KDFs would be a template > > > > wrapping hashes. For example, the CTR-KDF from SP800-108 could > > > > be instantiated like kdf-ctr(sha256). > > > > > > > > > > > > > > I think that, if the crypto API is going to grow a KDF facility, > > > it should be done right. Have a key type or flag or whatever that > > > says “this key may *only* be used to derive keys using such-and- > > > such algorithm”, and have a helper to derive a key. That helper > > > should take some useful parameters and mix them in: > > > > > > - What type of key is being derived? ECDSA signing key? HMAC > > > key? AES key? > > > > > > - Can user code access the derived key? > > > > > > - What is the key’s purpose? “Encrypt and authenticate a > > > hibernation image” would be a purpose. > > > > > > - Number of bytes. > > > > > > All of these parameters should be mixed in to the key derivation. > > > > > > Also, an AE key, even for AES+HMAC, should be just one derived > > > key. If you need 512 bits, ask for a 512-bit key, not two 256- > > > bit keys. > > > > Actually, it would be enormously helpful if we could reuse these > > pieces for the TPM as well. It has two KDFs: KDFa, which is the > > CTR-KDF from SP800-108 and KDFe which is the SP800-56A KDF for > > elliptic curve one pass Diffie Hellman, so if we're going to do the > > former, I'd really like the latter as well. > > > > The way the TPM parametrises input to both KDFs is > > > > (hashAlg, key, label, contextU, contextV, bits) > > > > Where > > > > hashAlg = the hash algorithm used as the PRF > > key = the input parameter of variable bit size or > > the x co-ordinate of the shared point > > label = An ASCII string representing the use > > contextU = public input U > > contextV = public input V > > bits = number of output bits > > > > Is that a good enough parametrisation (not the only way you > > distinguish uses is with the label, which is not > > recoverable)? ContextU and ContextV are simply concatenated to > > form the full Context of SP800-108, but we tend to need two > > separate inputs (for KDFe they're the public x co-ordinates of the > > points of the two inputs to ECDH for instance; in KDFa they're > > usually the local and remote nonces). > > > > The labels for TPM usage are things like "INTEGRITY" for HMAC keys > > or "CFB" when generating an aes128-cfb session key. For KDFe, the > > tpm seems to like the label "SECRET". Although the TPM specifies > > fixed short strings for the label, nothing prevents them being > > longer like the purpose you state above (essentially we could mix > > purpose, use and key type into the label and the contexts). > > > > That really ought to cover anything the kernel needs. > > But can you explain what's up with with KDFe? The obvious searches > end up with just warnings that the US currently has no government :( You mean you can't find SP100-56A because NIST is a government entity and it's discontinued its website because of the government shutdown? No idea, I only live here, you'll have to ask a real American. ACM does have a copy: http://delivery.acm.org/10.1145/2210000/2206270/SP800-56A_Revision1_Mar08-2007.pdf?ip=50.35.68.20&id=2206270&acc=OPEN&key=4D4702B0C3E38B35%2E4D4702B0C3E38B35%2E4D4702B0C3E38B35%2E6D218144511F3437&__acm__=1546993111_ed9c8bd24b2f838c829d428aac7f5d71 > Anyway, if we're talking about the TPM, it seems like the entire > "trusted key" mechanism in the kernel is missing the point. If I > want to encrypt something like a hibernation image on a machine with > a TPM, it makes essentially no sense to me that we would get a key > with a known raw value that is merely TPM-backed (i.e. the "trusted > key") and use that to decrypt the image. The right way to do it is > to the use the TPM as it was intended to be used: generate a single- > use key that protects the hibernation image and seal *that* directly > on the TPM, such that it can only be unsealed with appropriate PCR > values. Heck, we could even use one of the fancy NV counters such > that we *can't* decrypt the image later on. And using HMAC or any AE > construction the normal way is also wrong -- we should *hash* the > image and sign the hash directly on the TPM so that the restore code > can validate the PCR values that were in place when the hibernation > image was created. [0] Well, theoretically, trusted keys can be used for PCR sealed bundles, at least in 1.2 ... I'm not sure the 2.0 one actually works because you have to construct the policy session outside the kernel. > In other words, I think that a kernel-based encrypted hibernation > mechanism should create an image like this: > > - wrapped key > - instructions, if needed, for unwrapping This sounds like the format we use for TPM resident keys, except it would protect a TPM unseal bundle: https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h > - hash of the entire image except the hash and signature fields > - signature of the hash > > and the remainder is a regular hiberation image that is encrypted > against the key. No AE is needed -- just encryption. And there's no > trampoline, no weird per-page hashing, etc. Of course, this also > means that someone needs to audit the hibernation restore code to > make sure that there's no way for a malicious image to gain code > execution over the restoring kernel before the verification even > runs. Or some much more complicated hash can be used that supports > incremental verification. > > > (Also, do we have a sensible story of how the TPM interacts with > hibernation at all? Not really, no ... there is a TPM patch for LUKS, but trusted keys are unused within the kernel. > Presumably we should at least try to replay the PCR operations that > have occurred so that we can massage the PCRs into the same state > post-hibernation. Also, do we have any way for the kernel to sign > something with the TPM along with an attestation that the signature > was requested *by the kernel*? Something like a sub-hierarchy of > keys that the kernel explicitly prevents userspace from accessing?) We're just growing that now with the TPM asymmetric operations. Attesting that the kernel requested the signature is harder. The TPM can attest to log entries (as it does for the UEFI log and IMA) and it can certify keys, but that only proves they're TPM resident not who the requestor was. Effectively the latter is an assertion about who knows the key authority, which is hard to prove. > [0] If you take some data, run it through an authenticated encryption > algorithm, and sign (key, nonce, tag), I think you're operating > outside of the accepted security definitions if you expect this to > guarantee that the data wasn't tampered with. I'm reasonably > confident that there are quite a few excellent AE algorithms that > completely fail if used this like this. In fact, pretty much all of > the modern fast ones probably fail. AE is for when the key is > *secret*. Well, I think here, if we were actually trying to solve the problem of proving the hibernated image were the same one we would need to prove some log of the kernel operation came to a particular value *after* the hibernated image were restored ... it's not really possible to condition key release which must occur before the restore on that outcome, so it strikes me we need more than a simple release bound to PCR values. James
On Wed, 2019-01-09 at 07:45 +0100, Stephan Mueller wrote: > Am Mittwoch, 9. Januar 2019, 01:44:31 CET schrieb James Bottomley: > > Hi James, > > > Actually, it would be enormously helpful if we could reuse these > > pieces for the TPM as well. > > Could you please help me understand whether the KDFs in TPM are > directly usable as a standalone cipher primitive or does it go > together with additional key generation operations? They're used as generators ... which means they deterministically produce keys from what the TPM calls seeds so we can get crypto agility of TPM 2.0 ... well KDFa does. KDFe is simply what NIST recommends you do when using EC for a shared key agreement ... and really we shouldn't be using ECDH in the kernel without it. > > It has two KDFs: KDFa, which is the CTR-KDF from > > SP800-108 and KDFe which is the SP800-56A KDF for elliptic curve > > one pass Diffie Hellman, so if we're going to do the former, I'd > > really like the latter as well. > > > > The way the TPM parametrises input to both KDFs is > > > > (hashAlg, key, label, contextU, contextV, bits) > > > > Where > > > > hashAlg = the hash algorithm used as the PRF > > key = the input parameter of variable bit size or > > the x co-ordinate of the shared point > > label = An ASCII string representing the use > > contextU = public input U > > contextV = public input V > > bits = number of output bits > > When implementing KDFs as an extension of the kernel crypto API's RNG > facility we currently have to handle the limitiation of the existing > API. The label/context data (and when considering RFC 5869 HKDF > requring IKM, salt and additional information as input) currently > cannot directly be communicated through the API. > > The issue is that the RNG facility currently has the following > prototype defined: > > int (*generate)(struct crypto_rng *tfm, > const u8 *src, unsigned int slen, > u8 *dst, unsigned int dlen); > > The src pointer would need to take the label/context data. That's probably good enough. For both KDFa and KDFe the label contextU and ContextV are concatenated, so in both cases a single source is probably good enough. However, we also need to feed in the key somehow since it's usually used separately in the derivation functions. > Would it be appropriate, to implement a type cast to a structure from > the u8 pointer? > > E.g. for the aforementioned label/context data, we could define > something like > > struct crypto_kdf_ctr { > char *label; > size_t label_len; > u8 *contextU; > size_t contextU_len; > u8 *contextV; > size_t contextV_len; > }; > > And the implementation of the generate function for CTR KDF would > then have a type cast along the following lines: > > if (slen != sizeof(struct crypto_kdf_ctr)) > return -EINVAL; > const struct crypto_kdf_ctr *kdf_ctr_input = (struct > crypto_kdf_ctr *)src; > > > For different KDFs, different structs would be needed. Actually, we probably just need the input key (or secret material), the concatenation and the number of output bits. James
Am Mittwoch, 9. Januar 2019, 07:58:28 CET schrieb James Bottomley: Hi James, > On Wed, 2019-01-09 at 07:45 +0100, Stephan Mueller wrote: > > Am Mittwoch, 9. Januar 2019, 01:44:31 CET schrieb James Bottomley: > > > > Hi James, > > > > > Actually, it would be enormously helpful if we could reuse these > > > pieces for the TPM as well. > > > > Could you please help me understand whether the KDFs in TPM are > > directly usable as a standalone cipher primitive or does it go > > together with additional key generation operations? > > They're used as generators ... which means they deterministically > produce keys from what the TPM calls seeds so we can get crypto agility > of TPM 2.0 ... well KDFa does. KDFe is simply what NIST recommends you > do when using EC for a shared key agreement ... and really we shouldn't > be using ECDH in the kernel without it. > Thanks for clarifying. That would mean that indeed we would have hardware- provided KDF implementations that may be usable with the kernel crypto API. [...] > > > Would it be appropriate, to implement a type cast to a structure from > > the u8 pointer? > > > > E.g. for the aforementioned label/context data, we could define > > something like > > > > struct crypto_kdf_ctr { > > > > char *label; > > size_t label_len; > > u8 *contextU; > > size_t contextU_len; > > u8 *contextV; > > size_t contextV_len; > > > > }; > > > > And the implementation of the generate function for CTR KDF would > > > > then have a type cast along the following lines: > > if (slen != sizeof(struct crypto_kdf_ctr)) > > > > return -EINVAL; > > > > const struct crypto_kdf_ctr *kdf_ctr_input = (struct > > > > crypto_kdf_ctr *)src; > > > > > > For different KDFs, different structs would be needed. > > Actually, we probably just need the input key (or secret material), the > concatenation and the number of output bits. Thanks for confirming. Though, when it comes to HKDF (not that I see it being needed or required in the kernel), there is a need to split up the src pointer since the mentioned input is used in different ways. In order to try to get the implementation and thus the interface right, I would suggest to at least have a consensus on how to handle such situations. Thus, would the proposal be acceptable for such KDFs that may need to have different components communicated as input to the KDF? Ciao Stephan
On Wed, Jan 09, 2019 at 08:05:21AM +0100, Stephan Mueller wrote: > Am Mittwoch, 9. Januar 2019, 07:58:28 CET schrieb James Bottomley: > > Hi James, > > > On Wed, 2019-01-09 at 07:45 +0100, Stephan Mueller wrote: > > > Am Mittwoch, 9. Januar 2019, 01:44:31 CET schrieb James Bottomley: > > > > > > Hi James, > > > > > > > Actually, it would be enormously helpful if we could reuse these > > > > pieces for the TPM as well. > > > > > > Could you please help me understand whether the KDFs in TPM are > > > directly usable as a standalone cipher primitive or does it go > > > together with additional key generation operations? > > > > They're used as generators ... which means they deterministically > > produce keys from what the TPM calls seeds so we can get crypto agility > > of TPM 2.0 ... well KDFa does. KDFe is simply what NIST recommends you > > do when using EC for a shared key agreement ... and really we shouldn't > > be using ECDH in the kernel without it. > > > > Thanks for clarifying. That would mean that indeed we would have hardware- > provided KDF implementations that may be usable with the kernel crypto API. > > [...] > > > > > Would it be appropriate, to implement a type cast to a structure from > > > the u8 pointer? > > > > > > E.g. for the aforementioned label/context data, we could define > > > something like > > > > > > struct crypto_kdf_ctr { > > > > > > char *label; > > > size_t label_len; > > > u8 *contextU; > > > size_t contextU_len; > > > u8 *contextV; > > > size_t contextV_len; > > > > > > }; > > > > > > And the implementation of the generate function for CTR KDF would > > > > > > then have a type cast along the following lines: > > > if (slen != sizeof(struct crypto_kdf_ctr)) > > > > > > return -EINVAL; > > > > > > const struct crypto_kdf_ctr *kdf_ctr_input = (struct > > > > > > crypto_kdf_ctr *)src; > > > > > > > > > For different KDFs, different structs would be needed. > > > > Actually, we probably just need the input key (or secret material), the > > concatenation and the number of output bits. > > Thanks for confirming. Though, when it comes to HKDF (not that I see it being > needed or required in the kernel), there is a need to split up the src pointer > since the mentioned input is used in different ways. > > In order to try to get the implementation and thus the interface right, I > would suggest to at least have a consensus on how to handle such situations. > > Thus, would the proposal be acceptable for such KDFs that may need to have > different components communicated as input to the KDF? > FWIW, it's been very slow going since I've been working on other projects and I also need to be very sure to get the API changes right, but I still plan to change the KDF in fscrypt (a.k.a. ext4/f2fs/ubifs encryption) to HKDF-SHA512 as part of a larger set of improvements to how fscrypt encryption keys are managed. I sent the last patchset a year ago (https://marc.info/?l=linux-fsdevel&m=150879493206257) but I'm working to revive it. In the work-in-progress version in my git tree, this is the commit that adds a HKDF implementation as fs/crypto/hkdf.c: https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/commit/?id=e8a78767131c9717ee838f0c4e307948d65a4427 It basically just wraps a crypto_shash for "hmac(sha512)". I'd be fine with using a common implementation instead, provided that it gives the same functionality, including supporting user-specified salt and application-specific info strings, and isn't slower or more complex to use. (This comment is solely on the tangential discussion about KDF implementations; I've not looked at the hibernation image encryption stuff yet.) - Eric
Am Mittwoch, 9. Januar 2019, 09:21:04 CET schrieb Eric Biggers: Hi Eric, > > FWIW, it's been very slow going since I've been working on other projects > and I also need to be very sure to get the API changes right, but I still > plan to change the KDF in fscrypt (a.k.a. ext4/f2fs/ubifs encryption) to > HKDF-SHA512 as part of a larger set of improvements to how fscrypt > encryption keys are managed. I sent the last patchset a year ago > (https://marc.info/?l=linux-fsdevel&m=150879493206257) but I'm working to > revive it. In the work-in-progress version in my git tree, this is the > commit that adds a HKDF implementation as fs/crypto/hkdf.c: > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/commit/?i > d=e8a78767131c9717ee838f0c4e307948d65a4427 It basically just wraps a > crypto_shash for "hmac(sha512)". > > I'd be fine with using a common implementation instead, provided that it > gives the same functionality, including supporting user-specified salt and > application-specific info strings, and isn't slower or more complex to use. > > (This comment is solely on the tangential discussion about KDF > implementations; I've not looked at the hibernation image encryption stuff > yet.) Thanks for the clarification. I have started a generic HKDF implementation for the kernel crypto API which lead to the questions above. I would then also try to provide a HKDF proposal. To use the (H)KDF, I currently envision 2 calls apart from alloc/free. The following code would serve as an example. * Example without proper error handling: * char *keying_material = "\x00\x11\x22\x33\x44\x55\x66\x77"; * char *label_context = "\xde\xad\xbe\xef\x00\xde\xad\xbe\xef"; * kdf = crypto_alloc_rng(name, 0, 0); * crypto_rng_reset(kdf, keying_material, 8); * crypto_rng_generate(kdf, label_context, 9, outbuf, outbuflen); That hopefully should be simple enough. For HKDF, as mentioned, I would envision to use a struct instead of a char * for the label_context to communicate IKM, Salt, and the label/info information. Ciao Stephan
On Wed, 2019-01-09 at 08:05 +0100, Stephan Mueller wrote: > Am Mittwoch, 9. Januar 2019, 07:58:28 CET schrieb James Bottomley: > > Hi James, > > > On Wed, 2019-01-09 at 07:45 +0100, Stephan Mueller wrote: > > > Am Mittwoch, 9. Januar 2019, 01:44:31 CET schrieb James > > > Bottomley: > > > > > > Hi James, > > > > > > > Actually, it would be enormously helpful if we could reuse > > > > these pieces for the TPM as well. > > > > > > Could you please help me understand whether the KDFs in TPM are > > > directly usable as a standalone cipher primitive or does it go > > > together with additional key generation operations? > > > > They're used as generators ... which means they deterministically > > produce keys from what the TPM calls seeds so we can get crypto > > agility of TPM 2.0 ... well KDFa does. KDFe is simply what NIST > > recommends you do when using EC for a shared key agreement ... and > > really we shouldn't be using ECDH in the kernel without it. > > > > Thanks for clarifying. That would mean that indeed we would have > hardware-provided KDF implementations that may be usable with the > kernel crypto API. Just on this point, the TPM doesn't actually provide any KDFa or e API, so it can't be used for hardware acceleration (and even if it did, the TPM is a pretty slow engine, so software would be faster anyway). We need these algorithms in software because the TPM uses key agreements derived from shared secrets to produce session encryption keys to ensure confidentiality and integrity (HMAC key), so we establish the shared secret then have to derive our key in software and the TPM derives the same key internally and we use the shared derived key to symmetrically encrypt and/or HMAC secret communications. James
On Wed, Jan 09, 2019 at 11:17:45AM +0100, Stephan Mueller wrote: > Am Mittwoch, 9. Januar 2019, 09:21:04 CET schrieb Eric Biggers: > > Hi Eric, > > > > FWIW, it's been very slow going since I've been working on other projects > > and I also need to be very sure to get the API changes right, but I still > > plan to change the KDF in fscrypt (a.k.a. ext4/f2fs/ubifs encryption) to > > HKDF-SHA512 as part of a larger set of improvements to how fscrypt > > encryption keys are managed. I sent the last patchset a year ago > > (https://marc.info/?l=linux-fsdevel&m=150879493206257) but I'm working to > > revive it. In the work-in-progress version in my git tree, this is the > > commit that adds a HKDF implementation as fs/crypto/hkdf.c: > > https://git.kernel.org/pub/scm/linux/kernel/git/ebiggers/linux.git/commit/?id=e8a78767131c9717ee838f0c4e307948d65a4427 > > It basically just wraps a crypto_shash for "hmac(sha512)". > > > > I'd be fine with using a common implementation instead, provided that it > > gives the same functionality, including supporting user-specified salt and > > application-specific info strings, and isn't slower or more complex to use. > > > > (This comment is solely on the tangential discussion about KDF > > implementations; I've not looked at the hibernation image encryption stuff > > yet.) > > Thanks for the clarification. I have started a generic HKDF implementation for > the kernel crypto API which lead to the questions above. I would then also try > to provide a HKDF proposal. > > To use the (H)KDF, I currently envision 2 calls apart from alloc/free. The > following code would serve as an example. > > * Example without proper error handling: > * char *keying_material = "\x00\x11\x22\x33\x44\x55\x66\x77"; > * char *label_context = "\xde\xad\xbe\xef\x00\xde\xad\xbe\xef"; > * kdf = crypto_alloc_rng(name, 0, 0); > * crypto_rng_reset(kdf, keying_material, 8); > * crypto_rng_generate(kdf, label_context, 9, outbuf, outbuflen); > > That hopefully should be simple enough. > > For HKDF, as mentioned, I would envision to use a struct instead of a char * > for the label_context to communicate IKM, Salt, and the label/info > information. > > Ciao > Stephan > That would not meet my performance requirements as I want to precompute HKDF-Extract, and then do HKDF-Expand many times. Also the HKDF-Expand part should be thread-safe and not require allocating memory, especially not a whole crypto_shash tfm every time. So presumably with crypto_rng, crypto_rng_reset() would need to take the input keyring material and salt and do HKDF-Extract (like my fscrypt_init_hkdf()), and crypto_rng_generate() would need to take the application-specific info string and do HKDF-Expand (like my fscrypt_hkdf_expand()). It is ugly though. Please also consider just having simple crypto_hkdf_*() helper functions which wrap a HMAC tfm along the lines of my patch, rather than shoehorning it into the crypto_rng API. - Eric
Hi James On Tue, Jan 08, 2019 at 10:49:39PM -0800, James Bottomley wrote: > On Tue, 2019-01-08 at 17:43 -0800, Andy Lutomirski wrote: > > [Adding Jarkko because this stuff relates to the TPM.] > > > > On Tue, Jan 8, 2019 at 4:44 PM James Bottomley > > <James.Bottomley@hansenpartnership.com> wrote: > > > > > > On Tue, 2019-01-08 at 15:54 -0800, Andy Lutomirski wrote: > > > > > On Jan 7, 2019, at 11:09 PM, Stephan Mueller <smueller@chronox. > > > > > de> > > > > > wrote: > > > > > > > > > > Am Dienstag, 8. Januar 2019, 06:03:58 CET schrieb Herbert Xu: > > > > > > > > > > Hi Herbert, > > > > > > > > > > > Are we going to have multiple implementations for the same > > > > > > KDF? If not then the crypto API is not a good fit. To > > > > > > consolidate multiple implementations of the same KDF, simply > > > > > > provide helpers for them. > > > > > > > > > > It is unlikely to have multiple implementations of a KDF. > > > > > However, KDFs relate to hashes like block chaining modes to raw > > > > > block ciphers. Thus a KDF can be applied with different hashes. > > > > > > > > > > My idea was to add template support to RNGs (because KDFs are > > > > > effectively a type of RNG since they produce an arbitrary > > > > > output from a fixed input). The KDFs would be a template > > > > > wrapping hashes. For example, the CTR-KDF from SP800-108 could > > > > > be instantiated like kdf-ctr(sha256). > > > > > > > > > > > > > > > > > > I think that, if the crypto API is going to grow a KDF facility, > > > > it should be done right. Have a key type or flag or whatever that > > > > says “this key may *only* be used to derive keys using such-and- > > > > such algorithm”, and have a helper to derive a key. That helper > > > > should take some useful parameters and mix them in: > > > > > > > > - What type of key is being derived? ECDSA signing key? HMAC > > > > key? AES key? > > > > > > > > - Can user code access the derived key? > > > > > > > > - What is the key’s purpose? “Encrypt and authenticate a > > > > hibernation image” would be a purpose. > > > > > > > > - Number of bytes. > > > > > > > > All of these parameters should be mixed in to the key derivation. > > > > > > > > Also, an AE key, even for AES+HMAC, should be just one derived > > > > key. If you need 512 bits, ask for a 512-bit key, not two 256- > > > > bit keys. > > > > > > Actually, it would be enormously helpful if we could reuse these > > > pieces for the TPM as well. It has two KDFs: KDFa, which is the > > > CTR-KDF from SP800-108 and KDFe which is the SP800-56A KDF for > > > elliptic curve one pass Diffie Hellman, so if we're going to do the > > > former, I'd really like the latter as well. > > > > > > The way the TPM parametrises input to both KDFs is > > > > > > (hashAlg, key, label, contextU, contextV, bits) > > > > > > Where > > > > > > hashAlg = the hash algorithm used as the PRF > > > key = the input parameter of variable bit size or > > > the x co-ordinate of the shared point > > > label = An ASCII string representing the use > > > contextU = public input U > > > contextV = public input V > > > bits = number of output bits > > > > > > Is that a good enough parametrisation (not the only way you > > > distinguish uses is with the label, which is not > > > recoverable)? ContextU and ContextV are simply concatenated to > > > form the full Context of SP800-108, but we tend to need two > > > separate inputs (for KDFe they're the public x co-ordinates of the > > > points of the two inputs to ECDH for instance; in KDFa they're > > > usually the local and remote nonces). > > > > > > The labels for TPM usage are things like "INTEGRITY" for HMAC keys > > > or "CFB" when generating an aes128-cfb session key. For KDFe, the > > > tpm seems to like the label "SECRET". Although the TPM specifies > > > fixed short strings for the label, nothing prevents them being > > > longer like the purpose you state above (essentially we could mix > > > purpose, use and key type into the label and the contexts). > > > > > > > That really ought to cover anything the kernel needs. > > > > But can you explain what's up with with KDFe? The obvious searches > > end up with just warnings that the US currently has no government :( > > You mean you can't find SP100-56A because NIST is a government entity > and it's discontinued its website because of the government shutdown? > No idea, I only live here, you'll have to ask a real American. > > ACM does have a copy: > > http://delivery.acm.org/10.1145/2210000/2206270/SP800-56A_Revision1_Mar08-2007.pdf?ip=50.35.68.20&id=2206270&acc=OPEN&key=4D4702B0C3E38B35%2E4D4702B0C3E38B35%2E4D4702B0C3E38B35%2E6D218144511F3437&__acm__=1546993111_ed9c8bd24b2f838c829d428aac7f5d71 > > > Anyway, if we're talking about the TPM, it seems like the entire > > "trusted key" mechanism in the kernel is missing the point. If I > > want to encrypt something like a hibernation image on a machine with > > a TPM, it makes essentially no sense to me that we would get a key > > with a known raw value that is merely TPM-backed (i.e. the "trusted > > key") and use that to decrypt the image. The right way to do it is > > to the use the TPM as it was intended to be used: generate a single- > > use key that protects the hibernation image and seal *that* directly > > on the TPM, such that it can only be unsealed with appropriate PCR > > values. Heck, we could even use one of the fancy NV counters such > > that we *can't* decrypt the image later on. And using HMAC or any AE > > construction the normal way is also wrong -- we should *hash* the > > image and sign the hash directly on the TPM so that the restore code > > can validate the PCR values that were in place when the hibernation > > image was created. [0] > > Well, theoretically, trusted keys can be used for PCR sealed bundles, > at least in 1.2 ... I'm not sure the 2.0 one actually works because you > have to construct the policy session outside the kernel. > > > In other words, I think that a kernel-based encrypted hibernation > > mechanism should create an image like this: > > > > - wrapped key > > - instructions, if needed, for unwrapping > > This sounds like the format we use for TPM resident keys, except it > would protect a TPM unseal bundle: > > https://git.kernel.org/pub/scm/linux/kernel/git/jejb/openssl_tpm2_engine.git/tree/tpm2-asn.h > > > - hash of the entire image except the hash and signature fields > > - signature of the hash > > > > and the remainder is a regular hiberation image that is encrypted > > against the key. No AE is needed -- just encryption. And there's no > > trampoline, no weird per-page hashing, etc. Of course, this also > > means that someone needs to audit the hibernation restore code to > > make sure that there's no way for a malicious image to gain code > > execution over the restoring kernel before the verification even > > runs. Or some much more complicated hash can be used that supports > > incremental verification. > > > > > > (Also, do we have a sensible story of how the TPM interacts with > > hibernation at all? > > Not really, no ... there is a TPM patch for LUKS, but trusted keys are > unused within the kernel. > The trusted key has be used to produce encrypted key for EVM: https://sourceforge.net/p/linux-ima/wiki/Home/#creating-trusted-and-evm-encrypted-keys > > Presumably we should at least try to replay the PCR operations that > > have occurred so that we can massage the PCRs into the same state > > post-hibernation. Also, do we have any way for the kernel to sign > > something with the TPM along with an attestation that the signature > > was requested *by the kernel*? Something like a sub-hierarchy of > > keys that the kernel explicitly prevents userspace from accessing?) > > We're just growing that now with the TPM asymmetric operations. > Attesting that the kernel requested the signature is harder. The TPM > can attest to log entries (as it does for the UEFI log and IMA) and it > can certify keys, but that only proves they're TPM resident not who the > requestor was. Effectively the latter is an assertion about who knows > the key authority, which is hard to prove. > > > [0] If you take some data, run it through an authenticated encryption > > algorithm, and sign (key, nonce, tag), I think you're operating > > outside of the accepted security definitions if you expect this to > > guarantee that the data wasn't tampered with. I'm reasonably > > confident that there are quite a few excellent AE algorithms that > > completely fail if used this like this. In fact, pretty much all of > > the modern fast ones probably fail. AE is for when the key is > > *secret*. > > Well, I think here, if we were actually trying to solve the problem of > proving the hibernated image were the same one we would need to prove > some log of the kernel operation came to a particular value *after* the > hibernated image were restored ... it's not really possible to > condition key release which must occur before the restore on that > outcome, so it strikes me we need more than a simple release bound to > PCR values. > hm... I am studying your information. But I have a question... If PCR is not capped and the root be compromised, is it possible that a sealed bundle also be compromised? Is it possible that kernel can produce a sealed key with PCR by TPM when booting? Then kernel caps a PCR by a constant value before the root is available for userland. Then the sealed key can be exposed to userland or be attached on hibernate image. Even the root be compromised, the TPM trusted key is still secure. Thanks a lot! Joey Lee
Am Mittwoch, 9. Januar 2019, 18:34:55 CET schrieb Eric Biggers: Hi Eric, > That would not meet my performance requirements as I want to precompute > HKDF-Extract, and then do HKDF-Expand many times. Also the HKDF-Expand part > should be thread-safe and not require allocating memory, especially not a > whole crypto_shash tfm every time. > > So presumably with crypto_rng, crypto_rng_reset() would need to take the > input keyring material and salt and do HKDF-Extract (like my > fscrypt_init_hkdf()), and crypto_rng_generate() would need to take the > application-specific info string and do HKDF-Expand (like my > fscrypt_hkdf_expand()). Great, that was the idea I had in mind as well. Maybe the example was not right to convey that. Let me work on that. > > It is ugly though. Please also consider just having simple crypto_hkdf_*() > helper functions which wrap a HMAC tfm along the lines of my patch, rather > than shoehorning it into the crypto_rng API. > > - Eric Ciao Stephan
>> On Jan 8, 2019, at 10:49 PM, James Bottomley <James.Bottomley@hansenpartnership.com> wrote: >> >> On Tue, 2019-01-08 at 17:43 -0800, Andy Lutomirski wrote: >> [Adding Jarkko because this stuff relates to the TPM.] > >> Anyway, if we're talking about the TPM, it seems like the entire >> "trusted key" mechanism in the kernel is missing the point. If I >> want to encrypt something like a hibernation image on a machine with >> a TPM, it makes essentially no sense to me that we would get a key >> with a known raw value that is merely TPM-backed (i.e. the "trusted >> key") and use that to decrypt the image. The right way to do it is >> to the use the TPM as it was intended to be used: generate a single- >> use key that protects the hibernation image and seal *that* directly >> on the TPM, such that it can only be unsealed with appropriate PCR >> values. Heck, we could even use one of the fancy NV counters such >> that we *can't* decrypt the image later on. And using HMAC or any AE >> construction the normal way is also wrong -- we should *hash* the >> image and sign the hash directly on the TPM so that the restore code >> can validate the PCR values that were in place when the hibernation >> image was created. [0] > > Well, theoretically, trusted keys can be used for PCR sealed bundles, > at least in 1.2 ... I'm not sure the 2.0 one actually works because you > have to construct the policy session outside the kernel. I suppose I should go read the 2.0 spec. I’ve read the 1.2 spec, but I always assumed that 2.0 was essentially a superset of 1.2 functionality. >> Presumably we should at least try to replay the PCR operations that >> have occurred so that we can massage the PCRs into the same state >> post-hibernation. Also, do we have any way for the kernel to sign >> something with the TPM along with an attestation that the signature >> was requested *by the kernel*? Something like a sub-hierarchy of >> keys that the kernel explicitly prevents userspace from accessing?) > > We're just growing that now with the TPM asymmetric operations. > Attesting that the kernel requested the signature is harder. The TPM > can attest to log entries (as it does for the UEFI log and IMA) and it > can certify keys, but that only proves they're TPM resident not who the > requestor was. Effectively the latter is an assertion about who knows > the key authority, which is hard to prove. Can the kernel filter TPM 2.0 operations? If so, then a signature that the kernel would have prevented user code from generating is de facto an attestation that the kernel generated it (or that the kernel was compromised, which is sort of equivalent). > >> [0] If you take some data, run it through an authenticated encryption >> algorithm, and sign (key, nonce, tag), I think you're operating >> outside of the accepted security definitions if you expect this to >> guarantee that the data wasn't tampered with. I'm reasonably >> confident that there are quite a few excellent AE algorithms that >> completely fail if used this like this. In fact, pretty much all of >> the modern fast ones probably fail. AE is for when the key is >> *secret*. > > Well, I think here, if we were actually trying to solve the problem of > proving the hibernated image were the same one we would need to prove > some log of the kernel operation came to a particular value *after* the > hibernated image were restored ... it's not really possible to > condition key release which must occur before the restore on that > outcome, so it strikes me we need more than a simple release bound to > PCR values. I’m not sure I follow. Here are the two properties I’d like to see: 1. If you have an encrypted hibernation image, the only thing you should be able to do with it is to restore it. So only an actual Linux kernel in hibernation restore mode ought to be able to restore it. We get this if the image can only be read with appropriate PCRs and then only by the kernel. This way, you can’t just read out secrets from the image if you steal a laptop — you have to actually boot the thing. 2. You shouldn’t be able to create an intentionally corrupt image that pwns you when you restore it unless you have already pwned the kernel. Maybe the “kernel” bit in #1 can be relaxed to “root” without totally defeating the purpose, but if some random non-root process that happens to have access to /dev/tpm* can make a valid-looking TPM image, then I think we fail. Limiting it to the kernel is only dubiously better than limiting it to root until we implement lockdown, in which case it's important. #2 only really matters with lockdown. I suppose that a good summary of my opinion is that there is no point to kernel support for encrypted hibernation images until lockdown is upstream. --Andy
On Wed, 2019-01-09 at 10:34 -0800, Andy Lutomirski wrote: > > > On Jan 8, 2019, at 10:49 PM, James Bottomley <James.Bottomley@han > > > senpartnership.com> wrote: > > > > > > On Tue, 2019-01-08 at 17:43 -0800, Andy Lutomirski wrote: > > > [Adding Jarkko because this stuff relates to the TPM.] > > > Anyway, if we're talking about the TPM, it seems like the entire > > > "trusted key" mechanism in the kernel is missing the point. If I > > > want to encrypt something like a hibernation image on a machine > > > with a TPM, it makes essentially no sense to me that we would get > > > a key with a known raw value that is merely TPM-backed (i.e. the > > > "trusted key") and use that to decrypt the image. The right way > > > to do it is to the use the TPM as it was intended to be used: > > > generate a single-use key that protects the hibernation image and > > > seal *that* directly on the TPM, such that it can only be > > > unsealed with appropriate PCR values. Heck, we could even use > > > one of the fancy NV counters such that we *can't* decrypt the > > > image later on. And using HMAC or any AE construction the normal > > > way is also wrong -- we should *hash* the image and sign the hash > > > directly on the TPM so that the restore code can validate the PCR > > > values that were in place when the hibernation image was > > > created. [0] > > > > Well, theoretically, trusted keys can be used for PCR sealed > > bundles, at least in 1.2 ... I'm not sure the 2.0 one actually > > works because you have to construct the policy session outside the > > kernel. > > I suppose I should go read the 2.0 spec. I’ve read the 1.2 spec, but > I always assumed that 2.0 was essentially a superset of 1.2 > functionality. It was sold as an incremental upgrade, but in practice, adding crypto agility and flexible policy (the main 2.0 enhancements) meant that the API is redically different. > > > Presumably we should at least try to replay the PCR operations > > > that have occurred so that we can massage the PCRs into the same > > > state post-hibernation. Also, do we have any way for the kernel > > > to sign something with the TPM along with an attestation that the > > > signature was requested *by the kernel*? Something like a sub- > > > hierarchy of keys that the kernel explicitly prevents userspace > > > from accessing?) > > > > We're just growing that now with the TPM asymmetric operations. > > Attesting that the kernel requested the signature is harder. The > > TPM can attest to log entries (as it does for the UEFI log and IMA) > > and it can certify keys, but that only proves they're TPM resident > > not who the requestor was. Effectively the latter is an assertion > > about who knows the key authority, which is hard to prove. > > Can the kernel filter TPM 2.0 operations? There is a proposal for this, but the proposal is on the operations by type, not the content. Meaning we can't really police loading and using a kernel key because we can't distinguish whether any key is a kernel key. We can't forbid all key load operations to non-kernel because that removes most of the TPM usefulness as a keystore. > If so, then a signature that the kernel would have prevented user > code from generating is de facto an attestation that the kernel > generated it (or that the kernel was compromised, which is sort of > equivalent). The TPM's idea of this is it polices by authorization. Now one of the things we can do here is add what's called locality based authorization. we have three non-uefi localities to play with and we could enforce walling one off for the kernel only to use, so a kernel key could come with a policy requiring use of the kernel locality for use of the key. That would give you an effective guarantee that only the kernel could use this key. Note the enforcement of locality would require a key policy, which is easy for TPM 1.2, but requires the use of a policy session for TPM 2.0 which means we'd have to improve our policy session handling. > > > [0] If you take some data, run it through an authenticated > > > encryption algorithm, and sign (key, nonce, tag), I think you're > > > operating outside of the accepted security definitions if you > > > expect this to guarantee that the data wasn't tampered with. I'm > > > reasonably confident that there are quite a few excellent AE > > > algorithms that completely fail if used this like this. In fact, > > > pretty much all of the modern fast ones probably fail. AE is for > > > when the key is *secret*. > > > > Well, I think here, if we were actually trying to solve the problem > > of proving the hibernated image were the same one we would need to > > prove some log of the kernel operation came to a particular value > > *after* the hibernated image were restored ... it's not really > > possible to condition key release which must occur before the > > restore on that outcome, so it strikes me we need more than a > > simple release bound to PCR values. > > I’m not sure I follow. Here are the two properties I’d like to see: > > 1. If you have an encrypted hibernation image, the only thing you > should be able to do with it is to restore it. So only an actual > Linux kernel in hibernation restore mode ought to be able to restore > it. We get this if the image can only be read with appropriate PCRs > and then only by the kernel. This way, you can’t just read out > secrets from the image if you steal a laptop — you have to actually > boot the thing. Right, this we can do and if you use a TPM sealed encryption key, you can guarantee the image will only restore on the same physical system. You don't need PCRs for this, just the TPM and the locality enforcement. Note if someone has your laptop and the ability to boot their own kernels, they could always corrupt the kernel into decrypting the image or giving you the unsealed key, but there's no real way of preventing that even with PCR sealing or lockdown, so the basis for the threat model is very much my laptop in my possession running my kernel. > 2. You shouldn’t be able to create an intentionally corrupt image > that pwns you when you restore it unless you have already pwned the > kernel. So here there's a problem: the policy stated above governs key *use* not key creation, so anyone can create a key that has a locality restriction. The way to guarantee that the key was created by something that has access to the locality is to have a parent key with a locality use policy (key creation requires parent key use authorization). Which means every system would have to create a persistent parent key for the kernel to use (the kernel could do this and it could be made NV resident for persistence, so it's not impossible, just complicated). > Maybe the “kernel” bit in #1 can be relaxed to “root” without totally > defeating the purpose, but if some random non-root process that > happens to have access to /dev/tpm* can make a valid-looking TPM > image, then I think we fail. Limiting it to the kernel is only > dubiously better than limiting it to root until we implement > lockdown, in which case it's important. > > #2 only really matters with lockdown. > > I suppose that a good summary of my opinion is that there is no point > to kernel support for encrypted hibernation images until lockdown is > upstream. I really don't think lockdown helps. If we implement locality isolation for the kernels use of keys, a properly functioning kernel isn't going to be tricked into releasing one of its keys to userspace. A buggy kernel might be exploited to cause it to give one up but that would happen irrespective of lockdown and, of course, all bets are off if the attacker can boot their own kernel. James
On Wed, Jan 9, 2019 at 11:46 AM James Bottomley <James.Bottomley@hansenpartnership.com> wrote: > > On Wed, 2019-01-09 at 10:34 -0800, Andy Lutomirski wrote: > > > > On Jan 8, 2019, at 10:49 PM, James Bottomley <James.Bottomley@han > > > > senpartnership.com> wrote: > > > > > > If so, then a signature that the kernel would have prevented user > > code from generating is de facto an attestation that the kernel > > generated it (or that the kernel was compromised, which is sort of > > equivalent). > > The TPM's idea of this is it polices by authorization. Now one of the > things we can do here is add what's called locality based > authorization. we have three non-uefi localities to play with and we > could enforce walling one off for the kernel only to use, so a kernel > key could come with a policy requiring use of the kernel locality for > use of the key. That would give you an effective guarantee that only > the kernel could use this key. Note the enforcement of locality would > require a key policy, which is easy for TPM 1.2, but requires the use > of a policy session for TPM 2.0 which means we'd have to improve our > policy session handling. Hmm. On an *extremely* brief reading of what "locality" means, this seems entirely sensible. > > > > I’m not sure I follow. Here are the two properties I’d like to see: > > > > 1. If you have an encrypted hibernation image, the only thing you > > should be able to do with it is to restore it. So only an actual > > Linux kernel in hibernation restore mode ought to be able to restore > > it. We get this if the image can only be read with appropriate PCRs > > and then only by the kernel. This way, you can’t just read out > > secrets from the image if you steal a laptop — you have to actually > > boot the thing. > > Right, this we can do and if you use a TPM sealed encryption key, you > can guarantee the image will only restore on the same physical system. > You don't need PCRs for this, just the TPM and the locality > enforcement. > > Note if someone has your laptop and the ability to boot their own > kernels, they could always corrupt the kernel into decrypting the image > or giving you the unsealed key, but there's no real way of preventing > that even with PCR sealing or lockdown, so the basis for the threat > model is very much my laptop in my possession running my kernel. I'm not entirely sure I agree. With a TPM-aware bootloader, it really ought to be possible to seal to PCRs such that a corrupted kernel can't restore the image. Obviously a *compromised* but otherwise valid kernel will be able to restore the image. But this is all barking up the wrong tree. If you want your laptop to resist physical theft such that whoever stole it can boot it but can't directly extract any data, you want to use dm-crypt (or, haha, OPAL, but I just read a paper about some people who evaluated a bunch of drives and had a very hard time finding one that actually implemented OPAL in a usefully secure manner). A LUKS replacement or wrapper that does something intelligent with the TPM would be great. This kind of thing IMO does not belong in hibernation. IOW, I think we do get about as much as we would want if we just seal with a locality that only allows kernel use and ignore PCRs entirely. This makes it so that you need the ability to run ring 0 code to decrypt the image, which means that we get all the nice lockdown properties. > > > 2. You shouldn’t be able to create an intentionally corrupt image > > that pwns you when you restore it unless you have already pwned the > > kernel. > > So here there's a problem: the policy stated above governs key *use* > not key creation, so anyone can create a key that has a locality > restriction. The way to guarantee that the key was created by > something that has access to the locality is to have a parent key with > a locality use policy (key creation requires parent key use > authorization). Which means every system would have to create a > persistent parent key for the kernel to use (the kernel could do this > and it could be made NV resident for persistence, so it's not > impossible, just complicated). Why does anything here need to be persistent? The kernel could create a locality-restricted key on the fly, use it to sign and/or seal the hibernation image, and write the wrapped key blob into the hibernation image. > I suppose that a good summary of my opinion is that there is no point > > to kernel support for encrypted hibernation images until lockdown is > > upstream. > > I really don't think lockdown helps. If we implement locality > isolation for the kernels use of keys, a properly functioning kernel > isn't going to be tricked into releasing one of its keys to userspace. > A buggy kernel might be exploited to cause it to give one up but that > would happen irrespective of lockdown and, of course, all bets are off > if the attacker can boot their own kernel. > I'm not saying that lockdown helps. I'm saying that encrypting the hibernation image in the kernel may be pointless until the kernel supports lockdown. If we don't support lockdown, then user code can encrypt the hibernation image all by itself. The code will be easier to understand, more flexible, and won't require a kernel upgrade :) Honestly, no one should be using resume= anyway. Any distro with hibernation support worth its salt should support having the hibernation image on a dm-crypt volume, in which case it *must* support userspace-driven resume. Of course, my laptop that's been upgraded through many Fedora revisions doesn't survive a hibernate/resume cycle, but that's not the kernel's fault. --Andy P.S. One thing I do want to try is encrypted *swap*. The keying for that is trivial -- the kernel can just make a key, store it in ordinary kernel memory, and use it to encrypt and decrypt swap pages as needed. Getting replay protection or authentication may be tricky due to the need for metadata, but plain old AES-XTS or HPolyChaChaNotSpeckAnymore would be entirely straightforward and would get 90% of the benefit. Sure, swap could live on dm-crypt too, but that's an administration mess, and offering a strong and cheap alternative to mlock() for crypto programs to protect their secrets would be fantastic and encrypted swap plus some API to verify that anonymous memory is, in fact, backed by encrypted swap would do the job.
On Wed, 2019-01-09 at 12:12 -0800, Andy Lutomirski wrote: > On Wed, Jan 9, 2019 at 11:46 AM James Bottomley > <James.Bottomley@hansenpartnership.com> wrote: [...] > > > I’m not sure I follow. Here are the two properties I’d like to > > > see: > > > > > > 1. If you have an encrypted hibernation image, the only thing you > > > should be able to do with it is to restore it. So only an actual > > > Linux kernel in hibernation restore mode ought to be able to > > > restore it. We get this if the image can only be read with > > > appropriate PCRs and then only by the kernel. This way, you > > > can’t just read out secrets from the image if you steal a laptop > > > — you have to actually boot the thing. > > > > Right, this we can do and if you use a TPM sealed encryption key, > > you can guarantee the image will only restore on the same physical > > system. You don't need PCRs for this, just the TPM and the locality > > enforcement. > > > > Note if someone has your laptop and the ability to boot their own > > kernels, they could always corrupt the kernel into decrypting the > > image or giving you the unsealed key, but there's no real way of > > preventing that even with PCR sealing or lockdown, so the basis for > > the threat model is very much my laptop in my possession running my > > kernel. > > I'm not entirely sure I agree. With a TPM-aware bootloader, it > really ought to be possible to seal to PCRs such that a corrupted > kernel can't restore the image. Obviously a *compromised* but > otherwise valid kernel will be able to restore the image. It is possible to seal the key so that only the same booted kernel can restore the image, yes. One of the measurements that goes into the boot log is the hash of the kernel and you can seal to this value ... obviously if you upgrade your kernel RPM (or shim or grub) this value changes and you'd lose the ability to restore the hibernated image, but since the image is very kernel specific, that's probably OK. > But this is all barking up the wrong tree. If you want your laptop > to resist physical theft such that whoever stole it can boot it but > can't directly extract any data, you want to use dm-crypt (or, haha, > OPAL, but I just read a paper about some people who evaluated a bunch > of drives and had a very hard time finding one that actually > implemented OPAL in a usefully secure manner). A LUKS replacement or > wrapper that does something intelligent with the TPM would be > great. This kind of thing IMO does not belong in hibernation. Right, so the simplest way of doing this is to save the image on an encrypted partition or filesystem which must be unlocked by a password on boot. > IOW, I think we do get about as much as we would want if we just seal > with a locality that only allows kernel use and ignore PCRs entirely. > This makes it so that you need the ability to run ring 0 code to > decrypt the image, which means that we get all the nice lockdown > properties. Yes, it's a useful balance of security and ease of implementation, I think. > > > 2. You shouldn’t be able to create an intentionally corrupt image > > > that pwns you when you restore it unless you have already pwned > > > the kernel. > > > > So here there's a problem: the policy stated above governs key > > *use* not key creation, so anyone can create a key that has a > > locality restriction. The way to guarantee that the key was > > created by something that has access to the locality is to have a > > parent key with a locality use policy (key creation requires parent > > key use authorization). Which means every system would have to > > create a persistent parent key for the kernel to use (the kernel > > could do this and it could be made NV resident for persistence, so > > it's not impossible, just complicated). > > Why does anything here need to be persistent? The kernel could > create a locality-restricted key on the fly, use it to sign and/or > seal the hibernation image, and write the wrapped key blob into the > hibernation image. Yes, but so could I as a malicious user with a desire to create a bad hibernation image. I could do it entirely in userspace. I need access to the TPM to seal the key, but I'd get that: All you see in-kernel is a single policy sha256 sum and you can't reverse that back to the actual policy, so the kernel has no idea what policy is being sealed and so can't police the policy. In order to get the security that only the kernel created the hibernation key, you need to have a key parent with a policy that only allows in-kernel locality use so you know the child key was created by the kernel and nothing else. > > I suppose that a good summary of my opinion is that there is no > point > > > to kernel support for encrypted hibernation images until lockdown > > > is upstream. > > > > I really don't think lockdown helps. If we implement locality > > isolation for the kernels use of keys, a properly functioning > > kernel isn't going to be tricked into releasing one of its keys to > > userspace. A buggy kernel might be exploited to cause it to give > > one up but that would happen irrespective of lockdown and, of > > course, all bets are off if the attacker can boot their own kernel. > > > > I'm not saying that lockdown helps. I'm saying that encrypting the > hibernation image in the kernel may be pointless until the kernel > supports lockdown. If we don't support lockdown, then user code can > encrypt the hibernation image all by itself. The code will be easier > to understand, more flexible, and won't require a kernel upgrade :) Well, protection of at-rest images is one reason, but I accept that simply writing them to dm-crypt storage is a better way of doing this and solves the key problem nicely. James > Honestly, no one should be using resume= anyway. Any distro with > hibernation support worth its salt should support having the > hibernation image on a dm-crypt volume, in which case it *must* > support userspace-driven resume. Of course, my laptop that's been > upgraded through many Fedora revisions doesn't survive a > hibernate/resume cycle, but that's not the kernel's fault. > > --Andy > > P.S. One thing I do want to try is encrypted *swap*. The keying for > that is trivial -- the kernel can just make a key, store it in > ordinary kernel memory, and use it to encrypt and decrypt swap pages > as needed. Getting replay protection or authentication may be tricky > due to the need for metadata, but plain old AES-XTS or > HPolyChaChaNotSpeckAnymore would be entirely straightforward and > would > get 90% of the benefit. Sure, swap could live on dm-crypt too, but > that's an administration mess, and offering a strong and cheap > alternative to mlock() for crypto programs to protect their secrets > would be fantastic and encrypted swap plus some API to verify that > anonymous memory is, in fact, backed by encrypted swap would do the > job. >
Hi! > > > Note if someone has your laptop and the ability to boot their own > > > kernels, they could always corrupt the kernel into decrypting the > > > image or giving you the unsealed key, but there's no real way of > > > preventing that even with PCR sealing or lockdown, so the basis for > > > the threat model is very much my laptop in my possession running my > > > kernel. > > > > I'm not entirely sure I agree. With a TPM-aware bootloader, it > > really ought to be possible to seal to PCRs such that a corrupted > > kernel can't restore the image. Obviously a *compromised* but > > otherwise valid kernel will be able to restore the image. > > It is possible to seal the key so that only the same booted kernel can > restore the image, yes. One of the measurements that goes into the > boot log is the hash of the kernel and you can seal to this value ... > obviously if you upgrade your kernel RPM (or shim or grub) this value > changes and you'd lose the ability to restore the hibernated image, but > since the image is very kernel specific, that's probably OK. Non-ancient kernels actually support hibernation by one kernel and restore by another one. But yes, normally it is same kernel binary doing hibernation and restore. Pavel
On Tue, Jan 08, 2019 at 05:43:53PM -0800, Andy Lutomirski wrote: > (Also, do we have a sensible story of how the TPM interacts with > hibernation at all? Presumably we should at least try to replay the > PCR operations that have occurred so that we can massage the PCRs into > the same state post-hibernation. Also, do we have any way for the > kernel to sign something with the TPM along with an attestation that > the signature was requested *by the kernel*? Something like a > sub-hierarchy of keys that the kernel explicitly prevents userspace > from accessing?) Kernel can keep it is own key hierarchy in memory as TPM2 chips allow to offload data in encrypted form and load it to TPM when it needs to use it. The in-kernel resource manager that I initiated couple years ago provides this type of functionality. /Jarkko
On Fri, 2019-01-11 at 16:02 +0200, Jarkko Sakkinen wrote: > On Tue, Jan 08, 2019 at 05:43:53PM -0800, Andy Lutomirski wrote: > > (Also, do we have a sensible story of how the TPM interacts with > > hibernation at all? Presumably we should at least try to replay > > the PCR operations that have occurred so that we can massage the > > PCRs into the same state post-hibernation. Also, do we have any > > way for the kernel to sign something with the TPM along with an > > attestation that the signature was requested *by the > > kernel*? Something like a sub-hierarchy of keys that the kernel > > explicitly prevents userspace from accessing?) > > Kernel can keep it is own key hierarchy in memory as TPM2 chips allow > to offload data in encrypted form and load it to TPM when it needs to > use it. > > The in-kernel resource manager that I initiated couple years ago > provides this type of functionality. Actually, the resource manager only keeps volatile objects separated when in use not when offloaded. The problem here is that the object needs to be persisted across reboots, so either it gets written to the NV area, bypassing the resource manager and making it globally visible or it has to get stored in TPM form in the hibernation image, meaning anyone with access to the TPM who can read the image can extract and load it. Further: anyone with access to the TPM can create a bogus sealed key and encrypt a malicious hibernation image with it. So there are two additional problems 1. Given that the attacker may have access to the binary form of the key, can we make sure only the kernel can get it released? 2. How do we prevent an attacker with access to the TPM from creating a bogus sealed key? This is why I was thinking localities. James
On Thu, Jan 10, 2019 at 02:11:55AM +0800, joeyli wrote: > > Well, I think here, if we were actually trying to solve the problem of > > proving the hibernated image were the same one we would need to prove > > some log of the kernel operation came to a particular value *after* the > > hibernated image were restored ... it's not really possible to > > condition key release which must occur before the restore on that > > outcome, so it strikes me we need more than a simple release bound to > > PCR values. > > > > hm... I am studying your information. But I have a question... > > If PCR is not capped and the root be compromised, is it possible that a > sealed bundle also be compromised? > > Is it possible that kernel can produce a sealed key with PCR by TPM when > booting? Then kernel caps a PCR by a constant value before the root is > available for userland. Then the sealed key can be exposed to userland > or be attached on hibernate image. Even the root be compromised, the TPM > trusted key is still secure. I think this even might be reasonable. Especially when we land James' encrypted sessions patches at some point. /Jarkko
On Wed, Jan 09, 2019 at 10:34:42AM -0800, Andy Lutomirski wrote: > I suppose I should go read the 2.0 spec. I’ve read the 1.2 spec, but I > always assumed that 2.0 was essentially a superset of 1.2 > functionality. They are essentially different protocols. No real compatibility. > Can the kernel filter TPM 2.0 operations? If so, then a signature > that the kernel would have prevented user code from generating is de > facto an attestation that the kernel generated it (or that the kernel > was compromised, which is sort of equivalent). You shoud look into TPM resource manager that I implemented with great work from James on session swapping and see how far it scales what you have in mind. It is currently exposed only to the user space but could be easily made an in-kernel API. Side-topic: right now the TPM driver can be compiled as a module when its APIs are not used by the kernel (namely IMA and trusted keys) with some Kconfig magic. Since it looks like that there will be even more customers, I think it would make sense to make the TPM driver core as part of the core kernel (device drivers for different types of chips could still be modules). I've proposed this before maybe two times, but it has always been rejected. /Jarkko
On Fri, Jan 11, 2019 at 07:28:58AM -0800, James Bottomley wrote: > On Fri, 2019-01-11 at 16:02 +0200, Jarkko Sakkinen wrote: > > On Tue, Jan 08, 2019 at 05:43:53PM -0800, Andy Lutomirski wrote: > > > (Also, do we have a sensible story of how the TPM interacts with > > > hibernation at all? Presumably we should at least try to replay > > > the PCR operations that have occurred so that we can massage the > > > PCRs into the same state post-hibernation. Also, do we have any > > > way for the kernel to sign something with the TPM along with an > > > attestation that the signature was requested *by the > > > kernel*? Something like a sub-hierarchy of keys that the kernel > > > explicitly prevents userspace from accessing?) > > > > Kernel can keep it is own key hierarchy in memory as TPM2 chips allow > > to offload data in encrypted form and load it to TPM when it needs to > > use it. > > > > The in-kernel resource manager that I initiated couple years ago > > provides this type of functionality. > > Actually, the resource manager only keeps volatile objects separated > when in use not when offloaded. The problem here is that the object > needs to be persisted across reboots, so either it gets written to the > NV area, bypassing the resource manager and making it globally visible > or it has to get stored in TPM form in the hibernation image, meaning > anyone with access to the TPM who can read the image can extract and > load it. Further: anyone with access to the TPM can create a bogus > sealed key and encrypt a malicious hibernation image with it. So there > are two additional problems > > 1. Given that the attacker may have access to the binary form of the > key, can we make sure only the kernel can get it released? > 2. How do we prevent an attacker with access to the TPM from creating a > bogus sealed key? > > This is why I was thinking localities. Why you would want to go for localities and not seal to PCRs? /Jarkko
On Fri, 2019-01-18 at 16:33 +0200, Jarkko Sakkinen wrote: > On Fri, Jan 11, 2019 at 07:28:58AM -0800, James Bottomley wrote: > > On Fri, 2019-01-11 at 16:02 +0200, Jarkko Sakkinen wrote: > > > On Tue, Jan 08, 2019 at 05:43:53PM -0800, Andy Lutomirski wrote: > > > > (Also, do we have a sensible story of how the TPM interacts > > > > with hibernation at all? Presumably we should at least try to > > > > replay the PCR operations that have occurred so that we can > > > > massage the PCRs into the same state post-hibernation. Also, > > > > do we have any way for the kernel to sign something with the > > > > TPM along with an attestation that the signature was requested > > > > *by the kernel*? Something like a sub-hierarchy of keys that > > > > the kernel explicitly prevents userspace from accessing?) > > > > > > Kernel can keep it is own key hierarchy in memory as TPM2 chips > > > allow to offload data in encrypted form and load it to TPM when > > > it needs to use it. > > > > > > The in-kernel resource manager that I initiated couple years ago > > > provides this type of functionality. > > > > Actually, the resource manager only keeps volatile objects > > separated when in use not when offloaded. The problem here is that > > the object needs to be persisted across reboots, so either it gets > > written to the NV area, bypassing the resource manager and making > > it globally visible or it has to get stored in TPM form in the > > hibernation image, meaning anyone with access to the TPM who can > > read the image can extract and load it. Further: anyone with access > > to the TPM can create a bogus sealed key and encrypt a malicious > > hibernation image with it. So there are two additional problems > > > > 1. Given that the attacker may have access to the binary form of > > the > > key, can we make sure only the kernel can get it released? > > 2. How do we prevent an attacker with access to the TPM from > > creating a > > bogus sealed key? > > > > This is why I was thinking localities. > > Why you would want to go for localities and not seal to PCRs? Because the requested functionality was a key that would be accessible to the kernel and not to user space and also guaranteed created by the kernel. The only discriminator we have to enforce that is the locality (assuming we reserve a locality as accessible to the kernel but inaccessible to userspace). PCRs alone can't restrict where the key is accessed or created from. James
On Fri, Jan 18, 2019 at 12:59:06PM -0800, James Bottomley wrote: > On Fri, 2019-01-18 at 16:33 +0200, Jarkko Sakkinen wrote: > > On Fri, Jan 11, 2019 at 07:28:58AM -0800, James Bottomley wrote: > > > On Fri, 2019-01-11 at 16:02 +0200, Jarkko Sakkinen wrote: > > > > On Tue, Jan 08, 2019 at 05:43:53PM -0800, Andy Lutomirski wrote: > > > > > (Also, do we have a sensible story of how the TPM interacts > > > > > with hibernation at all? Presumably we should at least try to > > > > > replay the PCR operations that have occurred so that we can > > > > > massage the PCRs into the same state post-hibernation. Also, > > > > > do we have any way for the kernel to sign something with the > > > > > TPM along with an attestation that the signature was requested > > > > > *by the kernel*? Something like a sub-hierarchy of keys that > > > > > the kernel explicitly prevents userspace from accessing?) > > > > > > > > Kernel can keep it is own key hierarchy in memory as TPM2 chips > > > > allow to offload data in encrypted form and load it to TPM when > > > > it needs to use it. > > > > > > > > The in-kernel resource manager that I initiated couple years ago > > > > provides this type of functionality. > > > > > > Actually, the resource manager only keeps volatile objects > > > separated when in use not when offloaded. The problem here is that > > > the object needs to be persisted across reboots, so either it gets > > > written to the NV area, bypassing the resource manager and making > > > it globally visible or it has to get stored in TPM form in the > > > hibernation image, meaning anyone with access to the TPM who can > > > read the image can extract and load it. Further: anyone with access > > > to the TPM can create a bogus sealed key and encrypt a malicious > > > hibernation image with it. So there are two additional problems > > > > > > 1. Given that the attacker may have access to the binary form of > > > the > > > key, can we make sure only the kernel can get it released? > > > 2. How do we prevent an attacker with access to the TPM from > > > creating a > > > bogus sealed key? > > > > > > This is why I was thinking localities. > > > > Why you would want to go for localities and not seal to PCRs? > > Because the requested functionality was a key that would be accessible > to the kernel and not to user space and also guaranteed created by the > kernel. The only discriminator we have to enforce that is the locality > (assuming we reserve a locality as accessible to the kernel but > inaccessible to userspace). PCRs alone can't restrict where the key is > accessed or created from. OK, locality would probably make sense, assuming that the key is stored in nvram. /Jarkko /Jarkko
diff --git a/kernel/power/Kconfig b/kernel/power/Kconfig index f8fe57d1022e..506a3c5a7a0d 100644 --- a/kernel/power/Kconfig +++ b/kernel/power/Kconfig @@ -76,6 +76,20 @@ config HIBERNATION For more information take a look at <file:Documentation/power/swsusp.txt>. +config HIBERNATION_ENC_AUTH + bool "Hibernation encryption and authentication" + depends on HIBERNATION + select TRUSTED_KEYS + select CRYPTO_AES + select CRYPTO_HMAC + select CRYPTO_SHA512 + help + This option will encrypt and authenticate the memory snapshot image + of hibernation. It prevents that the snapshot image be arbitrarily + modified. A user can use the TPM's trusted key or user defined key + as the master key of hibernation. The TPM trusted key depends on TPM. + The security of user defined key relies on user space. + config ARCH_SAVE_PAGE_KEYS bool diff --git a/kernel/power/Makefile b/kernel/power/Makefile index e7e47d9be1e5..d949adbaf580 100644 --- a/kernel/power/Makefile +++ b/kernel/power/Makefile @@ -11,6 +11,7 @@ obj-$(CONFIG_FREEZER) += process.o obj-$(CONFIG_SUSPEND) += suspend.o obj-$(CONFIG_PM_TEST_SUSPEND) += suspend_test.o obj-$(CONFIG_HIBERNATION) += hibernate.o snapshot.o swap.o user.o +obj-$(CONFIG_HIBERNATION_ENC_AUTH) += snapshot_key.o obj-$(CONFIG_PM_AUTOSLEEP) += autosleep.o obj-$(CONFIG_PM_WAKELOCKS) += wakelock.o diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index abef759de7c8..ecc31e8e40d0 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -1034,6 +1034,36 @@ static ssize_t disk_store(struct kobject *kobj, struct kobj_attribute *attr, power_attr(disk); +#ifdef CONFIG_HIBERNATION_ENC_AUTH +static ssize_t disk_kmk_show(struct kobject *kobj, struct kobj_attribute *attr, + char *buf) +{ + if (snapshot_key_initialized()) + return sprintf(buf, "initialized\n"); + else + return sprintf(buf, "uninitialized\n"); +} + +static ssize_t disk_kmk_store(struct kobject *kobj, struct kobj_attribute *attr, + const char *buf, size_t n) +{ + int error = 0; + char *p; + int len; + + p = memchr(buf, '\n', n); + len = p ? p - buf : n; + if (strncmp(buf, "1", len)) + return -EINVAL; + + error = snapshot_key_init(); + + return error ? error : n; +} + +power_attr(disk_kmk); +#endif /* !CONFIG_HIBERNATION_ENC_AUTH */ + static ssize_t resume_show(struct kobject *kobj, struct kobj_attribute *attr, char *buf) { @@ -1138,6 +1168,9 @@ power_attr(reserved_size); static struct attribute * g[] = { &disk_attr.attr, +#ifdef CONFIG_HIBERNATION_ENC_AUTH + &disk_kmk_attr.attr, +#endif &resume_offset_attr.attr, &resume_attr.attr, &image_size_attr.attr, diff --git a/kernel/power/power.h b/kernel/power/power.h index 9e58bdc8a562..fe2dfa0d4d36 100644 --- a/kernel/power/power.h +++ b/kernel/power/power.h @@ -4,6 +4,12 @@ #include <linux/utsname.h> #include <linux/freezer.h> #include <linux/compiler.h> +#include <crypto/sha.h> + +/* The max size of encrypted key blob */ +#define KEY_BLOB_BUFF_LEN 512 +#define SNAPSHOT_KEY_SIZE SHA512_DIGEST_SIZE +#define DERIVED_KEY_SIZE SHA512_DIGEST_SIZE struct swsusp_info { struct new_utsname uts; @@ -20,6 +26,16 @@ struct swsusp_info { extern void __init hibernate_reserved_size_init(void); extern void __init hibernate_image_size_init(void); +#ifdef CONFIG_HIBERNATION_ENC_AUTH +/* kernel/power/snapshot_key.c */ +extern int snapshot_key_init(void); +extern bool snapshot_key_initialized(void); +extern int snapshot_get_auth_key(u8 *auth_key, bool may_sleep); +extern int snapshot_get_enc_key(u8 *enc_key, bool may_sleep); +#else +static inline int snapshot_key_init(void) { return 0; } +#endif /* !CONFIG_HIBERNATION_ENC_AUTH */ + #ifdef CONFIG_ARCH_HIBERNATION_HEADER /* Maximum size of architecture specific data in a hibernation header */ #define MAX_ARCH_HEADER_SIZE (sizeof(struct new_utsname) + 4) diff --git a/kernel/power/snapshot_key.c b/kernel/power/snapshot_key.c new file mode 100644 index 000000000000..3a569b505d8d --- /dev/null +++ b/kernel/power/snapshot_key.c @@ -0,0 +1,245 @@ +// SPDX-License-Identifier: GPL-2.0 + +/* snapshot keys handler + * + * Copyright (C) 2018 Lee, Chun-Yi <jlee@suse.com> + * + * This program is free software; you can redistribute it and/or + * modify it under the terms of the GNU General Public Licence + * as published by the Free Software Foundation; either version + * 2 of the Licence, or (at your option) any later version. + */ +#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt + +#include <linux/cred.h> +#include <linux/key-type.h> +#include <linux/slab.h> +#include <crypto/hash.h> +#include <crypto/sha.h> +#include <keys/trusted-type.h> +#include <keys/user-type.h> + +#include "power.h" + +static const char hash_alg[] = "sha512"; +static struct crypto_shash *hash_tfm; + +/* The master key of snapshot */ +static struct snapshot_key { + const char *key_name; + bool initialized; + unsigned int key_len; + u8 key[SNAPSHOT_KEY_SIZE]; +} skey = { + .key_name = "swsusp-kmk", +}; + +static int calc_hash(u8 *digest, const u8 *buf, unsigned int buflen, + bool may_sleep) +{ + struct shash_desc *desc; + int err; + + desc = kzalloc(sizeof(struct shash_desc) + + crypto_shash_descsize(hash_tfm), + may_sleep ? GFP_KERNEL : GFP_ATOMIC); + if (!desc) + return -ENOMEM; + + desc->tfm = hash_tfm; + desc->flags = may_sleep ? CRYPTO_TFM_REQ_MAY_SLEEP : 0; + err = crypto_shash_digest(desc, buf, buflen, digest); + shash_desc_zero(desc); + kzfree(desc); + + return err; +} + +static int calc_key_hash(u8 *key, unsigned int key_len, const char *salt, + u8 *hash, bool may_sleep) +{ + unsigned int salted_buf_len; + u8 *salted_buf; + int ret; + + if (!key || !hash_tfm || !hash) + return -EINVAL; + + salted_buf_len = strlen(salt) + 1 + SNAPSHOT_KEY_SIZE; + salted_buf = kzalloc(salted_buf_len, + may_sleep ? GFP_KERNEL : GFP_ATOMIC); + if (!salted_buf) + return -ENOMEM; + + strcpy(salted_buf, salt); + memcpy(salted_buf + strlen(salted_buf) + 1, key, key_len); + + ret = calc_hash(hash, salted_buf, salted_buf_len, may_sleep); + memzero_explicit(salted_buf, salted_buf_len); + kzfree(salted_buf); + + return ret; +} + +/* Derive authentication/encryption key */ +static int get_derived_key(u8 *derived_key, const char *derived_type_str, + bool may_sleep) +{ + int ret; + + if (!skey.initialized || !hash_tfm) + return -EINVAL; + + ret = calc_key_hash(skey.key, skey.key_len, derived_type_str, + derived_key, may_sleep); + + return ret; +} + +int snapshot_get_auth_key(u8 *auth_key, bool may_sleep) +{ + return get_derived_key(auth_key, "AUTH_KEY", may_sleep); +} + +int snapshot_get_enc_key(u8 *enc_key, bool may_sleep) +{ + return get_derived_key(enc_key, "ENC_KEY", may_sleep); +} + +bool snapshot_key_initialized(void) +{ + return skey.initialized; +} + +static bool invalid_key(u8 *key, unsigned int key_len) +{ + int i; + + if (!key || !key_len) + return true; + + if (key_len > SNAPSHOT_KEY_SIZE) { + pr_warn("Size of swsusp key more than: %d.\n", + SNAPSHOT_KEY_SIZE); + return true; + } + + /* zero keyblob is invalid key */ + for (i = 0; i < key_len; i++) { + if (key[i] != 0) + return false; + } + pr_warn("The swsusp key should not be zero.\n"); + + return true; +} + +static int trusted_key_init(void) +{ + struct trusted_key_payload *tkp; + struct key *key; + int err = 0; + + pr_debug("%s\n", __func__); + + /* find out swsusp-key */ + key = request_key(&key_type_trusted, skey.key_name, NULL); + if (IS_ERR(key)) { + pr_err("Request key error: %ld\n", PTR_ERR(key)); + err = PTR_ERR(key); + return err; + } + + down_write(&key->sem); + tkp = key->payload.data[0]; + if (invalid_key(tkp->key, tkp->key_len)) { + err = -EINVAL; + goto key_invalid; + } + skey.key_len = tkp->key_len; + memcpy(skey.key, tkp->key, tkp->key_len); + /* burn the original key contents */ + memzero_explicit(tkp->key, tkp->key_len); + +key_invalid: + up_write(&key->sem); + key_put(key); + + return err; +} + +static int user_key_init(void) +{ + struct user_key_payload *ukp; + struct key *key; + int err = 0; + + pr_debug("%s\n", __func__); + + /* find out swsusp-key */ + key = request_key(&key_type_user, skey.key_name, NULL); + if (IS_ERR(key)) { + pr_err("Request key error: %ld\n", PTR_ERR(key)); + err = PTR_ERR(key); + return err; + } + + down_write(&key->sem); + ukp = user_key_payload_locked(key); + if (!ukp) { + /* key was revoked before we acquired its semaphore */ + err = -EKEYREVOKED; + goto key_invalid; + } + if (invalid_key(ukp->data, ukp->datalen)) { + err = -EINVAL; + goto key_invalid; + } + skey.key_len = ukp->datalen; + memcpy(skey.key, ukp->data, ukp->datalen); + /* burn the original key contents */ + memzero_explicit(ukp->data, ukp->datalen); + +key_invalid: + up_write(&key->sem); + key_put(key); + + return err; +} + +/* this function may sleeps */ +int snapshot_key_init(void) +{ + int err; + + pr_debug("%s\n", __func__); + + if (skey.initialized) + return 0; + + hash_tfm = crypto_alloc_shash(hash_alg, 0, CRYPTO_ALG_ASYNC); + if (IS_ERR(hash_tfm)) { + pr_err("Can't allocate %s transform: %ld\n", + hash_alg, PTR_ERR(hash_tfm)); + return PTR_ERR(hash_tfm); + } + + err = trusted_key_init(); + if (err) + err = user_key_init(); + if (err) + goto key_fail; + + barrier(); + skey.initialized = true; + + pr_info("Snapshot key is initialled.\n"); + + return 0; + +key_fail: + crypto_free_shash(hash_tfm); + hash_tfm = NULL; + + return err; +}
This patch adds a snapshot keys handler for using the key retention service api to create keys for snapshot image encryption and authentication. This handler uses TPM trusted key as the snapshot master key, and the encryption key and authentication key are derived from the snapshot key. The user defined key can also be used as the snapshot master key , but user must be aware that the security of user key relies on user space. The name of snapshot key is fixed to "swsusp-kmk". User should use the keyctl tool to load the key blob to root's user keyring. e.g. # /bin/keyctl add trusted swsusp-kmk "load `cat swsusp-kmk.blob`" @u or create a new user key. e.g. # /bin/keyctl add user swsusp-kmk password @u Then the disk_kmk sysfs file can be used to trigger the initialization of snapshot key: # echo 1 > /sys/power/disk_kmk After the initialization be triggered, the secret in the payload of swsusp-key will be copied by hibernation and be erased. Then user can use keyctl to remove swsusp-kmk key from root's keyring. If user does not trigger the initialization by disk_kmk file after swsusp-kmk be loaded to kernel. Then the snapshot key will be initialled when hibernation be triggered. v2: - Fixed bug of trusted_key_init's return value. - Fixed wording in Kconfig - Removed VLA usage - Removed the checking of capability for writing disk_kmk. - Fixed Kconfig, select trusted key. - Add memory barrier before setting key initialized flag. Cc: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com> Cc: Pavel Machek <pavel@ucw.cz> Cc: Chen Yu <yu.c.chen@intel.com> Cc: Oliver Neukum <oneukum@suse.com> Cc: Ryan Chen <yu.chen.surf@gmail.com> Cc: David Howells <dhowells@redhat.com> Cc: Giovanni Gherdovich <ggherdovich@suse.cz> Cc: Randy Dunlap <rdunlap@infradead.org> Cc: Jann Horn <jannh@google.com> Cc: Andy Lutomirski <luto@kernel.org> Signed-off-by: "Lee, Chun-Yi" <jlee@suse.com> --- kernel/power/Kconfig | 14 +++ kernel/power/Makefile | 1 + kernel/power/hibernate.c | 33 ++++++ kernel/power/power.h | 16 +++ kernel/power/snapshot_key.c | 245 ++++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 309 insertions(+) create mode 100644 kernel/power/snapshot_key.c