Improve documentation, remove dead code plus some minor refactorings
This improves our code-to-comments ratio by a lot.
On the refactorings:
- Simplify the cluster join mode to just a single protobuf message -
a node can either join an existing cluster or bootstrap a new one.
All of the node-level setup like hostname and trust backend is done
using the setup call, since those are identical for both cases.
- We don't need a node name separate from the hostname. Ideally, we would
get rid of IP addresses for etcd as well.
- Google API design guidelines suggest the `List` term (vs. `Get`).
- Add username to comments for consistency. I think the names provide
useful context, but git blame is a thing. What do you think?
- Fixed or silenced some ignored error checks in preparation of using
an errcheck linter. Especially during early boot, many errors are
obviously not recoverable, but logging them can provide useful debugging info.
- Split up the common package into smaller subpackages.
- Remove the audit package (this will be a separate service that probably
uses it own database, rather than etcd).
- Move storage constants to storage package.
- Remove the unused KV type.
I also added a bunch of TODO comments with discussion points.
Added both of you as blocking reviewers - please comment if I
misunderstood any of your code.
Test Plan: Everything compiles and scripts:launch works (for whatever that's worth).
X-Origin-Diff: phab/D235
GitOrigin-RevId: 922fec5076e8d683e1138f26d2cb490de64a9777
diff --git a/core/pkg/tpm/tpm.go b/core/pkg/tpm/tpm.go
index 4638453..e3973d1 100644
--- a/core/pkg/tpm/tpm.go
+++ b/core/pkg/tpm/tpm.go
@@ -36,16 +36,51 @@
)
var (
- // SecureBootPCRs are all PCRs that measure the current Secure Boot configuration
+ // SecureBootPCRs are all PCRs that measure the current Secure Boot configuration.
+ // This is what we want if we rely on secure boot to verify boot integrity. The firmware
+ // hashes the secure boot policy and custom keys into the PCR.
+ //
+ // This requires an extra step that provisions the custom keys.
+ //
+ // Some background: https://mjg59.dreamwidth.org/48897.html?thread=1847297
+ // (the initramfs issue mentioned in the article has been solved by integrating
+ // it into the kernel binary, and we don't have a shim bootloader)
+ //
+ // PCR7 alone is not sufficient - it needs to be combined with firmware measurements.
SecureBootPCRs = []int{7}
// FirmwarePCRs are alle PCRs that contain the firmware measurements
// See https://trustedcomputinggroup.org/wp-content/uploads/TCG_EFI_Platform_1_22_Final_-v15.pdf
- FirmwarePCRs = []int{0, 2, 3}
+ FirmwarePCRs = []int{
+ 0, // platform firmware
+ 2, // option ROM code
+ 3, // option ROM configuration and data
+ }
- // FullSystemPCRs are all PCRs that contain any measurements up to the currently running EFI
- // payload.
- FullSystemPCRs = []int{0, 1, 2, 3, 4}
+ // FullSystemPCRs are all PCRs that contain any measurements up to the currently running EFI payload.
+ FullSystemPCRs = []int{
+ 0, // platform firmware
+ 1, // host platform configuration
+ 2, // option ROM code
+ 3, // option ROM configuration and data
+ 4, // EFI payload
+ }
+
+ // Using FullSystemPCRs is the most secure, but also the most brittle option since updating the EFI
+ // binary, updating the platform firmware, changing platform settings or updating the binary
+ // would invalidate the sealed data. It's annoying (but possible) to predict values for PCR4,
+ // and even more annoying for the firmware PCR (comparison to known values on similar hardware
+ // is the only thing that comes to mind).
+ //
+ // See also: https://github.com/mxre/sealkey (generates PCR4 from EFI image, BSD license)
+ //
+ // Using only SecureBootPCRs is the easiest and still reasonably secure, if we assume that the
+ // platform knows how to take care of itself (i.e. Intel Boot Guard), and that secure boot
+ // is implemented properly. It is, however, a much larger amount of code we need to trust.
+ //
+ // We do not care about PCR 5 (GPT partition table) since modifying it is harmless. All of
+ // the boot options and cmdline are hardcoded in the kernel image, and we use no bootloader,
+ // so there's no PCR for bootloader configuration or kernel cmdline.
)
var (