wolfTPM pubkey storage with policy based access restriction #296
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
this update uses the tpm to retreive the public key used to validate the image that will boot and restricts access to that key by tpm policy. when the image is updated it's signature is used to extend the PCR and when the image is loaded it's signature must match what was sealed in order to get the public key from the tpm. enabling this option is done by setting WOLFBOOT_TPM_KEYSTORE in .config
jpbland1
force-pushed
the
tpm-root-trust
branch
from
8dc3887
to
3fbc99d
Compare
dgarske
requested changes
Apr 11, 2023
src/image.c
Outdated
|
|
||
| #ifdef WOLFBOOT_TPM_KEYSTORE | ||
| static WOLFTPM2_SESSION wolftpm_session; | ||
| static uint32_t wolftpmPcrArray[1] = {16}; |
There was a problem hiding this comment.
Should this be an array of uint8_t?
src/image.c
Outdated
| #ifdef WOLFBOOT_TPM_KEYSTORE | ||
| static WOLFTPM2_SESSION wolftpm_session; | ||
| static uint32_t wolftpmPcrArray[1] = {16}; | ||
| static const uint32_t wolftpmKeystoreIndex = 0x01800200; |
There was a problem hiding this comment.
Can you make all of these overridable build-time macros?
src/image.c
Outdated
| /* start a policy session with parameter encryption */ | ||
| rc = wolfTPM2_StartSession(&wolftpm_dev, &wolftpm_session, NULL, NULL, | ||
| TPM_SE_POLICY, TPM_ALG_CFB); | ||
| if (rc != 0) |
src/image.c
Outdated
| /* set the auth session for the device */ | ||
| rc = wolfTPM2_SetAuthSession(&wolftpm_dev, 0, &wolftpm_session, | ||
| (TPMA_SESSION_decrypt | TPMA_SESSION_encrypt | TPMA_SESSION_continueSession)); | ||
| if (rc != 0) |
src/image.c
Outdated
| /* clear out the policy digest */ | ||
| ret = wolfTPM2_PolicyRestart(wolftpm_session.handle.hndl); | ||
| if (ret != TPM_RC_SUCCESS) | ||
| return -1; |
There was a problem hiding this comment.
Maybe just convert the TPM codes to negative?
There was a problem hiding this comment.
Convert TPM codes to negative?
src/image.c
Outdated
| if (ret != TPM_RC_SUCCESS) | ||
| return -1; | ||
|
|
||
| /* unload the intermidate key */ |
jpbland1
force-pushed
the
tpm-root-trust
branch
from
de2420a
to
7dd97be
Compare
dgarske
requested changes
Apr 12, 2023
src/image.c
Outdated
|
|
||
| #ifdef WOLFBOOT_TPM_KEYSTORE | ||
| static WOLFTPM2_SESSION wolftpm_session; | ||
| static uint32_t wolftpmPcrArray[1] = {16}; |
src/image.c
Outdated
| /* clear out the policy digest */ | ||
| ret = wolfTPM2_PolicyRestart(wolftpm_session.handle.hndl); | ||
| if (ret != TPM_RC_SUCCESS) | ||
| return -1; |
There was a problem hiding this comment.
Convert TPM codes to negative?
dgarske
requested changes
Apr 13, 2023
src/image.c
Outdated
|
|
||
| #ifdef WOLFBOOT_TPM_KEYSTORE | ||
| static WOLFTPM2_SESSION wolftpm_session; | ||
| static uint8_t wolftpmPcrArray[1] = {16}; |
There was a problem hiding this comment.
This PR index should be a build-time overridable value.
src/image.c
Outdated
| ret = wolfTPM2_ExtendPCR(&wolftpm_dev, wolftpmPcrArray[0], TPM_ALG_SHA256, | ||
| imageSignature, imageSignatureSz); | ||
| if (ret != TPM_RC_SUCCESS) | ||
| return -1 * ret; |
dgarske
previously approved these changes
Apr 14, 2023
arguments and reset out the PCR values so they're not impacted by previous calls
|
@jpbland1 please add some documentation in docs/Target.md. Create a new section e.g. under the STM32F407 target (based on a new config |
jpbland1
force-pushed
the
tpm-root-trust
branch
from
00e682b
to
371ff3b
Compare
danielinux
previously approved these changes
Apr 19, 2023
make the config/examples/stm32f4-tpm-keystore.config config use ecc256
move pcr reset and extend outside of session the tpm uses policy checking for modifying PCR's so we need to reset and extend the PCR's with the image hash before the session begins, currently tested unseal, having trouble getting the simulator to run update in order to test reseal
dgarske
approved these changes
May 4, 2023
|
Merging this now. Only boot with unseal tested, not an update scenario. Also needs instructions on how to preseal, since it requires doing manual signing and the extra key. That will come in future PR. |
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.
this update uses the tpm to retreive the public key used to validate the image that will boot and restricts access to that key by tpm policy. when the image is updated it's signature is used to extend the PCR and when the image is loaded it's signature must match what was sealed in order to get the public key from the tpm. enabling this option is done by setting WOLFBOOT_TPM_KEYSTORE in .config