On Mon, Sep 14, 2020 at 12:19:28PM -0700, James Bottomley wrote:
> On Mon, 2020-09-14 at 20:41 +0300, Jarkko Sakkinen wrote:
> > On Mon, Aug 17, 2020 at 02:35:06PM -0700, James Bottomley wrote:
> > > Create sysfs per hash groups with 24 PCR files in them one group,
> > > named pcr-<hash>, for each agile hash of the TPM. The files are
> > > plugged in to a PCR read function which is TPM version agnostic, so
> > > this works also for TPM 1.2 but the hash is only sha1 in that case.
> > >
> > > Note: the macros used to create the hashes emit spurious checkpatch
> > > warnings. Do not try to "fix" them as checkpatch recommends,
> > > otherwise
> > > they'll break.
> >
> > "PCR access is required because IMA tools should be able to run
> > without any sort of TSS dependencies."
> >
> > AFAIK, this is the only reason to merge this and it is missing from
> > the description. Perhaps you could either include that sentence, or
> > alternatively write something along the lines?
>
> Sure, I'll add all of them: it's IMA tools, early boot and key locking
> to PCR policy.
Great!
> > > Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.c
> > > om>
> > > Reviewed-by: Jerry Snitselaar <jsnitsel@xxxxxxxxxx>
> > > Tested-by: Thiago Jung Bauermann <bauerman@xxxxxxxxxxxxx>
> >
> > Please also cc this at least to Greg and Jason Gunthorpe next time.
>
> OK
>
> [...]
>
> > >
> > enum tpm_alg_misc {
> > TPM_ALG_ERROR = 0x0000,
> > TPM_ALG_KEYEDHASH = 0x0008,
> > TPM_ALG_NULL = 0x0010,
> > }
> >
> > enum tpm_alg_hash {
> > TPM_ALG_SHA1 = 0x0004,
> > TPM_ALG_SHA256 = 0x000B,
> > TPM_ALG_SHA384 = 0x000C,
> > TPM_ALG_SHA512 = 0x000D,
> > TPM_ALG_SM3_256 = 0x0012,
> > TPM_ALG_HASH_MAX,
> > };
>
> I can separate them if you insist, but the latter construction won't
> work. TPM_ALG_HASH_MAX will get set to the previous value plus one.
>
> You can see this with the test programme:
>
> ---
> #include <stdio.h>
> #include <stdlib.h>
> #include <unistd.h>
>
> enum tpm_alg_hash {
> TPM_ALG_SHA1 = 0x0004,
> TPM_ALG_SHA256 = 0x000B,
> TPM_ALG_SHA384 = 0x000C,
> TPM_ALG_SHA512 = 0x000D,
> TPM_ALG_SM3_256 = 0x0012,
> TPM_ALG_HASH_MAX,
> };
>
> int main()
> {
> printf("TPM_ALG_HASH_MAX = %d\n", TPM_ALG_HASH_MAX);
> }
> ---
>
> Which gives
>
> jejb@jarvis> ./a.out
> TPM_ALG_HASH_MAX = 19
>
> Which is clearly the wrong value (it's 0x12 + 1).
>
> That being so, is there any reason to separate up the algorithms enum?
>
> James
No, my bad.
/Jarkko
