feat(dcap): Impl DCAP RA types/handlers #139
Conversation
hu55a1n1
changed the title
There was a problem hiding this comment.
Without having a solid understanding of how DCAP works vs EPID, I can at least say this all seems good from a Rust perspective. Is there a particular part of the code whose logic you'd like someone to double-check?
Otherwise just some minor requests for documentation in the code, and a bunch of questions to help me understand what's going on.
| #[derive(Clone, PartialEq, Debug, Default)] | ||
| pub struct DcapAttestor; |
There was a problem hiding this comment.
I'd recommend adding docstrings for this type. Not sure in which PR(s) to do it, but I'd also recommend adding some docstrings for the Attestor trait and its methods.
| #[derive(Clone, Debug, PartialEq, Serialize)] | ||
| pub struct DcapAttestation { |
There was a problem hiding this comment.
Docstring would be helpful here
| use serde::Serialize; | ||
|
|
||
| pub type Quote = Quote3<Vec<u8>>; |
There was a problem hiding this comment.
Docstring explaining what this is would be helpful.
| #[derive(Clone, PartialEq, Debug, Default)] | ||
| pub struct DcapAttestor; | ||
|
|
||
| impl Attestor for DcapAttestor { |
There was a problem hiding this comment.
The logic in this impl looks exactly the same as the EpidAttestor's - is that right?
There was a problem hiding this comment.
Yes! I was thinking about using an alias, but I thought I should have duplicate code for now until we verify it works correctly (hopefully in a subsequent PR that makes DCAP attestation the default). Also I think we should remove EPID at some point.
Thank you for the review @thanethomson! This is more like a preparatory PR, so the newly added code isn;t being used at all. I will open a separate PR for resolving #70. I think we can review the logic then. |
Resolves: #38
Preps for: #70
What's in this PR
This PR defines quartz lib types and handlers for DCAP attestation.
The way this resolves #38 is by removing the
ReportDataHashVerifieraltogether, so the expectation is that users will verify the report data out-of-band. This approach is consistent with our EPID RA verification impl.Note for reviewers
I will make DCAP the default impl in a follow-up PR. Will also add tests to make sure the attestations created by our enclave work with our on-chain verifiers in that PR. So for now, there's nothing to test.