From d7ba221b4826c81229468bb6d9d5474eb697743a Mon Sep 17 00:00:00 2001 From: John Smith Date: Wed, 11 May 2022 09:37:54 -0400 Subject: [PATCH] fix signing and validation add timestamp to signed node info --- veilid-core/proto/veilid.capnp | 1 + veilid-core/src/routing_table/bucket_entry.rs | 9 +++++++++ veilid-core/src/routing_table/mod.rs | 10 ++++++++++ veilid-core/src/routing_table/node_ref.rs | 7 +++++++ .../src/rpc_processor/coders/signature.rs | 2 +- .../rpc_processor/coders/signed_node_info.rs | 7 ++++++- veilid-core/src/veilid_api/mod.rs | 19 +++++++++++++++++-- 7 files changed, 51 insertions(+), 4 deletions(-) diff --git a/veilid-core/proto/veilid.capnp b/veilid-core/proto/veilid.capnp index 40be60e6..9f42e5da 100644 --- a/veilid-core/proto/veilid.capnp +++ b/veilid-core/proto/veilid.capnp @@ -224,6 +224,7 @@ struct NodeInfo { struct SignedNodeInfo { nodeInfo @0 :NodeInfo; # node info signature @1 :Signature; # signature + timestamp @2 :UInt64; # when signed node info was generated } struct SenderInfo { diff --git a/veilid-core/src/routing_table/bucket_entry.rs b/veilid-core/src/routing_table/bucket_entry.rs index 570e6399..3d16fb7b 100644 --- a/veilid-core/src/routing_table/bucket_entry.rs +++ b/veilid-core/src/routing_table/bucket_entry.rs @@ -108,6 +108,15 @@ impl BucketEntry { } pub fn update_node_info(&mut self, signed_node_info: SignedNodeInfo) { + // Don't update with older node info, or something less valid + if let Some(current_sni) = &self.opt_signed_node_info { + if current_sni.signature.valid && !signed_node_info.signature.valid { + return; + } + if signed_node_info.timestamp < current_sni.timestamp { + return; + } + } self.opt_signed_node_info = Some(signed_node_info); } pub fn update_local_node_info(&mut self, local_node_info: LocalNodeInfo) { diff --git a/veilid-core/src/routing_table/mod.rs b/veilid-core/src/routing_table/mod.rs index 993d2783..701d294d 100644 --- a/veilid-core/src/routing_table/mod.rs +++ b/veilid-core/src/routing_table/mod.rs @@ -487,6 +487,16 @@ impl RoutingTable { node_id: DHTKey, signed_node_info: SignedNodeInfo, ) -> Result { + // validate signed node info is not something malicious + if node_id == self.node_id() { + return Err("can't register own node id in routing table".to_owned()); + } + if let Some(rpi) = &signed_node_info.node_info.relay_peer_info { + if rpi.node_id.key == node_id { + return Err("node can not be its own relay".to_owned()); + } + } + let nr = self.create_node_ref(node_id, |e| { e.update_node_info(signed_node_info); })?; diff --git a/veilid-core/src/routing_table/node_ref.rs b/veilid-core/src/routing_table/node_ref.rs index 13b88869..593d63f3 100644 --- a/veilid-core/src/routing_table/node_ref.rs +++ b/veilid-core/src/routing_table/node_ref.rs @@ -81,6 +81,13 @@ impl NodeRef { pub fn relay(&self) -> Option { let target_rpi = self.operate(|e| e.node_info().map(|n| n.relay_peer_info))?; target_rpi.and_then(|t| { + // If relay is ourselves, then return None, because we can't relay through ourselves + // and to contact this node we should have had an existing inbound connection + if t.node_id.key == self.routing_table.node_id() { + return None; + } + + // Register relay node and return noderef self.routing_table .register_node_with_signed_node_info(t.node_id.key, t.signed_node_info) .map_err(logthru_rtab!(error)) diff --git a/veilid-core/src/rpc_processor/coders/signature.rs b/veilid-core/src/rpc_processor/coders/signature.rs index 01f8b124..e41eb9a9 100644 --- a/veilid-core/src/rpc_processor/coders/signature.rs +++ b/veilid-core/src/rpc_processor/coders/signature.rs @@ -5,7 +5,7 @@ pub fn encode_signature( sig: &DHTSignature, builder: &mut veilid_capnp::ed25519_signature::Builder, ) { - if sig.valid { + if !sig.valid { panic!("don't encode invalid signatures"); } diff --git a/veilid-core/src/rpc_processor/coders/signed_node_info.rs b/veilid-core/src/rpc_processor/coders/signed_node_info.rs index f722ec5e..f9ec7b91 100644 --- a/veilid-core/src/rpc_processor/coders/signed_node_info.rs +++ b/veilid-core/src/rpc_processor/coders/signed_node_info.rs @@ -12,6 +12,8 @@ pub fn encode_signed_node_info( let mut sig_builder = builder.reborrow().init_signature(); encode_signature(&signed_node_info.signature, &mut sig_builder); + builder.reborrow().set_timestamp(signed_node_info.timestamp); + Ok(()) } @@ -32,5 +34,8 @@ pub fn decode_signed_node_info( .map_err(map_error_capnp_error!())?; let signature = decode_signature(&sig_reader); - SignedNodeInfo::new(node_info, NodeId::new(*node_id), signature).map_err(map_error_string!()) + let timestamp = reader.reborrow().get_timestamp(); + + SignedNodeInfo::new(node_info, NodeId::new(*node_id), signature, timestamp) + .map_err(map_error_string!()) } diff --git a/veilid-core/src/veilid_api/mod.rs b/veilid-core/src/veilid_api/mod.rs index 7d3e87de..b8c646c2 100644 --- a/veilid-core/src/veilid_api/mod.rs +++ b/veilid-core/src/veilid_api/mod.rs @@ -989,6 +989,7 @@ impl Default for PeerScope { pub struct SignedNodeInfo { pub node_info: NodeInfo, pub signature: DHTSignature, + pub timestamp: u64, } impl SignedNodeInfo { @@ -996,12 +997,18 @@ impl SignedNodeInfo { node_info: NodeInfo, node_id: NodeId, signature: DHTSignature, + timestamp: u64, ) -> Result { - let node_info_bytes = serde_cbor::to_vec(&node_info).map_err(map_to_string)?; + let mut node_info_bytes = serde_cbor::to_vec(&node_info).map_err(map_to_string)?; + let mut timestamp_bytes = serde_cbor::to_vec(×tamp).map_err(map_to_string)?; + + node_info_bytes.append(&mut timestamp_bytes); + verify(&node_id.key, &node_info_bytes, &signature)?; Ok(Self { node_info, signature, + timestamp, }) } @@ -1010,11 +1017,18 @@ impl SignedNodeInfo { node_id: NodeId, secret: &DHTKeySecret, ) -> Result { - let node_info_bytes = serde_cbor::to_vec(&node_info).map_err(map_to_string)?; + let timestamp = intf::get_timestamp(); + + let mut node_info_bytes = serde_cbor::to_vec(&node_info).map_err(map_to_string)?; + let mut timestamp_bytes = serde_cbor::to_vec(×tamp).map_err(map_to_string)?; + + node_info_bytes.append(&mut timestamp_bytes); + let signature = sign(&node_id.key, secret, &node_info_bytes)?; Ok(Self { node_info, signature, + timestamp, }) } @@ -1022,6 +1036,7 @@ impl SignedNodeInfo { Self { node_info, signature: DHTSignature::default(), + timestamp: intf::get_timestamp(), } } }