Message-Id: <D5FZTYOGY3IO.4SKIAS11102D@kernel.org>
Date: Thu, 07 Nov 2024 15:50:28 +0200
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Jarkko Sakkinen" <jarkko@...nel.org>,
 <linux-integrity@...r.kernel.org>, "Peter Huewe" <peterhuewe@....de>,
 "Jason Gunthorpe" <jgg@...pe.ca>
Cc: "James Bottomley" <James.Bottomley@...senPartnership.com>, "Mimi Zohar"
 <zohar@...ux.ibm.com>, "David Howells" <dhowells@...hat.com>, "Paul Moore"
 <paul@...l-moore.com>, "James Morris" <jmorris@...ei.org>, "Serge E.
 Hallyn" <serge@...lyn.com>, "open list" <linux-kernel@...r.kernel.org>,
 "open list:KEYS-TRUSTED" <keyrings@...r.kernel.org>, "open list:SECURITY
 SUBSYSTEM" <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH] tpm: Remove the documentation from tpm2-sessions.c

On Thu Nov 7, 2024 at 1:20 PM EET, Jarkko Sakkinen wrote:
> Nobody will maintain this, i.e. it is destined to rotten. It is better to
> just rip it off, and not have duplicate stuff that is already in the kernel
> documentation and function headers.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
>  drivers/char/tpm/tpm2-sessions.c | 68 ++------------------------------
>  1 file changed, 3 insertions(+), 65 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index a7c1b162251b..ff00e9483564 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -1,71 +1,9 @@
>  // SPDX-License-Identifier: GPL-2.0
> -
>  /*
> - * Copyright (C) 2018 James.Bottomley@...senPartnership.com
> - *
> - * 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:
> + * Copyright (c) 2018 James Bottomley <James.Bottomley@...senPartnership.com>
>   *
> - * 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.
> + * Cryptographic helper routines for handling TPM2 sessions for authorization
> + * HMAC and request response encryption.
>   */
>  
>  #include "tpm.h"

So no means to slander this. I just checked the kdoc's and also
documentation and they have all the content needed. So it is better
to focus to maintaining those and not have duplicate copies.

If there is a detail here missing from those I'd advice to contribute
that but I could not spot anything...

BR, Jarkko

Powered by blists - more mailing lists