Skip to content

Commit

Permalink
Fix incorrectly signed CrdsValues (#6696) (#6699)
Browse files Browse the repository at this point in the history 
automerge
  • Loading branch information
@mergify @solana-grimes
mergify[bot] authored and solana-grimes committed Nov 3, 2019
1 parent c86bf60 commit d9a9d65
Show file tree
Hide file tree
Showing 8 changed files with 320 additions and 276 deletions.
62 changes: 31 additions & 31 deletions core/src/cluster_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,20 @@
//! * layer 2 - Everyone else, if layer 1 is `2^10`, layer 2 should be able to fit `2^20` number of nodes.
//!
//! Bank needs to provide an interface for us to query the stake weight
use crate::contact_info::ContactInfo;
use crate::crds_gossip::CrdsGossip;
use crate::crds_gossip_error::CrdsGossipError;
use crate::crds_gossip_pull::{CrdsFilter, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS};
use crate::crds_value::{CrdsValue, CrdsValueLabel, EpochSlots, Vote};
use crate::packet::{to_shared_blob, Blob, Packet, SharedBlob};
use crate::repair_service::RepairType;
use crate::result::{Error, Result};
use crate::sendmmsg::{multicast, send_mmsg};
use crate::streamer::{BlobReceiver, BlobSender};
use crate::weighted_shuffle::{weighted_best, weighted_shuffle};
use crate::crds_value::CrdsValue;
use crate::{
contact_info::ContactInfo,
crds_gossip::CrdsGossip,
crds_gossip_error::CrdsGossipError,
crds_gossip_pull::{CrdsFilter, CRDS_GOSSIP_PULL_CRDS_TIMEOUT_MS},
crds_value::{CrdsData, CrdsValueLabel, EpochSlots, Vote},
packet::{to_shared_blob, Blob, Packet, SharedBlob},
repair_service::RepairType,
result::{Error, Result},
sendmmsg::{multicast, send_mmsg},
streamer::{BlobReceiver, BlobSender},
weighted_shuffle::{weighted_best, weighted_shuffle},
};
use bincode::{deserialize, serialize, serialized_size};
use core::cmp;
use itertools::Itertools;
Expand Down Expand Up @@ -195,8 +198,8 @@ impl ClusterInfo {

pub fn insert_self(&mut self, contact_info: ContactInfo) {
if self.id() == contact_info.id {
let mut value = CrdsValue::ContactInfo(contact_info.clone());
value.sign(&self.keypair);
let value =
CrdsValue::new_signed(CrdsData::ContactInfo(contact_info.clone()), &self.keypair);
let _ = self.gossip.crds.insert(value, timestamp());
}
}
Expand All @@ -205,17 +208,15 @@ impl ClusterInfo {
let mut my_data = self.my_data();
let now = timestamp();
my_data.wallclock = now;
let mut entry = CrdsValue::ContactInfo(my_data);
entry.sign(&self.keypair);
let entry = CrdsValue::new_signed(CrdsData::ContactInfo(my_data), &self.keypair);
self.gossip.refresh_push_active_set(stakes);
self.gossip
.process_push_message(&self.id(), vec![entry], now);
}

// TODO kill insert_info, only used by tests
pub fn insert_info(&mut self, contact_info: ContactInfo) {
let mut value = CrdsValue::ContactInfo(contact_info);
value.sign(&self.keypair);
let value = CrdsValue::new_signed(CrdsData::ContactInfo(contact_info), &self.keypair);
let _ = self.gossip.crds.insert(value, timestamp());
}

Expand Down Expand Up @@ -297,17 +298,18 @@ impl ClusterInfo {

pub fn push_epoch_slots(&mut self, id: Pubkey, root: u64, slots: BTreeSet<u64>) {
let now = timestamp();
let mut entry = CrdsValue::EpochSlots(EpochSlots::new(id, root, slots, now));
entry.sign(&self.keypair);
let entry = CrdsValue::new_signed(
CrdsData::EpochSlots(EpochSlots::new(id, root, slots, now)),
&self.keypair,
);
self.gossip
.process_push_message(&self.id(), vec![entry], now);
}

pub fn push_vote(&mut self, vote: Transaction) {
let now = timestamp();
let vote = Vote::new(&self.id(), vote, now);
let mut entry = CrdsValue::Vote(vote);
entry.sign(&self.keypair);
let entry = CrdsValue::new_signed(CrdsData::Vote(vote), &self.keypair);
self.gossip
.process_push_message(&self.id(), vec![entry], now);
}
Expand Down Expand Up @@ -915,7 +917,7 @@ impl ClusterInfo {
.expect("unable to serialize default filter") as usize;
let protocol = Protocol::PullRequest(
CrdsFilter::default(),
CrdsValue::ContactInfo(ContactInfo::default()),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default())),
);
let protocol_size =
serialized_size(&protocol).expect("unable to serialize gossip protocol") as usize;
Expand Down Expand Up @@ -1161,9 +1163,7 @@ impl ClusterInfo {
1
);
} else if caller.contact_info().is_some() {
if caller.contact_info().unwrap().pubkey()
== me.read().unwrap().gossip.id
{
if caller.contact_info().unwrap().id == me.read().unwrap().gossip.id {
warn!("PullRequest ignored, I'm talking to myself");
inc_new_counter_debug!("cluster_info-window-request-loopback", 1);
} else {
Expand Down Expand Up @@ -2384,7 +2384,8 @@ mod tests {
}

// now add this message back to the table and make sure after the next pull, the entrypoint is unset
let entrypoint_crdsvalue = CrdsValue::ContactInfo(entrypoint.clone());
let entrypoint_crdsvalue =
CrdsValue::new_unsigned(CrdsData::ContactInfo(entrypoint.clone()));
let cluster_info = Arc::new(RwLock::new(cluster_info));
ClusterInfo::handle_pull_response(
&cluster_info,
Expand All @@ -2401,7 +2402,7 @@ mod tests {

#[test]
fn test_split_messages_small() {
let value = CrdsValue::ContactInfo(ContactInfo::default());
let value = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
test_split_messages(value);
}

Expand All @@ -2411,13 +2412,12 @@ mod tests {
for i in 0..128 {
btree_slots.insert(i);
}
let value = CrdsValue::EpochSlots(EpochSlots {
let value = CrdsValue::new_unsigned(CrdsData::EpochSlots(EpochSlots {
from: Pubkey::default(),
root: 0,
slots: btree_slots,
signature: Signature::default(),
wallclock: 0,
});
}));
test_split_messages(value);
}

Expand All @@ -2441,7 +2441,7 @@ mod tests {
}

fn check_pull_request_size(filter: CrdsFilter) {
let value = CrdsValue::ContactInfo(ContactInfo::default());
let value = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
let protocol = Protocol::PullRequest(filter, value.clone());
assert!(serialized_size(&protocol).unwrap() <= PACKET_DATA_SIZE as u64);
}
Expand Down
52 changes: 0 additions & 52 deletions core/src/contact_info.rs
View file
Original file line number Diff line number Diff line change
@@ -1,21 +1,16 @@
use bincode::serialize;
use solana_sdk::pubkey::Pubkey;
#[cfg(test)]
use solana_sdk::rpc_port;
#[cfg(test)]
use solana_sdk::signature::{Keypair, KeypairUtil};
use solana_sdk::signature::{Signable, Signature};
use solana_sdk::timing::timestamp;
use std::borrow::Cow;
use std::cmp::{Ord, Ordering, PartialEq, PartialOrd};
use std::net::{IpAddr, SocketAddr};

/// Structure representing a node on the network
#[derive(Serialize, Deserialize, Clone, Debug)]
pub struct ContactInfo {
pub id: Pubkey,
/// signature of this ContactInfo
pub signature: Signature,
/// gossip address
pub gossip: SocketAddr,
/// address to connect to for replication
Expand Down Expand Up @@ -89,7 +84,6 @@ impl Default for ContactInfo {
rpc: socketaddr_any!(),
rpc_pubsub: socketaddr_any!(),
wallclock: 0,
signature: Signature::default(),
}
}
}
Expand All @@ -111,7 +105,6 @@ impl ContactInfo {
) -> Self {
Self {
id: *id,
signature: Signature::default(),
gossip,
tvu,
tvu_forwards,
Expand Down Expand Up @@ -242,51 +235,6 @@ impl ContactInfo {
}
}

impl Signable for ContactInfo {
fn pubkey(&self) -> Pubkey {
self.id
}

fn signable_data(&self) -> Cow<[u8]> {
#[derive(Serialize)]
struct SignData {
id: Pubkey,
gossip: SocketAddr,
tvu: SocketAddr,
tpu: SocketAddr,
tpu_forwards: SocketAddr,
repair: SocketAddr,
storage_addr: SocketAddr,
rpc: SocketAddr,
rpc_pubsub: SocketAddr,
wallclock: u64,
}

let me = self;
let data = SignData {
id: me.id,
gossip: me.gossip,
tvu: me.tvu,
tpu: me.tpu,
storage_addr: me.storage_addr,
tpu_forwards: me.tpu_forwards,
repair: me.repair,
rpc: me.rpc,
rpc_pubsub: me.rpc_pubsub,
wallclock: me.wallclock,
};
Cow::Owned(serialize(&data).expect("failed to serialize ContactInfo"))
}

fn get_signature(&self) -> Signature {
self.signature
}

fn set_signature(&mut self, signature: Signature) {
self.signature = signature
}
}

#[cfg(test)]
mod tests {
use super::*;
Expand Down
57 changes: 41 additions & 16 deletions core/src/crds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -165,11 +165,12 @@ impl Crds {
mod test {
use super::*;
use crate::contact_info::ContactInfo;
use crate::crds_value::CrdsData;

#[test]
fn test_insert() {
let mut crds = Crds::default();
let val = CrdsValue::ContactInfo(ContactInfo::default());
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
assert_eq!(crds.insert(val.clone(), 0).ok(), Some(None));
assert_eq!(crds.table.len(), 1);
assert!(crds.table.contains_key(&val.label()));
Expand All @@ -178,17 +179,23 @@ mod test {
#[test]
fn test_update_old() {
let mut crds = Crds::default();
let val = CrdsValue::ContactInfo(ContactInfo::default());
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
assert_eq!(crds.insert(val.clone(), 0), Ok(None));
assert_eq!(crds.insert(val.clone(), 1), Err(CrdsError::InsertFailed));
assert_eq!(crds.table[&val.label()].local_timestamp, 0);
}
#[test]
fn test_update_new() {
let mut crds = Crds::default();
let original = CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 0));
let original = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
0,
)));
assert_matches!(crds.insert(original.clone(), 0), Ok(_));
let val = CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 1));
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
1,
)));
assert_eq!(
crds.insert(val.clone(), 1).unwrap().unwrap().value,
original
Expand All @@ -198,14 +205,17 @@ mod test {
#[test]
fn test_update_timestamp() {
let mut crds = Crds::default();
let val = CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 0));
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
0,
)));
assert_eq!(crds.insert(val.clone(), 0), Ok(None));

