Remote Attestation Refactoring

[jul-sh]

There's some opportunity for refactoring our remote attestation implementation, this issue is meant to track aspects that could be improved. It's worth noting that we currently support remote attestation over both unary and bidi-streaming gRPC. We plan to remove support for attestations over bidi-streaming gRPC in the future. It'll be easiest to tackle these issues once that has happened.

Probable refactoring opportunities:

• Overlap between the Handshaker & Encryptor struct, both of which can represent the same state. See discussion on Implement Server Side Unary Remote Attestation #2596: (relevant comment)
• The Handshaker maintains duplicate handshake state in a transcript key and state enum, which are implicitly expected to have the same state. It'd be nice to deduplicate this, to increase resilience to future changes and prevent unexpected behavior.
• Shared configuration state is cloned into each Handshaker. Cloning shared state may lead to issues down the road when state changes may not be in all clones. It'd be nice to use references instead. Ref: Stop cloning additional_info into each ServerHandshaker #2574
• The handshaker exposes next_step method that optionally returns a next step (if not completed) and a is_completed method. They're expected to behave the same, if is_completed returns false next_step should return None. However with future changes that may not be the case, which would lead to infinite loops in our client implementation: link. It'd be nice to simplify the API of Handshaker in that regard. Ref Implement unary request client #2607
• Attestation clients expose an inconsistent send method, which expects a proto instance as an argument but returns a serialized proto. See [this chat thread for more details], and also this PR: Implement unary request client #2607

[jul-sh]

Issue 1 & 4 could be resolved relatively nicely by refactoring the signature of the handshaker.next_step. Currently it returns: anyhow::Result<Option<Vec<u8>>>. It uses a nested Result and Option, to effectively express an Enum of "in progress here's next message", "done, no message", "error".

Instead it could return an enum of "in progress, returns next message", "done, returns encryptor (consuming the handshaker)", "error".

[jul-sh]

The ideal structure would probably be:

For each client, the only state that is maintained on the server is a transcript of messages (equivalent of the current Transcript inside the handshakers, a Vec of bytes). When a new message is received, it is appended to the transcript. That transcript is then passed to a stateless function that derives verifies it and derives the next step (see suggested return signature in previous comment).

The benefits of this would be:

• Less state duplication leading to increased security. This is because there will be no more implicitly linked states that may not stay consistent.
• Simpler, easier for external verifiers to reason about
• Less items to keep in memory.

Would require substantial refactoring of existing code however. 🤷

[conradgrobler]

This has been superceded by newer implementation.