-
-
Notifications
You must be signed in to change notification settings - Fork 190
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Add support for TPM2 in all shell scripts #1031
Conversation
Complementary @aesrentai |
I saw that commit already actually, and I'll probably take some code from it. From a high level it takes a similar approach, creating a generic wrapper for each tpm command and then checking for tpm 1.2 or tpm2 in each wrapper. |
@jans23 @aesrentai @bluecmd and all others interested in TPM2 support under Heads, the commit referred above includes the whole support, which needs board owners to test the changes. I would be willing to bring the support upstream but do not own any TPM 2.0 board nor KGPE-D16 TPM 2.0 add-on. One or the other would be required for me to test this, otherwise SWTPM is required and would help everyone to test under qemu, which also needs a bit of love and code borrowed from safeboot global Makefile. One or the other landing first (qemu/one board confirming borrowed code to support 1.2 while changes confirming no regression under 1.2) would help everyone. If you want me to do this, you can contact me personally, donations of hardware/funding of the work always welcome. |
@tlaurion I can run a TPM1 or TPM2 interchangeably on one of my boards (already in CB 4.13) so would be willing to test. I also have a TPM2 only board in the lab which ive already ported to coreboot just haven't upstreamed the board yet. I did check out the above commit from the vaultboot repo and tried to build using the only board that seems to have TPM2 in that commit - BOARD=qemu-hvault-tpm2, but that failed with the openssl build on which tpm2-tss depends. Something wonky with the command line generating the apps/openssl output.
that being said, I do like the wrapper approach to compatibility. neat. |
5194ce6
to
76a7681
Compare
TPM2 cannot extend shell functions, only full binaries. Thus move recovery to its own file in /bin as it's the most common thing to be extended. An alternative is to use the tpm2 pcrextend with a precomputed pcr digest, however that will not allow the use of the generic tpm_extend function. Signed-off-by: Cody Ho <[email protected]>
Used to abstract away all the differences between TPM1.2 and TPM2. Eventually all code should be written to use this abstraction layer. Signed-off-by: Cody Ho <[email protected]>
Board specific scripts (ie t430-flash) have not been modified. Signed-off-by: Cody Ho <[email protected]>
Also remove most of tpm-reset and just call the tpmx code directly. It's being left in because many guides on the internet still have references to tpm-reset in them, so removing it may create unnecessary confusion. Signed-off-by: Cody Ho <[email protected]>
Contains various code cleanups and a note to retrieve USB device branding from USB device info, not just checking vendor id
76a7681
to
2cec856
Compare
@daringer in your board config did you change the following? For tpmx script to understand which TPM you have it needs to have the correct TPMtype set in boards/ export CONFIG_TPM=y That being said im not sure the tpm2 work is fully complete and it looks like the error is that tpmx is not passed the correct parameters at some point around the ehci module. I am due to start playing with the tpm2 variant soon as I have a tpm2 for one of my boards. might try later this evening. edit you also need to change CONFIG_USER_TPM1=y to CONFIG_USER_TPM2=y in config/coreboot-board.conf |
@daringer the issue appears to be the set -e command in tpmx (line 6). removing this line allows the $1 variable to pass through to the functions reset and extend in the wrapper. Leaving this in means $1 is always 'pipefail' |
@daringer @aesrentai @shamen123 @root-hardenedvault : collaboaration here would be awesome since a working implementation exists here and needs to be modified to be upstreamed under Heads. |
yup I can boot my system now, I have seen measured boot values in PCR, no /dev tpm device yet, but this is another issue, so in short: I am progressing towards a state where I can actually help developing on this. Apart from that TPM2 tooling (the modules from #893) are not upstream, @tlaurion how do you see this being included into heads ? one big PR including the modules and the |
@daringer I couldn't test myself, but having both in a PR wouldn't hurt if tpmx wrapper is still working for TPM 1! Congrats! I recall that recovery should be in its own file to be measured by tpm2 toolset? Having a PR to do some regression test for other boards would be a good step. Looking at discussions that happened on slack would give you insights on what needed to be done on acpi tables to have the device recognised from oses? Let me know, I don't have a strong opinion, as long as it doesn't break things. Last time I checked, @root-hardenedvault's implementation was working for him but building instructions were not clear and dependencies not updated, yet his changes were not upstreamed through PR, and he doesn't seem to reply on github on comments on past PR reviews. |
okok, sounds good to me, then I will likely first target a PR with something that builds and include all the necessary stuff (under the line this will be: #893 + #1031 on top of the current master) using this we can first check if things work as intended with the |
Again, wanting to know why not hardenedvault/vaultboot@4506fc2 |
@daringer @tlaurion apologies for the long wait, school got very busy. Now it's summer however and I'll have time to finish this along with #1030 . I completely forgot where I was, however, so I'll get back to you late this week on why I didn't just copy the entire hardenedvault wrapper-- I remember I had a reason but honestly I forgot what it was. |
@daringer @aesrentai We are at a stage, with KVM/QEMU support under #1188 for swtpm HOTP and local testing, to take a leadership decision on what implementation to chose and go forward. Personally, I would take @root-hardenedvault implementation, based on #893 and #907 prior work and go from there, making hardenedvault approach upstream (tested and used in hardware already). @aesrentai @daringer : your input on his approach and why you decided to create/use another one (while similar #1109 ) would be interesting prior of going forward. #1188 board configurations can be reused easily to create I'm asking for your input, since I would take that ball and move it forward under paid grant application work, thanks to Nlnet. |
#1188 is a very cool thing, TPM2 was not moved forward from my side as I miss some capable tpm2 hardware for some time now...
No strong opinion here, I adopted mainly @aesrentai's approach, so under the line from the risk perspective I would also suggest to go for the hardenedvault |
@aesrentai @daringer @shamen123 : I would call for participation under #1292 ! |
Superseded by #1292 |
This is broken and will be for the foreseeable future. The goal is to replace all tpm 1.2 commands with their tpm2 equivalent, as well as following tpm2 best practices to ensure maximum security (ie, enabling parameter encryption, locking the OS out TPM2 platform authorization, etc).
Note: this should be merged on top of #893 but for some reason if I select that as the branch to merge on top of github loses it and wants to merge 116 commits. Whenever that is merged I'll rebase on top of master.