Improve get_ek_certs to handle indices#455
Conversation
dgarske
force-pushed
the
ek_certs
branch
2 times, most recently
from
bc8c246 to
5b62ffd
Compare
There was a problem hiding this comment.
Pull request overview
This pull request enhances the get_ek_certs example to better handle various types of TPM NV indices beyond just EK certificate indices. The PR expands the validation range for NV indices to cover the entire TCG NV space (0x01C00000 - 0x01C07FFF) and adds new EK Policy index definitions for PolicyAuthorizeNV operations.
Changes:
- Added EK Policy index definitions for SHA256, SHA384, SHA512, and SM3_256 hash algorithms (0x7F01-0x7F04)
- Expanded NV index validation in
wolfTPM2_GetKeyTemplate_EKIndexto accept the full TCG NV space range instead of just the EK certificate range - Enhanced error handling in
get_ek_certsto identify and display information about non-certificate NV indices, including their type, size, attributes, and content
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| wolftpm/tpm2.h | Added four new EK Policy index constants for different hash algorithms (SHA256, SHA384, SHA512, SM3_256) in the TCG NV space |
| src/tpm2_wrap.c | Expanded the validation range in wolfTPM2_GetKeyTemplate_EKIndex from 0x1FF to 0x7FFF to accept all TCG NV space indices, with updated comment explaining the full range |
| examples/endorsement/get_ek_certs.c | Added comprehensive error handling and diagnostic output when encountering non-EK certificate indices, including a debug-mode function to display NV attributes and logic to read and display NV index contents |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
examples/endorsement/get_ek_certs.c
Outdated
| else if (offset == 0x7F03) indexType = "EK Policy Index (SHA512)"; | ||
| else if (offset == 0x7F04) indexType = "EK Policy Index (SM3_256)"; | ||
| } | ||
| else if (nvIndex > TPM_20_TCG_NV_SPACE + 0x7FFF) { |
There was a problem hiding this comment.
The range expansion from 0x1FF to 0x7FFF is a 32x increase with no explanation
for why 0x7FFF is the correct upper bound. The policy indices at 0x7F01-0x7F04
are clearly the motivation, but there's now a large unvalidated gap between
0x1FF and 0x7F01 that this function will silently accept and likely produce a
garbage template for.
Consider adding a guard inside the function body to reject the dead zone kinda like this:
if (offset > 0x1FF && offset < 0x7F01) {
return BAD_FUNC_ARG;
}
Also consider updating the comment to explain all four stages this gate is now
covering
examples/endorsement/get_ek_certs.c
Outdated
| if (rc != 0) { | ||
| printf("EK Index 0x%08x not valid\n", nvIndex); | ||
| const char* indexType = "Unknown"; | ||
| word32 offset = nvIndex - TPM_20_TCG_NV_SPACE; |
There was a problem hiding this comment.
offset is computed here unconditionally but the
check if (nvIndex < TPM_20_TCG_NV_SPACE) doesn't happen until a few lines
later. word32 is unsigned if nvIndex is below TPM_20_TCG_NV_SPACE
this silently wraps to a huge value before the guard ever fires.
Consider maybe moving the offset calculation inside the else branch after the bounds check
|
|
||
| /* EK Policy Indices for PolicyAuthorizeNV (0x7F01 - 0x7F04) */ | ||
| #define TPM2_NV_EK_POLICY_SHA256 (TPM_20_TCG_NV_SPACE + 0x7F01) | ||
| #define TPM2_NV_EK_POLICY_SHA384 (TPM_20_TCG_NV_SPACE + 0x7F02) |
There was a problem hiding this comment.
These four defines are added here but never actually referenced in the PR can you use these for the cosntants
| certSz = sizeof(certBuf); | ||
| } | ||
|
|
||
| rc = wolfTPM2_NVReadAuth(&dev, &nv, nvIndex, certBuf, &certSz, 0); |
There was a problem hiding this comment.
Should we open a new nv handle here if its not correctly initialized for nvIndex?
3 participants
No description provided.