Skip to content

Commit

Permalink
Check correct app startup behavior when settings file is corrupt
Browse files Browse the repository at this point in the history
  • Loading branch information
MarkusPettersson98 committed Dec 7, 2023
1 parent bb48287 commit 5bbac7f
Show file tree
Hide file tree
Showing 6 changed files with 92 additions and 6 deletions.
8 changes: 4 additions & 4 deletions mullvad-daemon/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,18 +105,18 @@ impl SettingsPersister {

/// Loads user settings, returning default settings if it should fail.
///
/// `read` allows the caller to decide how to load [`Settings`] from an
/// bitrary resource.
/// `load_settings` allows the caller to decide how to load [`Settings`]
/// from an bitrary resource.
///
/// `load_inner` will always succeed, even in the presence of IO operations.
/// Errors are handled gracefully by returning the default [`Settings`] if
/// necessary.
async fn load_inner<F, R>(read: F) -> LoadSettingsResult
async fn load_inner<F, R>(load_settings: F) -> LoadSettingsResult
where
F: FnOnce() -> R,
R: std::future::Future<Output = Result<Settings, Error>>,
{
match read().await {
match load_settings().await {
Ok(settings) => LoadSettingsResult {
settings,
should_save: false,
Expand Down
65 changes: 65 additions & 0 deletions test/test-manager/src/tests/tunnel_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -386,3 +386,68 @@ pub async fn test_connecting_state_when_corrupted_state_cache(

Ok(())
}

/// Verify that the app enables "Always require VPN" by default if the settings
/// cannot be parsed. This reduces the number of errors that lead to the daemon
/// unexpectedly starting into non-blocking mode.
///
/// What this test covers that unit-tests don't is that no leaks should occur
/// after the app has started with a corrupt settings file.
#[test_function]
pub async fn test_connecting_state_corrupt_settings(
_: TestContext,
rpc: ServiceClient,
mullvad_client: ManagementServiceClient,
) -> Result<(), Error> {
// Sanity check that restarting the daemon with the default settings does
// not block internet connectivity.
log::info!("Restarting the daemon");
rpc.restart_mullvad_daemon().await?;
let default_interface = rpc.get_default_interface().await?;
let inet_destination = "1.1.1.1:1337".parse().unwrap();
let detected_probes =
send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await?;
assert!(
detected_probes.any(),
"Could not reach the internet in an unblocked state"
);

// Simply corrupt the settings file and try to start the app. The settings
// file will automatically be repaired and written to disk. As such, no
// extra clean up is needed for recovering the test runner to a known-good
// state.
log::info!("Stopping the app");
rpc.stop_mullvad_daemon().await?;
// Intentionally corrupt the settings file.
let settings = rpc
.find_mullvad_app_settings_dir()
.await?
.join("settings.json");
log::info!(
"Intentionally writing garbage to the settings {file}",
file = settings.display()
);
rpc.write_file(settings, "cookie was here".into()).await?;

// Start the app & make sure that the app sets "Always require VPN". The
// side-effect of this is that no network traffic should leak after the app
// has started until the user either connect to Mullvad or toggle this
// setting.
log::info!("Starting the app back up again");
rpc.start_mullvad_daemon().await?;
wait_for_tunnel_state(mullvad_client, |state| state.is_disconnected())
.await
.map_err(|err| {
log::error!("App did not start in an expected state. \
App is not in the `Disconnected` state after starting with a corrupt or missing settings file!");
err
})?;
log::info!("App successfully recovered from a corrupt settings file");

// Start monitoring for network leaks
let detected_probes =
send_guest_probes(rpc.clone(), default_interface.clone(), inet_destination).await?;
assert!(detected_probes.none(), "Observed unexpected packets");

Ok(())
}
9 changes: 8 additions & 1 deletion test/test-rpc/src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,20 @@ impl ServiceClient {
.await?
}

/// Returns path of Mullvad app cache directorie on the test runner.
/// Returns path of Mullvad app cache directory on the test runner.
pub async fn find_mullvad_app_cache_dir(&self) -> Result<PathBuf, Error> {
self.client
.get_mullvad_app_cache_dir(tarpc::context::current())
.await?
}

/// Returns path of Mullvad app settings directory on the test runner.
pub async fn find_mullvad_app_settings_dir(&self) -> Result<PathBuf, Error> {
self.client
.get_mullvad_app_settings_dir(tarpc::context::current())
.await?
}

/// Send TCP packet
pub async fn send_tcp(
&self,
Expand Down
2 changes: 2 additions & 0 deletions test/test-rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ mod service {

async fn get_mullvad_app_cache_dir() -> Result<PathBuf, Error>;

async fn get_mullvad_app_settings_dir() -> Result<PathBuf, Error>;

/// Send TCP packet
async fn send_tcp(
interface: Option<String>,
Expand Down
7 changes: 6 additions & 1 deletion test/test-runner/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ pub fn find_traces() -> Result<Vec<AppTrace>, Error> {
// TODO: Check temp data

let caches = find_cache_traces()?;
let settings = find_settings_traces()?;
let traces = vec![
Path::new(r"/etc/mullvad-vpn/"),
&settings,
Path::new(r"/var/log/mullvad-vpn/"),
&caches,
Path::new(r"/opt/Mullvad VPN/"),
Expand All @@ -60,6 +61,10 @@ pub fn find_cache_traces() -> Result<PathBuf, Error> {
mullvad_paths::get_cache_dir().map_err(|error| Error::FileSystem(error.to_string()))
}

pub fn find_settings_traces() -> Result<PathBuf, Error> {
mullvad_paths::get_default_settings_dir().map_err(|error| Error::FileSystem(error.to_string()))
}

#[cfg(target_os = "macos")]
pub fn find_traces() -> Result<Vec<AppTrace>, Error> {
// TODO: Check GUI data
Expand Down
7 changes: 7 additions & 0 deletions test/test-runner/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,13 @@ impl Service for TestServer {
app::find_cache_traces()
}

async fn get_mullvad_app_settings_dir(
self,
_: context::Context,
) -> Result<PathBuf, test_rpc::Error> {
app::find_settings_traces()
}

async fn send_tcp(
self,
_: context::Context,
Expand Down

0 comments on commit 5bbac7f

Please sign in to comment.