Implement some additional hierarchy control and dictionary attack functions for TPM2 by chrisccoulson · Pull Request #109 · google/go-tpm

chrisccoulson · 2019-07-05T15:50:58Z

This adds API's for clearing the TPM, disabling the ability to clear the TPM with owner authorization, changing the hierarchy authorization values and configuring dictionary attack parameters.

This adds an API for calling TPM2_Clear with password authentication

This adds a simple API that wraps the TPM2_DictionaryAttackParameters command.

This adds an API that wraps around the TPM2_HierarchyChangeAuth command so that authorization values for the various hierarchies can be set.

This adds an API that wraps around TPM2_ClearControl to disable the ability to clear the TPM with owner authorization. As re-enabling owner clear requires platform authorization, the API only supports disabling owner clear (ie, TPM2_ClearControl called with disable=YES")

googlebot · 2019-07-05T15:51:08Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here (e.g. I signed it!) and we'll verify it.

What to do if you already signed the CLA

Individual signers

It's possible we don't have your GitHub username or you're using a different email address on your commit. Check your existing CLA data and verify that your email is set on your git commits.

Corporate signers

Your company has a Point of Contact who decides which employees are authorized to participate. Ask your POC to be added to the group of authorized contributors. If you don't know who your Point of Contact is, direct the Google project maintainer to go/cla#troubleshoot (Public version).
The email used to register you as an authorized contributor must be the email used for the Git commit. Check your existing CLA data and verify that your email is set on your git commits.
The email used to register you as an authorized contributor must also be attached to your GitHub account.

ℹ️ Googlers: Go here for more info.

Execute TPM provisioning on install using Chris Coulson's FDE utils code from https://github.com/chrisccoulson/ubuntu-core-fde-utils. This also requires google/go-tpm#109 which has been added to vendored packages. Try to provision the TPM as late as possible to prevent a situation where the installation fails after the TPM is provisioned. Provisioning happens just before sealing the LUKS device keyfile and the lockout authorization value is stored inside the encrypted partition. Signed-off-by: Claudio Matsuoka <claudio.matsuoka@canonical.com>

awly

Thanks for the PR.
Please also sign the CLA per the above comment

awly · 2019-07-09T23:52:22Z

tpm2/constants.go

+	cmdIncrementNVCounter         tpmutil.Command = 0x00000134
+	cmdWriteNV                    tpmutil.Command = 0x00000137
+	cmdPCREvent                   tpmutil.Command = 0x0000013C
+	cmdDictionaryAttackParameters tpmutil.Command = 0x0000013A


Move it one line higher, to keep sorted order

Thanks, I've done that now.

awly · 2019-07-09T23:52:48Z

tpm2/tpm2.go

+	return concat(ha, auth)
+}
+
+func Clear(rw io.ReadWriter, hierarchy tpmutil.Handle, password string) error {


Add doc comments to all exported funcs

awly · 2019-07-09T23:56:37Z

tpm2/tpm2.go

+}
+
+func encodeDisableOwnerClear(password string) ([]byte, error) {
+	lockout, err := tpmutil.Pack(HandleLockout)


Pass the handle as argument

awly · 2019-07-09T23:56:45Z

tpm2/tpm2.go

+	if err != nil {
+		return nil, err
+	}
+	param, err := tpmutil.Pack(true)


Pass this as argument

awly · 2019-07-09T23:57:13Z

tpm2/tpm2.go

+	return concat(lockout, auth, param)
+}
+
+func DisableOwnerClear(rw io.ReadWriter, password string) error {


Change this to ClearControl and take hierarchy and enable/disable as arguments

I've updated this now.

awly · 2019-07-10T00:00:54Z

tpm2/tpm2_test.go

+	}
+	defer FlushContext(rw, rootHandle)
+
+	persistentHandle := tpmutil.Handle(0x817FFFFF)


Add a comment explaining where this value comes from

I just copied this value from other tests in tpm2_test.go.

chrisccoulson · 2019-07-11T10:11:47Z

Thanks for the PR.
Please also sign the CLA per the above comment

I've hit this issue in the past, but I should be part of Canonical's corporate CLA with Google now.

awly · 2019-07-11T17:46:41Z

Thanks for the PR.
Please also sign the CLA per the above comment

I've hit this issue in the past, but I should be part of Canonical's corporate CLA with Google now.

Did you add your Canonical email address to the GitHub account? You can check it under https://github.com/settings/emails. It doesn't need to be primary, but needs to be added there.

awly · 2019-07-11T17:50:49Z

tpm2/tpm2.go

+	return concat(handle, auth, param)
+}
+
+// Control whether the TPM can be cleared with the Clear() API. Calling this with the third parameter set to


Per https://golang.org/doc/effective_go.html#commentary, the comment must start with // ClearControl ...

awly · 2019-07-11T17:51:34Z

tpm2/tpm2.go

