More trait simplification.

controller/src/controller.rs

@@ -498,6 +498,22 @@ impl Controller {
 }
 
 impl InnerProtocolLayer for Controller {
+    #[inline(always)]
+    fn should_respond_to(&self, _: &Verified<Identity>) -> 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<Identity>) -> 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<HostSystemImpl: ApplicationLayer + ?Sized>(
         &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<Identity>) -> 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<Identity>) -> 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(..) {

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(10);
-const EVENT_HANDLER_TASK_TIMEOUT: Duration = Duration::from_secs(5);
 
 /// An in-filesystem database that permits live editing.
 ///

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<impl Database>, runtime: &Runtime) -> i32 {
-    let handler = Controller::new(database.clone(), runtime.handle().clone()).await;
+    match Controller::new(database.clone(), runtime.handle().clone()).await {
-    if handler.is_err() {
+        Err(err) => {
-        eprintln!("FATAL: error initializing handler: {}", handler.err().unwrap().to_string());
+            eprintln!("FATAL: error initializing handler: {}", err.to_string());
-        exitcode::ERR_CONFIG
+            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
         }
+        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<zerotier_network_hypervisor::vl1::Identity>) -> bool {
-        true
-    }
-
-    fn has_trust_relationship(&self, id: &zerotier_crypto::verified::Verified<zerotier_network_hypervisor::vl1::Identity>) -> bool {
-        true
-    }
-}

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};

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<Identity>) -> 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<Identity>) -> 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<Identity>) -> 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<Identity>) -> 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<Identity>) -> 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<RootSet>) {
         let mut roots = self.roots.write().unwrap();
         if let Some(entry) = roots.sets.get_mut(&rs.name) {

network-hypervisor/src/vl1/peer.rs

@@ -593,7 +593,7 @@ impl Peer {
         source_path: &Arc<Path>,
         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;

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<crate::vl1::Identity>) -> bool {
+        true
+    }
+
+    fn has_trust_relationship(&self, id: &zerotier_crypto::verified::Verified<crate::vl1::Identity>) -> bool {
+        true
+    }
+
     fn handle_packet<Application: ApplicationLayer + ?Sized>(
         &self,
         app: &Application,

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();

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<Inner: InnerProtocolLayer + ?Sized + 'static> {
     state: RwLock<VL1ServiceMutableState>,
-    peer_filter: Arc<dyn PeerFilter>,
     vl1_data_storage: Arc<dyn VL1DataStorage>,
     inner: Arc<Inner>,
     buffer_pool: Arc<PacketBufferPool>,
@@ -49,12 +48,7 @@ struct VL1ServiceMutableState {
 }
 
 impl<Inner: InnerProtocolLayer + ?Sized + 'static> VL1Service<Inner> {
-    pub fn new(
+    pub fn new(vl1_data_storage: Arc<dyn VL1DataStorage>, inner: Arc<Inner>, settings: VL1Settings) -> Result<Arc<Self>, Box<dyn Error>> {
-        peer_filter: Arc<dyn PeerFilter>,
-        vl1_data_storage: Arc<dyn VL1DataStorage>,
-        inner: Arc<Inner>,
-        settings: VL1Settings,
-    ) -> Result<Arc<Self>, Box<dyn Error>> {
         let mut service = Self {
             state: RwLock::new(VL1ServiceMutableState {
                 daemons: Vec::with_capacity(2),
@@ -62,7 +56,6 @@ impl<Inner: InnerProtocolLayer + ?Sized + 'static> VL1Service<Inner> {
                 settings,
                 running: true,
             }),
-            peer_filter,
             vl1_data_storage,
             inner,
             buffer_pool: Arc::new(PacketBufferPool::new(
@@ -233,11 +226,6 @@ impl<Inner: InnerProtocolLayer + ?Sized + 'static> 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()