Remove ProvisionStatus and refactor provision API#104
Merged
chrisccoulson merged 2 commits intocanonical:masterfrom Sep 24, 2020
Merged
Remove ProvisionStatus and refactor provision API#104chrisccoulson merged 2 commits intocanonical:masterfrom
chrisccoulson merged 2 commits intocanonical:masterfrom
Conversation
ProvisionStatus is a problematic API - AttrValidSRK and AttrValidEK indicate that objects with the expected public areas exist at the expected handles, but can't indicate whether those objects were created with the correct templates and are actually valid objects. These attributes can't be used to make a decision about whether ProvisionTPM should be called or not, so just remove the ProvisionStatus API entirely. Rename ProvisionTPM to EnsureProvisioned (and make it a method of TPMConnection), and introduce a new error (ErrTPMProvisioningRequiresLockout) which is returned from EnsureProvisioned if it is called with ProvisionModeWithoutLockout but use of the lockout hierarchy is required to fully provision the TPM. This new error is an indication that EnsureProvisioned needs to be called again with a different mode in order to fully provision it.
pedronis
approved these changes
Sep 23, 2020
Collaborator
pedronis
left a comment
pedronis
left a comment
There was a problem hiding this comment.
thanks, couple small comments
| t.Fatalf("EnsureProvisioned failed: %v", err) | ||
| } | ||
|
|
||
| srk, err = tpm.CreateResourceContextFromTPM(tcg.SRKHandle) |
Collaborator
There was a problem hiding this comment.
this is an additional test now?
Collaborator
Author
There was a problem hiding this comment.
It is - it verifies that the TPM hasn't been cleared by making sure that the new primary key is identical to the original one.
| // has a null authorization value, then this will allow us to unseal the key without requiring any type of manual recovery. If the | ||
| // storage hierarchy has a non-null authorization value, ProvionTPM will fail. If the TPM owner has changed, ProvisionTPM might | ||
| // succeed, but UnsealFromTPM will fail with InvalidKeyFileError when retried. | ||
| if pErr := ProvisionTPM(tpm, ProvisionModeWithoutLockout, nil); pErr == nil { |
Collaborator
There was a problem hiding this comment.
the comment should maybe be expanded to cover the new || condition
Collaborator
Author
There was a problem hiding this comment.
I've actually changed this in #112 to just ignore the error instead, as there's not really any harm in attempting to unseal again even if provisioning fails with an error.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
ProvisionStatus is a problematic API - AttrValidSRK and AttrValidEK
indicate that objects with the expected public areas exist at the expected
handles, but can't indicate whether those objects were created with the
correct templates and are actually valid objects. These attributes can't
be used to make a decision about whether ProvisionTPM should be called or
not, so just remove the ProvisionStatus API entirely.
Rename ProvisionTPM to EnsureProvisioned (and make it a method of
TPMConnection), and introduce a new error (ErrTPMProvisioningRequiresLockout)
which is returned from EnsureProvisioned if it is called with
ProvisionModeWithoutLockout but use of the lockout hierarchy is required
to fully provision the TPM. This new error is an indication that
EnsureProvisioned needs to be called again with a different mode in order
to fully provision it.