Replace VerificationReport With New Evidence Types #3602

dolanbernard · 2023-10-04T19:57:59Z

Implement client-side handling of both old and new attestation evidence types
Replace VerificationReport with EvidenceKind/EvidenceMessage
Add placeholder logic in a few places to be replaced with server dcap upgrade

Motivation

Clients will be upgraded to 6.0 before servers. MobileCoin 5.0 used EPID verification and 6.0 uses DCAP. Because of this, clients need to be updated to handle both EPID and DCAP verification.

This PR also replaces the rigid VerificationReport type with the flexible EvidenceKind. Server code has been updated to assume this is the Epid variant for now. This placeholder code needs to be there for builds to work, but it will never run as it is being replaced with dcap verification.

Future Work

Server side dcap verification

Decoding an empty EvidenceMessage will fail already, so pass up EvidenceKind because we don't need to handle a None value anywhere else after it has been successfully decoded as Some.

connection/src/thick.rs

attest/ake/src/lib.rs

attest/ake/src/initiator.rs

fog/enclave_connection/src/error.rs

connection/src/thick.rs

fog/ledger/connection/src/router_client.rs

eranrund

There's a lot to digest here, so I made a first pass but I'll probably have more questions and will need to defer a lot to @nick-mobilecoin since he's much more familiar with this than I am.

My biggest confusion right now is that there seems to be places where DCAP is explicitly handled, places where it is skipped, and some confusing (to me) places that sometimes pass along serialized VerificationReport bytes, sometimes DcapEvidence, and try to deserialize either both or just one...

attest/ake/src/event.rs

attest/ake/src/initiator.rs

attest/ake/src/event.rs

fog/enclave_connection/Cargo.toml

fog/ingest/server/src/controller.rs

fog/ledger/connection/Cargo.toml

fog/view/connection/Cargo.toml

sgx/report-cache/untrusted/src/lib.rs

nick-mobilecoin

I realize Eran got comments in first so there may be some duplicates hopefully it's not too bad.

attest/ake/Cargo.toml

attest/ake/src/event.rs

attest/ake/src/initiator.rs

attest/ake/src/responder.rs

connection/src/thick.rs

Co-authored-by: Nick Santana <nick@mobilecoin.com> Co-authored-by: Eran Rundstein <eran@rundste.in>

Refactor dual verification logic and fix failing test

Co-authored-by: Nick Santana <nick@mobilecoin.com>

nick-mobilecoin

LGTM other than it should be comparing the remote_id to the report_data in the dcap verification

attest/ake/src/initiator.rs

Co-authored-by: Nick Santana <nick@mobilecoin.com>

eranrund

This is much cleaner, thank you!
I think the last remaining piece from my perspective is that maybe we should change APIs that are in-enclave-only to just return either the Epid or Dcap struct instead of the enum, since if I understood things correctly, there isn't ever a case where the actual enclave implementation supports both...

attest/ake/src/responder.rs

consensus/service/src/byzantine_ledger/metadata_provider.rs

crypto/ake/enclave/src/lib.rs

fog/ingest/server/src/controller.rs

eranrund · 2023-10-13T17:35:55Z

Gonna do another review pass and in the meanwhile I triggered CD deployment for this (https://github.com/mobilecoinfoundation/mobilecoin/actions/runs/6511334658) to further verify everything still works as expected.

nick-mobilecoin · 2023-10-13T17:52:06Z

Gonna do another review pass and in the meanwhile I triggered CD deployment for this (https://github.com/mobilecoinfoundation/mobilecoin/actions/runs/6511334658) to further verify everything still works as expected.

I don't think CD will work right now, we're depending on some dcap libraries, but the docker run container is the minimal container for EPID. I've crated a dcap run container for future DCAP CD.

eranrund

CD will be handled in a followup PR, so LGTM.

dolanbernard added 12 commits September 25, 2023 18:57

Rebase changes onto new branch

855d4c2

Update for DcapEvidence prost changes

851b4b3

Move time to AuthResponseInput

71cef28

Update clients

f54c8e4

Pass up EvidenceKind instead of EvidenceMessage

fd87bad

Decoding an empty EvidenceMessage will fail already, so pass up EvidenceKind because we don't need to handle a None value anywhere else after it has been successfully decoded as Some.

Replace VerificationReport with EvidenceKind

17d41aa

Replace VerificationReport with EvidenceKind

e4b70e7

Merge branch 'master' into client-evidence-decode2

2fed6aa

Lint

0eb37ea

Lint + build fix from merge

67b3224

Merge branch 'master' into client-evidence-decode2

1e34a63

Merge build errors

d060b26

nick-mobilecoin mentioned this pull request Oct 7, 2023

Update watcher to handle EvidenceKind #3608

Merged

dolanbernard added 9 commits October 10, 2023 03:16

Decode prost::DcapEvidence

f100597

Fix return type

4da0106

Update responder.rs

fb8fa97

Encode/Decode prost::DcapEvidence

59d514f

Lint

e51dcdc

Merge branch 'master' into client-evidence-decode2

327804b

Replace placeholder logic

b1d3888

Responder should decode VerificationReport

fce88b2

Throw correct error for verification failure

b4b9312