Utility for flushing active TPM handles. #8

sambdavidson · 2019-01-18T02:32:22Z

Reads all active handles through a call to the TPM rw:

... := tpm2.GetCapability(rw, tpm2.CapabilityHandles, ^uint32(0), 0x80000000)

Then flushes those handles with tpm2.FlushContext(...).

I am not 100% certain on the magic const 0x80000000 used in GetCapability but it appears to work correctly. I found it here: https://github.com/google/go-tpm/blob/master/tpm2/tpm2_test.go#L135

…ll active handles and flush them from the TPM

awly · 2019-01-18T21:47:31Z

Move the files to a separate directory under github.com/google/go-tpm-tools/cmd and make it package main. This is mostly useful as a standalone binary.

awly · 2019-01-18T21:51:04Z

0x80000000 is property parameter of the TPM2_GetCapability call.
For handles it defines which type of handle to return. The type of the handle (using TPM_HT_... constants) is placed in most significant octet of the property.
In this case it's TPM_HT_TRANSIENT.

The code should make it more obvious instead of using an opaque constant.

josephlr

Thanks @samdamana for the PR!

Overall, this is good functionality that we want to add to go-tpm-tools. However, we want to make sure the facilities are sufficiently clean and general. See my comments below:

tpm2tools/flush_active_handles.go

tpm2tools/flush_active_handles_test.go

tpm2tools/flush_active_handles.go

josephlr · 2019-01-18T23:02:04Z

tpm2tools/flush_active_handles.go

