Merge branch 'safer-user-pass-file-removal'

5 files changed, 74 insertions, 32 deletions

diff --git a/Cargo.lock b/Cargo.lock
index a9adae914f..f48e1f73d6 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -641,14 +641,6 @@ dependencies = [
 ]
 
 [[package]]
-name = "mktemp"
-version = "0.3.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "uuid 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
 name = "mullvad-cli"
 version = "0.1.0"
 dependencies = [
@@ -1197,10 +1189,10 @@ dependencies = [
  "lazy_static 0.2.8 (registry+https://github.com/rust-lang/crates.io-index)",
  "libc 0.2.29 (registry+https://github.com/rust-lang/crates.io-index)",
  "log 0.3.8 (registry+https://github.com/rust-lang/crates.io-index)",
- "mktemp 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)",
  "openvpn-plugin 0.2.0 (registry+https://github.com/rust-lang/crates.io-index)",
  "pfctl 0.1.0 (git+https://github.com/mullvad/pfctl-rs.git)",
  "talpid-ipc 0.1.0",
+ "uuid 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
 ]
 
 [[package]]
@@ -1416,15 +1408,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 
 [[package]]
 name = "uuid"
-version = "0.1.18"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "rand 0.3.15 (registry+https://github.com/rust-lang/crates.io-index)",
- "rustc-serialize 0.3.24 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
-[[package]]
-name = "uuid"
 version = "0.5.1"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 dependencies = [
@@ -1559,7 +1542,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum mime 0.3.3 (registry+https://github.com/rust-lang/crates.io-index)" = "153f98dde2b135dece079e5478ee400ae1bab13afa52d66590eacfc40e912435"
 "checksum mio 0.6.9 (registry+https://github.com/rust-lang/crates.io-index)" = "9e965267d4d58496fc4f740e9861118367f13570cadf66316ed2c3f2f14d87c7"
 "checksum miow 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)" = "8c1f2f3b1cf331de6896aabf6e9d55dca90356cc9960cca7eaaf408a355ae919"
-"checksum mktemp 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)" = "77001ceb9eed65439f3dc2a2543f9ba1417d912686bf224a7738d0966e6dcd69"
 "checksum native-tls 0.1.4 (registry+https://github.com/rust-lang/crates.io-index)" = "04b781c9134a954c84f0594b9ab3f5606abc516030388e8511887ef4c204a1e5"
 "checksum net2 0.2.29 (registry+https://github.com/rust-lang/crates.io-index)" = "bc01404e7568680f1259aa5729539f221cb1e6d047a0d9053cab4be8a73b5d67"
 "checksum nix 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)" = "47e49f6982987135c5e9620ab317623e723bd06738fd85377e8d55f57c8b6487"
@@ -1640,7 +1622,6 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 "checksum unreachable 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "382810877fe448991dfc7f0dd6e3ae5d58088fd0ea5e35189655f84e6814fa56"
 "checksum url 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "eeb819346883532a271eb626deb43c4a1bb4c4dd47c519bd78137c3e72a4fe27"
 "checksum utf8-ranges 1.0.0 (registry+https://github.com/rust-lang/crates.io-index)" = "662fab6525a98beff2921d7f61a39e7d59e0b425ebc7d0d9e66d316e55124122"
-"checksum uuid 0.1.18 (registry+https://github.com/rust-lang/crates.io-index)" = "78c590b5bd79ed10aad8fb75f078a59d8db445af6c743e55c4a53227fc01c13f"
 "checksum uuid 0.5.1 (registry+https://github.com/rust-lang/crates.io-index)" = "bcc7e3b898aa6f6c08e5295b6c89258d1331e9ac578cc992fb818759951bdc22"
 "checksum vcpkg 0.2.2 (registry+https://github.com/rust-lang/crates.io-index)" = "9e0a7d8bed3178a8fb112199d466eeca9ed09a14ba8ad67718179b4fd5487d0b"
 "checksum vec_map 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "887b5b631c2ad01628bbbaa7dd4c869f80d3186688f8d0b6f58774fbe324988c"
diff --git a/talpid-core/Cargo.toml b/talpid-core/Cargo.toml
index 5dcb634b0d..3d93e80713 100644
--- a/talpid-core/Cargo.toml
+++ b/talpid-core/Cargo.toml
@@ -11,7 +11,7 @@ jsonrpc-core = { git = "https://github.com/paritytech/jsonrpc", tag = "v7.1.0" }
 jsonrpc-macros = { git = "https://github.com/paritytech/jsonrpc", tag = "v7.1.0" }
 lazy_static = "0.2"
 log = "0.3"
-mktemp = "0.3"
+uuid = { version = "0.5", features = ["v4"] }
 
 openvpn-plugin = { version = "0.2", features = ["serialize"] }
 talpid-ipc = { path = "../talpid-ipc" }
diff --git a/talpid-core/src/lib.rs b/talpid-core/src/lib.rs
index cbdc08886e..dc37401ebf 100644
--- a/talpid-core/src/lib.rs
+++ b/talpid-core/src/lib.rs
@@ -12,13 +12,13 @@ extern crate duct;
 extern crate lazy_static;
 #[macro_use]
 extern crate log;
-extern crate mktemp;
 
 #[macro_use]
 extern crate error_chain;
 extern crate jsonrpc_core;
 #[macro_use]
 extern crate jsonrpc_macros;
+extern crate uuid;
 
 extern crate talpid_ipc;
 extern crate openvpn_plugin;
@@ -40,3 +40,5 @@ pub mod mpsc;
 
 /// Abstractions over different firewalls
 pub mod firewall;
+
+mod mktemp;
diff --git a/talpid-core/src/mktemp.rs b/talpid-core/src/mktemp.rs
new file mode 100644
index 0000000000..e8ba33c3c5
--- /dev/null
+++ b/talpid-core/src/mktemp.rs
@@ -0,0 +1,46 @@
+use std::env;
+use std::fs;
+use std::io;
+use std::path::{Path, PathBuf};
+
+use uuid::Uuid;
+
+#[derive(Debug)]
+pub struct TempFile {
+    path: PathBuf,
+}
+
+impl TempFile {
+    /// Create a new unique `TempFile`. The file will not exist after this.
+    pub fn new() -> Self {
+        TempFile { path: generate_path() }
+    }
+
+    pub fn to_path_buf(&self) -> PathBuf {
+        self.path.clone()
+    }
+}
+
+impl AsRef<Path> for TempFile {
+    fn as_ref(&self) -> &Path {
+        &self.path.as_path()
+    }
+}
+
+impl Drop for TempFile {
+    fn drop(&mut self) {
+        if let Err(e) = fs::remove_file(&self.path) {
+            if e.kind() != io::ErrorKind::NotFound {
+                error!(
+                    "Unable to remove temp file {}: {:?}",
+                    self.path.to_string_lossy(),
+                    e
+                );
+            }
+        }
+    }
+}
+
+fn generate_path() -> PathBuf {
+    env::temp_dir().join(Uuid::new_v4().to_string())
+}
diff --git a/talpid-core/src/tunnel/mod.rs b/talpid-core/src/tunnel/mod.rs
index b8d91c567e..489fdc8a94 100644
--- a/talpid-core/src/tunnel/mod.rs
+++ b/talpid-core/src/tunnel/mod.rs
@@ -1,7 +1,10 @@
 use mktemp;
 use net;
+
 use openvpn_plugin::types::OpenVpnPluginEvent;
+
 use process::openvpn::OpenVpnCommand;
+
 use std::env;
 use std::ffi::{OsStr, OsString};
 use std::fs;
@@ -63,7 +66,8 @@ impl TunnelEvent {
 /// Abstraction for monitoring a generic VPN tunnel.
 pub struct TunnelMonitor {
     monitor: OpenVpnMonitor,
-    _user_pass_file: mktemp::Temp,
+    /// Keep the `TempFile` for the user-pass file in the struct, so it's removed on drop.
+    _user_pass_file: mktemp::TempFile,
 }
 
 impl TunnelMonitor {
@@ -72,13 +76,22 @@ impl TunnelMonitor {
     pub fn new<L>(remote: net::Endpoint, account_token: &str, on_event: L) -> Result<Self>
         where L: Fn(TunnelEvent) + Send + Sync + 'static
     {
-        let on_openvpn_event = move |event, _env| match TunnelEvent::from_openvpn_event(&event) {
-            Some(tunnel_event) => on_event(tunnel_event),
-            None => debug!("Ignoring OpenVpnEvent {:?}", event),
-        };
         let user_pass_file = Self::create_user_pass_file(account_token)
             .chain_err(|| ErrorKind::CredentialsWriteError)?;
         let cmd = Self::create_openvpn_cmd(remote, user_pass_file.as_ref());
+        let user_pass_file_path = user_pass_file.to_path_buf();
+
+        let on_openvpn_event = move |event, _env| {
+            if event == OpenVpnPluginEvent::Up {
+                // The user-pass file has been read. Try to delete it early.
+                let _ = fs::remove_file(&user_pass_file_path);
+            }
+            match TunnelEvent::from_openvpn_event(&event) {
+                Some(tunnel_event) => on_event(tunnel_event),
+                None => debug!("Ignoring OpenVpnEvent {:?}", event),
+            }
+        };
+
         let monitor = openvpn::OpenVpnMonitor::new(cmd, on_openvpn_event, Self::get_plugin_path()?)
             .chain_err(|| ErrorKind::TunnelMonitoringError)?;
         Ok(
@@ -168,16 +181,16 @@ impl TunnelMonitor {
         }
     }
 
-    fn create_user_pass_file(account_token: &str) -> io::Result<mktemp::Temp> {
-        let path = mktemp::Temp::new_file()?;
+    fn create_user_pass_file(account_token: &str) -> io::Result<mktemp::TempFile> {
+        let temp_file = mktemp::TempFile::new();
         debug!(
             "Writing user-pass credentials to {}",
-            path.as_ref().to_string_lossy()
+            temp_file.as_ref().to_string_lossy()
         );
-        let mut file = fs::File::create(&path)?;
+        let mut file = fs::File::create(&temp_file)?;
         Self::set_user_pass_file_permissions(&file)?;
         write!(file, "{}\n-\n", account_token)?;
-        Ok(path)
+        Ok(temp_file)
     }
 
     #[cfg(unix)]