Verify headers and blocks only when needed (mimblewimble#3023)

* Verify headers and blocks only when needed

Curretnly we have some lightweigt validation implemented as part of
entity deserialization, which is safer and allows us to not parse the
entire object if some part is invalid. At the same time this logic
always applies when we read an entity, eg when reading from DB.

This PR introduces UntrustedHeader/Block which is used when we read from
the network. It does partial validation during read, then it is supposed
to be converted into regular header/block which doesn't validate itself.

Also this PR adds "lightweight" validation to block header read like we have
for block body, so we don't parse block body if the header is invalid.

Fixes mimblewimble#1642

* Move version validation to untrusted header

* update fuzz tests

chain/src/pipe.rs

@@ -25,8 +25,6 @@ use crate::store;
 use crate::txhashset;
 use crate::types::{Options, Tip};
 use crate::util::RwLock;
-use chrono::prelude::Utc;
-use chrono::Duration;
 use grin_store;
 use std::sync::Arc;
 
@@ -314,35 +312,6 @@ fn prev_header_store(
 /// to make it as cheap as possible. The different validations are also
 /// arranged by order of cost to have as little DoS surface as possible.
 fn validate_header(header: &BlockHeader, ctx: &mut BlockContext<'_>) -> Result<(), Error> {
-	// check version, enforces scheduled hard fork
-	if !consensus::valid_header_version(header.height, header.version) {
-		error!(
-			"Invalid block header version received ({:?}), maybe update Grin?",
-			header.version
-		);
-		return Err(ErrorKind::InvalidBlockVersion(header.version).into());
-	}
-
-	if header.timestamp > Utc::now() + Duration::seconds(12 * (consensus::BLOCK_TIME_SEC as i64)) {
-		// refuse blocks more than 12 blocks intervals in future (as in bitcoin)
-		// TODO add warning in p2p code if local time is too different from peers
-		return Err(ErrorKind::InvalidBlockTime.into());
-	}
-
-	if !ctx.opts.contains(Options::SKIP_POW) {
-		if !header.pow.is_primary() && !header.pow.is_secondary() {
-			return Err(ErrorKind::LowEdgebits.into());
-		}
-		let edge_bits = header.pow.edge_bits();
-		if !(ctx.pow_verifier)(header).is_ok() {
-			error!(
-				"pipe: error validating header with cuckoo edge_bits {}",
-				edge_bits
-			);
-			return Err(ErrorKind::InvalidPow.into());
-		}
-	}
-
 	// First I/O cost, delayed as late as possible.
 	let prev = prev_header_store(header, &mut ctx.batch)?;
 

core/fuzz/fuzz_targets/block_read_v1.rs

@@ -3,10 +3,10 @@ extern crate grin_core;
 #[macro_use]
 extern crate libfuzzer_sys;
 
-use grin_core::core::Block;
+use grin_core::core::UntrustedBlock;
 use grin_core::ser;
 
 fuzz_target!(|data: &[u8]| {
 	let mut d = data.clone();
-	let _t: Result<Block, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(1));
+	let _t: Result<UntrustedBlock, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(1));
 });

core/fuzz/fuzz_targets/block_read_v2.rs

@@ -3,10 +3,10 @@ extern crate grin_core;
 #[macro_use]
 extern crate libfuzzer_sys;
 
-use grin_core::core::Block;
+use grin_core::core::UntrustedBlock;
 use grin_core::ser;
 
 fuzz_target!(|data: &[u8]| {
 	let mut d = data.clone();
-	let _t: Result<Block, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(2));
+	let _t: Result<UntrustedBlock, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(2));
 });

core/fuzz/fuzz_targets/compact_block_read_v1.rs

@@ -3,10 +3,11 @@ extern crate grin_core;
 #[macro_use]
 extern crate libfuzzer_sys;
 
-use grin_core::core::CompactBlock;
+use grin_core::core::UntrustedCompactBlock;
 use grin_core::ser;
 
 fuzz_target!(|data: &[u8]| {
 	let mut d = data.clone();
-	let _t: Result<CompactBlock, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(1));
+	let _t: Result<UntrustedCompactBlock, ser::Error> =
+		ser::deserialize(&mut d, ser::ProtocolVersion(1));
 });

core/fuzz/fuzz_targets/compact_block_read_v2.rs

@@ -3,10 +3,11 @@ extern crate grin_core;
 #[macro_use]
 extern crate libfuzzer_sys;
 
