Re: [PATCH v7 12/21] tpm: Add NULL primary creation

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Sun Mar 31, 2024 at 7:00 PM EEST, Jarkko Sakkinen wrote:
> On Sat Mar 30, 2024 at 8:48 PM EET, Gabríel Arthúr Pétursson wrote:
> > On Tue, 2024-02-13 at 12:13 -0500, James Bottomley wrote:
> > > +	/* unique: key specific; for ECC it is two points */
> > > +	tpm_buf_append_u16(&template, 0);
> > > +	tpm_buf_append_u16(&template, 0);
> >
> > Shouldn't the points be 32 bytes each in size, filled with zeros?
> >
> > The TCP TPM 2.0 Provisioning Guidance defines the SRK Template as a
> > diff on top of Template L-2 (for ECC keys) as defined in the EK
> > Credential Profile 2.0 document.
> >
> > Template L-2 calls for the X and Y points to be of 32 bytes each,
> > filled with zeros. The Provisioning Guidance does not call for zero-
> > sized points.
> >
> > For example, let's create an ECC Endorsement Key using the standard
> > template then print its name:
> >
> >    tpm2_createek -G ecc -c /dev/null -u ./ek.pub
> >    tpm2_loadexternal -c n -u ./ek.pub
> >
> > Equivalently using tpm2_createprimary:
> >
> >    perl -e 'print "\0"x64' | tpm2_createprimary -C e -o ./ek.pub -G ecc -a 'fixedtpm|fixedparent|sensitivedataorigin|adminwithpolicy|restricted|decrypt' -L 837197674484b3f81a90cc8d46a5d724fd52d76e06520b64f2a1da1b331469aa -u -
> >    tpm2_loadexternal -c n -u ./ek.pub
> >
> > You'll find that the key's public modulus matches that of the EK
> > Certificate imprinted by the manufacturer, indicating we got the
> > template correct.
> >
> > To generate a standard SRK key, the TCG TPM2 Provisioning Guidance
> > states we should:
> >
> > 	1. set userWithAuth,
> > 	2. clear adminWithPolicy
> > 	3. set noDA, and
> > 	4. clear the authorization policy.
> >
> > There's no mention of alterations to the unique field.
> >
> > Let's also create the key in the null hierarchy:
> >
> >    perl -e 'print "\0"x64' | tpm2_createprimary -C n -o ./null.pub -G ecc -a 'fixedtpm|fixedparent|sensitivedataorigin|userwithauth|noda|restricted|decrypt' -u -
> >    tpm2_loadexternal -c n -u ./null.pub
> >
> > The name does not match the kernel's name for the null key.
>
> Null key is not provisioned, what is the motivation here?
>
> Not saying no, just asking for details...
>
> There's couple of things that lack in this patch set ATM:
>
> 1. Neither kselftest additions nor not even proper testing
>    instructions. Why 21 patches and zero tests? How one should
>    decide when the work is "complete"?
> 2. I still don't know what version of QEMU I should patch to
>    test corner cases from an URL, which I cannot recall :-)
>    Highlights the first issue.
>
> So for the time being the patch set is NAK just because we lack
> clear definition of done here. I revisit it only when I know how
> to test it.

Your feedback was great because it highlights the issue that the
excepted behaviour is non-existent at this point.

BR, Jarkko