Expose one unified NetworkSecurity struct, and make it log

9 files changed, 72 insertions, 50 deletions

diff --git a/talpid-core/src/security/linux/mod.rs b/talpid-core/src/security/linux/mod.rs
index de29219298..455e4cc755 100644
--- a/talpid-core/src/security/linux/mod.rs
+++ b/talpid-core/src/security/linux/mod.rs
@@ -18,7 +18,7 @@ use std::io;
 use std::net::{IpAddr, Ipv4Addr};
 use std::path::Path;
 
-use super::{NetworkSecurity, SecurityPolicy};
+use super::{NetworkSecurityT, SecurityPolicy};
 
 mod dns;
 use self::dns::DnsSettings;
@@ -71,17 +71,17 @@ enum End {
     Dst,
 }
 
-/// The Linux implementation for the `NetworkSecurity` trait.
-pub struct LinuxNetworkSecurity {
+/// The Linux implementation for the firewall and DNS.
+pub struct NetworkSecurity {
     dns_settings: DnsSettings,
     table_name: CString,
 }
 
-impl NetworkSecurity for LinuxNetworkSecurity {
+impl NetworkSecurityT for NetworkSecurity {
     type Error = Error;
 
     fn new(_cache_dir: impl AsRef<Path>) -> Result<Self> {
-        Ok(LinuxNetworkSecurity {
+        Ok(NetworkSecurity {
             dns_settings: DnsSettings::new()?,
             table_name: TABLE_NAME.clone(),
         })
@@ -117,7 +117,7 @@ impl NetworkSecurity for LinuxNetworkSecurity {
     }
 }
 
-impl LinuxNetworkSecurity {
+impl NetworkSecurity {
     fn send_and_process(&self, batch: &FinalizedBatch) -> Result<()> {
         let socket =
             mnl::Socket::new(mnl::Bus::Netfilter).chain_err(|| ErrorKind::NetlinkOpenError)?;
diff --git a/talpid-core/src/security/macos/mod.rs b/talpid-core/src/security/macos/mod.rs
index 331f2f29ec..abcfd8e404 100644
--- a/talpid-core/src/security/macos/mod.rs
+++ b/talpid-core/src/security/macos/mod.rs
@@ -1,7 +1,7 @@
 extern crate pfctl;
 extern crate tokio_core;
 
-use super::{NetworkSecurity, SecurityPolicy};
+use super::{NetworkSecurityT, SecurityPolicy};
 
 use ipnetwork::IpNetwork;
 
@@ -26,17 +26,17 @@ error_chain! {
 const ANCHOR_NAME: &'static str = "mullvad";
 
 /// The macOS firewall and DNS implementation.
-pub struct MacosNetworkSecurity {
+pub struct NetworkSecurity {
     pf: pfctl::PfCtl,
     pf_was_enabled: Option<bool>,
     dns_monitor: DnsMonitor,
 }
 
-impl NetworkSecurity for MacosNetworkSecurity {
+impl NetworkSecurityT for NetworkSecurity {
     type Error = Error;
 
     fn new(_cache_dir: impl AsRef<Path>) -> Result<Self> {
-        Ok(MacosNetworkSecurity {
+        Ok(NetworkSecurity {
             pf: pfctl::PfCtl::new()?,
             pf_was_enabled: None,
             dns_monitor: DnsMonitor::new()?,
@@ -61,7 +61,7 @@ impl NetworkSecurity for MacosNetworkSecurity {
     }
 }
 
-impl MacosNetworkSecurity {
+impl NetworkSecurity {
     fn set_rules(&mut self, policy: SecurityPolicy) -> Result<()> {
         let mut new_filter_rules = vec![];
 
diff --git a/talpid-core/src/security/mod.rs b/talpid-core/src/security/mod.rs
index 5dcbc87dd3..2575709345 100644
--- a/talpid-core/src/security/mod.rs
+++ b/talpid-core/src/security/mod.rs
@@ -5,6 +5,22 @@ use std::net::Ipv4Addr;
 use std::path::Path;
 use talpid_types::net::Endpoint;
 
+
+#[cfg(target_os = "macos")]
+#[path = "macos/mod.rs"]
+mod imp;
+
+#[cfg(target_os = "linux")]
+#[path = "linux/mod.rs"]
+mod imp;
+
+#[cfg(windows)]
+#[path = "windows/mod.rs"]
+mod imp;
+
+pub use self::imp::{Error, ErrorKind};
+
+
 #[cfg(unix)]
 lazy_static! {
     static ref PRIVATE_NETS: [Ipv4Network; 3] = [
@@ -44,8 +60,38 @@ pub enum SecurityPolicy {
     },
 }
 
-/// Abstract firewall interaction trait
-pub trait NetworkSecurity: Sized {
+/// Manages network security of the computer/device. Can apply and enforce security policies
+/// by manipulating the OS firewall and DNS settings.
+pub struct NetworkSecurity {
+    inner: imp::NetworkSecurity,
+}
+
+impl NetworkSecurity {
+    /// Returns a new `NetworkSecurity`, ready to apply policies.
+    pub fn new(cache_dir: impl AsRef<Path>) -> Result<Self, Error> {
+        Ok(NetworkSecurity {
+            inner: imp::NetworkSecurity::new(cache_dir)?,
+        })
+    }
+
+    /// Applies and starts enforcing the given `SecurityPolicy` Makes sure it is being kept in place
+    /// until this method is called again with another policy, or until `reset_policy` is called.
+    pub fn apply_policy(&mut self, policy: SecurityPolicy) -> Result<(), Error> {
+        debug!("Setting security policy: {:?}", policy);
+        self.inner.apply_policy(policy)
+    }
+
+    /// Resets/removes any currently enforced `SecurityPolicy`. Returns the system to the same state
+    /// it had before any policy was applied through this `NetworkSecurity` instance.
+    pub fn reset_policy(&mut self) -> Result<(), Error> {
+        debug!("Resetting security policy");
+        self.inner.reset_policy()
+    }
+}
+
+
+/// Abstract firewall interaction trait. Used by the OS specific implementations.
+trait NetworkSecurityT: Sized {
     /// The error type thrown by the implementer of this trait
     type Error: ::std::error::Error;
 
@@ -59,19 +105,3 @@ pub trait NetworkSecurity: Sized {
     /// modifying the system.
     fn reset_policy(&mut self) -> ::std::result::Result<(), Self::Error>;
 }
-
-
-#[cfg(target_os = "macos")]
-mod macos;
-#[cfg(target_os = "macos")]
-pub use self::macos::{Error, ErrorKind, MacosNetworkSecurity as NetworkSecurityImpl, Result};
-
-#[cfg(target_os = "linux")]
-mod linux;
-#[cfg(target_os = "linux")]
-pub use self::linux::{Error, ErrorKind, LinuxNetworkSecurity as NetworkSecurityImpl, Result};
-
-#[cfg(windows)]
-mod windows;
-#[cfg(windows)]
-pub use self::windows::{Error, ErrorKind, Result, WindowsNetworkSecurity as NetworkSecurityImpl};
diff --git a/talpid-core/src/security/windows/mod.rs b/talpid-core/src/security/windows/mod.rs
index 16fd707030..e762a4887a 100644
--- a/talpid-core/src/security/windows/mod.rs
+++ b/talpid-core/src/security/windows/mod.rs
@@ -1,6 +1,6 @@
 extern crate widestring;
 
-use super::{NetworkSecurity, SecurityPolicy};
+use super::{NetworkSecurityT, SecurityPolicy};
 use std::net::IpAddr;
 use std::path::Path;
 use std::ptr;
@@ -60,12 +60,12 @@ error_chain! {
 
 const WINFW_TIMEOUT_SECONDS: u32 = 2;
 
-/// The Windows implementation for the `NetworkSecurity` trait.
-pub struct WindowsNetworkSecurity {
+/// The Windows implementation for the firewall and DNS.
+pub struct NetworkSecurity {
     dns: WinDns,
 }
 
-impl NetworkSecurity for WindowsNetworkSecurity {
+impl NetworkSecurityT for NetworkSecurity {
     type Error = Error;
 
     fn new(cache_dir: impl AsRef<Path>) -> Result<Self> {
@@ -78,7 +78,7 @@ impl NetworkSecurity for WindowsNetworkSecurity {
             ).into_result()?
         };
         trace!("Successfully initialized windows firewall module");
-        Ok(WindowsNetworkSecurity { dns: windns })
+        Ok(NetworkSecurity { dns: windns })
     }
 
     fn apply_policy(&mut self, policy: SecurityPolicy) -> Result<()> {
@@ -106,14 +106,13 @@ impl NetworkSecurity for WindowsNetworkSecurity {
     }
 
     fn reset_policy(&mut self) -> Result<()> {
-        trace!("Resetting firewall policy");
         self.dns.reset_dns()?;
         unsafe { WinFw_Reset().into_result() }?;
         Ok(())
     }
 }
 
-impl Drop for WindowsNetworkSecurity {
+impl Drop for NetworkSecurity {
     fn drop(&mut self) {
         if unsafe { WinFw_Deinitialize().into_result().is_ok() } {
             trace!("Successfully deinitialized windows firewall module");
@@ -123,7 +122,7 @@ impl Drop for WindowsNetworkSecurity {
     }
 }
 
-impl WindowsNetworkSecurity {
+impl NetworkSecurity {
     fn set_connecting_state(
         &mut self,
         endpoint: &Endpoint,
diff --git a/talpid-core/src/tunnel_state_machine/blocked_state.rs b/talpid-core/src/tunnel_state_machine/blocked_state.rs
index 57203db275..6c50e2b9f7 100644
--- a/talpid-core/src/tunnel_state_machine/blocked_state.rs
+++ b/talpid-core/src/tunnel_state_machine/blocked_state.rs
@@ -8,7 +8,7 @@ use super::{
     ConnectingState, DisconnectedState, EventConsequence, ResultExt, SharedTunnelStateValues,
     TunnelCommand, TunnelState, TunnelStateTransition, TunnelStateWrapper,
 };
-use security::{NetworkSecurity, SecurityPolicy};
+use security::SecurityPolicy;
 
 /// No tunnel is running and all network connections are blocked.
 pub struct BlockedState;
@@ -16,7 +16,6 @@ pub struct BlockedState;
 impl BlockedState {
     fn set_security_policy(shared_values: &mut SharedTunnelStateValues, allow_lan: bool) {
         let policy = SecurityPolicy::Blocked { allow_lan };
-        debug!("Setting security policy: {:?}", policy);
         if let Err(error) = shared_values
             .security
             .apply_policy(policy)
diff --git a/talpid-core/src/tunnel_state_machine/connected_state.rs b/talpid-core/src/tunnel_state_machine/connected_state.rs
index b9856dbedc..224fce3e91 100644
--- a/talpid-core/src/tunnel_state_machine/connected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connected_state.rs
@@ -10,7 +10,7 @@ use super::{
     SharedTunnelStateValues, TunnelCommand, TunnelParameters, TunnelState, TunnelStateTransition,
     TunnelStateWrapper,
 };
-use security::{NetworkSecurity, SecurityPolicy};
+use security::SecurityPolicy;
 use tunnel::{CloseHandle, TunnelEvent, TunnelMetadata};
 
 pub struct ConnectedStateBootstrap {
@@ -50,8 +50,6 @@ impl ConnectedState {
             tunnel: self.metadata.clone(),
             allow_lan: self.tunnel_parameters.allow_lan,
         };
-
-        debug!("Setting security policy: {:?}", policy);
         shared_values
             .security
             .apply_policy(policy)
diff --git a/talpid-core/src/tunnel_state_machine/connecting_state.rs b/talpid-core/src/tunnel_state_machine/connecting_state.rs
index 8b2867b4fb..80fd94f364 100644
--- a/talpid-core/src/tunnel_state_machine/connecting_state.rs
+++ b/talpid-core/src/tunnel_state_machine/connecting_state.rs
@@ -18,7 +18,7 @@ use super::{
     TunnelState, TunnelStateTransition, TunnelStateWrapper,
 };
 use logging;
-use security::{NetworkSecurity, SecurityPolicy};
+use security::SecurityPolicy;
 use tunnel::{CloseHandle, TunnelEvent, TunnelMetadata, TunnelMonitor};
 
 const MIN_TUNNEL_ALIVE_TIME: Duration = Duration::from_millis(1000);
@@ -63,8 +63,6 @@ impl ConnectingState {
             relay_endpoint: endpoint.to_endpoint(),
             allow_lan,
         };
-
-        debug!("Setting security policy: {:?}", policy);
         shared_values
             .security
             .apply_policy(policy)
diff --git a/talpid-core/src/tunnel_state_machine/disconnected_state.rs b/talpid-core/src/tunnel_state_machine/disconnected_state.rs
index 1a22043f0a..d65f8f63de 100644
--- a/talpid-core/src/tunnel_state_machine/disconnected_state.rs
+++ b/talpid-core/src/tunnel_state_machine/disconnected_state.rs
@@ -6,14 +6,12 @@ use super::{
     BlockedState, ConnectingState, Error, EventConsequence, SharedTunnelStateValues, TunnelCommand,
     TunnelState, TunnelStateTransition, TunnelStateWrapper,
 };
-use security::NetworkSecurity;
 
 /// No tunnel is running.
 pub struct DisconnectedState;
 
 impl DisconnectedState {
     fn reset_security_policy(shared_values: &mut SharedTunnelStateValues) {
-        debug!("Resetting security policy");
         if let Err(error) = shared_values.security.reset_policy() {
             let chained_error = Error::with_chain(error, "Failed to reset security policy");
             error!("{}", chained_error.display_chain());
diff --git a/talpid-core/src/tunnel_state_machine/mod.rs b/talpid-core/src/tunnel_state_machine/mod.rs
index aed46f3ab5..f1ab145137 100644
--- a/talpid-core/src/tunnel_state_machine/mod.rs
+++ b/talpid-core/src/tunnel_state_machine/mod.rs
@@ -26,7 +26,7 @@ use self::connecting_state::ConnectingState;
 use self::disconnected_state::DisconnectedState;
 use self::disconnecting_state::{AfterDisconnect, DisconnectingState};
 use super::mpsc::IntoSender;
-use super::security::{NetworkSecurity, NetworkSecurityImpl};
+use super::security::NetworkSecurity;
 
 error_chain! {
     errors {
@@ -149,7 +149,7 @@ impl TunnelStateMachine {
         commands: mpsc::UnboundedReceiver<TunnelCommand>,
     ) -> Result<Self> {
         let security =
-            NetworkSecurityImpl::new(cache_dir).chain_err(|| ErrorKind::NetworkSecurityError)?;
+            NetworkSecurity::new(cache_dir).chain_err(|| ErrorKind::NetworkSecurityError)?;
         let mut shared_values = SharedTunnelStateValues { security };
 
         let initial_state = TunnelStateWrapper::new(&mut shared_values, ());
@@ -214,7 +214,7 @@ impl<T: TunnelState> From<EventConsequence<T>> for TunnelStateMachineAction {
 
 /// Values that are common to all tunnel states.
 struct SharedTunnelStateValues {
-    security: NetworkSecurityImpl,
+    security: NetworkSecurity,
 }
 
 /// Asynchronous result of an attempt to progress a state.