keymanager: Verify that policy was published in the consensus layer

[peternose]

Force on-chain publication of key manager policies

When the key manager policy is updated, the runtime should check whether this policy has actually been published in the consensus layer by using the light client verifier.

This will require adding the Rust implementation of the consensus state accessor for the key manager service. Then one can fetch the corresponding key manager Status and check that the correct policy has been published there.

Test

Happy path is already tested in trust-root/simple scenario. Unhappy path tested locally, the key manager doesn't update policy and constantly throws the following error:
{"caller":"worker.go:737","err":"worker/keymanager: failed to extract rpc response payload: rpc failure: 'policy hasn't been published'","level":"error","module":"worker/keymanager","msg":"failed to handle status update","ts":"2022-09-08T22:56:44.214990354Z"}

[codecov]

Codecov Report

Merging #4925 (10925b2) into master (fc568b0) will decrease coverage by 0.04%.
The diff coverage is 46.75%.

@@            Coverage Diff             @@
##           master    #4925      +/-   ##
==========================================
- Coverage   66.76%   66.71%   -0.05%     
==========================================
  Files         464      464              
  Lines       51127    51127              
==========================================
- Hits        34133    34110      -23     
- Misses      12815    12836      +21     
- Partials     4179     4181       +2     

Impacted Files	Coverage Δ
go/common/version/version.go	80.26% <ø> (ø)
go/consensus/api/grpc.go	67.54% <0.00%> (ø)
go/consensus/tendermint/light/client.go	46.60% <0.00%> (ø)
go/consensus/tendermint/seed/seed.go	65.34% <0.00%> (ø)
go/runtime/host/protocol/types.go	54.54% <ø> (ø)
go/consensus/tendermint/full/light.go	55.71% <38.88%> (-12.86%)	⬇️
go/runtime/registry/host.go	68.42% <56.14%> (-0.33%)	⬇️
go/worker/client/committee/node.go	75.48% <100.00%> (ø)
go/consensus/tendermint/abci/state/state.go	61.53% <0.00%> (-7.70%)	⬇️
go/consensus/tendermint/apps/staking/auth.go	62.96% <0.00%> (-7.41%)	⬇️
... and 31 more

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

[kostko]

This (and maybe even the verify method) could even return the policy.policy so upstream callers can do something like let policy = verify_policy_and_trusted_signers(untrusted_policy)?; which seems to be common now.

[peternose]

I applied your comment. Policy is now returned (just as it was before this PR).

I intentionally wanted to removed this for 4 reasons:

• The verify method had two responsibilities, as can be seen from the comment /// Verify the signatures and return the PolicySGX.
• I expect that verify returns true/false or ()/Err, and not a policy. However, I expect sign to return a signed policy.
• Policy had to be cloned, to achieve that Ok(self.policy.clone()). Unnecessary step.
• Passing policy back and forth just doesn't feel right as it can always be accessed via SignedPolicySGX. It would be nice if this would give us some kind of protection, but it doesn't. For example, if policy was private and could be accessed only via getter function verify, then everyone would be forced to call verify.

The only reason for is to have more readable code let policy = verify_policy....

[kostko]

There should be no need to clone a policy, you should be able to do something like:

pub fn verify<'a>(policy: &'a SignedPolicySGX) -> Result<&'a PolicySGX, Error> {

and then return a reference, e.g. &policy.policy.

For example, if policy was private and could be accessed only via getter function verify, then everyone would be forced to call verify.

Yeah that would be ideal but not sure how it would work with deserialization.