Use attest repo to compute expected measurements from image hashes#47
Conversation
ameba23
changed the title
* main: azure: bound attestation evidence payload size (#50) chore: move attestation-provider-server crate from proxy repo azure: stop AIA fetch once AK chain verifies test(attestation): cover Azure AK issuer fetch fallbacks docs(azure_attn): document potential network fetches add comment explaining the PEM guard add AIA_CA_ISSUERS_ACCESS_METHOD_OID variable fix(attestation): harden Azure AK issuer AIA fetching fix clippy: box MaaError add capture_azure_fixture test util cargo fmt fix(attestation): include Azure vTPM AK intermediates
Cache known GCP firmware indexed by MRTD
…at ah/gcp-hobgen has been merged
…for dstack ratls built attested certs, and attestation provider server implementations other than our own
… measurement policy
alexhulbert
left a comment
alexhulbert
left a comment
There was a problem hiding this comment.
lgtm! everything i commented was like super minor nitpicky stuff, feel free to ignore all of it :)
| true | ||
| } | ||
| return true; | ||
| ExpectedMeasurements::Image(image_hashes) => { |
There was a problem hiding this comment.
this is nested deep enough that its a bit hard to read, maybe split something around here into a separate fn?
| return false; | ||
| } | ||
|
|
||
| if let Some(rtmr1) = dcap_measurements.get(&DcapMeasurementRegister::RTMR1) |
There was a problem hiding this comment.
Will we ever have a policy w/o RTMR1 or RTMR2? making this pass validation when we don't have RTMR1/RTMR2 seems a bit sketchy
There was a problem hiding this comment.
Totally agree, good catch.
i have fixed this here to return false if the value is not present and made an issue to change the representation for DCAP measurements so that individual values are not optional: #67
| pub(crate) fn check_measurement_with_gcp_cache( | ||
| &self, | ||
| measurements: &MultiMeasurements, | ||
| platform_metadata: Option<PlatformMetadata>, |
There was a problem hiding this comment.
this could probably be a Option<&PlatformMetadata> without adding complexity anywhere if im not mistaken
| num_disks: 1, | ||
| acpi: Some(AcpiHashes { | ||
| loader: [ | ||
| 246, 12, 53, 229, 59, 178, 27, 70, 117, 207, 168, 219, 49, 14, 200, 142, 56, |
There was a problem hiding this comment.
this would be cleaner as hex
| /// verification | ||
| #[derive(Clone, Debug, Default)] | ||
| pub(crate) struct GcpFirmwareCache { | ||
| cache: Arc<RwLock<HashMap<[u8; 48], attest_measure::dcap::DcapFirmware>>>, |
There was a problem hiding this comment.
attest_measure::dcap::DcapFirmware is imported
| }, | ||
| }; | ||
|
|
||
| let expected_measurements = match expected_dcap_registers( |
There was a problem hiding this comment.
I will fix the todo in my code asap like u said so this doesn't potentially cause a panic
2 participants
This is a protocol breaking change as it changes the format of the wire message for attestation payload.
This is the first step towards integrating with the easy-tee/attest repo, and is based roughly on @Ruteri 's attest-integration-idea branch.
This adds platform metadata from the to the attestation payload which the attester produces and the verifier checks, and adds a new variant to the
MeasurementRecordenum used by in the measurement policy for verification, which allows image hashes to be specified rather than a full set of measurements.When verifying, if we have a policy which specifies image hashes rather than a full set of measurements, we we compute expected measurements using attest-measure's
expected_dcap_registersfunction based on the platform metadata and image hashes.There are still some things missing from this PR, and currently some very similar types are duplicated between this repo's
attestationcrate andattest-types. There is a little bit of friction there with wanting to maintain compatibility with our current measurement policy JSON format (for the attestation type names) and some things which i just didn't get to yet. But potentially this can become a lot neater, and we can lean more on the functionality in the attest repo.A separate PR to add a cache for known GCP firmware has been merged into this branch: #57
In order to accommodate this change we had to drop support for:
Related issue: #40
Paired PR in
attest: Easy-TEE/attest#13TODO:
attestationandattest. The issue is representing the NoAttestation case. Potentially we can useOption. Also potentially an issue with adding Nitro support in Add support for AWS Nitro #45VersionedAttestationformat. Or at least document that these will not work with 'portable' measurement policies due to the missing platform metadata.expected_dcap_registers.