Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Commit

Permalink
fix: prevent creating Section with elders info signed with wrong key
Browse files Browse the repository at this point in the history
  • Loading branch information
madadam committed Mar 4, 2021
1 parent ec53c63 commit f0f839c
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 14 deletions.
20 changes: 11 additions & 9 deletions examples/stress.rs
Original file line number Diff line number Diff line change
Expand Up @@ -439,17 +439,23 @@ impl Network {
return Ok(false);
};

// There can be a significant delay between a node being relocated and us receiving the
// `Relocated` event. Using the current node name instead of the one reported by the last
// `Relocated` event reduced send errors due to src location mismatch which would cause the
// section health to appear lower than it actually is.
let src = node.name().await;

// The message dst is unique so we use it also as its indentifier.
let bytes = bincode::serialize(&dst)?;
let signature_share = node
.sign_as_elder(&bytes, &public_key_set.public_key())
.await
.context("failed to sign probe")?;
.with_context(|| format!("failed to sign probe by {}", src))?;

let index = node
.our_index()
.await
.context("failed to retrieve key share index")?;
.with_context(|| format!("failed to retrieve key share index by {}", src))?;

let message = ProbeMessage {
proof_share: ProofShare {
Expand All @@ -460,12 +466,6 @@ impl Network {
};
let bytes = bincode::serialize(&message)?.into();

// There can be a significant delay between a node being relocated and us receiving the
// `Relocated` event. Using the current node name instead of the one reported by the last
// `Relocated` event reduced send errors due to src location mismatch which would cause the
// section health to appear lower than it actually is.
let src = node.name().await;

let itinerary = Itinerary {
src: SrcLocation::Node(src),
dst: DstLocation::Section(dst),
Expand All @@ -475,7 +475,9 @@ impl Network {
match node.send_message(itinerary, bytes).await {
Ok(()) => Ok(true),
Err(RoutingError::InvalidSrcLocation) => Ok(false), // node name changed
Err(error) => Err(Error::from(error).context("failed to send probe")),
Err(error) => {
Err(Error::from(error).context(format!("failed to send probe by {}", src)))
}
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/routing/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1030,7 +1030,7 @@ async fn handle_bounced_unknown_message() -> Result<()> {
let mut section_chain = SectionChain::new(pk0);
let _ = section_chain.insert(&pk0, pk1, pk1_signature);

let proven_elders_info = proven(&sk0, elders_info)?;
let proven_elders_info = proven(sk1_set.secret_key(), elders_info)?;
let section = Section::new(section_chain, proven_elders_info)?;
let section_key_share = create_section_key_share(&sk1_set, 0);

Expand Down Expand Up @@ -1123,7 +1123,7 @@ async fn handle_bounced_untrusted_message() -> Result<()> {
let mut chain = SectionChain::new(pk0);
let _ = chain.insert(&pk0, pk1, pk1_signature);

let proven_elders_info = proven(&sk0, elders_info)?;
let proven_elders_info = proven(sk1_set.secret_key(), elders_info)?;
let section = Section::new(chain.clone(), proven_elders_info)?;
let section_key_share = create_section_key_share(&sk1_set, 0);

Expand Down Expand Up @@ -1213,7 +1213,7 @@ async fn handle_sync() -> Result<()> {
assert_eq!(chain.insert(&pk0, pk1, pk1_signature), Ok(()));

let (old_elders_info, mut nodes) = create_elders_info();
let proven_old_elders_info = proven(&sk0, old_elders_info.clone())?;
let proven_old_elders_info = proven(sk1_set.secret_key(), old_elders_info.clone())?;
let old_section = Section::new(chain.clone(), proven_old_elders_info)?;

// Create our node
Expand Down Expand Up @@ -1374,7 +1374,7 @@ async fn handle_bounced_untrusted_sync() -> Result<()> {
chain.insert(&pk1, pk2, sig2)?;

let (elders_info, mut nodes) = create_elders_info();
let proven_elders_info = proven(&sk0, elders_info.clone())?;
let proven_elders_info = proven(sk2, elders_info.clone())?;
let section_full = Section::new(chain, proven_elders_info)?;
let section_trimmed = section_full.trimmed(2);

Expand Down
5 changes: 4 additions & 1 deletion src/section/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,11 @@ pub(crate) struct Section {
impl Section {
/// Creates a minimal `Section` initially containing only info about our elders
/// (`elders_info`).
///
/// Returns error if `elders_info` is not signed with the last key of `chain`.
pub fn new(chain: SectionChain, elders_info: Proven<EldersInfo>) -> Result<Self, Error> {
if !chain.has_key(&elders_info.proof.public_key) {
if elders_info.proof.public_key != *chain.last_key() {
error!("can't create section: elders_info signed with incorrect key");
// TODO: consider more specific error here.
return Err(Error::InvalidMessage);
}
Expand Down

0 comments on commit f0f839c

Please sign in to comment.