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

[launcher] Merge upstream/tdx_rtmr #513

Merged
merged 2 commits into from
Dec 11, 2024
Merged

[launcher] Merge upstream/tdx_rtmr #513

merged 2 commits into from
Dec 11, 2024

Conversation

jkl73
Copy link
Contributor

@jkl73 jkl73 commented Dec 3, 2024

Merge upstream/tdx_rtmr changes into main.

Squashed commit of the following:

commit 896c633
Author: Jessie Liu jessieqliu@google.com
Date: Wed Oct 30 18:31:27 2024 +0000

Revert "implement AttestationEvidence for TDX"

This reverts commit d15024692582d4fbbf1722c669dd89ce5d62804a.

commit d150246
Author: Jessie Liu jessieqliu@google.com
Date: Wed Oct 30 18:26:48 2024 +0000

implement AttestationEvidence for TDX

commit c38b622
Author: Jiankun Lü jiankun@google.com
Date: Thu Oct 10 18:37:30 2024 -0700

[launcher] Add TDX/RTMR attestation in launcher (#478)

Allow a TDX machine to create a TD quote and request a hardware
rooted attestation from the attestation verifier.

./launcher ci will now only run in linux.

Upgrade go-sev-guest.

Second commit:
Add some logging to make TDX/SEV attest cleaner.
Update cloudbuild files to make the substitution variables cleaner

Verified

This commit was signed with the committer’s verified signature.
sake92 Sakib Hadžiavdić
Squashed commit of the following:

commit 896c633
Author: Jessie Liu <jessieqliu@google.com>
Date:   Wed Oct 30 18:31:27 2024 +0000

    Revert "implement AttestationEvidence for TDX"

    This reverts commit d150246.

commit d150246
Author: Jessie Liu <jessieqliu@google.com>
Date:   Wed Oct 30 18:26:48 2024 +0000

    implement AttestationEvidence for TDX

commit c38b622
Author: Jiankun Lü <jiankun@google.com>
Date:   Thu Oct 10 18:37:30 2024 -0700

    [launcher] Add TDX/RTMR attestation in launcher (google#478)

    Allow a TDX machine to create a TD quote and request a hardware
    rooted attestation from the attestation verifier.

    ./launcher ci will now only run in linux.

    Upgrade go-sev-guest.

Signed-off-by: Jiankun Lu <jiankun@google.com>
@jkl73
Copy link
Contributor Author

jkl73 commented Dec 3, 2024

/gcbrun

@jkl73 jkl73 changed the title Merge upstream/tdx_rtmr [launcher] Merge upstream/tdx_rtmr Dec 3, 2024
@jkl73 jkl73 requested a review from alexmwu December 3, 2024 23:52
@jkl73
Copy link
Contributor Author

jkl73 commented Dec 4, 2024

/gcbrun

@jkl73
Copy link
Contributor Author

jkl73 commented Dec 4, 2024

/gcbrun

@jkl73
Copy link
Contributor Author

jkl73 commented Dec 4, 2024

/gcbrun

@jkl73
Copy link
Contributor Author

jkl73 commented Dec 4, 2024

/gcbrun

Comment on lines 108 to 110
qp, err := tg.GetQuoteProvider()
if err != nil || qp.IsSupported() != nil {
if err != nil { // for now GetQuoteProvider always return a nil err, still adding just for safety
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this not two if statements?

Copy link
Contributor

Choose a reason for hiding this comment

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

why is the error an Info?

qp, err := tg.GetQuoteProvider()
if err != nil {
logger.Error("...")
return nil, fmt.Errorf("...")
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use this condition to determine if it is running on a TDX machine or not.

So qp.IsSupported() will return error on a AMD machine, which is okay, we just need to switch to TPM attestation. So I don't think this warrants an error

}

// check if this is running on a TDX machine
qp, err := tg.GetQuoteProvider()
Copy link
Contributor

Choose a reason for hiding this comment

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

Aren't there better checks? Like if /dev/tdx-guest is available?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I want to avoid checking the device file directly, I think calling this is a easier route for us

Comment on lines 113 to 114
logger.Info(fmt.Sprintf("Cannot create the TDX quote provider (this is expected on a non-TDX machine): %v", qp.IsSupported().Error()))
}
Copy link
Contributor

Choose a reason for hiding this comment

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

if err := qp.IsSupported(); err != nil {
		logger.Info("Using TPM PCRs for measurement")
		// by default using TPM
		attestAgent.ar = &tpmAttestRoot{
			fetchedAK: ak,
			tpm:       tpm,
		}
}

Copy link
Contributor Author

@jkl73 jkl73 Dec 10, 2024

Choose a reason for hiding this comment

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

modifying to


	qp, _ := tg.GetQuoteProvider()
	if err := qp.IsSupported(); err != nil {
		logger.Info(fmt.Sprintf("Cannot create the TDX quote provider (this is expected on a non-TDX machine): %v", err))
		logger.Info("Using TPM PCRs for measurement")
		// if cannot get a TDX Quote Provider, by default using TPM as attest root
		attestAgent.ar = &tpmAttestRoot{
			fetchedAK: ak,
			tpm:       tpm,
		}

@jkl73 jkl73 force-pushed the mergetdx branch 3 times, most recently from fc4ce78 to 5cc38ff Compare December 11, 2024 01:36
@jkl73
Copy link
Contributor Author

jkl73 commented Dec 11, 2024

/gcbrun

Add some logging on support both TDX and SEV attestation.

Update cloudbuild file to make substitution variables
cleaner.

Signed-off-by: Jiankun Lu <jiankun@google.com>
@jkl73
Copy link
Contributor Author

jkl73 commented Dec 11, 2024

/gcbrun

@jkl73 jkl73 merged commit 86a7e85 into google:main Dec 11, 2024
11 checks passed
jkl73 added a commit to jkl73/go-tpm-tools that referenced this pull request Dec 17, 2024
This reverts commit 86a7e85.

Signed-off-by: Jiankun Lu <jiankun@google.com>
jkl73 added a commit to jkl73/go-tpm-tools that referenced this pull request Dec 17, 2024
This reverts commit 86a7e85.

Signed-off-by: Jiankun Lu <jiankun@google.com>
jkl73 added a commit to jkl73/go-tpm-tools that referenced this pull request Dec 17, 2024
This reverts commit 86a7e85.

Signed-off-by: Jiankun Lu <jiankun@google.com>
jkl73 added a commit to jkl73/go-tpm-tools that referenced this pull request Dec 17, 2024
This reverts commit 86a7e85.

Signed-off-by: Jiankun Lu <jiankun@google.com>
jkl73 added a commit that referenced this pull request Dec 17, 2024
This reverts commit 86a7e85.

Signed-off-by: Jiankun Lu <jiankun@google.com>
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

2 participants