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

TSS attestation endpoint #1001

Merged
merged 14 commits into from
Aug 13, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ At the moment this project **does not** adhere to
- Reshare confirmation ([#965](https://github.com/entropyxyz/entropy-core/pull/965))
- Set inital signers ([#971](https://github.com/entropyxyz/entropy-core/pull/971))
- Add parent key threshold dynamically ([#974](https://github.com/entropyxyz/entropy-core/pull/974))
- TSS attestation endpoint ([#1001](https://github.com/entropyxyz/entropy-core/pull/1001)

### Changed
- Fix TSS `AccountId` keys in chainspec ([#993](https://github.com/entropyxyz/entropy-core/pull/993))
Expand Down
13 changes: 13 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions crates/shared/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ serde ={ version="1.0", default-features=false, features=["derive"] }
serde_derive="1.0.147"
strum ="0.26.3"
strum_macros="0.26.4"
blake2 ={ version="0.10.4", default-features=false }

sp-runtime ={ version="32.0.0", default-features=false, optional=true, features=["serde"] }
sp-std ={ version="12.0.0", default-features=false }
Expand Down
20 changes: 20 additions & 0 deletions crates/shared/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#![allow(dead_code)]
use super::constants::VERIFICATION_KEY_LENGTH;
use blake2::{Blake2b512, Digest};
#[cfg(not(feature = "wasm"))]
use codec::alloc::vec::Vec;
use codec::{Decode, Encode};
Expand Down Expand Up @@ -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]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


impl QuoteInputData {
pub fn new(
tss_account_id: [u8; 32],
x25519_public_key: X25519PublicKey,
nonce: [u8; 32],
block_number: u32,
) -> Self {
let mut hasher = Blake2b512::new();
hasher.update(tss_account_id);
hasher.update(x25519_public_key);
hasher.update(nonce);
hasher.update(block_number.to_be_bytes());
Self(hasher.finalize().into())
}
}
4 changes: 3 additions & 1 deletion crates/threshold-signature-server/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ sha1 ="0.10.6"
sha2 ="0.10.8"
hkdf ="0.12.4"
project-root ={ version="0.2.2", optional=true }
tdx-quote ={ git="https://github.com/entropyxyz/tdx-quote", optional=true, features=["mock"] }

[dev-dependencies]
serial_test ="3.1.1"
Expand All @@ -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"] }
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@ameba23 ameba23 Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


# Note: We don't specify versions here because otherwise we run into a cyclical dependency between
# `entropy-tss` and `entropy-testing-utils` when we try and publish the `entropy-tss` crate.
Expand All @@ -102,7 +104,7 @@ vergen={ version="8.3.2", features=["build", "git", "gitcl"] }
default =['std']
std =["sp-core/std"]
test_helpers=["dep:project-root"]
unsafe =[]
unsafe =["dep:tdx-quote"]
alice =[]
bob =[]
# Enable this feature to run the integration tests for the wasm API of entropy-protocol
Expand Down
3 changes: 2 additions & 1 deletion crates/threshold-signature-server/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ use crate::{
r#unsafe::api::{delete, put, remove_keys, unsafe_get},
signing_client::{api::*, ListenerState},
user::api::*,
validator::api::new_reshare,
validator::api::{attest, new_reshare},
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
};

#[derive(Clone)]
Expand All @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this should go lower into the unsafe endpoints

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW I think it's fine as is

.route("/healthz", get(healthz))
.route("/version", get(get_version))
.route("/hashes", get(hashes))
Expand Down
44 changes: 44 additions & 0 deletions crates/threshold-signature-server/src/validator/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,3 +360,47 @@ pub async fn prune_old_holders(
validators_info.clone()
})
}

#[cfg(not(any(test, feature = "unsafe")))]
pub async fn attest(
State(_app_state): State<AppState>,
_input: Bytes,
) -> Result<StatusCode, ValidatorErr> {
// Non-mock attestation (the real thing) will go here
Err(ValidatorErr::NotImplemented)
}

#[cfg(any(test, feature = "unsafe"))]
pub async fn attest(
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
State(app_state): State<AppState>,
input: Bytes,
) -> Result<(StatusCode, Bytes), ValidatorErr> {
// TODO #982 confirm with the chain that an attestation should be happenning
ameba23 marked this conversation as resolved.
Show resolved Hide resolved
let nonce = input[..].try_into()?;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@ameba23 ameba23 Aug 13, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


let rpc = get_rpc(&app_state.configuration.endpoint).await?;

let block_number = rpc
.chain_get_header(None)
.await?
.ok_or_else(|| ValidatorErr::OptionUnwrapError("Failed to get block number".to_string()))?
.number;

// In the real thing this is the hardware key used in the quoting enclave
let signing_key = tdx_quote::SigningKey::random(&mut OsRng);

let (signer, x25519_secret) = get_signer_and_x25519_secret(&app_state.kv_store).await?;
let public_key = x25519_dalek::PublicKey::from(&x25519_secret);

let input_data = entropy_shared::QuoteInputData::new(
signer.signer().public().into(),
*public_key.as_bytes(),
nonce,
block_number,
);

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())))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
6 changes: 5 additions & 1 deletion crates/threshold-signature-server/src/validator/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
// You should have received a copy of the GNU Affero General Public License
// along with this program. If not, see <https://www.gnu.org/licenses/>.

use std::string::FromUtf8Error;
use std::{array::TryFromSliceError, string::FromUtf8Error};

use crate::signing_client::ProtocolErr;
use axum::{
Expand Down Expand Up @@ -93,6 +93,10 @@ pub enum ValidatorErr {
InvalidData,
#[error("Data is repeated")]
RepeatedData,
#[error("Not yet implemented")]
NotImplemented,
#[error("Input must be 32 bytes: {0}")]
TryFromSlice(#[from] TryFromSliceError),
}

impl IntoResponse for ValidatorErr {
Expand Down
22 changes: 22 additions & 0 deletions crates/threshold-signature-server/src/validator/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,3 +255,25 @@ async fn test_forbidden_keys() {
let should_pass = check_forbidden_key("test");
assert_eq!(should_pass.unwrap(), ());
}

#[tokio::test]
#[serial]
async fn test_attest() {
initialize_test_logger().await;
clean_tests();

let _cxt = test_node_process_testing_state(false).await;
let (_validator_ips, _validator_ids) = spawn_testing_validators(false).await;

let client = reqwest::Client::new();
let res = client
.post(format!("http://127.0.0.1:3001/attest"))
.body([0; 32].to_vec())
.send()
.await
.unwrap();
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();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}
1 change: 1 addition & 0 deletions deny.toml
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ exceptions=[
{ allow=["AGPL-3.0"], name="entropy-programs-core" },
{ allow=["AGPL-3.0"], name="entropy-programs-runtime" },
{ allow=["AGPL-3.0"], name="synedrion" },
{ allow=["AGPL-3.0"], name="tdx-quote" },

# These are the only crates using these licenses, put them here instead of globally allowing
# them
Expand Down
Loading