runtime: Add client node TEE freshness verification #4922

peternose · 2022-09-05T09:40:48Z

Add client node TEE freshness verification

Executor and key manager TEE nodes currently use node registration (which includes the randomly generated Runtime Attestation Key and nonce) in order to establish consensus layer view freshness.

But this approach doesn’t work for client nodes. Since client nodes do not currently register, there is no way for TEE runtimes running on them to verify consensus layer freshness. There is however a need for client nodes to be able to execute signed queries and requiring an up-to-date consensus layer view is crucial for verifying quotes and policies.

To do this the runtime just needs to generate a transaction containing a nonce and that transaction must be published in a block, with the host node providing the height at which the transaction was published, its index and Merkle proof of inclusion. The runtime can then use the light client to verify the specified block header and the provided Merkle proof using the verified transaction root hash.

The newly added registry.ProveFreshness transaction that accepts a fixed-size binary blob of 32 bytes and always succeeds without doing any processing should be used for this purpose. The runtime host protocol should be extended to support this new flow, initiated by the consensus verifier in the runtime (immediately after initializing the light client in case enabled in the static runtime config), e.g. adding the HostProveFreshness{Request,Response} messages.

Note that this check can be used very early on as it doesn’t require the node to be registered.

Test

Unhappy path tested locally. Happy path already included in trust root tests.

kostko

Just left some quick comments.

go/runtime/host/protocol/types.go

go/runtime/registry/host.go

runtime/src/consensus/tendermint/merkle.rs

go/runtime/registry/handlers.go

runtime/src/consensus/tendermint/verifier.rs

runtime/src/consensus/transaction.rs

codecov · 2022-09-07T07:20:39Z

Codecov Report

Merging #4922 (d80da34) into master (10925b2) will decrease coverage by 0.01%.
The diff coverage is 69.94%.

@@            Coverage Diff             @@
##           master    #4922      +/-   ##
==========================================
- Coverage   66.71%   66.70%   -0.02%     
==========================================
  Files         464      464              
  Lines       51127    51203      +76     
==========================================
+ Hits        34110    34155      +45     
- Misses      12836    12867      +31     
  Partials     4181     4181

Impacted Files	Coverage Δ
go/consensus/api/api.go	`69.23% <ø> (ø)`
go/consensus/api/grpc.go	`64.64% <0.00%> (-2.90%)`	⬇️
go/consensus/api/transaction/transaction.go	`44.85% <ø> (ø)`
go/consensus/tendermint/full/common.go	`66.10% <0.00%> (-1.29%)`	⬇️
go/consensus/tendermint/seed/seed.go	`64.70% <0.00%> (-0.65%)`	⬇️
go/runtime/host/protocol/types.go	`54.54% <ø> (ø)`
go/runtime/registry/host.go	`68.57% <75.32%> (+0.15%)`	⬆️
go/consensus/tendermint/full/full.go	`65.43% <75.75%> (+0.39%)`	⬆️
go/consensus/api/submission.go	`68.42% <83.33%> (+7.55%)`	⬆️
go/runtime/host/sandbox/sandbox.go	`70.00% <83.33%> (-7.82%)`	⬇️
... and 36 more

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

kostko

