-
Notifications
You must be signed in to change notification settings - Fork 4
fix: payment verification hardening -- cost units, close group, DoS prevention #58
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -10,6 +10,7 @@ use crate::payment::proof::{ | |
| deserialize_merkle_proof, deserialize_proof, detect_proof_type, ProofType, | ||
| }; | ||
| use crate::payment::quote::{verify_quote_content, verify_quote_signature}; | ||
| use crate::payment::verify_merkle_candidate_signature; | ||
| use evmlib::contract::merkle_payment_vault; | ||
| use evmlib::merkle_batch_payment::PoolHash; | ||
| use evmlib::merkle_payments::OnChainPaymentInfo; | ||
|
|
@@ -20,6 +21,7 @@ use lru::LruCache; | |
| use parking_lot::Mutex; | ||
| use saorsa_core::identity::node_identity::peer_id_from_public_key_bytes; | ||
| use std::num::NonZeroUsize; | ||
| use std::sync::Arc; | ||
| use std::time::SystemTime; | ||
| use tracing::{debug, info}; | ||
|
|
||
|
|
@@ -65,9 +67,11 @@ impl Default for EvmVerifierConfig { | |
|
|
||
| /// Configuration for the payment verifier. | ||
| /// | ||
| /// Callback to check if the local node is in the close group for a given address. | ||
| pub type CloseGroupChecker = Arc<dyn Fn(&[u8; 32]) -> Vec<RewardsAddress> + Send + Sync>; | ||
|
|
||
| /// All new data requires EVM payment on Arbitrum. The cache stores | ||
| /// previously verified payments to avoid redundant on-chain lookups. | ||
| #[derive(Debug, Clone)] | ||
| pub struct PaymentVerifierConfig { | ||
| /// EVM verifier configuration. | ||
| pub evm: EvmVerifierConfig, | ||
|
|
@@ -76,6 +80,26 @@ pub struct PaymentVerifierConfig { | |
| /// Local node's rewards address. | ||
| /// The verifier rejects payments that don't include this node as a recipient. | ||
| pub local_rewards_address: RewardsAddress, | ||
| /// Optional close group checker for merkle payments. | ||
| /// | ||
| /// Given a data address, returns the rewards addresses of nodes in the local | ||
| /// close group view for that address. Used to verify the storing node is | ||
| /// actually responsible for the data. If `None`, the check is skipped. | ||
| pub close_group_checker: Option<CloseGroupChecker>, | ||
| } | ||
|
|
||
| impl std::fmt::Debug for PaymentVerifierConfig { | ||
| fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { | ||
| f.debug_struct("PaymentVerifierConfig") | ||
| .field("evm", &self.evm) | ||
| .field("cache_capacity", &self.cache_capacity) | ||
| .field("local_rewards_address", &self.local_rewards_address) | ||
| .field( | ||
| "close_group_checker", | ||
| &self.close_group_checker.as_ref().map(|_| "<fn>"), | ||
| ) | ||
| .finish() | ||
| } | ||
| } | ||
|
|
||
| /// Status returned by payment verification. | ||
|
|
@@ -522,6 +546,17 @@ impl PaymentVerifier { | |
|
|
||
| let pool_hash = merkle_proof.winner_pool_hash(); | ||
|
|
||
| // Run cheap local checks BEFORE expensive on-chain queries. | ||
| // This prevents DoS via garbage proofs that trigger RPC lookups. | ||
| for candidate in &merkle_proof.winner_pool.candidate_nodes { | ||
| if !verify_merkle_candidate_signature(candidate) { | ||
| return Err(Error::Payment(format!( | ||
| "Invalid ML-DSA-65 signature on merkle candidate node (reward: {})", | ||
| candidate.reward_address | ||
| ))); | ||
| } | ||
| } | ||
|
|
||
| // Check pool cache first | ||
| let cached_info = { | ||
| let mut pool_cache = self.pool_cache.lock(); | ||
|
|
@@ -555,6 +590,31 @@ impl PaymentVerifier { | |
| paid_node_addresses, | ||
| }; | ||
|
|
||
| // Verify cost units: ensure on-chain packed commitments match what | ||
| // the signed node metrics produce. This prevents clients from submitting | ||
| // inflated/deflated cost units to manipulate pricing. | ||
| let packed_commitments = merkle_payment_vault::get_merkle_payment_packed_commitments( | ||
| &self.config.evm.network, | ||
| pool_hash, | ||
| on_chain_info.merkle_payment_timestamp, | ||
| ) | ||
| .await | ||
| .map_err(|e| { | ||
| Error::Payment(format!( | ||
| "Failed to get packed commitments for cost unit verification: {e}" | ||
| )) | ||
| })?; | ||
|
|
||
| merkle_proof | ||
| .winner_pool | ||
| .verify_cost_units(&packed_commitments, &pool_hash) | ||
| .map_err(|e| { | ||
| Error::Payment(format!( | ||
| "Cost unit verification failed for pool {}: {e}", | ||
| hex::encode(pool_hash) | ||
| )) | ||
| })?; | ||
|
|
||
| // Cache the pool info for subsequent chunks in the same batch | ||
| { | ||
| let mut pool_cache = self.pool_cache.lock(); | ||
|
|
@@ -572,20 +632,8 @@ impl PaymentVerifier { | |
| on_chain_info | ||
| }; | ||
|
|
||
| // pool_hash was derived from merkle_proof.winner_pool and used to query | ||
| // the contract. The contract only returns data if a payment exists for that | ||
| // hash. The ML-DSA signature check below ensures the pool contents are | ||
| // authentic (nodes actually signed their candidate quotes). | ||
|
|
||
| // Verify ML-DSA-65 signatures and timestamp/data_type consistency | ||
| // on all candidate nodes in the winner pool. | ||
| // Verify timestamp consistency (signatures already checked above before RPC). | ||
| for candidate in &merkle_proof.winner_pool.candidate_nodes { | ||
| if !crate::payment::verify_merkle_candidate_signature(candidate) { | ||
| return Err(Error::Payment(format!( | ||
| "Invalid ML-DSA-65 signature on merkle candidate node (reward: {})", | ||
| candidate.reward_address | ||
| ))); | ||
| } | ||
| if candidate.merkle_payment_timestamp != payment_info.merkle_payment_timestamp { | ||
| return Err(Error::Payment(format!( | ||
| "Candidate timestamp mismatch: expected {}, got {} (reward: {})", | ||
|
|
@@ -653,6 +701,18 @@ impl PaymentVerifier { | |
| } | ||
| } | ||
|
|
||
| // Verify this node is in the close group for the data address. | ||
| // This prevents nodes from accepting data they're not responsible for. | ||
| if let Some(ref checker) = self.config.close_group_checker { | ||
| let close_group_addrs = checker(xorname); | ||
| if !close_group_addrs.contains(&self.config.local_rewards_address) { | ||
| return Err(Error::Payment(format!( | ||
| "This node is not in the close group for address {}", | ||
| hex::encode(xorname) | ||
| ))); | ||
| } | ||
| } | ||
|
Comment on lines
+704
to
+714
|
||
|
|
||
| if tracing::enabled!(tracing::Level::INFO) { | ||
| info!( | ||
| "Merkle payment verified for {} (pool: {})", | ||
|
|
@@ -692,6 +752,7 @@ mod tests { | |
| evm: EvmVerifierConfig::default(), | ||
| cache_capacity: 100, | ||
| local_rewards_address: RewardsAddress::new([1u8; 20]), | ||
| close_group_checker: None, | ||
| }; | ||
| PaymentVerifier::new(config) | ||
| } | ||
|
|
@@ -1255,6 +1316,7 @@ mod tests { | |
| }, | ||
| cache_capacity: 100, | ||
| local_rewards_address: local_addr, | ||
| close_group_checker: None, | ||
| }; | ||
| let verifier = PaymentVerifier::new(config); | ||
|
|
||
|
|
@@ -1643,6 +1705,7 @@ mod tests { | |
| evm: EvmVerifierConfig::default(), | ||
| cache_capacity: 100, | ||
| local_rewards_address: RewardsAddress::new([1u8; 20]), | ||
| close_group_checker: None, | ||
| }; | ||
| let verifier = PaymentVerifier::new(config); | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
get_merkle_payment_packed_commitments()is an additional on-chain RPC call that currently happens immediately after fetchingget_merkle_payment_info(), before running other local validations that might fail (e.g.,verify_merkle_proof, paid-node index checks). To minimize RPC amplification from invalid proofs, consider deferring the packed-commitments RPC until after the local proof validations succeed, and only then runverify_cost_units()+ cache the pool.