Merge branch 'improve-rpc-file-permission-error-message'

2 files changed, 32 insertions, 20 deletions

diff --git a/mullvad-cli/src/main.rs b/mullvad-cli/src/main.rs
index e3475888fa..3e3bf2d62e 100644
--- a/mullvad-cli/src/main.rs
+++ b/mullvad-cli/src/main.rs
@@ -24,7 +24,13 @@ mod rpc;
 mod cmds;
 
 
-error_chain!{}
+use std::io;
+
+error_chain! {
+    foreign_links {
+        Io(io::Error);
+    }
+}
 
 quick_main!(run);
 
diff --git a/mullvad-cli/src/rpc.rs b/mullvad-cli/src/rpc.rs
index 00c3a53b48..2e1da7e528 100644
--- a/mullvad-cli/src/rpc.rs
+++ b/mullvad-cli/src/rpc.rs
@@ -2,7 +2,7 @@ use {Result, ResultExt};
 use serde;
 
 use std::fs::{File, Metadata};
-use std::io::{self, BufRead, BufReader};
+use std::io::{BufRead, BufReader};
 #[cfg(unix)]
 use std::path::Path;
 use std::path::PathBuf;
@@ -43,39 +43,45 @@ lazy_static! {
     static ref RPC_ADDRESS_FILE_PATH: PathBuf = ::std::env::temp_dir().join(".mullvad_rpc_address");
 }
 
-fn read_rpc_address() -> io::Result<(String, String)> {
+fn read_rpc_address() -> Result<(String, String)> {
+    let file = open_rpc_file()?;
+    let mut buf_file = BufReader::new(file);
+    let mut address = String::new();
+    buf_file.read_line(&mut address)?;
+    let mut shared_secret = String::new();
+    buf_file.read_line(&mut shared_secret)?;
+    Ok((address, shared_secret))
+}
+
+fn open_rpc_file() -> Result<File> {
     debug!(
         "Trying to read RPC address at {}",
         RPC_ADDRESS_FILE_PATH.to_string_lossy()
     );
     let file = File::open(&*RPC_ADDRESS_FILE_PATH)?;
-    if is_rpc_file_trusted(file.metadata()?) {
-        let mut buf_file = BufReader::new(file);
-        let mut address = String::new();
-        buf_file.read_line(&mut address)?;
-        let mut shared_secret = String::new();
-        buf_file.read_line(&mut shared_secret)?;
-        Ok((address, shared_secret))
-    } else {
-        Err(io::Error::new(
-            io::ErrorKind::Other,
-            "RPC file is not trusted",
-        ))
-    }
+    ensure_written_by_admin(file.metadata()?).chain_err(|| "Refusing to trust RPC file")?;
+
+    Ok(file)
 }
 
 #[cfg(unix)]
-fn is_rpc_file_trusted(metadata: Metadata) -> bool {
+fn ensure_written_by_admin(metadata: Metadata) -> Result<()> {
     use std::os::unix::fs::MetadataExt;
 
     let is_owned_by_root = metadata.uid() == 0;
     let is_read_only_by_non_owner = (metadata.mode() & 0o022) == 0;
 
-    is_owned_by_root && is_read_only_by_non_owner
+    ensure!(is_owned_by_root, "RPC file is not owned by root");
+    ensure!(
+        is_read_only_by_non_owner,
+        "RPC file is writable by non-root users"
+    );
+
+    Ok(())
 }
 
 #[cfg(windows)]
-fn is_rpc_file_trusted(_metadata: Metadata) -> bool {
+fn ensure_written_by_admin(_metadata: Metadata) -> Result<()> {
     // TODO: Check permissions correctly
-    true
+    Ok(())
 }