Skip to content

Commit 6fbf715

Browse files
authored
VPN-5469: Fix unsecured WiFi detection on macOS 14 and later (#10779)
* Fix unsecured WiFi detection on macOS 14 and later * Add generic unsecured Wi-Fi notification string * Network watcher should use a global cooldown
1 parent c284131 commit 6fbf715

File tree

11 files changed

+37
-39
lines changed

11 files changed

+37
-39
lines changed

src/mozillavpn.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1979,8 +1979,7 @@ void MozillaVPN::registerInspectorCommands() {
19791979
InspectorHandler::registerCommand(
19801980
"force_unsecured_network", "Force an unsecured network detection", 0,
19811981
[](InspectorHandler*, const QList<QByteArray>&) {
1982-
MozillaVPN::instance()->networkWatcher()->unsecuredNetwork("Dummy",
1983-
"Dummy");
1982+
MozillaVPN::instance()->networkWatcher()->unsecuredNetwork("Dummy");
19841983
return QJsonObject();
19851984
});
19861985

src/networkwatcher.cpp

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,8 @@ void NetworkWatcher::settingsChanged() {
121121
}
122122
}
123123

124-
void NetworkWatcher::unsecuredNetwork(const QString& networkName,
125-
const QString& networkId) {
126-
logger.debug() << "Unsecured network:" << logger.sensitive(networkName)
127-
<< "id:" << logger.sensitive(networkId);
124+
void NetworkWatcher::unsecuredNetwork(const QString& networkName) {
125+
logger.debug() << "Unsecured network:" << logger.sensitive(networkName);
128126

129127
#ifndef UNIT_TEST
130128
if (!m_reportUnsecuredNetwork) {
@@ -147,15 +145,12 @@ void NetworkWatcher::unsecuredNetwork(const QString& networkName,
147145
return;
148146
}
149147

150-
if (!m_networks.contains(networkId)) {
151-
m_networks.insert(networkId, QElapsedTimer());
152-
} else if (!m_networks[networkId].hasExpired(NETWORK_WATCHER_TIMER_MSEC)) {
148+
// Set a cooldown timer to prevent notification loops.
149+
if (!m_cooldown.hasExpired(NETWORK_WATCHER_TIMER_MSEC)) {
153150
logger.debug() << "Notification already shown. Ignoring unsecured network";
154151
return;
155152
}
156-
157-
// Let's activate the QElapsedTimer to avoid notification loops.
158-
m_networks[networkId].start();
153+
m_cooldown.start();
159154

160155
// We don't connect the system tray handler in the CTOR because it can be too
161156
// early. Maybe the NotificationHandler has not been created yet. We do it at

src/networkwatcher.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ class NetworkWatcher final : public QObject {
2525
void initialize();
2626

2727
// Public for the Inspector.
28-
void unsecuredNetwork(const QString& networkName, const QString& networkId);
28+
void unsecuredNetwork(const QString& networkName);
2929
// Used for the Inspector. simulateOffline = true to mock being disconnected,
3030
// false to restore.
3131
void simulateDisconnection(bool simulatedDisconnection);
@@ -46,8 +46,7 @@ class NetworkWatcher final : public QObject {
4646

4747
// Platform-specific implementation.
4848
NetworkWatcherImpl* m_impl = nullptr;
49-
50-
QMap<QString, QElapsedTimer> m_networks;
49+
QElapsedTimer m_cooldown;
5150

5251
// This is used to connect NotificationHandler lazily.
5352
bool m_firstNotification = true;

src/networkwatcherimpl.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,7 @@ class NetworkWatcherImpl : public QObject {
3636

3737
signals:
3838
// Fires when the Device Connects to an unsecured Network
39-
void unsecuredNetwork(const QString& networkName, const QString& networkId);
39+
void unsecuredNetwork(const QString& networkName);
4040
// Fires on when the connected WIFI Changes
4141
// TODO: Only windows-networkwatcher has this, the other plattforms should
4242
// too.

src/notificationhandler.cpp

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -279,9 +279,14 @@ void NotificationHandler::unsecuredNetworkNotification(
279279

280280
QString title =
281281
i18nStrings->t(I18nStrings::NotificationsUnsecuredNetworkTitle);
282-
QString message =
283-
i18nStrings->t(I18nStrings::NotificationsUnsecuredNetworkMessage)
284-
.arg(networkName);
282+
283+
QString message;
284+
if (networkName.isEmpty()) {
285+
message = i18nStrings->t(I18nStrings::NotificationsUnsecuredNetworkGeneric);
286+
} else {
287+
message = i18nStrings->t(I18nStrings::NotificationsUnsecuredNetworkMessage)
288+
.arg(networkName);
289+
}
285290

286291
notifyInternal(UnsecuredNetwork, title, message,
287292
Constants::UNSECURED_NETWORK_ALERT_MSEC);

src/platforms/linux/linuxnetworkwatcherworker.cpp

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -162,15 +162,14 @@ void LinuxNetworkWatcherWorker::checkDevices() {
162162

163163
if (!checkUnsecureFlags(rsnFlags.toInt(), wpaFlags.toInt())) {
164164
QString ssid = ap.property("Ssid").toString();
165-
QString bssid = ap.property("HwAddress").toString();
166165

167166
// We have found 1 unsecured network. We don't need to check other wifi
168167
// network devices.
169168
logger.warning() << "Unsecured AP detected!"
170169
<< "rsnFlags:" << rsnFlags.toInt()
171170
<< "wpaFlags:" << wpaFlags.toInt()
172171
<< "ssid:" << logger.sensitive(ssid);
173-
emit unsecuredNetwork(ssid, bssid);
172+
emit unsecuredNetwork(ssid);
174173
break;
175174
}
176175
}

src/platforms/linux/linuxnetworkwatcherworker.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ class LinuxNetworkWatcherWorker final : public QObject {
2222
void checkDevices();
2323

2424
signals:
25-
void unsecuredNetwork(const QString& networkName, const QString& networkId);
25+
void unsecuredNetwork(const QString& networkName);
2626

2727
public slots:
2828
void initialize();

src/platforms/macos/macosnetworkwatcher.mm

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -100,30 +100,31 @@ - (void)bssidDidChangeForWiFiInterfaceWithName:(NSString*)interfaceName {
100100
return;
101101
}
102102

103+
logger.debug() << "WiFi interface:" << [interface interfaceName];
103104
if (![interface powerOn]) {
104105
logger.debug() << "The interface is off";
105106
return;
106107
}
107108

108-
NSString* ssidNS = [interface ssid];
109-
if (!ssidNS) {
109+
if ([interface activePHYMode] == kCWPHYModeNone) {
110110
logger.debug() << "WiFi is not in used";
111111
return;
112112
}
113113

114-
QString ssid = QString::fromNSString(ssidNS);
115-
if (ssid.isEmpty()) {
116-
logger.debug() << "WiFi doesn't have a valid SSID";
114+
CWSecurity security = [interface security];
115+
if (security != kCWSecurityNone && security != kCWSecurityWEP) {
116+
// WiFi network appears to be reasonably secured.
117+
logger.debug() << "Secure WiFi interface";
117118
return;
118119
}
120+
logger.debug() << "Unsecured network found!";
119121

120-
CWSecurity security = [interface security];
121-
if (security == kCWSecurityNone || security == kCWSecurityWEP) {
122-
logger.debug() << "Unsecured network found!";
123-
emit unsecuredNetwork(ssid, ssid);
124-
return;
122+
QString ssid = QString::fromNSString([interface ssid]);
123+
if (ssid.isEmpty()) {
124+
// Note: Starting with macOS 14, retrieving the SSID requires location
125+
// permissions for the application, which we are unlikely to be granted.
126+
logger.debug() << "Unable to fetch WiFi SSID";
125127
}
126128

127-
logger.debug() << "Secure WiFi interface";
129+
emit unsecuredNetwork(ssid);
128130
}
129-

src/platforms/wasm/wasmnetworkwatcher.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,5 +22,5 @@ void WasmNetworkWatcher::initialize() { logger.debug() << "initialize"; }
2222

2323
void WasmNetworkWatcher::start() {
2424
logger.debug() << "actived";
25-
emit unsecuredNetwork("WifiName", "NetworkID");
25+
emit unsecuredNetwork("WifiName");
2626
}

src/platforms/windows/windowsnetworkwatcher.cpp

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,6 @@ void WindowsNetworkWatcher::processWlan(PWLAN_NOTIFICATION_DATA data) {
134134
(char)connectionInfo->wlanAssociationAttributes.dot11Ssid.ucSSID[i]));
135135
}
136136

137-
logger.debug() << "Unsecure network:" << logger.sensitive(ssid)
138-
<< "id:" << logger.sensitive(bssid);
139-
emit unsecuredNetwork(ssid, bssid);
140-
}
137+
logger.debug() << "Unsecure network:" << logger.sensitive(ssid);
138+
emit unsecuredNetwork(ssid);
139+
}

0 commit comments

Comments
 (0)