TSS attestation endpoint #1001

ameba23 · 2024-08-12T09:58:11Z

This is part of #982

This adds a HTTP endpoint /attest to the TS server.

With the unsafe feature enabled, this will create a mock TDX quote for testing the attestation logic on non-TDX hardware.

Without the unsafe feature enabled, this will currently just respond with an error. Eventually though, it will create an actual TDX quote.

The idea is that once there is a pallet handling attestation, this endpoint will be called by an offchain worker. The endpoint will create the quote and submit it to the chain in an extrinsic. That does not yet happen, as we don't yet have a pallet to handle this.

crates/threshold-signature-server/src/lib.rs

ameba23 · 2024-08-12T11:39:21Z

crates/shared/src/types.rs

@@ -99,3 +100,22 @@ pub enum HashingAlgorithm {

 /// A compressed, serialized [synedrion::ecdsa::VerifyingKey<k256::Secp256k1>]
 pub type EncodedVerifyingKey = [u8; VERIFICATION_KEY_LENGTH as usize];
+
+/// Input data to be included in a TDX attestation
+pub struct QuoteInputData(pub [u8; 64]);


This is in entropy-shared because it will also be used by the chain when verifying quote input

JesseAbram

just the two things but looks good

crates/threshold-signature-server/src/lib.rs

JesseAbram · 2024-08-12T14:58:36Z

crates/threshold-signature-server/src/lib.rs

@@ -178,6 +178,7 @@ pub fn app(app_state: AppState) -> Router {
        .route("/user/sign_tx", post(sign_tx))
        .route("/signer/proactive_refresh", post(proactive_refresh))
        .route("/validator/reshare", post(new_reshare))
+        .route("/attest", post(attest))


this should go lower into the unsafe endpoints

i can put it there for now but the idea is this same endpoint will be the real thing when unsafe feature is not present

idk we can always change it when we remove the unsafe, it makes sense to me but your call

FWIW I think it's fine as is

HCastano

Nice stuff 👍

HCastano · 2024-08-12T18:57:38Z

crates/threshold-signature-server/src/validator/tests.rs

+    assert_eq!(res.status(), 200);
+    let quote = res.bytes().await.unwrap();
+    // This verifies the signature in the quote
+    tdx_quote::Quote::from_bytes(&quote).unwrap();


Instead of unwrap-ing here we may want to check that the value we get back matches what we expect

crates/threshold-signature-server/src/validator/api.rs

HCastano · 2024-08-12T19:01:25Z

crates/threshold-signature-server/src/validator/api.rs

+    input: Bytes,
+) -> Result<(StatusCode, Bytes), ValidatorErr> {
+    // TODO #982 confirm with the chain that an attestation should be happenning
+    let nonce = input[..].try_into()?;


Suggested change

let nonce = input[..].try_into()?;

let nonce = input.as_slice().try_into()?;

i went for as_ref as as_slice doesn't seem to be available for Bytes

HCastano · 2024-08-12T19:02:28Z

crates/threshold-signature-server/Cargo.toml

@@ -83,6 +84,7 @@ ethers-core ="2.0.14"
 schnorrkel  ={ version="0.11.4", default-features=false, features=["std"] }
 schemars    ={ version="0.8.21" }
 subxt-signer="0.35.3"
+tdx-quote   ={ git="https://github.com/entropyxyz/tdx-quote", features=["mock"] }


Do we need to have this in both norm deps and dev deps?

the problem is if we don't, it wont compile in test mode without the unsafe feature present, which is the case in CI. Ideally i don't want tdx-quote with the mock feature available in production. but i do want it in a release build if the unsafe feature is present, in order to be able to test a release build on non-tdx hardware.

HCastano · 2024-08-12T19:08:36Z

crates/threshold-signature-server/src/validator/api.rs

+    let quote = tdx_quote::Quote::mock(signing_key.clone(), input_data.0);
+    // Here we would submit an attest extrinsic to the chain - but for now we just include it in the
+    // response
+    Ok((StatusCode::OK, Bytes::from(quote.as_bytes().to_vec())))


Just a small thing for the future - would be nice to have a From/Into implementation for turning these quotes into Bytes

I think thats only worth doing if we continue to include the quote in the http response. Initially i am proposing to only have on-chain quote verification, so the response body here will be empty and the quote will be included in an extrinsic instead. I want to keep the dependencies of tdx-quote as minimal as possible as its used in a pallet.

But if we decide to do quote verification by other TS servers, then we will want this.

HCastano · 2024-08-12T19:09:38Z

crates/threshold-signature-server/src/lib.rs

@@ -178,6 +178,7 @@ pub fn app(app_state: AppState) -> Router {
        .route("/user/sign_tx", post(sign_tx))
        .route("/signer/proactive_refresh", post(proactive_refresh))
        .route("/validator/reshare", post(new_reshare))
+        .route("/attest", post(attest))


FWIW I think it's fine as is

* master: TSS attestation endpoint (#1001)

Add attest endpoint

11f4b39

ameba23 marked this pull request as draft August 12, 2024 09:58

ameba23 added 8 commits August 12, 2024 12:11

Mv QuoteInputData to entropy-shared

f795f69

Add tdx_quote to deny.toml

5881b0e

Improve test for mock attestation

5c73ab4

Bump tdx-quote

10ee387

Rename variable

3b9d337

Changelog

3fb58a1

Changelog

fe60c04

Comment

bebc2f8

ameba23 commented Aug 12, 2024

View reviewed changes

crates/threshold-signature-server/src/lib.rs Outdated Show resolved Hide resolved

ameba23 commented Aug 12, 2024

View reviewed changes

Use mock attest fn if cfg(test)

f9afd5f

ameba23 marked this pull request as ready for review August 12, 2024 14:30

ameba23 requested review from HCastano and JesseAbram and removed request for HCastano August 12, 2024 14:30

JesseAbram approved these changes Aug 12, 2024

View reviewed changes

HCastano approved these changes Aug 12, 2024

View reviewed changes

ameba23 added 4 commits August 13, 2024 08:31

Small edits following review

1b7309d

Doccomments

e03014e

Improve test for mock attestation

2f0e2ae

Mv attestation stuff to attestation module

30f67d3

ameba23 merged commit fccd8a3 into master Aug 13, 2024
14 checks passed

ameba23 deleted the peg/attest-endpoint branch August 13, 2024 09:46

ameba23 added a commit that referenced this pull request Aug 13, 2024

Merge branch 'master' into peg/attestation-pallet

c5df03f

* master: TSS attestation endpoint (#1001)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TSS attestation endpoint #1001

TSS attestation endpoint #1001

ameba23 commented Aug 12, 2024 •

edited

Loading

ameba23 Aug 12, 2024

JesseAbram left a comment

JesseAbram Aug 12, 2024

ameba23 Aug 12, 2024

JesseAbram Aug 12, 2024

HCastano Aug 12, 2024

HCastano left a comment

HCastano Aug 12, 2024

HCastano Aug 12, 2024

ameba23 Aug 13, 2024 •

edited

Loading

HCastano Aug 12, 2024

ameba23 Aug 13, 2024 •

edited

Loading

HCastano Aug 12, 2024

ameba23 Aug 13, 2024

HCastano Aug 12, 2024

	let nonce = input[..].try_into()?;
	let nonce = input.as_slice().try_into()?;

TSS attestation endpoint #1001

TSS attestation endpoint #1001

Conversation

ameba23 commented Aug 12, 2024 • edited Loading

Choose a reason for hiding this comment

JesseAbram 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

HCastano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ameba23 Aug 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ameba23 Aug 13, 2024 • edited Loading

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

ameba23 commented Aug 12, 2024 •

edited

Loading

ameba23 Aug 13, 2024 •

edited

Loading

ameba23 Aug 13, 2024 •

edited

Loading