+	return concat(lockout, auth, params)
+}
+
+// Change the dictionary attack lockout parameters. The first parameter is the number of authorization failures


Same as above, start comment with // SetDictionaryAttackParameters ...

Thanks for the PR.
Please also sign the CLA per the above comment

I've hit this issue in the past, but I should be part of Canonical's corporate CLA with Google now.

Did you add your Canonical email address to the GitHub account? You can check it under https://github.com/settings/emails. It doesn't need to be primary, but needs to be added there.

My Canonical email address is registered with my github account. Note that I was only added to the CCLA after submitting this PR.

Hmm, I don't see anything wrong.
One last thing to check - did you change your github username since getting added to the CLA? Or perhaps there was a typo in it?

I've emailed our internal team responsible for this CLA github bot, will let you know when we figure out the problem.

josephlr

LGTM from the perspective of properly encoding the auth areas and decoding the responses (fairly easy as none of these commands return anything).

My only concern is that all of these commands use passwords for their authentication. We're trying to avoid having two methods for each command (Foo and FooWithAuth). However, I think this is actually fine, because these commands only use the Hierarchy handles, and those handles can only use password-based authorization.

@awly, do you know if this is actually true? It shouldn't be possible/necessary to do policy auth with things like HandleEndorsement or HandlePlatform.

josephlr · 2019-07-19T07:32:15Z

@chrisccoulson I know we have a CLA on file w/ Cannonical, so if you can resolve the merge conflicts, I can just override the CLA check.

awly · 2019-07-22T18:15:56Z

@awly, do you know if this is actually true? It shouldn't be possible/necessary to do policy auth with things like HandleEndorsement or HandlePlatform.

Sorry, I don't know.
But I think it's a good idea to standardize on taking AuthCommand as argument everywhere instead of password strings.

@chrisccoulson I know we have a CLA on file w/ Cannonical, so if you can resolve the merge conflicts, I can just override the CLA check.

I have a few email threads going about this, with @chrisccoulson and our internal CLA team.
Let's not override the CLA check if we can resolve this.

Grazfather · 2020-03-05T22:39:12Z

Just poking here, 'cause I had hacked in some of the same functionality, and it would be nice to have these officially supported!

EDIT: Looks like it's partially implemented in #163

langbeck · 2020-10-24T12:37:11Z

@awly and @josephlr, do you have any updates on this CLA issue?
I will need this functionality soon and, as @Grazfather said, it would be nice to have these officially supported.

josephlr · 2020-10-26T11:47:10Z

The CLA stuff shouldn't be a problem, as Cannonical has singed a new CLA w/ Google since the last update here. If there's still an issue, I'll manually override the CI if a rebase doesn't get the check to work correctly.

If @chrisccoulson is willing to rebase this on latest master (see #163 which implemented some of this functionality), I'd merge it.

chrisccoulson added 4 commits July 5, 2019 12:48

Add support for clearing TPM2 devices

4806c07

This adds an API for calling TPM2_Clear with password authentication

Add support for setting dictionary attack parameters

67499b4

This adds a simple API that wraps the TPM2_DictionaryAttackParameters command.

Add support for changing TPM2 hierarchy authorizations

c5ced97

This adds an API that wraps around the TPM2_HierarchyChangeAuth command so that authorization values for the various hierarchies can be set.

chrisccoulson added 2 commits July 8, 2019 14:30

Add tests for Clear and HierarchyChangeAuth

e66d769

Remove a spurious Logf

c6c7cb7

cmatsuoka mentioned this pull request Jul 9, 2019

recovery: integrate TPM provisioning canonical/snapd#7085

Merged

awly reviewed Jul 10, 2019

View reviewed changes

awly added the kokoro:run Enable presubmit tests for non-org members label Jul 10, 2019

kokoro-team removed the kokoro:run Enable presubmit tests for non-org members label Jul 10, 2019

chrisccoulson added 2 commits July 11, 2019 11:00

Address review comments

a3814f4

Add a couple more doc comments

9f5e4eb

awly reviewed Jul 11, 2019

View reviewed changes

Adjust doc comments

6cd31e7

josephlr approved these changes Jul 19, 2019

View reviewed changes

Merge from master, resolving conflicts

cf23a6c

Comments

Conversation

chrisccoulson commented Jul 5, 2019

Uh oh!

googlebot commented Jul 5, 2019

What to do if you already signed the CLA

Individual signers

Corporate signers

Uh oh!

awly left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chrisccoulson commented Jul 11, 2019

Uh oh!

awly commented Jul 11, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josephlr left a comment

Choose a reason for hiding this comment

Uh oh!

josephlr commented Jul 19, 2019

Uh oh!

awly commented Jul 22, 2019

Uh oh!

Grazfather commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

langbeck commented Oct 24, 2020

Uh oh!

josephlr commented Oct 26, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Grazfather commented Mar 5, 2020 •

edited

Loading