From 6aa5788354d7dcda5693625a94df854bc7c2125f Mon Sep 17 00:00:00 2001 From: Reisen Date: Mon, 5 Jul 2021 09:59:57 +0000 Subject: [PATCH] Confirm guardian set fails for non-gov emitter Change-Id: Idc49a19be551d4a3c4ae7cafe735995fa4ced0be --- solana/bridge/program/src/api/governance.rs | 11 +++ solana/bridge/program/src/api/post_message.rs | 4 +- solana/bridge/program/src/instructions.rs | 3 +- solana/bridge/program/src/lib.rs | 1 + solana/bridge/program/src/vaa.rs | 2 + solana/bridge/program/tests/common.rs | 2 + solana/bridge/program/tests/integration.rs | 96 +++++++++++-------- 7 files changed, 75 insertions(+), 44 deletions(-) diff --git a/solana/bridge/program/src/api/governance.rs b/solana/bridge/program/src/api/governance.rs index 212e4c7a..b28e88d2 100644 --- a/solana/bridge/program/src/api/governance.rs +++ b/solana/bridge/program/src/api/governance.rs @@ -24,6 +24,7 @@ use crate::{ vaa::ClaimableVAA, Error::{ InvalidFeeRecipient, + InvalidGovernanceKey, InvalidGuardianSetUpgrade, }, }; @@ -110,6 +111,16 @@ pub fn upgrade_guardian_set( accs: &mut UpgradeGuardianSet, _data: UpgradeGuardianSetData, ) -> Result<()> { + // Enforce only the expected governance key. + if format!( + "{}", + Pubkey::new_from_array(accs.vaa.message.meta().emitter_address) + ) != std::env!("EMITTER_ADDRESS") + || accs.vaa.message.meta().emitter_chain != 1 + { + return Err(InvalidGovernanceKey.into()); + } + accs.vaa.claim(ctx, accs.payer.key)?; // Set expiration time for the old set diff --git a/solana/bridge/program/src/api/post_message.rs b/solana/bridge/program/src/api/post_message.rs index 0cb80f21..335a0abb 100644 --- a/solana/bridge/program/src/api/post_message.rs +++ b/solana/bridge/program/src/api/post_message.rs @@ -79,6 +79,8 @@ pub fn post_message( data: PostMessageData, ) -> Result<()> { trace!("Message Address: {}", accs.message.info().key); + trace!("Emitter Address: {}", accs.emitter.info().key); + trace!("Nonce: {}", data.nonce); let msg_derivation = MessageDerivationData { emitter_key: accs.emitter.key.to_bytes(), @@ -88,8 +90,6 @@ pub fn post_message( sequence: None, }; - trace!("Verifying Message: {}, {}", accs.emitter.key, data.nonce); - accs.message .verify_derivation(ctx.program_id, &msg_derivation)?; diff --git a/solana/bridge/program/src/instructions.rs b/solana/bridge/program/src/instructions.rs index 6ff25660..fad321fe 100644 --- a/solana/bridge/program/src/instructions.rs +++ b/solana/bridge/program/src/instructions.rs @@ -248,13 +248,14 @@ pub fn upgrade_guardian_set( emitter: Pubkey, old_index: u32, new_index: u32, + sequence: u64, ) -> Instruction { let bridge = Bridge::<'_, { AccountState::Uninitialized }>::key(None, &program_id); let claim = Claim::<'_, { AccountState::Uninitialized }>::key( &ClaimDerivationData { emitter_address: emitter.to_bytes(), emitter_chain: CHAIN_ID_SOLANA, - sequence: 1, + sequence: sequence, }, &program_id, ); diff --git a/solana/bridge/program/src/lib.rs b/solana/bridge/program/src/lib.rs index 09cd29df..5445c473 100644 --- a/solana/bridge/program/src/lib.rs +++ b/solana/bridge/program/src/lib.rs @@ -61,6 +61,7 @@ enum Error { InvalidFeeRecipient, InvalidGovernanceAction, InvalidGovernanceChain, + InvalidGovernanceKey, InvalidGovernanceModule, InvalidGuardianSetUpgrade, InvalidHash, diff --git a/solana/bridge/program/src/vaa.rs b/solana/bridge/program/src/vaa.rs index ca35b44e..91ad55bb 100644 --- a/solana/bridge/program/src/vaa.rs +++ b/solana/bridge/program/src/vaa.rs @@ -20,6 +20,7 @@ use byteorder::{ use solana_program::pubkey::Pubkey; use solitaire::{ processors::seeded::Seeded, + trace, Context, CreationLamports::Exempt, Data, @@ -137,6 +138,7 @@ impl<'b, T: DeserializePayload> InstructionContext<'b> for ClaimableVAA<'b, T> { // Do the Posted Message verification // Verify that the claim account is derived correctly + trace!("Seq: {}", self.message.meta().sequence); self.claim.verify_derivation( program_id, &ClaimDerivationData { diff --git a/solana/bridge/program/tests/common.rs b/solana/bridge/program/tests/common.rs index 38aa44d1..407600b1 100644 --- a/solana/bridge/program/tests/common.rs +++ b/solana/bridge/program/tests/common.rs @@ -337,6 +337,7 @@ mod helpers { emitter: Pubkey, old_index: u32, new_index: u32, + sequence: u64, ) -> Result { execute( client, @@ -349,6 +350,7 @@ mod helpers { emitter, old_index, new_index, + sequence, )], ) } diff --git a/solana/bridge/program/tests/integration.rs b/solana/bridge/program/tests/integration.rs index 49e7f15b..5a57df93 100644 --- a/solana/bridge/program/tests/integration.rs +++ b/solana/bridge/program/tests/integration.rs @@ -68,20 +68,37 @@ use bridge::{ mod common; +const GOV_KEY: [u8; 64] = [ + 240, 133, 120, 113, 30, 67, 38, 184, 197, 72, 234, 99, 241, 21, 58, 225, 41, 157, 171, 44, + 196, 163, 134, 236, 92, 148, 110, 68, 127, 114, 177, 0, 173, 253, 199, 9, 242, 142, 201, + 174, 108, 197, 18, 102, 115, 0, 31, 205, 127, 188, 191, 56, 171, 228, 20, 247, 149, 170, + 141, 231, 147, 88, 97, 199, +]; + +struct Context { + public: Vec<[u8; 20]>, + secret: Vec, +} #[test] fn run_integration_tests() { let (ref payer, ref client, ref program) = common::setup(); - let (public_keys, secret_keys) = common::generate_keys(19); + let (public_keys, secret_keys) = common::generate_keys(6); + let mut context = Context { + public: public_keys, + secret: secret_keys, + }; - common::initialize(client, program, payer, &*public_keys); + common::initialize(client, program, payer, &*context.public.clone()); - test_bridge_messages(&public_keys, &secret_keys); - test_guardian_set_change(&public_keys, &secret_keys); - test_guardian_set_change_fails(&public_keys, &secret_keys); + // Tests are currently unhygienic as It's difficult to wrap `solana-test-validator` within the + // integration tests so for now we work around it by simply chain-calling our tests. + test_bridge_messages(&mut context); + test_guardian_set_change(&mut context); + test_guardian_set_change_fails(&mut context); } -fn test_bridge_messages(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) { +fn test_bridge_messages(context: &mut Context) { let (ref payer, ref client, ref program) = common::setup(); // Data/Nonce used for emitting a message we want to prove exists. @@ -90,33 +107,34 @@ fn test_bridge_messages(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) { let emitter = Keypair::new(); // Post the message, publishing the data for guardian consumption. - let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()); + let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()).unwrap(); // Emulate Guardian behaviour, verifying the data and publishing signatures/VAA. let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 0); - common::verify_signatures(client, program, payer, body, body_hash, &secret_keys, 0); - common::post_vaa(client, program, payer, vaa); + common::verify_signatures(client, program, payer, body, body_hash, &context.secret, 0).unwrap(); + common::post_vaa(client, program, payer, vaa).unwrap(); } -fn test_guardian_set_change(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) { +fn test_guardian_set_change(context: &mut Context) { // Initialize a wormhole bridge on Solana to test with. let (ref payer, ref client, ref program) = common::setup(); // Data/Nonce used for emitting a message we want to prove exists. let nonce = 12397; let message = b"Prove Me".to_vec(); - let emitter = Keypair::new(); + let emitter = Keypair::from_bytes(&GOV_KEY).unwrap(); // Post the message, publishing the data for guardian consumption. - let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()); + let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()).unwrap(); // Emulate Guardian behaviour, verifying the data and publishing signatures/VAA. let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 0); - common::verify_signatures(client, program, payer, body, body_hash, &secret_keys, 0); - common::post_vaa(client, program, payer, vaa); + common::verify_signatures(client, program, payer, body, body_hash, &context.secret, 0).unwrap(); + common::post_vaa(client, program, payer, vaa).unwrap(); // Upgrade the guardian set with a new set of guardians. - let (new_public_keys, new_secret_keys) = common::generate_keys(3); + let (new_public_keys, new_secret_keys) = common::generate_keys(6); + let nonce = 12398; let message = GovernancePayloadGuardianSetChange { new_guardian_set_index: 1, @@ -125,8 +143,8 @@ fn test_guardian_set_change(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()).unwrap(); let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 0); - common::verify_signatures(client, program, payer, body, body_hash, &secret_keys, 0); - common::post_vaa(client, program, payer, vaa); + common::verify_signatures(client, program, payer, body, body_hash, &context.secret, 0).unwrap(); + common::post_vaa(client, program, payer, vaa).unwrap(); common::upgrade_guardian_set( client, @@ -136,53 +154,49 @@ fn test_guardian_set_change(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) emitter.pubkey(), 0, 1, - ); + 1, + ).unwrap(); // Submit the message a second time with a new nonce. let nonce = 12399; - let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()); + let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()).unwrap(); + + context.public = new_public_keys; + context.secret = new_secret_keys; // Emulate Guardian behaviour, verifying the data and publishing signatures/VAA. let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 1); - common::verify_signatures(client, program, payer, body, body_hash, &new_secret_keys, 1); - common::post_vaa(client, program, payer, vaa); + common::verify_signatures(client, program, payer, body, body_hash, &context.secret, 1).unwrap(); + common::post_vaa(client, program, payer, vaa).unwrap(); } -fn test_guardian_set_change_fails(public_keys: &[[u8; 20]], secret_keys: &[SecretKey]) { +fn test_guardian_set_change_fails(context: &mut Context) { // Initialize a wormhole bridge on Solana to test with. let (ref payer, ref client, ref program) = common::setup(); - - // Data/Nonce used for emitting a message we want to prove exists. - let nonce = 12397; - let message = b"Prove Me".to_vec(); let emitter = Keypair::new(); - // Post the message, publishing the data for guardian consumption. - let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()); - - // Emulate Guardian behaviour, verifying the data and publishing signatures/VAA. - let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 0); - common::verify_signatures(client, program, payer, body, body_hash, &secret_keys, 0); - common::post_vaa(client, program, payer, vaa); - // Upgrade the guardian set with a new set of guardians. - let (new_public_keys, new_secret_keys) = common::generate_keys(3); - let nonce = 12398; + let (new_public_keys, new_secret_keys) = common::generate_keys(6); + let nonce = 12400; let message = GovernancePayloadGuardianSetChange { - new_guardian_set_index: 1, + new_guardian_set_index: 2, new_guardian_set: new_public_keys.clone(), }.try_to_vec().unwrap(); let message_key = common::post_message(client, program, payer, &emitter, nonce, message.clone()).unwrap(); - let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 0); + let (vaa, body, body_hash) = common::generate_vaa(&emitter, message.clone(), nonce, 1); - common::upgrade_guardian_set( + assert!(common::upgrade_guardian_set( client, program, payer, message_key, emitter.pubkey(), - 0, 1, - ); + 2, + 0, + ).is_err()); + + context.public = new_public_keys; + context.secret = new_secret_keys; }