Merge branch 'remove-disable-auth'

6 files changed, 17 insertions, 47 deletions

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 94bb2b60d6..53185757ed 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -26,7 +26,6 @@ Line wrap the file at 100 chars.                                              Th
 ### Added
 - Add `tunnel` subcommand to manage tunnel specific options in the CLI.
 - Add support for passing the `--mssfix` argument to OpenVPN tunnels.
-- Add `--disable-rpc-auth` flag to daemon to make it accept unauthorized control.
 - Add colors to terminal output on macOS and Linux.
 - Add details to mullvad CLI interface error for when it doesn't trust the RPC file.
 - Warn if daemon is running as a non-root user.
@@ -61,8 +60,6 @@ Line wrap the file at 100 chars.                                              Th
 - Fix OpenVPN warning about usage of AES-256-CBC cipher.
 - Fix "Out of time" screen status icon position.
 - Fix log newline characters on Windows.
-- Mullvad CLI can now be used with daemon instance that doesn't have the `--disable-rpc-auth`
-  flag set.
 - If necessary, create parent directories for RPC connection info file.
 
 
diff --git a/mullvad-daemon/src/cli.rs b/mullvad-daemon/src/cli.rs
index f70d69f947..4d424a31f2 100644
--- a/mullvad-daemon/src/cli.rs
+++ b/mullvad-daemon/src/cli.rs
@@ -10,7 +10,6 @@ pub struct Config {
     pub log_file: Option<PathBuf>,
     pub tunnel_log_file: Option<PathBuf>,
     pub resource_dir: Option<PathBuf>,
-    pub require_auth: bool,
     pub log_stdout_timestamps: bool,
     pub run_as_service: bool,
     pub register_service: bool,
@@ -28,7 +27,6 @@ pub fn get_config() -> Config {
     let log_file = matches.value_of_os("log_file").map(PathBuf::from);
     let tunnel_log_file = matches.value_of_os("tunnel_log_file").map(PathBuf::from);
     let resource_dir = matches.value_of_os("resource_dir").map(PathBuf::from);
-    let require_auth = !matches.is_present("disable_rpc_auth");
     let log_stdout_timestamps = !matches.is_present("disable_stdout_timestamps");
 
     let run_as_service = cfg!(windows) && matches.is_present("run_as_service");
@@ -39,7 +37,6 @@ pub fn get_config() -> Config {
         log_file,
         tunnel_log_file,
         resource_dir,
-        require_auth,
         log_stdout_timestamps,
         run_as_service,
         register_service,
@@ -79,11 +76,6 @@ fn create_app() -> App<'static, 'static> {
                 .help("Uses the given directory to read needed resources, such as certificates."),
         )
         .arg(
-            Arg::with_name("disable_rpc_auth")
-                .long("disable-rpc-auth")
-                .help("Don't require authentication on the RPC management interface."),
-        )
-        .arg(
             Arg::with_name("disable_stdout_timestamps")
             .long("disable-stdout-timestamps")
             .help("Don't log timestamps when logging to stdout, useful when running as a systemd service")
diff --git a/mullvad-daemon/src/main.rs b/mullvad-daemon/src/main.rs
index cdda2aa0bf..e6150f1373 100644
--- a/mullvad-daemon/src/main.rs
+++ b/mullvad-daemon/src/main.rs
@@ -217,11 +217,7 @@ struct Daemon {
 }
 
 impl Daemon {
-    pub fn new(
-        tunnel_log: Option<PathBuf>,
-        resource_dir: PathBuf,
-        require_auth: bool,
-    ) -> Result<Self> {
+    pub fn new(tunnel_log: Option<PathBuf>, resource_dir: PathBuf) -> Result<Self> {
         ensure!(
             !rpc_uniqueness_check::is_another_instance_running(),
             ErrorKind::DaemonIsAlreadyRunning
@@ -244,8 +240,7 @@ impl Daemon {
         let relay_selector = Self::create_relay_selector(rpc_handle.clone(), &resource_dir);
 
         let (tx, rx) = mpsc::channel();
-        let management_interface_broadcaster =
-            Self::start_management_interface(tx.clone(), require_auth)?;
+        let management_interface_broadcaster = Self::start_management_interface(tx.clone())?;
         let state = TunnelState::NotRunning;
         let target_state = TargetState::Unsecured;
         Ok(Daemon {
@@ -294,10 +289,9 @@ impl Daemon {
     // Returns a handle that allows notifying all subscribers on events.
     fn start_management_interface(
         event_tx: mpsc::Sender<DaemonEvent>,
-        require_auth: bool,
     ) -> Result<management_interface::EventBroadcaster> {
         let multiplex_event_tx = IntoSender::from(event_tx.clone());
-        let server = Self::start_management_interface_server(multiplex_event_tx, require_auth)?;
+        let server = Self::start_management_interface_server(multiplex_event_tx)?;
         let event_broadcaster = server.event_broadcaster();
         Self::spawn_management_interface_wait_thread(server, event_tx);
         Ok(event_broadcaster)
@@ -305,14 +299,8 @@ impl Daemon {
 
     fn start_management_interface_server(
         event_tx: IntoSender<TunnelCommand, DaemonEvent>,
-        require_auth: bool,
     ) -> Result<ManagementInterfaceServer> {
-        let shared_secret = if require_auth {
-            Some(uuid::Uuid::new_v4().to_string())
-        } else {
-            warn!("RPC management interface allows unauthorized access");
-            None
-        };
+        let shared_secret = uuid::Uuid::new_v4().to_string();
 
         let server = ManagementInterfaceServer::start(event_tx, shared_secret.clone())
             .chain_err(|| ErrorKind::ManagementInterfaceError("Failed to start server"))?;
@@ -321,8 +309,7 @@ impl Daemon {
             server.address()
         );
 
-        let written_shared_secret = shared_secret.unwrap_or(String::from(""));
-        rpc_address_file::write(server.address(), &written_shared_secret).chain_err(|| {
+        rpc_address_file::write(server.address(), &shared_secret).chain_err(|| {
             ErrorKind::ManagementInterfaceError("Failed to write RPC connection info to file")
         })?;
         Ok(server)
@@ -897,7 +884,7 @@ fn run_standalone(config: cli::Config) -> Result<()> {
     }
 
     let resource_dir = config.resource_dir.unwrap_or_else(|| get_resource_dir());
-    let daemon = Daemon::new(config.tunnel_log_file, resource_dir, config.require_auth)
+    let daemon = Daemon::new(config.tunnel_log_file, resource_dir)
         .chain_err(|| "Unable to initialize daemon")?;
 
     let shutdown_handle = daemon.shutdown_handle();
diff --git a/mullvad-daemon/src/management_interface.rs b/mullvad-daemon/src/management_interface.rs
index cd8f8f82e9..53922f5838 100644
--- a/mullvad-daemon/src/management_interface.rs
+++ b/mullvad-daemon/src/management_interface.rs
@@ -209,7 +209,7 @@ pub struct ManagementInterfaceServer {
 impl ManagementInterfaceServer {
     pub fn start<T>(
         tunnel_tx: IntoSender<TunnelCommand, T>,
-        shared_secret: Option<String>,
+        shared_secret: String,
     ) -> talpid_ipc::Result<Self>
     where
         T: From<TunnelCommand> + 'static + Send,
@@ -283,11 +283,11 @@ impl EventBroadcaster {
 struct ManagementInterface<T: From<TunnelCommand> + 'static + Send> {
     subscriptions: Arc<ActiveSubscriptions>,
     tx: Mutex<IntoSender<TunnelCommand, T>>,
-    shared_secret: Option<String>,
+    shared_secret: String,
 }
 
 impl<T: From<TunnelCommand> + 'static + Send> ManagementInterface<T> {
-    pub fn new(tx: IntoSender<TunnelCommand, T>, shared_secret: Option<String>) -> Self {
+    pub fn new(tx: IntoSender<TunnelCommand, T>, shared_secret: String) -> Self {
         ManagementInterface {
             subscriptions: Default::default(),
             tx: Mutex::new(tx),
@@ -357,7 +357,7 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterface<T> {
     }
 
     fn check_auth(&self, meta: &Meta) -> Result<(), Error> {
-        if self.shared_secret.is_none() || meta.authenticated.load(Ordering::SeqCst) {
+        if meta.authenticated.load(Ordering::SeqCst) {
             trace!("auth success");
             Ok(())
         } else {
@@ -383,18 +383,13 @@ impl<T: From<TunnelCommand> + 'static + Send> ManagementInterfaceApi for Managem
     type Metadata = Meta;
 
     fn auth(&self, meta: Self::Metadata, shared_secret: String) -> BoxFuture<(), Error> {
-        if let Some(ref self_shared_secret) = self.shared_secret {
-            let authenticated = &shared_secret == self_shared_secret;
-            meta.authenticated.store(authenticated, Ordering::SeqCst);
-            debug!("authenticated: {}", authenticated);
-            if authenticated {
-                Box::new(future::ok(()))
-            } else {
-                Box::new(future::err(Error::internal_error()))
-            }
-        } else {
-            warn!("Ignoring auth call since authentication is disabled");
+        let authenticated = shared_secret == self.shared_secret;
+        meta.authenticated.store(authenticated, Ordering::SeqCst);
+        debug!("authenticated: {}", authenticated);
+        if authenticated {
             Box::new(future::ok(()))
+        } else {
+            Box::new(future::err(Error::internal_error()))
         }
     }
 
diff --git a/mullvad-daemon/src/system_service.rs b/mullvad-daemon/src/system_service.rs
index 2f3c22f85e..0bee0cba24 100644
--- a/mullvad-daemon/src/system_service.rs
+++ b/mullvad-daemon/src/system_service.rs
@@ -73,7 +73,7 @@ fn run_service(_arguments: Vec<OsString>) -> Result<()> {
         .unwrap();
 
     let resource_dir = get_resource_dir();
-    let daemon = Daemon::new(config.tunnel_log_file, resource_dir, config.require_auth)
+    let daemon = Daemon::new(config.tunnel_log_file, resource_dir)
         .chain_err(|| "Unable to initialize daemon")?;
     let shutdown_handle = daemon.shutdown_handle();
 
diff --git a/mullvad-daemon/tests/common/mod.rs b/mullvad-daemon/tests/common/mod.rs
index 5356f4ee1e..b18998f432 100644
--- a/mullvad-daemon/tests/common/mod.rs
+++ b/mullvad-daemon/tests/common/mod.rs
@@ -45,7 +45,6 @@ impl DaemonRunner {
         let process = cmd!(
             DAEMON_EXECUTABLE_PATH,
             "-v",
-            "--disable-rpc-auth",
             "--resource-dir",
             "dist-assets"
         ).dir("..")