Looks good (pending on #4904), some minor comments.

kostko · 2022-09-08T08:39:29Z

go/consensus/api/submission.go

@@ -63,7 +63,14 @@ type SubmissionManager interface {
 	// with the passed signer and submits it to consensus backend.
 	//
 	// It also automatically handles retries in case the nonce was incorrectly estimated.
-	SignAndSubmitTx(ctx context.Context, signer signature.Signer, tx *transaction.Transaction) error
+	SignAndSubmitTx(ctx context.Context, signer signature.Signer, tx *transaction.Transaction) (*transaction.SignedTransaction, error)


Is there a need now for SignAndSubmitTx to also return the transaction or is having this only in SignAndSubmitTxWithProof enough (this would reduce the amount of changes required)?

No, currently there is no need. I just wanted to be consistent with SignAndSubmitTxWithProof. It is actually interesting that we never need the signed transaction, neither the height at which it was published.
I can remove it if you like.

I would remove it.

Removed and rebased.

go/runtime/host/sandbox/sandbox.go

kostko · 2022-09-12T07:02:57Z

Rebase on master now that #4904 is merged.

go/consensus/api/submission.go

runtime/src/consensus/tendermint/merkle.rs

kostko · 2022-09-12T09:08:35Z

runtime/src/consensus/tendermint/merkle.rs

+            return None;
+        }
+        match total {
+            0 => panic!("cannot call compute_hash_from_aunts() with 0 total"),


Given that this is checked above in which case there is an early return this could easily be changed to an unreachable! (with a comment that this is handled above) to make this explicit.

Agree, unreachable is better. However, I didn't want to remove this as I wanted to be consistent with GO version.

kostko · 2022-09-12T09:09:44Z

runtime/src/consensus/tendermint/merkle.rs

+        index: i64,
+        total: i64,
+        leaf_hash: Hash,
+        inner_hashes: &Vec<Hash>,


Could this be changed to &[Hash] to avoid conversions to vec?

Yes, much better.

peternose force-pushed the peternose/feature/client-freshness-verification branch 3 times, most recently from 63ce5f0 to 6e537f9 Compare September 5, 2022 11:46

kostko reviewed Sep 5, 2022

View reviewed changes

runtime/src/consensus/tendermint/verifier.rs Outdated Show resolved Hide resolved

runtime/src/consensus/transaction.rs Outdated Show resolved Hide resolved

peternose force-pushed the peternose/feature/client-freshness-verification branch 2 times, most recently from babd6f1 to d727058 Compare September 6, 2022 22:13

peternose changed the title ~~Add client node TEE freshness verification~~ runtime: Add client node TEE freshness verification Sep 6, 2022

peternose force-pushed the peternose/feature/client-freshness-verification branch from d727058 to 5fcedbc Compare September 7, 2022 07:22

peternose marked this pull request as ready for review September 7, 2022 09:46

peternose requested review from Yawning, peterjgilbert, pro-wh and ptrus as code owners September 7, 2022 09:46

peternose requested a review from kostko September 7, 2022 09:46

kostko reviewed Sep 8, 2022

View reviewed changes

peternose force-pushed the peternose/feature/client-freshness-verification branch from 5fcedbc to a051f6d Compare September 9, 2022 06:32

kostko mentioned this pull request Sep 9, 2022

Bind TEE attestations to nodes and enforce freshness #4926

Merged

peternose force-pushed the peternose/feature/client-freshness-verification branch from a051f6d to ab96348 Compare September 12, 2022 07:25

peternose requested a review from kostko September 12, 2022 08:08

kostko reviewed Sep 12, 2022

View reviewed changes

peternose force-pushed the peternose/feature/client-freshness-verification branch 2 times, most recently from 07a95b3 to e729284 Compare September 12, 2022 13:23

kostko approved these changes Sep 12, 2022

View reviewed changes

peternose added 3 commits September 12, 2022 15:34

runtime/consensus/verifier: Refactor verify state freshness

9a48162

runtime/consensus: Add transaction structs

8e7db1d

runtime/consensus/tendermint: Add support for Merkle proofs

435e3e0

peternose added 3 commits September 12, 2022 15:34

go/runtime/registry: Refactor RHP handler

5da9605

runtime: Add client node TEE freshness verification

fa52dea

go/runtime/host/sandbox: Fix runtime restart on termination

d80da34

peternose force-pushed the peternose/feature/client-freshness-verification branch from e729284 to d80da34 Compare September 12, 2022 13:36

peternose merged commit f2e760b into master Sep 12, 2022

peternose deleted the peternose/feature/client-freshness-verification branch September 12, 2022 15:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

runtime: Add client node TEE freshness verification #4922

runtime: Add client node TEE freshness verification #4922

peternose commented Sep 5, 2022 •

edited

Loading

kostko left a comment

codecov bot commented Sep 7, 2022 •

edited

Loading

kostko left a comment

kostko Sep 8, 2022

peternose Sep 8, 2022

kostko Sep 9, 2022

peternose Sep 12, 2022

kostko commented Sep 12, 2022

kostko Sep 12, 2022

peternose Sep 12, 2022

kostko Sep 12, 2022

peternose Sep 12, 2022

runtime: Add client node TEE freshness verification #4922

runtime: Add client node TEE freshness verification #4922

Conversation

peternose commented Sep 5, 2022 • edited Loading

Add client node TEE freshness verification

Test

kostko left a comment

Choose a reason for hiding this comment

codecov bot commented Sep 7, 2022 • edited Loading

Codecov Report

kostko left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kostko commented Sep 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

peternose commented Sep 5, 2022 •

edited

Loading

codecov bot commented Sep 7, 2022 •

edited

Loading