Create common measurement types for attestation #3369
Conversation
|
Current dependencies on/for this PR:
This comment was auto-generated by Graphite. |
…urementSet` The underlying implementation of `mc-attest-verifier-config::TrustedMeasurementSet` has been changed to no longer support a dedicated config advisory entry. The creation of a verifier from the `TrustedMeasurementSet` has also been removed. `mc-attest-verifier-config::StatusVerifierConfig` has been renamed to `mc-attest-verifier-config::TrustedMeasurement`
nick-mobilecoin
force-pushed
the
nick/trusted-measurements
branch
from
28113eb to
0ef12e9
Compare
|
LGTM. I don't think anything actually uses this. @cbeck88 as the original author, do you know if this ever got used? |
|
Nothing is actually using it yet, it's fine to change it |
|
Do you want to include the expected result enum for the "StatusVerifier" which you added in that other PR? Or is that not going to become a part of the config here? |
Are you talking about, https://github.com/mobilecoinfoundation/attestation/blob/945015c9cf6510766fddf8ecfe383cd5e29fc161/verifier/src/advisories.rs#LL23C26-L23C26 |
Also utilize `assert_matches!` instead of `assert!(matches!(`. `assert_matches!` outputs the actual value in failure cases while `assert!(matches!(` only communicates the line that the assert occured at.
|
Later is fine, just trying to understand the direction. It's not clear to me right now if we need to supply that enum as an actual configuration option, or if we can just always configure it to expect "up-to-date". |
|
I just ran across, mobilecoin/attest/verifier/src/status.rs Line 25 in 3c7891c It looks like EPID behaves similar to DCAP there and our interface keeps the advisory types separate. Looking at the possible statuses,
We probably only want to allow the top 4, thus the separation logic between hardening and config. I think I'll leave this PR as it is for now and see where things progress to. |
Add back the mitigated config advisories for the MRENCLAVE and MRSIGNER trusted measurements. Previously the desire was to have a list of advisories with a flag indicating what status was acceptable based on the flags at <https://api.portal.trustedservices.intel.com/documentation#pcs-tcb-info-model-v3>. After some thought, changing here would require consumers to change from their current use of 2 lists for each advisory type, while not providing any benefit to said consumers. The current logic of combining what's in the config and hardening advisories for the `ConfigurationAndSWHardeningNeeded` state will suffice.
|
After some thought I decided to bring back the config advisories, I think dropping them and adding a status flag causes unnecessary churn for consumers with no real benefit. |
nick-mobilecoin
changed the title
mc-attest-verifier-config::TrustedMeasurementSet
The `Evidence` wrapper now takes the `mc-sgx-dcap-types::Collateral`
instead of the `SignedTcbInfo` in the `new()` method. This is to allow
deriving the QE identity and other necessary fields from the
`Collateral`.
### Motivation
This is a first step in getting *all* of the data needed to verify a quote in one place. At this time it just replaces the tcb info.
The plan is to also add in getting the qe_identity from the collateral and verifying it.
#### Some things still being flushed out
Mbedtls works in the enclaves, but has been a thorn for other clients, so we want to make it easier for clients to provide alternative implementations. I'm calling that the `CertChainVerifier`. With that in mind, my current understanding of the inputs needed to the final verification object is:
```mermaid
flowchart LR
Q(Quote) --> E[Evidence]
C(Collateral) --> E
E --> V[Verifier]
T[Trust Roots] --> V
S[Time] --> V
CT[CertChainVerfier] --> V
M[Measurements] --> V
```
I think we can move the `Trust Roots` be internal to the `CertChainVerifier`. The `Time` is needed for verifying the `QeIdentity` and `TcbInfo`.
For the measurements, I'm thinking we'll want to move, or mimic, the new types from the MobileCoin repo into this repo, mobilecoinfoundation/mobilecoin#3369.
The `TrustedIdentity` is a way for clients to specify which application enclaves they trust. This brings in the types from mobilecoinfoundation/mobilecoin#3369 I renamed to `Identity` instead of `Measurement`, because there was some confusion in the types for mobilecoinfoundation/mobilecoin#3369 and the term measurement only seems to be used for the MRENCLAVE value in [ECDSA QuoteLibReference](https://download.01.org/intel-sgx/sgx-dcap/1.16/linux/docs/Intel_SGX_ECDSA_QuoteLibReference_DCAP_API.pdf). In the SGX SDK the term measurement is used for the MRENCLAVE or MRSIGNER hash value. [ECDSA QuoteLibReference](https://download.01.org/intel-sgx/sgx-dcap/1.16/linux/docs/Intel_SGX_ECDSA_QuoteLibReference_DCAP_API.pdf) "identity" is the term section 3.8 uses for MRENCLAVE or MRSIGNER + ISV PROD ID.
The underlying implementation of
The config advisories have been kept. After further investigation it feels like an unnecessary breaking change to remove them.mc-attest-verifier-config::TrustedMeasurementSethas been changed tono longer support a dedicated config advisory entry.
The creation of a verifier from the
TrustedMeasurementSethasbeen removed.
mc-attest-verifier-config::StatusVerifierConfighas been renamed tomc-attest-verifier-config::TrustedMeasurementMotivation
Preparation of a common type to use for trusted measurements. In order for clients to support EPID and DCAP attestation they need to lose knowledge of the verifier implementation and instead pass the trusted measurement values to functionality that can determine how to attest the enclave based on the format of the provided attestation evidence.
Future Work
Update the interfaces that are passing a verifier to pass this new trusted measurement type.