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

Ensure correct validator order by using ValidatorInfo from chain rather than from user #425

Merged
merged 4 commits into from
Oct 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 23 additions & 11 deletions crypto/server/src/user/api.rs
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
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;
}
}