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

VPN-5681 - remove iOS VPN profile when logging out or resetting #8329

Merged
merged 11 commits into from
Oct 24, 2023
6 changes: 6 additions & 0 deletions src/connectionmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,12 @@ void ConnectionManager::quit() {
}
}

void ConnectionManager::deleteOSTunnelConfig() {
if (m_impl) {
m_impl->deleteOSTunnelConfig();
}
}

Gela marked this conversation as resolved.
Show resolved Hide resolved
void ConnectionManager::handshakeTimeout() {
logger.debug() << "Timeout while waiting for handshake";

Expand Down
1 change: 1 addition & 0 deletions src/connectionmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ class ConnectionManager : public QObject, public LogSerializer {
bool switchServers(const ServerData& serverData);
void backendFailure();
void updateRequired();
void deleteOSTunnelConfig();

const ServerData& currentServer() const { return m_serverData; }

Expand Down
5 changes: 5 additions & 0 deletions src/controllerimpl.h
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,11 @@ class ControllerImpl : public QObject {
// "disconnecting" state until the "disconnected" signal is received.
virtual void deactivate(ConnectionManager::Reason reason) = 0;

// This method is used to remove the tunnel config from the operating
// system. This is used upon logging out and resetting, so the
// tunnel cannot be reactivated in system settings.
virtual void deleteOSTunnelConfig(){};

// This method is used to retrieve the VPN tunnel status (mainly the number
// of bytes sent and received). It's called always when the VPN tunnel is
// active.
Expand Down
3 changes: 3 additions & 0 deletions src/mozillavpn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -834,6 +834,8 @@ void MozillaVPN::logout() {
ProductsHandler::instance()->stopProductsRegistration();
}

connectionManager()->deleteOSTunnelConfig();

// update-required state is the only one we want to keep when logging out.
if (state() != StateUpdateRequired) {
setState(StateInitialize);
Expand Down Expand Up @@ -1292,6 +1294,7 @@ void MozillaVPN::hardReset() {
SettingsHolder* settingsHolder = SettingsHolder::instance();
Q_ASSERT(settingsHolder);
settingsHolder->hardReset();
connectionManager()->deleteOSTunnelConfig();
}

void MozillaVPN::hardResetAndQuit() {
Expand Down
2 changes: 2 additions & 0 deletions src/platforms/ios/ioscontroller.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ class IOSController final : public ControllerImpl {

void deactivate(ConnectionManager::Reason reason) override;

void deleteOSTunnelConfig() override;

void checkStatus() override;

void getBackendLogs(std::function<void(const QString&)>&& callback) override;
Expand Down
38 changes: 20 additions & 18 deletions src/platforms/ios/ioscontroller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -142,25 +142,25 @@
Q_ASSERT(settingsHolder);

[impl connectWithDnsServer:config.m_dnsServer.toNSString()
serverIpv6Gateway:config.m_serverIpv6Gateway.toNSString()
serverPublicKey:config.m_serverPublicKey.toNSString()
serverIpv4AddrIn:config.m_serverIpv4AddrIn.toNSString()
serverPort:config.m_serverPort
allowedIPAddressRanges:allowedIPAddressRangesNS
reason:reason
gleanDebugTag:settingsHolder->gleanDebugTagActive()
? settingsHolder->gleanDebugTag().toNSString()
: @""
serverIpv6Gateway:config.m_serverIpv6Gateway.toNSString()
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was a linter change when merging main back in

serverPublicKey:config.m_serverPublicKey.toNSString()
serverIpv4AddrIn:config.m_serverIpv4AddrIn.toNSString()
serverPort:config.m_serverPort
allowedIPAddressRanges:allowedIPAddressRangesNS
reason:reason
gleanDebugTag:settingsHolder->gleanDebugTagActive()
? settingsHolder->gleanDebugTag().toNSString()
: @""
isSuperDooperFeatureActive:Feature::get(Feature::Feature_superDooperMetrics)->isSupported()
installationId:config.m_installationId.toNSString()
isOnboarding:MozillaVPN::instance()->state() == App::StateOnboarding
disconnectCallback:^() {
logger.error() << "IOSSWiftController - disconnecting";
emit disconnected();
}
onboardingCompletedCallback:^() {
MozillaVPN::instance()->onboardingCompleted();
}];
installationId:config.m_installationId.toNSString()
isOnboarding:MozillaVPN::instance()->state() == App::StateOnboarding
disconnectCallback:^() {
logger.error() << "IOSSWiftController - disconnecting";
emit disconnected();
}
onboardingCompletedCallback:^() {
MozillaVPN::instance()->onboardingCompleted();
}];
}

void IOSController::deactivate(ConnectionManager::Reason reason) {
Expand All @@ -181,6 +181,8 @@
[impl disconnect];
}

void IOSController::deleteOSTunnelConfig() { [impl deleteOSTunnelConfig]; }

void IOSController::checkStatus() {
logger.debug() << "Checking status";

Expand Down
17 changes: 14 additions & 3 deletions src/platforms/ios/ioscontroller.swift
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public class IOSControllerImpl : NSObject {
@objc func connect(dnsServer: String, serverIpv6Gateway: String, serverPublicKey: String, serverIpv4AddrIn: String, serverPort: Int, allowedIPAddressRanges: Array<VPNIPAddressRange>, reason: Int, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
IOSControllerImpl.logger.debug(message: "Connecting")

let _ = TunnelManager.withTunnel { tunnel in
TunnelManager.withTunnel { tunnel in
// Let's remove the previous config if it exists.
(tunnel.protocolConfiguration as? NETunnelProviderProtocol)?.destroyConfigurationReference()

Expand Down Expand Up @@ -157,7 +157,7 @@ public class IOSControllerImpl : NSObject {
}

func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
let _ = TunnelManager.withTunnel { tunnel in
TunnelManager.withTunnel { tunnel in
let proto = NETunnelProviderProtocol(tunnelConfiguration: config)
proto!.providerBundleIdentifier = TunnelManager.vpnBundleId
proto!.disconnectOnSleep = false
Expand Down Expand Up @@ -238,10 +238,21 @@ public class IOSControllerImpl : NSObject {
TunnelManager.session?.stopTunnel()
}

@objc func deleteOSTunnelConfig() {
IOSControllerImpl.logger.info(message: "Removing tunnel from iOS System Preferences")
TunnelManager.withTunnel { tunnel in
tunnel.removeFromPreferences(completionHandler: { error in
if let error = error {
IOSControllerImpl.logger.info(message: "Error when removing tunnel \(error.localizedDescription)")
}
})
}
}

@objc func checkStatus(callback: @escaping (String, String, String) -> Void) {
IOSControllerImpl.logger.info(message: "Check status")

let _ = TunnelManager.withTunnel { tunnel in
TunnelManager.withTunnel { tunnel in
let proto = tunnel.protocolConfiguration as? NETunnelProviderProtocol
if proto == nil {
callback("", "", "")
Expand Down
2 changes: 1 addition & 1 deletion src/platforms/ios/iostunnelmanager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ class TunnelManager {
}
}

static func withTunnel(_ f: (_ tunnel: NETunnelProviderManager) throws -> Any) -> Any? {
@discardableResult static func withTunnel(_ f: (_ tunnel: NETunnelProviderManager) throws -> Any) -> Any? {
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows us to ignore the result, which gives some cleaner code where we were using this.

guard let tunnel = TunnelManager.instance.tunnel else {
logger.error(message: "Attempted to use the VPN tunnel, but it's not available.")
return nil
Expand Down
Loading