From 9c82aa2b29883d8e70f8247180c72ee0b7f98b78 Mon Sep 17 00:00:00 2001 From: Adam Ierymenko Date: Tue, 3 Jan 2023 10:18:47 -0500 Subject: [PATCH] More trait simplification. --- controller/src/controller.rs | 36 ++++++++-------- controller/src/filedatabase.rs | 3 +- controller/src/main.rs | 63 +++++++++------------------- network-hypervisor/src/vl1/mod.rs | 2 +- network-hypervisor/src/vl1/node.rs | 40 +++++++++--------- network-hypervisor/src/vl1/peer.rs | 6 +-- network-hypervisor/src/vl2/switch.rs | 8 ++++ service/src/main.rs | 2 +- vl1-service/src/vl1service.rs | 14 +------ 9 files changed, 72 insertions(+), 102 deletions(-) diff --git a/controller/src/controller.rs b/controller/src/controller.rs index 679be5b39..7f0bbb91d 100644 --- a/controller/src/controller.rs +++ b/controller/src/controller.rs @@ -498,6 +498,22 @@ impl Controller { } impl InnerProtocolLayer for Controller { + #[inline(always)] + fn should_respond_to(&self, _: &Verified) -> bool { + // Controllers always have to establish sessions to process requests. We don't really know if + // a member is relevant until we have looked up both the network and the member, since whether + // or not to "learn" unknown members is a network level option. + true + } + + fn has_trust_relationship(&self, id: &Verified) -> bool { + self.recently_authorized + .read() + .unwrap() + .get(&id.fingerprint) + .map_or(false, |by_network| by_network.values().any(|t| *t > ms_monotonic())) + } + fn handle_packet( &self, host_system: &HostSystemImpl, @@ -512,7 +528,7 @@ impl InnerProtocolLayer for Controller { ) -> PacketHandlerResult { match verb { protocol::message_type::VL2_NETWORK_CONFIG_REQUEST => { - if !host_system.peer_filter().should_respond_to(&source.identity) { + if !self.should_respond_to(&source.identity) { return PacketHandlerResult::Ok; // handled and ignored } @@ -638,24 +654,6 @@ impl InnerProtocolLayer for Controller { } } -impl PeerFilter for Controller { - #[inline(always)] - fn should_respond_to(&self, _: &Verified) -> bool { - // Controllers always have to establish sessions to process requests. We don't really know if - // a member is relevant until we have looked up both the network and the member, since whether - // or not to "learn" unknown members is a network level option. - true - } - - fn has_trust_relationship(&self, id: &Verified) -> bool { - self.recently_authorized - .read() - .unwrap() - .get(&id.fingerprint) - .map_or(false, |by_network| by_network.values().any(|t| *t > ms_monotonic())) - } -} - impl Drop for Controller { fn drop(&mut self) { for h in self.daemons.lock().unwrap().drain(..) { diff --git a/controller/src/filedatabase.rs b/controller/src/filedatabase.rs index 396995625..2850064ba 100644 --- a/controller/src/filedatabase.rs +++ b/controller/src/filedatabase.rs @@ -20,8 +20,7 @@ use crate::cache::Cache; use crate::database::{Change, Database, Error}; use crate::model::*; -const IDENTITY_SECRET_FILENAME: &'static str = "identity.secret"; -const EVENT_HANDLER_TASK_TIMEOUT: Duration = Duration::from_secs(5); +const EVENT_HANDLER_TASK_TIMEOUT: Duration = Duration::from_secs(10); /// An in-filesystem database that permits live editing. /// diff --git a/controller/src/main.rs b/controller/src/main.rs index 570bb3e26..6c9d29f3c 100644 --- a/controller/src/main.rs +++ b/controller/src/main.rs @@ -2,53 +2,41 @@ use std::sync::Arc; -use clap::{Arg, Command}; - use zerotier_network_controller::database::Database; use zerotier_network_controller::filedatabase::FileDatabase; use zerotier_network_controller::Controller; -use zerotier_network_hypervisor::vl1::PeerFilter; use zerotier_network_hypervisor::{VERSION_MAJOR, VERSION_MINOR, VERSION_REVISION}; use zerotier_utils::exitcode; use zerotier_utils::tokio::runtime::Runtime; use zerotier_vl1_service::VL1Service; async fn run(database: Arc, runtime: &Runtime) -> i32 { - let handler = Controller::new(database.clone(), runtime.handle().clone()).await; - if handler.is_err() { - eprintln!("FATAL: error initializing handler: {}", handler.err().unwrap().to_string()); - exitcode::ERR_CONFIG - } else { - let handler = handler.unwrap(); - - let svc = VL1Service::new( - Arc::new(AdmitAllPeerFilter), - database.clone(), - handler.clone(), - zerotier_vl1_service::VL1Settings::default(), - ); - - if svc.is_ok() { - let svc = svc.unwrap(); - svc.node().init_default_roots(); - - handler.start(&svc).await; - - zerotier_utils::wait_for_process_abort(); - println!("Terminate signal received, shutting down..."); - exitcode::OK - } else { - eprintln!("FATAL: error launching service: {}", svc.err().unwrap().to_string()); - exitcode::ERR_IOERR + match Controller::new(database.clone(), runtime.handle().clone()).await { + Err(err) => { + eprintln!("FATAL: error initializing handler: {}", err.to_string()); + exitcode::ERR_CONFIG } + Ok(handler) => match VL1Service::new(database.clone(), handler.clone(), zerotier_vl1_service::VL1Settings::default()) { + Err(err) => { + eprintln!("FATAL: error launching service: {}", err.to_string()); + exitcode::ERR_IOERR + } + Ok(svc) => { + svc.node().init_default_roots(); + handler.start(&svc).await; + zerotier_utils::wait_for_process_abort(); + println!("Terminate signal received, shutting down..."); + exitcode::OK + } + }, } } fn main() { const REQUIRE_ONE_OF_ARGS: [&'static str; 2] = ["postgres", "filedb"]; - let global_args = Command::new("zerotier-controller") + let global_args = clap::Command::new("zerotier-controller") .arg( - Arg::new("filedb") + clap::Arg::new("filedb") .short('f') .long("filedb") .takes_value(true) @@ -58,7 +46,7 @@ fn main() { .required_unless_present_any(&REQUIRE_ONE_OF_ARGS), ) .arg( - Arg::new("postgres") + clap::Arg::new("postgres") .short('p') .long("postgres") .takes_value(true) @@ -98,14 +86,3 @@ fn main() { std::process::exit(exitcode::ERR_IOERR) } } - -struct AdmitAllPeerFilter; -impl PeerFilter for AdmitAllPeerFilter { - fn should_respond_to(&self, id: &zerotier_crypto::verified::Verified) -> bool { - true - } - - fn has_trust_relationship(&self, id: &zerotier_crypto::verified::Verified) -> bool { - true - } -} diff --git a/network-hypervisor/src/vl1/mod.rs b/network-hypervisor/src/vl1/mod.rs index 98c7b6d74..4135866c5 100644 --- a/network-hypervisor/src/vl1/mod.rs +++ b/network-hypervisor/src/vl1/mod.rs @@ -18,7 +18,7 @@ pub use event::Event; pub use identity::Identity; pub use inetaddress::InetAddress; pub use mac::MAC; -pub use node::{ApplicationLayer, InnerProtocolLayer, Node, PacketHandlerResult, PeerFilter}; +pub use node::{ApplicationLayer, InnerProtocolLayer, Node, PacketHandlerResult}; pub use path::Path; pub use peer::Peer; pub use rootset::{Root, RootSet}; diff --git a/network-hypervisor/src/vl1/node.rs b/network-hypervisor/src/vl1/node.rs index 797bc120a..ca08e770a 100644 --- a/network-hypervisor/src/vl1/node.rs +++ b/network-hypervisor/src/vl1/node.rs @@ -47,9 +47,6 @@ pub trait ApplicationLayer: Sync + Send { /// Save this node's identity to the data store, returning true on success. fn save_node_identity(&self, id: &Verified) -> bool; - /// Get the PeerFilter implementation used to check whether this node should communicate at VL1 with other peers. - fn peer_filter(&self) -> &dyn PeerFilter; - /// Get a pooled packet buffer for internal use. fn get_buffer(&self) -> PooledPacketBuffer; @@ -115,22 +112,6 @@ pub trait ApplicationLayer: Sync + Send { fn time_clock(&self) -> i64; } -/// Trait providing functions to determine what peers we should talk to. -pub trait PeerFilter: Sync + Send { - /// Check if this node should respond to messages from a given peer at all. - /// - /// If this returns false, the node simply drops messages on the floor and refuses - /// to init V2 sessions. - fn should_respond_to(&self, id: &Verified) -> bool; - - /// Check if this node has any trust relationship with the provided identity. - /// - /// This should return true if there is any special trust relationship such as mutual - /// membership in a network or for controllers the peer's membership in any network - /// they control. - fn has_trust_relationship(&self, id: &Verified) -> bool; -} - /// Result of a packet handler. pub enum PacketHandlerResult { /// Packet was handled successfully. @@ -149,6 +130,24 @@ pub enum PacketHandlerResult { /// it could also be implemented for testing or "off label" use of VL1 to carry different protocols. #[allow(unused)] pub trait InnerProtocolLayer: Sync + Send { + /// Check if this node should respond to messages from a given peer at all. + /// + /// The default implementation always returns true. + fn should_respond_to(&self, id: &Verified) -> bool { + true + } + + /// Check if this node has any trust relationship with the provided identity. + /// + /// This should return true if there is any special trust relationship. It controls things + /// like sharing of detailed P2P connectivity data, which should be limited to peers with + /// some privileged relationship like mutual membership in a network. + /// + /// The default implementation always returns true. + fn has_trust_relationship(&self, id: &Verified) -> bool { + true + } + /// Handle a packet, returning true if it was handled by the next layer. /// /// Do not attempt to handle OK or ERROR. Instead implement handle_ok() and handle_error(). @@ -980,7 +979,7 @@ impl Node { let mut whois_queue = self.whois_queue.lock().unwrap(); if let Some(qi) = whois_queue.get_mut(&received_identity.address) { let address = received_identity.address; - if app.peer_filter().should_respond_to(&received_identity) { + if inner.should_respond_to(&received_identity) { let mut peers = self.peers.write().unwrap(); if let Some(peer) = peers.get(&address).cloned().or_else(|| { Peer::new(&self.identity, received_identity, time_ticks) @@ -1007,6 +1006,7 @@ impl Node { /// /// This will only replace an existing root set with a newer one. It won't add a new root set, which must be /// done by an authorized user or administrator not just by a root. + #[allow(unused)] pub(crate) fn on_remote_update_root_set(&self, received_from: &Identity, rs: Verified) { let mut roots = self.roots.write().unwrap(); if let Some(entry) = roots.sets.get_mut(&rs.name) { diff --git a/network-hypervisor/src/vl1/peer.rs b/network-hypervisor/src/vl1/peer.rs index fd09965f2..32e912d0b 100644 --- a/network-hypervisor/src/vl1/peer.rs +++ b/network-hypervisor/src/vl1/peer.rs @@ -593,7 +593,7 @@ impl Peer { source_path: &Arc, payload: &PacketBuffer, ) -> PacketHandlerResult { - if !(app.peer_filter().should_respond_to(&self.identity) || node.this_node_is_root() || node.is_peer_root(self)) { + if !(inner.should_respond_to(&self.identity) || node.this_node_is_root() || node.is_peer_root(self)) { debug_event!( app, "[vl1] dropping HELLO from {} due to lack of trust relationship", @@ -787,7 +787,7 @@ impl Peer { message_id: MessageId, payload: &PacketBuffer, ) -> PacketHandlerResult { - if node.this_node_is_root() || app.peer_filter().should_respond_to(&self.identity) { + if node.this_node_is_root() || inner.should_respond_to(&self.identity) { let mut addresses = payload.as_bytes(); while addresses.len() >= ADDRESS_SIZE { if !self @@ -833,7 +833,7 @@ impl Peer { message_id: MessageId, payload: &PacketBuffer, ) -> PacketHandlerResult { - if app.peer_filter().should_respond_to(&self.identity) || node.is_peer_root(self) { + if inner.should_respond_to(&self.identity) || node.is_peer_root(self) { self.send(app, node, None, time_ticks, |packet| { let mut f: &mut OkHeader = packet.append_struct_get_mut().unwrap(); f.verb = message_type::VL1_OK; diff --git a/network-hypervisor/src/vl2/switch.rs b/network-hypervisor/src/vl2/switch.rs index 3e2a3f32f..6f103ddaa 100644 --- a/network-hypervisor/src/vl2/switch.rs +++ b/network-hypervisor/src/vl2/switch.rs @@ -11,6 +11,14 @@ pub struct Switch {} #[allow(unused_variables)] impl InnerProtocolLayer for Switch { + fn should_respond_to(&self, id: &zerotier_crypto::verified::Verified) -> bool { + true + } + + fn has_trust_relationship(&self, id: &zerotier_crypto::verified::Verified) -> bool { + true + } + fn handle_packet( &self, app: &Application, diff --git a/service/src/main.rs b/service/src/main.rs index c5817dee9..52fe57242 100644 --- a/service/src/main.rs +++ b/service/src/main.rs @@ -212,7 +212,7 @@ fn main() { if let Ok(_tokio_runtime) = zerotier_utils::tokio::runtime::Builder::new_multi_thread().enable_all().build() { let test_inner = Arc::new(DummyInnerLayer); let datadir = open_datadir(&flags); - let svc = VL1Service::new(todo!(), datadir, test_inner, zerotier_vl1_service::VL1Settings::default()); + let svc = VL1Service::new(datadir, test_inner, zerotier_vl1_service::VL1Settings::default()); if svc.is_ok() { let svc = svc.unwrap(); svc.node().init_default_roots(); diff --git a/vl1-service/src/vl1service.rs b/vl1-service/src/vl1service.rs index e280bfe40..24262de91 100644 --- a/vl1-service/src/vl1service.rs +++ b/vl1-service/src/vl1service.rs @@ -34,7 +34,6 @@ pub trait VL1DataStorage: Sync + Send { /// a test harness or just the controller for a controller that runs stand-alone. pub struct VL1Service { state: RwLock, - peer_filter: Arc, vl1_data_storage: Arc, inner: Arc, buffer_pool: Arc, @@ -49,12 +48,7 @@ struct VL1ServiceMutableState { } impl VL1Service { - pub fn new( - peer_filter: Arc, - vl1_data_storage: Arc, - inner: Arc, - settings: VL1Settings, - ) -> Result, Box> { + pub fn new(vl1_data_storage: Arc, inner: Arc, settings: VL1Settings) -> Result, Box> { let mut service = Self { state: RwLock::new(VL1ServiceMutableState { daemons: Vec::with_capacity(2), @@ -62,7 +56,6 @@ impl VL1Service { settings, running: true, }), - peer_filter, vl1_data_storage, inner, buffer_pool: Arc::new(PacketBufferPool::new( @@ -233,11 +226,6 @@ impl ApplicationLayer for VL1Servi self.vl1_data_storage.save_node_identity(id) } - #[inline(always)] - fn peer_filter(&self) -> &dyn PeerFilter { - self.peer_filter.as_ref() - } - #[inline] fn get_buffer(&self) -> zerotier_network_hypervisor::protocol::PooledPacketBuffer { self.buffer_pool.get()