feat: Sign `TeeProof` in notary #477

piotr-roslaniec · 2025-02-17T18:19:06Z

Rebased over main with a bunch of squashed commits, hence the commit history may look weird (first few)
Creating TeeProof by re-using signing logic from Origo
Replacing "wire structs" with tokio::codec
Requested reviews for early draft
Requested reviews: 1
Created issues:
- chore: Refactor shared proof structures into web-prover-core crate #490
- feat: Implement JSON value matching for Manifest validation #491

Autoparallel

Left some feedback for you. Good work! I think that this cleaned up a lot.

I also think another review would be good from @mattes or @lonerapier perhaps to make sure I'm not missing anything.

book/book.toml

client/src/lib.rs

client/src/origo.rs

Autoparallel · 2025-02-18T15:06:15Z

notary/src/tee.rs


-  let response = ManifestResponse::from_payload(&response_header, &response_body)?;
-  debug!("{:?}", response);
+// TODO: Should TeeProof and other proofs be moved to `proofs` crate?


proofs crate is really for working with client-side-prover and doing NIVC folding proofs. So, no, not in my opinion.

We're going to pull that crate out of here eventually (probably)

Autoparallel · 2025-02-18T15:07:43Z

notary/src/tee.rs

-  let response = ManifestResponse::from_payload(&response_header, &response_body)?;
-  debug!("{:?}", response);
+// TODO: Should TeeProof and other proofs be moved to `proofs` crate?
+// Otherwise, adding TeeProof::manifest necessitates extra dependencies on the client


maybe we need something like a web-prover-core crate or something. I see what you mean. Given the way we want to abstract the proofs crate, we wouldn't want any of our specific manifest stuff in there anyway. Good comments

Created: #490

Autoparallel · 2025-02-18T15:11:38Z

notary/src/tee.rs


