diff --git a/CHANGELOG.md b/CHANGELOG.md index f6ae0ab7..83530aae 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,9 @@ **UNRELEASED** -- Add private route example +- veilid-core: + - Add private route example + - Add `require_inbound_relay` option in VeilidConfig. Default is false, but if enabled, forces OutboundOnly/InboundRelay mode. Can be used as an extra layer of IP address obscurity for some threat models. (@neequ57) + - Fix crash when peer info has missing or unsupported node ids **Changed in Veilid 0.4.7** diff --git a/veilid-core/src/network_manager/bootstrap/direct_bootstrap/mod.rs b/veilid-core/src/network_manager/bootstrap/direct_bootstrap/mod.rs index bf1862be..e6919a0e 100644 --- a/veilid-core/src/network_manager/bootstrap/direct_bootstrap/mod.rs +++ b/veilid-core/src/network_manager/bootstrap/direct_bootstrap/mod.rs @@ -135,12 +135,19 @@ impl NetworkManager { bsrec.dial_info_details().to_vec(), // Dial info is as specified in the bootstrap list ), )); - - Some(Arc::new(PeerInfo::new( + let bspi = match PeerInfo::new( RoutingDomain::PublicInternet, bsrec.node_ids().clone(), sni, - ))) + ) { + Ok(v) => v, + Err(e) => { + veilid_log!(self error "Bootstrap has invalid peer info: {}", e); + return None; + } + }; + + Some(Arc::new(bspi)) } }) .collect(); diff --git a/veilid-core/src/network_manager/bootstrap/txt_bootstrap/mod.rs b/veilid-core/src/network_manager/bootstrap/txt_bootstrap/mod.rs index dc92b6cb..fe295b1a 100644 --- a/veilid-core/src/network_manager/bootstrap/txt_bootstrap/mod.rs +++ b/veilid-core/src/network_manager/bootstrap/txt_bootstrap/mod.rs @@ -144,11 +144,19 @@ impl NetworkManager { ), )); - Some(Arc::new(PeerInfo::new( + let bspi = match PeerInfo::new( RoutingDomain::PublicInternet, bsrec.node_ids().clone(), sni, - ))) + ) { + Ok(v) => v, + Err(e) => { + veilid_log!(self error "Bootstrap has invalid peer info: {}", e); + return None; + } + }; + + Some(Arc::new(bspi)) } }) .collect(); diff --git a/veilid-core/src/routing_table/bucket_entry.rs b/veilid-core/src/routing_table/bucket_entry.rs index c84b7597..2a762f01 100644 --- a/veilid-core/src/routing_table/bucket_entry.rs +++ b/veilid-core/src/routing_table/bucket_entry.rs @@ -588,9 +588,13 @@ impl BucketEntryInner { }; // Peer info includes all node ids, even unvalidated ones let node_ids = self.node_ids(); - let opt_pi = opt_current_sni - .as_ref() - .map(|s| Arc::new(PeerInfo::new(routing_domain, node_ids, *s.clone()))); + let opt_pi = match opt_current_sni { + Some(s) => match PeerInfo::new(routing_domain, node_ids, *s.clone()) { + Ok(v) => Some(Arc::new(v)), + Err(_) => None, + }, + None => None, + }; // Cache the peerinfo pi_cache.insert(routing_domain, opt_pi.clone()); diff --git a/veilid-core/src/routing_table/routing_table_inner/mod.rs b/veilid-core/src/routing_table/routing_table_inner/mod.rs index 7ef04512..ac516a80 100644 --- a/veilid-core/src/routing_table/routing_table_inner/mod.rs +++ b/veilid-core/src/routing_table/routing_table_inner/mod.rs @@ -752,6 +752,9 @@ impl RoutingTableInner { F: FnOnce(&mut RoutingTableInner, &mut BucketEntryInner), { let routing_table = self.routing_table(); + if node_ids.is_empty() { + bail!("Can't create node with no node id"); + } // Ensure someone isn't trying register this node itself if routing_table.matches_own_node_id(node_ids) { @@ -761,11 +764,14 @@ impl RoutingTableInner { // Look up all bucket entries and make sure we only have zero or one // If we have more than one, pick the one with the best cryptokind to add node ids to let mut best_entry: Option> = None; + let mut supported_node_ids = TypedNodeIdGroup::new(); for node_id in node_ids.iter() { // Ignore node ids we don't support if !VALID_CRYPTO_KINDS.contains(&node_id.kind) { continue; } + supported_node_ids.add(*node_id); + // Find the first in crypto sort order let bucket_index = routing_table.calculate_bucket_index(node_id); let bucket = self.get_bucket(bucket_index); @@ -798,8 +804,13 @@ impl RoutingTableInner { return Ok(nr); } + // Fail out if we can't handle this node + if supported_node_ids.is_empty() { + bail!("Not registering node with no supported node ids"); + } + // If no entry exists yet, add the first entry to a bucket, possibly evicting a bucket member - let first_node_id = node_ids[0]; + let first_node_id = supported_node_ids[0]; let bucket_entry = routing_table.calculate_bucket_index(&first_node_id); let bucket = self.get_bucket_mut(bucket_entry); let new_entry = bucket.add_new_entry(first_node_id.value); diff --git a/veilid-core/src/routing_table/routing_table_inner/routing_domains/mod.rs b/veilid-core/src/routing_table/routing_table_inner/routing_domains/mod.rs index 8f40ccb7..99545c5b 100644 --- a/veilid-core/src/routing_table/routing_table_inner/routing_domains/mod.rs +++ b/veilid-core/src/routing_table/routing_table_inner/routing_domains/mod.rs @@ -419,6 +419,7 @@ impl RoutingDomainDetailCommon { routing_table.node_ids(), signed_node_info, ) + .expect("our own peerinfo should never fail") } fn clear_cache(&self) { diff --git a/veilid-core/src/routing_table/tests/test_serialize_routing_table.rs b/veilid-core/src/routing_table/tests/test_serialize_routing_table.rs index ba1a4d36..cc881273 100644 --- a/veilid-core/src/routing_table/tests/test_serialize_routing_table.rs +++ b/veilid-core/src/routing_table/tests/test_serialize_routing_table.rs @@ -1,74 +1,31 @@ use super::*; use crate::{routing_table::*, RegisteredComponents}; -pub async fn test_routingtable_buckets_round_trip() { - let original_registry = mock_registry::init("a").await; - let copy_registry = mock_registry::init("b").await; - - // Wrap to close lifetime of 'inner' which is borrowed here so terminate() can succeed - // (it also .write() locks routing table inner) - { - let original = original_registry.routing_table(); - let copy = copy_registry.routing_table(); - - // Add lots of routes to `original` here to exercise all various types. - - let (serialized_bucket_map, all_entry_bytes) = original.serialized_buckets(); - - RoutingTable::populate_routing_table_inner( - &mut copy.inner.write(), - serialized_bucket_map, - all_entry_bytes, - ) - .unwrap(); - - let original_inner = &*original.inner.read(); - let copy_inner = &*copy.inner.read(); - - let routing_table_keys: Vec<_> = original_inner.buckets.keys().clone().collect(); - let copy_keys: Vec<_> = copy_inner.buckets.keys().clone().collect(); - - assert_eq!(routing_table_keys.len(), copy_keys.len()); - - for crypto in routing_table_keys { - // The same keys are present in the original and copy RoutingTables. - let original_buckets = original_inner.buckets.get(crypto).unwrap(); - let copy_buckets = copy_inner.buckets.get(crypto).unwrap(); - - // Recurse into RoutingTable.inner.buckets - for (left_buckets, right_buckets) in original_buckets.iter().zip(copy_buckets.iter()) { - // Recurse into RoutingTable.inner.buckets.entries - for ((left_crypto, left_entries), (right_crypto, right_entries)) in - left_buckets.entries().zip(right_buckets.entries()) - { - assert_eq!(left_crypto, right_crypto); - - assert_eq!( - format!("{:?}", left_entries), - format!("{:?}", right_entries) - ); - } - } - } - } - - // Even if these are mocks, we should still practice good hygiene. - mock_registry::terminate(original_registry).await; - mock_registry::terminate(copy_registry).await; +fn make_mock_typed_node_id(kind: CryptoKind, idx: u8) -> TypedNodeId { + TypedNodeId::new( + kind, + NodeId::new([ + idx, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, + 0, 0, 0, 0, + ]), + ) } -pub fn test_round_trip_peerinfo() { - let mut tks = TypedPublicKeyGroup::new(); - tks.add(TypedPublicKey::new( - CRYPTO_KIND_VLD0, - PublicKey::new([ - 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, - 0, 0, 0, - ]), - )); - let pi: PeerInfo = PeerInfo::new( +fn make_mock_typed_node_id_group(vld0: bool, unknown: bool) -> TypedNodeIdGroup { + let mut tks = TypedNodeIdGroup::new(); + if vld0 { + tks.add(make_mock_typed_node_id(CRYPTO_KIND_VLD0, 0)); + } + if unknown { + tks.add(make_mock_typed_node_id(CryptoKind([1, 2, 3, 4]), 0)); + } + tks +} + +fn make_mock_peer_info(node_ids: TypedNodeIdGroup) -> EyreResult { + PeerInfo::new( RoutingDomain::PublicInternet, - tks.into(), + node_ids, SignedNodeInfo::Direct(SignedDirectNodeInfo::new( NodeInfo::new( NetworkClass::OutboundOnly, @@ -82,7 +39,91 @@ pub fn test_round_trip_peerinfo() { Timestamp::new(0), Vec::new(), )), - ); + ) +} + +fn add_mock_data(routing_table: &VeilidComponentGuard<'_, RoutingTable>) { + let pi = + make_mock_peer_info(make_mock_typed_node_id_group(true, false)).expect("should be valid"); + routing_table + .register_node_with_peer_info(Arc::new(pi), true) + .expect("should register"); + let pi2 = + make_mock_peer_info(make_mock_typed_node_id_group(true, true)).expect("should be valid"); + routing_table + .register_node_with_peer_info(Arc::new(pi2), true) + .expect("should register"); + + let _ = make_mock_peer_info(make_mock_typed_node_id_group(false, false)) + .expect_err("should fail with no node ids"); + + let pi3 = + make_mock_peer_info(make_mock_typed_node_id_group(false, true)).expect("should be valid"); + + let _ = routing_table + .register_node_with_peer_info(Arc::new(pi3), true) + .expect_err("should fail with only unsupported node ids"); +} +pub async fn test_routingtable_buckets_round_trip() { + let original_registry = mock_registry::init("a").await; + let copy_registry = mock_registry::init("b").await; + + // Wrap to close lifetime of 'inner' which is borrowed here so terminate() can succeed + // (it also .write() locks routing table inner) + { + let original = original_registry.routing_table(); + let copy = copy_registry.routing_table(); + + add_mock_data(&original); + + let (serialized_bucket_map, all_entry_bytes) = original.serialized_buckets(); + + RoutingTable::populate_routing_table_inner( + &mut copy.inner.write(), + serialized_bucket_map, + all_entry_bytes, + ) + .unwrap(); + + let original_inner = &*original.inner.read(); + let copy_inner = &*copy.inner.read(); + + let original_crypto_kinds: Vec<_> = original_inner.buckets.keys().clone().collect(); + let copy_crypto_kinds: Vec<_> = copy_inner.buckets.keys().clone().collect(); + + assert_eq!(original_crypto_kinds.len(), copy_crypto_kinds.len()); + + for crypto in original_crypto_kinds { + // The same keys are present in the original and copy RoutingTables. + let original_buckets = original_inner.buckets.get(crypto).unwrap(); + let copy_buckets = copy_inner.buckets.get(crypto).unwrap(); + + // Recurse into RoutingTable.inner.buckets + for (left_bucket, right_bucket) in original_buckets.iter().zip(copy_buckets.iter()) { + // Recurse into RoutingTable.inner.buckets.entries + for ((left_node_id, left_entry), (right_node_id, right_entry)) in + left_bucket.entries().zip(right_bucket.entries()) + { + assert_eq!(left_node_id, right_node_id); + + let s = left_entry.with(original_inner, |_rti, e| serialize_json(e)); + let s2 = right_entry.with(copy_inner, |_rti, e| serialize_json(e)); + + assert_eq!(s, s2); + } + } + } + } + + // Even if these are mocks, we should still practice good hygiene. + mock_registry::terminate(original_registry).await; + mock_registry::terminate(copy_registry).await; +} + +pub fn test_round_trip_peerinfo() { + let pi = + make_mock_peer_info(make_mock_typed_node_id_group(true, true)).expect("should be valid"); + let s = serialize_json(&pi); let pi2 = deserialize_json(&s).expect("Should deserialize"); let s2 = serialize_json(&pi2); diff --git a/veilid-core/src/routing_table/types/peer_info.rs b/veilid-core/src/routing_table/types/peer_info.rs index 411c7619..feea83f2 100644 --- a/veilid-core/src/routing_table/types/peer_info.rs +++ b/veilid-core/src/routing_table/types/peer_info.rs @@ -34,13 +34,26 @@ impl PeerInfo { routing_domain: RoutingDomain, node_ids: TypedNodeIdGroup, signed_node_info: SignedNodeInfo, - ) -> Self { - assert!(!node_ids.is_empty() && node_ids.len() <= MAX_CRYPTO_KINDS); - Self { + ) -> EyreResult { + if node_ids.is_empty() { + bail!( + "no node ids for peer info ({:?})\n{:#?}", + routing_domain, + signed_node_info + ); + } else if node_ids.len() > MAX_CRYPTO_KINDS { + bail!( + "too many node ids for peer info ({:?}): {:?}\n{:#?}", + routing_domain, + node_ids, + signed_node_info + ); + } + Ok(Self { routing_domain, node_ids, signed_node_info, - } + }) } pub fn validate(&self, crypto: &Crypto) -> VeilidAPIResult<()> { diff --git a/veilid-core/src/routing_table/types/signed_node_info.rs b/veilid-core/src/routing_table/types/signed_node_info.rs index abc5bc1c..a6ac3b8b 100644 --- a/veilid-core/src/routing_table/types/signed_node_info.rs +++ b/veilid-core/src/routing_table/types/signed_node_info.rs @@ -69,11 +69,14 @@ impl SignedNodeInfo { pub fn relay_peer_info(&self, routing_domain: RoutingDomain) -> Option> { match self { SignedNodeInfo::Direct(_) => None, - SignedNodeInfo::Relayed(r) => Some(Arc::new(PeerInfo::new( - routing_domain, - r.relay_ids().clone(), - SignedNodeInfo::Direct(r.relay_info().clone()), - ))), + SignedNodeInfo::Relayed(r) => Some(Arc::new( + PeerInfo::new( + routing_domain, + r.relay_ids().clone(), + SignedNodeInfo::Direct(r.relay_info().clone()), + ) + .unwrap(), // validate() above, should have ensured this succeeds + )), } } pub fn has_any_dial_info(&self) -> bool { diff --git a/veilid-core/src/rpc_processor/coders/peer_info.rs b/veilid-core/src/rpc_processor/coders/peer_info.rs index 85f5b17d..2081539c 100644 --- a/veilid-core/src/rpc_processor/coders/peer_info.rs +++ b/veilid-core/src/rpc_processor/coders/peer_info.rs @@ -47,9 +47,6 @@ pub fn decode_peer_info( if node_ids.is_empty() { return Err(RPCError::protocol("no verified node ids")); } - Ok(PeerInfo::new( - decode_context.routing_domain, - node_ids, - signed_node_info, - )) + PeerInfo::new(decode_context.routing_domain, node_ids, signed_node_info) + .map_err(RPCError::map_invalid_format("invalid peer info")) }