Fix bugs around creating ECDAA key

[akakou]

There are the following bugs around creating ECDAA key:

• CurveBNP256 number is wrong
  • TPM expects 0x10 to TPM2_ECC_BN_P256 but this library does not set so.
• ECDAA count (in TPMS_SCHEME_ECDAA) size is wrong
  • TPM expects 16bit but, actually 32bit in go-tpm.

So this PR fixes these bugs.

Reference

• https://trustedcomputinggroup.org/wp-content/uploads/TSS_Overview_Common_Structures_Version-0.9_Revision-03_Review_030918.pdf

[google-cla]

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

[chrisfenner]

Would you mind fixing this one too (0x11)? I'm fine replacing all the constants in this const block for EllipticCurve with explicit values from the spec, btw.

[akakou]

Did you mean the CurveBNP638?

[akakou]

@chrisfenner

[akakou]

I replaced them on a new commit!

[chrisfenner]

This change looks good, only a small fix for CurveBNP638.

BTW, we are doing a major refactor to the API in https://github.com/google/go-tpm/tree/tpmdirect - have you thought about how ECDAA could be exposed via the legacy command model, or do you want to build this in TPMDirect?

[akakou]

Umm...now I'm trying to implement some TPM commands (e.g. TPM2_Commit #287) which are needed by ECDAA, to tpmdirect.

But I don't think that I develop ECDAA API (e.g. ECDAA_JOIN) directly, because there is some process that should not be in the scope of the TPM.
(FIDO ECDAA defines the other module running on a client named ASM. There are some processes for ECDAA running on ASM, not on TPM)

https://fidoalliance.org/specs/fido-v2.0-id-20180227/fido-ecdaa-algorithm-v2.0-id-20180227.html#ecdaa-sign-split-between-tpm-and-asm

[chrisfenner]

Sounds good. Do you want to send a PR to the tpmdirect branch instead of master?

[akakou]

You are right! I will change the destination branch to tpmdirect.

[akakou]

But I am concerned that the master branch remains wrong about BN256 and ECDAA.
Because some people using BN256 or ECDAA with master branch may be confused.

I would like to confirm just in case; the tpmdirect branch will be merged with the master branch finally, right?

[chrisfenner]

That's right, in the long run we expect to merge tpmdirect to master and cut a new release where that is the default go-tpm API and the current one is deprecated. As far as timeline, depends on how long it takes to get it done and bake it - with your help on ECDAA it can be that much sooner :)

[chrisfenner]

@akakou, sorry for being unclear. Thanks for working on this.

These files touched by this PR (constants.go and structures.go) are part of the "legacy" (non-direct) API. If you want to fix them for the master branch I'd be fine with it. Thus, this PR can go to go-tpm:master, however it'd be additional work to figure out how to expose ECDAA through the legacy API without breaking current users. IMHO that's not going to be worth the effort to build safely, given the number of different flavors of Sign in the legacy API.

The alternative, if you're OK with depending on the tpmdirect branch for a while, would be to cancel this PR and send a different one to go-tpm:tpmdirect with:

• TPMS_SCHEME_ECDAA added around here, in direct's structures.go
• A type alias for internal.TPMSSchemeECDAA as tpms.SchemeECDAA here, in direct's tpms.go
• No need to fix TPMECCCurve in direct's constants.go because it's already right
• (Optional, could be in a follow-on PR) a test demonstrating the creation and use of an ECDAA key in a new file just for ECDAA testing (since ECDAA is complicated) in tpm2

[akakou]

This discussion was solved because we decided that contribute to tpmdirect.
So I close this PR.