From 1626ec383a46952959708b5f7a5b2b87e1886597 Mon Sep 17 00:00:00 2001 From: teor Date: Thu, 8 Apr 2021 03:13:52 +1000 Subject: [PATCH] Add InventoryHash and MetaAddr proptests (#1985) * Make proptest dependencies consistent between chain and network * Implement Arbitrary for InventoryHash and use it in tests * Impl Arbitrary for MetaAddr and use it in tests Also test some extreme times in MetaAddr sanitization. --- zebra-network/Cargo.toml | 2 +- zebra-network/src/meta_addr.rs | 8 ++ zebra-network/src/meta_addr/arbitrary.rs | 31 ++++++ zebra-network/src/meta_addr/tests.rs | 2 + zebra-network/src/meta_addr/tests/check.rs | 45 ++++++++ .../src/meta_addr/tests/preallocate.rs | 90 +++++++-------- zebra-network/src/meta_addr/tests/prop.rs | 35 ++---- zebra-network/src/meta_addr/tests/vectors.rs | 28 +++++ zebra-network/src/protocol/external.rs | 2 + .../src/protocol/external/arbitrary.rs | 54 +++++++++ .../protocol/external/tests/preallocate.rs | 103 +++++++----------- zebra-network/src/protocol/external/types.rs | 1 + 12 files changed, 263 insertions(+), 138 deletions(-) create mode 100644 zebra-network/src/meta_addr/arbitrary.rs create mode 100644 zebra-network/src/meta_addr/tests/check.rs create mode 100644 zebra-network/src/meta_addr/tests/vectors.rs create mode 100644 zebra-network/src/protocol/external/arbitrary.rs diff --git a/zebra-network/Cargo.toml b/zebra-network/Cargo.toml index 33193bade..27adc79b3 100644 --- a/zebra-network/Cargo.toml +++ b/zebra-network/Cargo.toml @@ -37,6 +37,6 @@ zebra-chain = { path = "../zebra-chain" } [dev-dependencies] proptest = "0.10" -proptest-derive = "0.3.0" +proptest-derive = "0.3" zebra-test = { path = "../zebra-test/" } diff --git a/zebra-network/src/meta_addr.rs b/zebra-network/src/meta_addr.rs index e16d5033d..04f2521ff 100644 --- a/zebra-network/src/meta_addr.rs +++ b/zebra-network/src/meta_addr.rs @@ -18,6 +18,11 @@ use crate::protocol::{external::MAX_PROTOCOL_MESSAGE_LEN, types::PeerServices}; use PeerAddrState::*; +#[cfg(any(test, feature = "proptest-impl"))] +use proptest_derive::Arbitrary; +#[cfg(any(test, feature = "proptest-impl"))] +mod arbitrary; + #[cfg(test)] mod tests; @@ -28,6 +33,7 @@ mod tests; /// [`AddressBook::maybe_connected_peers`] and /// [`AddressBook::reconnection_peers`]. #[derive(Copy, Clone, Debug, Eq, PartialEq)] +#[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub enum PeerAddrState { /// The peer has sent us a valid message. /// @@ -196,6 +202,8 @@ impl MetaAddr { let last_seen = Utc.timestamp(ts - ts.rem_euclid(interval), 0); MetaAddr { addr: self.addr, + // services are sanitized during parsing, so we don't need to make + // any changes here services: self.services, last_seen, // the state isn't sent to the remote peer, but sanitize it anyway diff --git a/zebra-network/src/meta_addr/arbitrary.rs b/zebra-network/src/meta_addr/arbitrary.rs new file mode 100644 index 000000000..6352d83ed --- /dev/null +++ b/zebra-network/src/meta_addr/arbitrary.rs @@ -0,0 +1,31 @@ +use proptest::{arbitrary::any, arbitrary::Arbitrary, prelude::*}; + +use super::{MetaAddr, PeerAddrState, PeerServices}; + +use std::{net::SocketAddr, time::SystemTime}; + +impl Arbitrary for MetaAddr { + type Parameters = (); + + fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { + ( + any::(), + any::(), + any::(), + any::(), + ) + .prop_map( + |(addr, services, last_seen, last_connection_state)| MetaAddr { + addr, + services, + // TODO: implement constraints on last_seen as part of the + // last_connection_state refactor in #1849 + last_seen: last_seen.into(), + last_connection_state, + }, + ) + .boxed() + } + + type Strategy = BoxedStrategy; +} diff --git a/zebra-network/src/meta_addr/tests.rs b/zebra-network/src/meta_addr/tests.rs index a7ac2ac35..76e880cbe 100644 --- a/zebra-network/src/meta_addr/tests.rs +++ b/zebra-network/src/meta_addr/tests.rs @@ -1,2 +1,4 @@ +mod check; mod preallocate; mod prop; +mod vectors; diff --git a/zebra-network/src/meta_addr/tests/check.rs b/zebra-network/src/meta_addr/tests/check.rs new file mode 100644 index 000000000..061854d5f --- /dev/null +++ b/zebra-network/src/meta_addr/tests/check.rs @@ -0,0 +1,45 @@ +//! Shared test checks for MetaAddr + +use super::super::MetaAddr; + +use crate::constants::TIMESTAMP_TRUNCATION_SECONDS; + +/// Make sure that the sanitize function reduces time and state metadata +/// leaks. +pub(crate) fn sanitize_avoids_leaks(entry: &MetaAddr) { + let sanitized = entry.sanitize(); + + // We want the sanitized timestamp to: + // - be a multiple of the truncation interval, + // - have a zero nanoseconds component, and + // - be within the truncation interval of the original timestamp. + assert_eq!( + sanitized.get_last_seen().timestamp() % TIMESTAMP_TRUNCATION_SECONDS, + 0 + ); + assert_eq!(sanitized.get_last_seen().timestamp_subsec_nanos(), 0); + // handle underflow and overflow by skipping the check + // the other check will ensure correctness + let lowest_time = entry + .get_last_seen() + .timestamp() + .checked_sub(TIMESTAMP_TRUNCATION_SECONDS); + let highest_time = entry + .get_last_seen() + .timestamp() + .checked_add(TIMESTAMP_TRUNCATION_SECONDS); + if let Some(lowest_time) = lowest_time { + assert!(sanitized.get_last_seen().timestamp() > lowest_time); + } + if let Some(highest_time) = highest_time { + assert!(sanitized.get_last_seen().timestamp() < highest_time); + } + + // Sanitize to the the default state, even though it's not serialized + assert_eq!(sanitized.last_connection_state, Default::default()); + // We want the other fields to be unmodified + assert_eq!(sanitized.addr, entry.addr); + // Services are sanitized during parsing, so we don't need to make + // any changes in sanitize() + assert_eq!(sanitized.services, entry.services); +} diff --git a/zebra-network/src/meta_addr/tests/preallocate.rs b/zebra-network/src/meta_addr/tests/preallocate.rs index 00543851d..c9fb9dbb4 100644 --- a/zebra-network/src/meta_addr/tests/preallocate.rs +++ b/zebra-network/src/meta_addr/tests/preallocate.rs @@ -1,61 +1,51 @@ //! Tests for trusted preallocation during deserialization. -use super::super::{MetaAddr, PeerAddrState, PeerServices, META_ADDR_SIZE}; +use super::super::{MetaAddr, META_ADDR_SIZE}; use zebra_chain::serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN}; -use chrono::{TimeZone, Utc}; +use proptest::prelude::*; use std::convert::TryInto; -/// Confirm that each MetaAddr takes exactly META_ADDR_SIZE bytes when serialized. -/// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound. -#[test] -fn meta_addr_size_is_correct() { - let addr = MetaAddr { - addr: ([192, 168, 0, 0], 8333).into(), - services: PeerServices::default(), - last_seen: Utc.timestamp(1_573_680_222, 0), - last_connection_state: PeerAddrState::Responded, - }; - let serialized = addr - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - assert!(serialized.len() == META_ADDR_SIZE) -} - -/// Verifies that... -/// 1. The smallest disallowed vector of `MetaAddrs`s is too large to fit in a legal Zcash message -/// 2. The largest allowed vector is small enough to fit in a legal Zcash message -#[test] -fn meta_addr_max_allocation_is_correct() { - let addr = MetaAddr { - addr: ([192, 168, 0, 0], 8333).into(), - services: PeerServices::default(), - last_seen: Utc.timestamp(1_573_680_222, 0), - last_connection_state: PeerAddrState::Responded, - }; - let max_allocation: usize = MetaAddr::max_allocation().try_into().unwrap(); - let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1); - for _ in 0..(MetaAddr::max_allocation() + 1) { - smallest_disallowed_vec.push(addr); +proptest! { + /// Confirm that each MetaAddr takes exactly META_ADDR_SIZE bytes when serialized. + /// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound. + #[test] + fn meta_addr_size_is_correct(addr in MetaAddr::arbitrary()) { + let serialized = addr + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + assert!(serialized.len() == META_ADDR_SIZE) } - let smallest_disallowed_serialized = smallest_disallowed_vec - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - // Check that our smallest_disallowed_vec is only one item larger than the limit - assert!(((smallest_disallowed_vec.len() - 1) as u64) == MetaAddr::max_allocation()); - // Check that our smallest_disallowed_vec is too big to send in a valid Zcash message - assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN); - // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency) - smallest_disallowed_vec.pop(); - let largest_allowed_vec = smallest_disallowed_vec; - let largest_allowed_serialized = largest_allowed_vec - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); + /// Verifies that... + /// 1. The smallest disallowed vector of `MetaAddrs`s is too large to fit in a legal Zcash message + /// 2. The largest allowed vector is small enough to fit in a legal Zcash message + #[test] + fn meta_addr_max_allocation_is_correct(addr in MetaAddr::arbitrary()) { + let max_allocation: usize = MetaAddr::max_allocation().try_into().unwrap(); + let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1); + for _ in 0..(MetaAddr::max_allocation() + 1) { + smallest_disallowed_vec.push(addr); + } + let smallest_disallowed_serialized = smallest_disallowed_vec + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + // Check that our smallest_disallowed_vec is only one item larger than the limit + assert!(((smallest_disallowed_vec.len() - 1) as u64) == MetaAddr::max_allocation()); + // Check that our smallest_disallowed_vec is too big to send in a valid Zcash message + assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN); - // Check that our largest_allowed_vec contains the maximum number of MetaAddrs - assert!((largest_allowed_vec.len() as u64) == MetaAddr::max_allocation()); - // Check that our largest_allowed_vec is small enough to fit in a Zcash message. - assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN); + // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency) + smallest_disallowed_vec.pop(); + let largest_allowed_vec = smallest_disallowed_vec; + let largest_allowed_serialized = largest_allowed_vec + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + + // Check that our largest_allowed_vec contains the maximum number of MetaAddrs + assert!((largest_allowed_vec.len() as u64) == MetaAddr::max_allocation()); + // Check that our largest_allowed_vec is small enough to fit in a Zcash message. + assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN); + } } diff --git a/zebra-network/src/meta_addr/tests/prop.rs b/zebra-network/src/meta_addr/tests/prop.rs index 0e928fd86..8533d7285 100644 --- a/zebra-network/src/meta_addr/tests/prop.rs +++ b/zebra-network/src/meta_addr/tests/prop.rs @@ -1,29 +1,16 @@ -use super::super::*; +//! Randomised property tests for MetaAddr. -// XXX remove this test and replace it with a proptest instance. -#[test] -fn sanitize_truncates_timestamps() { - zebra_test::init(); +use super::{super::MetaAddr, check}; - let services = PeerServices::default(); - let addr = "127.0.0.1:8233".parse().unwrap(); +use proptest::prelude::*; - let entry = MetaAddr { - services, - addr, - last_seen: Utc.timestamp(1_573_680_222, 0), - last_connection_state: Responded, +proptest! { + /// Make sure that the sanitize function reduces time and state metadata + /// leaks. + #[test] + fn sanitized_fields(entry in MetaAddr::arbitrary()) { + zebra_test::init(); + + check::sanitize_avoids_leaks(&entry); } - .sanitize(); - - // We want the sanitized timestamp to be a multiple of the truncation interval. - assert_eq!( - entry.get_last_seen().timestamp() % crate::constants::TIMESTAMP_TRUNCATION_SECONDS, - 0 - ); - // We want the state to be the default - assert_eq!(entry.last_connection_state, Default::default()); - // We want the other fields to be unmodified - assert_eq!(entry.addr, addr); - assert_eq!(entry.services, services); } diff --git a/zebra-network/src/meta_addr/tests/vectors.rs b/zebra-network/src/meta_addr/tests/vectors.rs new file mode 100644 index 000000000..538ada7b8 --- /dev/null +++ b/zebra-network/src/meta_addr/tests/vectors.rs @@ -0,0 +1,28 @@ +//! Test vectors for MetaAddr. + +use super::{super::MetaAddr, check}; + +use chrono::{MAX_DATETIME, MIN_DATETIME}; + +/// Make sure that the sanitize function handles minimum and maximum times. +#[test] +fn sanitize_extremes() { + zebra_test::init(); + + let min_time_entry = MetaAddr { + addr: "127.0.0.1:8233".parse().unwrap(), + services: Default::default(), + last_seen: MIN_DATETIME, + last_connection_state: Default::default(), + }; + + let max_time_entry = MetaAddr { + addr: "127.0.0.1:8233".parse().unwrap(), + services: Default::default(), + last_seen: MAX_DATETIME, + last_connection_state: Default::default(), + }; + + check::sanitize_avoids_leaks(&min_time_entry); + check::sanitize_avoids_leaks(&max_time_entry); +} diff --git a/zebra-network/src/protocol/external.rs b/zebra-network/src/protocol/external.rs index 7a158b483..9a1e2472b 100644 --- a/zebra-network/src/protocol/external.rs +++ b/zebra-network/src/protocol/external.rs @@ -7,6 +7,8 @@ mod message; /// Newtype wrappers for primitive types. pub mod types; +#[cfg(any(test, feature = "proptest-impl"))] +mod arbitrary; #[cfg(test)] mod tests; diff --git a/zebra-network/src/protocol/external/arbitrary.rs b/zebra-network/src/protocol/external/arbitrary.rs new file mode 100644 index 000000000..3173350e1 --- /dev/null +++ b/zebra-network/src/protocol/external/arbitrary.rs @@ -0,0 +1,54 @@ +use proptest::{arbitrary::any, arbitrary::Arbitrary, prelude::*}; + +use super::InventoryHash; + +use zebra_chain::{block, transaction}; + +impl InventoryHash { + /// Generate a proptest strategy for Inv Errors + pub fn error_strategy() -> BoxedStrategy { + Just(InventoryHash::Error).boxed() + } + + /// Generate a proptest strategy for Inv Tx hashes + pub fn tx_strategy() -> BoxedStrategy { + // using any:: causes a trait impl error + // when building the zebra-network crate separately + (any::<[u8; 32]>()) + .prop_map(transaction::Hash) + .prop_map(InventoryHash::Tx) + .boxed() + } + + /// Generate a proptest strategy for Inv Block hashes + pub fn block_strategy() -> BoxedStrategy { + (any::<[u8; 32]>()) + .prop_map(block::Hash) + .prop_map(InventoryHash::Block) + .boxed() + } + + /// Generate a proptest strategy for Inv FilteredBlock hashes + pub fn filtered_block_strategy() -> BoxedStrategy { + (any::<[u8; 32]>()) + .prop_map(block::Hash) + .prop_map(InventoryHash::FilteredBlock) + .boxed() + } +} + +impl Arbitrary for InventoryHash { + type Parameters = (); + + fn arbitrary_with(_args: Self::Parameters) -> Self::Strategy { + prop_oneof![ + Self::error_strategy(), + Self::tx_strategy(), + Self::block_strategy(), + Self::filtered_block_strategy(), + ] + .boxed() + } + + type Strategy = BoxedStrategy; +} diff --git a/zebra-network/src/protocol/external/tests/preallocate.rs b/zebra-network/src/protocol/external/tests/preallocate.rs index 87d0e92b9..e7647d77f 100644 --- a/zebra-network/src/protocol/external/tests/preallocate.rs +++ b/zebra-network/src/protocol/external/tests/preallocate.rs @@ -2,73 +2,50 @@ use super::super::inv::{InventoryHash, INV_HASH_SIZE}; -use zebra_chain::{ - block, - serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN}, - transaction, -}; +use zebra_chain::serialization::{TrustedPreallocate, ZcashSerialize, MAX_PROTOCOL_MESSAGE_LEN}; +use proptest::prelude::*; use std::convert::TryInto; -/// Confirm that each InventoryHash takes exactly INV_HASH_SIZE bytes when serialized. -/// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound. -#[test] -fn inv_hash_size_is_correct() { - let block_hash = block::Hash([1u8; 32]); - let tx_hash = transaction::Hash([1u8; 32]); - let inv_block = InventoryHash::Block(block_hash); - let serialized_inv_block = inv_block - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - assert!(serialized_inv_block.len() == INV_HASH_SIZE); - - let inv_filtered_block = InventoryHash::FilteredBlock(block_hash); - let serialized_inv_filtered = inv_filtered_block - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - assert!(serialized_inv_filtered.len() == INV_HASH_SIZE); - - let inv_tx = InventoryHash::Tx(tx_hash); - let serialized_inv_tx = inv_tx - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - assert!(serialized_inv_tx.len() == INV_HASH_SIZE); - - let inv_err = InventoryHash::Error; - let serializd_inv_err = inv_err - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - assert!(serializd_inv_err.len() == INV_HASH_SIZE) -} - -/// Verifies that... -/// 1. The smallest disallowed vector of `InventoryHash`s is too large to fit in a legal Zcash message -/// 2. The largest allowed vector is small enough to fit in a legal Zcash message -#[test] -fn inv_hash_max_allocation_is_correct() { - let inv = InventoryHash::Error; - let max_allocation: usize = InventoryHash::max_allocation().try_into().unwrap(); - let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1); - for _ in 0..(InventoryHash::max_allocation() + 1) { - smallest_disallowed_vec.push(inv); +proptest! { + /// Confirm that each InventoryHash takes exactly INV_HASH_SIZE bytes when serialized. + /// This verifies that our calculated `TrustedPreallocate::max_allocation()` is indeed an upper bound. + #[test] + fn inv_hash_size_is_correct(inv in InventoryHash::arbitrary()) { + let serialized_inv = inv + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + assert!(serialized_inv.len() == INV_HASH_SIZE); } - let smallest_disallowed_serialized = smallest_disallowed_vec - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); - // Check that our smallest_disallowed_vec is only one item larger than the limit - assert!(((smallest_disallowed_vec.len() - 1) as u64) == InventoryHash::max_allocation()); - // Check that our smallest_disallowed_vec is too big to fit in a Zcash message. - assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN); - // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency) - smallest_disallowed_vec.pop(); - let largest_allowed_vec = smallest_disallowed_vec; - let largest_allowed_serialized = largest_allowed_vec - .zcash_serialize_to_vec() - .expect("Serialization to vec must succeed"); + /// Verifies that... + /// 1. The smallest disallowed vector of `InventoryHash`s is too large to fit in a legal Zcash message + /// 2. The largest allowed vector is small enough to fit in a legal Zcash message + #[test] + fn inv_hash_max_allocation_is_correct(inv in InventoryHash::arbitrary()) { + let max_allocation: usize = InventoryHash::max_allocation().try_into().unwrap(); + let mut smallest_disallowed_vec = Vec::with_capacity(max_allocation + 1); + for _ in 0..(InventoryHash::max_allocation() + 1) { + smallest_disallowed_vec.push(inv); + } + let smallest_disallowed_serialized = smallest_disallowed_vec + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + // Check that our smallest_disallowed_vec is only one item larger than the limit + assert!(((smallest_disallowed_vec.len() - 1) as u64) == InventoryHash::max_allocation()); + // Check that our smallest_disallowed_vec is too big to fit in a Zcash message. + assert!(smallest_disallowed_serialized.len() > MAX_PROTOCOL_MESSAGE_LEN); - // Check that our largest_allowed_vec contains the maximum number of InventoryHashes - assert!((largest_allowed_vec.len() as u64) == InventoryHash::max_allocation()); - // Check that our largest_allowed_vec is small enough to fit in a Zcash message. - assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN); + // Create largest_allowed_vec by removing one element from smallest_disallowed_vec without copying (for efficiency) + smallest_disallowed_vec.pop(); + let largest_allowed_vec = smallest_disallowed_vec; + let largest_allowed_serialized = largest_allowed_vec + .zcash_serialize_to_vec() + .expect("Serialization to vec must succeed"); + + // Check that our largest_allowed_vec contains the maximum number of InventoryHashes + assert!((largest_allowed_vec.len() as u64) == InventoryHash::max_allocation()); + // Check that our largest_allowed_vec is small enough to fit in a Zcash message. + assert!(largest_allowed_serialized.len() <= MAX_PROTOCOL_MESSAGE_LEN); + } } diff --git a/zebra-network/src/protocol/external/types.rs b/zebra-network/src/protocol/external/types.rs index 3f61da1c3..d1b6b3202 100644 --- a/zebra-network/src/protocol/external/types.rs +++ b/zebra-network/src/protocol/external/types.rs @@ -78,6 +78,7 @@ bitflags! { /// Note that bits 24-31 are reserved for temporary experiments; other /// service bits should be allocated via the ZIP process. #[derive(Default)] + #[cfg_attr(any(test, feature = "proptest-impl"), derive(Arbitrary))] pub struct PeerServices: u64 { /// NODE_NETWORK means that the node is a full node capable of serving /// blocks, as opposed to a light client that makes network requests but