+// FlushActiveHandles calls FlushContext() on all active handles within the 
+// TPM at io.ReadWriter rw. Returns nil if successful or TPM has no active
+// handles.
+func FlushActiveHandles(rw io.ReadWriter) error {


As @awly said, flushing all transient handles is best put in as a standalone binary. The function to list all handles of a specific type; however, should be a library function.

* Implement LoadExternal Add integration test, without loading private key * Implement Certify This does not work yet! Committing to keep the changes, for some other work. * Refactor encodePasswordAuthArea Use correct size of inputs, support multiple password sessions. * Make Certify work Fix CreatePrimary with signing keys Fix LoadExternal with private key Fix integration test for certifying a TPM key Add integration test for certifying externally-loaded key * Unify TPM2B_PUBLIC parsing and check Certify signatures Also add handling for default RSA key exponent returned from TPM. * Add doc comment to Certify * Address review comments - Fix grammar - Add comment about big.Int and TPM endianness - Add a const for empty passwords in tests - Increase RSA key size - Extract anonymous struct in Certify into named one * Add README with instructions on running tests * Merge an if statement

…es main under cmd/

…ogle/... Fixed up some comments.

sambdavidson · 2019-01-28T23:03:08Z

Ready for another look.

cmd/flush_handles.go

awly · 2019-01-28T23:19:52Z

cmd/flush_handles.go

+	for _, handleType := range handleTypes {
+		err = flushHandlesOfType(rw, handleType)
+		if err != nil {
+			log.Fatalf("%v", err)


log.Fatal will prevent any deferred calls. Move OpenTPM + this loop to another func that returns an error. That way deferred rw.Close can run before log.Fatal

cmd/flush_handles.go

cmd/flush_handles_test.go

awly · 2019-01-28T23:26:49Z

tpm2tools/handles.go

+	}
+	handles := make([]tpmutil.Handle, len(vals))
+	for i, v := range vals {
+		handles[i] = v.(tpmutil.Handle)


Use a checked assertion here

tpm2tools/handles_test.go

tpm2tools/tpm2toolstest/load_random_external_key.go

sambdavidson

Updated the code to address your feedback.

cmd/flush_handles.go

sambdavidson · 2019-01-28T23:42:06Z

cmd/flush_handles.go

+	for _, handleType := range handleTypes {
+		err = flushHandlesOfType(rw, handleType)
+		if err != nil {
+			log.Fatalf("%v", err)


cmd/flush_handles.go

cmd/flush_handles_test.go

sambdavidson · 2019-01-29T00:19:17Z

tpm2tools/handles.go

+	}
+	handles := make([]tpmutil.Handle, len(vals))
+	for i, v := range vals {
+		handles[i] = v.(tpmutil.Handle)


tpm2tools/handles_test.go

tpm2tools/tpm2toolstest/load_random_external_key.go

sambdavidson

Your suggestions have been implemented.

cmd/flush_handles/flush_handles.go

awly · 2019-01-29T18:22:06Z

cmd/flush_handles_test.go

+		{false, false, false, true, []tpm2.HandleType{tpm2.HandleTypeTransient, tpm2.HandleTypeLoadedSession, tpm2.HandleTypeSavedSession}},
+	} {
+		// Overwrite flag pointer values.
+		flushTransient = &test.flushTransientFlag


TL;DR: avoid global state (including flags) whenever possible.

Generally speaking, tests shouldn't know about flags at all (unless you test flag parsing).
Flags are func main-level concern and all other code should get configuration passed explicitly through func arguments or struct fields.
Looking outside of their context into the global vars (which get set through flags at runtime) makes testing harder: you can't parallelize tests that read flags due to memory safety, you must remember to clean up modified values to make tests order-independent.

However, this program is not production-critical so we don't need to be so strict about code structure or testing.

awly · 2019-01-29T18:22:51Z

cmd/flush_handles/flush_handles_test.go

+		t.Fatal(err)
+	}
+	if len(h) != 0 {
+		t.Fatalf("Simulator should be empty of transient handles; got: %d", len(h))


Put "got" before "want" in message

Fixed I think?

tpm2tools/handles.go

sambdavidson

@awly 's suggestions have been implemented.

cmd/flush_handles/flush_handles.go

sambdavidson · 2019-01-29T19:30:24Z

cmd/flush_handles/flush_handles_test.go

+		t.Fatal(err)
+	}
+	if len(h) != 0 {
+		t.Fatalf("Simulator should be empty of transient handles; got: %d", len(h))


Fixed I think?

tpm2tools/handles.go

sambdavidson

Most of @josephlr 's suggestions and changes have been implemented. Sorry this is my first Github PR. I am just learning how to do these sub-review things.

sambdavidson · 2019-01-29T19:36:19Z

tpm2tools/flush_active_handles.go

+// FlushActiveHandles calls FlushContext() on all active handles within the 
+// TPM at io.ReadWriter rw. Returns nil if successful or TPM has no active
+// handles.
+func FlushActiveHandles(rw io.ReadWriter) error {


tpm2tools/flush_active_handles.go

tpm2tools/flush_active_handles_test.go

sambdavidson · 2019-01-29T19:39:16Z

tpm2tools/flush_active_handles_test.go

+    }
+    defer simulator.Close()
+
+    // Loads then flushes 1, 2, ...maxHandles handles.


It only loads in transient handles. I kind of like looping load then flush. It helped me identify bugs in the past where I returned incorrect numbers of handles.

tpm2tools/flush_active_handles_test.go

awly · 2019-01-29T19:48:57Z

LGTM, leaving this up to @josephlr

mikedanese · 2019-01-29T21:04:56Z

tpm2tools/tpm2toolstest/load_random_external_key.go

+	public := tpm2.Public{
+		Type:       tpm2.AlgRSA,
+		NameAlg:    tpm2.AlgSHA1,
+		Attributes: tpm2.FlagSign | tpm2.FlagSensitiveDataOrigin | tpm2.FlagUserWithAuth,


SensitiveDataOrigin doesn't make sense in this context. I'm disappointed if this is not an error.

Its not haha. But removed.

josephlr

@samdamana This code looks great, the only remaining things are organisational, mainly just to use the internal package.

tpm2tools/tpm2toolstest/load_random_external_key.go

tpm2tools/handles_test.go

cmd/flush_handles/flush_handles_test.go

josephlr

LGTM, thanks again!!

Implement autotools build for TPM and Platform code

Created flush_active_handles utility library. The library will read a…

2c39f19

…ll active handles and flush them from the TPM

josephlr requested changes Jan 18, 2019

View reviewed changes

josephlr reviewed Jan 18, 2019

View reviewed changes

sambdavidson added 4 commits January 25, 2019 14:39

WIP. Changed handle library to generic tool getter. Added flush handl…

49ddf73

…es main under cmd/

Move binary out of /tpm2tools subfolder.

ed3ffe8

Completed flush_handles binary. Added test library. Added tests.

03b8bfb

Changed all references from github.com/samdamana/... to github.com/go…

d139a71

…ogle/... Fixed up some comments.

awly reviewed Jan 28, 2019

View reviewed changes

sambdavidson added 2 commits January 28, 2019 15:37

Move flush_handles.go and its test to their own directory.

f82d27e

Fixed various errors and code styles. Addressing PR feedback.

01d868b

sambdavidson commented Jan 29, 2019

View reviewed changes

awly reviewed Jan 29, 2019

View reviewed changes

sambdavidson added 2 commits January 29, 2019 11:31

Addressed additional pull request feedback. Code cleanup.

61dbdde

More comment fixes.

ee4dcba

sambdavidson commented Jan 29, 2019

View reviewed changes

mikedanese reviewed Jan 29, 2019

View reviewed changes

Removed tpm2.FlagSensitiveDataOrigin from LoadRandomExternalKey.

5425b9e

josephlr requested changes Jan 30, 2019

View reviewed changes

sambdavidson added 3 commits January 29, 2019 16:14

Merge remote-tracking branch 'upstream/master'

b40802d

Moved LoadRandomExternalKey functionality into the internal package.

5a7670d

Changed creating simulator to using internal.GetTPM()

6edc114

josephlr approved these changes Jan 30, 2019

View reviewed changes

josephlr merged commit f38bf7b into google:master Jan 30, 2019

josephlr pushed a commit to josephlr/go-tpm-tools that referenced this pull request Jul 3, 2019

Merge pull request google#8 from flihp/linux-build-libtpm-libplatform

0afb667

Implement autotools build for TPM and Platform code

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Utility for flushing active TPM handles. #8

Utility for flushing active TPM handles. #8

sambdavidson commented Jan 18, 2019

awly commented Jan 18, 2019

awly commented Jan 18, 2019

josephlr left a comment

josephlr Jan 18, 2019

sambdavidson Jan 29, 2019

sambdavidson commented Jan 28, 2019

awly Jan 28, 2019

sambdavidson Jan 28, 2019

awly Jan 28, 2019

sambdavidson Jan 29, 2019

sambdavidson left a comment

sambdavidson Jan 28, 2019

sambdavidson Jan 29, 2019

sambdavidson left a comment

awly Jan 29, 2019

awly Jan 29, 2019

sambdavidson Jan 29, 2019

sambdavidson left a comment

sambdavidson Jan 29, 2019

sambdavidson left a comment

sambdavidson Jan 29, 2019

sambdavidson Jan 29, 2019

awly commented Jan 29, 2019

mikedanese Jan 29, 2019

sambdavidson Jan 29, 2019

josephlr left a comment

josephlr left a comment

Utility for flushing active TPM handles. #8

Utility for flushing active TPM handles. #8

Conversation

sambdavidson commented Jan 18, 2019

awly commented Jan 18, 2019

awly commented Jan 18, 2019

josephlr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sambdavidson commented Jan 28, 2019

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

sambdavidson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sambdavidson 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

sambdavidson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sambdavidson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awly commented Jan 29, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

josephlr left a comment

Choose a reason for hiding this comment

josephlr left a comment

Choose a reason for hiding this comment