shill: WiFiEndpoint: Update Security and NetworkMode
Retrieve new values for network_mode_ and security_mode_ in
the course of WiFiEndpoint::PropertiesChanged(). To do this,
maintain state that tracks the various hints we have retrieved
from the BSS properties we've seen so far, so we can merge
the newly retrieved data with this history to maintain an
accurate view of the current implied security mode.
BUG=chromium:206299
TEST=Unit tests, autotest (network_WiFiSecMat/120ChangeBSSSecurity)
Change-Id: Ib6a269b4cb51e0bc016335e5212b0cd21a26b404
Reviewed-on: https://gerrit.chromium.org/gerrit/46531
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/wifi_endpoint.cc b/wifi_endpoint.cc
index a91ced6..d63ceca 100644
--- a/wifi_endpoint.cc
+++ b/wifi_endpoint.cc
@@ -60,7 +60,7 @@
network_mode_ = ParseMode(
properties.find(WPASupplicant::kBSSPropertyMode)->second);
- set_security_mode(ParseSecurity(properties));
+ set_security_mode(ParseSecurity(properties, &security_flags_));
has_rsn_property_ = ContainsKey(properties, WPASupplicant::kPropertyRSN);
has_wpa_property_ = ContainsKey(properties, WPASupplicant::kPropertyWPA);
@@ -88,12 +88,36 @@
void WiFiEndpoint::PropertiesChanged(
const map<string, ::DBus::Variant> &properties) {
SLOG(WiFi, 2) << __func__;
+ bool should_notify = false;
map<string, ::DBus::Variant>::const_iterator properties_it =
properties.find(WPASupplicant::kBSSPropertySignal);
if (properties_it != properties.end()) {
signal_strength_ = properties_it->second.reader().get_int16();
SLOG(WiFi, 2) << "WiFiEndpoint " << bssid_string_ << " signal is now "
<< signal_strength_;
+ should_notify = true;
+ }
+
+ properties_it = properties.find(WPASupplicant::kBSSPropertyMode);
+ if (properties_it != properties.end()) {
+ string new_mode = ParseMode(properties_it->second);
+ if (new_mode != network_mode_) {
+ network_mode_ = new_mode;
+ SLOG(WiFi, 2) << "WiFiEndpoint " << bssid_string_ << " mode is now "
+ << network_mode_;
+ should_notify = true;
+ }
+ }
+
+ const char *new_security_mode = ParseSecurity(properties, &security_flags_);
+ if (new_security_mode != security_mode()) {
+ set_security_mode(new_security_mode);
+ SLOG(WiFi, 2) << "WiFiEndpoint " << bssid_string_ << " security is now "
+ << security_mode();
+ should_notify = true;
+ }
+
+ if (should_notify) {
device_->NotifyEndpointChanged(this);
}
}
@@ -275,39 +299,41 @@
// static
const char *WiFiEndpoint::ParseSecurity(
- const map<string, ::DBus::Variant> &properties) {
- set<KeyManagement> rsn_key_management_methods;
+ const map<string, ::DBus::Variant> &properties, SecurityFlags *flags) {
if (ContainsKey(properties, WPASupplicant::kPropertyRSN)) {
// TODO(quiche): check type before casting
const map<string, ::DBus::Variant> rsn_properties(
properties.find(WPASupplicant::kPropertyRSN)->second.
operator map<string, ::DBus::Variant>());
- ParseKeyManagementMethods(rsn_properties, &rsn_key_management_methods);
+ set<KeyManagement> key_management;
+ ParseKeyManagementMethods(rsn_properties, &key_management);
+ flags->rsn_8021x = ContainsKey(key_management, kKeyManagement802_1x);
+ flags->rsn_psk = ContainsKey(key_management, kKeyManagementPSK);
}
- set<KeyManagement> wpa_key_management_methods;
if (ContainsKey(properties, WPASupplicant::kPropertyWPA)) {
// TODO(quiche): check type before casting
const map<string, ::DBus::Variant> rsn_properties(
properties.find(WPASupplicant::kPropertyWPA)->second.
operator map<string, ::DBus::Variant>());
- ParseKeyManagementMethods(rsn_properties, &wpa_key_management_methods);
+ set<KeyManagement> key_management;
+ ParseKeyManagementMethods(rsn_properties, &key_management);
+ flags->wpa_8021x = ContainsKey(key_management, kKeyManagement802_1x);
+ flags->wpa_psk = ContainsKey(key_management, kKeyManagementPSK);
}
- bool wep_privacy = false;
if (ContainsKey(properties, WPASupplicant::kPropertyPrivacy)) {
- wep_privacy = properties.find(WPASupplicant::kPropertyPrivacy)->second.
+ flags->privacy = properties.find(WPASupplicant::kPropertyPrivacy)->second.
reader().get_bool();
}
- if (ContainsKey(rsn_key_management_methods, kKeyManagement802_1x) ||
- ContainsKey(wpa_key_management_methods, kKeyManagement802_1x)) {
+ if (flags->rsn_8021x || flags->wpa_8021x) {
return flimflam::kSecurity8021x;
- } else if (ContainsKey(rsn_key_management_methods, kKeyManagementPSK)) {
+ } else if (flags->rsn_psk) {
return flimflam::kSecurityRsn;
- } else if (ContainsKey(wpa_key_management_methods, kKeyManagementPSK)) {
+ } else if (flags->wpa_psk) {
return flimflam::kSecurityWpa;
- } else if (wep_privacy) {
+ } else if (flags->privacy) {
return flimflam::kSecurityWep;
} else {
return flimflam::kSecurityNone;
diff --git a/wifi_endpoint.h b/wifi_endpoint.h
index ec16dea..2527d4b 100644
--- a/wifi_endpoint.h
+++ b/wifi_endpoint.h
@@ -26,6 +26,19 @@
class WiFiEndpoint : public Endpoint {
public:
+ struct SecurityFlags {
+ SecurityFlags()
+ : rsn_8021x(false),
+ rsn_psk(false),
+ wpa_8021x(false),
+ wpa_psk(false),
+ privacy(false) {}
+ bool rsn_8021x;
+ bool rsn_psk;
+ bool wpa_8021x;
+ bool wpa_psk;
+ bool privacy;
+ };
struct VendorInformation {
std::string wps_manufacturer;
std::string wps_model_name;
@@ -114,12 +127,16 @@
uint16 frequency,
int16 signal_dbm);
// Maps mode strings from supplicant into flimflam's nomenclature, as defined
- // in chromeos/dbus/service_constants.h
+ // in chromeos/dbus/service_constants.h.
static const char *ParseMode(const std::string &mode_string);
- // Parses an Endpoint's properties to identify approprirate flimflam
- // security property value, as defined in chromeos/dbus/service_constants.h
+ // Parses an Endpoint's properties to identify an approprirate flimflam
+ // security property value, as defined in chromeos/dbus/service_constants.h.
+ // The stored data in the |flags| parameter is merged with the provided
+ // properties, and the security value returned is the result of the
+ // merger.
static const char *ParseSecurity(
- const std::map<std::string, ::DBus::Variant> &properties);
+ const std::map<std::string, ::DBus::Variant> &properties,
+ SecurityFlags *flags);
// Parses an Endpoint's properties' "RSN" or "WPA" sub-dictionary, to
// identify supported key management methods (802.1x or PSK).
static void ParseKeyManagementMethods(
@@ -172,6 +189,7 @@
bool ieee80211w_required_;
bool has_rsn_property_;
bool has_wpa_property_;
+ SecurityFlags security_flags_;
ProxyFactory *proxy_factory_;
WiFiRefPtr device_;
diff --git a/wifi_endpoint_unittest.cc b/wifi_endpoint_unittest.cc
index 3b7b3ae..1f57990 100644
--- a/wifi_endpoint_unittest.cc
+++ b/wifi_endpoint_unittest.cc
@@ -26,7 +26,8 @@
using std::string;
using std::vector;
using ::testing::_;
-using testing::HasSubstr;
+using ::testing::HasSubstr;
+using ::testing::Mock;
using ::testing::NiceMock;
namespace shill {
@@ -68,20 +69,30 @@
return args;
}
+ map<string, ::DBus::Variant> make_privacy_args(bool is_private) {
+ map<string, ::DBus::Variant> props;
+ props[WPASupplicant::kPropertyPrivacy].writer().append_bool(is_private);
+ return props;
+ }
+
map<string, ::DBus::Variant> make_security_args(
const string &security_protocol,
const string &key_management_method) {
map<string, ::DBus::Variant> args;
::DBus::MessageIter writer;
writer = args[security_protocol].writer();
- writer <<
- make_key_management_args(make_string_vector1(key_management_method));
+ vector<string> key_management_method_vector;
+ if (!key_management_method.empty()) {
+ key_management_method_vector = make_string_vector1(key_management_method);
+ }
+ writer << make_key_management_args(key_management_method_vector);
return args;
}
const char *ParseSecurity(
const map<string, ::DBus::Variant> &properties) {
- return WiFiEndpoint::ParseSecurity(properties);
+ WiFiEndpoint::SecurityFlags security_flags;
+ return WiFiEndpoint::ParseSecurity(properties, &security_flags);
}
void AddIEWithData(uint8_t type, vector<uint8_t> data, vector<uint8_t> *ies) {
@@ -235,9 +246,7 @@
}
TEST_F(WiFiEndpointTest, ParseSecurityWEP) {
- map<string, ::DBus::Variant> top_params;
- top_params[WPASupplicant::kPropertyPrivacy].writer().append_bool(true);
- EXPECT_STREQ(flimflam::kSecurityWep, ParseSecurity(top_params));
+ EXPECT_STREQ(flimflam::kSecurityWep, ParseSecurity(make_privacy_args(true)));
}
TEST_F(WiFiEndpointTest, ParseSecurityNone) {
@@ -561,15 +570,26 @@
}
}
-TEST_F(WiFiEndpointTest, PropertiesChanged) {
+TEST_F(WiFiEndpointTest, PropertiesChangedNone) {
+ WiFiEndpointRefPtr endpoint =
+ MakeOpenEndpoint(NULL, wifi(), "ssid", "00:00:00:00:00:01");
+ EXPECT_EQ(flimflam::kModeManaged, endpoint->network_mode());
+ EXPECT_EQ(flimflam::kSecurityNone, endpoint->security_mode());
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(0);
+ map<string, ::DBus::Variant> no_changed_properties;
+ endpoint->PropertiesChanged(no_changed_properties);
+ EXPECT_EQ(flimflam::kModeManaged, endpoint->network_mode());
+ EXPECT_EQ(flimflam::kSecurityNone, endpoint->security_mode());
+}
+
+TEST_F(WiFiEndpointTest, PropertiesChangedStrength) {
WiFiEndpointRefPtr endpoint =
MakeOpenEndpoint(NULL, wifi(), "ssid", "00:00:00:00:00:01");
map<string, ::DBus::Variant> changed_properties;
- ::DBus::MessageIter writer;
int16_t signal_strength = 10;
EXPECT_NE(signal_strength, endpoint->signal_strength());
- writer =
+ ::DBus::MessageIter writer =
changed_properties[WPASupplicant::kBSSPropertySignal].writer();
writer << signal_strength;
@@ -578,6 +598,76 @@
EXPECT_EQ(signal_strength, endpoint->signal_strength());
}
+TEST_F(WiFiEndpointTest, PropertiesChangedNetworkMode) {
+ WiFiEndpointRefPtr endpoint =
+ MakeOpenEndpoint(NULL, wifi(), "ssid", "00:00:00:00:00:01");
+ EXPECT_EQ(flimflam::kModeManaged, endpoint->network_mode());
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ map<string, ::DBus::Variant> changed_properties;
+ ::DBus::MessageIter writer =
+ changed_properties[WPASupplicant::kBSSPropertyMode].writer();
+ writer << string(WPASupplicant::kNetworkModeAdHoc);
+ endpoint->PropertiesChanged(changed_properties);
+ EXPECT_EQ(flimflam::kModeAdhoc, endpoint->network_mode());
+}
+
+TEST_F(WiFiEndpointTest, PropertiesChangedSecurityMode) {
+ WiFiEndpointRefPtr endpoint =
+ MakeOpenEndpoint(NULL, wifi(), "ssid", "00:00:00:00:00:01");
+ EXPECT_EQ(flimflam::kSecurityNone, endpoint->security_mode());
+
+ // Upgrade to WEP if privacy flag is added.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ endpoint->PropertiesChanged(make_privacy_args(true));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurityWep, endpoint->security_mode());
+
+ // Make sure we don't downgrade if no interesting arguments arrive.
+ map<string, ::DBus::Variant> no_changed_properties;
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(0);
+ endpoint->PropertiesChanged(no_changed_properties);
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurityWep, endpoint->security_mode());
+
+ // Another upgrade to 802.1x.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ endpoint->PropertiesChanged(make_security_args("RSN", "something-eap"));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurity8021x, endpoint->security_mode());
+
+ // Add WPA-PSK, however this is trumped by RSN 802.1x above, so we don't
+ // change our security nor do we notify anyone.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(0);
+ endpoint->PropertiesChanged(make_security_args("WPA", "something-psk"));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurity8021x, endpoint->security_mode());
+
+ // If nothing changes, we should stay the same.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(0);
+ endpoint->PropertiesChanged(no_changed_properties);
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurity8021x, endpoint->security_mode());
+
+ // However, if the BSS updates to no longer support 802.1x, we degrade
+ // to WPA.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ endpoint->PropertiesChanged(make_security_args("RSN", ""));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurityWpa, endpoint->security_mode());
+
+ // Losing WPA brings us back to WEP (since the privacy flag hasn't changed).
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ endpoint->PropertiesChanged(make_security_args("WPA", ""));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurityWep, endpoint->security_mode());
+
+ // From WEP to open security.
+ EXPECT_CALL(*wifi(), NotifyEndpointChanged(_)).Times(1);
+ endpoint->PropertiesChanged(make_privacy_args(false));
+ Mock::VerifyAndClearExpectations(wifi());
+ EXPECT_EQ(flimflam::kSecurityNone, endpoint->security_mode());
+}
+
TEST_F(WiFiEndpointTest, HasRsnWpaProperties) {
{
WiFiEndpointRefPtr endpoint =