init validator tx - verify ownership of used keys

.changelog/unreleased/improvements/2163-init-validator-own-keys.md

@@ -0,0 +1,2 @@
+- Require to verify ownership of all validator keys when initialized on-chain.
+  ([\#2163](https://github.com/anoma/namada/pull/2163))

apps/src/lib/client/tx.rs

@@ -27,7 +27,9 @@ use rand::rngs::OsRng;
 use super::rpc;
 use crate::cli::{args, safe_exit};
 use crate::client::rpc::query_wasm_code_hash;
-use crate::client::tx::signing::{default_sign, SigningTxData};
+use crate::client::tx::signing::{
+    default_sign, init_validator_signing_data, SigningTxData,
+};
 use crate::client::tx::tx::ProcessTxResponse;
 use crate::config::TendermintMode;
 use crate::facade::tendermint_rpc::endpoint::broadcast::tx_sync::Response;
@@ -363,6 +365,7 @@ pub async fn submit_init_validator<'a>(
 
     let validator_key_alias = format!("{}-key", alias);
     let consensus_key_alias = format!("{}-consensus-key", alias);
+    let protocol_key_alias = format!("{}-protocol-key", alias);
 
     let threshold = match threshold {
         Some(threshold) => threshold,
@@ -477,7 +480,25 @@ pub async fn submit_init_validator<'a>(
         scheme,
     )
     .unwrap();
-    let protocol_key = validator_keys.get_protocol_keypair().ref_to();
+    let protocol_sk = validator_keys.get_protocol_keypair();
+    let protocol_key = protocol_sk.to_public();
+
+    // Store the protocol key in the wallet so that we can sign the tx with it
+    // to verify ownership
+    display_line!(namada.io(), "Storing protocol key in the wallet...");
+    let password = read_and_confirm_encryption_password(unsafe_dont_encrypt);
+    namada
+        .wallet_mut()
+        .await
+        .insert_keypair(
+            protocol_key_alias,
+            tx_args.wallet_alias_force,
+            protocol_sk.clone(),
+            password,
+            None,
+            None,
+        )
+        .map_err(|err| error::Error::Other(err.to_string()))?;
 
     let validator_vp_code_hash =
         query_wasm_code_hash(namada, validator_vp_code_path.to_str().unwrap())
@@ -546,9 +567,17 @@ pub async fn submit_init_validator<'a>(
         validator_vp_code_hash: extra_section_hash,
     };
 
+    // Put together all the PKs that we have to sign with to verify ownership
+    let mut all_pks = data.account_keys.clone();
+    all_pks.push(consensus_key.to_public());
+    all_pks.push(eth_cold_pk);
+    all_pks.push(eth_hot_pk);
+    all_pks.push(data.protocol_key.clone());
+
     tx.add_code_from_hash(tx_code_hash).add_data(data);
 
-    let signing_data = aux_signing_data(namada, &tx_args, None, None).await?;
+    let signing_data =
+        init_validator_signing_data(namada, &tx_args, all_pks).await?;
 
     tx::prepare_tx(
         namada,

sdk/src/signing.rs

@@ -384,6 +384,51 @@ pub async fn aux_signing_data<'a>(
     })
 }
 
+pub async fn init_validator_signing_data<'a>(
+    context: &impl Namada<'a>,
+    args: &args::Tx<SdkTypes>,
+    validator_keys: Vec<common::PublicKey>,
+) -> Result<SigningTxData, Error> {
+    let mut public_keys = if args.wrapper_fee_payer.is_none() {
+        tx_signers(context, args, None).await?
+    } else {
+        vec![]
+    };
+    public_keys.extend(validator_keys.clone());
+
+    let account_public_keys_map =
+        Some(AccountPublicKeysMap::from_iter(validator_keys));
+
+    let fee_payer = if args.disposable_signing_key {
+        context
+            .wallet_mut()
+            .await
+            .gen_disposable_signing_key(&mut OsRng)
+            .to_public()
+    } else {
+        match &args.wrapper_fee_payer {
+            Some(keypair) => keypair.to_public(),
+            None => public_keys.get(0).ok_or(TxError::InvalidFeePayer)?.clone(),
+        }
+    };
+
+    if fee_payer == masp_tx_key().to_public() {
+        other_err(
+            "The gas payer cannot be the MASP, please provide a different gas \
+             payer."
+                .to_string(),
+        )?;
+    }
+
+    Ok(SigningTxData {
+        owner: None,
+        public_keys,
+        threshold: 0,
+        account_public_keys_map,
+        fee_payer,
+    })
+}
+
 /// Informations about the post-tx balance of the tx's source. Used to correctly
 /// handle fee validation in the wrapper tx
 pub struct TxSourcePostBalance {

shared/src/vm/host_env.rs

@@ -2076,6 +2076,85 @@ where
     sentinel.set_invalid_commitment();
 }
 
