Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix ending up in blocked state when disabling split tunnel #7473

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@ Line wrap the file at 100 chars. Th
### Fixed
- (macOS and Windows only) Add the correct route when using obfuscation with Wireguard.

#### macOS
- Fix daemon ending up in blocked state if the user toggled split tunneling without having granted
Full Disk Access to `mullvad-daemon`. This could only ever be accomplished from the CLI.


## [2025.2] - 2025-01-08
### Fixed
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ interface IProps {
}

export default function NotificationArea(props: IProps) {
const { showFullDiskAccessSettings, reconnectTunnel } = useAppContext();
const { showFullDiskAccessSettings } = useAppContext();

const account = useSelector((state: IReduxState) => state.account);
const locale = useSelector((state: IReduxState) => state.userInterface.locale);
Expand Down Expand Up @@ -80,8 +80,7 @@ export default function NotificationArea(props: IProps) {
const disableSplitTunneling = useCallback(async () => {
setIsModalOpen(false);
await setSplitTunnelingState(false);
await reconnectTunnel();
}, [reconnectTunnel, setSplitTunnelingState]);
}, [setSplitTunnelingState]);

const notificationProviders: InAppNotificationProvider[] = [
new ConnectingNotificationProvider({ tunnelState }),
Expand Down
43 changes: 34 additions & 9 deletions mullvad-daemon/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,7 +1021,7 @@ impl Daemon {
TunnelStateTransition::Disconnected => TunnelState::Disconnected { location: None },
TunnelStateTransition::Connecting(endpoint) => {
let feature_indicators = compute_feature_indicators(
&self.settings.to_settings(),
self.settings.settings(),
&endpoint,
self.parameters_generator.last_relay_was_overridden().await,
);
Expand All @@ -1033,7 +1033,7 @@ impl Daemon {
}
TunnelStateTransition::Connected(endpoint) => {
let feature_indicators = compute_feature_indicators(
&self.settings.to_settings(),
self.settings.settings(),
&endpoint,
self.parameters_generator.last_relay_was_overridden().await,
);
Expand Down Expand Up @@ -1199,7 +1199,7 @@ impl Daemon {
.active_features()
.any(|f| matches!(&f, FeatureIndicator::ServerIpOverride));
let new_feature_indicators =
compute_feature_indicators(&self.settings.to_settings(), endpoint, ip_override);
compute_feature_indicators(self.settings.settings(), endpoint, ip_override);
// Update and broadcast the new feature indicators if they have changed
if *feature_indicators != new_feature_indicators {
// Make sure to update the daemon's actual tunnel state. Otherwise, feature
Expand Down Expand Up @@ -1539,11 +1539,36 @@ impl Daemon {
tx: ResponseTx<(), Error>,
) {
let save_result = match update {
ExcludedPathsUpdate::SetState(state) => self
.settings
.update(move |settings| settings.split_tunnel.enable_exclusions = state)
.await
.map_err(Error::SettingsError),
ExcludedPathsUpdate::SetState(state) => {
let split_tunnel_was_enabled =
self.settings.settings().split_tunnel.enable_exclusions;
let save_result = self
.settings
.update(move |settings| settings.split_tunnel.enable_exclusions = state)
.await
.map_err(Error::SettingsError);
// If the user enables split tunneling without also enabling Full Disk Access
// (FDA), the daemon will enter the error state. This is unlikely, since it should
// only be possible via the CLI or if the user manages to disable FDA after having
// successfully enabled split tunneling. In any case, We have observed users
// getting confused over being blocked in this case, and this we may want to
// reconnect after disabling split tunneling.
//
// Since FDA is an implementation detail of split tunneling, we don't actually have
// a way of getting this information at this point, so we fallback to issuing a
// reconnect if the user disables split tunneling while in the error state. This
// code can be removed if we ever remove our dependency on FDA.
if cfg!(target_os = "macos") {
let split_tunnel_will_be_disabled = !state;
if self.tunnel_state.is_in_error_state()
&& split_tunnel_was_enabled
&& split_tunnel_will_be_disabled
{
self.reconnect_tunnel();
}
}
save_result
}
ExcludedPathsUpdate::SetPaths(paths) => self
.settings
.update(move |settings| settings.split_tunnel.apps = paths)
Expand Down Expand Up @@ -2508,7 +2533,7 @@ impl Daemon {
{
Ok(settings_changed) => {
if settings_changed {
let settings = self.settings.to_settings();
let settings = self.settings.settings();
let resolvers =
dns::addresses_from_options(&settings.tunnel_options.dns_options);
self.send_tunnel_command(TunnelCommand::Dns(
Expand Down
4 changes: 4 additions & 0 deletions mullvad-daemon/src/settings/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,10 @@ impl SettingsPersister {
Ok(())
}

pub const fn settings(&self) -> &Settings {
&self.settings
}

pub fn to_settings(&self) -> Settings {
self.settings.clone()
}
Expand Down
Loading