Skip to content

TPM2 parameter encryption#22630

Merged
poettering merged 1 commit intosystemd:mainfrom
grigorig:tpm2-parameter-encryption
Mar 16, 2022
Merged

TPM2 parameter encryption#22630
poettering merged 1 commit intosystemd:mainfrom
grigorig:tpm2-parameter-encryption

Conversation

@grigorig
Copy link
Copy Markdown
Contributor

Note: this PR builds upon unmerged PR #22563.

This implements parameter encryption for sealing and unsealing secrets to a TPM. A separate salted and unbound HMAC session is used for that. The session key derivation process seems complicated, but in the end it boils down to encrypting a randomly chosen salt with a public key (in this case, the primary key). Only the TPM knows the secret part, and can decode the salt to derive the key correctly.

As far as security guarantees go, this provides confidentiality, integrity and replay protection. So common attacks based on eavesdropping or manipulating the TPM interface will fail. It does not however protect against man in the middle attacks. We would need to authenticate the TPM for that, possibly with its endorsement certificate (if available).

tpm-tss2 libraries provide a handy mechanism to dump data exchanged with TPMs into a pcap file, which then can be analyzed with Wireshark. That's what I did for testing. It would still be great if an actual security researcher could check whether the implementation has any obvious loopholes or weaknesses.

@grigorig
Copy link
Copy Markdown
Contributor Author

I assume the focal-amd64 test fails because of bugs in an old tpm2-tss library version. Is there a way to get detailed journal logs from that test? The autopkgtest site seems to be unresponsive (pages never finish loading).

@mrc0mmand
Copy link
Copy Markdown
Member

I assume the focal-amd64 test fails because of bugs in an old tpm2-tss library version. Is there a way to get detailed journal logs from that test? The autopkgtest site seems to be unresponsive (pages never finish loading).

You should be able to download https://autopkgtest.ubuntu.com/results/autopkgtest-focal-upstream-systemd-ci-systemd-ci/focal/amd64/s/systemd-upstream/20220227_140531_93b90@/artifacts.tar.gz which should have the test journal in the artifacts subfolder.

@grigorig
Copy link
Copy Markdown
Contributor Author

Now THAT is annoying: this issue seems to be culprit. It was fixed in version 2.3.3. And Ubuntu Focal ships... version 2.3.2. :)

Since old Ubuntu versions are not going to ship updated systemd, maybe we could make version 2.3.3 (or 2.4.0) the minimum?

@bluca
Copy link
Copy Markdown
Member

bluca commented Feb 28, 2022

yeah we can push it to 2.3.3, I'll upload to the PPA

@grigorig
Copy link
Copy Markdown
Contributor Author

Great, that's even better, at least for testing purposes. Note that I didn't actually check if version 2.3.3 fully fixes the problem, but if it isn't too much work please update the library.

@bluca
Copy link
Copy Markdown
Member

bluca commented Feb 28, 2022

uploaded 2.3.3 and re-triggered

@grigorig grigorig force-pushed the tpm2-parameter-encryption branch 3 times, most recently from fa09ff7 to df684e0 Compare March 2, 2022 23:54
@grigorig
Copy link
Copy Markdown
Contributor Author

grigorig commented Mar 3, 2022

uploaded 2.3.3 and re-triggered

That seems to have worked, after the network issues (?) were fixed. Thanks for the update.

@poettering poettering added reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks tpm2 labels Mar 3, 2022
@grigorig grigorig force-pushed the tpm2-parameter-encryption branch from df684e0 to 6328cf7 Compare March 12, 2022 09:01
@grigorig grigorig changed the title WIP: TPM2 parameter encryption TPM2 parameter encryption Mar 12, 2022
@grigorig grigorig marked this pull request as ready for review March 12, 2022 09:03
@bluca bluca removed the reviewed/needs-rework 🔨 PR has been reviewed and needs another round of reworks label Mar 12, 2022
@poettering
Copy link
Copy Markdown
Member

lgtm, ready to merge once #22563 is in.

@poettering poettering added postponed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed labels Mar 14, 2022
@poettering
Copy link
Copy Markdown
Member

Setting "postponed" because #22563 needs to be merged first.

@bluca bluca removed the good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed label Mar 14, 2022
@bluca
Copy link
Copy Markdown
Member

bluca commented Mar 14, 2022

(removing green label for now to avoid accidental merges, will re add it later)

@poettering
Copy link
Copy Markdown
Member

#22563 has been merged now. Please rebase. Should be ready for commit then too.

@bluca bluca added postponed good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed and removed postponed labels Mar 16, 2022
Use a salted, unbound HMAC session with the primary key used as tpmKey,
which mean that the random salt will be encrypted with the primary
key while in transit. Decrypt/encrypt flags are set on the new session
with AES in CFB mode. There is no fallback to XOR mode.

This provides confidentiality and replay protection, both when sealing
and unsealing. There is no protection against man in the middle
attacks since we have no way to authenticate the TPM at the moment.
The exception is unsealing with PIN, as an attacker will be unable
to generate the proper HMAC digest.
@grigorig grigorig force-pushed the tpm2-parameter-encryption branch from 6328cf7 to 61b3f2f Compare March 16, 2022 19:13
@poettering poettering merged commit da29de2 into systemd:main Mar 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

good-to-merge/waiting-for-ci 👍 PR is good to merge, but CI hasn't passed at time of review. Please merge if you see CI has passed tpm2

Development

Successfully merging this pull request may close these issues.

4 participants