-
Notifications
You must be signed in to change notification settings - Fork 164
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
Full Virtual TPM for VMs and Containers #4071
Conversation
Please don't mind the many commits, I will squash them later. This is more or less ready for review even though there are tasks on the list (I'll do while the review is going on). |
d17afe0
to
e34bc2d
Compare
Isn't /dev/tpm0 the TPM 1.X device, and /dev/tpmrm0 the TPM 2.0 device? I assume we need to keep the current vtpm linuxkit container around since there are some AZIOT VMs which use the existing eve-tools to access that. |
Would it make sense to also add some way for an app instance to find out about the host's TPM? Or will that be implicit in the signature it will see for its EK?
It might make sense to have the encryption key be a function of the App instance UUID. That would prevent someone from reusing it in another VM they control. Note that I haven't thought through the attack scenario in detail - other than a user with physical access can temporarly remove the disk and mount it on a different computer and copy /persist/foo/A to /persist/foo/B. But if we put this under /persist/vault/ then that possibility goes away. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is quite hard to review this PR due to the choice to rename the existing vtpm to ptpm and having the new be called vtmp.
Can we defer that rename until the end of the process so that the diffs make some more sense?
AFAIK TPM version is determined by GetCap packet and examining the returned version, here we only support TPM 2.0 with the
yes, I will keep it running in the VTPM container alongside the SWTPM till all have upgraded to newer versions of AZIOT with no eve-tools patches. |
should be less cluttered now. |
|
Is there a way to temporarily suppress the Yetus warning? Regarding the capital letter. Yep, in some cases, it can be unnecessary. Still, in others, when you write a sentence, I find it useful as it can signal that the commit message was not carefully written in a "get off me" style. |
No I think it is not a rule but parser that has no idea about |
I've downloaded the Yetus results from here to check if there are other issues as well, but the reports are empty... Strange. |
Yes I tried that too, here is my local run : $ make mini-yetus MYETUS_VERBOSE=y
Running mini-yetus
./tools/mini-yetus.sh -f
[+] Running mini-yetus
[+] No source branch specified. Using the main branch: master
[+] No destination branch specified. Using the current branch: swtpm_in_vm
[+] Running yetus on the changes...
[+] Full results:
.codespellignorelines:10:0:blanks:tabs in line
.codespellignorelines:1:0:blanks:tabs in line
.codespellignorelines:11:0:blanks:tabs in line
.codespellignorelines:12:0:blanks:tabs in line
.codespellignorelines:2:0:blanks:tabs in line
.codespellignorelines:3:0:blanks:tabs in line
.codespellignorelines:4:0:blanks:tabs in line
.codespellignorelines:5:0:blanks:tabs in line
.codespellignorelines:6:0:blanks:tabs in line
.codespellignorelines:7:0:blanks:tabs in line
.codespellignorelines:8:0:blanks:tabs in line
.codespellignorelines:9:0:blanks:tabs in line
.hadolint.yaml:1:1:yamllint: warning: missing document start "---" (document-start)
/home/shah/shah-dev/eve/pkg/pillar/hypervisor/kvm.go:198:0:detsecrets:db3d405b10675998c030223177d42e71b4e7a312:Secret Keyword
/home/shah/shah-dev/eve/pkg/pillar/hypervisor/kvm.go:4:1:revive: should have a package comment https://revive.run/r#package-comments
/home/shah/shah-dev/eve/pkg/pillar/hypervisor/kvm_test.go:2371:0:detsecrets:db3d405b10675998c030223177d42e71b4e7a312:Secret Keyword
/home/shah/shah-dev/eve/pkg/pillar/hypervisor/kvm_test.go:56:0:detsecrets:e7ea4f94cb4af75c6643566ca6d95d9433b8a6f2:Secret Keyword
/home/shah/shah-dev/eve/pkg/pillar/scripts/onboot.sh:121:0:shelldocs:ERROR: function percent_used has no @audience
/home/shah/shah-dev/eve/pkg/pillar/scripts/onboot.sh:121:0:shelldocs:ERROR: function percent_used has no @stability
/home/shah/shah-dev/eve/pkg/pillar/scripts/onboot.sh:139:0:shelldocs:ERROR: function free_space has no @audience
/home/shah/shah-dev/eve/pkg/pillar/scripts/onboot.sh:139:0:shelldocs:ERROR: function free_space has no @stability
/home/shah/shah-dev/eve/pkg/vtpm/Dockerfile:33:25:hadolint: invalid flag: --keep-git-dir
/home/shah/shah-dev/eve/pkg/vtpm/patch/patch-tpm2-tools.diff:11:0:blanks:end of line
/home/shah/shah-dev/eve/pkg/vtpm/patch/patch-tpm2-tools.diff:15:0:blanks:end of line
/home/shah/shah-dev/eve/pkg/vtpm/patch/patch-tpm2-tools.diff:17:0:blanks:end of line
/home/shah/shah-dev/eve/pkg/vtpm/patch/patch-tpm2-tools.diff:24:0:blanks:end of line
/home/shah/shah-dev/eve/pkg/vtpm/patch/patch-tpm2-tools.diff:7:0:blanks:end of line
.syft.yaml:7:1:yamllint: warning: missing document start "---" (document-start)
[+] Results stored in /tmp/tmp.GXzKfYfdpI/yetus-output |
Ok, in general, it looks like it is ready to be merged. Does anyone who reviewed the logic have any more comments, or are all satisfied? @milan-zededa, @christoph-zededa, @eriknordmark ? |
I see there is still two TODOs which are not checked. The alpine tag can be done separately, but I wonder about " make sure each swtpm instance gets its's own unique and persistent seed (and by extension SRK, EK, etc)". Is this done? |
yes it is : shah@shah:~/shah-dev/eve/swtpm-seed-test$ cat test-swtpm-seed.sh
#!/bin/bash
CWD=$(pwd)
TPM_SRV_PORT1=2000
TPM_CTR_PORT1=2001
TPM_SRV_PORT2=3000
TPM_CTR_PORT2=3001
EK_HANDLE=0x81000001
TPM_STATE1=/tmp/swtpm-seed-test1
TPM_STATE2=/tmp/swtpm-seed-test2
rm -rf $TPM_STATE1
rm -rf $TPM_STATE2
mkdir -p $TPM_STATE1
mkdir -p $TPM_STATE2
flushtpm() {
tpm2 flushcontext -t
tpm2 flushcontext -l
tpm2 flushcontext -s
}
swtpm socket --tpm2 \
--server port="$TPM_SRV_PORT1" \
--ctrl type=tcp,port="$TPM_CTR_PORT1" \
--tpmstate dir="$TPM_STATE1" \
--flags not-need-init,startup-clear &
PID1=$!
swtpm socket --tpm2 \
--server port="$TPM_SRV_PORT2" \
--ctrl type=tcp,port="$TPM_CTR_PORT2" \
--tpmstate dir="$TPM_STATE2" \
--flags not-need-init,startup-clear &
PID2=$!
# create first EK and export it
export TPM2TOOLS_TCTI="swtpm:host=localhost,port=$TPM_SRV_PORT1"
tpm2 clear
tpm2 createek -c ek1.ctx
flushtpm
tpm2 readpublic -Q -c ek1.ctx -f pem -o ek1.pem
# create second EK and export it
export TPM2TOOLS_TCTI="swtpm:host=localhost,port=$TPM_SRV_PORT2"
tpm2 clear
tpm2 createek -c ek2.ctx
flushtpm
tpm2 readpublic -Q -c ek2.ctx -f pem -o ek2.pem
kill $PID1
kill $PID2
rm ek1.ctx ek2.ctx
rm -rf $TPM_STATE1 $TPM_STATE2
shah@shah:~/shah-dev/eve/swtpm-seed-test$
shah@shah:~/shah-dev/eve/swtpm-seed-test$
shah@shah:~/shah-dev/eve/swtpm-seed-test$
shah@shah:~/shah-dev/eve/swtpm-seed-test$ ./test-swtpm-seed.sh
shah@shah:~/shah-dev/eve/swtpm-seed-test$ ls
ek1.pem ek2.pem test-swtpm-seed.sh
shah@shah:~/shah-dev/eve/swtpm-seed-test$ diff <(openssl rsa -in ek1.pem -pubin -outform DER -pubout | openssl dgst -sha256) \
<(openssl rsa -in ek2.pem -pubin -outform DER -pubout | openssl dgst -sha256)
writing RSA key
writing RSA key
1c1
< SHA2-256(stdin)= 90d39a7ff6f1b377898d1dc7867d63e4f6c09c24a39b8eabc0d61a3cbc4edfab
---
> SHA2-256(stdin)= 01d4226284fd74c939ebae367dea6d51c22e3f496337ead3b8b52c83a017262f
shah@shah:~/shah-dev/eve/swtpm-seed-test$ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added three questions as comments, and it seems like commitlint wants to to have sentences which start with a captital letter.
Running the tests
b13bc8e
to
7839ea7
Compare
Just a clean up and renaming old vtpm binary to its new name ptpm. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Move the patch file to a more appropriate location. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Create nessessary directories in /persist and /run for swtpm and vtpm to store their data and set the correct permissions. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Remove unnecessary access to /config and add access to /persist in the vtpm container. /persist is used to store the TPM state. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Add apparmor profiles for swtpm and ptpm and update the existing profiles for vtpm to reflect the changes. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Move old vtpm docs to PTPM.md and update VTPM.md with new content reflecting the new vtpm service. Signed-off-by: Shahriyar Jalayeri <[email protected]>
…e VMs This commit brings SWTPM in EVE, in order to emulate Virtual TPM in the VMS, making TPM accessible in Vms like a normal TPM device under /dev/tpm* . SWTPM saves and loads the TPM state on/from the disk, so at the next VM boot all the TPM keys, TPM NVRAM data, etc are present. In addition SWTPM is configured to encrypt the TPM state files using a 256-bit AES key. We use the same key as vault key, this key stored in TPM and access to it is authorized using a PCR policy, this makes it easier to manage since if we use a separate key for SWTPM states, there is one more key to backup/restore from the controller. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Restrict vtpm access to needed directories only, it doesn't need access to all of the /persist and /run, just /persist/swtpm and /run/swtpm. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Just in case the swtpm takes a bit longer to start up, due to the system being busy or TPM being slow to release the encryption key, increase the timeout to 10 seconds. Signed-off-by: Shahriyar Jalayeri <[email protected]>
Problem with existing VTPM service
First of all, VTPM has a misleading name since it is not actually a Virtual TPM, but a HTTP server which through its APIs, exposes a limited set of the host's TPM functionalities to the callers. As a result, there is no TPM device present in the VM (no
/dev/tpm*
). This is problematic since some services (for example Azure Identity Service) expects a TPM device is available and is programmed to communicate directly with the TPM for credential transfer and signing purposes.To fix this issue, we patch the Azure Identity Service to translate its TPM calls to VTPM calls (the patch set is maintained in EVE-TOOLS repository). This approach while working, can lead to maintenance issues. Currently we support AZIOT version 1.2. We claim to support the 1.4 version too, but in reality we revert the Azure Identity Service to a commit between version 1.2 and 1.3 to apply the patches and are lucky that compiled binaries are still compatible with version 1.4.
Full Virtual TPM in VMs and Containers
As mentioned above, using VTPM is not optimal. VTPM comes with its own set of problems1, in addition it creates a limiting environment and any external program that relies on a TPM requires creating and maintaining its own set of patches. To solve this problem, This PR uses Software TPM (SWTPM) to emulate a Virtual TPM per VMs.
Fortunately SWTPM integrates well with QEMU, in order to emulate a Virtual TPM in the VM, we create a SWTPM instance per VM. The SWTPM instance is configured to use a Unix Domain Socket as a communication line, after passing the socket path to the QEMU virtual TPM configurations, it automatically creates a virtual TPM device for the VM which is accessible like a normal TPM under
/dev/tpm*
. We can create the same setup for bare metal containers using Linux Virtual TPM Proxy driver to create arbitrary virtual TPM devices on the host, backed by a SWTPM process and mapped to the container as/dev/tpmX
device.SWTPM saves and loads the TPM state on/from the disk, so at the next VM boot all the TPM keys, TPM NVRAM data, etc are present. In addition SWTPM is configured to encrypt the TPM state files using a 256 bit AES key (we use the same key as vault key, this key stored in TPM and access is authorized using a PCR policy, this makes it easier to manage since if we use a separate key for SWTPM states, there is one more key to backup/restore from the controller). We can pass the constant VM id and the encryption key through a KDF to create unique keys for each VM/Container if needed, but it brings no added security. Here is a diagram, showing the basic architecture of the new VTPM:
1: For example it relies on a set of TPM management utilities (TPM-TOOLS) to communicate with the host TPM, this dependency not only increase the attack surface, but also can cause maintenance problem, because TPM-TOOLS is consist of a set of command-line utilities (Not APIs) and a next version of TPM-TOOLS can simply change the command line arguments behavior and break the VTPM functionality without notice.
TODO
and move it a new packageshahzededa/eve-alpine
(Update alpine mirrors for 3.16 to build swtpm #4091)Eden Test
lf-edge/eden#1011
Manual Tests
So far I have tested the following with a Ubuntu 22.04.2 LTS :
1- Sign and verify with the TPM using the endorsement hierarchy [successful]
2- Sign with the TPM and verify with openssl [successful]
Manually Test AzureIoTEdge (Ubuntu 20.04, Aziot 1.4.0 and EVE-TOOLS)
Manually deploying a ubuntu-20.04-server-amd64 and running the test-script I wrote for eden :
ubuntu@vm01:~$ sudo iotedge system status System services: aziot-edged Running aziot-identityd Running aziot-keyd Running aziot-certd Running aziot-tpmd Running
Manually Test AzureIoTEdge (Ubuntu 20.04, Aziot latest (1.5.x) without EVE-TOOLS, using Virtual TPM)
Manually deploying a ubuntu-22.04-server-amd64 and running the test_make_tpm_keys and then test_ubuntu22.04_aziot_latest :
[00:31:28.097] ubuntu@vm01:~$ sudo iotedge system status System services: [00:31:35.783] aziot-edged Running [00:31:35.784] aziot-identityd Running [00:31:35.785] aziot-keyd Running [00:31:35.785] aziot-certd Running [00:31:35.786] aziot-tpmd Running [00:31:35.787]