Skip to content

Commit

Permalink
Ensure correct validator order by using ValidatorInfo from chain rath…
Browse files Browse the repository at this point in the history 
…er than from user (#425)

* Use validator info from chain rather than from user

* Update tests for using validator info from chain

* Clippy

* add regression test

---------

Co-authored-by: Jesse Abramowitz <jesse@entropy.xyz>
  • Loading branch information
@ameba23 @JesseAbram
ameba23 and JesseAbram authored Oct 13, 2023
1 parent aa26acd commit 753ebfa
Show file tree
Hide file tree
Showing 2 changed files with 80 additions and 103 deletions.
34 changes: 23 additions & 11 deletions crypto/server/src/user/api.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -117,12 +117,17 @@ pub async fn sign_tx(
let decrypted_message =
signed_msg.decrypt(signer.signer()).map_err(|e| UserErr::Decryption(e.to_string()))?;

let user_tx_req: UserTransactionRequest = serde_json::from_slice(&decrypted_message)?;
let mut user_tx_req: UserTransactionRequest = serde_json::from_slice(&decrypted_message)?;
check_stale(user_tx_req.timestamp)?;
let raw_message = hex::decode(user_tx_req.transaction_request.clone())?;
let sig_hash = hex::encode(Hasher::keccak(&raw_message));
let subgroup_signers = get_current_subgroup_signers(&api, &sig_hash).await?;
check_signing_group(subgroup_signers, &user_tx_req.validators_info, signer.account_id())?;
check_signing_group(&subgroup_signers, &user_tx_req.validators_info, signer.account_id())?;

// Use the validator info from chain as we can be sure it is in the correct order and the
// details are correct
user_tx_req.validators_info = subgroup_signers;

let tx_id = create_unique_tx_id(&signing_address, &sig_hash);

let has_key = check_for_key(&signing_address, &app_state.kv_store).await?;
Expand Down Expand Up @@ -369,7 +374,7 @@ pub async fn confirm_registered(
pub async fn get_current_subgroup_signers(
api: &OnlineClient<EntropyConfig>,
sig_hash: &str,
) -> Result<Vec<SubxtAccountId32>, UserErr> {
) -> Result<Vec<ValidatorInfo>, UserErr> {
let mut subgroup_signers = vec![];
let number = Arc::new(BigInt::from_str_radix(sig_hash, 16)?);
let futures = (0..SIGNING_PARTY_SIZE)
Expand All @@ -393,16 +398,21 @@ pub async fn get_current_subgroup_signers(
let threshold_address_query = entropy::storage()
.staking_extension()
.threshold_servers(subgroup_info[index_of_signer].clone());
let threshold_address = api
let server_info = api
.storage()
.at_latest()
.await?
.fetch(&threshold_address_query)
.await?
.ok_or(UserErr::SubgroupError("Stash Fetch Error"))?
.tss_account;

Ok::<_, UserErr>(threshold_address)
.ok_or(UserErr::SubgroupError("Stash Fetch Error"))?;

Ok::<_, UserErr>(ValidatorInfo {
x25519_public_key: server_info.x25519_public_key,
ip_address: SocketAddrV4::from_str(std::str::from_utf8(
&server_info.endpoint,
)?)?,
tss_account: server_info.tss_account,
})
}
})
.collect::<Vec<_>>();
Expand All @@ -415,18 +425,20 @@ pub async fn get_current_subgroup_signers(

/// Checks if a validator is in the current selected signing committee
pub fn check_signing_group(
subgroup_signers: Vec<SubxtAccountId32>,
subgroup_signers: &[ValidatorInfo],
validators_info: &Vec<ValidatorInfo>,
my_id: &<EntropyConfig as Config>::AccountId,
) -> Result<(), UserErr> {
let subgroup_signer_ids: Vec<SubxtAccountId32> =
subgroup_signers.iter().map(|signer| signer.tss_account.clone()).collect();
// Check that validators given by the user match those from get_current_subgroup_signers
for validator in validators_info {
if !subgroup_signers.contains(&validator.tss_account) {
if !subgroup_signer_ids.contains(&validator.tss_account) {
return Err(UserErr::InvalidSigner("Invalid Signer in Signing group"));
}
}
// Finally, check that we ourselves are in the signing group
if !subgroup_signers.contains(my_id) {
if !subgroup_signer_ids.contains(my_id) {
return Err(UserErr::InvalidSigner(
"Signing group is valid, but this threshold server is not in the group",
));
Expand Down
149 changes: 57 additions & 92 deletions crypto/server/src/user/tests.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ use entropy_constraints::{Architecture, Evm, Parse};
use entropy_protocol::{
protocol_transport::{noise::noise_handshake_initiator, SubscribeMessage, WsConnection},
user::{user_participates_in_dkg_protocol, user_participates_in_signing_protocol},
PartyId, ValidatorInfo,
KeyParams, PartyId, ValidatorInfo,
};
use entropy_shared::{Acl, KeyVisibility, OcwMessage};
use futures::{
Expand All @@ -36,7 +36,10 @@ use subxt::{
utils::{AccountId32 as subxtAccountId32, Static},
Config, OnlineClient,
};
use synedrion::k256::ecdsa::{RecoveryId, Signature as k256Signature, VerifyingKey};
use synedrion::{
k256::ecdsa::{RecoveryId, Signature as k256Signature, VerifyingKey},
KeyShare,
};
use testing_utils::{
constants::{
ALICE_STASH_ADDRESS, BAREBONES_PROGRAM_WASM_BYTECODE, MESSAGE_SHOULD_FAIL,
Expand Down Expand Up @@ -172,35 +175,16 @@ async fn test_sign_tx_no_chain() {
generic_msg.timestamp = SystemTime::now();
let test_user_res =
submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;
let mut i = 0;
for res in test_user_res {
let mut res = res.unwrap();
assert_eq!(res.status(), 200);
let chunk = res.chunk().await.unwrap().unwrap();
let signing_result: Result<(String, Signature), String> =
serde_json::from_slice(&chunk).unwrap();
assert_eq!(signing_result.clone().unwrap().0.len(), 88);
let mut decoded_sig = base64::decode(signing_result.clone().unwrap().0).unwrap();
let recovery_digit = decoded_sig.pop().unwrap();
let signature = k256Signature::from_slice(&decoded_sig).unwrap();
let recover_id = RecoveryId::from_byte(recovery_digit).unwrap();
let recovery_key_from_sig = VerifyingKey::recover_from_prehash(
&message_should_succeed_hash,
&signature,
recover_id,
)
.unwrap();
assert_eq!(keyshare_option.clone().unwrap().verifying_key(), recovery_key_from_sig);
let mnemonic = if i == 0 { DEFAULT_MNEMONIC } else { DEFAULT_BOB_MNEMONIC };
let sk = <sr25519::Pair as Pair>::from_string(mnemonic, None).unwrap();
let sig_recovery = <sr25519::Pair as Pair>::verify(
&signing_result.clone().unwrap().1,
base64::decode(signing_result.unwrap().0).unwrap(),
&sr25519::Public(sk.public().0),
);
assert!(sig_recovery);
i += 1;
}

verify_signature(test_user_res, message_should_succeed_hash, keyshare_option.clone()).await;

generic_msg.timestamp = SystemTime::now();
generic_msg.validators_info = generic_msg.validators_info.into_iter().rev().collect::<Vec<_>>();
let test_user_res_order =
submit_transaction_requests(validator_ips_and_keys.clone(), generic_msg.clone(), one).await;

verify_signature(test_user_res_order, message_should_succeed_hash, keyshare_option.clone())
.await;

generic_msg.timestamp = SystemTime::now();
// test failing cases
Expand All @@ -214,29 +198,6 @@ async fn test_sign_tx_no_chain() {
);
}

let mut generic_msg_bad_validators = generic_msg.clone();
generic_msg_bad_validators.validators_info[0].x25519_public_key = [0; 32];
generic_msg.timestamp = SystemTime::now();

let test_user_failed_x25519_pub_key = submit_transaction_requests(
validator_ips_and_keys.clone(),
generic_msg_bad_validators,
one,
)
.await;

let mut responses = test_user_failed_x25519_pub_key.into_iter();
assert_eq!(
responses.next().unwrap().unwrap().text().await.unwrap(),
"{\"Err\":\"Subscribe message rejected: Decryption(\\\"Public key does not match that \
given in UserTransactionRequest or register transaction\\\")\"}"
);

assert_eq!(
responses.next().unwrap().unwrap().text().await.unwrap(),
"{\"Err\":\"Oneshot timeout error: channel closed\"}"
);

// Test attempting to connect over ws by someone who is not in the signing group
let validator_ip_and_key = validator_ips_and_keys[0].clone();
let connection_attempt_handle = tokio::spawn(async move {
Expand Down Expand Up @@ -267,12 +228,7 @@ async fn test_sign_tx_no_chain() {
let subscribe_response: Result<(), String> =
serde_json::from_str(&response_message).unwrap();

assert_eq!(
Err("Decryption(\"Public key does not match that given in UserTransactionRequest or \
register transaction\")"
.to_string()),
subscribe_response
);
assert_eq!(Err("NoListener(\"no listener\")".to_string()), subscribe_response);
// The stream should not continue to send messages
// returns true if this part of the test passes
encrypted_connection.recv().await.is_err()
Expand Down Expand Up @@ -829,35 +785,8 @@ async fn test_sign_tx_user_participates() {
let signature_base64 = base64::encode(sig_result.unwrap().to_rsv_bytes());
assert_eq!(signature_base64.len(), 88);

let mut i = 0;
for res in test_user_res {
let mut res = res.unwrap();
assert_eq!(res.status(), 200);
let chunk = res.chunk().await.unwrap().unwrap();
let signing_result: Result<(String, Signature), String> =
serde_json::from_slice(&chunk).unwrap();
assert_eq!(signing_result.clone().unwrap().0.len(), 88);
let mut decoded_sig = base64::decode(signing_result.clone().unwrap().0).unwrap();
let recovery_digit = decoded_sig.pop().unwrap();
let signature = k256Signature::from_slice(&decoded_sig).unwrap();
let recover_id = RecoveryId::from_byte(recovery_digit).unwrap();
let recovery_key_from_sig = VerifyingKey::recover_from_prehash(
&message_should_succeed_hash,
&signature,
recover_id,
)
.unwrap();
assert_eq!(users_keyshare_option.clone().unwrap().verifying_key(), recovery_key_from_sig);
let mnemonic = if i == 0 { DEFAULT_MNEMONIC } else { DEFAULT_BOB_MNEMONIC };
let sk = <sr25519::Pair as Pair>::from_string(mnemonic, None).unwrap();
let sig_recovery = <sr25519::Pair as Pair>::verify(
&signing_result.clone().unwrap().1,
base64::decode(signing_result.unwrap().0).unwrap(),
&sr25519::Public(sk.public().0),
);
assert!(sig_recovery);
i += 1;
}
verify_signature(test_user_res, message_should_succeed_hash, users_keyshare_option.clone())
.await;

generic_msg.timestamp = SystemTime::now();
// test failing cases
Expand Down Expand Up @@ -885,13 +814,12 @@ async fn test_sign_tx_user_participates() {
let mut responses = test_user_failed_x25519_pub_key.into_iter();
assert_eq!(
responses.next().unwrap().unwrap().text().await.unwrap(),
"{\"Err\":\"Subscribe message rejected: Decryption(\\\"Public key does not match that \
given in UserTransactionRequest or register transaction\\\")\"}"
"{\"Err\":\"Timed out waiting for remote party\"}"
);

assert_eq!(
responses.next().unwrap().unwrap().text().await.unwrap(),
"{\"Err\":\"Oneshot timeout error: channel closed\"}"
"{\"Err\":\"Timed out waiting for remote party\"}"
);

// Test attempting to connect over ws by someone who is not in the signing group
Expand Down Expand Up @@ -1132,3 +1060,40 @@ async fn test_register_with_private_key_visibility() {

assert!(keyshare_result.is_ok());
}

pub async fn verify_signature(
test_user_res: Vec<Result<reqwest::Response, reqwest::Error>>,
message_should_succeed_hash: [u8; 32],
keyshare_option: Option<KeyShare<KeyParams>>,
) {
let mut i = 0;
for res in test_user_res {
let mut res = res.unwrap();

assert_eq!(res.status(), 200);
let chunk = res.chunk().await.unwrap().unwrap();
let signing_result: Result<(String, Signature), String> =
serde_json::from_slice(&chunk).unwrap();
assert_eq!(signing_result.clone().unwrap().0.len(), 88);
let mut decoded_sig = base64::decode(signing_result.clone().unwrap().0).unwrap();
let recovery_digit = decoded_sig.pop().unwrap();
let signature = k256Signature::from_slice(&decoded_sig).unwrap();
let recover_id = RecoveryId::from_byte(recovery_digit).unwrap();
let recovery_key_from_sig = VerifyingKey::recover_from_prehash(
&message_should_succeed_hash,
&signature,
recover_id,
)
.unwrap();
assert_eq!(keyshare_option.clone().unwrap().verifying_key(), recovery_key_from_sig);
let mnemonic = if i == 0 { DEFAULT_MNEMONIC } else { DEFAULT_BOB_MNEMONIC };
let sk = <sr25519::Pair as Pair>::from_string(mnemonic, None).unwrap();
let sig_recovery = <sr25519::Pair as Pair>::verify(
&signing_result.clone().unwrap().1,
base64::decode(signing_result.unwrap().0).unwrap(),
&sr25519::Public(sk.public().0),
);
assert!(sig_recovery);
i += 1;
}
}

0 comments on commit 753ebfa

Please sign in to comment.