shill: Ethernet: Send EAPOL logoff and logon
Use newly created "EAPLogoff" and "EAPLogon" commands in
wpa_supplicant to de-authenticate and speed authentication
on 802.1x wired networks respectively. Add small fixes to
make sure that logoff happens during "Unload" and eap
credential changes.
CQ-DEPEND=CL:48803
BUG=chromium:234290
TEST=New unit tests. Modified autotest
network_8021xWiredAuthentication tests logoff/logon using
profile pop/push (see https://gerrit.chromium.org/gerrit/48806).
Change-Id: Ic9f6e74371e7f57bd3f28b1a68be5fc603da0fdd
Reviewed-on: https://gerrit.chromium.org/gerrit/48802
Reviewed-by: Christopher Wiley <wiley@chromium.org>
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index 92f3b6e..0274460 100644
--- a/Makefile
+++ b/Makefile
@@ -358,6 +358,7 @@
error_unittest.o \
ethernet_unittest.o \
ethernet_eap_provider_unittest.o \
+ ethernet_eap_service_unittest.o \
ethernet_service_unittest.o \
file_reader_unittest.o \
hook_table_unittest.o \
diff --git a/dbus_bindings/supplicant-interface.xml b/dbus_bindings/supplicant-interface.xml
index 2b3a5e2..d7ca543 100644
--- a/dbus_bindings/supplicant-interface.xml
+++ b/dbus_bindings/supplicant-interface.xml
@@ -41,6 +41,8 @@
<method name="FlushBSS">
<arg name="age" type="u" direction="in"/>
</method>
+ <method name="EAPLogoff"/>
+ <method name="EAPLogon"/>
<property name="Capabilities" type="a{sv}" access="read"/>
<property name="State" type="s" access="read"/>
<property name="Scanning" type="b" access="read"/>
diff --git a/ethernet.cc b/ethernet.cc
index a17b133..d37af1e 100644
--- a/ethernet.cc
+++ b/ethernet.cc
@@ -278,10 +278,14 @@
}
supplicant_interface_proxy_->SelectNetwork(supplicant_network_path_);
+ supplicant_interface_proxy_->EAPLogon();
return true;
}
void Ethernet::StopSupplicant() {
+ if (supplicant_interface_proxy_.get()) {
+ supplicant_interface_proxy_->EAPLogoff();
+ }
supplicant_interface_proxy_.reset();
if (!supplicant_interface_path_.empty() && supplicant_process_proxy_.get()) {
try {
diff --git a/ethernet_eap_service.cc b/ethernet_eap_service.cc
index f22f321..59e410f 100644
--- a/ethernet_eap_service.cc
+++ b/ethernet_eap_service.cc
@@ -42,6 +42,10 @@
manager()->ethernet_eap_provider()->OnCredentialsChanged();
}
+bool EthernetEapService::Unload() {
+ Service::Unload();
+ manager()->ethernet_eap_provider()->OnCredentialsChanged();
+ return false;
+}
+
} // namespace shill
-
-
diff --git a/ethernet_eap_service.h b/ethernet_eap_service.h
index 5a201fb..8148df7 100644
--- a/ethernet_eap_service.h
+++ b/ethernet_eap_service.h
@@ -27,8 +27,10 @@
// Inherited from Service.
virtual std::string GetDeviceRpcId(Error *error);
virtual std::string GetStorageIdentifier() const;
+ virtual bool Is8021x() const { return true; }
virtual bool IsVisible() const { return false; }
virtual void OnEapCredentialsChanged();
+ virtual bool Unload();
};
} // namespace shill
diff --git a/ethernet_eap_service_unittest.cc b/ethernet_eap_service_unittest.cc
new file mode 100644
index 0000000..5620568
--- /dev/null
+++ b/ethernet_eap_service_unittest.cc
@@ -0,0 +1,68 @@
+// Copyright (c) 2013 The Chromium OS Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "shill/ethernet_eap_service.h"
+
+#include <base/bind.h>
+#include <chromeos/dbus/service_constants.h>
+#include <gtest/gtest.h>
+
+#include "shill/mock_control.h"
+#include "shill/mock_ethernet_eap_provider.h"
+#include "shill/mock_event_dispatcher.h"
+#include "shill/mock_manager.h"
+#include "shill/mock_metrics.h"
+#include "shill/technology.h"
+
+using testing::Return;
+
+namespace shill {
+
+class EthernetEapServiceTest : public testing::Test {
+ public:
+ EthernetEapServiceTest()
+ : metrics_(&dispatcher_),
+ manager_(&control_, &dispatcher_, &metrics_, NULL),
+ service_(new EthernetEapService(&control_,
+ &dispatcher_,
+ &metrics_,
+ &manager_)) {}
+ virtual ~EthernetEapServiceTest() {}
+
+ protected:
+ MockControl control_;
+ MockEventDispatcher dispatcher_;
+ MockMetrics metrics_;
+ MockManager manager_;
+ MockEthernetEapProvider provider_;
+ scoped_refptr<EthernetEapService> service_;
+};
+
+TEST_F(EthernetEapServiceTest, MethodOverrides) {
+ EXPECT_EQ("/", service_->GetDeviceRpcId(NULL));
+ EXPECT_EQ("etherneteap_all", service_->GetStorageIdentifier());
+ EXPECT_EQ(Technology::kEthernetEap, service_->technology());
+ EXPECT_TRUE(service_->Is8021x());
+ EXPECT_FALSE(service_->IsVisible());
+}
+
+TEST_F(EthernetEapServiceTest, OnEapCredentialsChanged) {
+ EXPECT_CALL(manager_, ethernet_eap_provider()).WillOnce(Return(&provider_));
+ EXPECT_CALL(provider_, OnCredentialsChanged());
+ service_->OnEapCredentialsChanged();
+}
+
+TEST_F(EthernetEapServiceTest, OnEapCredentialPropertyChanged) {
+ EXPECT_CALL(manager_, ethernet_eap_provider()).WillOnce(Return(&provider_));
+ EXPECT_CALL(provider_, OnCredentialsChanged());
+ service_->OnPropertyChanged(flimflam::kEapPasswordProperty);
+}
+
+TEST_F(EthernetEapServiceTest, Unload) {
+ EXPECT_CALL(manager_, ethernet_eap_provider()).WillOnce(Return(&provider_));
+ EXPECT_CALL(provider_, OnCredentialsChanged());
+ EXPECT_FALSE(service_->Unload());
+}
+
+} // namespace shill
diff --git a/ethernet_unittest.cc b/ethernet_unittest.cc
index bd4499f..6c6f6ac 100644
--- a/ethernet_unittest.cc
+++ b/ethernet_unittest.cc
@@ -433,6 +433,7 @@
"fi.w1.wpa_supplicant1.UnknownError",
"test threw fi.w1.wpa_supplicant1.UnknownError")));
EXPECT_CALL(*interface_proxy, SelectNetwork(_)).Times(0);
+ EXPECT_CALL(*interface_proxy, EAPLogon()).Times(0);
EXPECT_FALSE(InvokeStartEapAuthentication());
Mock::VerifyAndClearExpectations(mock_service_);
Mock::VerifyAndClearExpectations(mock_eap_service_);
@@ -448,6 +449,7 @@
EXPECT_CALL(*interface_proxy, AddNetwork(_))
.WillOnce(Return(kFirstNetworkPath));
EXPECT_CALL(*interface_proxy, SelectNetwork(StrEq(kFirstNetworkPath)));
+ EXPECT_CALL(*interface_proxy, EAPLogon());
EXPECT_TRUE(InvokeStartEapAuthentication());
Mock::VerifyAndClearExpectations(mock_service_);
Mock::VerifyAndClearExpectations(mock_eap_service_);
@@ -464,15 +466,19 @@
EXPECT_CALL(*interface_proxy, AddNetwork(_))
.WillOnce(Return(kSecondNetworkPath));
EXPECT_CALL(*interface_proxy, SelectNetwork(StrEq(kSecondNetworkPath)));
+ EXPECT_CALL(*interface_proxy, EAPLogon());
EXPECT_TRUE(InvokeStartEapAuthentication());
EXPECT_EQ(kSecondNetworkPath, GetSupplicantNetworkPath());
}
TEST_F(EthernetTest, StopSupplicant) {
MockSupplicantProcessProxy *process_proxy = supplicant_process_proxy_.get();
+ MockSupplicantInterfaceProxy *interface_proxy =
+ supplicant_interface_proxy_.get();
StartSupplicant();
SetIsEapAuthenticated(true);
SetSupplicantNetworkPath("/network/1");
+ EXPECT_CALL(*interface_proxy, EAPLogoff());
EXPECT_CALL(*process_proxy, RemoveInterface(StrEq(kInterfacePath)));
InvokeStopSupplicant();
EXPECT_EQ(NULL, GetSupplicantInterfaceProxy());
diff --git a/mock_supplicant_interface_proxy.h b/mock_supplicant_interface_proxy.h
index 3d462d8..849b1e4 100644
--- a/mock_supplicant_interface_proxy.h
+++ b/mock_supplicant_interface_proxy.h
@@ -25,6 +25,8 @@
MOCK_METHOD1(AddNetwork, ::DBus::Path(
const std::map<std::string, ::DBus::Variant> &args));
MOCK_METHOD0(EnableHighBitrates, void());
+ MOCK_METHOD0(EAPLogoff, void());
+ MOCK_METHOD0(EAPLogon, void());
MOCK_METHOD0(Disconnect, void());
MOCK_METHOD1(FlushBSS, void(const uint32_t &age));
MOCK_METHOD0(Reassociate, void());
diff --git a/service.h b/service.h
index 1bd5cbd..7a92a28 100644
--- a/service.h
+++ b/service.h
@@ -212,7 +212,7 @@
virtual bool IsLoadableFrom(StoreInterface *storage) const;
// Returns true if the service uses 802.1x for key management.
- virtual bool Is8021x() const { return false; };
+ virtual bool Is8021x() const { return false; }
// Loads the service from persistent |storage|. Returns true on success.
virtual bool Load(StoreInterface *storage);
diff --git a/supplicant_interface_proxy.cc b/supplicant_interface_proxy.cc
index e6508c9..56deb7e 100644
--- a/supplicant_interface_proxy.cc
+++ b/supplicant_interface_proxy.cc
@@ -48,6 +48,26 @@
}
}
+void SupplicantInterfaceProxy::EAPLogoff() {
+ SLOG(DBus, 2) << __func__;
+ try {
+ return proxy_.EAPLogoff();
+ } catch (const DBus::Error &e) {
+ LOG(ERROR) << "DBus exception: " << e.name() << ": " << e.what();
+ throw; // Re-throw the exception.
+ }
+}
+
+void SupplicantInterfaceProxy::EAPLogon() {
+ SLOG(DBus, 2) << __func__;
+ try {
+ return proxy_.EAPLogon();
+ } catch (const DBus::Error &e) {
+ LOG(ERROR) << "DBus exception: " << e.name() << ": " << e.what();
+ throw; // Re-throw the exception.
+ }
+}
+
void SupplicantInterfaceProxy::Disconnect() {
SLOG(DBus, 2) << __func__;
try {
diff --git a/supplicant_interface_proxy.h b/supplicant_interface_proxy.h
index b959bde..6ddaabe 100644
--- a/supplicant_interface_proxy.h
+++ b/supplicant_interface_proxy.h
@@ -35,6 +35,8 @@
virtual ::DBus::Path AddNetwork(
const std::map<std::string, ::DBus::Variant> &args);
virtual void EnableHighBitrates();
+ virtual void EAPLogon();
+ virtual void EAPLogoff();
virtual void Disconnect();
virtual void FlushBSS(const uint32_t &age);
virtual void Reassociate();
diff --git a/supplicant_interface_proxy_interface.h b/supplicant_interface_proxy_interface.h
index fbdea38..aa9afcb 100644
--- a/supplicant_interface_proxy_interface.h
+++ b/supplicant_interface_proxy_interface.h
@@ -21,6 +21,8 @@
virtual ::DBus::Path AddNetwork(
const std::map<std::string, ::DBus::Variant> &args) = 0;
virtual void EnableHighBitrates() = 0;
+ virtual void EAPLogoff() = 0;
+ virtual void EAPLogon() = 0;
virtual void Disconnect() = 0;
virtual void FlushBSS(const uint32_t &age) = 0;
virtual void Reassociate() = 0;