crds.update_label_timestamp(&val.label(), 1);
assert_eq!(crds.table[&val.label()].local_timestamp, 1);
assert_eq!(crds.table[&val.label()].insert_timestamp, 0);

let val2 = CrdsValue::ContactInfo(ContactInfo::default());
let val2 = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
assert_eq!(val2.label().pubkey(), val.label().pubkey());
assert_matches!(crds.insert(val2.clone(), 0), Ok(Some(_)));

Expand All @@ -221,15 +231,15 @@ mod test {

let mut ci = ContactInfo::default();
ci.wallclock += 1;
let val3 = CrdsValue::ContactInfo(ci);
let val3 = CrdsValue::new_unsigned(CrdsData::ContactInfo(ci));
assert_matches!(crds.insert(val3.clone(), 3), Ok(Some(_)));
assert_eq!(crds.table[&val2.label()].local_timestamp, 3);
assert_eq!(crds.table[&val2.label()].insert_timestamp, 3);
}
#[test]
fn test_find_old_records() {
let mut crds = Crds::default();
let val = CrdsValue::ContactInfo(ContactInfo::default());
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
assert_eq!(crds.insert(val.clone(), 1), Ok(None));

assert!(crds.find_old_labels(0).is_empty());
Expand All @@ -239,7 +249,7 @@ mod test {
#[test]
fn test_remove() {
let mut crds = Crds::default();
let val = CrdsValue::ContactInfo(ContactInfo::default());
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
assert_matches!(crds.insert(val.clone(), 1), Ok(_));

assert_eq!(crds.find_old_labels(1), vec![val.label()]);
Expand All @@ -248,7 +258,7 @@ mod test {
}
#[test]
fn test_equal() {
let val = CrdsValue::ContactInfo(ContactInfo::default());
let val = CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::default()));
let v1 = VersionedCrdsValue::new(1, val.clone());
let v2 = VersionedCrdsValue::new(1, val);
assert_eq!(v1, v2);
Expand All @@ -258,12 +268,15 @@ mod test {
fn test_hash_order() {
let v1 = VersionedCrdsValue::new(
1,
CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 0)),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
0,
))),
);
let v2 = VersionedCrdsValue::new(1, {
let mut contact_info = ContactInfo::new_localhost(&Pubkey::default(), 0);
contact_info.rpc = socketaddr!("0.0.0.0:0");
CrdsValue::ContactInfo(contact_info)
CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info))
});

assert_eq!(v1.value.label(), v2.value.label());
Expand All @@ -285,11 +298,17 @@ mod test {
fn test_wallclock_order() {
let v1 = VersionedCrdsValue::new(
1,
CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 1)),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
1,
))),
);
let v2 = VersionedCrdsValue::new(
1,
CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::default(), 0)),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::default(),
0,
))),
);
assert_eq!(v1.value.label(), v2.value.label());
assert!(v1 > v2);
Expand All @@ -301,11 +320,17 @@ mod test {
fn test_label_order() {
let v1 = VersionedCrdsValue::new(
1,
CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::new_rand(), 0)),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::new_rand(),
0,
))),
);
let v2 = VersionedCrdsValue::new(
1,
CrdsValue::ContactInfo(ContactInfo::new_localhost(&Pubkey::new_rand(), 0)),
CrdsValue::new_unsigned(CrdsData::ContactInfo(ContactInfo::new_localhost(
&Pubkey::new_rand(),
0,
))),
);
assert_ne!(v1, v2);
assert!(!(v1 == v2));
Expand Down
Loading

0 comments on commit d9a9d65

Please sign in to comment.