-
Notifications
You must be signed in to change notification settings - Fork 13
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
kola: add test for TPM- and Tang-based disk encryption #493
Conversation
// As advised in the Flatcar documentation, we then remove the ROOT label from the existing | ||
// root partition, which is vda9 in the QEMU disk image. | ||
IgnitionConfigRootTang = `{ | ||
"ignition": { |
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.
We could use a Butane configuration for better reading - but we need to drop the limitation of Clevis on the Butane side too: https://github.com/coreos/butane/blob/e859cb40d7c1d0c24e38311b2d51c4eb0a91ec88/config/flatcar/v1_2_exp/translate.go#L27
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.
Good point!
My thinking was that it would be best to first merge clevis support and after that open a PR to update the Butane spec, to avoid confusion. But once the Butane spec is updated, I'd be happy to bump the butane
Go dependency in mantle
and rewrite the config here. We can either hold off the merge of this PR until then, or I can open a second PR later. I don't have a preference either way; let me know what option you think is best.
kola/tests/misc/tang.go
Outdated
"units": [{ | ||
"name": "remove-root-label.service", | ||
"enabled": true, | ||
"contents": "[Service]\nType=oneshot\nExecStart=wipefs -a /dev/vda9\n[Install]\nWantedBy=multi-user.target" |
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.
Isn't it possible to reformat this partition with Ignition? I would prefer that because otherwise it's not clear which rootfs is used in the initrd in the first run.
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 think some filesystems entry like that could work, here in butane yaml:
variant: flatcar
version: 1.0.0
storage:
filesystems:
- device: /dev/disk/by-partlabel/ROOT
wipe_filesystem: true
format: none
Ignition json:
{
"ignition": {
"version": "3.3.0"
},
"storage": {
"filesystems": [
{
"device": "/dev/disk/by-partlabel/ROOT",
"format": "none",
"wipeFilesystem": true
}
]
}
}
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.
Yes, that works. I changed it as you suggested.
kola/tests/misc/tang.go
Outdated
// between this test and how the network is setup for the VMs, and also it might bind Tang to an interface with a public IP. | ||
// An alternative approach would be to add another TAP interface to the bridge and let the Tang server bind there, but that would | ||
// require the Tang setup to happen outside of these tests and introducing more complexity in different parts of the code base. | ||
// I'll decide whether to rewrite this or leave it as it is based on feedback by reviewers. |
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.
With Platforms: []string{"qemu"},
that seems ok
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.
Okay, I removed the TODO
comment.
kola/tests/misc/tpm.go
Outdated
"units": [{ | ||
"name": "remove-root-label.service", | ||
"enabled": true, | ||
"contents": "[Service]\nType=oneshot\nExecStart=wipefs -a /dev/vda9\n[Install]\nWantedBy=multi-user.target" |
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.
(same comment as above)
2d69649
to
cf1c0bb
Compare
I added the kernel arguments to the tests that are now required to enable clevis unlock in the initramfs (see the thread here). I also solved the issue with tests failing because |
cdc5f6d
to
1cc45e0
Compare
Oh, the Go compiler is not yet happy about |
1cc45e0
to
5ccf116
Compare
That symbol was introduced in Go 1.20, but the pipeline used Go 1.19. I suppose the pipeline sees that the I bumped the Go version in |
Thanks, that should work then |
Add test for TPM- and Tang-based disk encryption
This PR adds four new tests, for root and non-root encryption using TPM and Tang. It accompanies PRs in the bootengine and scripts repositories.
How to use
Run the new tests, e.g. with
kola run -k -b cl -p qemu-unpriv --board amd64-usr --qemu-image $SOME_IMAGE cl.tpm.* cl.tang.*
in a Docker container. Note that the Docker image needs to be rebuilt because of the new dependency onswtpm
for the TPM emulation with QEMU. If you run the tests natively, you'll also need to ensure thatswtpm
is in$PATH
.The four tests should fail for Flatcar images without TPM- and Tang-based disk encryption support and succeed for images with such support.
Testing done
I ran the four new tests in Docker. On images built from the PR in the scripts repository, the nonroot tests pass. The root tests fail because the
systemd-cryptsetup@rootencrypted.service
fails on reboot, although the disks are decrypted properly. This is because the root disk has to be decrypted in the initramfs, causing the subsequentsystemd-cryptsetup
unit to fail. I will wait for the discussion on how to best fix this in the PR in the scripts repository.changelog/
directory (user-facing change, bug fix, security fix, update)/boot
and/usr
size, packages, list files for any missing binaries, kernel modules, config files, kernel modules, etc.