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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 2023-12-07 at 06:52 +0200, Jarkko Sakkinen wrote:
> 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.

Um, you mean you mentioned it once and I explained that the API is
unitary so logically it does belong in one patch and you didn't mention
it again?

> 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.

There's no length limit on patch sizes, just the recommendation to keep
the changes logical.  One patch per API is actually illogical and
contrary to the guide because the APIs come in sets, so you'd miss the
logical reviewability with that split.  I suppose what I could do is
split it into three logically complete API sets: 1) primary creation;
2) session start/end 3) rest of the session HMAC helpers.  That would
give three patches of 400-600 lines each.

James





[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [Linux Kernel]     [Linux Kernel Hardening]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux SCSI]

  Powered by Linux