sync hosts more safely; refactor: split off config

5 files changed, 236 insertions, 197 deletions

diff --git a/src/daemon.rs b/src/daemon.rs
index bf6105c..57667eb 100644
--- a/src/daemon.rs
+++ b/src/daemon.rs
@@ -1,33 +1,23 @@
-use core::net::SocketAddr;
-use dbus::{channel::MatchingReceiver, message::MatchRule};
-use dbus_crossroads::{Context, Crossroads, MethodErr};
-use defguard_wireguard_rs::{
-    host::Peer, key::Key, net::IpAddrMask, InterfaceConfiguration, WGApi, WireguardInterfaceApi,
-};
-use log::{debug, error, info, warn};
-use serde::{Deserialize, Serialize};
+use dbus::channel::MatchingReceiver;
+use defguard_wireguard_rs::{net::IpAddrMask, WGApi, WireguardInterfaceApi};
+use log::{debug, error, info};
 use std::{
     collections::{BTreeSet, HashMap},
-    fs::File,
-    io::{ErrorKind, Read, Write},
-    marker::PhantomData,
-    net::ToSocketAddrs,
     ops::DerefMut,
+    path::PathBuf,
     str::FromStr,
     sync::Arc,
-    time::SystemTime,
 };
 use thiserror::Error;
 use tokio::{
     net::TcpListener,
     runtime::Builder,
     signal::unix::{signal, SignalKind},
-    sync::{broadcast, RwLock},
+    sync::RwLock,
     task,
 };
-use xdg::BaseDirectories;
 
