shill: wifi: move RekeyInProgress property from Device to Service
It (arguably) belongs there, and also needs to be there for tests to
make better use of it.
BUG=b:171086223
TEST=unit tests
Change-Id: I77efdc21fe0f521e06b1f847e0e0471b19f4e6e7
Disallow-Recycled-Builds: hatch-cq, kevin-cq, kukui-cq, octopus-cq, scarlet-cq
Reviewed-on: https://chromium-review.googlesource.com/c/chromiumos/platform2/+/2594708
Commit-Queue: Nicolas Boichat <drinkcat@chromium.org>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Reviewed-by: Brian Norris <briannorris@chromium.org>
diff --git a/shill/doc/device-api.txt b/shill/doc/device-api.txt
index f2c4276..ba62b71 100644
--- a/shill/doc/device-api.txt
+++ b/shill/doc/device-api.txt
@@ -639,17 +639,6 @@
these counters from all interfaces every 60 seconds
so this value returned might be slightly old.
- boolean RekeyInProgress [readonly]
-
- (Defined in WiFi)
-
- True when the connected network is attempting to
- rekey. PSK networks can periodically change pairwise
- or group keys for increased protection against attacks.
- This flag guards against state changes in shill that
- would otherwise change the service sort order and
- potentially disrupt connectivity.
-
uint16 ScanInterval [readwrite]
(Defined in WiFi and Cellular)
diff --git a/shill/doc/service-api.txt b/shill/doc/service-api.txt
index c69ea44..c0b575d 100644
--- a/shill/doc/service-api.txt
+++ b/shill/doc/service-api.txt
@@ -1757,6 +1757,15 @@
See also the Name property. (Note that there is
no SSID property for reasons explained above.)
+ boolean WiFi.RekeyInProgress [readonly]
+
+ True when the connected network is attempting to
+ rekey. PSK networks can periodically change pairwise
+ or group keys for increased protection against attacks.
+ This flag guards against state changes in shill that
+ would otherwise change the service sort order and
+ potentially disrupt connectivity.
+
string WiFi.RoamState [readonly]
The state of the service during a within-ESS roam. The
@@ -1776,7 +1785,6 @@
"ready" IP has been renewed, and the service
is awaiting portal detection results.
-
string WiFi.SSID [readonly]
(WiFi only) The service's SSID. Must have a non-zero
diff --git a/shill/wifi/wifi.cc b/shill/wifi/wifi.cc
index 28e9402..8cb4a4b 100644
--- a/shill/wifi/wifi.cc
+++ b/shill/wifi/wifi.cc
@@ -146,7 +146,6 @@
has_already_completed_(false),
is_roaming_in_progress_(false),
pending_eap_failure_(Service::kFailureNone),
- is_rekey_in_progress_(false),
is_debugging_connection_(false),
eap_state_handler_(new SupplicantEAPStateHandler()),
bgscan_short_interval_seconds_(kDefaultBgscanShortIntervalSeconds),
@@ -185,7 +184,6 @@
HelpRegisterDerivedBool(store, kMacAddressRandomizationEnabledProperty,
&WiFi::GetRandomMacEnabled,
&WiFi::SetRandomMacEnabled);
- store->RegisterConstBool(kRekeyInProgressProperty, &is_rekey_in_progress_);
store->RegisterDerivedKeyValueStore(
kLinkStatisticsProperty,
@@ -803,7 +801,9 @@
supplicant_bss_ = new_bss;
has_already_completed_ = false;
is_roaming_in_progress_ = false;
- SetIsRekeyInProgress(false);
+ if (current_service_) {
+ current_service_->SetIsRekeyInProgress(false);
+ }
// Any change in CurrentBSS means supplicant is actively changing our
// connectivity. We no longer need to track any previously pending
@@ -1893,8 +1893,8 @@
ipconfig()->RenewIP();
affected_service->SetRoamState(Service::kRoamStateConfiguring);
}
- } else if (is_rekey_in_progress_) {
- SetIsRekeyInProgress(false);
+ } else if (affected_service->is_rekey_in_progress()) {
+ affected_service->SetIsRekeyInProgress(false);
LOG(INFO) << link_name()
<< " EAP re-key complete. No need to renew L3 configuration.";
}
@@ -1938,7 +1938,7 @@
// reordering the service list, set the roam state to keep track of the
// actual state.
affected_service->SetRoamState(Service::kRoamStateAssociating);
- } else if (!is_rekey_in_progress_) {
+ } else if (!affected_service->is_rekey_in_progress()) {
// Ignore transitions into these states when roaming is in progress, to
// avoid bothering the user when roaming, or re-keying.
if (old_state == WPASupplicant::kInterfaceStateCompleted) {
@@ -1946,7 +1946,7 @@
// nothing when it happens in a PSK network. Unless roaming is in
// progress, we assume supplicant state transitions from completed to an
// auth/assoc state are a result of a re-key.
- SetIsRekeyInProgress(true);
+ affected_service->SetIsRekeyInProgress(true);
} else {
affected_service->SetState(Service::kStateAssociating);
}
@@ -3270,12 +3270,4 @@
return true;
}
-void WiFi::SetIsRekeyInProgress(bool is_rekey_in_progress) {
- if (is_rekey_in_progress == is_rekey_in_progress_) {
- return;
- }
- is_rekey_in_progress_ = is_rekey_in_progress;
- adaptor()->EmitBoolChanged(kRekeyInProgressProperty, is_rekey_in_progress_);
-}
-
} // namespace shill
diff --git a/shill/wifi/wifi.h b/shill/wifi/wifi.h
index 71ecc9d..37d2e5c 100644
--- a/shill/wifi/wifi.h
+++ b/shill/wifi/wifi.h
@@ -609,8 +609,6 @@
void SetSupplicantInterfaceProxy(
std::unique_ptr<SupplicantInterfaceProxyInterface> proxy);
- void SetIsRekeyInProgress(bool is_rekey_in_progress);
-
// Pointer to the provider object that maintains WiFiService objects.
WiFiProvider* provider_;
@@ -686,10 +684,6 @@
// there is one), and store it in |pending_eap_failure_| to be used later when
// we actually disconnect from the network.
Service::ConnectFailure pending_eap_failure_;
- // Indicates that the current BSS has attempted to "re-key". We optimistically
- // assume that this succeeds and don't perform any state transitions to avoid
- // disrupting connectivity.
- bool is_rekey_in_progress_;
// Indicates that we are debugging a problematic connection.
bool is_debugging_connection_;
// Tracks the process of an EAP negotiation.
diff --git a/shill/wifi/wifi_service.cc b/shill/wifi/wifi_service.cc
index c29604d..e432b60 100644
--- a/shill/wifi/wifi_service.cc
+++ b/shill/wifi/wifi_service.cc
@@ -83,7 +83,8 @@
expecting_disconnect_(false),
certificate_file_(new CertificateFile()),
provider_(provider),
- roam_state_(kRoamStateIdle) {
+ roam_state_(kRoamStateIdle),
+ is_rekey_in_progress_(false) {
// Must be constructed with a SecurityClass. We only detect (for internal and
// informational purposes) the specific mode in use later.
CHECK(IsValidSecurityClass(security_)) << base::StringPrintf(
@@ -111,6 +112,8 @@
&vendor_information_);
HelpRegisterConstDerivedString(kWifiRoamStateProperty,
&WiFiService::CalculateRoamState);
+ store->RegisterConstBool(kWifiRekeyInProgressProperty,
+ &is_rekey_in_progress_);
hex_ssid_ = base::HexEncode(ssid_.data(), ssid_.size());
store->RegisterConstString(kWifiHexSsid, &hex_ssid_);
@@ -1186,4 +1189,13 @@
}
}
+void WiFiService::SetIsRekeyInProgress(bool is_rekey_in_progress) {
+ if (is_rekey_in_progress == is_rekey_in_progress_) {
+ return;
+ }
+ is_rekey_in_progress_ = is_rekey_in_progress;
+ adaptor()->EmitBoolChanged(kWifiRekeyInProgressProperty,
+ is_rekey_in_progress_);
+}
+
} // namespace shill
diff --git a/shill/wifi/wifi_service.h b/shill/wifi/wifi_service.h
index 935286e..2098272 100644
--- a/shill/wifi/wifi_service.h
+++ b/shill/wifi/wifi_service.h
@@ -108,6 +108,9 @@
std::string GetRoamStateString() const;
std::string CalculateRoamState(Error* error);
+ void SetIsRekeyInProgress(bool is_rekey_in_progress);
+ bool is_rekey_in_progress() const { return is_rekey_in_progress_; }
+
virtual bool HasEndpoints() const { return !endpoints_.empty(); }
bool IsVisible() const override;
bool IsSecurityMatch(const std::string& security) const;
@@ -356,6 +359,10 @@
// preserve the service sort order. |roam_state_| is valid during this process
// (while the Service is Online but reassociation is happening) only.
RoamState roam_state_;
+ // Indicates that the current BSS has attempted to "re-key". We optimistically
+ // assume that this succeeds and don't perform any state transitions to avoid
+ // disrupting connectivity.
+ bool is_rekey_in_progress_;
};
} // namespace shill
diff --git a/shill/wifi/wifi_test.cc b/shill/wifi/wifi_test.cc
index 50723d4..9acf570 100644
--- a/shill/wifi/wifi_test.cc
+++ b/shill/wifi/wifi_test.cc
@@ -1019,7 +1019,6 @@
}
bool GetSupplicantPresent() { return wifi_->supplicant_present_; }
bool GetIsRoamingInProgress() { return wifi_->is_roaming_in_progress_; }
- bool GetIsRekeyInProgress() { return wifi_->is_rekey_in_progress_; }
void SetIsRoamingInProgress(bool is_roaming_in_progress) {
wifi_->is_roaming_in_progress_ = is_roaming_in_progress;
}
@@ -3481,11 +3480,11 @@
EXPECT_CALL(*service, IsConnected(nullptr)).WillRepeatedly(Return(true));
EXPECT_CALL(*service, SetState(_)).Times(0);
ReportStateChanged(WPASupplicant::kInterfaceState4WayHandshake);
- ASSERT_TRUE(GetIsRekeyInProgress());
+ ASSERT_TRUE(GetCurrentService()->is_rekey_in_progress());
ReportStateChanged(WPASupplicant::kInterfaceStateGroupHandshake);
- ASSERT_TRUE(GetIsRekeyInProgress());
+ ASSERT_TRUE(GetCurrentService()->is_rekey_in_progress());
ReportStateChanged(WPASupplicant::kInterfaceStateCompleted);
- ASSERT_FALSE(GetIsRekeyInProgress());
+ ASSERT_FALSE(GetCurrentService()->is_rekey_in_progress());
Mock::VerifyAndClearExpectations(service.get());
}
diff --git a/system_api/dbus/shill/dbus-constants.h b/system_api/dbus/shill/dbus-constants.h
index 942d964..a9087bc 100644
--- a/system_api/dbus/shill/dbus-constants.h
+++ b/system_api/dbus/shill/dbus-constants.h
@@ -217,7 +217,8 @@
const char kWifiHexSsid[] = "WiFi.HexSSID";
const char kWifiHiddenSsid[] = "WiFi.HiddenSSID";
const char kWifiPhyMode[] = "WiFi.PhyMode";
-const char kWifiRoamStateProperty[] = "RoamState";
+const char kWifiRekeyInProgressProperty[] = "WiFi.RekeyInProgress";
+const char kWifiRoamStateProperty[] = "WiFi.RoamState";
const char kWifiVendorInformationProperty[] = "WiFi.VendorInformation";
// Base VPN Service property names.
@@ -385,7 +386,6 @@
const char kMacAddressRandomizationSupportedProperty[] =
"MACAddressRandomizationSupported";
const char kNetDetectScanPeriodSecondsProperty[] = "NetDetectScanPeriodSeconds";
-const char kRekeyInProgressProperty[] = "RekeyInProgress";
const char kWakeOnWiFiFeaturesEnabledProperty[] = "WakeOnWiFiFeaturesEnabled";
const char kWakeToScanPeriodSecondsProperty[] = "WakeToScanPeriodSeconds";
const char kWifiSupportedFrequenciesProperty[] = "WiFi.SupportedFrequencies";