+/// Verify a transaction signature
+#[allow(clippy::too_many_arguments)]
+pub fn tx_verify_tx_section_signature<MEM, DB, H, CA>(
+    env: &TxVmEnv<MEM, DB, H, CA>,
+    hash_list_ptr: u64,
+    hash_list_len: u64,
+    public_keys_map_ptr: u64,
+    public_keys_map_len: u64,
+    threshold: u8,
+    max_signatures_ptr: u64,
+    max_signatures_len: u64,
+) -> TxResult<i64>
+where
+    MEM: VmMemory,
+    DB: storage::DB + for<'iter> storage::DBIter<'iter>,
+    H: StorageHasher,
+    CA: WasmCacheAccess,
+{
+    let (hash_list, gas) = env
+        .memory
+        .read_bytes(hash_list_ptr, hash_list_len as _)
+        .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
+
+    let sentinel = unsafe { env.ctx.sentinel.get() };
+    tx_charge_gas(env, gas)?;
+    let hashes = <[Hash; 1]>::try_from_slice(&hash_list)
+        .map_err(TxRuntimeError::EncodingError)?;
+
+    let (public_keys_map, gas) = env
+        .memory
+        .read_bytes(public_keys_map_ptr, public_keys_map_len as _)
+        .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
+    tx_charge_gas(env, gas)?;
+    let public_keys_map =
+        namada_core::types::account::AccountPublicKeysMap::try_from_slice(
+            &public_keys_map,
+        )
+        .map_err(TxRuntimeError::EncodingError)?;
+
+    tx_charge_gas(env, gas)?;
+
+    let (max_signatures, gas) = env
+        .memory
+        .read_bytes(max_signatures_ptr, max_signatures_len as _)
+        .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?;
+    tx_charge_gas(env, gas)?;
+    let max_signatures = Option::<u8>::try_from_slice(&max_signatures)
+        .map_err(TxRuntimeError::EncodingError)?;
+
+    let tx = unsafe { env.ctx.tx.get() };
+
+    tx_charge_gas(
+        env,
+        gas::VERIFY_TX_SIG_GAS * public_keys_map.idx_to_pk.len() as u64,
+    )?;
+    match tx.verify_signatures(
+        &hashes,
+        public_keys_map,
+        &None,
+        threshold,
+        max_signatures,
+        // This uses VpGasMeter, so we're charging the gas here manually
+        // instead
+        &mut None,
+    ) {
+        Ok(_) => Ok(HostEnvResult::Success.to_i64()),
+        Err(err) => match err {
+            namada_core::proto::Error::OutOfGas(inner) => {
+                sentinel.set_out_of_gas();
+                Err(TxRuntimeError::OutOfGas(inner))
+            }
+            namada_core::proto::Error::InvalidSectionSignature(_) => {
+                Ok(HostEnvResult::Fail.to_i64())
+            }
+            _ => Ok(HostEnvResult::Fail.to_i64()),
+        },
+    }
+}
+
 /// Evaluate a validity predicate with the given input data.
 pub fn vp_eval<MEM, DB, H, EVAL, CA>(
     env: &VpVmEnv<'static, MEM, DB, H, EVAL, CA>,

shared/src/vm/wasm/host_env.rs

@@ -86,7 +86,8 @@ where
             "namada_tx_get_native_token" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_get_native_token),
             "namada_tx_log_string" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_log_string),
             "namada_tx_ibc_execute" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_ibc_execute),
-            "namada_tx_set_commitment_sentinel" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_set_commitment_sentinel)
+            "namada_tx_set_commitment_sentinel" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_set_commitment_sentinel),
+            "namada_tx_verify_tx_section_signature" => Function::new_native_with_env(wasm_store, env.clone(), host_env::tx_verify_tx_section_signature),
         },
     }
 }

tests/src/vm_host_env/tx.rs

@@ -507,4 +507,13 @@ mod native_tx_host_env {
     native_host_fn!(tx_log_string(str_ptr: u64, str_len: u64));
     native_host_fn!(tx_charge_gas(used_gas: u64));
     native_host_fn!("non-result", tx_set_commitment_sentinel());
+    native_host_fn!(tx_verify_tx_section_signature(
+        hash_list_ptr: u64,
+        hash_list_len: u64,
+        public_keys_map_ptr: u64,
+        public_keys_map_len: u64,
+        threshold: u8,
+        max_signatures_ptr: u64,
+        max_signatures_len: u64,
+    ) -> i64);
 }

tx_prelude/src/lib.rs

@@ -19,7 +19,6 @@ use std::marker::PhantomData;
 pub use borsh::{BorshDeserialize, BorshSerialize};
 pub use borsh_ext;
 use borsh_ext::BorshSerializeExt;
-pub use namada_core::ledger::eth_bridge;
 pub use namada_core::ledger::governance::storage as gov_storage;
 pub use namada_core::ledger::parameters::storage as parameters_storage;
 pub use namada_core::ledger::storage::types::encode;
@@ -28,11 +27,14 @@ pub use namada_core::ledger::storage_api::{
     ResultExt, StorageRead, StorageWrite,
 };
 pub use namada_core::ledger::tx_env::TxEnv;
+pub use namada_core::ledger::{eth_bridge, parameters};
 pub use namada_core::proto::{Section, Tx};
+use namada_core::types::account::AccountPublicKeysMap;
 pub use namada_core::types::address::Address;
 use namada_core::types::chain::CHAIN_ID_LENGTH;
 pub use namada_core::types::ethereum_events::EthAddress;
 use namada_core::types::internal::HostEnvResult;
+use namada_core::types::key::common;
 use namada_core::types::storage::TxIndex;
 pub use namada_core::types::storage::{
     self, BlockHash, BlockHeight, Epoch, Header, BLOCK_HASH_LENGTH,
@@ -361,3 +363,36 @@ impl TxEnv for Ctx {
 pub fn tx_ibc_execute() {
     unsafe { namada_tx_ibc_execute() }
 }
+
+/// Verify section signatures against the given list of keys
+pub fn verify_signatures_of_pks(
+    ctx: &Ctx,
+    tx: &Tx,
+    pks: Vec<common::PublicKey>,
+) -> EnvResult<bool> {
+    let max_signatures_per_transaction =
+        parameters::max_signatures_per_transaction(ctx)?;
+
+    // Require signatures from all the given keys
+    let threshold = u8::try_from(pks.len()).into_storage_result()?;
+    let public_keys_index_map = AccountPublicKeysMap::from_iter(pks);
+
+    // Serialize parameters
+    let max_signatures = max_signatures_per_transaction.serialize_to_vec();
+    let public_keys_map = public_keys_index_map.serialize_to_vec();
+    let targets = [tx.raw_header_hash()].serialize_to_vec();
+
+    let valid = unsafe {
+        namada_tx_verify_tx_section_signature(
+            targets.as_ptr() as _,
+            targets.len() as _,
+            public_keys_map.as_ptr() as _,
+            public_keys_map.len() as _,
+            threshold,
+            max_signatures.as_ptr() as _,
+            max_signatures.len() as _,
+        )
+    };
+
+    Ok(HostEnvResult::is_success(valid))
+}

vm_env/src/lib.rs

@@ -117,6 +117,18 @@ pub mod tx {
 
         /// Set the sentinel for a wrong tx section commitment
         pub fn namada_tx_set_commitment_sentinel();
+
+        // Verify the signatures of a tx
+        pub fn namada_tx_verify_tx_section_signature(
+            hash_list_ptr: u64,
+            hash_list_len: u64,
+            public_keys_map_ptr: u64,
+            public_keys_map_len: u64,
+            threshold: u8,
+            max_signatures_ptr: u64,
+            max_signatures_len: u64,
+        ) -> i64;
+
     }
 }
 

wasm/wasm_source/src/tx_init_validator.rs

@@ -32,6 +32,22 @@ fn apply_tx(ctx: &mut Ctx, tx_data: Tx) -> TxResult {
         .code
         .hash();
 
+    // Check that the tx has been signed with all the keys to be used for the
+    // validator account
+    let mut all_pks = init_validator.account_keys.clone();
+    all_pks.push(init_validator.consensus_key.clone());
+    all_pks.push(key::common::PublicKey::Secp256k1(
+        init_validator.eth_cold_key.clone(),
+    ));
+    all_pks.push(key::common::PublicKey::Secp256k1(
+        init_validator.eth_hot_key.clone(),
+    ));
+    all_pks.push(init_validator.protocol_key.clone());
+    if !matches!(verify_signatures_of_pks(ctx, &signed, all_pks), Ok(true)) {
+        debug_log!("Keys ownership signature verification failed");
+        panic!()
+    }
+
     // Register the validator in PoS
     match ctx.init_validator(init_validator, validator_vp_code_hash) {
         Ok(validator_address) => {