Re: [PATCH v5 12/17] tpm: Add full HMAC and encrypt/decrypt session handling code

Jarkko Sakkinen
2023-12-07T04:52:28Z

On Mon Nov 27, 2023 at 9:08 PM EET, James Bottomley wrote:
> Add session based HMAC authentication plus parameter decryption and
> response encryption using AES. The basic design is to segregate all
> the nasty crypto, hash and hmac code into tpm2-sessions.c and export a
> usable API. The API first of all starts off by gaining a session with
>
> tpm2_start_auth_session()
>
> which initiates a session with the TPM and allocates an opaque
> tpm2_auth structure to handle the session parameters. The design is
> that session use will be single threaded from start to finish under
> the ops lock, so the tpm2_auth structure is stored in struct
> tpm2_chip. Then the use is simply:
>
> * tpm_buf_append_name() in place of the tpm_buf_append_u32 for the
> handles
>
> * tpm_buf_append_hmac_session() where tpm2_append_auth() would go
>
> * tpm_buf_fill_hmac_session() called after the entire command buffer
> is finished but before tpm_transmit_cmd() is called which computes
> the correct HMAC and places it in the command at the correct
> location.

Split each exported function into a separate patches. This too big
chunk of diff to be reviawable, i.e. it is impossible to give
reviewed-by in this form. I think I've commented this also throughout
the series, and it has not been changed.

There needs to be a patch per each exported API function so that they
can be looked into detail. This patch does not align with submission
guidelines in the form it is either.

I can give only one half review right now, which "yes wee need this"
but cannot give any rational feedback whether it is in the form we
would like it to be.

BR, Jarkko