From c9595d85498d2c2711d5aa309d575a78ff10444e Mon Sep 17 00:00:00 2001 From: John Smith Date: Sat, 19 Nov 2022 19:05:43 -0500 Subject: [PATCH] pr fixes --- veilid-core/src/crypto/key.rs | 39 +++++++++-------- veilid-core/src/crypto/tests/test_dht_key.rs | 4 +- .../src/routing_table/route_spec_store.rs | 43 ++++++++++++++----- veilid-core/src/rpc_processor/rpc_route.rs | 41 +++++++++--------- veilid-core/src/veilid_api/privacy.rs | 27 ++++++++++++ 5 files changed, 102 insertions(+), 52 deletions(-) diff --git a/veilid-core/src/crypto/key.rs b/veilid-core/src/crypto/key.rs index 90f57297..c5468084 100644 --- a/veilid-core/src/crypto/key.rs +++ b/veilid-core/src/crypto/key.rs @@ -175,7 +175,8 @@ macro_rules! byte_array_type { impl fmt::Display for $name { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { - write!(f, "{}", String::from(self)) + //write!(f, "{}", String::from(self)) + write!(f, "{}", self.encode()) } } @@ -189,12 +190,13 @@ macro_rules! byte_array_type { impl From<&$name> for String { fn from(value: &$name) -> Self { - let mut s = String::new(); - for n in 0..($size / 8) { - let b: [u8; 8] = value.bytes[n * 8..(n + 1) * 8].try_into().unwrap(); - s.push_str(hex::encode(b).as_str()); - } - s + // let mut s = String::new(); + // for n in 0..($size / 8) { + // let b: [u8; 8] = value.bytes[n * 8..(n + 1) * 8].try_into().unwrap(); + // s.push_str(hex::encode(b).as_str()); + // } + // s + value.encode() } } @@ -208,17 +210,18 @@ macro_rules! byte_array_type { impl TryFrom<&str> for $name { type Error = VeilidAPIError; fn try_from(value: &str) -> Result { - let mut out = $name::default(); - if value == "" { - return Ok(out); - } - if value.len() != ($size * 2) { - apibail_generic!(concat!(stringify!($name), " is incorrect length")); - } - match hex::decode_to_slice(value, &mut out.bytes) { - Ok(_) => Ok(out), - Err(err) => Err(VeilidAPIError::generic(err)), - } + // let mut out = $name::default(); + // if value == "" { + // return Ok(out); + // } + // if value.len() != ($size * 2) { + // apibail_generic!(concat!(stringify!($name), " is incorrect length")); + // } + // match hex::decode_to_slice(value, &mut out.bytes) { + // Ok(_) => Ok(out), + // Err(err) => Err(VeilidAPIError::generic(err)), + // } + Self::try_decode(value) } } }; diff --git a/veilid-core/src/crypto/tests/test_dht_key.rs b/veilid-core/src/crypto/tests/test_dht_key.rs index 3b6ded61..d6876ac3 100644 --- a/veilid-core/src/crypto/tests/test_dht_key.rs +++ b/veilid-core/src/crypto/tests/test_dht_key.rs @@ -138,11 +138,11 @@ pub async fn test_key_conversions() { // Assert string roundtrip assert_eq!(String::from(&dht_key2_back), dht_key2_string); - assert!(key::DHTKey::try_from("") == Ok(key::DHTKey::default())); - assert!(key::DHTKeySecret::try_from("") == Ok(key::DHTKeySecret::default())); // These conversions should fail assert!(key::DHTKey::try_from("whatever").is_err()); assert!(key::DHTKeySecret::try_from("whatever").is_err()); + assert!(key::DHTKey::try_from("").is_err()); + assert!(key::DHTKeySecret::try_from("").is_err()); assert!(key::DHTKey::try_from(" ").is_err()); assert!(key::DHTKeySecret::try_from(" ").is_err()); assert!(key::DHTKey::try_from( diff --git a/veilid-core/src/routing_table/route_spec_store.rs b/veilid-core/src/routing_table/route_spec_store.rs index 8d1c0cf3..bf0fc2db 100644 --- a/veilid-core/src/routing_table/route_spec_store.rs +++ b/veilid-core/src/routing_table/route_spec_store.rs @@ -456,6 +456,11 @@ impl RouteSpecStore { bail!("Not allocating route longer than max route hop count"); } + // Get relay node id if we have one + let opt_relay_id = rti + .relay_node(RoutingDomain::PublicInternet) + .map(|nr| nr.node_id()); + // Get list of all nodes, and sort them for selection let cur_ts = intf::get_timestamp(); let filter = Box::new( @@ -466,6 +471,13 @@ impl RouteSpecStore { } let v = v.unwrap(); + // Exclude our relay if we have one + if let Some(relay_id) = opt_relay_id { + if k == relay_id { + return false; + } + } + // Exclude nodes on our local network let on_local_network = v.with(rti, |_rti, e| { e.node_info(RoutingDomain::LocalNetwork).is_some() @@ -714,7 +726,8 @@ impl RouteSpecStore { return Ok(None); } // Validate signatures to ensure the route was handled by the nodes and not messed with - for (hop_n, hop_public_key) in rsd.hops.iter().enumerate() { + // This is in private route (reverse) order as we are receiving over the route + for (hop_n, hop_public_key) in rsd.hops.iter().rev().enumerate() { // The last hop is not signed, as the whole packet is signed if hop_n == signatures.len() { // Verify the node we received the routed operation from is the last hop in our route @@ -843,7 +856,7 @@ impl RouteSpecStore { pub fn compile_safety_route( &self, safety_selection: SafetySelection, - private_route: PrivateRoute, + mut private_route: PrivateRoute, ) -> EyreResult> { let inner = &mut *self.inner.lock(); let routing_table = self.unlocked_inner.routing_table.clone(); @@ -854,15 +867,17 @@ impl RouteSpecStore { if pr_hopcount > max_route_hop_count { bail!("private route hop count too long"); } - let PrivateRouteHops::FirstHop(pr_first_hop) = &private_route.hops else { - bail!("compiled private route should have first hop"); - }; - // See if we are using a safety route, if not, short circuit this operation let safety_spec = match safety_selection { + // Safety route spec to use + SafetySelection::Safe(safety_spec) => safety_spec, + // Safety route stub with the node's public key as the safety route key since it's the 0th hop SafetySelection::Unsafe(sequencing) => { - // Safety route stub with the node's public key as the safety route key since it's the 0th hop - let opt_first_hop = match &pr_first_hop.node { + let Some(pr_first_hop_node) = private_route.pop_first_hop() else { + bail!("compiled private route should have first hop"); + }; + + let opt_first_hop = match pr_first_hop_node { RouteNode::NodeId(id) => rti.lookup_node_ref(routing_table.clone(), id.key), RouteNode::PeerInfo(pi) => rti.register_node_with_signed_node_info( routing_table.clone(), @@ -889,7 +904,10 @@ impl RouteSpecStore { first_hop, })); } - SafetySelection::Safe(safety_spec) => safety_spec, + }; + + let PrivateRouteHops::FirstHop(pr_first_hop) = &private_route.hops else { + bail!("compiled private route should have first hop"); }; // Get the safety route to use from the spec @@ -930,6 +948,7 @@ impl RouteSpecStore { // Each loop mutates 'nonce', and 'blob_data' let mut nonce = Crypto::get_random_nonce(); let crypto = routing_table.network_manager().crypto(); + // Forward order (safety route), but inside-out for h in (1..safety_rsd.hops.len()).rev() { // Get blob to encrypt for next hop blob_data = { @@ -1132,7 +1151,8 @@ impl RouteSpecStore { let crypto = routing_table.network_manager().crypto(); // Loop for each hop let hop_count = rsd.hops.len(); - for h in (0..hop_count).rev() { + // iterate hops in private route order (reverse, but inside out) + for h in 0..hop_count { let nonce = Crypto::get_random_nonce(); let blob_data = { @@ -1178,7 +1198,8 @@ impl RouteSpecStore { let private_route = PrivateRoute { public_key: key.clone(), - hop_count: hop_count.try_into().unwrap(), + // add hop for 'FirstHop' + hop_count: (hop_count + 1).try_into().unwrap(), hops: PrivateRouteHops::FirstHop(route_hop), }; Ok(private_route) diff --git a/veilid-core/src/rpc_processor/rpc_route.rs b/veilid-core/src/rpc_processor/rpc_route.rs index 4267d43d..ac2513db 100644 --- a/veilid-core/src/rpc_processor/rpc_route.rs +++ b/veilid-core/src/rpc_processor/rpc_route.rs @@ -110,16 +110,6 @@ impl RPCProcessor { } }?; - if !matches!(next_private_route.hops, PrivateRouteHops::Empty) { - // Sign the operation if this is not our last hop - // as the last hop is already signed by the envelope - let node_id = self.routing_table.node_id(); - let node_id_secret = self.routing_table.node_id_secret(); - let sig = sign(&node_id, &node_id_secret, &routed_operation.data) - .map_err(RPCError::internal)?; - routed_operation.signatures.push(sig); - } - // Pass along the route let next_hop_route = RPCOperationRoute { safety_route: SafetyRoute { @@ -313,7 +303,7 @@ impl RPCProcessor { }; // Get the statement - let route = match msg.operation.into_kind() { + let mut route = match msg.operation.into_kind() { RPCOperationKind::Statement(s) => match s.into_detail() { RPCStatementDetail::Route(s) => s, _ => panic!("not a route statement"), @@ -333,25 +323,24 @@ impl RPCProcessor { match route.safety_route.hops { // There is a safety route hop SafetyRouteHops::Data(ref d) => { - // See if this is last hop in safety route, if so, we're decoding a PrivateRoute not a RouteHop - let (blob_tag, blob_data) = if let Some(b) = d.blob.last() { - (*b, &d.blob[0..d.blob.len() - 1]) - } else { - return Err(RPCError::protocol("no bytes in blob")); - }; - // Decrypt the blob with DEC(nonce, DH(the SR's public key, this hop's secret) let node_id_secret = self.routing_table.node_id_secret(); let dh_secret = self .crypto .cached_dh(&route.safety_route.public_key, &node_id_secret) .map_err(RPCError::protocol)?; - let dec_blob_data = Crypto::decrypt_aead(blob_data, &d.nonce, &dh_secret, None) + let mut dec_blob_data = Crypto::decrypt_aead(&d.blob, &d.nonce, &dh_secret, None) .map_err(RPCError::protocol)?; + + // See if this is last hop in safety route, if so, we're decoding a PrivateRoute not a RouteHop + let Some(dec_blob_tag) = dec_blob_data.pop() else { + return Err(RPCError::protocol("no bytes in blob")); + }; + let dec_blob_reader = RPCMessageData::new(dec_blob_data).get_reader()?; // Decode the blob appropriately - if blob_tag == 1 { + if dec_blob_tag == 1 { // PrivateRoute let private_route = { let pr_reader = dec_blob_reader @@ -367,7 +356,7 @@ impl RPCProcessor { &private_route, ) .await?; - } else if blob_tag == 0 { + } else if dec_blob_tag == 0 { // RouteHop let route_hop = { let rh_reader = dec_blob_reader @@ -425,6 +414,16 @@ impl RPCProcessor { return Err(RPCError::protocol("route should not be at the end")); } + // Sign the operation if this is not our last hop + // as the last hop is already signed by the envelope + if route_hop.next_hop.is_some() { + let node_id = self.routing_table.node_id(); + let node_id_secret = self.routing_table.node_id_secret(); + let sig = sign(&node_id, &node_id_secret, &route.operation.data) + .map_err(RPCError::internal)?; + route.operation.signatures.push(sig); + } + // Make next PrivateRoute and pass it on self.process_route_private_route_hop( route.operation, diff --git a/veilid-core/src/veilid_api/privacy.rs b/veilid-core/src/veilid_api/privacy.rs index f3cac31c..352b716b 100644 --- a/veilid-core/src/veilid_api/privacy.rs +++ b/veilid-core/src/veilid_api/privacy.rs @@ -82,6 +82,32 @@ impl PrivateRoute { }), } } + + /// Remove the first unencrypted hop if possible + pub fn pop_first_hop(&mut self) -> Option { + match &mut self.hops { + PrivateRouteHops::FirstHop(first_hop) => { + let first_hop_node = first_hop.node.clone(); + + // Reduce hop count + if self.hop_count > 0 { + self.hop_count -= 1; + } else { + error!("hop count should not be 0 for first hop"); + } + + // Go to next hop + self.hops = match first_hop.next_hop.take() { + Some(rhd) => PrivateRouteHops::Data(rhd), + None => PrivateRouteHops::Empty, + }; + + return Some(first_hop_node); + } + PrivateRouteHops::Data(_) => return None, + PrivateRouteHops::Empty => return None, + } + } } impl fmt::Display for PrivateRoute { @@ -123,6 +149,7 @@ pub struct SafetyRoute { impl SafetyRoute { pub fn new_stub(public_key: DHTKey, private_route: PrivateRoute) -> Self { + assert!(matches!(private_route.hops, PrivateRouteHops::Data(_))); Self { public_key, hop_count: 0,