-use grin_core::core::CompactBlock;
+use grin_core::core::UntrustedCompactBlock;
 use grin_core::ser;
 
 fuzz_target!(|data: &[u8]| {
 	let mut d = data.clone();
-	let _t: Result<CompactBlock, ser::Error> = ser::deserialize(&mut d, ser::ProtocolVersion(2));
+	let _t: Result<UntrustedCompactBlock, ser::Error> =
+		ser::deserialize(&mut d, ser::ProtocolVersion(2));
 });

core/src/core/block.rs

@@ -17,6 +17,7 @@
 use crate::util::RwLock;
 use chrono::naive::{MAX_DATE, MIN_DATE};
 use chrono::prelude::{DateTime, NaiveDateTime, Utc};
+use chrono::Duration;
 use std::collections::HashSet;
 use std::fmt;
 use std::iter::FromIterator;
@@ -34,7 +35,7 @@ use crate::core::{
 
 use crate::global;
 use crate::keychain::{self, BlindingFactor};
-use crate::pow::{Difficulty, Proof, ProofOfWork};
+use crate::pow::{verify_size, Difficulty, Proof, ProofOfWork};
 use crate::ser::{self, FixedLength, PMMRable, Readable, Reader, Writeable, Writer};
 use crate::util::{secp, static_secp_instance};
 
@@ -52,6 +53,12 @@ pub enum Error {
 	TooHeavy,
 	/// Block weight (based on inputs|outputs|kernels) exceeded.
 	WeightExceeded,
+	/// Block version is invalid for a given block height
+	InvalidBlockVersion(HeaderVersion),
+	/// Block time is invalid
+	InvalidBlockTime,
+	/// Invalid POW
+	InvalidPow,
 	/// Kernel not valid due to lock_height exceeding block header height
 	KernelLockHeight(u64),
 	/// Underlying tx related error
@@ -289,48 +296,44 @@ impl Writeable for BlockHeader {
 	}
 }
 
+fn read_block_header(reader: &mut dyn Reader) -> Result<BlockHeader, ser::Error> {
+	let version = HeaderVersion::read(reader)?;
+	let (height, timestamp) = ser_multiread!(reader, read_u64, read_i64);
+	let prev_hash = Hash::read(reader)?;
+	let prev_root = Hash::read(reader)?;
+	let output_root = Hash::read(reader)?;
+	let range_proof_root = Hash::read(reader)?;
+	let kernel_root = Hash::read(reader)?;
+	let total_kernel_offset = BlindingFactor::read(reader)?;
+	let (output_mmr_size, kernel_mmr_size) = ser_multiread!(reader, read_u64, read_u64);
+	let pow = ProofOfWork::read(reader)?;
+
+	if timestamp > MAX_DATE.and_hms(0, 0, 0).timestamp()
+		|| timestamp < MIN_DATE.and_hms(0, 0, 0).timestamp()
+	{
+		return Err(ser::Error::CorruptedData);
+	}
+
+	Ok(BlockHeader {
+		version,
+		height,
+		timestamp: DateTime::<Utc>::from_utc(NaiveDateTime::from_timestamp(timestamp, 0), Utc),
+		prev_hash,
+		prev_root,
+		output_root,
+		range_proof_root,
+		kernel_root,
+		total_kernel_offset,
+		output_mmr_size,
+		kernel_mmr_size,
+		pow,
+	})
+}
+
 /// Deserialization of a block header
 impl Readable for BlockHeader {
 	fn read(reader: &mut dyn Reader) -> Result<BlockHeader, ser::Error> {
-		let version = HeaderVersion::read(reader)?;
-		let (height, timestamp) = ser_multiread!(reader, read_u64, read_i64);
-		let prev_hash = Hash::read(reader)?;
-		let prev_root = Hash::read(reader)?;
-		let output_root = Hash::read(reader)?;
-		let range_proof_root = Hash::read(reader)?;
-		let kernel_root = Hash::read(reader)?;
-		let total_kernel_offset = BlindingFactor::read(reader)?;
-		let (output_mmr_size, kernel_mmr_size) = ser_multiread!(reader, read_u64, read_u64);
-		let pow = ProofOfWork::read(reader)?;
-
-		if timestamp > MAX_DATE.and_hms(0, 0, 0).timestamp()
-			|| timestamp < MIN_DATE.and_hms(0, 0, 0).timestamp()
-		{
-			return Err(ser::Error::CorruptedData);
-		}
-
-		// Check the block version before proceeding any further.
-		// We want to do this here because blocks can be pretty large
-		// and we want to halt processing as early as possible.
-		// If we receive an invalid block version then the peer is not on our hard-fork.
-		if !consensus::valid_header_version(height, version) {
-			return Err(ser::Error::InvalidBlockVersion);
-		}
-
-		Ok(BlockHeader {
-			version,
-			height,
-			timestamp: DateTime::<Utc>::from_utc(NaiveDateTime::from_timestamp(timestamp, 0), Utc),
-			prev_hash,
-			prev_root,
-			output_root,
-			range_proof_root,
-			kernel_root,
-			total_kernel_offset,
-			output_mmr_size,
-			kernel_mmr_size,
-			pow,
-		})
+		read_block_header(reader)
 	}
 }
 
@@ -397,6 +400,59 @@ impl BlockHeader {
 	}
 }
 
+impl From<UntrustedBlockHeader> for BlockHeader {
+	fn from(header: UntrustedBlockHeader) -> Self {
+		header.0
+	}
+}
+
+/// Block header which does lightweight validation as part of deserialization,
+/// it supposed to be used when we can't trust the channel (eg network)
+pub struct UntrustedBlockHeader(BlockHeader);
+
+/// Deserialization of an untrusted block header
+impl Readable for UntrustedBlockHeader {
+	fn read(reader: &mut dyn Reader) -> Result<UntrustedBlockHeader, ser::Error> {
+		let header = read_block_header(reader)?;
+		if header.timestamp
+			> Utc::now() + Duration::seconds(12 * (consensus::BLOCK_TIME_SEC as i64))
+		{
+			// refuse blocks more than 12 blocks intervals in future (as in bitcoin)
+			// TODO add warning in p2p code if local time is too different from peers
+			error!(
+				"block header {} validation error: block time is more than 12 blocks in future",
+				header.hash()
+			);
+			return Err(ser::Error::CorruptedData);
+		}
+
+		// Check the block version before proceeding any further.
+		// We want to do this here because blocks can be pretty large
+		// and we want to halt processing as early as possible.
+		// If we receive an invalid block version then the peer is not on our hard-fork.
+		if !consensus::valid_header_version(header.height, header.version) {
+			return Err(ser::Error::InvalidBlockVersion);
+		}
+
+		if !header.pow.is_primary() && !header.pow.is_secondary() {
+			error!(
+				"block header {} validation error: invalid edge bits",
+				header.hash()
+			);
+			return Err(ser::Error::CorruptedData);
+		}
+		if let Err(e) = verify_size(&header) {
+			error!(
+				"block header {} validation error: invalid POW: {}",
+				header.hash(),
+				e
+			);
+			return Err(ser::Error::CorruptedData);
+		}
+		Ok(UntrustedBlockHeader(header))
+	}
+}
+
 /// A block as expressed in the MimbleWimble protocol. The reward is
 /// non-explicit, assumed to be deducible from block height (similar to
 /// bitcoin's schedule) and expressed as a global transaction fee (added v.H),
@@ -435,16 +491,7 @@ impl Writeable for Block {
 impl Readable for Block {
 	fn read(reader: &mut dyn Reader) -> Result<Block, ser::Error> {
 		let header = BlockHeader::read(reader)?;
-
 		let body = TransactionBody::read(reader)?;
-
-		// Now "lightweight" validation of the block.
-		// Treat any validation issues as data corruption.
-		// An example of this would be reading a block
-		// that exceeded the allowed number of inputs.
-		body.validate_read(Weighting::AsBlock)
-			.map_err(|_| ser::Error::CorruptedData)?;
-
 		Ok(Block { header, body })
 	}
 }
@@ -770,3 +817,36 @@ impl Block {
 		Ok(())
 	}
 }
+
+impl From<UntrustedBlock> for Block {
+	fn from(block: UntrustedBlock) -> Self {
+		block.0
+	}
+}
+
+/// Block which does lightweight validation as part of deserialization,
+/// it supposed to be used when we can't trust the channel (eg network)
+pub struct UntrustedBlock(Block);
+
+/// Deserialization of an untrusted block header
+impl Readable for UntrustedBlock {
+	fn read(reader: &mut dyn Reader) -> Result<UntrustedBlock, ser::Error> {
+		// we validate header here before parsing the body
+		let header = UntrustedBlockHeader::read(reader)?;
+		let body = TransactionBody::read(reader)?;
+
+		// Now "lightweight" validation of the block.
+		// Treat any validation issues as data corruption.
+		// An example of this would be reading a block
+		// that exceeded the allowed number of inputs.
+		body.validate_read(Weighting::AsBlock).map_err(|e| {
+			error!("read validation error: {}", e);
+			ser::Error::CorruptedData
+		})?;
+		let block = Block {
+			header: header.into(),
+			body,
+		};
+		Ok(UntrustedBlock(block))
+	}
+}