Skip to content

Commit

Permalink
VPN-5532: Prevent persistent VPN config system prompt (#8430)
Browse files Browse the repository at this point in the history
  • Loading branch information
MattLichtenstein authored Nov 1, 2023
1 parent d1a65f1 commit b0c4c0a
Show file tree
Hide file tree
Showing 8 changed files with 49 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,7 @@ private void initServiceConnection(){
private final int EVENT_PERMISSION_REQURED = 6;
private final int EVENT_DISCONNECTED = 2;
private final int EVENT_ONBOARDING_COMPLETED = 9;
private final int EVENT_VPN_CONFIG_PERMISSION_RESPONSE = 10;

public void onPermissionRequest(int code, Parcel data) {
if(code != EVENT_PERMISSION_REQURED){
Expand All @@ -183,6 +184,11 @@ public void onNotificationPermissionRequest() {
protected void onActivityResult(int requestCode, int resultCode, Intent data) {
if(requestCode == PERMISSION_TRANSACTION){
// THATS US!

// At this point, the user has made a selection on the system config permission modal to either allow or not allow
// the vpn configuration to be created, so it is safe to run activation retries via ConnectionManager::startHandshakeTimer()
// without the possibility or re-prompting (flickering) the modal while it is currently being displayed
onServiceMessage(EVENT_VPN_CONFIG_PERMISSION_RESPONSE,"");
if( resultCode == RESULT_OK ){
// Prompt accepted, tell service to retry.
dispatchParcel(ACTION_RESUME_ACTIVATE,"");
Expand Down
13 changes: 12 additions & 1 deletion src/connectionmanager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,10 @@ void ConnectionManager::deleteOSTunnelConfig() {
}
}

void ConnectionManager::startHandshakeTimer() {
m_handshakeTimer.start(HANDSHAKE_TIMEOUT_SEC * 1000);
}

void ConnectionManager::handshakeTimeout() {
logger.debug() << "Timeout while waiting for handshake";

Expand Down Expand Up @@ -571,7 +575,14 @@ void ConnectionManager::activateNext() {
const InterfaceConfig& config = m_activationQueue.first();

logger.debug() << "Activating peer" << logger.keys(config.m_serverPublicKey);
m_handshakeTimer.start(HANDSHAKE_TIMEOUT_SEC * 1000);

// Mobile platforms will begin handshake timer once we know the VPN
// configuration has been set with the system (to avoid persistently
// re-triggering the system prompt - VPN-5532)
#if defined(MZ_LINUX) || defined(MZ_WINDOWS) || defined(MZ_MACOS)
startHandshakeTimer();
#endif

m_impl->activate(config, stateToReason(m_state));

// Move to the confirming state if we are awaiting any connection handshakes.
Expand Down
1 change: 1 addition & 0 deletions src/connectionmanager.h
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ class ConnectionManager : public QObject, public LogSerializer {
void backendFailure();
void updateRequired();
void deleteOSTunnelConfig();
void startHandshakeTimer();

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

Expand Down
9 changes: 9 additions & 0 deletions src/platforms/android/androidcontroller.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

#include "androidutils.h"
#include "androidvpnactivity.h"
#include "connectionmanager.h"
#include "errorhandler.h"
#include "feature.h"
#include "i18nstrings.h"
Expand Down Expand Up @@ -110,6 +111,14 @@ AndroidController::AndroidController() {
}
},
Qt::QueuedConnection);
connect(
activity, &AndroidVPNActivity::eventVpnConfigPermissionResponse, this,
[]() {
ConnectionManager* connectionManager =
MozillaVPN::instance()->connectionManager();
connectionManager->startHandshakeTimer();
},
Qt::QueuedConnection);
}
AndroidController::~AndroidController() { MZ_COUNT_DTOR(AndroidController); }

Expand Down
3 changes: 3 additions & 0 deletions src/platforms/android/androidvpnactivity.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,9 @@ void AndroidVPNActivity::handleServiceMessage(int code, const QString& data) {
case ServiceEvents::EVENT_ONBOARDING_COMPLETED:
emit eventOnboardingCompleted();
break;
case ServiceEvents::EVENT_VPN_CONFIG_PERMISSION_RESPONSE:
emit eventVpnConfigPermissionResponse();
break;
default:
Q_ASSERT(false);
}
Expand Down
6 changes: 6 additions & 0 deletions src/platforms/android/androidvpnactivity.h
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@ enum ServiceEvents {
// The Daemon need's the app to ask for notification
// permissions, to show the "you're connected" messages.
EVENT_REQUEST_NOTIFICATION_PERMISSION = 8,
// Signals MozillaVPN that we have completed onboarding
EVENT_ONBOARDING_COMPLETED = 9,
// Signals the ConnectionManager that it may now allow
// activation retries now that the system vpn config modal
// has been responded to
EVENT_VPN_CONFIG_PERMISSION_RESPONSE = 10,
};
typedef enum ServiceEvents ServiceEvents;

Expand All @@ -96,6 +101,7 @@ class AndroidVPNActivity : public QObject {
void eventStatisticUpdate(const QString& data);
void eventActivationError(const QString& data);
void eventOnboardingCompleted();
void eventVpnConfigPermissionResponse();
void eventRequestGleanUploadEnabledState();

private:
Expand Down
4 changes: 4 additions & 0 deletions src/platforms/ios/ioscontroller.mm
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,10 @@
}
onboardingCompletedCallback:^() {
MozillaVPN::instance()->onboardingCompleted();
}
vpnConfigPermissionResponseCallback:^() {
ConnectionManager* connectionManager = MozillaVPN::instance()->connectionManager();
connectionManager->startHandshakeTimer();
}];
}

Expand Down
11 changes: 8 additions & 3 deletions src/platforms/ios/ioscontroller.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,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) {
@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, vpnConfigPermissionResponseCallback: @escaping () -> Void) {
IOSControllerImpl.logger.debug(message: "Connecting")

TunnelManager.withTunnel { tunnel in
Expand Down Expand Up @@ -152,11 +152,11 @@ public class IOSControllerImpl : NSObject {

let config = TunnelConfiguration(name: VPN_NAME, interface: interface, peers: peerConfigurations)

return self.configureTunnel(config: config, reason: reason, serverName: serverIpv4AddrIn + ":\(serverPort )", gleanDebugTag: gleanDebugTag, isSuperDooperFeatureActive: isSuperDooperFeatureActive, installationId: installationId, isOnboarding: isOnboarding, disconnectCallback: disconnectCallback, onboardingCompletedCallback: onboardingCompletedCallback)
return self.configureTunnel(config: config, reason: reason, serverName: serverIpv4AddrIn + ":\(serverPort )", gleanDebugTag: gleanDebugTag, isSuperDooperFeatureActive: isSuperDooperFeatureActive, installationId: installationId, isOnboarding: isOnboarding, disconnectCallback: disconnectCallback, onboardingCompletedCallback: onboardingCompletedCallback, vpnConfigPermissionResponseCallback: vpnConfigPermissionResponseCallback)
}
}

func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void) {
func configureTunnel(config: TunnelConfiguration, reason: Int, serverName: String, gleanDebugTag: String, isSuperDooperFeatureActive: Bool, installationId: String, isOnboarding: Bool, disconnectCallback: @escaping () -> Void, onboardingCompletedCallback: @escaping () -> Void, vpnConfigPermissionResponseCallback: @escaping () -> Void) {
TunnelManager.withTunnel { tunnel in
let proto = NETunnelProviderProtocol(tunnelConfiguration: config)
proto!.providerBundleIdentifier = TunnelManager.vpnBundleId
Expand Down Expand Up @@ -185,6 +185,11 @@ public class IOSControllerImpl : NSObject {
tunnel.isEnabled = true

return tunnel.saveToPreferences { saveError in
// At this point, the user has made a selection on the system config permission modal to either allow or not allow
// the vpn configuration to be created, so it is safe to run activation retries via ConnectionManager::startHandshakeTimer()
// without the possibility or re-prompting (flickering) the modal while it is currently being displayed
vpnConfigPermissionResponseCallback()

if let error = saveError {
IOSControllerImpl.logger.error(message: "Connect Tunnel Save Error: \(error)")
disconnectCallback()
Expand Down

0 comments on commit b0c4c0a

Please sign in to comment.