pr fixes

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();
+                // let mut s = String::new();
-                for n in 0..($size / 8) {
+                // for n in 0..($size / 8) {
-                    let b: [u8; 8] = value.bytes[n * 8..(n + 1) * 8].try_into().unwrap();
+                //     let b: [u8; 8] = value.bytes[n * 8..(n + 1) * 8].try_into().unwrap();
-                    s.push_str(hex::encode(b).as_str());
+                //     s.push_str(hex::encode(b).as_str());
-                }
+                // }
-                s
+                // 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<Self, Self::Error> {
-                let mut out = $name::default();
+                // let mut out = $name::default();
-                if value == "" {
+                // if value == "" {
-                    return Ok(out);
+                //     return Ok(out);
-                }
+                // }
-                if value.len() != ($size * 2) {
+                // if value.len() != ($size * 2) {
-                    apibail_generic!(concat!(stringify!($name), " is incorrect length"));
+                //     apibail_generic!(concat!(stringify!($name), " is incorrect length"));
-                }
+                // }
-                match hex::decode_to_slice(value, &mut out.bytes) {
+                // match hex::decode_to_slice(value, &mut out.bytes) {
-                    Ok(_) => Ok(out),
+                //     Ok(_) => Ok(out),
-                    Err(err) => Err(VeilidAPIError::generic(err)),
+                //     Err(err) => Err(VeilidAPIError::generic(err)),
-                }
+                // }
+                Self::try_decode(value)
             }
         }
     };

veilid-core/src/crypto/tests/test_dht_key.rs

@@ -138,13 +138,13 @@ 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(
         "qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqq"
     )

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<Option<CompiledRoute>> {
         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 {
+        // 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) => {
+                let Some(pr_first_hop_node) = private_route.pop_first_hop() else {
                     bail!("compiled private route should have first hop");
                 };
 
-        // See if we are using a safety route, if not, short circuit this operation
+                let opt_first_hop = match pr_first_hop_node {
-        let safety_spec = match safety_selection {
-            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 {
                     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)

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,

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<RouteNode> {
+        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,