Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/runtime/registry: Fix key manager (quote) policy updates #5111

Merged
merged 1 commit into from
Dec 27, 2022

Conversation

peternose
Copy link
Contributor

Problems

  • Key manager (quote) policy updates don't work as expected as the consensus verifier will reject policy updates as his view of the state is one block behind the host. This is Tendermint specific and we should retry the update if it fails.
  • Runtime info was also not updated if the runtime upgraded to a new version. As a consequence the quote policy was never updated, even if the new version supported this feature.

Test

Retries can be tested by running .buildkite/scripts/test_e2e.sh -s=e2e.runtime.keymanager-upgrade on an SGX instance.

Before:

// Status update (reason: policy change)
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy mismatch","published":"SignedPolicySGX { policy: PolicySGX { serial: 1,...}}","untrusted":"SignedPolicySGX { policy: PolicySGX { serial: 2,...}}"}
{... "err":"policy hasn't been published","msg":"failed dispatching key manager policy update to runtime",...}
// ...
// Status update (reason: a new key manager node was added to the status)
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy update dispatched",...} // Policy changed!

After:

// Status update (reason: policy change)
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy mismatch","published":"SignedPolicySGX { policy: PolicySGX { serial: 1,...}}","untrusted":"SignedPolicySGX { policy: PolicySGX { serial: 2,...}}"}
{... "err":"policy hasn't been published","msg":"failed dispatching key manager policy update to runtime",...}
// Retry.
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy mismatch","published":"SignedPolicySGX { policy: PolicySGX { serial: 1,...}}","untrusted":"SignedPolicySGX { policy: PolicySGX { serial: 2,...}}"}
{... "err":"policy hasn't been published","msg":"failed dispatching key manager policy update to runtime",...}
// Retry.
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy update dispatched",...} // Policy changed!
// ...
// Status update (reason: a new key manager node was added to the status)
{... "msg":"got key manager policy update","policy":{"policy":{"serial":2,...}}"}
{... "msg":"key manager policy update dispatched",...} // Policy changed (no effect as policy was already up-to-date)!

When a key manager (quote) policy update fails, the host should retry the
update until the policy is updated. For example, when using Tendermint as
a backend service, the first update will always fail because the consensus
verifier sees new blocks with a one-block delay.
@peternose peternose marked this pull request as ready for review December 26, 2022 05:58
@codecov
Copy link

codecov bot commented Dec 26, 2022

Codecov Report

Merging #5111 (d438583) into master (0315a47) will increase coverage by 0.01%.
The diff coverage is 75.71%.

@@            Coverage Diff             @@
##           master    #5111      +/-   ##
==========================================
+ Coverage   66.79%   66.80%   +0.01%     
==========================================
  Files         497      497              
  Lines       53135    53149      +14     
==========================================
+ Hits        35491    35508      +17     
+ Misses      13320    13313       -7     
- Partials     4324     4328       +4     
Impacted Files Coverage Δ
go/common/sgx/common.go 66.01% <ø> (ø)
go/common/sgx/pcs/http.go 14.70% <ø> (ø)
go/runtime/enclaverpc/api/api.go 60.00% <ø> (ø)
go/runtime/host/protocol/types.go 54.54% <ø> (ø)
go/worker/keymanager/p2p/protocol.go 100.00% <ø> (ø)
go/common/sgx/pcs/quote.go 55.70% <14.28%> (-0.97%) ⬇️
go/common/sgx/pcs/tcb.go 50.40% <25.00%> (-3.59%) ⬇️
go/worker/keymanager/worker.go 65.82% <69.56%> (-0.21%) ⬇️
go/runtime/registry/host.go 72.35% <97.05%> (+0.78%) ⬆️
go/worker/common/committee/keymanager.go 88.70% <100.00%> (+0.18%) ⬆️
... and 34 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@peternose peternose merged commit 6def98b into master Dec 27, 2022
@peternose peternose deleted the peternose/bugfix/km-policy-updates branch December 27, 2022 09:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants