Skip to content

Support for TPM2_NV_NVReadLock for tpmdirect (tpm2)#406

Merged
chrisfenner merged 1 commit intogoogle:mainfrom
9elements:NVReadLock
Jul 23, 2025
Merged

Support for TPM2_NV_NVReadLock for tpmdirect (tpm2)#406
chrisfenner merged 1 commit intogoogle:mainfrom
9elements:NVReadLock

Conversation

@mynetz
Copy link
Copy Markdown
Contributor

@mynetz mynetz commented Jul 23, 2025

This commit adds support for the NVReadLock command and adds a test case for it. For completeness it also adds a test case for NVWriteLock.

I needed NVReadLock for a current project und found only NVWriteLock was implemented. The command is listed in google/go-tpm/issues#278 as a nice to have. I would like to upstream this.

@mynetz mynetz requested review from a team, alexmwu and jkl73 as code owners July 23, 2025 15:15
@google-cla
Copy link
Copy Markdown

google-cla Bot commented Jul 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Comment thread tpm2/test/nv_test.go Outdated
@mynetz
Copy link
Copy Markdown
Contributor Author

mynetz commented Jul 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

I'll check. I assumed I am already in the CLA as others from 9e contributed...

Copy link
Copy Markdown
Member

@chrisfenner chrisfenner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this change! One more nit (same testing nit as before) and then I'm happy to merge.

Comment thread tpm2/test/nv_test.go Outdated
This commit adds support for the NVReadLock command and adds a test case for it. For completeness, it also adds a test case for NVWriteLock.

Signed-off-by: Christian Grönke <[email protected]>
@chrisfenner
Copy link
Copy Markdown
Member

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.

I'll check. I assumed I am already in the CLA as others from 9e contributed...

@mynetz I don't claim to super understand the Google CLA system but I believe that the CLA expects an entry for each individual contributor. While a few others from 9elements have already contributed I think it wants to see a signature from you as well.

@mynetz
Copy link
Copy Markdown
Contributor Author

mynetz commented Jul 23, 2025

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).
View this failed invocation of the CLA check for more information.
For the most up to date status, view the checks section at the bottom of the pull request.

I'll check. I assumed I am already in the CLA as others from 9e contributed...

@mynetz I don't claim to super understand the Google CLA system but I believe that the CLA expects an entry for each individual contributor. While a few others from 9elements have already contributed I think it wants to see a signature from you as well.

Yes. I needed to be added somewhere. Now the validation succeeded.

@chrisfenner chrisfenner merged commit 2a10c73 into google:main Jul 23, 2025
4 checks passed
@chrisfenner
Copy link
Copy Markdown
Member

Thanks again for the change @mynetz!

@mynetz
Copy link
Copy Markdown
Contributor Author

mynetz commented Jul 23, 2025

@chrisfenner: You're welcome. The abstraction you (plural) choose for the API makes this form of contribution very eays.

I'll use this brief exchange for a question: While trying to understand the go-tpm/tpm2 code and also the problem, I saw that there is no example code in go-tpm/examples for the newer tpm2 API. I did build an example for myself using an NVRAM space and a PCR policy to access. Would this be of interest for the project?

Then I polish up the code and create a PR: https://gist.github.com/mynetz/552200d13d3e0386a57d009a435bd0b8

@chrisfenner
Copy link
Copy Markdown
Member

@chrisfenner: You're welcome. The abstraction you (plural) choose for the API makes this form of contribution very eays.

I'll use this brief exchange for a question: While trying to understand the go-tpm/tpm2 code and also the problem, I saw that there is no example code in go-tpm/examples for the newer tpm2 API. I did build an example for myself using an NVRAM space and a PCR policy to access. Would this be of interest for the project?

Then I polish up the code and create a PR: https://gist.github.com/mynetz/552200d13d3e0386a57d009a435bd0b8

Thank you!

Yes, I think example code using the new API is of general interest. Today our "new API" examples are all in https://github.com/google/go-tpm/tree/main/tpm2/test. But an examples directory right next to it would be awesome.

If you wanted to create an examples/tpm-nv (or whatever) directory based on https://gist.github.com/mynetz/552200d13d3e0386a57d009a435bd0b8 I'd be happy to approve the PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants