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

merge queue: embarking main (fc5fe33) and #3843 together #3845

Closed
wants to merge 4 commits into from
Closed
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
3 changes: 3 additions & 0 deletions .changelog/unreleased/improvements/3843-check-genesis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Validate a chain ID of genesis on ABCI InitChain request
prior to applying it to ensure it's not been tampered with.
([\#3843](https://github.com/anoma/namada/pull/3843))
3 changes: 2 additions & 1 deletion .github/workflows/scripts/e2e.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,6 @@
"e2e::wallet_tests::wallet_encrypted_key_cmds_env_var": 1,
"e2e::wallet_tests::wallet_unencrypted_key_cmds": 1,
"e2e::ledger_tests::masp_txs_and_queries": 82,
"e2e::ledger_tests::test_genesis_chain_id_change": 35
"e2e::ledger_tests::test_genesis_chain_id_change": 35,
"e2e::ledger_tests::test_genesis_manipulation": 103
}
73 changes: 69 additions & 4 deletions crates/apps_lib/src/config/genesis/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use std::str::FromStr;

use borsh::{BorshDeserialize, BorshSerialize};
use borsh_ext::BorshSerializeExt;
use eyre::eyre;
use namada_macros::BorshDeserializer;
#[cfg(feature = "migrations")]
use namada_migrations::*;
Expand Down Expand Up @@ -106,6 +107,8 @@ impl Finalized {

/// Try to read all genesis and the chain metadata TOML files from the given
/// directory.
///
/// The consistency of the files is checked with [`Finalized::is_valid`].
pub fn read_toml_files(input_dir: &Path) -> eyre::Result<Self> {
let vps_file = input_dir.join(templates::VPS_FILE_NAME);
let tokens_file = input_dir.join(templates::TOKENS_FILE_NAME);
Expand All @@ -121,14 +124,20 @@ impl Finalized {
let parameters = read_toml(&parameters_file, "Parameters")?;
let transactions = read_toml(&transactions_file, "Transactions")?;
let metadata = read_toml(&metadata_file, "Chain metadata")?;
Ok(Self {
let genesis = Self {
vps,
tokens,
balances,
parameters,
transactions,
metadata,
})
};

if !genesis.is_valid() {
return Err(eyre!("Invalid genesis files"));
}

Ok(genesis)
}

/// Find the address of the configured native token
Expand Down Expand Up @@ -485,6 +494,54 @@ impl Finalized {
pub fn get_token_address(&self, alias: &Alias) -> Option<&Address> {
self.tokens.token.get(alias).map(|token| &token.address)
}

// Validate the chain ID against the genesis contents
pub fn is_valid(&self) -> bool {
let Self {
vps,
tokens,
balances,
parameters,
transactions,
metadata,
} = self.clone();
let Metadata {
chain_id,
genesis_time,
consensus_timeout_commit,
address_gen,
} = metadata.clone();

let Some(chain_id_prefix) = chain_id.prefix() else {
tracing::warn!(
"Invalid Chain ID \"{chain_id}\" - unable to find a prefix"
);
return false;
};
let metadata = Metadata {
chain_id: chain_id_prefix.clone(),
genesis_time,
consensus_timeout_commit,
address_gen,
};
let to_finalize = ToFinalize {
vps,
tokens,
balances,
parameters,
transactions,
metadata,
};
let derived_chain_id = derive_chain_id(chain_id_prefix, &to_finalize);
let is_valid = derived_chain_id == chain_id;
if !is_valid {
tracing::warn!(
"Invalid chain ID. This indicates that something in the \
genesis files might have been modified."
);
}
is_valid
}
}

/// Create the [`Finalized`] chain configuration. Derives the chain ID from the
Expand Down Expand Up @@ -541,8 +598,7 @@ pub fn finalize(
parameters,
transactions,
};
let to_finalize_bytes = to_finalize.serialize_to_vec();
let chain_id = ChainId::from_genesis(chain_id_prefix, to_finalize_bytes);
let chain_id = derive_chain_id(chain_id_prefix, &to_finalize);

// Construct the `Finalized` chain
let ToFinalize {
Expand Down Expand Up @@ -575,6 +631,15 @@ pub fn finalize(
}
}

/// Derive a chain ID from genesis contents
pub fn derive_chain_id(
chain_id_prefix: ChainIdPrefix,
to_finalize: &ToFinalize,
) -> ChainId {
let to_finalize_bytes = to_finalize.serialize_to_vec();
ChainId::from_genesis(chain_id_prefix, to_finalize_bytes)
}

/// Chain genesis config to be finalized. This struct is used to derive the
/// chain ID to construct a [`Finalized`] chain genesis config.
#[derive(
Expand Down
7 changes: 7 additions & 0 deletions crates/core/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,13 @@ impl ChainId {
}
errors
}

/// Find the prefix of a valid ChainId.
pub fn prefix(&self) -> Option<ChainIdPrefix> {
let ChainId(chain_id) = self;
let (prefix, _) = chain_id.rsplit_once(CHAIN_ID_PREFIX_SEP)?;
Some(ChainIdPrefix(prefix.to_string()))
}
}

/// Height of a block, i.e. the level. The `default` is the
Expand Down
4 changes: 2 additions & 2 deletions crates/node/src/shell/init_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,14 +133,14 @@ where
let genesis = {
let chain_dir = self.base_dir.join(chain_id);
genesis::chain::Finalized::read_toml_files(&chain_dir)
.expect("Missing genesis files")
.expect("Missing or invalid genesis files")
};
#[cfg(any(test, fuzzing, feature = "benches"))]
let genesis = {
let chain_dir = self.base_dir.join(chain_id);
if chain_dir.join(genesis::chain::METADATA_FILE_NAME).exists() {
genesis::chain::Finalized::read_toml_files(&chain_dir)
.expect("Missing genesis files")
.expect("Missing or invalid genesis files")
} else {
genesis::make_dev_genesis(num_validators, &chain_dir)
}
Expand Down
74 changes: 72 additions & 2 deletions crates/tests/src/e2e/ledger_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ use color_eyre::eyre::Result;
use color_eyre::owo_colors::OwoColorize;
use namada_apps_lib::cli::context::ENV_VAR_CHAIN_ID;
use namada_apps_lib::client::utils::PRE_GENESIS_DIR;
use namada_apps_lib::config::genesis::chain;
use namada_apps_lib::config::genesis::templates::TokenBalances;
use namada_apps_lib::config::utils::convert_tm_addr_to_socket_addr;
use namada_apps_lib::config::{self, ethereum_bridge};
use namada_apps_lib::tendermint_config::net::Address as TendermintAddress;
use namada_apps_lib::wallet;
use namada_apps_lib::wallet::{self, Alias};
use namada_core::chain::ChainId;
use namada_core::token::NATIVE_MAX_DECIMAL_PLACES;
use namada_sdk::address::Address;
use namada_sdk::chain::Epoch;
use namada_sdk::chain::{ChainIdPrefix, Epoch};
use namada_sdk::time::DateTimeUtc;
use namada_sdk::token;
use namada_test_utils::TestWasms;
Expand Down Expand Up @@ -2738,3 +2740,71 @@ fn test_genesis_chain_id_change() -> Result<()> {

Ok(())
}

/// Test that any changes done to a genesis config after a chain is finalized
/// will make it fail validation.
#[test]
fn test_genesis_manipulation() -> Result<()> {
let test = setup::single_node_net().unwrap();

set_ethereum_bridge_mode(
&test,
&test.net.chain_id,
Who::Validator(0),
ethereum_bridge::ledger::Mode::Off,
None,
);

let chain_dir = test.get_chain_dir(Who::Validator(0));
let genesis = chain::Finalized::read_toml_files(&chain_dir).unwrap();

let modified_genesis = [
{
let mut genesis = genesis.clone();
genesis
.balances
.token
.insert(Alias::from("test"), TokenBalances(Default::default()));
genesis
},
{
let mut genesis = genesis.clone();
genesis.balances.token.remove(&Alias::from("NAM"));
genesis
},
{
let mut genesis = genesis.clone();
genesis.metadata.address_gen = None;
genesis
},
{
let mut genesis = genesis.clone();
// Invalid chain ID
genesis.metadata.chain_id = ChainId("Invalid ID".to_string());
genesis
},
{
let mut genesis = genesis.clone();
// Random valid chain ID
genesis.metadata.chain_id = ChainId::from_genesis(
ChainIdPrefix::from_str("TEST").unwrap(),
[1, 2, 3],
);
genesis
},
];

for genesis in modified_genesis {
// Any modification should invalide the genesis
assert!(!genesis.is_valid());

genesis.write_toml_files(&chain_dir).unwrap();

// A node should fail to start-up
let result =
start_namada_ledger_node_wait_wasm(&test, Some(0), Some(40));
assert!(result.is_err())
}

Ok(())
}