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

Integrate nonce verification as part of the TDX quote validation procedure. #395

Merged
merged 7 commits into from
Dec 28, 2023

Conversation

jrjatin
Copy link
Contributor

@jrjatin jrjatin commented Nov 28, 2023

With the integration of the validate package, the go-tdx-guest library now supports TDX quote validation using nonce. I have implemented nonce verification in the TDX quote verification function and added corresponding testcases.

server/verify_test.go Outdated Show resolved Hide resolved
Verification *tv.Options
}

// TdxDefaultOptions returns a default verification options for TDX
// TdxDefaultValidateOpts returns a default validation policy for TDX attestation quote on GCE.
func TdxDefaultValidateOpts(tpmNonce []byte) *validate.Options {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: it appears this is not always the same as the tpmNonce. We should just name this teeNonce or tdxNonce to be clear this is used in validating the TDX quote contents

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Copy link
Contributor

Choose a reason for hiding this comment

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

I still see tpmNonce

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Missed changing name in this function. Now I have renamed here as well.

server/verify_tdx.go Outdated Show resolved Hide resolved
server/verify_test.go Show resolved Hide resolved
cmd/verify.go Show resolved Hide resolved
server/verify_test.go Outdated Show resolved Hide resolved
server/verify_test.go Show resolved Hide resolved
server/verify_test.go Outdated Show resolved Hide resolved
server/verify_test.go Outdated Show resolved Hide resolved
@jrjatin jrjatin requested a review from alexmwu December 19, 2023 06:37
@@ -143,3 +149,82 @@ func TestHwAttestationPass(t *testing.T) {
})
}
}

func TestTdxAttestation(t *testing.T) {
file1, err := os.Create("attest")
Copy link
Contributor

Choose a reason for hiding this comment

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

Tests should use https://pkg.go.dev/testing#T.TempDirtemp then create files in that temp directory.

Copy link
Contributor

Choose a reason for hiding this comment

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

Should have a more descriptive name like attestFile

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for this info. Created a temp dir and renamed it to attestFile

Comment on lines 220 to 229
var out []byte
if format == "binarypb" {
out, err = proto.Marshal(attestation)
if err != nil {
return nil, fmt.Errorf("failed to marshal attestation proto: %v", attestation)
}
} else {
out = []byte(marshalOptions.Format(attestation))
}
return out, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

This function would make more sense returning an Attestation proto message rather than the marshaled proto. This should be test-specific logic, and AFAICT we don't use the format flag right now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I have changed function output to attestation proto message. I have added format flag as textproto in verify args so that marshalling logic is test specific.

cmd/verify_test.go Outdated Show resolved Hide resolved
cmd/verify_test.go Outdated Show resolved Hide resolved
Verification *tv.Options
}

// TdxDefaultOptions returns a default verification options for TDX
// TdxDefaultValidateOpts returns a default validation policy for TDX attestation quote on GCE.
func TdxDefaultValidateOpts(tpmNonce []byte) *validate.Options {
Copy link
Contributor

Choose a reason for hiding this comment

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

I still see tpmNonce

server/verify_tdx.go Outdated Show resolved Hide resolved
@jrjatin jrjatin requested a review from alexmwu December 28, 2023 07:13
"io"
"sync"
"testing"

"github.com/google/go-attestation/attest"
"github.com/google/go-tpm-tools/simulator"
"github.com/google/go-tpm/legacy/tpm2"
gotpm "github.com/google/go-tpm/tpm2"
Copy link
Contributor

Choose a reason for hiding this comment

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

Why did you rename the import here? Generally avoid this: https://google.github.io/styleguide/go/decisions#import-renaming

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since tpm2 is imported from 2 different packages, so I have added alias to one of tpm2 import. I have renamed the alias to gtpm2 to avoid any confusion.

@jrjatin jrjatin requested a review from alexmwu December 28, 2023 18:06
@alexmwu alexmwu merged commit 7c76635 into google:main Dec 28, 2023
10 of 11 checks passed
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Feb 22, 2024
New Features:
[launcher] Add TEE server IPC implementation google#367
[launcher] Enable memory monitoring in CS google#391
Use TDX quote provider to attest and verify google#405
Integrate nonce verification as part of the TDX quote validation procedure. google#395
Add RISC V support google#407
[launcher] Use resizable integrity-fs with in-memory tags google#412

Bug Fixes:
[launcher] Fix launcher exit code google#384
[launcher] Handle exit code checking during deferral evaluation google#392
[cmd] Skip tests that call setGCEAKTemplate google#402
[launcher] Fix teeserver context reset issue & add container signature cache google#397
Set all unused parameters as _ to fix CI lint failure google#411
[launcher] Make customtoken test sleep to mitigate clock skew google#413

Other Changes:
Add eventlog parse logics for memory monitoring google#404
[launcher]: Add memory monitor measurement logics google#408
Update go-tdx-guest version to v0.3.1 google#414

New Contributors:
@KeithMoyer in google#392
@vbalain in google#405
@aimixsaka in google#407
@alexmwu alexmwu mentioned this pull request Feb 22, 2024
alexmwu added a commit that referenced this pull request Feb 22, 2024
New Features:
[launcher] Add TEE server IPC implementation #367
[launcher] Enable memory monitoring in CS #391
Use TDX quote provider to attest and verify #405
Integrate nonce verification as part of the TDX quote validation procedure. #395
Add RISC V support #407
[launcher] Use resizable integrity-fs with in-memory tags #412

Bug Fixes:
[launcher] Fix launcher exit code #384
[launcher] Handle exit code checking during deferral evaluation #392
[cmd] Skip tests that call setGCEAKTemplate #402
[launcher] Fix teeserver context reset issue & add container signature cache #397
Set all unused parameters as _ to fix CI lint failure #411
[launcher] Make customtoken test sleep to mitigate clock skew #413

Other Changes:
Add eventlog parse logics for memory monitoring #404
[launcher]: Add memory monitor measurement logics #408
Update go-tdx-guest version to v0.3.1 #414

New Contributors:
@KeithMoyer in #392
@vbalain in #405
@aimixsaka in #407
alexmwu added a commit to alexmwu/go-tpm-tools that referenced this pull request Mar 29, 2024
New Features:
[launcher] Add TEE server IPC implementation google#367
[launcher] Enable memory monitoring in CS google#391
Use TDX quote provider to attest and verify google#405
Integrate nonce verification as part of the TDX quote validation procedure. google#395
Add RISC V support google#407
[launcher] Use resizable integrity-fs with in-memory tags google#412

Bug Fixes:
[launcher] Fix launcher exit code google#384
[launcher] Handle exit code checking during deferral evaluation google#392
[cmd] Skip tests that call setGCEAKTemplate google#402
[launcher] Fix teeserver context reset issue & add container signature cache google#397
Set all unused parameters as _ to fix CI lint failure google#411
[launcher] Make customtoken test sleep to mitigate clock skew google#413

Other Changes:
Add eventlog parse logics for memory monitoring google#404
[launcher]: Add memory monitor measurement logics google#408
Update go-tdx-guest version to v0.3.1 google#414

New Contributors:
@KeithMoyer in google#392
@vbalain in google#405
@aimixsaka in google#407
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.

2 participants