make openssl libtss2 tpm2 reproducible #1630
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@JonathonHall-Purism more testing needed but leaving here to provide diffoscope output and discuss approaches taken. This also bumps tpm2-tools to 5.2 (was more then needed) and tpm2-tss to 3.2.2 |
Ease cleaning up everything. IMOH better then real.clean target Signed-off-by: Thierry Laurion <[email protected]>
…rl script contains reproducible date and fake compiler_flags hardcode VERSION='reproducible_build' into generated configure script to get rid of generate random git abbrev 8/12 chars (could not find source) patches/openssl-3.0.8.patch: clean up tpm2-tools/tpm2-tss: hack configure scripts to not contain hardcoded libs and other rpath related strings, using sed instead of patching configure script like cryptsetup2 patch Will be clened up in other commits. Leaving here as trace for autotools sed patching for reproducible builds. CircleCI: change working dir from project->heads so that CircleCI and local builds are from heads directory, helping reproducible builds TODO: change other patches a well and generalize to gpg toolstack, removing patches that are a maintainership burden. Signed-off-by: Thierry Laurion <[email protected]>
…ACKAGE_VERSION string which configure.ac points to ./VERSION already tpm2-tools-5.6 patch: comment out git versioning output under ./VERSION; module: output current version under ./VERSION instead. Document under module Signed-off-by: Thierry Laurion <[email protected]>
…move patch 3.2.0->3.2.2 disable static lib builds Signed-off-by: Thierry Laurion <[email protected]>
cf97b65
to
8208c86
Compare
final rom still not reproducible, but tools packed under tools.cpio are. |
|
Replaying notes linuxboot/heads-wiki#70 (comment) So coreboot part not reproducible (assembly of the rom) for nitropad-n41. Will build other coreboot forks: purism/nitrokey and post results in next comment. Seems like this PR (CircleCI path change to be heads not project) and past work makes other issues considered fixed as well. Modifying OP. |
top roms are the same. full rom is missing from output on both CI and local as #1534 states. |
|
Just realized that coreboot hardcodes |
This comment has been minimized.
This comment has been minimized.
|
Rebuilding CircleCI roms without cache. clean local build produced same result for librem_15v4, not sure what is happening here. |
|
Ok opening another issue on coreboot reproducibility. My insight is that we modify coreboot build options compared to what coreboot tries to do reproducibily, keeping maybe not needed generally applied customizations that should only exist per board configs for old version (coreboot 4.11 artifacts maybe). @JonathonHall-Purism this PR is ready for review, other traces above not relevant. |
Be advised, command arguments between tpm2-tools versions are very unstable. Make sure that all invocations in scripts still work after updates. |
|
@tlaurion The changes here look fine to me (and they do reproduce correctly for me 🥳 ) but of course we need to test it. Thanks for the heads-up @krystian-hebel . What testing have you done so far? |
|
I tested over qemu-tpm2 and saw only warning for tpm about verifying pubkey? Sealing/unsealing ops worked up to DUK sealing/unsealing. |
tlaurion
commented
Apr 9, 2024
|
tlaurion/heads contained a release 5.0.1 that was aimed to prepare a version somewhere in the past. I deleted that release tag. |
…calls as opposed to patch version specific autotools/configure scripts to force reproducible builds Signed-off-by: Thierry Laurion <[email protected]>
b31f340
to
be71430
Compare
|
@JonathonHall-Purism : added TODOs in modules/tpm2* so that sed approach can be uniformized for all other modules instead of maintaining patches which are version specific and high maintenance costs to maintain reproducible builds across version bumps. Ready for review once CircleCI builds SHOULD produce local=CircleCI equivalent builds, hopefully. |
Where error -32 is linked with qubesos->qubes-usb-proxy->qemu which is unrelated to this PR/issue (requires shutdown of sys-usb qube + testing qube, passing usb Security dongle all over again and it normally succeeds). It also seems that NK3 devices are more picky about timing, which some users reported the dongle not being discovered in time in OEM factory reset, sometimes resulting in the device not being detected recognised properly which results in OEM factory-reset/Re-Ownership failing.... EDIT: opened Nitrokey#48 (comment) Testing on real hardware needed. |
|
I'll test it on L1UM v2 (TPM2) but it'll probably be late next week (rest of today/tomorrow is pretty booked and I am away Monday-Tuesday next week) |
|
TPM reset trace on qemu-whiptail-tpm2 (with debug traces), resealing TPM DUK since previously setup: This debug trace hides the warning I talked about previously (only regression noted by upgrading toolstack): |
|
|
Rebasing on master... |
|
@JonathonHall-Purism #1630 (comment) So you want me to silence that warning before merging or you are ok doing that in another separate issue? |
Do you have any idea what the warning means? It seems like we should not just ignore it, but maybe we are doing what it's warning us about |
@JonathonHall-Purism my basic interpretation was that to use the public key side we would need to to other things, but we don't need it since we only seal? |
|
@tlaurion I think you can silence the warning. TL;DR: I'm not sure why the warning only occurs now, it seems like it should have occurred before as well (but it didn't, confirmed on L1UM v2). However, the warning is intended to indicate that tpm2-tools can't prevent an MITM on the TPM with the information given, which is unchanged in this PR. We have some logic relating to this, which is also unchanged. Maybe this new functionality could improve our MITM protection, but it's not trivial to fit into Heads IMO, I'll open a discussion. Again, it's unchanged in this PR other than the addition of the warning, as far as I can tell. From the updated tpm2-startauthsession man page: There's a bit more context in the commit adding it: tpm2-software/tpm2-tools@1719eaf Providing the name would prevent the warning, since tpm2-tools can be sure an MITM is not occurring. (I also compared the rest of the man pages from 5.2 to 5.6, I don't see anything that would have changed meaning, since that was a concern raised.) I don't see why we did not get this warning before (and we didn't, I just checked on L1UM v2). The warning was there, and the new logic doesn't cause it to appear when it wouldn't have before. We do have some logic that is intended to identify a MITM on the TPM - kexec-save-default signs a digest of the primary handle, and kexec-select-boot checks it. This logic may be limited, but with the purpose being to avoid sending secret information to a TPM that may be MITM'd, I think it is fulfilling that. I'll open a discussion with more thoughts. |
tlaurion
added a commit
to tlaurion/heads
that referenced
this pull request
Apr 26, 2024
Discussed under linuxboot#1630 (comment) TODO added in code. Signed-off-by: Thierry Laurion <[email protected]>
Discussed under linuxboot#1630 (comment) TODO added in code. Signed-off-by: Thierry Laurion <[email protected]>
|
@JonathonHall-Purism should do it |
JonathonHall-Purism
approved these changes
Apr 26, 2024
tlaurion
added a commit
to tlaurion/heads
that referenced
this pull request
Apr 26, 2024
Discussed under linuxboot#1630 (comment) TODO added in code. Signed-off-by: Thierry Laurion <[email protected]>
tlaurion
added a commit
to tlaurion/heads
that referenced
this pull request
May 2, 2024
Discussed under linuxboot#1630 (comment) TODO added in code. Signed-off-by: Thierry Laurion <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
fixes #1616 directly.
Past work related to fixating pkg-config config correctly and limiting host system to bleed in builds fixed some previous issues as well, where this PR fixed CircleCI to use working dir of heads (as opposed to project in the past):
closes #734
closes #892
TODO: