Merge branch 'windns-integration'

13 files changed, 414 insertions, 190 deletions

diff --git a/appveyor.yml b/appveyor.yml
index 2a0acdb8ff..0ef02e3394 100644
--- a/appveyor.yml
+++ b/appveyor.yml
@@ -51,7 +51,7 @@ install:
 
 # This is the "test phase", tweak it as you see fit
 test_script:
-  - bash -x build_winfw.sh
+  - bash -x build_windows_libraries.sh
   - cargo build
   - cargo test
 
diff --git a/build_winfw.sh b/build_windows_libraries.sh
index 521fe007fa..2726170c75 100644
--- a/build_winfw.sh
+++ b/build_windows_libraries.sh
@@ -1,7 +1,8 @@
 set -eu
 
 # List of solutions to build
-CPP_SOLUTIONS=${CPP_SOLUTIONS:-"winfw"}
+WINFW_SOLUTIONS=${WINFW_SOLUTIONS:-"winfw"}
+WINDNS_SOLUTIONS=${WINDNS_SOLUTIONS:-"windns"}
 
 # Override this variable to set your own list of build configurations for
 # wfpctl
@@ -12,20 +13,19 @@ CPP_BUILD_TARGETS=${CPP_BUILD_TARGETS:-"x86 x64"}
 CARGO_TARGET_DIR=${CARGO_TARGET_DIR:-"./target/"}
 
 
-# Builds all 4 variations of the wfpctl.dll library. Takes an argument that is
-# the root of the WFP repository.
-function build_wfpctl
+# Builds visual studio projects
+function build_project
 {
   local path="$1"
+  local solutions="$2"
 
   # Sometimes the build output needs to be cleaned up
   rm -r $path/bin/* || true
-  
+
   set -x
   for mode in $CPP_BUILD_MODES; do
     for target in $CPP_BUILD_TARGETS; do
-      cmd.exe "/c msbuild.exe $(to_win_path $path/winfw.sln) /p:Configuration=$mode /p:Platform=$target /t:$CPP_SOLUTIONS"
-		
+      cmd.exe "/c msbuild.exe $(to_win_path $path) /p:Configuration=$mode /p:Platform=$target /t:$solutions"
     done
   done
   set +x
@@ -37,7 +37,7 @@ function to_win_path
   # if it's a relative path and starts with a dot (.), don't transform the
   # drive prefix (/c/ -> C:\)
   if echo $unixpath | grep '^\.' >/dev/null; then
-    echo $unixpath | sed -e 's/^\///' -e 's/\//\\/g' 
+    echo $unixpath | sed -e 's/^\///' -e 's/\//\\/g'
   # if it's an absolute path, transform the drive prefix
   else
     # remove the cygrdive prefix if it's there
@@ -46,21 +46,6 @@ function to_win_path
   fi
 }
 
-function copy_outputs
-{
-  local wfp_root_path=$1
-
-  for mode in $CPP_BUILD_MODES; do
-    for target in $CPP_BUILD_TARGETS; do
-      local dll_path=$(get_wfp_output_path $wfp_root_path $target $mode)
-      local cargo_target=$(get_cargo_target_dir $target $mode)
-      mkdir -p $cargo_target
-      cp "$dll_path/winfw.dll" $cargo_target
-    done
-  done
-
-}
-
 function get_wfp_output_path
 {
   local wfp_root=$1
@@ -102,26 +87,6 @@ function get_cargo_target_dir
   echo "$CARGO_TARGET_DIR/$platform_triplet/${build_mode,,}"
 }
 
-# Rust isn't internally consistent w.r.t. architecture names - for build
-# targets 32 bit x86 is calles i686, for hosts, it's x86. Hence these need to
-# be converted.
-function host_arch_to_target_arch
-{
-  local host_arch=$1
-
-  case $host_arch in
-    "x86")
-      echo "i686"
-      ;;
-    "x64")
-      echo "x86_64"
-      ;;
-    *)
-      echo $build_target
-      ;;
-  esac
-}
-
 # Since Microsoft likes to name their architectures differently from Rust, this
 # function tries to match microsoft names to Rust names.
 function arch_from_build_target
@@ -153,10 +118,11 @@ function rustc_host_arch
 function main
 {
 
-  local wfp_root_path=${CPP_ROOT_PATH:-"./windows/winfw"}
+  local winfw_root_path=${CPP_ROOT_PATH:-"./windows/winfw"}
+  local windns_root_path=${CPP_ROOT_PATH:-"./windows/windns"}
 
-  build_wfpctl $wfp_root_path
-  copy_outputs $wfp_root_path
+  build_project "$winfw_root_path/winfw.sln" "$WINFW_SOLUTIONS"
+  build_project "$windns_root_path/windns.sln" "$WINDNS_SOLUTIONS"
 }
 
 main
diff --git a/electron-builder.yml b/electron-builder.yml
index 83bea2c583..a7c5c5e77a 100644
--- a/electron-builder.yml
+++ b/electron-builder.yml
@@ -73,6 +73,8 @@ win:
       to: .
     - from: ./windows/winfw/bin/x64-Release/winfw.dll
       to: .
+    - from: ./windows/windns/bin/x64-Release/windns.dll
+      to: .
     - from: ./dist-assets/binaries/windows/openvpn.exe
       to: .
 
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index ff936ef8ea..892c438188 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -239,7 +239,7 @@ impl Daemon {
 
         let (tx, rx) = mpsc::channel();
         let management_interface_broadcaster =
-            Self::start_management_interface(tx.clone(), cache_dir)?;
+            Self::start_management_interface(tx.clone(), cache_dir.clone())?;
         let state = TunnelState::NotRunning;
         let target_state = TargetState::Unsecured;
         Ok(Daemon {
@@ -260,7 +260,7 @@ impl Daemon {
             http_handle,
             tokio_remote,
             relay_selector,
-            firewall: FirewallProxy::new().chain_err(|| ErrorKind::FirewallError)?,
+            firewall: FirewallProxy::new(&cache_dir).chain_err(|| ErrorKind::FirewallError)?,
             current_relay: None,
             tunnel_endpoint: None,
             tunnel_metadata: None,
diff --git a/talpid-core/build.rs b/talpid-core/build.rs
index 7cf5b9475a..7b433cf28f 100644
--- a/talpid-core/build.rs
+++ b/talpid-core/build.rs
@@ -4,8 +4,21 @@ mod win {
     use std::path::PathBuf;
 
     static WINFW_BUILD_DIR: &'static str = "..\\windows\\winfw\\bin";
+    static WINDNS_BUILD_DIR: &'static str = "..\\windows\\windns\\bin";
 
     pub fn default_winfw_output_dir() -> PathBuf {
+        manifest_dir()
+            .join(WINFW_BUILD_DIR)
+            .join(&target_platform_dir())
+    }
+
+    pub fn default_windns_output_dir() -> PathBuf {
+        manifest_dir()
+            .join(WINDNS_BUILD_DIR)
+            .join(&target_platform_dir())
+    }
+
+    fn target_platform_dir() -> PathBuf {
         let target = env::var("TARGET").expect("TARGET env var not set");
 
         let target_dir = match target.as_str() {
@@ -13,8 +26,7 @@ mod win {
             "x86_64-pc-windows-msvc" => format!("x64-{}", get_build_mode()),
             _ => panic!("uncrecognized target: {}", target),
         };
-
-        manifest_dir().join(WINFW_BUILD_DIR).join(&target_dir)
+        target_dir.into()
     }
 
     fn manifest_dir() -> PathBuf {
@@ -52,6 +64,18 @@ fn main() {
             .expect("failed to construct path for winfw include directory")
     );
     println!("cargo:rustc-link-lib=dylib=winfw");
+
+    let windns_dir = env::var_os("WINDNS_INCLUDE_DIR")
+        .map(PathBuf::from)
+        .unwrap_or_else(default_windns_output_dir);
+    println!(
+        "cargo:rustc-link-search={}",
+        windns_dir
+            .to_str()
+            .expect("failed to construct path for windns include directory")
+    );
+
+    println!("cargo:rustc-link-lib=dylib=windns");
 }
 
 #[cfg(not(windows))]
diff --git a/talpid-core/src/firewall/linux/mod.rs b/talpid-core/src/firewall/linux/mod.rs
index 6b8e213f54..7761afd7e9 100644
--- a/talpid-core/src/firewall/linux/mod.rs
+++ b/talpid-core/src/firewall/linux/mod.rs
@@ -1,4 +1,5 @@
 use error_chain::ChainedError;
+use std::path::Path;
 
 use super::{Firewall, SecurityPolicy};
 
@@ -20,7 +21,7 @@ pub struct Netfilter {
 impl Firewall for Netfilter {
     type Error = Error;
 
-    fn new() -> Result<Self> {
+    fn new<P: AsRef<Path>>(_cache_dir: P) -> Result<Self> {
         Ok(Netfilter {
             dns_settings: DnsSettings::new()?,
         })
diff --git a/talpid-core/src/firewall/macos/mod.rs b/talpid-core/src/firewall/macos/mod.rs
index 5cacf9a2dc..394fd916d3 100644
--- a/talpid-core/src/firewall/macos/mod.rs
+++ b/talpid-core/src/firewall/macos/mod.rs
@@ -5,6 +5,7 @@ use self::pfctl::ipnetwork::{IpNetwork, Ipv4Network};
 use super::{Firewall, SecurityPolicy};
 
 use std::net::Ipv4Addr;
+use std::path::Path;
 
 use talpid_types::net;
 
@@ -32,7 +33,7 @@ pub struct PacketFilter {
 impl Firewall for PacketFilter {
     type Error = Error;
 
-    fn new() -> Result<Self> {
+    fn new<P: AsRef<Path>>(_cache_dir: P) -> Result<Self> {
         Ok(PacketFilter {
             pf: pfctl::PfCtl::new()?,
             pf_was_enabled: None,
diff --git a/talpid-core/src/firewall/mod.rs b/talpid-core/src/firewall/mod.rs
index 6b2a073d40..2902e24ccd 100644
--- a/talpid-core/src/firewall/mod.rs
+++ b/talpid-core/src/firewall/mod.rs
@@ -1,7 +1,6 @@
+use std::path::Path;
 use talpid_types::net::Endpoint;
 
-mod system_state;
-
 
 /// A enum that describes firewall rules strategy
 #[derive(Debug, Clone, Eq, PartialEq)]
@@ -31,7 +30,7 @@ pub trait Firewall {
     type Error: ::std::error::Error;
 
     /// Create new instance of Firewall
-    fn new() -> ::std::result::Result<Self, Self::Error>
+    fn new<P: AsRef<Path>>(cache_dir: P) -> ::std::result::Result<Self, Self::Error>
     where
         Self: Sized;
 
diff --git a/talpid-core/src/firewall/windows/dns.rs b/talpid-core/src/firewall/windows/dns.rs
new file mode 100644
index 0000000000..73593e7ccb
--- /dev/null
+++ b/talpid-core/src/firewall/windows/dns.rs
@@ -0,0 +1,217 @@
+extern crate widestring;
+
+use super::ffi;
+use super::system_state::SystemStateWriter;
+
+use self::widestring::WideCString;
+use libc;
+use std::net::IpAddr;
+use std::path::Path;
+use std::ptr;
+use std::slice;
+
+const DNS_STATE_FILENAME: &'static str = "dns-state-backup";
+
+error_chain!{
+    errors{
+        /// Failure to initialize WinDns
+        Initialization{
+            description("Failed to initialize WinDns")
+        }
+
+        /// Failure to deinitialize WinDns
+        Deinitialization{
+            description("Failed to deinitialize WinDns")
+        }
+
+        /// Failure to set new DNS servers
+        Setting{
+            description("Failed to set new DNS servers")
+        }
+
+        /// Failure to reset DNS settings
+        Resetting{
+            description("Failed to reset DNS")
+        }
+
+        /// Failure to reset DNS settings from backup
+        Recovery{
+            description("Failed to recover to backed up system state")
+        }
+    }
+
+    foreign_links {
+        Io(::std::io::Error) #[doc = "IO error, most probably occurs when reading system state backup"];
+    }
+}
+
+pub struct WinDns {
+    backup_writer: SystemStateWriter,
+}
+
+impl WinDns {
+    pub fn new<P: AsRef<Path>>(cache_dir: P) -> Result<Self> {
+        unsafe { WinDns_Initialize(Some(ffi::error_sink), ptr::null_mut()).into_result()? };
+
+        let backup_writer = SystemStateWriter::new(
+            cache_dir
+                .as_ref()
+                .join(DNS_STATE_FILENAME)
+                .into_boxed_path(),
+        );
+        let mut dns = WinDns { backup_writer };
+        dns.restore_system_backup()?;
+        Ok(dns)
+    }
+
+    pub fn set_dns(&mut self, servers: &[IpAddr]) -> Result<()> {
+        info!(
+            "Setting DNS servers - {}",
+            servers
+                .iter()
+                .map(|ip| ip.to_string())
+                .collect::<Vec<String>>()
+                .join(", ")
+        );
+        let widestring_ips = servers
+            .iter()
+            .map(|ip| ip.to_string().encode_utf16().collect::<Vec<_>>())
+            .map(|ip| WideCString::new(ip).unwrap())
+            .collect::<Vec<_>>();
+
+        let mut ip_ptrs = widestring_ips
+            .iter()
+            .map(|ip_cstr| ip_cstr.as_ptr())
+            .collect::<Vec<_>>();
+
+        unsafe {
+            WinDns_Set(
+                ip_ptrs.as_mut_ptr(),
+                widestring_ips.len() as u32,
+                Some(write_system_state_backup_cb),
+                &self.backup_writer as *const _ as *const libc::c_void,
+            ).into_result()
+        }
+    }
+
+    pub fn reset_dns(&mut self) -> Result<()> {
+        trace!("Resetting DNS");
+        unsafe { WinDns_Reset().into_result()? };
+
+        if let Err(e) = self.backup_writer.remove_backup() {
+            warn!("Failed to remove DNS state backup file: {}", e);
+        }
+        Ok(())
+    }
+
+    fn restore_dns_settings(&mut self, data: &[u8]) -> Result<()> {
+        unsafe { WinDns_Recover(data.as_ptr(), data.len() as u32) }.into_result()
+    }
+
+    fn restore_system_backup(&mut self) -> Result<()> {
+        if let Some(previous_state) = self.backup_writer.read_backup()? {
+            info!("Restoring DNS state from backup");
+            self.restore_dns_settings(&previous_state)?;
+            trace!("Successfully restored DNS state");
+            if let Err(e) = self.backup_writer.remove_backup() {
+                error!(
+                    "Failed to remove DNS config backup after restoring it: {}",
+                    e
+                );
+            }
+            return Ok(());
+        }
+        trace!("No dns state to restore");
+        Ok(())
+    }
+}
+
+impl Drop for WinDns {
+    fn drop(&mut self) {
+        if unsafe { WinDns_Deinitialize().into_result().is_ok() } {
+            trace!("Successfully deinitialized WinDns");
+        } else {
+            error!("Failed to deinitialize WinDns");
+        }
+    }
+}
+
+
+ffi_error!(InitializationResult, ErrorKind::Initialization.into());
+ffi_error!(DeinitializationResult, ErrorKind::Deinitialization.into());
+ffi_error!(SettingResult, ErrorKind::Setting.into());
+ffi_error!(ResettingResult, ErrorKind::Resetting.into());
+ffi_error!(RecoveringResult, ErrorKind::Recovery.into());
+
+
+/// A callback for writing system state data
+pub extern "system" fn write_system_state_backup_cb(
+    blob: *const u8,
+    length: u32,
+    state_writer_ptr: *mut libc::c_void,
+) -> i32 {
+    let state_writer = state_writer_ptr as *mut SystemStateWriter;
+    if state_writer.is_null() {
+        error!("State writer pointer is null, can't save system state backup");
+        return -1;
+    }
+
+    unsafe {
+        trace!(
+            "Writing {} bytes to store system state backup to {}",
+            length,
+            (*state_writer).backup_path.to_string_lossy()
+        );
+        let data = slice::from_raw_parts(blob, length as usize);
+        match (*state_writer).write_backup(data) {
+            Ok(()) => 0,
+            Err(e) => {
+                error!(
+                    "Failed to write system state backup to {} because {}",
+                    (*state_writer).backup_path.to_string_lossy(),
+                    e
+                );
+                e.raw_os_error().unwrap_or(-1)
+            }
+        }
+    }
+}
+
+
+type DNSConfigSink =
+    extern "system" fn(data: *const u8, length: u32, state_writer: *mut libc::c_void) -> i32;
+
+#[allow(non_snake_case)]
+extern "system" {
+
+    #[link_name(WinDns_Initialize)]
+    pub fn WinDns_Initialize(
+        sink: Option<ffi::ErrorSink>,
+        sink_context: *mut libc::c_void,
+    ) -> InitializationResult;
+
+    // WinDns_Deinitialize:
+    //
+    // Call this function once before unloading WINDNS or exiting the process.
+    #[link_name(WinDns_Deinitialize)]
+    pub fn WinDns_Deinitialize() -> DeinitializationResult;
+
+    // Configure which DNS servers should be used and start enforcing these settings.
+    #[link_name(WinDns_Set)]
+    pub fn WinDns_Set(
+        ips: *mut *const u16,
+        n_ips: u32,
+        callback: Option<DNSConfigSink>,
+        backup_writer: *const libc::c_void,
+    ) -> SettingResult;
+
+    // Revert server settings to what they were before calling WinDns_Set.
+    //
+    // (Also taking into account external changes to DNS settings that have ocurred
+    // during the period of enforcing specific settings.)
+    #[link_name(WinDns_Reset)]
+    pub fn WinDns_Reset() -> ResettingResult;
+
+    #[link_name(WinDns_Recover)]
+    pub fn WinDns_Recover(data: *const u8, length: u32) -> RecoveringResult;
+}
diff --git a/talpid-core/src/firewall/windows/ffi.rs b/talpid-core/src/firewall/windows/ffi.rs
new file mode 100644
index 0000000000..029989359e
--- /dev/null
+++ b/talpid-core/src/firewall/windows/ffi.rs
@@ -0,0 +1,38 @@
+use libc::{c_char, c_void};
+
+pub type ErrorSink = extern "system" fn(msg: *const c_char, ctx: *mut c_void);
+
+pub extern "system" fn error_sink(msg: *const c_char, _ctx: *mut c_void) {
+    use std::ffi::CStr;
+    if msg.is_null() {
+        error!("Log message from FFI boundary is NULL");
+    } else {
+        error!("{}", unsafe { CStr::from_ptr(msg).to_string_lossy() });
+    }
+}
+
+#[macro_export]
+macro_rules! ffi_error {
+    ($result:ident, $error:expr) => {
+        #[repr(C)]
+        #[derive(Debug)]
+        pub struct $result {
+            success: bool,
+        }
+
+        impl $result {
+            pub fn into_result(self) -> Result<()> {
+                match self.success {
+                    true => Ok(()),
+                    false => Err($error),
+                }
+            }
+        }
+
+        impl Into<Result<()>> for $result {
+            fn into(self) -> Result<()> {
+                self.into_result()
+            }
+        }
+    };
+}
diff --git a/talpid-core/src/firewall/windows/mod.rs b/talpid-core/src/firewall/windows/mod.rs
index f85dc090ca..01c6dd4c53 100644
--- a/talpid-core/src/firewall/windows/mod.rs
+++ b/talpid-core/src/firewall/windows/mod.rs
@@ -2,40 +2,76 @@ extern crate widestring;
 
 use super::{Firewall, SecurityPolicy};
 use std::net::IpAddr;
+use std::path::Path;
 use std::ptr;
 
-use self::ffi::*;
+use self::winfw::*;
 use talpid_types::net::Endpoint;
 
 use self::widestring::WideCString;
 
+
+#[macro_use]
+mod ffi;
+mod dns;
+mod system_state;
+
+use self::dns::WinDns;
+
 error_chain!{
     errors{
-        #[doc = "Windows firewall module error"]
-        WinFwFailure(desc: &'static str){
-            description("Opaque WinFw failure")
-            display("WinFw failed when {}", desc)
+        /// Failure to initialize windows firewall module
+        Initialization{
+            description("Failed to initialise windows firewall module")
+        }
+
+        /// Failure to deinitialize windows firewall module
+        Deinitialization{
+            description("Failed to deinitialize windows firewall module")
+        }
+
+        /// Failure to apply a firewall _connected_ policy
+        ApplyingConnectedPolicy{
+            description("Failed to apply firewall policy for when the daemon is connecting to a tunnel")
+        }
+
+        /// Failure to apply a firewall _connecting_ policy
+        ApplyingConnectingPolicy{
+            description("Failed to apply firewall policy for when the daemon is connected to a tunnel")
+        }
+
+        /// Failure to reset firewall policies
+        ResettingPolicy{
+            description("Failed to reset firewall policies")
         }
     }
+
+    links {
+        WinDns(dns::Error, dns::ErrorKind) #[doc = "WinDNS failure"];
+    }
 }
 
 const WINFW_TIMEOUT_SECONDS: u32 = 2;
 
 /// The Windows implementation for the `Firewall` trait.
 pub struct WindowsFirewall {
-    _unused: [u8; 0],
+    dns: WinDns,
 }
 
 impl Firewall for WindowsFirewall {
     type Error = Error;
 
-    fn new() -> Result<Self> {
-        let ok =
-            unsafe { WinFw_Initialize(WINFW_TIMEOUT_SECONDS, Some(error_sink), ptr::null_mut()) };
-        ok.into_result("initialize WinFw").map(|_| {
-            trace!("Successfully initialized WinFw");
-            WindowsFirewall { _unused: [] }
-        })
+    fn new<P: AsRef<Path>>(cache_dir: P) -> Result<Self> {
+        let windns = WinDns::new(cache_dir)?;
+        unsafe {
+            WinFw_Initialize(
+                WINFW_TIMEOUT_SECONDS,
+                Some(ffi::error_sink),
+                ptr::null_mut(),
+            ).into_result()?
+        };
+        trace!("Successfully initialized windows firewall module");
+        Ok(WindowsFirewall { dns: windns })
     }
 
     fn apply_policy(&mut self, policy: SecurityPolicy) -> Result<()> {
@@ -60,17 +96,18 @@ impl Firewall for WindowsFirewall {
 
     fn reset_policy(&mut self) -> Result<()> {
         trace!("Resetting firewall policy");
-        let ok = unsafe { WinFw_Reset() };
-        ok.into_result("resetting firewall")
+        self.dns.reset_dns()?;
+        unsafe { WinFw_Reset().into_result() }?;
+        Ok(())
     }
 }
 
 impl Drop for WindowsFirewall {
     fn drop(&mut self) {
-        if unsafe { WinFw_Deinitialize().is_ok() } {
-            trace!("Successfully deinitialized WinFw");
+        if unsafe { WinFw_Deinitialize().into_result().is_ok() } {
+            trace!("Successfully deinitialized windows firewall module");
         } else {
-            error!("Failed to deinitialize WinFw");
+            error!("Failed to deinitialize windows firewall module");
         };
     }
 }
@@ -91,8 +128,7 @@ impl WindowsFirewall {
             protocol: WinFwProt::from(endpoint.protocol),
         };
 
-        let ok = unsafe { WinFw_ApplyPolicyConnecting(winfw_settings, &winfw_relay) };
-        ok.into_result("applying 'connecting' policy")
+        unsafe { WinFw_ApplyPolicyConnecting(winfw_settings, &winfw_relay).into_result() }
     }
 
     fn widestring_ip(ip: &IpAddr) -> WideCString {
@@ -120,58 +156,26 @@ impl WindowsFirewall {
             protocol: WinFwProt::from(endpoint.protocol),
         };
 
-        let ok = unsafe {
+        self.dns.set_dns(&vec![tunnel_metadata.gateway.into()])?;
+        unsafe {
             WinFw_ApplyPolicyConnected(
                 winfw_settings,
                 &winfw_relay,
                 tunnel_alias.as_wide_c_str().as_ptr(),
                 gateway_str.as_wide_c_str().as_ptr(),
-            )
-        };
-        ok.into_result("applying 'connected' policy")
+            ).into_result()
+        }
     }
 }
 
 
 #[allow(non_snake_case)]
-mod ffi {
-
-    extern crate libc;
-    use super::{ErrorKind, Result};
-    use std::ffi::CStr;
-    use std::os::raw::c_char;
-    use std::ptr;
+mod winfw {
+    use super::{ffi, ErrorKind, Result};
+    use libc;
     use talpid_types::net::TransportProtocol;
 
     #[repr(C)]
-    pub struct WinFwResult {
-        ok: bool,
-    }
-
-    impl WinFwResult {
-        pub fn into_result(self, description: &'static str) -> Result<()> {
-            match self.ok {
-                true => Ok(()),
-                false => Err(ErrorKind::WinFwFailure(description).into()),
-            }
-        }
-
-        pub fn is_ok(&self) -> bool {
-            self.ok
-        }
-    }
-
-    pub type ErrorSink = extern "system" fn(msg: *const c_char, ctx: *mut libc::c_void);
-
-    pub extern "system" fn error_sink(msg: *const c_char, _ctx: *mut libc::c_void) {
-        if msg == ptr::null() {
-            error!("log message from WinFw is NULL");
-        } else {
-            error!("{}", unsafe { CStr::from_ptr(msg).to_string_lossy() });
-        }
-    }
-
-    #[repr(C)]
     pub struct WinFwRelay {
         pub ip: *const libc::wchar_t,
         pub port: u16,
@@ -209,22 +213,34 @@ mod ffi {
         }
     }
 
+    ffi_error!(InitializationResult, ErrorKind::Initialization.into());
+    ffi_error!(DeinitializationResult, ErrorKind::Deinitialization.into());
+    ffi_error!(
+        ApplyConnectedResult,
+        ErrorKind::ApplyingConnectedPolicy.into()
+    );
+    ffi_error!(
+        ApplyConnectingResult,
+        ErrorKind::ApplyingConnectingPolicy.into()
+    );
+    ffi_error!(ResettingPolicyResult, ErrorKind::ResettingPolicy.into());
+
     extern "system" {
         #[link_name(WinFw_Initialize)]
         pub fn WinFw_Initialize(
             timeout: libc::c_uint,
-            sink: Option<ErrorSink>,
+            sink: Option<ffi::ErrorSink>,
             sink_context: *mut libc::c_void,
-        ) -> WinFwResult;
+        ) -> InitializationResult;
 
         #[link_name(WinFw_Deinitialize)]
-        pub fn WinFw_Deinitialize() -> WinFwResult;
+        pub fn WinFw_Deinitialize() -> DeinitializationResult;
 
         #[link_name(WinFw_ApplyPolicyConnecting)]
         pub fn WinFw_ApplyPolicyConnecting(
             settings: &WinFwSettings,
             relay: &WinFwRelay,
-        ) -> WinFwResult;
+        ) -> ApplyConnectingResult;
 
         #[link_name(WinFw_ApplyPolicyConnected)]
         pub fn WinFw_ApplyPolicyConnected(
@@ -232,9 +248,9 @@ mod ffi {
             relay: &WinFwRelay,
             tunnelIfaceAlias: *const libc::wchar_t,
             primaryDns: *const libc::wchar_t,
-        ) -> WinFwResult;
+        ) -> ApplyConnectedResult;
 
         #[link_name(WinFw_Reset)]
-        pub fn WinFw_Reset() -> WinFwResult;
+        pub fn WinFw_Reset() -> ResettingPolicyResult;
     }
 }
diff --git a/talpid-core/src/firewall/system_state.rs b/talpid-core/src/firewall/windows/system_state.rs
index 18782d91cf..4695c13bc4 100644
--- a/talpid-core/src/firewall/system_state.rs
+++ b/talpid-core/src/firewall/windows/system_state.rs
@@ -3,10 +3,7 @@
 //! reboots
 use std::fs;
 use std::io;
-use std::path::{Path, PathBuf};
-use std::slice;
-
-const STATE_BACKUP_FILENAME: &str = "system_state_backup";
+use std::path::Path;
 
 /// This struct is responsible for saving a binary blob to disk. The binary blob is intended to
 /// store system state that should be resotred when the security policy is reset.
@@ -18,12 +15,10 @@ pub struct SystemStateWriter {
 impl SystemStateWriter {
     /// Creates a new SystemStateWriter which will use a file in the cache directory to store system
     /// state that has to be restored.
-    pub fn new<P: AsRef<Path>>(cache_dir: P) -> Self {
-        let backup_path = cache_dir
-            .as_ref()
-            .join(STATE_BACKUP_FILENAME)
-            .into_boxed_path();
-        Self { backup_path }
+    pub fn new<P: AsRef<Path>>(backup_path: P) -> Self {
+        Self {
+            backup_path: backup_path.as_ref().to_owned().into_boxed_path(),
+        }
     }
 
     /// Writes a binary blob representing the system state to the backup location before any
@@ -32,15 +27,9 @@ impl SystemStateWriter {
         fs::write(&self.backup_path, &data)
     }
 
-    /// Tries to read a previously saved backup and deletes it after reading it if it exists.
-    pub fn consume_state_backup(&self) -> io::Result<Option<Vec<u8>>> {
-        match fs::read(&self.backup_path) {
-            Ok(blob) => {
-                if let Err(e) = self.remove_state_file() {
-                    error!("Failed to remove system state backup: {}", e)
-                };
-                Ok(Some(blob))
-            }
+    pub fn read_backup(&self) -> io::Result<Option<Vec<u8>>> {
+        match fs::read(&self.backup_path).map(|blob| Some(blob)) {
+            Ok(b) => Ok(b),
             Err(e) => match e.kind() {
                 io::ErrorKind::NotFound => Ok(None),
                 _ => Err(e),
@@ -48,8 +37,9 @@ impl SystemStateWriter {
         }
     }
 
+
     /// Removes a previously created state backup if it exists.
-    pub fn remove_state_file(&self) -> io::Result<()> {
+    pub fn remove_backup(&self) -> io::Result<()> {
         match fs::remove_file(&self.backup_path) {
             Err(e) => {
                 if e.kind() != io::ErrorKind::NotFound {
@@ -66,98 +56,65 @@ impl SystemStateWriter {
 #[cfg(test)]
 mod tests {
     extern crate tempfile;
+
     use super::*;
 
     #[test]
     fn can_create_backup() {
         let temp_dir = tempfile::tempdir().expect("failed to crate temp dir");
+        let temp_file = temp_dir.path().join("test_file");
 
         let mock_system_state: Vec<_> = b"8.8.8.8\n8.8.4.4\n".to_vec();
-        let writer = SystemStateWriter::new(&temp_dir);
+        let writer = SystemStateWriter::new(&temp_file);
         writer
             .write_backup(&mock_system_state)
             .expect("failed to write system state");
 
         let backup = writer
-            .consume_state_backup()
+            .read_backup()
             .expect("error when reading system state backup")
             .expect("expected to read system state backup");
         assert_eq!(backup, mock_system_state);
-
-        let empty_read = writer
-            .consume_state_backup()
-            .expect("error when reading system state backup");
-        assert_eq!(empty_read, None);
     }
 
     #[test]
     fn can_succeed_without_backup() {
         let temp_dir = tempfile::tempdir().expect("failed to crate temp dir");
+        let temp_file = temp_dir.path().join("test_file");
 
-        let writer = SystemStateWriter::new(&temp_dir);
+        let writer = SystemStateWriter::new(&temp_file);
         let backup = writer
-            .consume_state_backup()
+            .read_backup()
             .expect("error when reading system state backup");
         assert_eq!(backup, None);
     }
 
-    #[cfg(unix)]
-    #[test]
-    fn cant_read_without_access() {
-        let temp_dir = PathBuf::from("/dev/null");
-
-        let writer = SystemStateWriter::new(&temp_dir);
-        let mock_system_state: Vec<_> = b"8.8.8.8\n8.8.4.4\n".to_vec();
-
-        let failure = writer
-            .write_backup(&mock_system_state)
-            .expect_err("successfully wrote backup file to a directory in /dev/null");
-        assert_eq!(failure.kind(), io::ErrorKind::Other);
-
-        let recovery_failure = writer
-            .consume_state_backup()
-            .expect_err("successfully read backup file in /dev/null");
-        assert_eq!(recovery_failure.kind(), io::ErrorKind::Other);
-    }
-
     #[test]
     fn can_remove_when_no_backup_exists() {
         let temp_dir = tempfile::tempdir().expect("failed to crate temp dir");
+        let temp_file = temp_dir.path().join("test_file");
 
-        let writer = SystemStateWriter::new(&temp_dir);
-        writer.remove_state_file().expect(
-            "Encountered IO error when running remove_state_file when no state file exists",
-        );
+        let writer = SystemStateWriter::new(&temp_file);
+        writer
+            .remove_backup()
+            .expect("Encountered IO error when running remove_backup when no state file exists");
     }
 
     #[test]
     fn can_remove_backup() {
         let temp_dir = tempfile::tempdir().expect("failed to crate temp dir");
-        let writer = SystemStateWriter::new(&temp_dir);
+        let temp_file = temp_dir.path().join("test_file");
+        let writer = SystemStateWriter::new(&temp_file);
         let mock_system_state = b"8.8.8.8\n8.8.4.4\n".to_vec();
 
         writer
             .write_backup(&mock_system_state)
             .expect("Failed to write backup");
-        writer
-            .remove_state_file()
-            .expect("Failed to remove state file");
+        writer.remove_backup().expect("Failed to remove state file");
 
         let empty_backup = writer
-            .consume_state_backup()
+            .read_backup()
             .expect("Encountered IO error when no backup file exists");
         assert_eq!(empty_backup, None);
     }
-
-    #[cfg(unix)]
-    #[test]
-    fn cant_remove_backup_with_io_error() {
-        let temp_dir = PathBuf::from("/dev/null");
-
-        let writer = SystemStateWriter::new(&temp_dir);
-        let removal_failure = writer
-            .remove_state_file()
-            .expect_err("successfully removed state file in /dev/null");
-        assert_eq!(removal_failure.kind(), io::ErrorKind::Other);
-    }
 }
diff --git a/talpid-core/src/lib.rs b/talpid-core/src/lib.rs
index a005cd6b8d..ea8e9dd890 100644
--- a/talpid-core/src/lib.rs
+++ b/talpid-core/src/lib.rs
@@ -27,6 +27,9 @@ extern crate openvpn_plugin;
 extern crate talpid_ipc;
 extern crate talpid_types;
 
+#[cfg(windows)]
+extern crate libc;
+
 /// Working with processes.
 pub mod process;