lazy flush for the auth session
Subsystems (5)
Jarkko Sakkinen
maintainerv2 cover lazy flush for the auth session
This patch set aims to address https://bugzilla.kernel.org/show_bug.cgi?id=219229
v1: https://lore.kernel.org/linux-integrity/[email protected]/
Jarkko Sakkinen (6):
tpm: Remove documentation from the header of tpm2-sessions.c
tpm: Return on tpm2_create_null_primary() failure
tpm: Return on tpm2_create_primary() failure in tpm2_load_null()
tpm: flush the null key only when /dev/tpm0 is accessed
tpm: Allocate chip->auth in tpm2_start_auth_session()
tpm: flush the auth session only when /dev/tpm0 is open
drivers/char/tpm/tpm-chip.c | 14 +++ drivers/char/tpm/tpm-dev-common.c | 8 ++ drivers/char/tpm/tpm-interface.c | 10 +- drivers/char/tpm/tpm2-cmd.c | 3 + drivers/char/tpm/tpm2-sessions.c | 156 ++++++++++++------------------ include/linux/tpm.h | 2 + 6 files changed, 98 insertions(+), 95 deletions(-)
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 1/6 tpm: Remove documentation from the header of tpm2-sessions.c
The documentation in the file header is duplicate documentation to kernel doc comments in the function declarations and tpm-security.rst. Wipe it off as nobody will ever take care of keeping it up to date.
---
v2:
- Refine the commit message.
--- drivers/char/tpm/tpm2-sessions.c | 65 -------------------------------- 1 file changed, 65 deletions(-)
drivers/char/tpm/tpm2-sessions.c +0 -65
@@ -1,71 +1,6 @@// SPDX-License-Identifier: GPL-2.0 -/* * Copyright (C) 2018 [email protected] -* -* Cryptographic helper routines for handling TPM2 sessions for -* authorization HMAC and request response encryption. -* -* The idea is to ensure that every TPM command is HMAC protected by a -* session, meaning in-flight tampering would be detected and in -* addition all sensitive inputs and responses should be encrypted. -* -* The basic way this works is to use a TPM feature called salted -* sessions where a random secret used in session construction is -* encrypted to the public part of a known TPM key. The problem is we -* have no known keys, so initially a primary Elliptic Curve key is -* derived from the NULL seed (we use EC because most TPMs generate -* these keys much faster than RSA ones). The curve used is NIST_P256 -* because that's now mandated to be present in 'TCG TPM v2.0 -* Provisioning Guidance' -* -* Threat problems: the initial TPM2_CreatePrimary is not (and cannot -* be) session protected, so a clever Man in the Middle could return a -* public key they control to this command and from there intercept -* and decode all subsequent session based transactions. The kernel -* cannot mitigate this threat but, after boot, userspace can get -* proof this has not happened by asking the TPM to certify the NULL -* key. This certification would chain back to the TPM Endorsement -* Certificate and prove the NULL seed primary had not been tampered -* with and thus all sessions must have been cryptographically secure. -* To assist with this, the initial NULL seed public key name is made -* available in a sysfs file. -* -* Use of these functions: -* -* The design is all the crypto, hash and hmac gunk is confined in this -* file and never needs to be seen even by the kernel internal user. To -* the user there's an init function tpm2_sessions_init() that needs to -* be called once per TPM which generates the NULL seed primary key. -* -* These are the usage functions: -* -* tpm2_start_auth_session() which allocates the opaque auth structure -* and gets a session from the TPM. This must be called before -* any of the following functions. The session is protected by a -* session_key which is derived from a random salt value -* encrypted to the NULL seed. -* tpm2_end_auth_session() kills the session and frees the resources. -* Under normal operation this function is done by -* tpm_buf_check_hmac_response(), so this is only to be used on -* error legs where the latter is not executed. -* tpm_buf_append_name() to add a handle to the buffer. This must be -* used in place of the usual tpm_buf_append_u32() for adding -* handles because handles have to be processed specially when -* calculating the HMAC. In particular, for NV, volatile and -* permanent objects you now need to provide the name. -* tpm_buf_append_hmac_session() which appends the hmac session to the -* buf in the same way tpm_buf_append_auth does(). -* tpm_buf_fill_hmac_session() This calculates the correct hash and -* places it in the buffer. It must be called after the complete -* command buffer is finalized so it can fill in the correct HMAC -* based on the parameters. -* tpm_buf_check_hmac_response() which checks the session response in -* the buffer and calculates what it should be. If there's a -* mismatch it will log a warning and return an error. If -* tpm_buf_append_hmac_session() did not specify -* TPM_SA_CONTINUE_SESSION then the session will be closed (if it -* hasn't been consumed) and the auth structure freed. */ #include "tpm.h"
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 2/6 tpm: Return on tpm2_create_null_primary() failure
In reply to Jarkko Sakkinentpm2_sessions_init() ignores the return value oftpm2_create_null_primary().
Address this by returning on failure.
---
v2:
- Refined the commit message.
--- drivers/char/tpm/tpm2-sessions.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/char/tpm/tpm2-sessions.c +3 -1
@@ -1288,8 +1288,10 @@ int tpm2_sessions_init(struct tpm_chip *chip)int rc; rc =tpm2_create_null_primary (chip); -if (rc) +if (rc) {dev_err (&chip->dev,"TPM: security failed (NULL seed derivation): %d\n" , rc); +return rc; + } chip->auth =kmalloc (sizeof (*chip->auth),GFP_KERNEL );if (!chip->auth)
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 3/6 tpm: Return on tpm2_create_primary() failure in tpm2_load_null()
In reply to Jarkko Sakkinentpm2_load_null() ignores the return value of tpm2_create_primary(). Further, it does not heal from the situation when memcmp() returns zero.
Address this by returning on failure and saving the null key if there was no detected interference in the bus.
---
v2:
- Refined the commit message.
- Reverted tpm2_create_primary() changes. They are not required if tmp_null_key is used as the parameter.
--- drivers/char/tpm/tpm2-sessions.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)
drivers/char/tpm/tpm2-sessions.c +18 -7
@@ -850,22 +850,32 @@ static int tpm2_parse_start_auth_session(struct tpm2_auth *auth,static int tpm2_load_null (struct tpm_chip *chip,u32 *null_key) { -int rc;unsigned int offset =0 ;/* dummy offset for null seed context */ u8 name[SHA256_DIGEST_SIZE +2 ]; +u32 tmp_null_key; +int rc; rc =tpm2_load_context (chip, chip->null_key_context, &offset, - null_key); -if (rc != -EINVAL ) + &tmp_null_key); +if (rc != -EINVAL ) { +if (!rc) + *null_key = tmp_null_key;return rc; + }/* an integrity failure may mean the TPM has been reset */ dev_err (&chip->dev,"NULL key integrity failure!\n" ); -/* check the null name against what we know */ -tpm2_create_primary (chip,TPM2_RH_NULL ,NULL , name); -if (memcmp (name, chip->null_key_name,sizeof (name)) ==0 ) -/* name unchanged, assume transient integrity failure */ + + rc =tpm2_create_primary (chip,TPM2_RH_NULL , &tmp_null_key, name); +if (rc)return rc; + +/* Return the null key if the name has not been changed: */ +if (memcmp (name, chip->null_key_name,sizeof (name)) ==0 ) { + *null_key = tmp_null_key; +return 0 ; + } +/* * Fatal TPM failure: the NULL seed has actually changed, so * the TPM must have been illegally reset. All in-kernel TPM
@@ -874,6 +884,7 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)* userspace programmes can't be compromised by it. */ dev_err (&chip->dev,"NULL name has changed, disabling TPM due to interference\n" ); +tpm2_flush_context (chip, tmp_null_key); chip->flags |=TPM_CHIP_FLAG_DISABLE ;return rc;
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 4/6 tpm: flush the null key only when /dev/tpm0 is accessed
In reply to Jarkko SakkinenInstead of flushing and reloading the null key for every single auth session, flush it only when:
1. User space needs to access /dev/tpm{rm}0.
2. When going to sleep.
3. When unregistering the chip.
This removes the need to load and swap the null key between TPM and regular memory per transaction, when the user space is not using the chip.
---
v2:
- Refined the commit message.
- Added tested-by from Pengyu Ma <[email protected]>.
- Removed spurious pr_info() statement.
--- drivers/char/tpm/tpm-chip.c | 13 +++++++++++++ drivers/char/tpm/tpm-dev-common.c | 7 +++++++ drivers/char/tpm/tpm-interface.c | 9 +++++++-- drivers/char/tpm/tpm2-cmd.c | 3 +++ drivers/char/tpm/tpm2-sessions.c | 17 ++++++++++++++--- include/linux/tpm.h | 2 ++ 6 files changed, 46 insertions(+), 5 deletions(-)
drivers/char/tpm/tpm-chip.c +13 -0
@@ -674,6 +674,19 @@ EXPORT_SYMBOL_GPL(tpm_chip_register);*/ void tpm_chip_unregister (struct tpm_chip *chip) { +#ifdef CONFIG_TCG_TPM2_HMAC +int rc; + + rc =tpm_try_get_ops (chip); +if (!rc) { +if (chip->flags &TPM_CHIP_FLAG_TPM2 ) { +tpm2_flush_context (chip, chip->null_key); + chip->null_key =0 ; + } +tpm_put_ops (chip); + } +#endif +tpm_del_legacy_sysfs (chip);if (tpm_is_hwrng_enabled (chip))hwrng_unregister (&chip->hwrng);
drivers/char/tpm/tpm-dev-common.c +7 -0
@@ -27,6 +27,13 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,struct tpm_header *header = (void *)buf;ssize_t ret, len; +#ifdef CONFIG_TCG_TPM2_HMAC +if (chip->flags &TPM_CHIP_FLAG_TPM2 ) { +tpm2_flush_context (chip, chip->null_key); + chip->null_key =0 ; + } +#endif + ret =tpm2_prepare_space (chip, space, buf, bufsiz);/* If the command is not implemented by the TPM, synthesize a * response with a TPM2_RC_COMMAND_CODE return for user-space.
drivers/char/tpm/tpm-interface.c +7 -2
@@ -379,10 +379,15 @@ int tpm_pm_suspend(struct device *dev) rc =tpm_try_get_ops (chip);if (!rc) { -if (chip->flags &TPM_CHIP_FLAG_TPM2 ) +if (chip->flags &TPM_CHIP_FLAG_TPM2 ) { +#ifdef CONFIG_TCG_TPM2_HMAC +tpm2_flush_context (chip, chip->null_key); + chip->null_key =0 ; +#endif tpm2_shutdown (chip,TPM2_SU_STATE ); -else + }else { rc =tpm1_pm_suspend (chip, tpm_suspend_pcr); + }tpm_put_ops (chip); }
drivers/char/tpm/tpm2-cmd.c +3 -0
@@ -364,6 +364,9 @@ void tpm2_flush_context(struct tpm_chip *chip, u32 handle)struct tpm_buf buf;int rc; +if (!handle) +return ; + rc =tpm_buf_init (&buf,TPM2_ST_NO_SESSIONS ,TPM2_CC_FLUSH_CONTEXT );if (rc) { dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n",
drivers/char/tpm/tpm2-sessions.c +14 -3
@@ -855,11 +855,19 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)u32 tmp_null_key;int rc; +/* fast path */ +if (chip->null_key) { + *null_key = chip->null_key; +return 0 ; + } + rc =tpm2_load_context (chip, chip->null_key_context, &offset, &tmp_null_key);if (rc != -EINVAL ) { -if (!rc) +if (!rc) { + chip->null_key = tmp_null_key; *null_key = tmp_null_key; + }return rc; }
@@ -872,6 +880,7 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)/* Return the null key if the name has not been changed: */ if (memcmp (name, chip->null_key_name,sizeof (name)) ==0 ) { + chip->null_key = tmp_null_key; *null_key = tmp_null_key;return 0 ; }
@@ -949,7 +958,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)tpm_buf_append_u16 (&buf,TPM_ALG_SHA256 ); rc =tpm_transmit_cmd (chip, &buf,0 ,"start auth session" ); -tpm2_flush_context (chip, null_key);if (rc ==TPM2_RC_SUCCESS ) rc =tpm2_parse_start_auth_session (auth, &buf);
@@ -1281,7 +1289,10 @@ static int tpm2_create_null_primary(struct tpm_chip *chip) rc =tpm2_save_context (chip, null_key, chip->null_key_context,sizeof (chip->null_key_context), &offset); -tpm2_flush_context (chip, null_key); +if (rc) +tpm2_flush_context (chip, null_key); +else + chip->null_key = null_key; }return rc;
include/linux/tpm.h +2 -0
@@ -205,6 +205,8 @@ struct tpm_chip {#ifdef CONFIG_TCG_TPM2_HMAC /* details for communication security via sessions */ +/* loaded null key */ +u32 null_key;/* saved context for NULL seed */ u8 null_key_context[TPM2_MAX_CONTEXT_SIZE ];/* name of NULL seed */
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 5/6 tpm: Allocate chip->auth in tpm2_start_auth_session()
In reply to Jarkko SakkinenMove allocation of chip->auth to tpm2_start_auth_session() so that the field can be used as flag to tell whether auth session is active or not.
--- drivers/char/tpm/tpm2-sessions.c | 43 +++++++++++++++++++------------- 1 file changed, 25 insertions(+), 18 deletions(-)
drivers/char/tpm/tpm2-sessions.c +25 -18
@@ -419,7 +419,8 @@ static void tpm2_KDFe(u8 z[EC_PT_SZ], const char *str, u8 *pt_u, u8 *pt_v,sha256_final (&sctx, out); } -static void tpm_buf_append_salt (struct tpm_buf *buf,struct tpm_chip *chip) +static void tpm_buf_append_salt (struct tpm_buf *buf,struct tpm_chip *chip, +struct tpm2_auth *auth) {struct crypto_kpp *kpp;struct kpp_request *req;
@@ -478,7 +479,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)sg_set_buf (&s[0 ], chip->null_ec_key_x,EC_PT_SZ );sg_set_buf (&s[1 ], chip->null_ec_key_y,EC_PT_SZ );kpp_request_set_input (req, s,EC_PT_SZ *2 ); -sg_init_one (d, chip->auth->salt,EC_PT_SZ ); +sg_init_one (d, auth->salt,EC_PT_SZ );kpp_request_set_output (req, d,EC_PT_SZ );crypto_kpp_compute_shared_secret (req);kpp_request_free (req);
@@ -489,8 +490,7 @@ static void tpm_buf_append_salt(struct tpm_buf *buf, struct tpm_chip *chip)* This works because KDFe fully consumes the secret before it * writes the salt */ -tpm2_KDFe (chip->auth->salt,"SECRET" , x, chip->null_ec_key_x, - chip->auth->salt); +tpm2_KDFe (auth->salt,"SECRET" , x, chip->null_ec_key_x, auth->salt); out:crypto_free_kpp (kpp);
@@ -789,6 +789,8 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf,/* manually close the session if it wasn't consumed */ tpm2_flush_context (chip, auth->handle);memzero_explicit (auth,sizeof (*auth)); +kfree (auth); + chip->auth =NULL ; }else {/* reset for next use */ auth->session =TPM_HEADER_SIZE ;
@@ -817,6 +819,8 @@ void tpm2_end_auth_session(struct tpm_chip *chip)tpm2_flush_context (chip, auth->handle);memzero_explicit (auth,sizeof (*auth)); +kfree (auth); + chip->auth =NULL ; }EXPORT_SYMBOL (tpm2_end_auth_session);
@@ -913,25 +917,29 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key)*/ int tpm2_start_auth_session (struct tpm_chip *chip) { +struct tpm2_auth *auth;struct tpm_buf buf; -struct tpm2_auth *auth = chip->auth; -int rc;u32 null_key; +int rc; -if (!auth) { -dev_warn_once (&chip->dev,"auth session is not active\n" ); +if (chip->auth) { +dev_warn_once (&chip->dev,"auth session is active\n" );return 0 ; } + auth =kzalloc (sizeof (*auth),GFP_KERNEL ); +if (!auth) +return -ENOMEM ; + rc =tpm2_load_null (chip, &null_key);if (rc) - goto out; + goto err; auth->session =TPM_HEADER_SIZE ; rc =tpm_buf_init (&buf,TPM2_ST_NO_SESSIONS ,TPM2_CC_START_AUTH_SESS );if (rc) - goto out; + goto err;/* salt key handle */ tpm_buf_append_u32 (&buf, null_key);
@@ -943,7 +951,7 @@ int tpm2_start_auth_session(struct tpm_chip *chip)tpm_buf_append (&buf, auth->our_nonce,sizeof (auth->our_nonce));/* append encrypted salt and squirrel away unencrypted in auth */ -tpm_buf_append_salt (&buf, chip); +tpm_buf_append_salt (&buf, chip, auth);/* session type (HMAC, audit or policy) */ tpm_buf_append_u8 (&buf,TPM2_SE_HMAC );
@@ -964,10 +972,13 @@ int tpm2_start_auth_session(struct tpm_chip *chip)tpm_buf_destroy (&buf); -if (rc) - goto out; +if (rc ==TPM2_RC_SUCCESS ) { + chip->auth = auth; +return 0 ; + } - out: +err: +kfree (auth);return rc; }EXPORT_SYMBOL (tpm2_start_auth_session);
@@ -1315,10 +1326,6 @@ int tpm2_sessions_init(struct tpm_chip *chip)return rc; } - chip->auth =kmalloc (sizeof (*chip->auth),GFP_KERNEL ); -if (!chip->auth) -return -ENOMEM ; -return rc; }EXPORT_SYMBOL (tpm2_sessions_init);
Signature · 1 line
2.46.0
Jarkko Sakkinen
maintainerPATCH v2 6/6 tpm: flush the auth session only when /dev/tpm0 is open
In reply to Jarkko SakkinenInstead of flushing and reloading the auth session for every single transaction, keep the session open unless /dev/tpm0 is open. In practice this means applying TPM2_SA_CONTINUE_SESSION to the session attributes. Flush the session always when /dev/tpm0 is written.
--- drivers/char/tpm/tpm-chip.c | 1 + drivers/char/tpm/tpm-dev-common.c | 1 + drivers/char/tpm/tpm-interface.c | 1 + drivers/char/tpm/tpm2-sessions.c | 4 ++++ 4 files changed, 7 insertions(+)
drivers/char/tpm/tpm-chip.c +1 -0
@@ -680,6 +680,7 @@ void tpm_chip_unregister(struct tpm_chip *chip) rc =tpm_try_get_ops (chip);if (!rc) {if (chip->flags &TPM_CHIP_FLAG_TPM2 ) { +tpm2_end_auth_session (chip);tpm2_flush_context (chip, chip->null_key); chip->null_key =0 ; }
drivers/char/tpm/tpm-dev-common.c +1 -0
@@ -29,6 +29,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,#ifdef CONFIG_TCG_TPM2_HMAC if (chip->flags &TPM_CHIP_FLAG_TPM2 ) { +tpm2_end_auth_session (chip);tpm2_flush_context (chip, chip->null_key); chip->null_key =0 ; }
drivers/char/tpm/tpm-interface.c +1 -0
@@ -381,6 +381,7 @@ int tpm_pm_suspend(struct device *dev)if (!rc) {if (chip->flags &TPM_CHIP_FLAG_TPM2 ) {#ifdef CONFIG_TCG_TPM2_HMAC +tpm2_end_auth_session (chip);tpm2_flush_context (chip, chip->null_key); chip->null_key =0 ;#endif
drivers/char/tpm/tpm2-sessions.c +4 -0
@@ -268,6 +268,10 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, }#ifdef CONFIG_TCG_TPM2_HMAC +/* The first write to /dev/tpm{rm0} will flush the session. */ +if (!chip->is_open) + attributes |=TPM2_SA_CONTINUE_SESSION ; +/* * The Architecture Guide requires us to strip trailing zeros * before computing the HMAC
Signature · 1 line
2.46.0