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 activate-credential service to metadata server #4132

Merged
merged 5 commits into from
Aug 8, 2024

Conversation

shjala
Copy link
Member

@shjala shjala commented Aug 6, 2024

Activate credential provides proof that the Endorsement Key (EK) and a signing key (in this case eve created AIK) are owned by the same TPM. This is a way to extend the trust from EK (which theoretically comes with OEM certificate) to a arbitrary TPM resident, restricted, signing key. Beside the EK<->AIK proof, this endpoint allows signing arbitrary data with the AIK.

This is part of some primarily work for #4071

go-tpm has move the old tpm2 interfaces to "legacy/tpm2",
this commit updates tpm2 import paths in pillar to
reflect the change and make it possible to use
"legacy/tpm2/credactivation" apis.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
@github-actions github-actions bot requested a review from rucoder August 6, 2024 15:27
@shjala shjala changed the title Add activate credential service to metadata server Add activate-credential service to metadata server Aug 6, 2024
@shjala shjala requested a review from uncleDecart August 6, 2024 15:31
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Note that you have some zfs build issue.

pkg/pillar/cmd/tpmmgr/tpmmgr.go Show resolved Hide resolved
pkg/pillar/cmd/msrv/handlers.go Outdated Show resolved Hide resolved
pkg/pillar/cmd/msrv/handlers.go Outdated Show resolved Hide resolved
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

LGTM

@eriknordmark
Copy link
Contributor

The go tests fail at
=== RUN TestTpmActivateCred
time="2024-08-06T19:03:38Z" level=fatal msg="Failed to wait for TPM ready state: Something is wrong with the TPM : stat /dev/tpmrm0: no such file or directory" pid=1234 source=test

@shjala
Copy link
Member Author

shjala commented Aug 7, 2024

The go tests fail at === RUN TestTpmActivateCred time="2024-08-06T19:03:38Z" level=fatal msg="Failed to wait for TPM ready state: Something is wrong with the TPM : stat /dev/tpmrm0: no such file or directory" pid=1234 source=test

working on it.

Comment on lines +102 to +107
// as it contains the type. so make sure the length is greater than 2.
if len(credBlob) < 2 || len(encryptedSecret) < 2 {
Copy link
Contributor

Choose a reason for hiding this comment

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

sure the length is greater than 2

but your check is if it is greater or equal than 2

Copy link
Member Author

Choose a reason for hiding this comment

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

I just want to skip the first two bytes, so it should have at least 2 bytes, it is up to ActivateCredential how it wants to handle empty buffer.

Activate credential provides proof that the Endorsement Key (EK) and
a signig key (in this case eve created AIK) are owned by the same TPM.
This is way to extend the trust from EK (which theoretically comes with
OEM certificate) to a arbitrary TPM resident, restricted, signing key.

The added service allows to stablish trust to the AIK and sign arbitrary
data with it.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
If EvictControl fails, log a warning and continue. This is a non-fatal
error and can happen if the handle does not exist in the TPM.

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
Copy link
Contributor

@eriknordmark eriknordmark left a comment

Choose a reason for hiding this comment

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

Kick off tests

@milan-zededa
Copy link
Contributor

Tests all green but there is one grammar error pointed out by Yetus.

Copy link
Contributor

@uncleDecart uncleDecart left a comment

Choose a reason for hiding this comment

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

LGTM, big thanks from me for writing tests :)

Signed-off-by: Shahriyar Jalayeri <shahriyar@zededa.com>
@eriknordmark eriknordmark merged commit 46c0ac0 into lf-edge:master Aug 8, 2024
18 of 19 checks passed
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.

5 participants