runtime/src/enclave_rpc: Verify RPC quotes with key manager quote policy #5092
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
peternose
force-pushed
the
peternose/feature/quote-policy
branch
3 times, most recently
from
3ac1830
to
28b322f
Compare
peternose
requested review from
kostko,
Yawning,
peterjgilbert,
pro-wh and
ptrus
as code owners
kostko
reviewed
Dec 9, 2022
Every deployment has its own quote policy which cannot change while the runtime is running. Fetching the policy on every attestation is time consuming and can be done only once, even before the protocol is fully initialized.
Verify that the policy belongs to the key manager and that it has been published in the consensus layer.
When multiple key managers were running, the last known status of the runtime's key manager was overwritten with each status update. On runtime (re)starts, this resulted in the wrong policy being set.
peternose
force-pushed
the
peternose/feature/quote-policy
branch
from
28b322f
to
52abc7e
Compare
Codecov Report
@@ Coverage Diff @@
## master #5092 +/- ##
==========================================
+ Coverage 67.18% 67.45% +0.26%
==========================================
Files 501 501
Lines 53381 53459 +78
==========================================
+ Hits 35864 36059 +195
+ Misses 13163 13074 -89
+ Partials 4354 4326 -28
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
kostko
approved these changes
Dec 12, 2022
| @@ -321,6 +325,9 @@ pub struct Features { | |||
| /// Schedule control feature. | |||
| #[cbor(optional)] | |||
| pub schedule_control: Option<FeatureScheduleControl>, | |||
| /// A feature specifying that the runtime supports updating key manager's quote policy. | |||
| #[cbor(optional)] | |||
| pub key_manager_quote_policy_updates: bool, | |||
There was a problem hiding this comment.
We should probably implement a custom Default that sets this to true.
peternose
commented
Dec 12, 2022
| stCh, stSub := n.consensus.KeyManager().WatchStatuses() | ||
| defer stSub.Close() | ||
|
|
||
| // Subscribe to runtime host events. | ||
| // Subscribe to epoch transitions (quote policy might change). | ||
| epoCh, sub, err := n.consensus.Beacon().WatchEpochs(ctx) |
There was a problem hiding this comment.
@kostko As key managers have different upgrade procedure as compute runtimes, changing the quote policy on epoch transitions might not be ok as every key manager can run its own version (which might not be the active one, can be higher or lower). I see few options:
- Keep the way it is and expect key manager upgrades to follow versioning.
- Follow versions of key manager nodes and use the latest one.
- Use quote policy of the key manager we are talking to (we would probably need to fetch the quote policy during session initialization, as we cannot know to which key manager the p2p stack will send the request).
There was a problem hiding this comment.
Hm good point. I think using the active deployment for the quote policy makes the most sense.
peternose
force-pushed
the
peternose/feature/quote-policy
branch
from
52abc7e
to
d9ed76b
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently EnclaveRPC session establishment always uses the default quote verification policy to verify quotes. When the runtime is configured with a consensus layer trust root it could instead use the quote verification policy specified in the consensus layer for the key manager runtime.
How things work: