Re: [PATCH 1/3] tpm: Disable TCG_TPM2_HMAC by default

[Jarkko Sakkinen]

On Tue May 28, 2024 at 4:04 AM EEST, Jarkko Sakkinen wrote:
> On Tue May 28, 2024 at 2:44 AM EEST, James Bottomley wrote:
> > On Tue, 2024-05-28 at 02:17 +0300, Jarkko Sakkinen wrote:
> > > On Tue May 28, 2024 at 12:36 AM EEST, James Bottomley wrote:
> > > > On Mon, 2024-05-27 at 22:53 +0300, Jarkko Sakkinen wrote:
> > > > > On Mon May 27, 2024 at 8:57 PM EEST, James Bottomley wrote:
> > > > > > On Mon, 2024-05-27 at 18:34 +0300, Jarkko Sakkinen wrote:
> > > > [...]
> > > > > > > While looking at code I started to wanted what was the
> > > > > > > reasoning for adding *undocumented* "TPM2_OA_TMPL" in
> > > > > > > include/linux/tpm.h.It should really be in tpm2-sessions.c
> > > > > > > and named something like TPM2_NULL_KEY_OA or similar.
> > > > > > 
> > > > > > Well, because you asked for it. I originally had all the flags
> > > > > > spelled out and I'm not a fan of this obscurity, but you have
> > > > > > to do stuff like this to get patches accepted:
> > > > > > 
> > > > > > https://lore.kernel.org/linux-integrity/CZCKTWU6ZCC9.2UTEQPEVICYHL@suppilovahvero/
> > > > > 
> > > > > I still think the constant does make sense.
> > > > 
> > > > I'm not so sure.  The TCG simply defines it as a collection of
> > > > flags and every TPM tool set I've seen simply uses a list of flags
> > > > as well.  The original design was that the template would be in
> > > > this one place and everything else would call into it.  I think the
> > > > reason all template construction looks similar is for ease of
> > > > auditing (it's easy to get things, particularly the flags, wrong).
> > > > 
> > > > If it only has one use case, it should be spelled out but if
> > > > someone else would use it then it should be in the tpm.h shared
> > > > header.
> > > 
> > > It is used only in tpm2-sessions.c and for the null key so there it
> > > should be. And it is also lacking the associated documentation. Now
> > > both name and context it is used is lost.
> >
> > The comment above the whole thing says what it is and where it comes
> > from:
> >
> > 	/*
> > 	 * create the template.  Note: in order for userspace to
> > 	 * verify the security of the system, it will have to create
> > 	 * and certify this NULL primary, meaning all the template
> > 	 * parameters will have to be identical, so conform exactly to
> > 	 * the TCG TPM v2.0 Provisioning Guidance for the SRK ECC
> > 	 * key H template (H has zero size unique points)
> > 	 */
> >
> > If we put the broken out flags back it's all fully documented.
>
> Not the most productive conclusion when refusing to follow properly a
> trivial request in the review feedback tbh.

In any case this particular constant can be revisited when otherwise
changes happen in the area. It is what it is for the time being. I
just need to use more strict and dense filter when check the patch
revisions next time.

BR, Jarkko