diff --git a/crypto/server/src/user/api.rs b/crypto/server/src/user/api.rs index ea8add194..859735920 100644 --- a/crypto/server/src/user/api.rs +++ b/crypto/server/src/user/api.rs @@ -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?; @@ -369,7 +374,7 @@ pub async fn confirm_registered( pub async fn get_current_subgroup_signers( api: &OnlineClient, sig_hash: &str, -) -> Result, UserErr> { +) -> Result, UserErr> { let mut subgroup_signers = vec![]; let number = Arc::new(BigInt::from_str_radix(sig_hash, 16)?); let futures = (0..SIGNING_PARTY_SIZE) @@ -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::>(); @@ -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, + subgroup_signers: &[ValidatorInfo], validators_info: &Vec, my_id: &::AccountId, ) -> Result<(), UserErr> { + let subgroup_signer_ids: Vec = + 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", )); diff --git a/crypto/server/src/user/tests.rs b/crypto/server/src/user/tests.rs index 07fe16989..d45a1da60 100644 --- a/crypto/server/src/user/tests.rs +++ b/crypto/server/src/user/tests.rs @@ -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::{ @@ -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, @@ -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 = ::from_string(mnemonic, None).unwrap(); - let sig_recovery = ::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::>(); + 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 @@ -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 { @@ -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() @@ -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 = ::from_string(mnemonic, None).unwrap(); - let sig_recovery = ::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 @@ -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 @@ -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>, + message_should_succeed_hash: [u8; 32], + keyshare_option: Option>, +) { + 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 = ::from_string(mnemonic, None).unwrap(); + let sig_recovery = ::verify( + &signing_result.clone().unwrap().1, + base64::decode(signing_result.unwrap().0).unwrap(), + &sr25519::Public(sk.public().0), + ); + assert!(sig_recovery); + i += 1; + } +}