feat(esapi): add missing TPM commands in non_volatile_storage#649
feat(esapi): add missing TPM commands in non_volatile_storage#649hyperfinitism wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds Rust ESAPI wrappers and integration tests for several TPM non-volatile storage commands, filling gaps in the non_volatile_storage command module.
Changes:
- Added wrappers for NV bit setting, read/write locks, global write lock, auth change, and NV certification.
- Added integration tests covering the new command wrappers.
- Added examples and API documentation for the new methods.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
tss-esapi/src/context/tpm_commands/non_volatile_storage.rs |
Adds new ESAPI wrapper methods and documentation for missing NV commands. |
tss-esapi/tests/integration_tests/context_tests/tpm_commands/non_volatile_storage_tests.rs |
Adds integration tests for the new NV command wrappers. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| pub fn nv_global_write_lock(&mut self, auth_handle: AuthHandle) -> Result<()> { | ||
| ReturnCode::ensure_success( | ||
| unsafe { | ||
| Esys_NV_GlobalWriteLock( | ||
| self.mut_context(), | ||
| auth_handle.into(), |
There was a problem hiding this comment.
As per comment above, this isn't needed.
| /// context.nv_global_write_lock(AuthHandle::Owner) | ||
| /// .expect("Call to nv_global_write_lock failed"); |
There was a problem hiding this comment.
As per comment above, this isn't needed.
| /// # let owner_nv_index_attributes = NvIndexAttributes::builder() | ||
| /// # .with_owner_write(true) | ||
| /// # .with_owner_read(true) | ||
| /// # .with_nv_index_type(NvIndexType::Bits) |
There was a problem hiding this comment.
I'm kind of torn - ideally we'd show this part of the setup too, since it's important that the NV attributes has this property set. But it's quite a bit of text added to the example.
| /// # .with_nv_index(nv_index) | ||
| /// # .with_index_name_algorithm(HashingAlgorithm::Sha256) | ||
| /// # .with_index_attributes(nv_index_attributes) | ||
| /// # .with_index_auth_policy(digest) |
There was a problem hiding this comment.
I think at least here it'd be good to make this visible in the example.
There was a problem hiding this comment.
Addressed. Made this visible.
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `auth_handle` - An [AuthHandle] for the authorization (Platform hierarchy). |
There was a problem hiding this comment.
I think this is incomplete - the spec says:
This command requires either platformAuth/platformPolicy or ownerAuth/ownerPolicy. The Index will be locked whether the index was defined using Owner Authorization or Platform Authorization.
Perhaps add a bit of text about what can be provided here
There was a problem hiding this comment.
Expanded the description of this argument.
| pub fn nv_global_write_lock(&mut self, auth_handle: AuthHandle) -> Result<()> { | ||
| ReturnCode::ensure_success( | ||
| unsafe { | ||
| Esys_NV_GlobalWriteLock( | ||
| self.mut_context(), | ||
| auth_handle.into(), |
There was a problem hiding this comment.
As per comment above, this isn't needed.
| /// context.nv_global_write_lock(AuthHandle::Owner) | ||
| /// .expect("Call to nv_global_write_lock failed"); |
There was a problem hiding this comment.
As per comment above, this isn't needed.
Added the following wrapper functions with integration tests for these commands: - nv_set_bits (ESAPI spec 11.3.109) - nv_write_lock (11.3.110) - nv_global_write_lock (11.3.111) - nv_read_lock (11.3.113) - nv_change_auth (11.3.114) - nv_certify (11.3.115) Signed-off-by: Takuma IMAMURA <209989118+hyperfinitism@users.noreply.github.com>
hyperfinitism
force-pushed
the
feature/nvs-commands
branch
from
78a98fa to
33a4e0b
Compare
This pull request implements the following Esys wrapper functions with integration tests for these commands:
These were extracted from #625.