Attestation quote verification should check both run-time and build-time measurement values #1303
Conversation
| 215, 250, 254, 138, 53, 32, 201, 66, 166, 4, 164, 7, 222, 3, 174, 109, 197, 248, 127, 39, 66, | ||
| 139, 37, 56, 135, 49, 24, 183, | ||
| ]; | ||
| const ACCEPTED_MEASUREMENT: [u8; 32] = [0; 32]; |
There was a problem hiding this comment.
I plan to add this in a followup. This chainspec is currently not being used anywhere, so should not be an issue to leave it like this for now.
| @@ -66,6 +66,7 @@ runtime | |||
| - Non persistent TSS signer and x25519 keypair ([#1216](https://github.com/entropyxyz/entropy-core/pull/1216)) | |||
| - Extract PCK certificate chain from quotes ([#1209](https://github.com/entropyxyz/entropy-core/pull/1209)) | |||
| - Allow different versions for programs ([#1250](https://github.com/entropyxyz/entropy-core/pull/1250)) | |||
| - Attestation quote verification should check both run-time and build-time measurement values ([#1303](https://github.com/entropyxyz/entropy-core/pull/1303)) | |||
There was a problem hiding this comment.
Not sure if this counts as breaking. Old builds of entropy-tss will no longer be compatible with the chain as it is built here - but that is not external-facing.
There was a problem hiding this comment.
If we had a running network depending on this then yes this would be problematic. So yeah I'd mention it here
| @@ -66,6 +66,7 @@ runtime | |||
| - Non persistent TSS signer and x25519 keypair ([#1216](https://github.com/entropyxyz/entropy-core/pull/1216)) | |||
| - Extract PCK certificate chain from quotes ([#1209](https://github.com/entropyxyz/entropy-core/pull/1209)) | |||
| - Allow different versions for programs ([#1250](https://github.com/entropyxyz/entropy-core/pull/1250)) | |||
| - Attestation quote verification should check both run-time and build-time measurement values ([#1303](https://github.com/entropyxyz/entropy-core/pull/1303)) | |||
There was a problem hiding this comment.
If we had a running network depending on this then yes this would be problematic. So yeah I'd mention it here
There was a problem hiding this comment.
Because some events and genesis config entries are changing, maybe we add the Bump "spec_version" tag to the PR
| #[pallet::weight( <T as Config>::WeightInfo::change_accepted_mrtd_values())] | ||
| pub fn change_accepted_mrtd_values( | ||
| #[pallet::weight( <T as Config>::WeightInfo::change_accepted_measurement_values())] | ||
| pub fn change_accepted_measurement_values( |
There was a problem hiding this comment.
Actually, because of this we may even need to bump the transaction version
ameba23
added
Bump `spec_version`
TBH i still don't totally understand how they are computed. The are basically the hash of the bootloader, and initial ramdisk (initrd) which contains the kernel and in our case the image for the initramfs. The reason i have made this PR is because if i make a single byte change to the entropy-tss binary and rebuild the image, RTMR1 changes. MRTD is 'build time' and RTMR0-3 are 'runtime' - but as far as i can see the only one which can actually change after the VM has booted is RTMR3 which is the hash of an event log which can be 'extended' by a special call (which is used by Dstack to do measurements of a docker container - but we do not use it). The most direct explanation is in the intel whitepaper section 4.2.2 https://www.intel.com/content/www/us/en/content-details/783067/whitepaper-linux-stacks-for-intel-trust-domain-extension-1-0.html theres also this more high level article from intel: https://www.intel.com/content/www/us/en/developer/articles/community/runtime-integrity-measure-and-attest-trust-domain.html and my notes about this stuff: entropyxyz/tdx-build-system#6 |
Currently we check only build-time (MRTD) measurement values when verifying attestations.
This PR changes this so that we hash together both the MRTD register value and all four run-time measurement registers (rtmr0,1,2,3).
I'm not sure whether we actually need all four - the first two are the ones that matter. But i think it does not harm to put them in anyway.