From 72e6d3370ed272b77ffd867cd3f14c414fd30d02 Mon Sep 17 00:00:00 2001 From: Hernando Castano Date: Fri, 8 Nov 2024 11:00:37 +0700 Subject: [PATCH] Add quote guards to `ServerInfo` related extrinsics (#1123) * Add quote guard to `change_endpoint` extrinsic * Add quote guard to `change_threshold_accounts` extrinsic * Bump metadata * RustFmt * Get `entropy-client` test for `change_endpoint` working * Get `change_threshold_account` test compiling Doesn't work yet though, but this is at least a good checkpoint * Add way to `request_attestation` from client * Almost have `change_threshold_account` test working Faling to verify the PCK though... * TaploFmt * Be a bit more descriptive with the TSS public key variable * Make `update_threshold_account()` use updated PCK It looks like if the TSS Account ID and X25519 public keys are changing then we're probably on different hardware so the PCK will also change. This also gets the client test for the extrisnic passing. * Clean up `change_threshold_account()` test * Clean up the `request_attestation` client method a bit * Get `entropy-test-cli` compiling again * Remove unnecessary `.clone()` * Update `test-cli` for new extrinsic arguments * Get staking tests working again * Get Staking benchmarks compiling * Get `change_endpoint` benchmark working * Get `change_threshold_accounts` benchmark working * RustFmt benches * Use better mock endpoint * Switch to requiring a PCK chain instead of a certificate directly This matches what `validate()` does and prevents us from having an invalid PCK become part of `ServerInfo`. * Bump metadata * Update `client` to use PCK certificate chains * Update the `test-cli` to use PCK certificate chains * Undo some formatting changes * Variables mystery amount * Add `CHANGELOG` entry * Revert "Add `CHANGELOG` entry" This reverts commit fe7aadd6b01dcb839f3026ed5bdb57042a64f588. * Updated `CHANGELOG` without formatting --- CHANGELOG.md | 6 +- Cargo.lock | 1 + crates/client/Cargo.toml | 1 + crates/client/entropy_metadata.scale | Bin 209698 -> 209774 bytes crates/client/src/client.rs | 45 +++++- crates/client/src/tests.rs | 112 +++++++++++++-- crates/test-cli/src/lib.rs | 23 ++- crates/testing-utils/src/lib.rs | 2 + .../src/helpers/validator.rs | 2 +- pallets/staking/src/benchmarking.rs | 78 ++++++++-- pallets/staking/src/lib.rs | 101 +++++++++++-- pallets/staking/src/tests.rs | 134 ++++++++++++++++-- 12 files changed, 442 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ada5aaea8..4665f8a79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,7 +17,10 @@ At the moment this project **does not** adhere to `AttestationQueue` config types were removed from the Attestation pallet. - In [#1068](https://github.com/entropyxyz/entropy-core/pull/1068) an extra type `PckCertChainVerifier` was added to the staking extension pallet's `Config` trait. -- In [#1134](https://github.com/entropyxyz/entropy-core/pull/1134/) the ```no-sync``` option was removed +- In [#1123](https://github.com/entropyxyz/entropy-core/pull/1123/) the `change_endpoint()` and + `change_threshold_accounts()` extrinsics got new TDX `quote` related parameters added. +- In [#1134](https://github.com/entropyxyz/entropy-core/pull/1134/) the `--no-sync` option was + removed. ### Added - Protocol message versioning ([#1140](https://github.com/entropyxyz/entropy-core/pull/1140)) @@ -26,6 +29,7 @@ At the moment this project **does not** adhere to - Use correct key rotation endpoint in OCW ([#1104](https://github.com/entropyxyz/entropy-core/pull/1104)) - Change attestation flow to be pull based ([#1109](https://github.com/entropyxyz/entropy-core/pull/1109/)) - Handle PCK certificates ([#1068](https://github.com/entropyxyz/entropy-core/pull/1068)) +- Add quote guards to `ServerInfo` related extrinsics ([#1123](https://github.com/entropyxyz/entropy-core/pull/1123/)) - Remove declare synced ([#1134](https://github.com/entropyxyz/entropy-core/pull/1134/)) ## [0.3.0-rc.1](https://github.com/entropyxyz/entropy-core/compare/release/v0.2.0...release/v0.3.0-rc.1) - 2024-10-04 diff --git a/Cargo.lock b/Cargo.lock index d1f661d4e..46fcf7257 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2533,6 +2533,7 @@ dependencies = [ "sp-keyring 34.0.0", "subxt", "synedrion", + "tdx-quote", "thiserror 2.0.0", "tokio", "tracing", diff --git a/crates/client/Cargo.toml b/crates/client/Cargo.toml index 3798ff9c8..ed0ac71ac 100644 --- a/crates/client/Cargo.toml +++ b/crates/client/Cargo.toml @@ -40,6 +40,7 @@ tokio ={ version="1.41", features=["time"] } serial_test ="3.1.1" sp-keyring ="34.0.0" entropy-testing-utils={ path="../testing-utils" } +tdx-quote ={ version="0.0.1", features=["mock"] } [features] default=["native", "full-client-native"] diff --git a/crates/client/entropy_metadata.scale b/crates/client/entropy_metadata.scale index fcfbd8800746eaa47293aebd17012d87cefbfd01..00e10f14ada10341859b6eaa12033d6e59070578 100644 GIT binary patch delta 84 zcmZ4VjOX1mo(-8VWH}fWQu9&@@-y>FOc-UtQj={;E$kQ=MG8yvOHwD7yzpQYn7sFe c%jDMA>Vi;Fgu3ScFWUdVVBG%y1(UKD0Ax@hv;Y7A delta 44 zcmV+{0Mq~O<_x0d46teF3j_fmWo~3}Z)t9HlZNO>0SJ@N=tP4}>4#0}0k=)*0wGGp C(i6A< diff --git a/crates/client/src/client.rs b/crates/client/src/client.rs index 5e5ace6e0..98e73bda6 100644 --- a/crates/client/src/client.rs +++ b/crates/client/src/client.rs @@ -341,8 +341,10 @@ pub async fn change_endpoint( rpc: &LegacyRpcMethods, user_keypair: sr25519::Pair, new_endpoint: String, + quote: Vec, ) -> anyhow::Result { - let change_endpoint_tx = entropy::tx().staking_extension().change_endpoint(new_endpoint.into()); + let change_endpoint_tx = + entropy::tx().staking_extension().change_endpoint(new_endpoint.into(), quote); let in_block = submit_transaction_with_pair(api, rpc, &user_keypair, &change_endpoint_tx, None).await?; let result_event = in_block @@ -358,13 +360,18 @@ pub async fn change_threshold_accounts( user_keypair: sr25519::Pair, new_tss_account: String, new_x25519_public_key: String, + new_pck_certificate_chain: Vec>, + quote: Vec, ) -> anyhow::Result { let tss_account = SubxtAccountId32::from_str(&new_tss_account)?; + let x25519_public_key = hex::decode(new_x25519_public_key)? + .try_into() + .map_err(|_| anyhow!("X25519 pub key needs to be 32 bytes"))?; let change_threshold_accounts = entropy::tx().staking_extension().change_threshold_accounts( tss_account, - hex::decode(new_x25519_public_key)? - .try_into() - .map_err(|_| anyhow!("X25519 pub key needs to be 32 bytes"))?, + x25519_public_key, + new_pck_certificate_chain, + quote, ); let in_block = submit_transaction_with_pair(api, rpc, &user_keypair, &change_threshold_accounts, None) @@ -414,3 +421,33 @@ async fn jumpstart_inner( Ok(()) } + +/// An extrinsic to indicate to the chain that it should expect an attestation from the `signer` at +/// some point in the near future. +/// +/// The returned `nonce` must be used when generating a `quote` for the chain. +#[tracing::instrument( + skip_all, + fields( + attestee = ?attestee.public(), + ) +)] +pub async fn request_attestation( + api: &OnlineClient, + rpc: &LegacyRpcMethods, + attestee: sr25519::Pair, +) -> Result, ClientError> { + tracing::debug!("{} is requesting an attestation.", attestee.public()); + + let request_attestation = entropy::tx().attestation().request_attestation(); + + let result = + submit_transaction_with_pair(api, rpc, &attestee, &request_attestation, None).await?; + let result_event = result + .find_first::()? + .ok_or(crate::errors::SubstrateError::NoEvent)?; + + let nonce = result_event.0; + + Ok(nonce) +} diff --git a/crates/client/src/tests.rs b/crates/client/src/tests.rs index 819bf8bae..93e406037 100644 --- a/crates/client/src/tests.rs +++ b/crates/client/src/tests.rs @@ -11,17 +11,23 @@ use crate::{ }, get_api, get_rpc, EntropyConfig, }, - change_endpoint, change_threshold_accounts, register, remove_program, store_program, + change_endpoint, change_threshold_accounts, register, remove_program, request_attestation, + store_program, substrate::query_chain, update_programs, }; + use entropy_testing_utils::{ - constants::{TEST_PROGRAM_WASM_BYTECODE, TSS_ACCOUNTS}, - helpers::{derive_mock_pck_verifying_key, encode_verifying_key}, + constants::{TEST_PROGRAM_WASM_BYTECODE, TSS_ACCOUNTS, X25519_PUBLIC_KEYS}, + helpers::encode_verifying_key, jump_start_network, spawn_testing_validators, substrate_context::test_context_stationary, test_node_process_testing_state, ChainSpecType, }; +use rand::{ + rngs::{OsRng, StdRng}, + SeedableRng, +}; use serial_test::serial; use sp_core::{sr25519, Pair, H256}; use sp_keyring::AccountKeyring; @@ -36,7 +42,33 @@ async fn test_change_endpoint() { let api = get_api(&substrate_context.node_proc.ws_url).await.unwrap(); let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap(); - let result = change_endpoint(&api, &rpc, one.into(), "new_endpoint".to_string()).await.unwrap(); + // By using this `Alice` account we can skip the `request_attestation` step since this is + // already set up at genesis. + let tss_account_id = &TSS_ACCOUNTS[0]; + let x25519_public_key = X25519_PUBLIC_KEYS[0]; + + // This nonce is what was used in the genesis config for `Alice`. + let nonce = [0; 32]; + + let quote = { + let signing_key = tdx_quote::SigningKey::random(&mut OsRng); + let public_key = sr25519::Public(tss_account_id.0); + + // We need to add `1` here since the quote is being checked in the next block + let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1; + + let input_data = + entropy_shared::QuoteInputData::new(public_key, x25519_public_key, nonce, block_number); + + let mut pck_seeder = StdRng::from_seed(public_key.0); + let pck = tdx_quote::SigningKey::random(&mut pck_seeder); + + tdx_quote::Quote::mock(signing_key.clone(), pck, input_data.0).as_bytes().to_vec() + }; + + let result = + change_endpoint(&api, &rpc, one.into(), "new_endpoint".to_string(), quote).await.unwrap(); + assert_eq!( format!("{:?}", result), format!( @@ -57,22 +89,72 @@ async fn test_change_threshold_accounts() { let api = get_api(&substrate_context.node_proc.ws_url).await.unwrap(); let rpc = get_rpc(&substrate_context.node_proc.ws_url).await.unwrap(); - let x25519_public_key = [0u8; 32]; - let result = change_threshold_accounts( + + // We need to use an account that's not a validator (so not our default development/test accounts) + // otherwise we're not able to update the TSS and X25519 keys for our existing validator. + let non_validator_seed = + "gospel prosper cactus remember snap enact refuse review bind rescue guard sock"; + let (tss_signer_pair, x25519_secret) = + entropy_testing_utils::get_signer_and_x25519_secret_from_mnemonic(non_validator_seed) + .unwrap(); + + let tss_public_key = tss_signer_pair.signer().public(); + let x25519_public_key = x25519_dalek::PublicKey::from(&x25519_secret); + + // We need to give our new TSS account some funds before it can request an attestation. + let dest = tss_signer_pair.account_id().clone().into(); + let amount = 10 * entropy_shared::MIN_BALANCE; + let balance_transfer_tx = entropy::tx().balances().transfer_allow_death(dest, amount); + let _transfer_result = crate::substrate::submit_transaction_with_pair( &api, &rpc, - one.into(), - AccountId32(one.pair().public().0.into()).to_string(), - hex::encode(x25519_public_key), + &one.pair(), + &balance_transfer_tx, + None, ) .await .unwrap(); - let provisioning_certification_key = { - let key = derive_mock_pck_verifying_key(&TSS_ACCOUNTS[0]); - BoundedVec(encode_verifying_key(&key).unwrap().to_vec()) + // When we request an attestation we get a nonce back that we must use when generating our quote. + let nonce = request_attestation(&api, &rpc, tss_signer_pair.signer().clone()).await.unwrap(); + let nonce: [u8; 32] = nonce.try_into().unwrap(); + + let mut pck_seeder = StdRng::from_seed(tss_public_key.0.clone()); + let pck = tdx_quote::SigningKey::random(&mut pck_seeder); + let encoded_pck = encode_verifying_key(&pck.verifying_key()).unwrap().to_vec(); + + // Our runtime is using the mock `PckCertChainVerifier`, which means that the expected + // "certificate" basically is just our TSS account ID. This account needs to match the one + // used to sign the following `quote`. + let pck_certificate_chain = vec![tss_public_key.0.to_vec()]; + + let quote = { + // We need to add `1` here since the quote is being checked in the next block + let block_number = rpc.chain_get_header(None).await.unwrap().unwrap().number + 1; + + let input_data = entropy_shared::QuoteInputData::new( + tss_public_key, + *x25519_public_key.as_bytes(), + nonce, + block_number, + ); + + let signing_key = tdx_quote::SigningKey::random(&mut OsRng); + tdx_quote::Quote::mock(signing_key.clone(), pck.clone(), input_data.0).as_bytes().to_vec() }; + let result = change_threshold_accounts( + &api, + &rpc, + one.into(), + tss_public_key.to_string(), + hex::encode(*x25519_public_key.as_bytes()), + pck_certificate_chain, + quote, + ) + .await + .unwrap(); + assert_eq!( format!("{:?}", result), format!( @@ -80,10 +162,10 @@ async fn test_change_threshold_accounts() { events::ThresholdAccountChanged( AccountId32(one.pair().public().0), ServerInfo { - tss_account: AccountId32(one.pair().public().0), - x25519_public_key, + tss_account: AccountId32(tss_public_key.0), + x25519_public_key: *x25519_public_key.as_bytes(), endpoint: "127.0.0.1:3001".as_bytes().to_vec(), - provisioning_certification_key, + provisioning_certification_key: BoundedVec(encoded_pck), } ) ) diff --git a/crates/test-cli/src/lib.rs b/crates/test-cli/src/lib.rs index 9bc063167..6a9007861 100644 --- a/crates/test-cli/src/lib.rs +++ b/crates/test-cli/src/lib.rs @@ -145,6 +145,11 @@ enum CliCommand { ChangeEndpoint { /// New endpoint to change to (ex. "127.0.0.1:3001") new_endpoint: String, + /// The Intel TDX quote used to prove that this TSS is running on TDX hardware. + /// + /// The quote format is specified in: + /// https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf + quote: String, /// The mnemonic for the validator stash account to use for the call, should be stash address #[arg(short, long)] mnemonic_option: Option, @@ -155,6 +160,13 @@ enum CliCommand { new_tss_account: String, /// New x25519 public key new_x25519_public_key: String, + /// The new Provisioning Certification Key (PCK) certificate chain to be used for the TSS. + new_pck_certificate_chain: Vec, + /// The Intel TDX quote used to prove that this TSS is running on TDX hardware. + /// + /// The quote format is specified in: + /// https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf + quote: String, /// The mnemonic for the validator stash account to use for the call, should be stash address #[arg(short, long)] mnemonic_option: Option, @@ -433,7 +445,7 @@ pub async fn run_command( Ok("Got status".to_string()) }, - CliCommand::ChangeEndpoint { new_endpoint, mnemonic_option } => { + CliCommand::ChangeEndpoint { new_endpoint, quote, mnemonic_option } => { let mnemonic = if let Some(mnemonic_option) = mnemonic_option { mnemonic_option } else { @@ -443,13 +455,16 @@ pub async fn run_command( let user_keypair = ::from_string(&mnemonic, None)?; println!("User account for current call: {}", user_keypair.public()); - let result_event = change_endpoint(&api, &rpc, user_keypair, new_endpoint).await?; + let result_event = + change_endpoint(&api, &rpc, user_keypair, new_endpoint, quote.into()).await?; println!("Event result: {:?}", result_event); Ok("Endpoint changed".to_string()) }, CliCommand::ChangeThresholdAccounts { new_tss_account, new_x25519_public_key, + new_pck_certificate_chain, + quote, mnemonic_option, } => { let mnemonic = if let Some(mnemonic_option) = mnemonic_option { @@ -460,12 +475,16 @@ pub async fn run_command( let user_keypair = ::from_string(&mnemonic, None)?; println!("User account for current call: {}", user_keypair.public()); + let new_pck_certificate_chain = + new_pck_certificate_chain.iter().cloned().map(|i| i.into()).collect::<_>(); let result_event = change_threshold_accounts( &api, &rpc, user_keypair, new_tss_account, new_x25519_public_key, + new_pck_certificate_chain, + quote.into(), ) .await?; println!("Event result: {:?}", result_event); diff --git a/crates/testing-utils/src/lib.rs b/crates/testing-utils/src/lib.rs index 22329cb1c..435cd9afa 100644 --- a/crates/testing-utils/src/lib.rs +++ b/crates/testing-utils/src/lib.rs @@ -27,3 +27,5 @@ pub use entropy_tss::helpers::tests::{ }; pub use node_proc::TestNodeProcess; pub use substrate_context::*; + +pub use entropy_tss::helpers::validator::get_signer_and_x25519_secret_from_mnemonic; diff --git a/crates/threshold-signature-server/src/helpers/validator.rs b/crates/threshold-signature-server/src/helpers/validator.rs index b571e0181..3811b0414 100644 --- a/crates/threshold-signature-server/src/helpers/validator.rs +++ b/crates/threshold-signature-server/src/helpers/validator.rs @@ -88,7 +88,7 @@ fn get_x25519_secret_from_hkdf(hkdf: &Hkdf) -> Result( if validate_also { let block_number = 0; - let endpoint = vec![20, 20]; + let endpoint = b"http://localhost:3001".to_vec(); let (quote, joining_server_info) = prepare_attestation_for_validate::( threshold, x25519_public_key, @@ -172,33 +172,86 @@ benchmarks! { let caller: T::AccountId = whitelisted_caller(); let bonder: T::AccountId = account("bond", 0, SEED); let threshold: T::AccountId = account("threshold", 0, SEED); + + let endpoint = b"http://localhost:3001"; let x25519_public_key = NULL_ARR; - prep_bond_and_validate::(true, caller.clone(), bonder.clone(), threshold, NULL_ARR); + let validate_also = true; + prep_bond_and_validate::( + validate_also, + caller.clone(), + bonder.clone(), + threshold.clone(), + x25519_public_key.clone(), + ); - }: _(RawOrigin::Signed(bonder.clone()), vec![30]) + // For quote verification this needs to be the _next_ block, and right now we're at block `0`. + let block_number = 1; + let quote = prepare_attestation_for_validate::( + threshold, + x25519_public_key, + endpoint.clone().to_vec(), + block_number, + ) + .0; + }: _(RawOrigin::Signed(bonder.clone()), endpoint.to_vec(), quote) verify { - assert_last_event::(Event::::EndpointChanged(bonder, vec![30]).into()); + assert_last_event::(Event::::EndpointChanged(bonder, endpoint.to_vec()).into()); } change_threshold_accounts { let s in 0 .. MAX_SIGNERS as u32; + let caller: T::AccountId = whitelisted_caller(); let _bonder: T::AccountId = account("bond", 0, SEED); - let validator_id_res = ::ValidatorId::try_from(_bonder.clone()).or(Err(Error::::InvalidValidatorId)); - let validator_id_signers = ::ValidatorId::try_from(caller.clone()).or(Err(Error::::InvalidValidatorId)).unwrap(); - let bonder: T::ValidatorId = validator_id_res.expect("Issue converting account id into validator id"); + + let validator_id_res = ::ValidatorId::try_from(_bonder.clone()) + .or(Err(Error::::InvalidValidatorId)); + let validator_id_signers = ::ValidatorId::try_from(caller.clone()) + .or(Err(Error::::InvalidValidatorId)) + .unwrap(); + let bonder: T::ValidatorId = + validator_id_res.expect("Issue converting account id into validator id"); + let threshold: T::AccountId = account("threshold", 0, SEED); + let new_threshold: T::AccountId = account("new_threshold", 0, SEED); + let x25519_public_key: [u8; 32] = NULL_ARR; - prep_bond_and_validate::(true, caller.clone(), _bonder.clone(), threshold, NULL_ARR); + let endpoint = b"http://localhost:3001".to_vec(); + + let validate_also = true; + prep_bond_and_validate::( + validate_also, + caller.clone(), + _bonder.clone(), + threshold.clone(), + x25519_public_key.clone(), + ); + + // For quote verification this needs to be the _next_ block, and right now we're at block `0`. + let block_number = 1; + let (quote , joining_server_info) = prepare_attestation_for_validate::( + new_threshold.clone(), + x25519_public_key, + endpoint.clone().to_vec(), + block_number, + ); + + let pck_certificate_chain = joining_server_info.pck_certificate_chain; + let signers = vec![validator_id_signers.clone(); s as usize]; Signers::::put(signers.clone()); - - }: _(RawOrigin::Signed(_bonder.clone()), _bonder.clone(), NULL_ARR) + }: _( + RawOrigin::Signed(_bonder.clone()), + new_threshold.clone(), + x25519_public_key.clone(), + pck_certificate_chain, + quote + ) verify { let server_info = ServerInfo { - endpoint: vec![20, 20], - tss_account: _bonder.clone(), + endpoint: b"http://localhost:3001".to_vec(), + tss_account: new_threshold.clone(), x25519_public_key: NULL_ARR, provisioning_certification_key: MOCK_PCK_DERIVED_FROM_NULL_ARRAY.to_vec().try_into().unwrap(), }; @@ -330,6 +383,7 @@ benchmarks! { x25519_public_key.clone() ); + // For quote verification this needs to be the _next_ block, and right now we're at block `0`. let block_number = 1; let (quote, joining_server_info) = prepare_attestation_for_validate::(threshold_account.clone(), x25519_public_key, endpoint.clone(), block_number); diff --git a/pallets/staking/src/lib.rs b/pallets/staking/src/lib.rs index fcd37e88a..8154f3bde 100644 --- a/pallets/staking/src/lib.rs +++ b/pallets/staking/src/lib.rs @@ -378,12 +378,26 @@ pub mod pallet { #[pallet::call] impl Pallet { - /// Allows a validator to change their endpoint so signers can find them when they are coms - /// manager `endpoint`: nodes's endpoint + /// Allows a validator to change the endpoint used by their Threshold Siganture Scheme + /// (TSS) server. + /// + /// # Expects TDX Quote + /// + /// A valid TDX quote must be passed along in order to ensure that the validator is running + /// TDX hardware. In order for the chain to be aware that a quote is expected from the + /// validator `pallet_attestation::request_attestation()` must be called first. + /// + /// The quote format is specified in: + /// https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf #[pallet::call_index(0)] #[pallet::weight(::WeightInfo::change_endpoint())] - pub fn change_endpoint(origin: OriginFor, endpoint: Vec) -> DispatchResult { + pub fn change_endpoint( + origin: OriginFor, + endpoint: Vec, + quote: Vec, + ) -> DispatchResult { let who = ensure_signed(origin)?; + ensure!( endpoint.len() as u32 <= T::MaxEndpointLength::get(), Error::::EndpointTooLong @@ -396,31 +410,60 @@ pub mod pallet { ThresholdServers::::try_mutate(&validator_id, |maybe_server_info| { if let Some(server_info) = maybe_server_info { + // Before we modify the `server_info`, we want to check that the validator is + // still running TDX hardware. + ensure!( + >::verify_quote( + &server_info.tss_account.clone(), + server_info.x25519_public_key, + server_info.provisioning_certification_key.clone(), + quote + ) + .is_ok(), + Error::::FailedAttestationCheck + ); + server_info.endpoint.clone_from(&endpoint); + Ok(()) } else { Err(Error::::NoBond) } })?; + Self::deposit_event(Event::EndpointChanged(who, endpoint)); Ok(()) } - /// Allows a validator to change their threshold key so can confirm done when coms manager - /// `new_account`: nodes's threshold account + /// Allows a validator to change their associated threshold server AccountID and X25519 + /// public key. + /// + /// # Expects TDX Quote + /// + /// A valid TDX quote must be passed along in order to ensure that the validator is running + /// TDX hardware. In order for the chain to be aware that a quote is expected from the + /// validator `pallet_attestation::request_attestation()` must be called first. + /// + /// The **new** TSS AccountID must be used when requesting this quote. + /// + /// The quote format is specified in: + /// https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf #[pallet::call_index(1)] #[pallet::weight(::WeightInfo::change_threshold_accounts(MAX_SIGNERS as u32))] pub fn change_threshold_accounts( origin: OriginFor, tss_account: T::AccountId, x25519_public_key: X25519PublicKey, + pck_certificate_chain: Vec>, + quote: Vec, ) -> DispatchResultWithPostInfo { + let who = ensure_signed(origin)?; + ensure!( !ThresholdToStash::::contains_key(&tss_account), Error::::TssAccountAlreadyExists ); - let who = ensure_signed(origin)?; let stash = Self::get_stash(&who)?; let validator_id = ::ValidatorId::try_from(stash) .or(Err(Error::::InvalidValidatorId))?; @@ -431,20 +474,48 @@ pub mod pallet { Error::::NoChangingThresholdAccountWhenSigner ); - let new_server_info: ServerInfo = - ThresholdServers::::try_mutate(&validator_id, |maybe_server_info| { + let provisioning_certification_key = + T::PckCertChainVerifier::verify_pck_certificate_chain(pck_certificate_chain) + .map_err(|error| { + let e: Error = error.into(); + e + })?; + + let new_server_info: ServerInfo = ThresholdServers::::try_mutate( + &validator_id, + |maybe_server_info| { if let Some(server_info) = maybe_server_info { - server_info.tss_account = tss_account.clone(); + // Before we modify the `server_info`, we want to check that the validator is + // still running TDX hardware. + ensure!( + >::verify_quote( + &tss_account.clone(), + x25519_public_key, + provisioning_certification_key.clone(), + quote + ) + .is_ok(), + Error::::FailedAttestationCheck + ); + + server_info.tss_account = tss_account; server_info.x25519_public_key = x25519_public_key; - ThresholdToStash::::insert(&tss_account, &validator_id); + server_info.provisioning_certification_key = provisioning_certification_key; + + ThresholdToStash::::insert(&server_info.tss_account, &validator_id); + Ok(server_info.clone()) } else { Err(Error::::NoBond) } - })?; + }, + )?; + Self::deposit_event(Event::ThresholdAccountChanged(validator_id, new_server_info)); - Ok(Some(::WeightInfo::change_threshold_accounts(signers.len() as u32)) - .into()) + + let actual_weight = + ::WeightInfo::change_threshold_accounts(signers.len() as u32); + Ok(Some(actual_weight).into()) } /// Wraps's Substrate's `unbond` extrinsic but checks to make sure targeted account is not a signer or next signer @@ -521,9 +592,11 @@ pub mod pallet { /// Wrap's Substrate's `staking_pallet::validate()` extrinsic, but enforces that /// information about a validator's threshold server is provided. /// + /// # Expects TDX Quote + /// /// A valid TDX quote must be passed along in order to ensure that the validator candidate /// is running TDX hardware. In order for the chain to be aware that a quote is expected - /// from the candidate, `pallet_attestation::request_attestation()` must be called first. + /// from the candidate `pallet_attestation::request_attestation()` must be called first. /// /// The quote format is specified in: /// https://download.01.org/intel-sgx/latest/dcap-latest/linux/docs/Intel_TDX_DCAP_Quoting_Library_API.pdf diff --git a/pallets/staking/src/tests.rs b/pallets/staking/src/tests.rs index 0340d4cf9..01eef85a6 100644 --- a/pallets/staking/src/tests.rs +++ b/pallets/staking/src/tests.rs @@ -82,7 +82,7 @@ fn it_takes_in_an_endpoint() { let joining_server_info = JoiningServerInfo { tss_account: 3, x25519_public_key: NULL_ARR, - endpoint: vec![20; 26], + endpoint: [20; (crate::tests::MaxEndpointLength::get() + 1) as usize].to_vec(), pck_certificate_chain: vec![[0u8; 32].to_vec()], }; assert_noop!( @@ -156,6 +156,8 @@ fn it_will_not_allow_validator_to_use_existing_tss_account() { #[test] fn it_changes_endpoint() { new_test_ext().execute_with(|| { + let endpoint = b"http://localhost:3001".to_vec(); + assert_ok!(FrameStaking::bond( RuntimeOrigin::signed(1), 100u64, @@ -165,7 +167,7 @@ fn it_changes_endpoint() { let joining_server_info = JoiningServerInfo { tss_account: 3, x25519_public_key: NULL_ARR, - endpoint: vec![20], + endpoint: endpoint.clone(), pck_certificate_chain: vec![[0u8; 32].to_vec()], }; assert_ok!(Staking::validate( @@ -175,16 +177,52 @@ fn it_changes_endpoint() { VALID_QUOTE.to_vec(), )); - assert_ok!(Staking::change_endpoint(RuntimeOrigin::signed(1), vec![30])); - assert_eq!(Staking::threshold_server(1).unwrap().endpoint, vec![30]); + assert_ok!(Staking::change_endpoint( + RuntimeOrigin::signed(1), + endpoint.clone(), + VALID_QUOTE.to_vec() + )); + assert_eq!(Staking::threshold_server(1).unwrap().endpoint, endpoint); assert_noop!( - Staking::change_endpoint(RuntimeOrigin::signed(3), vec![30]), + Staking::change_endpoint(RuntimeOrigin::signed(3), endpoint, VALID_QUOTE.to_vec()), Error::::NoBond ); }); } +#[test] +fn it_doesnt_change_endpoint_with_invalid_quote() { + new_test_ext().execute_with(|| { + let endpoint = b"http://localhost:3001".to_vec(); + + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(1), + 100u64, + pallet_staking::RewardDestination::Account(1), + )); + + let joining_server_info = JoiningServerInfo { + tss_account: 3, + x25519_public_key: NULL_ARR, + endpoint: endpoint.clone(), + pck_certificate_chain: vec![[0u8; 32].to_vec()], + }; + + assert_ok!(Staking::validate( + RuntimeOrigin::signed(1), + pallet_staking::ValidatorPrefs::default(), + joining_server_info.clone(), + VALID_QUOTE.to_vec(), + )); + + assert_noop!( + Staking::change_endpoint(RuntimeOrigin::signed(1), endpoint, INVALID_QUOTE.to_vec()), + Error::::FailedAttestationCheck + ); + }) +} + #[test] fn it_changes_threshold_account() { new_test_ext().execute_with(|| { @@ -194,11 +232,12 @@ fn it_changes_threshold_account() { pallet_staking::RewardDestination::Account(1), )); + let pck_certificate_chain = vec![vec![0u8; 32]]; let joining_server_info = JoiningServerInfo { tss_account: 3, x25519_public_key: NULL_ARR, endpoint: vec![20], - pck_certificate_chain: vec![[0u8; 32].to_vec()], + pck_certificate_chain: pck_certificate_chain.clone(), }; assert_ok!(Staking::validate( RuntimeOrigin::signed(1), @@ -207,12 +246,24 @@ fn it_changes_threshold_account() { VALID_QUOTE.to_vec(), )); - assert_ok!(Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 4, NULL_ARR)); + assert_ok!(Staking::change_threshold_accounts( + RuntimeOrigin::signed(1), + 4, + NULL_ARR, + pck_certificate_chain.clone(), + VALID_QUOTE.to_vec() + )); assert_eq!(Staking::threshold_server(1).unwrap().tss_account, 4); assert_eq!(Staking::threshold_to_stash(4).unwrap(), 1); assert_noop!( - Staking::change_threshold_accounts(RuntimeOrigin::signed(4), 5, NULL_ARR), + Staking::change_threshold_accounts( + RuntimeOrigin::signed(4), + 5, + NULL_ARR, + pck_certificate_chain.clone(), + VALID_QUOTE.to_vec() + ), Error::::NotController ); @@ -227,7 +278,7 @@ fn it_changes_threshold_account() { tss_account: 5, x25519_public_key: NULL_ARR, endpoint: vec![20], - pck_certificate_chain: vec![[0u8; 32].to_vec()], + pck_certificate_chain: pck_certificate_chain.clone(), }; assert_ok!(Staking::validate( RuntimeOrigin::signed(2), @@ -237,18 +288,66 @@ fn it_changes_threshold_account() { )); assert_noop!( - Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 5, NULL_ARR), + Staking::change_threshold_accounts( + RuntimeOrigin::signed(1), + 5, + NULL_ARR, + pck_certificate_chain.clone(), + VALID_QUOTE.to_vec() + ), Error::::TssAccountAlreadyExists ); Signers::::put(vec![1]); assert_noop!( - Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 9, NULL_ARR,), + Staking::change_threshold_accounts( + RuntimeOrigin::signed(1), + 9, + NULL_ARR, + pck_certificate_chain.clone(), + VALID_QUOTE.to_vec() + ), Error::::NoChangingThresholdAccountWhenSigner ); }); } +#[test] +fn it_doesnt_allow_changing_threshold_account_with_invalid_quote() { + new_test_ext().execute_with(|| { + assert_ok!(FrameStaking::bond( + RuntimeOrigin::signed(1), + 100u64, + pallet_staking::RewardDestination::Account(1), + )); + + let pck_certificate_chain = vec![[0u8; 32].to_vec()]; + let joining_server_info = JoiningServerInfo { + tss_account: 3, + x25519_public_key: NULL_ARR, + endpoint: vec![20], + pck_certificate_chain: pck_certificate_chain.clone(), + }; + assert_ok!(Staking::validate( + RuntimeOrigin::signed(1), + pallet_staking::ValidatorPrefs::default(), + joining_server_info.clone(), + VALID_QUOTE.to_vec(), + )); + + assert_noop!( + Staking::change_threshold_accounts( + RuntimeOrigin::signed(1), + 4, + NULL_ARR, + pck_certificate_chain.clone(), + INVALID_QUOTE.to_vec() + ), + Error::::FailedAttestationCheck + ); + }) +} + #[test] fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() { new_test_ext().execute_with(|| { @@ -258,11 +357,12 @@ fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() { pallet_staking::RewardDestination::Account(1), )); + let pck_certificate_chain = vec![[0u8; 32].to_vec()]; let joining_server_info = JoiningServerInfo { tss_account: 3, x25519_public_key: NULL_ARR, endpoint: vec![20], - pck_certificate_chain: vec![[0u8; 32].to_vec()], + pck_certificate_chain: pck_certificate_chain.clone(), }; assert_ok!(Staking::validate( RuntimeOrigin::signed(1), @@ -282,7 +382,7 @@ fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() { tss_account: 5, x25519_public_key: NULL_ARR, endpoint: vec![20], - pck_certificate_chain: vec![[0u8; 32].to_vec()], + pck_certificate_chain: pck_certificate_chain.clone(), }; assert_ok!(Staking::validate( RuntimeOrigin::signed(2), @@ -292,7 +392,13 @@ fn it_will_not_allow_existing_tss_account_when_changing_threshold_account() { )); assert_noop!( - Staking::change_threshold_accounts(RuntimeOrigin::signed(1), 5, NULL_ARR), + Staking::change_threshold_accounts( + RuntimeOrigin::signed(1), + 5, + NULL_ARR, + pck_certificate_chain.clone(), + VALID_QUOTE.to_vec() + ), Error::::TssAccountAlreadyExists ); });