wolfTPM Support for sealing/unsealing based on a PCR that is signed externally #294
Conversation
…xternally. Use an external key to sign a PCR digest. Allows a new signed policy to be sent with updates to continue allowing a sealed secret to be unsealed when PCR's change. This resolves the issue with PCR brittleness.
examples/boot/secret_seal.c
Outdated
|
|
||
| /* Start an authenticated session (salted / unbound) */ | ||
| rc = wolfTPM2_StartSession(&dev, &tpmSession, &storage, NULL, | ||
| TPM_SE_HMAC, paramEncAlg); |
There was a problem hiding this comment.
shouldn't this be TPM_SE_POLICY? I thought you had to use a policy session or a trial session to do make policies
There was a problem hiding this comment.
Creating a sealed object does not require a policy session. We have a signed PCR policy digest we write when creating the keyed hashed (seal) object. No policy session is needed.
| printf("Session Handle 0x%x\n", (word32)tpmSession.handle.hndl); | ||
| printf("Parameter Encryption: %s\n", TPM2_GetAlgName(paramEncAlg)); | ||
|
|
||
| rc = wolfTPM2_SetAuthSession(&dev, 1, &tpmSession, |
There was a problem hiding this comment.
use auth index 0, otherwise I think it uses the default session handle
There was a problem hiding this comment.
It will not enable parameter encryption if index 0 is used.
|
|
||
| /* Load Key Public Info */ | ||
| #if !defined(NO_FILESYSTEM) | ||
| static int LoadAuthKeyInfo(WOLFTPM2_DEV* dev, WOLFTPM2_KEY* authKey, |
There was a problem hiding this comment.
not DRY, maybe this should go in the test common code, along with the hex to byte we repeat everywhere
There was a problem hiding this comment.
All the code for this is already in wolfTPM2_ImportPublicKeyBuffer. This is just to load a file, which I don't want in the library proper.
| printf("Parameter Encryption: %s\n", TPM2_GetAlgName(paramEncAlg)); | ||
|
|
||
| /* enable parameter encryption for unseal */ | ||
| rc = wolfTPM2_SetAuthSession(&dev, 1, &tpmSession, |
examples/pcr/policy_sign.c
Outdated
| return rc; | ||
| } | ||
|
|
||
| if (XSTRNCMP(keyFile, ".pem", XSTRLEN(".pem")) == 0) { |
There was a problem hiding this comment.
this doesn't seem right, wouldn't this just check the first 4 characters of the filename to match .pem?
| if (rc == 0) { | ||
| if (authKey->pub.publicArea.type == TPM_ALG_ECC) { | ||
| sigAlg = authKey->pub.publicArea.parameters.eccDetail.scheme.scheme; | ||
| rc = TPM2_GetHashType(pcrAlg); |
There was a problem hiding this comment.
why set rc if you're not going to check it before overwriting?
There was a problem hiding this comment.
just realized you cast it to hashType, whoops
| &policyDigestSz); | ||
| } | ||
| inSz = *digestSz; /* capture input digest size (for policyDigestOld) */ | ||
| rc = TPM2_GetHashType(pcrAlg); |
There was a problem hiding this comment.
why set rc if you're going to overwrite it before checking?
There was a problem hiding this comment.
Haven't found a better way to solve this. Same pattern is used elsewhere. Resolves a compiler warning about int to enum.
There was a problem hiding this comment.
sorry I didn't see hashType was being set to it, which is fine. I thought it was being overwritten by 7163
src/tpm2_wrap.c
Outdated
| policyDigestSz, sizeof(TPM_ALG_ID)); | ||
| /* policyRef */ | ||
| if (rc == 0 && policyRefSz > 0) { | ||
| rc = wc_HashUpdate(&hash_ctx, hashType, digest, inSz); |
There was a problem hiding this comment.
should hash policyRef, not digest
| policyAuthNVIn->authHandle = parent->hndl; | ||
|
|
||
| rc = TPM2_PolicyAuthorizeNV(policyAuthNVIn); | ||
| rc = wc_HashFinal(&hash_ctx, hashType, digest); |
There was a problem hiding this comment.
this will overwrite the contents of digest, might not be wise if the caller needs the buffer's contents
There was a problem hiding this comment.
I designed it this way. The "digest" are in both input and output (as documented). This is because some policies take in an "old" digest to get a new one (chaining them).
| policyAuthNVIn->authHandle = parent->hndl; | ||
|
|
||
| rc = TPM2_PolicyAuthorizeNV(policyAuthNVIn); | ||
| rc = wc_HashFinal(&hash_ctx, hashType, digest); |
There was a problem hiding this comment.
this also overwrites digest, this might be okay since it's the new policyDigest anyway right?
dgarske
force-pushed
the
secret_seal
branch
7 times, most recently
from
b9858a7
to
c2191c5
Compare
… avoid parallel builds.
Use an external key to sign a PCR digest. Allows a new signed policy to be sent with updates to continue allowing a sealed secret to be unsealed when PCR's change. This resolves the issue with PCR brittleness.
Removed experimental policy examples (will put back as draft PR).