diff --git a/gossip/src/duplicate_shred.rs b/gossip/src/duplicate_shred.rs index adfd89595a0..940d0bf75e2 100644 --- a/gossip/src/duplicate_shred.rs +++ b/gossip/src/duplicate_shred.rs @@ -98,7 +98,8 @@ pub enum Error { /// - Must match the expected shred version /// - Must both sigverify for the correct leader /// - Must have a merkle root conflict, otherwise `shred1` and `shred2` must have the same `shred_type` -/// - If `shred1` and `shred2` share the same index they must be not equal +/// - If `shred1` and `shred2` share the same index they must be not have equal payloads excluding the +/// retransmitter signature /// - If `shred1` and `shred2` do not share the same index and are data shreds /// verify that they indicate an index conflict. One of them must be the /// LAST_SHRED_IN_SLOT, however the other shred must have a higher index. @@ -147,7 +148,7 @@ where } if shred1.index() == shred2.index() { - if shred1.payload() != shred2.payload() { + if shred1.is_shred_duplicate(shred2) { return Ok(()); } return Err(Error::InvalidDuplicateShreds); @@ -314,7 +315,7 @@ pub(crate) mod tests { solana_ledger::shred::{ProcessShredsStats, ReedSolomonCache, Shredder}, solana_sdk::{ hash::Hash, - signature::{Keypair, Signer}, + signature::{Keypair, Signature, Signer}, system_transaction, }, std::sync::Arc, @@ -1255,4 +1256,82 @@ pub(crate) mod tests { ); } } + + #[test] + fn test_retransmitter_signature_invalid() { + let mut rng = rand::thread_rng(); + let leader = Arc::new(Keypair::new()); + let (slot, parent_slot, reference_tick, version) = (53084024, 53084023, 0, 0); + let shredder = Shredder::new(slot, parent_slot, reference_tick, version).unwrap(); + let next_shred_index = rng.gen_range(0..32_000); + let leader_schedule = |s| { + if s == slot { + Some(leader.pubkey()) + } else { + None + } + }; + let data_shred = + new_rand_data_shred(&mut rng, next_shred_index, &shredder, &leader, true, true); + let coding_shred = + new_rand_coding_shreds(&mut rng, next_shred_index, 10, &shredder, &leader, true)[0] + .clone(); + let mut data_shred_different_retransmitter_payload = data_shred.clone().into_payload(); + shred::layout::set_retransmitter_signature( + &mut data_shred_different_retransmitter_payload, + &Signature::new_unique(), + ) + .unwrap(); + let data_shred_different_retransmitter = + Shred::new_from_serialized_shred(data_shred_different_retransmitter_payload).unwrap(); + let mut coding_shred_different_retransmitter_payload = coding_shred.clone().into_payload(); + shred::layout::set_retransmitter_signature( + &mut coding_shred_different_retransmitter_payload, + &Signature::new_unique(), + ) + .unwrap(); + let coding_shred_different_retransmitter = + Shred::new_from_serialized_shred(coding_shred_different_retransmitter_payload).unwrap(); + + let test_cases = vec![ + // Same data shred from different retransmitter + (data_shred, data_shred_different_retransmitter), + // Same coding shred from different retransmitter + (coding_shred, coding_shred_different_retransmitter), + ]; + for (shred1, shred2) in test_cases.iter().flat_map(|(a, b)| [(a, b), (b, a)]) { + assert_matches!( + from_shred( + shred1.clone(), + Pubkey::new_unique(), // self_pubkey + shred2.payload().clone(), + Some(leader_schedule), + rng.gen(), // wallclock + 512, // max_size + version, + ) + .err() + .unwrap(), + Error::InvalidDuplicateShreds + ); + + let chunks: Vec<_> = from_shred_bypass_checks( + shred1.clone(), + Pubkey::new_unique(), // self_pubkey + shred2.clone(), + rng.gen(), // wallclock + 512, // max_size + ) + .unwrap() + .collect(); + assert!(chunks.len() > 4); + + assert_matches!( + into_shreds(&leader.pubkey(), chunks, version) + .err() + .unwrap(), + Error::InvalidDuplicateShreds + ); + } + } } diff --git a/ledger/src/shred.rs b/ledger/src/shred.rs index f2b7a84ed1e..dc57307c42a 100644 --- a/ledger/src/shred.rs +++ b/ledger/src/shred.rs @@ -576,6 +576,37 @@ impl Shred { Self::ShredData(_) => Err(Error::InvalidShredType), } } + + /// Returns true if the other shred has the same ShredId, i.e. (slot, index, + /// shred-type), but different payload. + /// Retransmitter's signature is ignored when comparing payloads. + pub fn is_shred_duplicate(&self, other: &Shred) -> bool { + if self.id() != other.id() { + return false; + } + fn get_payload(shred: &Shred) -> &[u8] { + let Ok(offset) = shred.retransmitter_signature_offset() else { + return shred.payload(); + }; + // Assert that the retransmitter's signature is at the very end of + // the shred payload. + debug_assert_eq!(offset + SIZE_OF_SIGNATURE, shred.payload().len()); + shred + .payload() + .get(..offset) + .unwrap_or_else(|| shred.payload()) + } + get_payload(self) != get_payload(other) + } + + fn retransmitter_signature_offset(&self) -> Result { + match self { + Self::ShredCode(ShredCode::Merkle(shred)) => shred.retransmitter_signature_offset(), + Self::ShredData(ShredData::Merkle(shred)) => shred.retransmitter_signature_offset(), + Self::ShredCode(ShredCode::Legacy(_)) => Err(Error::InvalidShredVariant), + Self::ShredData(ShredData::Legacy(_)) => Err(Error::InvalidShredVariant), + } + } } // Helper methods to extract pieces of the shred from the payload @@ -786,7 +817,7 @@ pub mod layout { .ok_or(Error::InvalidPayloadSize(shred.len())) } - pub(crate) fn set_retransmitter_signature( + pub fn set_retransmitter_signature( shred: &mut [u8], signature: &Signature, ) -> Result<(), Error> { diff --git a/ledger/src/shred/merkle.rs b/ledger/src/shred/merkle.rs index e569999ba3d..3b9e29934f3 100644 --- a/ledger/src/shred/merkle.rs +++ b/ledger/src/shred/merkle.rs @@ -445,7 +445,7 @@ macro_rules! impl_merkle_shred { Ok(()) } - fn retransmitter_signature_offset(&self) -> Result { + pub(super) fn retransmitter_signature_offset(&self) -> Result { let ShredVariant::$variant { proof_size, chained,