Received-SPF: pass (google.com: domain of linux-kernel+bounces-179242-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Wed, 15 May 2024 01:51:50 +0300
Message-Id: <D19QI1MDD734.3FA17QYMV3TD3@kernel.org>
Cc: <kernel-team@cloudflare.com>
Subject: Re: [RFC PATCH 1/2] tpm: add some algorithm and constant
 definitions from the TPM spec
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Ignat Korchagin" <ignat@cloudflare.com>, "James Bottomley"
 <James.Bottomley@HansenPartnership.com>, "Mimi Zohar"
 <zohar@linux.ibm.com>, "David Howells" <dhowells@redhat.com>, "Paul Moore"
 <paul@paul-moore.com>, "James Morris" <jmorris@namei.org>,
 <serge@hallyn.com>, <linux-integrity@vger.kernel.org>,
 <keyrings@vger.kernel.org>, <linux-kernel@vger.kernel.org>
References: <20240503221634.44274-1-ignat@cloudflare.com>
 <20240503221634.44274-2-ignat@cloudflare.com>
In-Reply-To: <20240503221634.44274-2-ignat@cloudflare.com>

I'll go through the code changes. I've barely skimmed them because I
needed to understand the gist of the cover letter first and was busy
with 6.10 release and asymmetric keys.

On Sat May 4, 2024 at 1:16 AM EEST, Ignat Korchagin wrote:
> In preparation for implementing TPM derived keys we need to use some new =
TPM
> functionality in the kernel, so add relevant constant definitions.

Define "TPM derived key" what is. It is *undefined* at this point of the
Git history.

>
> Signed-off-by: Ignat Korchagin <ignat@cloudflare.com>
> ---
>  include/linux/tpm.h | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/tpm.h b/include/linux/tpm.h
> index 4ee9d13749ad..5be0808b1b91 100644
> --- a/include/linux/tpm.h
> +++ b/include/linux/tpm.h
> @@ -35,6 +35,7 @@ struct trusted_key_options;
>  enum tpm_algorithms {
>  	TPM_ALG_ERROR		=3D 0x0000,
>  	TPM_ALG_SHA1		=3D 0x0004,
> +	TPM_ALG_HMAC		=3D 0x0005,
>  	TPM_ALG_KEYEDHASH	=3D 0x0008,
>  	TPM_ALG_SHA256		=3D 0x000B,
>  	TPM_ALG_SHA384		=3D 0x000C,

I had a point after all when asking for split and it was about this
patch. This should be a separate comment and with a clear rationale what
for this is required for the new key type.

It is a logically separate change [1].

> @@ -209,6 +210,7 @@ enum tpm2_return_codes {
>  	TPM2_RC_DISABLED	=3D 0x0120,
>  	TPM2_RC_UPGRADE		=3D 0x012D,
>  	TPM2_RC_COMMAND_CODE    =3D 0x0143,
> +	TPM2_RC_OBJECT_MEMORY	=3D 0x0902,
>  	TPM2_RC_TESTING		=3D 0x090A, /* RC_WARN */
>  	TPM2_RC_REFERENCE_H0	=3D 0x0910,
>  	TPM2_RC_RETRY		=3D 0x0922,

Ditto.

> @@ -227,6 +229,7 @@ enum tpm2_command_codes {
>  	TPM2_CC_CREATE		        =3D 0x0153,
>  	TPM2_CC_LOAD		        =3D 0x0157,
>  	TPM2_CC_SEQUENCE_UPDATE         =3D 0x015C,
> +	TPM2_CC_SIGN		        =3D 0x015D,
>  	TPM2_CC_UNSEAL		        =3D 0x015E,
>  	TPM2_CC_CONTEXT_LOAD	        =3D 0x0161,
>  	TPM2_CC_CONTEXT_SAVE	        =3D 0x0162,

Ditto.

> @@ -234,6 +237,7 @@ enum tpm2_command_codes {
>  	TPM2_CC_VERIFY_SIGNATURE        =3D 0x0177,
>  	TPM2_CC_GET_CAPABILITY	        =3D 0x017A,
>  	TPM2_CC_GET_RANDOM	        =3D 0x017B,
> +	TPM2_CC_HASH	        	=3D 0x017D,
>  	TPM2_CC_PCR_READ	        =3D 0x017E,
>  	TPM2_CC_PCR_EXTEND	        =3D 0x0182,
>  	TPM2_CC_EVENT_SEQUENCE_COMPLETE =3D 0x0185,


Ditto.

> @@ -243,7 +247,8 @@ enum tpm2_command_codes {
>  };
> =20
>  enum tpm2_permanent_handles {
> -	TPM2_RS_PW		=3D 0x40000009,
> +	TPM2_RH_OWNER =3D 0x40000001,

Ditto.

> +	TPM2_RS_PW    =3D 0x40000009,
>  };
> =20
>  enum tpm2_capabilities {
> @@ -312,9 +317,12 @@ struct tpm_buf {
>  };
> =20
>  enum tpm2_object_attributes {
> -	TPM2_OA_FIXED_TPM		=3D BIT(1),
> -	TPM2_OA_FIXED_PARENT		=3D BIT(4),
> -	TPM2_OA_USER_WITH_AUTH		=3D BIT(6),
> +	TPM2_OA_FIXED_TPM 		=3D BIT(1),
> +	TPM2_OA_FIXED_PARENT 		=3D BIT(4),
> +	TPM2_OA_SENSITIVE_DATA_ORIGIN	=3D BIT(5),
> +	TPM2_OA_USER_WITH_AUTH 		=3D BIT(6),
> +	TPM2_OA_RESTRICTED 		=3D BIT(16),
> +	TPM2_OA_SIGN 			=3D BIT(18),
>  };
> =20
>  enum tpm2_session_attributes {

Ditto.

[1] https://www.kernel.org/doc/html/latest/process/submitting-patches.html#=
separate-your-changes

BR, Jarkko