TPM simulator #178

hslatman · 2023-02-17T15:56:06Z

The TPM simulator from https://github.com/google/go-tpm-tools is used to test TPM interactions.

maraino · 2023-02-17T20:43:27Z

@hslatman Do we really need to vendor?

hslatman · 2023-02-17T22:53:35Z

Strictly speaking, I don't think it's required. I probably added it out of habit. I don't think it's bad necessary; it's an extra way of knowing what code your build is using, apart from the hashes and the Go module proxy. Given that crypto is more of a library, maybe I should remove it again.

There's one interesting thing about why it's useful for this specific PR though: the https://github.com/google/go-tpm-tools repository includes the C source code for the TPM simulator. That code isn't go get'd automatically; Go ignores it. By having github.com/google/go-tpm-tools in the vendor dir, and after manual copying of the C source code into that dir, the simulator can be used successfully. Otherwise it'll be missing the required header files. The problem is explained here and here.

Do you have a suggestion on how to do this without having the vendor dir? Including it as a submodule and partly rewriting the search/import paths might work? Only downloading the TPM source dir in GH actions might also be an option, but then it's not as easy to run locally.

maraino · 2023-02-17T23:48:31Z

Do you have a suggestion on how to do this without having the vendor dir? Including it as a submodule and partly rewriting the search/import paths might work? Only downloading the TPM source dir in GH actions might also be an option, but then it's not as easy to run locally.

Perhaps we can copy the full simulator folder, change the necessary imports, and keep just that. We might want to add specific tags to enable the simulator too.

hslatman · 2023-02-18T08:23:03Z

Sounds OK. Will try that. Simulator is only used in tests that have the cgo tag. Testing with cgo disabled skips those.

…