+/// Check if `manifest`, `request`, and `response` all fulfill requirements necessary for
+/// a proof to be created
+fn validate_notarization_legal(


in here we also need to check the JSON path exists and contains a value?

I think this is what should happen in <Request|Response>.is_subset_of. Currently, they match the existence of keys and not the value. I can address this one case in this PR but I'd rather start working on the "condition DSL" straight away instead:

web-prover/proofs/src/program/http.rs

Lines 188 to 195 in ae9adf2

// TODO: We are NOT checking the body contents yet! So this "DSL" is not supported:

// "body": {

// "json": [

// "hello"

// ],

// "contains": "world"

// }

Created: #491

proofs/src/program/manifest.rs

0xJepsen · 2025-02-18T20:05:59Z

notary/src/tee.rs

+  let to_sign = VerifyReply {
+    value:    format!("0x{}", hex::encode(manifest_hash)),
+    manifest: manifest.clone(),
+  };
+  let origo_sig = sign_verification(to_sign, State(state)).unwrap();
+  let as_bytes = hex::decode(&origo_sig.signature[2..]).unwrap();
+  let signature = hex::encode(&as_bytes);
+  Ok(TeeProof { data, signature })


This is pretty close to what I was using for verification in the contracts for the origo flow. Is there a change we could match it up? Essentially we return this object here:

web-prover/client/src/origo.rs

Line 35 in 586c8ab

pub struct SignedVerificationReply {

where the leaves are a 32byte hash of the value and a 32byte hash of the manifest, Then the digest is the root.

Should this be the same struct, SignedVerificationReply for some reason (compatibility between modes, onchain verification, etc)?

If we are able to keep it the same struct our on-chain logic will work with it which is nice. If we can use the same hashing and signing logic we used for the origo mode to populate the struct then it should work with the contract verification seamlessly

So we should have the Tee proof return a SignedVerificationReply and the Data

mattes · 2025-02-18T23:14:07Z

The wasm issue is resolved by merging latest main.

lonerapier · 2025-02-19T10:27:38Z

notary/src/origo.rs

-    .unwrap()
-    .insert(query.session_id.to_string(), VerifierInputs { request_messages, response_messages });
+  state.verifier_sessions.lock().unwrap().insert(query.session_id.to_string(), VerifierInputs {
+    request_messages:  request_messages.clone(),


why needing a clone here?

Good catch, it's not needed. Probably an artifact of some refactor I missed.

lonerapier · 2025-02-19T10:31:11Z

proofs/src/program/manifest.rs

@@ -82,6 +82,24 @@ pub struct Manifest {
  pub response: ManifestResponse,


do you think we can move manifest completely out of proofs into a web-prover-core as @Autoparallel mentioned, and just keep origo related inputs code in proofs crate?

maybe we can create a origoManifest on top of Manifest separately?

I think the separation between Origo and TEE/Proxy manifest makes sense.

Created: #490

piotr-roslaniec · 2025-02-19T10:51:29Z

Thanks everyone for looking into this. I received more feedback for the draft than I anticipated, so if there are no other comments (and the tests pass) we are good to merge.

piotr-roslaniec force-pushed the feat/sign-tee-proof branch 5 times, most recently from caf3c24 to ae9adf2 Compare February 18, 2025 13:50

piotr-roslaniec requested a review from Autoparallel February 18, 2025 14:05

Autoparallel reviewed Feb 18, 2025

View reviewed changes

piotr-roslaniec requested review from mattes and lonerapier February 18, 2025 16:04

piotr-roslaniec force-pushed the feat/sign-tee-proof branch 3 times, most recently from 49968a1 to e844774 Compare February 18, 2025 17:16

mattes approved these changes Feb 18, 2025

View reviewed changes

0xJepsen reviewed Feb 18, 2025

View reviewed changes

piotr-roslaniec force-pushed the feat/sign-tee-proof branch 3 times, most recently from 440ac27 to a9f74ca Compare February 18, 2025 21:14

devloper and others added 11 commits February 19, 2025 09:24

Quickly hack out some code for tee decrypt

b376718

(wip) check decrypted response with manifest

f526afd

(wip) parsing and validation requests

3eda228

(wip) clean up tests

51183b4

(wip) implementation draft

2606335

(wip) proxy verifies requests and responses

9ff22b6

(wip) refactoring origo signing

5a19de6

(wip) handle all cases when parsing app data in tee

2b12646

(wip) reuse origo signature

ec48246

(wip) use framed codec for notary-client comms

c2a96a0

(wip) apply pr suggestions

170fb96

piotr-roslaniec force-pushed the feat/sign-tee-proof branch from a9f74ca to 170fb96 Compare February 19, 2025 08:36

(wip) reuse origo mode signature

eee2f62

piotr-roslaniec force-pushed the feat/sign-tee-proof branch 2 times, most recently from ebc259b to f500c5d Compare February 19, 2025 10:34

lonerapier reviewed Feb 19, 2025

View reviewed changes

piotr-roslaniec mentioned this pull request Feb 19, 2025

chore: Refactor shared proof structures into web-prover-core crate #490

Open

(wip) use traits for deser

393b1d3

piotr-roslaniec force-pushed the feat/sign-tee-proof branch from f500c5d to 393b1d3 Compare February 19, 2025 10:45

piotr-roslaniec marked this pull request as ready for review February 19, 2025 10:46

piotr-roslaniec mentioned this pull request Feb 19, 2025

feat: Implement JSON value matching for Manifest validation #491

Open

piotr-roslaniec merged commit f0fc22c into main Feb 19, 2025
22 checks passed

piotr-roslaniec deleted the feat/sign-tee-proof branch February 19, 2025 17:43

0xJepsen mentioned this pull request Feb 19, 2025

chore: bump versions #494

Merged

piotr-roslaniec mentioned this pull request Feb 20, 2025

chore: Refactor into web-prover-core #499

Draft

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Sign `TeeProof` in notary #477

feat: Sign `TeeProof` in notary #477

piotr-roslaniec commented Feb 17, 2025 •

edited

Loading

Autoparallel left a comment

Autoparallel Feb 18, 2025

Autoparallel Feb 18, 2025

piotr-roslaniec Feb 19, 2025

Autoparallel Feb 18, 2025

piotr-roslaniec Feb 18, 2025

piotr-roslaniec Feb 19, 2025

0xJepsen Feb 18, 2025

piotr-roslaniec Feb 18, 2025

0xJepsen Feb 18, 2025

0xJepsen Feb 18, 2025

mattes commented Feb 18, 2025

lonerapier Feb 19, 2025

piotr-roslaniec Feb 19, 2025

lonerapier Feb 19, 2025

piotr-roslaniec Feb 19, 2025

piotr-roslaniec commented Feb 19, 2025

	// TODO: We are NOT checking the body contents yet! So this "DSL" is not supported:
	// "body": {
	// "json": [
	// "hello"
	// ],
	// "contains": "world"
	// }

		@@ -82,6 +82,24 @@ pub struct Manifest {
		pub response: ManifestResponse,

feat: Sign TeeProof in notary #477

feat: Sign TeeProof in notary #477

Conversation

piotr-roslaniec commented Feb 17, 2025 • edited Loading

Autoparallel 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

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

Choose a reason for hiding this comment

mattes commented Feb 18, 2025

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

piotr-roslaniec commented Feb 19, 2025

feat: Sign `TeeProof` in notary #477

feat: Sign `TeeProof` in notary #477

piotr-roslaniec commented Feb 17, 2025 •

edited

Loading