Remove ProvisionStatus and refactor provision API #104
Conversation
This file contains 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
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
| t.Fatalf("EnsureProvisioned failed: %v", err) | ||
| } | ||
|
|
||
| srk, err = tpm.CreateResourceContextFromTPM(tcg.SRKHandle) |
There was a problem hiding this comment.
this is an additional test now?
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.
| @@ -321,7 +321,7 @@ func unsealKeyFromTPM(tpm *TPMConnection, k *SealedKeyObject, pin string) ([]byt | |||
| // 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 { | |||
There was a problem hiding this comment.
the comment should maybe be expanded to cover the new || condition
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.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.