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

TPM2 Quote takes an Algorithm where it should take a Scheme #201

Open
chrisfenner opened this issue Jul 30, 2020 · 2 comments
Open

TPM2 Quote takes an Algorithm where it should take a Scheme #201

chrisfenner opened this issue Jul 30, 2020 · 2 comments

Comments

@chrisfenner
Copy link
Member

func Quote(rw io.ReadWriter, signingHandle tpmutil.Handle, parentPassword, ownerPassword string, toQuote []byte, sel PCRSelection, sigAlg Algorithm) ([]byte, *Signature, error) {

The last parameter, sigAlg, is an Algorithm which corresponds to TPM_ALG in the spec, but it should be some type that corresponds to TPMT_SIG_SCHEME in the spec. As written, the only parameter that can be passed to Quote successfully is AlgNull (meaning you have to set the scheme on the AK when you create it).

This is what the test does:

go-tpm/tpm2/test/tpm2_test.go

Lines 1258 to 1279 in ac5b427

func TestQuote(t *testing.T) {
rw := openTPM(t)
defer rw.Close()
params := Public{
Type: AlgRSA,
NameAlg: AlgSHA256,
Attributes: FlagSignerDefault | FlagNoDA,
RSAParameters: &RSAParams{
Sign: &SigScheme{
Alg: AlgRSASSA,
Hash: AlgSHA256,
},
KeyBits: 2048,
},
}
keyHandle, pub, _, _, _, _, err := CreatePrimaryEx(rw, HandleEndorsement, pcrSelection7, emptyPassword, emptyPassword, params)
if err != nil {
t.Fatalf("CreatePrimaryEx failed: %s", err)
}
defer FlushContext(rw, keyHandle)
attestation, signature, err := Quote(rw, keyHandle, emptyPassword, emptyPassword, nil, pcrSelection7, AlgNull)

@josephlr
Copy link
Member

We could try to create a scheme based on the Alg, but that seems like a difficult proposition (why key length do we use for RSASSA for instance).

The real solution would be having the params for all functions match the Spec directly. Maybe we should open a tracking issue.

@chrisfenner
Copy link
Member Author

Yeah, I don't see how to fix this without a frustrating breaking change to the API.

Long term, I would love to reflect the spec into the API. In the short term, it may be good to fix this and eat the breaking change, or simply document that it only supports AlgNull and that you have to create an AK with the signing scheme that you want.

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