Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement TPM2_Commit #287

Closed
akakou opened this issue Jun 24, 2022 · 12 comments
Closed

Implement TPM2_Commit #287

akakou opened this issue Jun 24, 2022 · 12 comments

Comments

@akakou
Copy link
Contributor

akakou commented Jun 24, 2022

It needs when implementing ECDAA(e.g. #278 (comment)).

tpm2_commit(1) - Performs the first part of an ECC anonymous signing operation. The TPM will perform the point multiplications on the provided points and return intermediate signing values. The signing key is an ECC key. The key cannot be a sign+decrypt key and must have an anonymous signing scheme. TPM_ALG_ECDAA is the only supported anonymous scheme.

https://manpages.ubuntu.com/manpages/jammy/man1/tpm2_commit.1.html

@akakou
Copy link
Contributor Author

akakou commented Jul 8, 2022

Now I'm implementing the test code of TPM2_Commit, but not work.
Do you have any idea to solve this error?

go test -run TestCommit
--- FAIL: TestCommit (0.01s)
    commit_test.go:102: could not commit: TPM_RC_AUTH_MISSING: command requires an authorization session for handle and it is not present.
FAIL
exit status 1
FAIL    github.com/google/go-tpm/direct/tpm2    0.012s

https://github.com/akakou/go-tpm/blob/tpm2-commit/direct/tpm2/commit_test.go

@chrisfenner
Copy link
Member

Can you try removing the space between the comma-separated type annotations at https://github.com/akakou/go-tpm/blob/dafdbfea0c93185d68e8afe8eccf4c520fa48456/direct/tpm2/tpm2.go#L625? I.e.,

	SignHandle handle `gotpm:"handle,auth"`

@akakou
Copy link
Contributor Author

akakou commented Jul 10, 2022

Thank you! It's one of the bugs and I fixed it.
But it has never worked still...and panic with the error following:

--- FAIL: TestCommit (0.00s)
    commit_test.go:102: could not commit: TPM_RC_SIZE (handle 0): structure is the wrong size
FAIL
exit status 1
FAIL    github.com/google/go-tpm/direct/tpm2    0.008s

Do you have any idea?

@akakou
Copy link
Contributor Author

akakou commented Jul 12, 2022

I added to create a key that is not the primary key because the basic sample program needs to run TPM2_Create.
(But I don't know why it is needed.)

However, the program has the same error as before...

@chrisfenner
Copy link
Member

TPM_RC_SIZE (handle 0): structure is the wrong size

looks like an ill-formed error code to me. Handle indices start at 1, so "handle 0" doesn't make a lot of sense. I'm assuming the test code got back raw error code 0x95, but we should investigate whether there's a bug in the error interpretation stack. Filed #290 to follow up on our side.

Confession: I still don't understand ECDAA. Please correct me where my crypto veers astray 😅

P1: tpm2b.ECCPoint{
	Point: tpms.ECCPoint{
		X: tpm2b.ECCParameter{
			Buffer: []byte{
				0x1, 0x1,
			},
		},
		Y: tpm2b.ECCParameter{
			Buffer: []byte{0x1, 0x1},
		},
	},
	Size: 8,
},

P1 doesn't look like a real ECC point to me. Is 0x0101,0x0101 a legitimate BNP256 point?

Second issue, looks like you added TPM2B_ECC_Point but there is some subtlety about marshalling I need to point out:

// TPM2BECCPoint represents a TPM2B_ECC_POINT.
// See definition in Part 2: Structures, section 11.2.5.3.
type TPM2BECCPoint struct {
	Size  uint16
	Point TPMSECCPoint
}

In go-tpm tpmdirect, sized 2Bs' sizes are never passed in by the user. Instead they are computed on the fly. Can you fix this to

// TPM2BECCPoint represents a TPM2B_ECC_POINT.
// See definition in Part 2: Structures, section 11.2.5.3.
type TPM2BECCPoint struct {
	Point TPMSECCPoint `gotpm:"sized"`
}

The sized type annotation instructs the marshalling/unmarshlaling

@akakou
Copy link
Contributor Author

akakou commented Jul 14, 2022

Thank you! You are right and I fixed them.

Specifically, I fixed them as:

  1. Set x=0x1, y=0x2 to P1.
  2. Use annotation to express the size of TPM2B_ECC_POINT

@akakou
Copy link
Contributor Author

akakou commented Jul 14, 2022

But the program calls the same error...

@akakou
Copy link
Contributor Author

akakou commented Jul 25, 2022

I compared worked TPM2_Commit(tpm2-tools) and this library.
Mainly, I compared what binaries each library sent to TPM.

These are the binaries:

go-tpm

\x80\x02\x00\x00\x00\x40\x00\x00\x01\x91\x40\x00\x00\x01\x00\x00\x00\x09\x40\x00\x00\x09\x00\x00\x00\x00\x00\x00\x07\x00\x03\x00\x01\x03\x00\x00\x00\x1a\x00\x23\x00\x04\x00\x04\x00\x72\x00\x00\x00\x10\x00\x1a\x00\x0b\x00\x00\x00\x10\x00\x10\x00\x00\x00\x00

tpm2-tools

\x80\x02\x00\x00\x00\x65\x00\x00\x01\x8b\x80\xff\xff\xff\x00\x00\x00\x49\x02\x00\x00\x00\x00\x20\x14\xe2\x02\x60\x66\x57\x5f\x9c\xab\x50\x13\x21\x36\x37\xc3\xfd\x24\x86\xf8\xae\x9f\xa1\x0b\x62\x3e\x4a\x00\xa8\x91\x8d\xf3\x10\x01\x00\x20\x2f\xc0\x53\x68\x30\xa0\x0b\xc3\x88\x5f\x43\xed\x73\x00\xa8\x79\x87\xae\x96\x5a\x78\xa3\xe1\xda\x40\xa4\xa0\x0d\xae\x11\x4f\xca\x00\x04\x00\x00\x00\x00\x00\x00\x00\x00

@akakou
Copy link
Contributor Author

akakou commented Jul 26, 2022

Sorry...I had added the unnecessary parameter to the request of TPM2_Commit.
Specifically, we should not add the count to the request.

@akakou
Copy link
Contributor Author

akakou commented Jul 26, 2022

@chrisfenner

So I fixed it and it works.
Could you check the PR (#296) and merge it?

@akakou
Copy link
Contributor Author

akakou commented Jul 27, 2022

#296 (comment)

@akakou
Copy link
Contributor Author

akakou commented Aug 24, 2022

It is implemented at #296, so I closed this issue.

@akakou akakou closed this as completed Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants