Re: [PATCH v5 0/5] Lazy flush for the auth session

[Roberto Sassu]

On Fri, 2024-10-11 at 17:06 +0300, Jarkko Sakkinen wrote:
> On Sat, 2024-09-21 at 15:08 +0300, Jarkko Sakkinen wrote:
> > This patch set aims to fix:
> > https://bugzilla.kernel.org/show_bug.cgi?id=219229.
> > 
> > The baseline for the series is the v6.11 tag.
> > 
> > v4:
> > https://lore.kernel.org/linux-integrity/20240918203559.192605-1-jarkko@xxxxxxxxxx/
> > v3:
> > https://lore.kernel.org/linux-integrity/20240917154444.702370-1-jarkko@xxxxxxxxxx/
> > v2:
> > https://lore.kernel.org/linux-integrity/20240916110714.1396407-1-jarkko@xxxxxxxxxx/
> > v1:
> > https://lore.kernel.org/linux-integrity/20240915180448.2030115-1-jarkko@xxxxxxxxxx/
> > 
> > Jarkko Sakkinen (5):
> >   tpm: Return on tpm2_create_null_primary() failure
> >   tpm: Implement tpm2_load_null() rollback
> >   tpm: flush the null key only when /dev/tpm0 is accessed
> >   tpm: Allocate chip->auth in tpm2_start_auth_session()
> >   tpm: flush the auth session only when /dev/tpm0 is open
> > 
> >  drivers/char/tpm/tpm-chip.c       |  14 ++++
> >  drivers/char/tpm/tpm-dev-common.c |   8 +++
> >  drivers/char/tpm/tpm-interface.c  |  10 ++-
> >  drivers/char/tpm/tpm2-cmd.c       |   3 +
> >  drivers/char/tpm/tpm2-sessions.c  | 109 ++++++++++++++++++----------
> > --
> >  include/linux/tpm.h               |   2 +
> >  6 files changed, 102 insertions(+), 44 deletions(-)
> 
> The summarize some discussions:
> 
> 1. I'll address Stefan's remarks.
> 2. We know that these patches address the desktop boot.
> 3. IMA is too slow => add a boot option for IMA default off. I.e.
>    IMA will not use the feature unless you specifically ask.

Initially, I thought that maybe it would not be good to have an event
log with unmodified and altered measurement entries. Then, I tried to
think if we can really prevent an active interposer from injecting
arbitrary PCR extends and pretending that those events actually
happened.

If I understood James's cover letter correctly, the kernel can detect
whether a TPM reset occurred, but not that a PCR extend occurred (maybe
with a shadow PCR?).

Second point, do we really want to take the responsibility to disable
the protection on behalf of users? Maybe a better choice is to let them
consciously disable HMAC protection.

So, maybe we should keep the HMAC protection enabled, and if the number
of PCR extends is above a certain threshold, we can print a warning
message in the kernel log.

Roberto

> 4. Random generation can be optimized a lot with or without
>    encryption. Not sure if  I have time to do ths right now
>    but I have already patch planned for this.
> 
> What is blocking me is the James' request to not include
> functional fixes. The problem with that is that if comply
> to that request I will have to postpone all the performacne
> fixes and send a patch set with only functional fixes and
> go all review rounds with that before moving forward.
> 
> This is just how priorities go in kernel and doing by the
> book. Is that really necessary?
> 
> Since I've just started in a new job any patches can be
> expected earliest next week. That's why I was rushing with
> the patch set in the first place because I knew that there
> will be otherwise a few week delay but we'll get there :-)
> 
> BR, Jarkko
>