Skip to content
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

attestation: validate GCP machine state instead of PCR 0 #1343

Merged
merged 3 commits into from
Mar 6, 2023

Conversation

thomasten
Copy link
Member

PCR 0 changed on GCP. The new value can be calculated as follows:

func main() {
	ev1 := &bytes.Buffer{}
	binary.Write(ev1, binary.LittleEndian, utf16.Encode([]rune("GCE Virtual Firmware v2\x00")))
	pcr := extend(make([]byte, 32), ev1.Bytes())                                      // EV_S_CRTM_VERSION
	pcr = extend(pcr, append([]byte("GCE NonHostInfo\x00\x01"), make([]byte, 15)...)) // EV_NONHOST_INFO
	pcr = extend(pcr, []byte{0, 0, 0, 0})                                             // EV_SEPARATOR
	fmt.Printf("%x\n", pcr)
}

func extend(last, data []byte) []byte {
	digest := sha256.Sum256(data)
	h := sha256.New()
	h.Write(last)
	h.Write(digest[:])
	return h.Sum(nil)
}

If you replace v2 with v1 in the first event, you get the old PCR value.

This means that PCR 0 will change if the GCE firmware version number is increased. We should stop comparing PCR 0 against a fixed value. Instead, we can replay the event log to validate the PCR and then validate the events. Fortunately, VerifyAttestation of go-tpm-tools already does most of this for us. This PR checks the result of the func and drops the fixed value for PCR 0.

I'm not sure if I catched all places that need to be changed to drop PCR 0. Please check.

For now I only tested the change using constellation verify against a cluster created with the released CLI v2.5.3 (with PCR 0 set to warnOnly).

@thomasten thomasten added this to the v2.6.0 milestone Mar 5, 2023
@netlify
Copy link

netlify bot commented Mar 5, 2023

Deploy Preview for constellation-docs canceled.

Name Link
🔨 Latest commit 2060455
🔍 Latest deploy log https://app.netlify.com/sites/constellation-docs/deploys/6405a2169602b900085b0b67

Copy link
Member

@daniel-weisse daniel-weisse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PCR[10] is used by Linux IMA, which writes a hash hash over TPM registers 0-7 to it.
I'm not 100% certain if this still happens in out images, but we might want to remove this value from our config as well.

@derpsteb
Copy link
Member

derpsteb commented Mar 6, 2023

The default config also needs to be updated (?). I think this should be removed.

@thomasten
Copy link
Member Author

PCR[10] is used by Linux IMA, which writes a hash hash over TPM registers 0-7 to it. I'm not 100% certain if this still happens in out images, but we might want to remove this value from our config as well.

Yes, I saw a warning for PCR 10. Removed it.

@thomasten
Copy link
Member Author

The default config also needs to be updated (?). I think this should be removed.

Done. Not sure if I have to do anything with measurements_enterprise.go.

@derpsteb
Copy link
Member

derpsteb commented Mar 6, 2023

I don't think so. Seems to me like those are always updated based on the build-os-image.yaml measurements.

@thomasten thomasten merged commit c94d1db into main Mar 6, 2023
@thomasten thomasten deleted the fix/gcp-pcr0 branch March 6, 2023 12:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants