Verify headers and blocks only when needed (#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 #1642

* Move version validation to untrusted header

* update fuzz tests
This commit is contained in:
hashmap 2019-10-27 08:40:52 +01:00 committed by GitHub
parent 8a7da89d47
commit 1f5de6beb9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 190 additions and 111 deletions

View file

@ -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)?;

16
core/fuzz/Cargo.lock generated
View file

@ -365,7 +365,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
[[package]]
name = "grin_core"
version = "2.1.0-beta.1"
version = "3.0.0-alpha.1"
dependencies = [
"blake2-rfc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"byteorder 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
@ -374,8 +374,8 @@ dependencies = [
"enum_primitive 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
"failure 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
"failure_derive 0.1.5 (registry+https://github.com/rust-lang/crates.io-index)",
"grin_keychain 2.1.0-beta.1",
"grin_util 2.1.0-beta.1",
"grin_keychain 3.0.0-alpha.1",
"grin_util 3.0.0-alpha.1",
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
"lru-cache 0.1.1 (registry+https://github.com/rust-lang/crates.io-index)",
@ -393,19 +393,19 @@ dependencies = [
name = "grin_core-fuzz"
version = "0.0.3"
dependencies = [
"grin_core 2.1.0-beta.1",
"grin_keychain 2.1.0-beta.1",
"grin_core 3.0.0-alpha.1",
"grin_keychain 3.0.0-alpha.1",
"libfuzzer-sys 0.1.0 (git+https://github.com/rust-fuzz/libfuzzer-sys.git)",
]
[[package]]
name = "grin_keychain"
version = "2.1.0-beta.1"
version = "3.0.0-alpha.1"
dependencies = [
"blake2-rfc 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)",
"byteorder 1.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
"digest 0.7.6 (registry+https://github.com/rust-lang/crates.io-index)",
"grin_util 2.1.0-beta.1",
"grin_util 3.0.0-alpha.1",
"hmac 0.6.3 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"log 0.4.6 (registry+https://github.com/rust-lang/crates.io-index)",
@ -437,7 +437,7 @@ dependencies = [
[[package]]
name = "grin_util"
version = "2.1.0-beta.1"
version = "3.0.0-alpha.1"
dependencies = [
"backtrace 0.3.14 (registry+https://github.com/rust-lang/crates.io-index)",
"base64 0.9.3 (registry+https://github.com/rust-lang/crates.io-index)",

View file

@ -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));
});

View file

@ -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));
});

View file

@ -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));
});

View file

@ -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));
});

View file

@ -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))
}
}

View file

@ -16,7 +16,7 @@
use rand::{thread_rng, Rng};
use crate::core::block::{Block, BlockHeader, Error};
use crate::core::block::{Block, BlockHeader, Error, UntrustedBlockHeader};
use crate::core::hash::{DefaultHashable, Hashed};
use crate::core::id::ShortIdentifiable;
use crate::core::{Output, ShortId, TxKernel};
@ -221,15 +221,41 @@ impl Readable for CompactBlock {
let nonce = reader.read_u64()?;
let body = CompactBlockBody::read(reader)?;
let cb = CompactBlock {
Ok(CompactBlock {
header,
nonce,
body,
})
}
}
impl From<UntrustedCompactBlock> for CompactBlock {
fn from(ucb: UntrustedCompactBlock) -> Self {
ucb.0
}
}
/// Compackt 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 UntrustedCompactBlock(CompactBlock);
/// Implementation of Readable for an untrusted compact block, defines how to read a
/// compact block from a binary stream.
impl Readable for UntrustedCompactBlock {
fn read(reader: &mut dyn Reader) -> Result<UntrustedCompactBlock, ser::Error> {
let header = UntrustedBlockHeader::read(reader)?;
let nonce = reader.read_u64()?;
let body = CompactBlockBody::read(reader)?;
let cb = CompactBlock {
header: header.into(),
nonce,
body,
};
// Now validate the compact block and treat any validation error as corrupted data.
cb.validate_read().map_err(|_| ser::Error::CorruptedData)?;
Ok(cb)
Ok(UntrustedCompactBlock(cb))
}
}

View file

@ -53,6 +53,9 @@ where
{
/// Create a new graph with given parameters
pub fn new(max_edges: T, max_sols: u32, proof_size: usize) -> Result<Graph<T>, Error> {
if to_u64!(max_edges) >= u64::max_value() / 2 {
return Err(ErrorKind::Verification(format!("graph is to big to build")))?;
}
let max_nodes = 2 * to_u64!(max_edges);
Ok(Graph {
max_edges,
@ -483,5 +486,4 @@ mod test {
}
Ok(())
}
}

View file

@ -429,7 +429,7 @@ fn read_number(bits: &Vec<u8>, bit_start: usize, bit_count: usize) -> u64 {
impl Readable for Proof {
fn read(reader: &mut dyn Reader) -> Result<Proof, ser::Error> {
let edge_bits = reader.read_u8()?;
if edge_bits == 0 || edge_bits > 64 {
if edge_bits == 0 || edge_bits > 63 {
return Err(ser::Error::CorruptedData);
}
@ -509,7 +509,7 @@ mod tests {
#[test]
fn test_proof_rw() {
for edge_bits in 10..64 {
for edge_bits in 10..63 {
let mut proof = Proof::new(gen_proof(edge_bits as u32));
proof.edge_bits = edge_bits;
let mut buf = Cursor::new(Vec::new());
@ -541,5 +541,4 @@ mod tests {
}
v
}
}

View file

@ -161,13 +161,13 @@ impl MessageHandler for Protocol {
"handle_payload: received block: msg_len: {}",
msg.header.msg_len
);
let b: core::Block = msg.body()?;
let b: core::UntrustedBlock = msg.body()?;
// We default to NONE opts here as we do not know know yet why this block was
// received.
// If we requested this block from a peer due to our node syncing then
// the peer adapter will override opts to reflect this.
adapter.block_received(b, &self.peer_info, chain::Options::NONE)?;
adapter.block_received(b.into(), &self.peer_info, chain::Options::NONE)?;
Ok(None)
}
@ -190,9 +190,9 @@ impl MessageHandler for Protocol {
"handle_payload: received compact block: msg_len: {}",
msg.header.msg_len
);
let b: core::CompactBlock = msg.body()?;
let b: core::UntrustedCompactBlock = msg.body()?;
adapter.compact_block_received(b, &self.peer_info)?;
adapter.compact_block_received(b.into(), &self.peer_info)?;
Ok(None)
}
@ -212,8 +212,8 @@ impl MessageHandler for Protocol {
// "header first" block propagation - if we have not yet seen this block
// we can go request it from some of our peers
Type::Header => {
let header: core::BlockHeader = msg.body()?;
adapter.header_received(header, &self.peer_info)?;
let header: core::UntrustedBlockHeader = msg.body()?;
adapter.header_received(header.into(), &self.peer_info)?;
Ok(None)
}
@ -229,8 +229,9 @@ impl MessageHandler for Protocol {
for chunk in (0..count).collect::<Vec<_>>().chunks(chunk_size) {
let mut headers = vec![];
for _ in chunk {
let (header, bytes_read) = msg.streaming_read()?;
headers.push(header);
let (header, bytes_read) =
msg.streaming_read::<core::UntrustedBlockHeader>()?;
headers.push(header.into());
total_bytes_read += bytes_read;
}
adapter.headers_received(&headers, &self.peer_info)?;