-
Notifications
You must be signed in to change notification settings - Fork 0
Add support for Microsoft Azure Attestation (MAA) #19
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
Conversation
Verify azure attestation locally - not using MAA API
… into peg/add-maa * 'peg/add-maa' of github.com:flashbots/attested-tls-proxy: Clippy Get AK certificate from vTPM Add NV index reader (for reading AK certificate) Verify azure attestation locally - not using MAA API
* peg/dummy-attestation: Add dummy attestation server/client
* main: Add section to README with CLI differences to cvm-reverse-proxy
| @@ -0,0 +1,99 @@ | |||
| //! Data Center Attestation Primitives (DCAP) evidence generation and verification | |||
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.
This is refactored from /src/attestation/mod.rs
| /// Allows any EKU - we could change this to only accept 1.3.6.1.4.1.567.10.3.12 which is the EKU | ||
| /// given in the AK certificate | ||
| struct AnyEku; | ||
|
|
||
| impl webpki::ExtendedKeyUsageValidator for AnyEku { | ||
| fn validate(&self, _iter: webpki::KeyPurposeIdIter<'_, '_>) -> Result<(), webpki::Error> { | ||
| 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.
Not sure about the security implications here. Is this variable or just a fixed value for the case of Azure vTPMs? if the latter, maybe we should consider also fixing it like the certificates above
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 will look into this a bit more. There are two EKUs in the AK certificates that im getting, and i expect that at least one of them is specific to these type of certificates and probably worth checking for. Will check the azure docs.
I wanted to already add a check here but i am having issues with the OID encoding. It seems to work a bit differently that with OIDs used in certificate extensions.
src/attestation/azure/mod.rs
Outdated
| expected_input_data: [u8; 64], | ||
| pccs_url: Option<String>, | ||
| now: u64, | ||
| ) -> Result<super::measurements::Measurements, MaaError> { |
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.
Should this function return an Error & the expected measurements or just an error?
| ak_certificate_pem: String, | ||
| /// vTPM quote | ||
| quote: vtpm::Quote, | ||
| /// Raw TCG event log bytes (UEFI + IMA) [currently not used] |
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.
Is this worth reading? What would we do with it when verifying?
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.
Maybe 🤔
Because Azure v6 instances will be using the openHCL paravisor that should probably allow replaying the events reproducibly. But the question here is: is this raw TCG event log vector being signed by either the AK or being part of the TD quote input data? There should be a way to bind it to the whole flow if you want to make use of it. wdyt?
| let (remaining_bytes, ak_certificate) = X509Certificate::from_der(&ak_certificate_der)?; | ||
|
|
||
| // Check that AK public key matches that from TPM quote and HCL claims | ||
| let ak_from_certificate = RsaPubKey::from_certificate(&ak_certificate)?; |
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.
Can we safely assume the AK will always be RSA? I was having difficulties comparing public keys in an algorithm-agnostic way - so this will now only work with RSA.
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 question. Since this only targets azure's vTPM attestation, we could fix it to RSA since it seems that this is the algorithm used for their TPM AKs. But you raise a good point in case a patch happens and changes happen in the future.
Maybe as a follow-up PR, one could consider a way to identify the used algorithm from the certificate before extracting it? wdyt?
For azure, measurments should be PCRs, not registers from TDX quote
|
|
||
| // Do DCAP verification | ||
| let tdx_quote_bytes = BASE64_URL_SAFE.decode(attestation_document.tdx_quote_base64)?; | ||
| let _dcap_measurements = |
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.
If i understood right, coping the behavior from cvm-reverse-proxy, we can ignore these and only check the PCR hash values, as these are what are determined by our image.
Closes #16
This adds support for TDX attestation generation and verification on Microsoft Azure
It includes reading and verifying the Attestation Key certificate from the vTPM, ported from flashbots/cvm-reverse-proxy#47 from @MoeMahhouk.
TODO: