On Mon, Sep 21, 2020 at 07:28:09PM -0700, James Bottomley wrote:
> The current implementation appends a migratable flag to the end of a
> key, meaning the format isn't exactly interoperable because the using
> party needs to know to strip this extra byte. However, all other
> consumers of TPM sealed blobs expect the unseal to return exactly the
> key. Since TPM2 keys have a key property flag that corresponds to
> migratable, use that flag instead and make the actual key the only
> sealed quantity. This is secure because the key properties are bound
> to a hash in the private part, so if they're altered the key won't
> load.
>
> Backwards compatibility is implemented by detecting whether we're
> loading a new format key or not and correctly setting migratable from
> the last byte of old format keys.
>
> Signed-off-by: James Bottomley <James.Bottomley@xxxxxxxxxxxxxxxxxxxxx>
> Reviewed-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
Tested-by: Jarkko Sakkinen <jarkko.sakkinen@xxxxxxxxxxxxxxx>
➜ tpm2-scripts (master) ✗ sudo keyctl add trusted kmk2 "new 32 blobauth=world keyhandle=80000000 migratable=1" @u
add_key: Invalid argument
➜ tpm2-scripts (master) ✗ sudo keyctl add trusted kmk2 "new 32 blobauth=world keyhandle=80000000 migratable=0" @u
608433517
Showed the -EINVAL example just to point out this:
case Opt_migratable:
if (*args[0].from == '0')
pay->migratable = 0;
else
return -EINVAL;
break;
I think it should just set migratable in both cases even if no-op,
given that it takes the value and also the documentation says that
"migratable=1" is legit:
"migratable= 0|1 indicating permission to reseal to new PCR values,
default 1 (resealing allowed)"
Obviously not a concern of this patch but this is still IMHO a bug.
Would be nce if you could drop a prepending patch to fix this, when you
rebase the series, with this fixes tag:
Fixes: d00a1c72f7f4 ("keys: add new trusted key-type")
BTW, please check my fixes so that I can push them ASAP and you get to
rebase this and we can land it. Now everything is properly tested.
/Jarkko
