From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C9EAE17BA6; Tue, 5 Aug 2025 13:50:21 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754401821; cv=none; b=Gxe6erbncdhk+TvfZNGtCkEjNdeWkXKesQL4IYKnetoAaalk299jEtf3FEsNLZb0fvPEzh5M3J+PhUXbnkO4j00fQTWx4QYqXHaVIPCS/wk2AuVrIE7sBqs5hxFuKL2YmNHxTA0qLoH5Wrr1swksWDeIcz0Mlh1G/mtMy4QAosw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1754401821; c=relaxed/simple; bh=F5t8xJ5CFrnY9G4jDrUDgOwh23swmqjxFNWWlPTOCyo=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=U15dRVfJVUALEmbsLjpy1G9jBMqO0x0PtXepa0rHpR93hXGGhu8l7kny6GMRCtl80UnBzNjlTHSIMOCurLpI44qroK7N/O30CJ/RasgOTAgI4+pSAFzF+3Ypp8jbMWYaTtebEuqmcO4o9uHb0Y/MVJoX8afJFxXNRJ+pmO48D1A= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=B9xslZTb; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="B9xslZTb" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 136EFC4CEF0; Tue, 5 Aug 2025 13:50:20 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1754401821; bh=F5t8xJ5CFrnY9G4jDrUDgOwh23swmqjxFNWWlPTOCyo=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=B9xslZTbD8HbLbiUcA1KLZXP36BTybp83TSe8qctTAADyJHn2D7fwl+rYgN3g6p+x xmIb1kj3FZPRyUNNggNnv0UxWXrnFyevx9tMEaQeD/ZwJ77AjCu0kKfIckWo3C7wYx o7drok2+o9RpkiuDLivdGnI4KnahdQijCFCf7FHkqNZZ2YPopEuQsEnuBHPYNVYEn3 hS9/p2i7zKzHWVIOpRmhe84v6EwdCQHFqnM0etycS9xriY3x5wHs/xHK1V1SzirdGP mxcCNG0Uf/8DVrgsZvaGdV4D3yHJcSUWS/4ieYSwTj2zG/K6/3POSuFZ/a72kfCa8d zdW1BLfpX4GXA== Date: Tue, 5 Aug 2025 16:50:17 +0300 From: Jarkko Sakkinen To: Eric Biggers Cc: Peter Huewe , linux-integrity@vger.kernel.org, Jason Gunthorpe , James Bottomley , linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v2 1/2] tpm: Compare HMAC values in constant time Message-ID: References: <20250801212422.9590-1-ebiggers@kernel.org> <20250801212422.9590-2-ebiggers@kernel.org> Precedence: bulk X-Mailing-List: linux-integrity@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20250801212422.9590-2-ebiggers@kernel.org> On Fri, Aug 01, 2025 at 02:24:21PM -0700, Eric Biggers wrote: > In tpm_buf_check_hmac_response(), compare the HMAC values in constant > time using crypto_memneq() instead of in variable time using memcmp(). > > This is worthwhile to follow best practices and to be consistent with > MAC comparisons elsewhere in the kernel. However, in this driver the > side channel seems to have been benign: the HMAC input data is > guaranteed to always be unique, which makes the usual MAC forgery via > timing side channel not possible. Specifically, the HMAC input data in > tpm_buf_check_hmac_response() includes the "our_nonce" field, which was > generated by the kernel earlier, remains under the control of the > kernel, and is unique for each call to tpm_buf_check_hmac_response(). > > Signed-off-by: Eric Biggers > --- > drivers/char/tpm/Kconfig | 1 + > drivers/char/tpm/tpm2-sessions.c | 6 +++--- > 2 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/drivers/char/tpm/Kconfig b/drivers/char/tpm/Kconfig > index dddd702b2454a..f9d8a4e966867 100644 > --- a/drivers/char/tpm/Kconfig > +++ b/drivers/char/tpm/Kconfig > @@ -31,10 +31,11 @@ config TCG_TPM2_HMAC > bool "Use HMAC and encrypted transactions on the TPM bus" > default X86_64 > select CRYPTO_ECDH > select CRYPTO_LIB_AESCFB > select CRYPTO_LIB_SHA256 > + select CRYPTO_LIB_UTILS > help > Setting this causes us to deploy a scheme which uses request > and response HMACs in addition to encryption for > communicating with the TPM to prevent or detect bus snooping > and interposer attacks (see tpm-security.rst). Saying Y > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c > index bdb119453dfbe..5fbd62ee50903 100644 > --- a/drivers/char/tpm/tpm2-sessions.c > +++ b/drivers/char/tpm/tpm2-sessions.c > @@ -69,10 +69,11 @@ > #include > #include > #include > #include > #include > +#include > > /* maximum number of names the TPM must remember for authorization */ > #define AUTH_MAX_NAMES 3 > > #define AES_KEY_BYTES AES_KEYSIZE_128 > @@ -827,16 +828,15 @@ int tpm_buf_check_hmac_response(struct tpm_chip *chip, struct tpm_buf *buf, > sha256_update(&sctx, auth->our_nonce, sizeof(auth->our_nonce)); > sha256_update(&sctx, &auth->attrs, 1); > /* we're done with the rphash, so put our idea of the hmac there */ > tpm2_hmac_final(&sctx, auth->session_key, sizeof(auth->session_key) > + auth->passphrase_len, rphash); > - if (memcmp(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE) == 0) { > - rc = 0; > - } else { > + if (crypto_memneq(rphash, &buf->data[offset_s], SHA256_DIGEST_SIZE)) { > dev_err(&chip->dev, "TPM: HMAC check failed\n"); > goto out; > } > + rc = 0; > > /* now do response decryption */ > if (auth->attrs & TPM2_SA_ENCRYPT) { > /* need key and IV */ > tpm2_KDFa(auth->session_key, SHA256_DIGEST_SIZE > -- > 2.50.1 > I think we might want to also backport this to stables. BR, Jarkko