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

Add ECC AIK template and cache; Work around for CertifyCreation #72

Merged
merged 3 commits into from
Feb 21, 2020

Conversation

jkl73
Copy link
Contributor

@jkl73 jkl73 commented Jan 24, 2020

Certain TPM (the one on my Lenovo X1 carbon) cannot handle CeritfyCreation with a null signing handle correctly. It will throw a TPM_RC_INSUFFICIENT error when we trying to do that.

One work around is that we pass in a non-null signing handle into CertifyCreation.
This PR implement this work around as well as add a ECC AIK template.

AIK ECC key will be cached for performance concerns.
I picked the handle 0x81008FFF (I picked randomly from "Storage -> Available" from Table 11 from https://trustedcomputinggroup.org/wp-content/uploads/RegistryOfReservedTPM2HandlesAndLocalities_v1p1_pub.pdf

@jkl73 jkl73 marked this pull request as ready for review January 24, 2020 00:17
@jkl73 jkl73 changed the title Add ECC AIK template and cache; Walkaroud for CertifyCreation Add ECC AIK template and cache; Work around for CertifyCreation Jan 24, 2020
@jkl73 jkl73 requested a review from josephlr January 24, 2020 00:21
@jkl73 jkl73 force-pushed the fix-certifycreation branch 5 times, most recently from 276b1d0 to a3b82f6 Compare January 29, 2020 01:05
Copy link
Member

@josephlr josephlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the most part this looks good, just some comment nits.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
AIK-RSA AIK-ECC EK-ECC SRK-ECC will all be cached
@jkl73 jkl73 force-pushed the fix-certifycreation branch from a3b82f6 to d3ec7f3 Compare February 13, 2020 01:53
Copy link
Member

@josephlr josephlr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just fix the formatting/structure comments, and then submit.

@jkl73 jkl73 force-pushed the fix-certifycreation branch from d3ec7f3 to f6fe21f Compare February 20, 2020 23:01
Certain TPM (the one on my Lenovo X1 Carbon) cannot handle
CertifyCreation with a null signing key correctly (will throw
TPM_RC_INSUFFICENT error). This work around will try CertifyCreation
again with a non-null (AIK-ECC) signing handle if that happens.
@jkl73 jkl73 force-pushed the fix-certifycreation branch from f6fe21f to d86d009 Compare February 20, 2020 23:46
@jkl73 jkl73 merged commit a00745d into google:master Feb 21, 2020
@savely-krasovsky
Copy link
Contributor

savely-krasovsky commented Apr 10, 2024

@jkl73 I understand that this is PR is from 2020, but I catch this issue even with your patch on Lenovo X1 Yoga 3rd Gen (Windows 10). Still getting TPM_RC_INSUFFICENT error: failed to certify creation: handle 0, error code 0x1a : the TPM was unable to unmarshal a value because there were not enough octets in the input buffer. I am trying to use it without admit rights if that important.

I am not a TPM expert, but I observe that I cannot get ECC SRK without administrative rights, probably because after reboot Windows initializes only RSA one by default. Maybe it's connected, because this patch tries to use ECC cached key.

@jkl73
Copy link
Contributor Author

jkl73 commented Apr 10, 2024

@jkl73 I understand that this is PR is from 2020, but I catch this issue even with your patch on Lenovo X1 Yoga 3rd Gen (Windows 10). Still getting TPM_RC_INSUFFICENT error: failed to certify creation: handle 0, error code 0x1a : the TPM was unable to unmarshal a value because there were not enough octets in the input buffer. I am trying to use it without admit rights if that important.

I'm not familiar with how admin rights affecting TPM on windows, does it work with admin rights?

I am not a TPM expert, but I observe that I cannot get ECC SRK without administrative rights, probably because after reboot Windows initializes only RSA one by default. Maybe it's connected, because this patch tries to use ECC cached key.

Also maybe your TPM doesn't support the default AIKTemplateECC parameter (CurveNISTP256)? Or maybe you can try to use the RSA SRK to do the workaround.

@savely-krasovsky
Copy link
Contributor

I'm not familiar with how admin rights affecting TPM on windows, does it work with admin rights?

Basically Windows doesn't allow you to get ECC SRK without admin rights for some reason. But after getting it for the first time (for example by spawning one-shot Windows service which just calling client.StorageRootKeyECC()), it will be available for other users without requiring admin rights, weird.

I didn't try with admin rights with production-ready piece of software because it does not meant to run under admin.

Also maybe your TPM doesn't support the default AIKTemplateECC parameter (CurveNISTP256)?

Probably, but it successfully passing those lines:

signer, err := AttestationKeyECC(k.rw)
if err != nil {
	return nil, fmt.Errorf("failed to create fallback signing key: %w", err)
}

Otherwise it would return error here, isn't it?

Or maybe you can try to use the RSA SRK to do the workaround.

We are already using RSA SRK and for the majority of users it works flawlessly, sealing, unsealing, etc. But during testing getting error at unsealing stage with this Lenovo Yoga.

Does it worth a try to change this line from: signer, err := AttestationKeyECC(k.rw) to signer, err := AttestationKeyRSA(k.rw)?

@jkl73
Copy link
Contributor Author

jkl73 commented Apr 10, 2024

I'm not familiar with how admin rights affecting TPM on windows, does it work with admin rights?

Basically Windows doesn't allow you to get ECC SRK without admin rights for some reason. But after getting it for the first time (for example by spawning one-shot Windows service which just calling client.StorageRootKeyECC()), it will be available for other users without requiring admin rights, weird.

I didn't try with admin rights with production-ready piece of software because it does not meant to run under admin.

Also maybe your TPM doesn't support the default AIKTemplateECC parameter (CurveNISTP256)?

Probably, but it successfully passing those lines:

signer, err := AttestationKeyECC(k.rw)
if err != nil {
	return nil, fmt.Errorf("failed to create fallback signing key: %w", err)
}

Otherwise it would return error here, isn't it?

Or maybe you can try to use the RSA SRK to do the workaround.

We are already using RSA SRK and for the majority of users it works flawlessly, sealing, unsealing, etc. But during testing getting error at unsealing stage with this Lenovo Yoga.

Does it worth a try to change this line from: signer, err := AttestationKeyECC(k.rw) to signer, err := AttestationKeyRSA(k.rw)?

Yes, I think it's worth to try, at least it should reveal if the problem is related to the ecc algo or not.

savely-krasovsky added a commit to savely-krasovsky/go-tpm-tools that referenced this pull request Apr 11, 2024
jkl73 pushed a commit that referenced this pull request Apr 15, 2024
* fix invalid check and restore workaround from #72

* fix: check both param and handle errors
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

Successfully merging this pull request may close these issues.

None yet

3 participants