On Fri Dec 8, 2023 at 1:09 AM EET, James Bottomley wrote: > 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. That would be good enough. I've now also verified that call chains look to go through with bpftrace. I have full green flag for including from blackbox testing perspective, the feature has (in my opinion) correct design and also since distributions are gaining TPN sealed encryption this sorts that orthogonally. So in that sense as long as the code changes will become clean enough this is definitely something that we want to Linux (just saying this that polishing work does not go down the drain). I mean with this, it closes how things are sealed e.g. in modern Mac's, at least to the level that the computer is still open for the owner :-) > > James BR, Jarkko
