diff --git a/aptos-node/src/storage.rs b/aptos-node/src/storage.rs index 68be47ed816..e818be5a282 100644 --- a/aptos-node/src/storage.rs +++ b/aptos-node/src/storage.rs @@ -11,7 +11,7 @@ use aptos_indexer_grpc_table_info::internal_indexer_db_service::InternalIndexerD use aptos_logger::{debug, info}; use aptos_storage_interface::{DbReader, DbReaderWriter}; use aptos_types::{ - ledger_info::{set_waypoint_version, LedgerInfoWithSignatures}, + ledger_info::LedgerInfoWithSignatures, transaction::Version, waypoint::Waypoint, }; @@ -198,7 +198,6 @@ pub fn initialize_database_and_checkpoints( ); let waypoint = node_config.base.waypoint.waypoint(); - set_waypoint_version(waypoint.version()); Ok(( db_rw, backup_service, diff --git a/consensus/consensus-types/tests/ledger_info_waypoint_regression.rs b/consensus/consensus-types/tests/ledger_info_waypoint_regression.rs new file mode 100644 index 00000000000..528fb97907e --- /dev/null +++ b/consensus/consensus-types/tests/ledger_info_waypoint_regression.rs @@ -0,0 +1,125 @@ +// Regression tests for the waypoint verification bypass (audit finding C1). +// +// Previously, `LedgerInfoWithSignatures::verify_signatures` returned `Ok(())` +// for any ledger info with `version <= waypoint_version` (a process-global set +// at node startup). Because decoupled execution gives every ordered QC and +// CommitDecision `version == 0`, the bypass covered ALL live consensus +// messages: forged messages with empty aggregate signatures were accepted. +// +// These tests pin the fixed behavior: signature verification always runs. + +use aptos_consensus_types::{ + pipeline::commit_decision::CommitDecision, quorum_cert::QuorumCert, vote_data::VoteData, +}; +use aptos_crypto::{ + hash::{CryptoHash, ACCUMULATOR_PLACEHOLDER_HASH}, + HashValue, +}; +use aptos_types::{ + aggregate_signature::AggregateSignature, + block_info::BlockInfo, + ledger_info::{LedgerInfo, LedgerInfoWithSignatures}, + validator_signer::ValidatorSigner, + validator_verifier::{ValidatorConsensusInfo, ValidatorVerifier}, +}; + +fn seven_validator_setup() -> (Vec, ValidatorVerifier) { + let signers: Vec = (0..7).map(|i| ValidatorSigner::random([i; 32])).collect(); + let infos: Vec<_> = signers + .iter() + .map(|s| ValidatorConsensusInfo::new(s.author(), s.public_key(), 1)) + .collect(); + ( + signers, + ValidatorVerifier::new_with_quorum_voting_power(infos, 5).unwrap(), + ) +} + +fn block_info(round: u64, version: u64) -> BlockInfo { + BlockInfo::new( + 1, // epoch + round, + HashValue::random(), // id + HashValue::random(), // executed_state_id + version, + 12345, + None, + ) +} + +#[test] +fn forged_commit_decision_with_empty_signature_is_rejected() { + let (_, verifier) = seven_validator_setup(); + + // The exact forgery from the C1 PoC: a CommitDecision carrying an EMPTY + // aggregate signature over a version-0 ledger info (the shape of every + // commit decision under decoupled execution). + let forged = CommitDecision::new(LedgerInfoWithSignatures::new( + LedgerInfo::new(block_info(10, 0), HashValue::zero()), + AggregateSignature::empty(), + )); + assert!( + forged.verify(&verifier).is_err(), + "a CommitDecision with no valid signatures must never verify" + ); +} + +#[test] +fn forged_version_zero_quorum_cert_with_empty_signature_is_rejected() { + let (_, verifier) = seven_validator_setup(); + + // A version-0 ordered QC — the exact shape produced by decoupled + // execution — with an empty aggregate signature. VoteData must be + // well-formed (QuorumCert::verify checks consensus_data_hash == + // vote_data.hash() and vote_data.verify()). + let parent = BlockInfo::new( + 1, + 10, + HashValue::random(), + *ACCUMULATOR_PLACEHOLDER_HASH, + 0, + 12345, + None, + ); + let proposed = BlockInfo::new( + 1, + 11, + HashValue::random(), + *ACCUMULATOR_PLACEHOLDER_HASH, + 0, + 12346, + None, + ); + let vote_data = VoteData::new(proposed, parent.clone()); + let qc_ledger_info = LedgerInfoWithSignatures::new( + LedgerInfo::new(parent, vote_data.hash()), + AggregateSignature::empty(), + ); + let qc = QuorumCert::new(vote_data, qc_ledger_info); + assert!( + qc.verify(&verifier).is_err(), + "an ordered QC with no valid signatures must never verify" + ); +} + +#[test] +fn honestly_signed_commit_decision_still_verifies() { + // Control: legitimate quorum-signed messages must keep passing. + let (signers, verifier) = seven_validator_setup(); + + let ledger_info = LedgerInfo::new(block_info(10, 0), HashValue::zero()); + // 5 of 7 voting power = quorum + let signatures: Vec<_> = signers + .iter() + .take(5) + .map(|s| (s.author(), s.sign(&ledger_info).unwrap())) + .collect(); + let aggregate = verifier + .aggregate_signatures(signatures.iter().map(|(a, s)| (a, s))) + .unwrap(); + let decision = CommitDecision::new(LedgerInfoWithSignatures::new(ledger_info, aggregate)); + assert!( + decision.verify(&verifier).is_ok(), + "a quorum-signed CommitDecision must verify" + ); +} diff --git a/state-sync/inter-component/event-notifications/src/lib.rs b/state-sync/inter-component/event-notifications/src/lib.rs index 4d695fd4199..a20ad9b1bb2 100644 --- a/state-sync/inter-component/event-notifications/src/lib.rs +++ b/state-sync/inter-component/event-notifications/src/lib.rs @@ -13,7 +13,6 @@ use aptos_storage_interface::{ use aptos_types::{ contract_event::ContractEvent, event::EventKey, - ledger_info::get_waypoint_version, on_chain_config::{ ConfigurationResource, OnChainConfig, OnChainConfigPayload, OnChainConfigProvider, }, @@ -415,11 +414,11 @@ pub struct DbBackedOnChainConfig { } impl DbBackedOnChainConfig { - pub fn new(reader: Arc, mut version: Version) -> Self { - let waypoint_version = get_waypoint_version().expect("Waypoint version is missing"); - if version < waypoint_version { - version = waypoint_version as Version; - } + /// `version` must already be clamped by the caller to the DB's first + /// available version (see `read_on_chain_configs`), so that reads never + /// target state the node does not hold (e.g. pre-waypoint history on a + /// fast-synced node). No waypoint-based adjustment happens here. + pub fn new(reader: Arc, version: Version) -> Self { Self { reader, version } } } diff --git a/state-sync/inter-component/event-notifications/src/tests.rs b/state-sync/inter-component/event-notifications/src/tests.rs index 65f5c98904b..b9f62d66157 100644 --- a/state-sync/inter-component/event-notifications/src/tests.rs +++ b/state-sync/inter-component/event-notifications/src/tests.rs @@ -17,7 +17,6 @@ use aptos_types::{ account_config::NEW_EPOCH_EVENT_V2_MOVE_TYPE_TAG, contract_event::ContractEvent, event::EventKey, - ledger_info::set_waypoint_version, on_chain_config, on_chain_config::OnChainConfig, transaction::{Transaction, Version, WriteSetPayload}, @@ -569,9 +568,7 @@ fn create_database() -> Arc> { &db_rw, &genesis_txn )); - - // Initialize the global waypoint version - set_waypoint_version(waypoint.version()); + let _ = waypoint; Arc::new(RwLock::new(db_rw)) } diff --git a/state-sync/state-sync-driver/src/bootstrapper.rs b/state-sync/state-sync-driver/src/bootstrapper.rs index 07555d77adc..35d8d3765ae 100644 --- a/state-sync/state-sync-driver/src/bootstrapper.rs +++ b/state-sync/state-sync-driver/src/bootstrapper.rs @@ -75,6 +75,13 @@ impl VerifiedEpochStates { self.fetched_epoch_ending_ledger_infos } + /// Returns the latest trusted epoch state. Pre-waypoint (pre-migration) + /// ledger infos never update this — see `update_verified_epoch_states`. + #[cfg(test)] + pub fn latest_epoch_state(&self) -> &EpochState { + &self.latest_epoch_state + } + /// Sets `fetched_epoch_ending_ledger_infos` to true pub fn set_fetched_epoch_ending_ledger_infos(&mut self) { self.fetched_epoch_ending_ledger_infos = true; @@ -140,7 +147,18 @@ impl VerifiedEpochStates { return Ok(()); } - // For non-waypoint epochs: verify the ledger info against the latest epoch state + // Ledger infos before the waypoint predate the L1 migration and are + // not signature-verifiable (different historical format). Accept them + // as unverified archive data: retain them for chunk-commit lookups, + // but never install their next_epoch_state as trusted, and never let + // them reach signature verification or the waypoint ratchet below. + if ledger_info.version() < waypoint.version() { + self.highest_fetched_epoch_ending_version = ledger_info.version(); + self.insert_new_epoch_ending_ledger_info(epoch_ending_ledger_info.clone())?; + return Ok(()); + } + + // For post-waypoint epochs: verify the ledger info against the latest epoch state self.latest_epoch_state .verify(epoch_ending_ledger_info) .map_err(|error| { diff --git a/state-sync/state-sync-driver/src/tests/bootstrapper.rs b/state-sync/state-sync-driver/src/tests/bootstrapper.rs index d8c7c60fc12..8893fa4cbdc 100644 --- a/state-sync/state-sync-driver/src/tests/bootstrapper.rs +++ b/state-sync/state-sync-driver/src/tests/bootstrapper.rs @@ -29,9 +29,16 @@ use aptos_data_streaming_service::{ }; use aptos_time_service::TimeService; use aptos_types::{ + aggregate_signature::AggregateSignature, + block_info::BlockInfo, + epoch_state::EpochState, + ledger_info::{LedgerInfo, LedgerInfoWithSignatures}, transaction::{TransactionOutputListWithProof, Version}, + validator_signer::ValidatorSigner, + validator_verifier::{ValidatorConsensusInfo, ValidatorVerifier}, waypoint::Waypoint, }; +use aptos_crypto::hash::HashValue; use claims::{assert_matches, assert_none, assert_ok}; use futures::{channel::{mpsc, oneshot}, FutureExt, SinkExt}; use mockall::{predicate::eq, Sequence}; @@ -1592,6 +1599,119 @@ async fn test_waypoint_satisfiable() { assert_matches!(error, Error::UnsatisfiableWaypoint(_)); } +/// Regression test for audit findings C1/H1 (waypoint verification bypass). +/// A forged pre-waypoint epoch-ending ledger info (empty aggregate +/// signature, attacker-chosen `next_epoch_state`) must still be ACCEPTED as +/// unverified archive data (pre-migration history is not +/// signature-verifiable), but must NOT install its `next_epoch_state` as +/// trusted. +#[test] +fn test_pre_waypoint_ledger_info_accepted_as_unverified_archive_only() { + // Create a bootstrapper for a victim node (trusted epoch state at epoch 0) + let driver_configuration = create_full_node_driver_configuration(); + let (mut bootstrapper, _) = + create_bootstrapper(driver_configuration, MockStreamingClient::new(), None, false); + + // Waypoint anchored at version 1000 + let waypoint_ledger_info = create_random_epoch_ending_ledger_info(1000, 0); + let waypoint = Waypoint::new_any(waypoint_ledger_info.ledger_info()); + + let verified_epoch_states = bootstrapper.get_verified_epoch_states(); + assert_eq!(verified_epoch_states.latest_epoch_state().epoch, 0); + + // The forgery: version 500 (< waypoint), epoch 0, `next_epoch_state` for + // epoch 1, EMPTY aggregate signature + let forged = create_random_epoch_ending_ledger_info(500, 0); + verified_epoch_states + .update_verified_epoch_states(&forged, &waypoint) + .expect("pre-waypoint archive data must still be accepted"); + + // It is retained for chunk-commit lookups... + assert!(verified_epoch_states + .all_epoch_ending_ledger_infos() + .contains(&forged)); + + // ...but its `next_epoch_state` (epoch 1) was NOT installed as trusted + assert_eq!(verified_epoch_states.latest_epoch_state().epoch, 0); +} + +/// Regression test for the H1/F2 attack chain: the attacker forges a +/// pre-waypoint epoch-ending ledger info whose `next_epoch_state` contains +/// the ATTACKER'S validator set (accepted via the bypass), then signs a +/// post-waypoint ledger info with the attacker's OWN keys so it verifies +/// against the installed fake epoch state — poisoning trust and reaching +/// the `verify_waypoint` panic. Post-fix, the pre-waypoint ledger info +/// never installs its `next_epoch_state`, so the post-waypoint forgery +/// fails the epoch continuity check. +#[test] +fn test_forged_pre_waypoint_chain_cannot_escalate_to_post_waypoint_forgery() { + let driver_configuration = create_full_node_driver_configuration(); + let (mut bootstrapper, _) = + create_bootstrapper(driver_configuration, MockStreamingClient::new(), None, false); + + let waypoint_ledger_info = create_random_epoch_ending_ledger_info(1000, 0); + let waypoint = Waypoint::new_any(waypoint_ledger_info.ledger_info()); + + let verified_epoch_states = bootstrapper.get_verified_epoch_states(); + + // Step 1: forged pre-waypoint ledger info carrying the attacker's + // validator set, with an EMPTY aggregate signature + let attacker_signer = ValidatorSigner::random([42; 32]); + let make_attacker_verifier = || { + ValidatorVerifier::new(vec![ValidatorConsensusInfo::new( + attacker_signer.author(), + attacker_signer.public_key(), + 1, + )]) + }; + let forged_pre = LedgerInfoWithSignatures::new( + LedgerInfo::new( + BlockInfo::new( + 0, // epoch matches the victim's trusted epoch + 0, + HashValue::zero(), + HashValue::random(), + 500, // version < waypoint + 0, + Some(EpochState::new(1, make_attacker_verifier())), + ), + HashValue::random(), + ), + AggregateSignature::empty(), + ); + verified_epoch_states + .update_verified_epoch_states(&forged_pre, &waypoint) + .expect("pre-waypoint archive data must still be accepted"); + + // Step 2: post-waypoint ledger info signed by the attacker's OWN key. + // This only verifies if step 1 installed the attacker's validator set. + let post_waypoint_ledger_info = LedgerInfo::new( + BlockInfo::new( + 1, // epoch continues the forged chain + 0, + HashValue::zero(), + HashValue::random(), + 1500, // version > waypoint + 0, + Some(EpochState::new(2, make_attacker_verifier())), + ), + HashValue::random(), + ); + let attacker_sig = attacker_signer.sign(&post_waypoint_ledger_info).unwrap(); + let attacker_aggregate = make_attacker_verifier() + .aggregate_signatures([(attacker_signer.author(), attacker_sig)].iter().map(|(a, s)| (a, s))) + .unwrap(); + let forged_post = + LedgerInfoWithSignatures::new(post_waypoint_ledger_info, attacker_aggregate); + + assert!( + verified_epoch_states + .update_verified_epoch_states(&forged_post, &waypoint) + .is_err(), + "post-waypoint forgery must fail: the trusted epoch state never left epoch 0" + ); +} + /// Creates a bootstrapper for testing fn create_bootstrapper( driver_configuration: DriverConfiguration, diff --git a/state-sync/state-sync-driver/src/tests/driver.rs b/state-sync/state-sync-driver/src/tests/driver.rs index e6996e20abd..633766472d0 100644 --- a/state-sync/state-sync-driver/src/tests/driver.rs +++ b/state-sync/state-sync-driver/src/tests/driver.rs @@ -31,7 +31,6 @@ use aptos_storage_service_notifications::StorageServiceNotificationListener; use aptos_time_service::TimeService; use aptos_types::{ event::EventKey, - ledger_info::set_waypoint_version, transaction::{Transaction, WriteSetPayload}, waypoint::Waypoint, }; @@ -346,9 +345,6 @@ async fn create_driver_for_tests_with_waypoint( let genesis_ledger_info = db_rw.reader.get_latest_ledger_info().unwrap(); let waypoint = Waypoint::new_any(genesis_ledger_info.ledger_info()); - // Set the global waypoint version for event notifications - set_waypoint_version(waypoint.version()); - create_driver_for_tests(node_config, waypoint, event_key_subscriptions).await } diff --git a/state-sync/state-sync-driver/src/tests/driver_factory.rs b/state-sync/state-sync-driver/src/tests/driver_factory.rs index a80ae04354f..cffec38e8b6 100644 --- a/state-sync/state-sync-driver/src/tests/driver_factory.rs +++ b/state-sync/state-sync-driver/src/tests/driver_factory.rs @@ -24,7 +24,6 @@ use aptos_storage_interface::DbReaderWriter; use aptos_storage_service_client::StorageServiceClient; use aptos_temppath::TempPath; use aptos_time_service::TimeService; -use aptos_types::ledger_info::set_waypoint_version; use aptos_vm::aptos_vm::AptosVMBlockExecutor; use futures::{FutureExt, StreamExt}; use std::{collections::HashMap, sync::Arc}; @@ -55,9 +54,6 @@ fn test_new_initialized_configs() { let genesis_ledger_info = db_rw.reader.get_latest_ledger_info().unwrap(); let waypoint = aptos_types::waypoint::Waypoint::new_any(genesis_ledger_info.ledger_info()); - // Set the global waypoint version for event notifications - set_waypoint_version(waypoint.version()); - // Create mempool and consensus notifiers let (mempool_notifier, _) = new_mempool_notifier_listener_pair(100); let (_, consensus_listener) = new_consensus_notifier_listener_pair(0); diff --git a/types/src/ledger_info.rs b/types/src/ledger_info.rs index facbe165fd6..1e45dc5251d 100644 --- a/types/src/ledger_info.rs +++ b/types/src/ledger_info.rs @@ -28,23 +28,10 @@ use std::{ ops::{Deref, DerefMut}, sync::{ atomic::{AtomicBool, Ordering}, - Arc, OnceLock, + Arc, }, }; -/// Global waypoint version storage for bypassing verification of historical data -static WAYPOINT_VERSION: OnceLock = OnceLock::new(); - -/// Initialize the waypoint version (should be called once during node startup) -pub fn set_waypoint_version(version: Version) { - let _ = WAYPOINT_VERSION.set(version); -} - -/// Get the waypoint version if it has been set -pub fn get_waypoint_version() -> Option { - WAYPOINT_VERSION.get().copied() -} - /// This structure serves a dual purpose. /// /// First, if this structure is signed by 2f+1 validators it signifies the state of the ledger at @@ -318,13 +305,11 @@ impl LedgerInfoWithV0 { &self, validator: &ValidatorVerifier, ) -> ::std::result::Result<(), VerifyError> { - // Check if this LedgerInfo is before the waypoint version - if let Some(waypoint_version) = get_waypoint_version() { - if self.ledger_info().version() <= waypoint_version { - return Ok(()); - } - } - + // Signatures are always verified. Pre-waypoint (pre-migration) ledger + // infos are not signature-verifiable and are handled explicitly by the + // state-sync bootstrapper's historical path instead of being + // short-circuited here — this shared primitive is on the live + // consensus path (quorum certs, commit decisions, epoch changes). validator.verify_multi_signatures(self.ledger_info(), &self.signatures) }