________________________________ From: Mariano Cano ***@***.***> Sent: Saturday, February 18, 2023 12:48:42 AM To: smallstep/crypto ***@***.***> Cc: Herman Slatman ***@***.***>; Mention ***@***.***> Subject: Re: [smallstep/crypto] TPM simulator (PR #178) Do you have a suggestion on how to do this without having the vendor dir? Including it as a submodule and partly rewriting the search/import paths might work? Only downloading the TPM source dir in GH actions might also be an option, but then it's not as easy to run locally. Perhaps we can copy the full simulator folder, change the necessary imports, and keep just that. We might want to add specific tags to enable the simulator too. — Reply to this email directly, view it on GitHub<#178 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAJJZRB4H75XL34RV5XEV53WYAE5VANCNFSM6AAAAAAU7S2G2A>. You are receiving this because you were mentioned.Message ID: ***@***.***>

The `simulator` from `go-tpm-tools` is included as a copy to make it easier to work with the TPM simulator written in C.

Opening and closing of the `TPM` has been refactored such that there's now only one location in the code where a selection is made between the `go-tpm` and the `go-attestation` TPMs. Only one of them will be instantiated and used until the `TPM` is closed (and reopened) again. Both types support the simulator to be injected. The simulator is currently only used for testing. The simulator doesn't cover all code paths (yet). This can probably be improved by mocking certain calls out, for example to return expected errors.

Using the TPM simulator now requires tests to be ran with: ```console CGO_ENALBED=1 go test ./tpm -v -tags tpmsimulator ``` Both the `CGO_ENABLED` env var and the `tpmsimulator` build tag are required. Tests that use the TPM simulator will not be included in the test binary, otherwise.

hslatman · 2023-02-21T20:42:27Z

Added tpmsimulator build tag. Not providing it skips the simulator tests. It also requires CGO_ENABLED=1 if specified.

maraino

Generally ok, although there's a bunch of code that is not part of this review.

tpm/ek.go

maraino · 2023-02-21T21:02:03Z

tpm/ek.go

-		Type: fmt.Sprintf("%T", ek.Public), // TODO: proper description string; on EK struct
+	o := struct {
+		Type string `json:"type"`
+		PEM  string `json:"pem,omitempty"`


Would the DER here make it easier to manage DER []byte and then DER: ek.certificate.Raw

I'll see if it makes sense to provide DER by default and keep providing the utilities for the PEM format. I think in step-tpm-plugin, the PEM output is more useful, but for direct outputs, having the raw bytes may be easier.

maraino · 2023-02-21T22:27:51Z

tpm/random.go

+// GenerateRandom returns `number` of random bytes generated by the TPM.
+func (t *TPM) GenerateRandom(ctx context.Context, number uint16) ([]byte, error) {


According to tpm2_getrandom.1.md, the size is bounded to the maximum size of a hash algorithm output in bytes. We should probably add something similar to the docs.

You might also change the argument name to size.

maraino · 2023-02-21T22:34:16Z

tpm/simulator/internal/internal/doc.go

+// against the system OpenSSL library. Without CGO, this package just provides
+// stubs which always return failure. This allows the simulator package to be
+// built when cross compiling go-tpm-tools (which is incompatible with CGO).
+package internal


internal/internal I guess it won't be difficult to move this one level up.

Yea, I agree it's not super nice, but this is the easiest way to just copy/paste the entire directory with minimal changes to the original code and no changes to import paths. It's all internal, and not meant to be used outside of the tests for this package (at the moment), so I think it's OK to leave it as is.

maraino · 2023-02-21T22:56:25Z

tpm/simulator/internal/simulator.go

+// Package simulator provides a go interface to the Microsoft TPM2 simulator.
+//
+//nolint:gosec // copied package from github.com/google/go-tpm-tools/simulator; intentionally using math.rand for seed
+package internal


I think we can expose this in the simulator package so we don't have that internal/internal

I guess it is easier to copy the full directory to an internal package and deal with it. Up to you.

maraino · 2023-02-21T23:07:00Z

tpm/tpm.go

+	if t.enableSimulator {
+		sim := simulator.New()
+		if err := sim.Open(ctx); err != nil {
+			return fmt.Errorf("failed opening TPM simulator: %w", err)
+		}
+		at, err := attest.OpenTPM(&attest.OpenConfig{
+			TPMVersion:     attest.TPMVersion20,
+			CommandChannel: sim,
+		})
+		if err != nil {
+			return fmt.Errorf("failed opening attest.TPM: %w", err)
+		}
+		t.attestTPM = at
+		t.rwc = sim
+	} else {
+		if isGoTPMCall(ctx) {
+			rwc, err := open.TPM(t.deviceName)
+			if err != nil {
+				return fmt.Errorf("failed opening TPM: %w", err)
+			}
+			t.rwc = rwc
+		} else {
+			at, err := attest.OpenTPM(t.attestConfig)
+			if err != nil {
+				return fmt.Errorf("failed opening TPM: %w", err)
+			}
+			t.attestTPM = at
+		}
+	}


Ok, optionally an open method in go:build tpmsimulator and another in go:build !tpmsimulator.

Ideally you should be able to enable the simulator from a third-party package too just using the tags. This can also be handled in the open package too.

At the moment I do not intend the C TPM simulator in this repo to be used externally. In fact, people trying to use it would have the same issue go get-ing the dependency as we had in the first place. Or are you thinking of different use cases?

I might refactor the open/closing of the simulator. Not entirely happy with this approach yet, and I'll be looking at some non-happy path cases this week that might lead to a better implementation.

This code works, and I can see the .c and .h files in my ~/go/pkg/mod/.... I don't know if more complex stuff won't, but if this library provides useful stuff that we can use for testing or while developing on a macOS, I would say we should allow it.

package main import ( "fmt" "log" "github.com/google/go-tpm-tools/simulator" "github.com/google/go-tpm/tpm2" ) func main() { sim, err := simulator.Get() if err != nil { log.Fatal(err) } defer sim.Close() b, err := tpm2.GetRandom(sim, 32) if err != nil { log.Fatal(err) } fmt.Printf("%x\n", b) }

I moved back to where I started. Just removed the copy for now and rely on the import directly, but the tags are still present. Third party packages should now be able to do simulator.New() (from this package). If they vendor their dependencies however, it won't work.

github-actions bot added the needs triage label Feb 17, 2023

hslatman added 2 commits February 20, 2023 20:46

Return *EK to be consistent with AK and Key types

2a255b5

Include github.com/google/go-tpm-tools/simulator as dependency

9271ec6

The `simulator` from `go-tpm-tools` is included as a copy to make it easier to work with the TPM simulator written in C.

hslatman force-pushed the herman/tpm-simulator branch from 5c25cbd to 54526ee Compare February 20, 2023 20:10

hslatman force-pushed the herman/tpm-simulator branch from 54526ee to da2018f Compare February 20, 2023 20:21

hslatman added 3 commits February 20, 2023 21:34

Fix linting issues

4c8915f

Skip the SKAE extension test because it's not complete yet

c87e03e

hslatman requested a review from maraino February 21, 2023 20:48

hslatman marked this pull request as ready for review February 21, 2023 22:59

maraino reviewed Feb 21, 2023

View reviewed changes

Remove vendored copy of github.com/google/go-tpm-tools/simulator

bf2d776

hslatman merged commit 0003205 into herman/tpm Feb 22, 2023

hslatman deleted the herman/tpm-simulator branch February 22, 2023 08:46

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TPM simulator #178

TPM simulator #178

hslatman commented Feb 17, 2023

maraino commented Feb 17, 2023

hslatman commented Feb 17, 2023 •

edited

Loading

maraino commented Feb 17, 2023

hslatman commented Feb 18, 2023 via email

hslatman commented Feb 21, 2023

maraino left a comment

maraino Feb 21, 2023

hslatman Feb 21, 2023

maraino Feb 21, 2023

maraino Feb 21, 2023

hslatman Feb 21, 2023

maraino Feb 21, 2023

maraino Feb 21, 2023

maraino Feb 21, 2023

maraino Feb 21, 2023

hslatman Feb 21, 2023 •

edited

Loading

maraino Feb 22, 2023

hslatman Feb 22, 2023 •

edited

Loading

		// GenerateRandom returns `number` of random bytes generated by the TPM.
		func (t *TPM) GenerateRandom(ctx context.Context, number uint16) ([]byte, error) {

TPM simulator #178

TPM simulator #178

Conversation

hslatman commented Feb 17, 2023

maraino commented Feb 17, 2023

hslatman commented Feb 17, 2023 • edited Loading

maraino commented Feb 17, 2023

hslatman commented Feb 18, 2023 via email

hslatman commented Feb 21, 2023

maraino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hslatman Feb 21, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hslatman Feb 22, 2023 • edited Loading

Choose a reason for hiding this comment

hslatman commented Feb 17, 2023 •

edited

Loading

hslatman Feb 21, 2023 •

edited

Loading

hslatman Feb 22, 2023 •

edited

Loading