This repository was archived by the owner on Aug 5, 2022. It is now read-only.
support whole-disk encryption with TPM 2.0 #326
Merged
Conversation
This file contains hidden or 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
pohly
force-pushed
the
tpm2
branch
4 times, most recently
from
edf9806 to
5fb94fc
Compare
Contributor
Author
|
@mythi can you have a look at the PR? It passed all tests now and includes a workaround for the unbuildable librealsense. The test_comm_btcheck looks unrelated. test this please |
mythi
reviewed
Nov 15, 2017
Contributor
mythi
left a comment
mythi
left a comment
There was a problem hiding this comment.
No obvious 'request changes' from me. However, my preference is to keep the PR open until we get the TPM2 in stable swtpm branches instead of pulling from our own forks.
A couple of questions also added.
| SECTION = "apps" | ||
|
|
||
| DEPENDS = "libtasn1 fuse expect socat glib-2.0 libtpms2" | ||
|
|
Contributor
There was a problem hiding this comment.
how about adding PROVIDES for these recipes? We'd avoid making changes to '2' and back.
Contributor
Author
There was a problem hiding this comment.
You mean, override swtpm*.bb with swtpm2*.bb? I didn't want to do that because I want to keep the choice open between using one or the other.
Contributor
There was a problem hiding this comment.
Yes. My thinking was to make the selection easier for the end-user. Now modifications are needed in many places.
| # Without this we end up taking TPM2.0 recipes from meta-security | ||
| # instead of meta-measured. We only need meta-tpm because of swtpm. | ||
| # If we move that to meta-measured, we could instead drop meta-tpm. | ||
| BBMASK += "meta-security/meta-tpm/recipes-tpm/tpm2" |
Contributor
There was a problem hiding this comment.
With swtpm2 and others copied to meta-refkit, do we need meta-security at all? Dropping it could simplify BBMASKing.
Contributor
Author
There was a problem hiding this comment.
meta-security is still the place from where we get the TPM 1.2 recipes plus some other things, like keyutils.
Contributor
There was a problem hiding this comment.
OK, at quick glance I did not find any other use for meta-security than meta-tpm (supported recipes says keyutils@security-framework)
| # -t 0x80020002 = TPMA_NV_OWNERWRITE|TPMA_NV_OWNERREAD|TPMA_NV_READ_STCLEAR | ||
| # (https://trustedcomputinggroup.org/wp-content/uploads/TPM-Rev-2.0-Part-2-Structures-01.38.pdf) | ||
| # | ||
| # tpm2.0-tools master supports --attibutes=ownerwrite|ownerread|read_stdclear, but |
Contributor
There was a problem hiding this comment.
Just for my own education: is master same as road to 3.x (referenced by the initramfs code)? Is that going to be available soon?
Contributor
Author
There was a problem hiding this comment.
There is now a 3.0rc1 based on master, so 3.0 might not be too far away.
mythi
reviewed
Dec 4, 2017
| .......................................................................... | ||
|
|
||
| CONFIG_TCG_TPM=y | ||
| CONFIG_TCG_CRB=m |
Contributor
There was a problem hiding this comment.
Looks like we need CONFIG_TCG_CRB=y or have the module installed and loaded for fTPM to get the device interface available in initramfs.
Contributor
Author
|
On Mon, 2017-12-04 at 08:52 +0000, Mikko Ylinen wrote:
+CONFIG_TCG_TPM=y
+CONFIG_TCG_CRB=m
Looks like we need CONFIG_TCG_CRB=y or have the module installed and
loaded for fTPM to get the device interface available in initramfs.
Building it into the kernel is less intrusive IMHO. Adding the right
set of kernel modules into the initramfs would be harder.
|
Contributor
|
On Mon, Dec 4, 2017 at 11:18 AM, Patrick Ohly wrote:
On Mon, 2017-12-04 at 08:52 +0000, Mikko Ylinen wrote:
> +CONFIG_TCG_TPM=y
> +CONFIG_TCG_CRB=m
>
> Looks like we need CONFIG_TCG_CRB=y or have the module installed and
> loaded for fTPM to get the device interface available in initramfs.
Building it into the kernel is less intrusive IMHO. Adding the right
set of kernel modules into the initramfs would be harder.
I thought so too. Can you make the change?
|
Dumping the keyfile with "od" was done only for debugging and accidentally leaked the key. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
This makes it possible to build tpm2-tools from meta-measured. That layer is used instead of meta-security/meta-tpm because WindRiver is also using meta-measured. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
These recipes are derived from the corresponding swtpm recipes in meta-security. swtpm2 is a superset of the stable swtpm and supports both TPM1.2 and TPM2.0: * once it gets released, we could instead just bump the version of the normal swtpm recipe * in refkit, we build only swtpm2 and use it for both TPM1.2 and TPM2.0 The recipes get added here because the code is still experimental. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Now that we can run swtpm2 entirely as normal user, the CUSE support is no longer needed. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Code for connecting qemu with a TPM provided by a running swtpm has
been merged into upstream qemu. It makes it possible to do:
swtpm socket -d --pid file=/tmp/swtpm.pid --tpmstate dir=/path/to/my-tpm \
--ctrl type=unixio,path=/tmp/swtpm-ctrl \
--log file=/tmp/swtpm.log,level=5 &
qemu-system-x86_64 \
[...] \
-chardev socket,id=chrtpm,path=/tmp/swtpm-sock \
-tpmdev emulator,id=tpm0,chardev=chrtpm \
-device tpm-tis,tpmdev=tpm0 \
[...]
The non-upstreamable chardev-connect-socket-to-a-spawned-command.patch
makes it possible to spawn swtpm from inside qemu:
-chardev 'socket,id=chrtpm0,cmd=exec swtpm_oe.sh socket --terminate --ctrl type=unixio,,clientfd=0 --tpmstate dir=... --log level=10,,file=.../swtpm.log --tpm2' \
-tpmdev emulator,id=tpm0,chardev=chrtpm0 \
-device tpm-tis,tpmdev=tpm0
We do that because then we don't need to modify runqemu or the test
framework.
Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
This gets done for the sake of consistency with other layers (meta-security, meta-measured) and the naming of the kernel config files. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
The previous approach of setting KERNEL_FEATURES globally had some problems: - it was based on the assumption that all BSPs have meta data which contains the specified features, breaking compilation for those BSPs which don't have them or have them under different names - it couldn't be used to enable features that are not yet supported by the BSPs, like TPM2 To address both of these problems, kernel config fragments are stored again together with the distro itself. The files were copied from yocto-kernel-cache and meta-measured, so the actual config is unchanged. The new linux-%.bbappend is meant to be a permanent config change and does not address a temporary issue in one of the upstream layers, therefore it intentionally gets created under meta-refkit-core/recipes-bsp and not somewhere under meta-refkit-core/bbappends. The alternative would be to leave kernel reconfiguration to the developer who sets up a build. This would lead to a worse out-of-the-box experience when building refkit and therefore has been rejected. For a discussion of this problem see https://bugzilla.yoctoproject.org/show_bug.cgi?id=8191 Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Previously it was only a kernel module. Because we don't add kernel modules to the refkit initramfs, TPM support was not available during early boot on hardware using such a TPM: installation with TPM2-enabled whole-disk encryption worked, but booting failed. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Contributor
Author
|
On Mon, 2017-12-04 at 11:54 +0000, Mikko Ylinen wrote:
> Building it into the kernel is less intrusive IMHO. Adding the
right
> set of kernel modules into the initramfs would be harder.
>
>
I thought so too. Can you make the change?
Done.
In the meantime, Stefan Berger has also added tags to his latest tpm2
preview branches, so I was able to switch back to just using the
upstream repos in the recipes.
Perhaps this can be merged now?
|
Contributor
|
@pohly Great, thanks! Looks good to me. There is one more thing to check before merging. I noticed this when there was a failure: an empty |
Contributor
Author
|
On Mon, 2017-12-04 at 23:57 -0800, Mikko Ylinen wrote:
I noticed this when there was a failure:
an empty $keyfile flows through until cryptsetup open (and
luks_cleanup) if neither /dev/tpm* is found nor
REFKIT_DISK_ENCRYPTION_PASSWORD is set.
I'll add a warning message about the empty keyfile. It's not necessary
a hard error (but close), so calling "fatal" might be a bit too
drastic.
It triggers faulty parameters to dd at least: bs="$(stat -c '%s'
"$keyfile") (bs=0).
I'll add an if check for that.
|
When we failed to retrieve a password, zapping the empty keyfile led to dd being called with bs=0, triggering an error. Using count=0 is fine. While at it, the regular dd output gets silenced because it is irrelevant and might even expose some undesirable information (length of password). Not finding a password was not reported well. Now there is an explicit error message about it. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
During installation, TPM 1.2 or 2.0 have to be chosen explicitly via the "tpm" env variable. Detecting whether a /dev/tpm0 supports TPM 1.2 or 2.0 turned out to be tricky (tpm2-software/tpm2-tools#604) and probably isn't worth the effort and potential error cases. Conceptually TPM 2.0 supports is similar to the one for 1.2, with one exception: because it is possible to use NVRAM without taking ownership and taking ownership with known passwords wouldn't provide any advantages, the TPM 2.0 support code skips that step. Testing switches to swtpm2 for both the existing TPM1.2 test and the new TPM2.0 test. It just gets parameterized differently. Signed-off-by: Patrick Ohly <patrick.ohly@intel.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
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.
Besides enabling and testing TPM 2.0 support for whole-disk
encryption, this PR also enhances how refkit handles kernel
configuration.