-use crate::{daemon_dbus::*, daemon_network::*};
+use crate::{daemon_config::*, daemon_dbus::*, daemon_network::*};
 
 #[derive(Debug, Error)]
 pub enum DaemonError {
@@ -51,53 +41,10 @@ pub enum DaemonError {
     DbusError(#[from] dbus::Error),
 }
 
-#[derive(Serialize, Deserialize, Clone)]
-pub enum Endpoint {
-    Ip(SocketAddr),
-    Domain(String, u16),
-}
-
-// subset of defguard_wireguard_rs::host::Peer, with hostname added
-#[derive(Serialize, Deserialize)]
-pub struct PeerConfig {
-    pub psk: Option<Key>,
-    pub ips: Vec<(IpAddrMask, Option<String>)>,
-    // if false: the hostnames are kept around for sharing, but we personally do not use them
-    pub use_hostnames: bool,
-    pub endpoint: Option<Endpoint>,
-
-    pub last_changed: SystemTime,
-    pub known_to: Vec<usize>,
-
-    pub mäsch_endpoint: SocketAddr,
-}
-
-fn default_wg_port() -> u16 {
-    51820
-}
-
-#[derive(Serialize, Deserialize)]
-pub struct Network {
-    pub privkey: String,
-    // this really should be a different type, but this is what defguard takes...
-    pub address: String,
-    #[serde(default = "default_wg_port")]
-    pub listen_port: u16,
-    pub peers: HashMap<Key, PeerConfig>,
-
-    pub mäsch_port: u16,
-}
-
-#[derive(Serialize, Deserialize, Default)]
-pub struct Config {
-    pub networks: HashMap<String, Network>,
-}
-
-// TODO das überschreibt änderungen an /etc/hosts während der runtime :(
 pub struct State {
     pub conf: Config,
     pub nw_handles: HashMap<String, (WGApi, task::JoinHandle<()>)>,
-    pub hostfile: Option<(String, BTreeSet<String>)>,
+    pub hostnames: BTreeSet<(String, String)>,
 }
 
 impl Drop for State {
@@ -109,60 +56,31 @@ impl Drop for State {
 }
 
 pub fn daemon() -> Result<(), DaemonError> {
-    let config_path = BaseDirectories::with_prefix("mäsch")?.place_state_file("daemon.json")?;
-    let config: Config = match File::open(config_path) {
-        Ok(f) => serde_json::from_reader(f)?,
-        Err(e) => match e.kind() {
-            ErrorKind::NotFound => Config::default(),
-            _ => Err(e)?,
-        },
-    };
+    let (config_path, config) = load_config()?;
     info!("read config");
 
-    let hostfile = match File::open("/etc/hosts") {
-        Ok(mut f) => {
-            let mut r = String::new();
-            f.read_to_string(&mut r)?;
-
-            let seen_hostnames: BTreeSet<String> = r
-                .lines()
-                .map(|l| {
-                    l.split_whitespace()
-                        .take_while(|dom| dom.chars().next().unwrap() != '#')
-                        .skip(1)
-                })
-                .flatten()
-                .map(|dom| dom.to_owned())
-                .collect();
-
-            Some((r, seen_hostnames))
-        }
-        Err(e) => {
-            warn!("failed to read /etc/hosts: {e}");
-            None
-        }
-    };
-
+    // NOTE this should not be upgraded to a multi-writer structure carelessly, as we also use the
+    // exclusivity of write locks to ensure only one thread at a time syncs the config to disk
     let state = Arc::new(RwLock::new(State {
         conf: config,
         nw_handles: HashMap::new(),
-        hostfile,
+        hostnames: BTreeSet::new(),
     }));
 
     let rt = Builder::new_current_thread().enable_all().build()?;
-    rt.block_on(run_networks(state))?;
+    rt.block_on(run_networks(state, config_path))?;
 
     Ok(())
 }
 
-async fn run_networks(state: Arc<RwLock<State>>) -> Result<(), DaemonError> {
+async fn run_networks(state: Arc<RwLock<State>>, config_path: PathBuf) -> Result<(), DaemonError> {
     let mut state_rw_guard = state.write().await;
     let state_rw = state_rw_guard.deref_mut();
 
     // load existing configurations
+    let mut hostname_pairs = BTreeSet::new();
     for (name, nw) in &state_rw.conf.networks {
-        let wg_api = add_network(
-            &mut state_rw.hostfile,
+        let (wg_api, mut new_hostnames) = add_network(
             name.clone(),
             nw.privkey.clone(),
             nw.address.clone(),
@@ -171,6 +89,8 @@ async fn run_networks(state: Arc<RwLock<State>>) -> Result<(), DaemonError> {
         )
         .await?;
 
+        hostname_pairs.append(&mut new_hostnames);
+
         let addr = IpAddrMask::from_str(&nw.address)?.ip;
         let h = task::spawn(print_error(run_network(
             state.clone(),
@@ -182,46 +102,15 @@ async fn run_networks(state: Arc<RwLock<State>>) -> Result<(), DaemonError> {
 
         debug!("loaded configuration for {0}", name);
     }
+    match sync_hostnames(&hostname_pairs) {
+        Ok(_) => (),
+        Err(e) => error!("Failed to sync hostnames to disk: {e}"),
+    };
     info!("loaded all existing configurations");
     drop(state_rw_guard);
 
     // set up dbus interface
-    let mut cr = Crossroads::new();
-    let state_ref = state.clone();
-    let if_token = cr.register("de.a.maesch", move |b| {
-        b.signal::<(String, String), _>("Proposal", ("network", "peer_data"));
-        b.method_with_cr_async(
-            "AddNetwork",
-            ("name", "key", "ip", "listen_port", "maesch_port"),
-            ("success",),
-            move |ctx, _, args: (String, String, String, u16, u16)| {
-                debug!("Received AddNetwork");
-                handle_add_network(ctx, state_ref.clone(), args)
-            },
-        );
-    });
-    cr.insert("/de/a/maesch", &[if_token], ());
-
-    // drive dbus interface
-    let (res, c) = dbus_tokio::connection::new_system_sync()?;
-    cr.set_async_support(Some((
-        c.clone(),
-        Box::new(|x| {
-            tokio::spawn(x);
-        }),
-    )));
-    let _ = tokio::spawn(print_error(async {
-        res.await;
-        Result::<!, &'static str>::Err("lost connection to dbus!")
-    }));
-    let receive_token = c.start_receive(
-        MatchRule::new_method_call(),
-        Box::new(move |msg, conn| {
-            cr.handle_message(msg, conn).unwrap();
-            true
-        }),
-    );
-    c.request_name("de.a.maesch", false, true, false).await?;
+    let (c, recv_token) = start_dbus(state.clone(), config_path).await?;
 
     // wait for SIGTERM/SIGINT
     let mut sigterm_fut = signal(SignalKind::terminate())?;
@@ -235,7 +124,7 @@ async fn run_networks(state: Arc<RwLock<State>>) -> Result<(), DaemonError> {
     };
 
     // clean exit
-    c.stop_receive(receive_token);
+    c.stop_receive(recv_token);
     let mut state_rw_guard = state.write().await;
     for (_, (wg_api, h)) in state_rw_guard.nw_handles.drain() {
         let _ = wg_api.remove_interface();
diff --git a/src/daemon_config.rs b/src/daemon_config.rs
new file mode 100644
index 0000000..246a736
--- /dev/null
+++ b/src/daemon_config.rs
@@ -0,0 +1,121 @@
+use atomic_write_file::AtomicWriteFile;
+use defguard_wireguard_rs::{key::Key, net::IpAddrMask};
+use log::info;
+use serde::{Deserialize, Serialize};
+use std::{
+    collections::{BTreeSet, HashMap},
+    fs::File,
+    io::{ErrorKind, Read, Write},
+    net::SocketAddr,
+    path::PathBuf,
+    time::SystemTime,
+};
+use xdg::BaseDirectories;
+
+use crate::daemon::DaemonError;
+
+#[derive(Serialize, Deserialize, Clone)]
+pub enum Endpoint {
+    Ip(SocketAddr),
+    Domain(String, u16),
+}
+
+// subset of defguard_wireguard_rs::host::Peer, with hostname added
+#[derive(Serialize, Deserialize)]
+pub struct PeerConfig {
+    pub psk: Option<Key>,
+    pub ips: Vec<(IpAddrMask, Option<String>)>,
+    // if false: the hostnames are kept around for sharing, but we personally do not use them
+    pub use_hostnames: bool,
+    pub endpoint: Option<Endpoint>,
+
+    pub last_changed: SystemTime,
+    pub known_to: Vec<usize>,
+
+    pub mäsch_endpoint: SocketAddr,
+}
+
+fn default_wg_port() -> u16 {
+    51820
+}
+
+#[derive(Serialize, Deserialize)]
+pub struct Network {
+    pub privkey: String,
+    // this really should be a different type, but this is what defguard takes...
+    pub address: String,
+    #[serde(default = "default_wg_port")]
+    pub listen_port: u16,
+    pub peers: HashMap<Key, PeerConfig>,
+
+    pub mäsch_port: u16,
+}
+
+#[derive(Serialize, Deserialize, Default)]
+pub struct Config {
+    pub networks: HashMap<String, Network>,
+}
+
+pub fn load_config() -> Result<(PathBuf, Config), DaemonError> {
+    let config_path = BaseDirectories::with_prefix("mäsch")?.place_state_file("daemon.json")?;
+    let config: Config = match File::open(&config_path) {
+        Ok(f) => serde_json::from_reader(f)?,
+        Err(e) => match e.kind() {
+            ErrorKind::NotFound => Config::default(),
+            _ => Err(e)?,
+        },
+    };
+    info!("read config");
+
+    Ok((config_path, config))
+}
+
+pub fn write_config(conf: &Config, path: &PathBuf) -> Result<(), std::io::Error> {
+    let mut f = AtomicWriteFile::open(path)?;
+    serde_json::to_writer(&mut f, conf)?;
+    f.commit()?;
+
+    Ok(())
+}
+
+pub fn sync_hostnames(hostnames: &BTreeSet<(String, String)>) -> Result<(), std::io::Error> {
+    let mut hosts_str = match File::open("/etc/hosts") {
+        Ok(mut f) => {
+            let mut r = String::new();
+            f.read_to_string(&mut r)?;
+            r
+        }
+        Err(e) => match e.kind() {
+            ErrorKind::NotFound => "".to_owned(),
+            _ => return Err(e),
+        },
+    };
+
+    let seen_hostnames: BTreeSet<String> = hosts_str
+        .lines()
+        .map(|l| {
+            l.split_whitespace()
+                .take_while(|dom| dom.chars().next().unwrap() != '#')
+                .skip(1)
+        })
+        .flatten()
+        .map(|dom| dom.to_owned())
+        .collect();
+
+    for (ip, dom) in hostnames {
+        if !seen_hostnames.contains(dom.as_str()) {
+            hosts_str.push_str(ip);
+            hosts_str.push(' ');
+            hosts_str.push_str(&format!("{dom}"));
+            hosts_str.push('\n');
+        }
+    }
+
+    info!("Syncing host file");
+
+    let mut f = AtomicWriteFile::open("/etc/hosts")?;
+    f.write(hosts_str.as_bytes())?;
+    f.commit()?;
+
+    Ok(())
+}
diff --git a/src/daemon_dbus.rs b/src/daemon_dbus.rs
index 7065eec..5ed4d94 100644
--- a/src/daemon_dbus.rs
+++ b/src/daemon_dbus.rs
@@ -1,30 +1,69 @@
 use base64::prelude::*;
-use dbus::{channel::MatchingReceiver, message::MatchRule};
-use dbus_crossroads::{Context, Crossroads, MethodErr};
-use defguard_wireguard_rs::{
-    host::Peer, key::Key, net::IpAddrMask, InterfaceConfiguration, WGApi, WireguardInterfaceApi,
+use dbus::{
+    channel::{MatchingReceiver, Token},
+    message::MatchRule,
+    nonblock::SyncConnection,
 };
+use dbus_crossroads::{Context, Crossroads, MethodErr};
+use defguard_wireguard_rs::{key::Key, net::IpAddrMask, WireguardInterfaceApi};
 use log::{debug, error, info, warn};
 use rand::Rng;
 use std::{
-    collections::{BTreeSet, HashMap},
-    marker::PhantomData,
-    ops::DerefMut,
-    str::FromStr,
+    collections::HashMap, marker::PhantomData, ops::DerefMut, path::PathBuf, str::FromStr,
     sync::Arc,
 };
-use tokio::{
-    net::TcpListener,
-    sync::{broadcast, RwLock},
-    task,
-};
+use tokio::{net::TcpListener, sync::RwLock, task};
+
+use crate::{daemon::*, daemon_config::*, daemon_network::*};
+
+pub async fn start_dbus(
+    state: Arc<RwLock<State>>,
+    config_path: PathBuf,
+) -> Result<(Arc<SyncConnection>, Token), DaemonError> {
+    let mut cr = Crossroads::new();
+    let if_token = cr.register("de.a.maesch", move |b| {
+        b.signal::<(String, String), _>("Proposal", ("network", "peer_data"));
+        b.method_with_cr_async(
+            "AddNetwork",
+            ("name", "key", "ip", "listen_port", "maesch_port"),
+            ("success",),
+            move |ctx, _, args: (String, String, String, u16, u16)| {
+                debug!("Received AddNetwork");
+                handle_add_network(ctx, state.clone(), config_path.clone(), args)
+            },
+        );
+    });
+    cr.insert("/de/a/maesch", &[if_token], ());
+
+    // drive dbus interface
+    let (res, c) = dbus_tokio::connection::new_system_sync()?;
+    cr.set_async_support(Some((
+        c.clone(),
+        Box::new(|x| {
+            tokio::spawn(x);
+        }),
+    )));
+    let _ = tokio::spawn(print_error(async {
+        res.await;
+        Result::<!, &'static str>::Err("lost connection to dbus!")
+    }));
+    let receive_token = c.start_receive(
+        MatchRule::new_method_call(),
+        Box::new(move |msg, conn| {
+            cr.handle_message(msg, conn).unwrap();
+            true
+        }),
+    );
+    c.request_name("de.a.maesch", false, true, false).await?;
 
-use crate::{daemon::*, daemon_network::*};
+    Ok((c, receive_token))
+}
 
 // TODO also take peers
 pub async fn handle_add_network(
     mut ctx: Context,
     state: Arc<RwLock<State>>,
+    config_path: PathBuf,
     (name, may_key, may_ip, may_lp, may_mp): (String, String, String, u16, u16),
 ) -> PhantomData<(bool,)> {
     // NOTE: this is kinda stupid: we convert to a string later anyways, as thats what
@@ -55,11 +94,7 @@ pub async fn handle_add_network(
     let lp = if may_lp == 0 { 25565 } else { may_lp };
     let mp = if may_mp == 0 { 51820 } else { may_mp };
 
-    let mut state_rw_guard = state.write().await;
-    let state_rw = state_rw_guard.deref_mut();
-
-    let wg_api = match add_network(
-        &mut state_rw.hostfile,
+    let (wg_api, hostnames) = match add_network(
         name.clone(),
         key.to_string(),
         ip_string,
@@ -68,14 +103,13 @@ pub async fn handle_add_network(
     )
     .await
     {
-        Ok(wg_api) => wg_api,
+        Ok(v) => v,
         Err(e) => {
             warn!("AddNetwork couldn't add network: {e}");
             return ctx.reply(Err(MethodErr::failed(&e)));
         }
     };
 
-    // TODO ins wg_api
     let listener = match TcpListener::bind((ip, mp)).await {
         Ok(l) => l,
         Err(e) => {
@@ -90,9 +124,21 @@ pub async fn handle_add_network(
         name.clone(),
     )));
 
+    let mut state_rw_guard = state.write().await;
+    let state_rw = state_rw_guard.deref_mut();
     state_rw.nw_handles.insert(name.clone(), (wg_api, h));
 
-    // TODO save new config
+    // NOTE this _is_ thread-safe, as we hold an exclusive write handle to the state.
+    match write_config(&state_rw.conf, &config_path) {
+        Ok(_) => info!("Synced config"),
+        Err(e) => error!("Couldn't sync config: {e}"),
+    }
+    // similarly, this is still racy w.r.t. other processes running on the system, but at least
+    // no other thread from this program should be able to concurrently call this
+    match sync_hostnames(&hostnames) {
+        Ok(_) => (),
+        Err(e) => error!("Failed to sync hostnames to disk: {e}"),
+    };
 
     ctx.reply(Ok((true,)))
 }
diff --git a/src/daemon_network.rs b/src/daemon_network.rs
index 895e409..97349c8 100644
--- a/src/daemon_network.rs
+++ b/src/daemon_network.rs
@@ -1,29 +1,26 @@
-use atomic_write_file::AtomicWriteFile;
 use defguard_wireguard_rs::{
-    host::Peer, key::Key, net::IpAddrMask, InterfaceConfiguration, WGApi, WireguardInterfaceApi,
+    host::Peer, key::Key, InterfaceConfiguration, WGApi, WireguardInterfaceApi,
 };
-use log::{debug, error, info, warn};
+//use log::{debug, error, info, warn};
 use std::{
     collections::{BTreeSet, HashMap},
-    io::Write,
     net::ToSocketAddrs,
     sync::Arc,
 };
-use tokio::{
-    net::TcpListener,
-    sync::{broadcast, RwLock},
-};
+use tokio::{net::TcpListener, sync::RwLock};
 
 use crate::daemon::*;
+use crate::daemon_config::*;
 
 pub async fn add_network(
-    hostfile: &mut Option<(String, BTreeSet<String>)>,
     name: String,
     privkey: String,
     address: String,
     port: u16,
     peers: &HashMap<Key, PeerConfig>,
-) -> Result<WGApi, DaemonError> {
+) -> Result<(WGApi, BTreeSet<(String, String)>), DaemonError> {
+    let mut hostname_pairs = BTreeSet::new();
+
     let wg = WGApi::new(name.clone(), false)?;
     let defguard_peers = peers
         .iter()
@@ -59,38 +56,23 @@ pub async fn add_network(
         peers: defguard_peers,
     })?;
 
-    if let Some((hosts_str, hosts)) = hostfile {
-        peers
-            .values()
-            .map(|peer| {
-                if peer.use_hostnames {
-                    peer.ips
-                        .iter()
-                        .map(|(mask, may_dom)| {
-                            if let Some(dom) = may_dom
-                                && hosts.insert(dom.clone())
-                            {
-                                hosts_str.push_str(&format!("{}", mask.ip));
-                                hosts_str.push('\t');
-                                hosts_str.push_str(&dom);
-                                hosts_str.push('\n');
-                            }
-                        })
-                        .count();
-                }
-            })
-            .count();
-    }
-
-    if let Some((hosts_str, _)) = hostfile {
-        debug!("writing hosts file: {hosts_str}");
-
-        let mut f = AtomicWriteFile::open("/etc/hosts")?;
-        f.write(hosts_str.as_bytes())?;
-        f.commit()?;
-    }
+    peers
+        .values()
+        .map(|peer| {
+            if peer.use_hostnames {
+                peer.ips
+                    .iter()
+                    .map(|(mask, may_dom)| {
+                        if let Some(dom) = may_dom {
+                            hostname_pairs.insert((format!("{}", mask.ip), dom.clone()));
+                        }
+                    })
+                    .count();
+            }
+        })
+        .count();
 
-    Ok(wg)
+    Ok((wg, hostname_pairs))
 }
 
 pub async fn run_network(
diff --git a/src/main.rs b/src/main.rs
index 705b39d..d549db5 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -3,6 +3,7 @@
 #![feature(if_let_guard)]
 
 pub mod daemon;
+pub mod daemon_config;
 pub mod daemon_dbus;
 pub mod daemon_network;