shill: WiFi: Separate out EAP Event tracking
Separate EAP event tracking into a separate class, so it can be
used by more than one device type. This also makes unit testing
slightly simpler. While here also move the code that extracts
remote certification to another WPASupplicant static method.
BUG=chromium:224509
TEST=Unit tests
Change-Id: I7f4e7f5e03044f3f21dc179ea13878add5ae1c9d
Reviewed-on: https://gerrit.chromium.org/gerrit/47010
Reviewed-by: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
diff --git a/Makefile b/Makefile
index 8e34018..c0cb3c1 100644
--- a/Makefile
+++ b/Makefile
@@ -295,6 +295,7 @@
sockets.o \
static_ip_parameters.o \
supplicant_bss_proxy.o \
+ supplicant_eap_state_handler.o \
supplicant_interface_proxy.o \
supplicant_network_proxy.o \
supplicant_process_proxy.o \
@@ -409,6 +410,7 @@
mock_sockets.o \
mock_store.o \
mock_supplicant_bss_proxy.o \
+ mock_supplicant_eap_state_handler.o \
mock_supplicant_interface_proxy.o \
mock_supplicant_network_proxy.o \
mock_supplicant_process_proxy.o \
@@ -443,6 +445,7 @@
scope_logger_unittest.o \
service_under_test.o \
service_unittest.o \
+ supplicant_eap_state_handler_unittest.o \
shill_unittest.o \
shims/certificates_unittest.o \
shims/netfilter_queue_processor_unittest.o \
diff --git a/mock_supplicant_eap_state_handler.cc b/mock_supplicant_eap_state_handler.cc
new file mode 100644
index 0000000..75ab90a
--- /dev/null
+++ b/mock_supplicant_eap_state_handler.cc
@@ -0,0 +1,14 @@
+// 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/mock_supplicant_eap_state_handler.h"
+
+namespace shill {
+
+MockSupplicantEAPStateHandler::MockSupplicantEAPStateHandler()
+ : SupplicantEAPStateHandler() {}
+
+MockSupplicantEAPStateHandler::~MockSupplicantEAPStateHandler() {}
+
+} // namespace shill
diff --git a/mock_supplicant_eap_state_handler.h b/mock_supplicant_eap_state_handler.h
new file mode 100644
index 0000000..7f15ae2
--- /dev/null
+++ b/mock_supplicant_eap_state_handler.h
@@ -0,0 +1,33 @@
+// 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.
+
+#ifndef SHILL_MOCK_SUPPLICANT_EAP_STATE_HANDLER_H_
+#define SHILL_MOCK_SUPPLICANT_EAP_STATE_HANDLER_H_
+
+#include <string>
+
+#include <gmock/gmock.h>
+
+#include "shill/supplicant_eap_state_handler.h"
+
+namespace shill {
+
+class MockSupplicantEAPStateHandler : public SupplicantEAPStateHandler {
+ public:
+ MockSupplicantEAPStateHandler();
+ virtual ~MockSupplicantEAPStateHandler();
+
+ MOCK_METHOD3(ParseStatus, bool(const std::string &status,
+ const std::string ¶meter,
+ Service::ConnectFailure *failure));
+ MOCK_METHOD0(Reset, void());
+ MOCK_METHOD0(is_eap_in_progress, bool());
+
+ private:
+ DISALLOW_COPY_AND_ASSIGN(MockSupplicantEAPStateHandler);
+};
+
+} // namespace shill
+
+#endif // SHILL_SUPPLICANT_EAP_STATE_HANDLER_H_
diff --git a/supplicant_eap_state_handler.cc b/supplicant_eap_state_handler.cc
new file mode 100644
index 0000000..39c9421
--- /dev/null
+++ b/supplicant_eap_state_handler.cc
@@ -0,0 +1,71 @@
+// 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/supplicant_eap_state_handler.h"
+
+#include "shill/logging.h"
+#include "shill/wpa_supplicant.h"
+
+namespace shill {
+
+using std::string;
+
+SupplicantEAPStateHandler::SupplicantEAPStateHandler()
+ : is_eap_in_progress_(false) {}
+
+SupplicantEAPStateHandler::~SupplicantEAPStateHandler() {}
+
+bool SupplicantEAPStateHandler::ParseStatus(const string &status,
+ const string ¶meter,
+ Service::ConnectFailure *failure) {
+ if (status == WPASupplicant::kEAPStatusAcceptProposedMethod) {
+ LOG(INFO) << "EAP: accepted method " << parameter;
+ } else if (status == WPASupplicant::kEAPStatusCompletion) {
+ if (parameter == WPASupplicant::kEAPParameterSuccess) {
+ LOG(INFO) << "EAP: Completed authentication successfully.";
+ is_eap_in_progress_ = false;
+ return true;
+ } else if (parameter == WPASupplicant::kEAPParameterFailure) {
+ // If there was a TLS error, use this instead of the generic failure.
+ if (tls_error_ == WPASupplicant::kEAPStatusLocalTLSAlert) {
+ *failure = Service::kFailureEAPLocalTLS;
+ } else if (tls_error_ ==
+ WPASupplicant::kEAPStatusRemoteTLSAlert) {
+ *failure = Service::kFailureEAPRemoteTLS;
+ } else {
+ *failure = Service::kFailureEAPAuthentication;
+ }
+ } else {
+ LOG(ERROR) << "EAP: Unexpected " << status << " parameter: " << parameter;
+ }
+ } else if (status == WPASupplicant::kEAPStatusLocalTLSAlert ||
+ status == WPASupplicant::kEAPStatusRemoteTLSAlert) {
+ tls_error_ = status;
+ } else if (status ==
+ WPASupplicant::kEAPStatusRemoteCertificateVerification) {
+ if (parameter == WPASupplicant::kEAPParameterSuccess) {
+ LOG(INFO) << "EAP: Completed remote certificate verification.";
+ } else {
+ // wpa_supplicant doesn't currently have a verification failure
+ // message. We will instead get a RemoteTLSAlert above.
+ LOG(ERROR) << "EAP: Unexpected " << status << " parameter: " << parameter;
+ }
+ } else if (status == WPASupplicant::kEAPStatusParameterNeeded) {
+ LOG(ERROR) << "EAP: Authentication aborted due to missing authentication "
+ << "parameter: " << parameter;
+ *failure = Service::kFailureEAPAuthentication;
+ } else if (status == WPASupplicant::kEAPStatusStarted) {
+ LOG(INFO) << "EAP: Authentication starting.";
+ is_eap_in_progress_ = true;
+ }
+
+ return false;
+}
+
+void SupplicantEAPStateHandler::Reset() {
+ is_eap_in_progress_ = false;
+ tls_error_ = "";
+}
+
+} // namespace shill
diff --git a/supplicant_eap_state_handler.h b/supplicant_eap_state_handler.h
new file mode 100644
index 0000000..c0c80a8
--- /dev/null
+++ b/supplicant_eap_state_handler.h
@@ -0,0 +1,53 @@
+// 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.
+
+#ifndef SHILL_SUPPLICANT_EAP_STATE_HANDLER_H_
+#define SHILL_SUPPLICANT_EAP_STATE_HANDLER_H_
+
+#include <string>
+
+#include "shill/service.h"
+
+namespace shill {
+
+// This object tracks the state of wpa_supplicant's EAP association.
+// It parses events from wpa_supplicant and can notify callers when
+// wpa_supplicant succeeds or fails authentication. In the latter
+// case it can explain the failure in detail based on the course of
+// events leading up to it.
+class SupplicantEAPStateHandler {
+ public:
+ SupplicantEAPStateHandler();
+ virtual ~SupplicantEAPStateHandler();
+
+ // Receive the |status| and |parameter| from an EAP event and returns
+ // true if this state transition indicates that the EAP authentication
+ // process has succeeded. If instead the EAP authentication has failed,
+ // |failure| will be set to reflect the type of failure that occurred,
+ // false will be returned. If this EAP event has no direct outcome,
+ // this function returns false without changing |failure|.
+ virtual bool ParseStatus(const std::string &status,
+ const std::string ¶meter,
+ Service::ConnectFailure *failure);
+
+ // Resets the internal state of the handler.
+ virtual void Reset();
+
+ virtual bool is_eap_in_progress() { return is_eap_in_progress_; }
+
+ private:
+ friend class SupplicantEAPStateHandlerTest;
+
+ // The stored TLS error type which may lead to an EAP failure.
+ std::string tls_error_;
+
+ // Whether or not an EAP authentication is in progress. Note
+ // specifically that an EAP failure in wpa_supplicant does not
+ // automatically cause the EAP process to stop, while success does.
+ bool is_eap_in_progress_;
+};
+
+} // namespace shill
+
+#endif // SHILL_SUPPLICANT_EAP_STATE_HANDLER_H_
diff --git a/supplicant_eap_state_handler_unittest.cc b/supplicant_eap_state_handler_unittest.cc
new file mode 100644
index 0000000..642c471
--- /dev/null
+++ b/supplicant_eap_state_handler_unittest.cc
@@ -0,0 +1,156 @@
+// 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/supplicant_eap_state_handler.h"
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "shill/mock_log.h"
+#include "shill/wpa_supplicant.h"
+
+using std::string;
+using testing::_;
+using testing::EndsWith;
+using testing::Mock;
+
+namespace shill {
+
+class SupplicantEAPStateHandlerTest : public testing::Test {
+ public:
+ SupplicantEAPStateHandlerTest() : failure_(Service::kFailureUnknown) {}
+ virtual ~SupplicantEAPStateHandlerTest() {}
+
+ protected:
+ void StartEAP() {
+ EXPECT_CALL(log_, Log(logging::LOG_INFO, _,
+ EndsWith("Authentication starting.")));
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusStarted, "",
+ &failure_));
+ Mock::VerifyAndClearExpectations(&log_);
+ }
+
+ const string &GetTLSError() { return handler_.tls_error_; }
+
+ SupplicantEAPStateHandler handler_;
+ Service::ConnectFailure failure_;
+ ScopedMockLog log_;
+};
+
+TEST_F(SupplicantEAPStateHandlerTest, Construct) {
+ EXPECT_FALSE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, AuthenticationStarting) {
+ StartEAP();
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, AcceptedMethod) {
+ StartEAP();
+ const string kEAPMethod("EAP-ROCHAMBEAU");
+ EXPECT_CALL(log_, Log(logging::LOG_INFO, _,
+ EndsWith("accepted method " + kEAPMethod)));
+ EXPECT_FALSE(handler_.ParseStatus(
+ WPASupplicant::kEAPStatusAcceptProposedMethod, kEAPMethod, &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, SuccessfulCompletion) {
+ StartEAP();
+ EXPECT_CALL(log_, Log(_, _,
+ EndsWith("Completed authentication successfully.")));
+ EXPECT_TRUE(handler_.ParseStatus(WPASupplicant::kEAPStatusCompletion,
+ WPASupplicant::kEAPParameterSuccess,
+ &failure_));
+ EXPECT_FALSE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, EAPFailureGeneric) {
+ StartEAP();
+ // An EAP failure without a previous TLS indication yields a generic failure.
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusCompletion,
+ WPASupplicant::kEAPParameterFailure,
+ &failure_));
+
+ // Since it hasn't completed successfully, we must assume even in failure
+ // that wpa_supplicant is continuing the EAP authentication process.
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureEAPAuthentication, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, EAPFailureLocalTLSIndication) {
+ StartEAP();
+ // A TLS indication should be stored but a failure should not be returned.
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusLocalTLSAlert, "",
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ(WPASupplicant::kEAPStatusLocalTLSAlert, GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+
+ // An EAP failure with a previous TLS indication yields a specific failure.
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusCompletion,
+ WPASupplicant::kEAPParameterFailure,
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ(Service::kFailureEAPLocalTLS, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, EAPFailureRemoteTLSIndication) {
+ StartEAP();
+ // A TLS indication should be stored but a failure should not be returned.
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusRemoteTLSAlert, "",
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ(WPASupplicant::kEAPStatusRemoteTLSAlert, GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+
+ // An EAP failure with a previous TLS indication yields a specific failure.
+ EXPECT_FALSE(handler_.ParseStatus(WPASupplicant::kEAPStatusCompletion,
+ WPASupplicant::kEAPParameterFailure,
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ(Service::kFailureEAPRemoteTLS, failure_);
+}
+
+
+TEST_F(SupplicantEAPStateHandlerTest, BadRemoteCertificateVerification) {
+ StartEAP();
+ const string kStrangeParameter("ennui");
+ EXPECT_CALL(log_, Log(logging::LOG_ERROR, _,EndsWith(
+ string("Unexpected ") +
+ WPASupplicant::kEAPStatusRemoteCertificateVerification +
+ " parameter: " + kStrangeParameter)));
+ EXPECT_FALSE(handler_.ParseStatus(
+ WPASupplicant::kEAPStatusRemoteCertificateVerification, kStrangeParameter,
+ &failure_));
+ // Although we reported an error, this shouldn't mean failure.
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureUnknown, failure_);
+}
+
+TEST_F(SupplicantEAPStateHandlerTest, ParameterNeeded) {
+ StartEAP();
+ const string kAuthenticationParameter("nudge nudge say no more");
+ EXPECT_CALL(log_, Log(logging::LOG_ERROR, _,EndsWith(
+ string("aborted due to missing authentication parameter: ") +
+ kAuthenticationParameter)));
+ EXPECT_FALSE(handler_.ParseStatus(
+ WPASupplicant::kEAPStatusParameterNeeded, kAuthenticationParameter,
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailureEAPAuthentication, failure_);
+}
+
+} // namespace shill
diff --git a/wifi.cc b/wifi.cc
index 2136fee..3f1021d 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -36,6 +36,7 @@
#include "shill/rtnl_handler.h"
#include "shill/scope_logger.h"
#include "shill/shill_time.h"
+#include "shill/supplicant_eap_state_handler.h"
#include "shill/supplicant_interface_proxy_interface.h"
#include "shill/supplicant_network_proxy_interface.h"
#include "shill/supplicant_process_proxy_interface.h"
@@ -98,7 +99,7 @@
fast_scans_remaining_(kNumFastScanAttempts),
has_already_completed_(false),
is_debugging_connection_(false),
- is_eap_in_progress_(false),
+ eap_state_handler_(new SupplicantEAPStateHandler()),
bgscan_short_interval_seconds_(kDefaultBgscanShortIntervalSeconds),
bgscan_signal_threshold_dbm_(kDefaultBgscanSignalThresholdDbm),
scan_pending_(false),
@@ -525,7 +526,6 @@
SLOG(WiFi, 3) << "WiFi " << link_name() << " CurrentBSS "
<< supplicant_bss_ << " -> " << new_bss;
supplicant_bss_ = new_bss;
- supplicant_tls_error_ = "";
has_already_completed_ = false;
// Any change in CurrentBSS means supplicant is actively changing our
@@ -556,9 +556,9 @@
HandleRoam(new_bss);
}
- // Reset eap_in_progress_ after calling HandleDisconnect() so it can
- // be used to detect disconnects during EAP authentication.
- is_eap_in_progress_ = false;
+ // Reset the EAP handler only after calling HandleDisconnect() above
+ // so our EAP state could be used to detect a failed authentication.
+ eap_state_handler_->Reset();
// If we are selecting a new service, or if we're clearing selection
// of a something other than the pending service, call SelectService.
@@ -912,80 +912,25 @@
return;
}
- map<string, ::DBus::Variant>::const_iterator properties_it =
- properties.find(WPASupplicant::kInterfacePropertyDepth);
- if (properties_it == properties.end()) {
- LOG(ERROR) << __func__ << " no depth parameter.";
- return;
+ string subject;
+ uint32 depth;
+ if (WPASupplicant::ExtractRemoteCertification(properties, &subject, &depth)) {
+ current_service_->AddEAPCertification(subject, depth);
}
- uint32 depth = properties_it->second.reader().get_uint32();
- properties_it = properties.find(WPASupplicant::kInterfacePropertySubject);
- if (properties_it == properties.end()) {
- LOG(ERROR) << __func__ << " no subject parameter.";
- return;
- }
- string subject(properties_it->second.reader().get_string());
- current_service_->AddEAPCertification(subject, depth);
}
void WiFi::EAPEventTask(const string &status, const string ¶meter) {
- Service::ConnectFailure failure = Service::kFailureUnknown;
-
if (!current_service_) {
LOG(ERROR) << "WiFi " << link_name() << " " << __func__
<< " with no current service.";
return;
}
-
- if (status == WPASupplicant::kEAPStatusAcceptProposedMethod) {
- LOG(INFO) << link_name() << ": accepted EAP method " << parameter;
- } else if (status == WPASupplicant::kEAPStatusCompletion) {
- if (parameter == WPASupplicant::kEAPParameterSuccess) {
- SLOG(WiFi, 2) << link_name() << ": EAP: "
- << "Completed authentication.";
- is_eap_in_progress_ = false;
- } else if (parameter == WPASupplicant::kEAPParameterFailure) {
- // If there was a TLS error, use this instead of the generic failure.
- if (supplicant_tls_error_ == WPASupplicant::kEAPStatusLocalTLSAlert) {
- failure = Service::kFailureEAPLocalTLS;
- } else if (supplicant_tls_error_ ==
- WPASupplicant::kEAPStatusRemoteTLSAlert) {
- failure = Service::kFailureEAPRemoteTLS;
- } else {
- failure = Service::kFailureEAPAuthentication;
- }
- } else {
- LOG(ERROR) << link_name() << ": EAP: "
- << "Unexpected " << status << " parameter: " << parameter;
- }
- } else if (status == WPASupplicant::kEAPStatusLocalTLSAlert ||
- status == WPASupplicant::kEAPStatusRemoteTLSAlert) {
- supplicant_tls_error_ = status;
- } else if (status ==
- WPASupplicant::kEAPStatusRemoteCertificateVerification) {
- if (parameter == WPASupplicant::kEAPParameterSuccess) {
- SLOG(WiFi, 2) << link_name() << ": EAP: "
- << "Completed remote certificate verification.";
- } else {
- // wpa_supplicant doesn't currently have a verification failure
- // message. We will instead get a RemoteTLSAlert above.
- LOG(ERROR) << link_name() << ": EAP: "
- << "Unexpected " << status << " parameter: " << parameter;
- }
- } else if (status == WPASupplicant::kEAPStatusParameterNeeded) {
- LOG(ERROR) << link_name() << ": EAP: "
- << "Authentication due to missing authentication parameter: "
- << parameter;
- failure = Service::kFailureEAPAuthentication;
- } else if (status == WPASupplicant::kEAPStatusStarted) {
- SLOG(WiFi, 2) << link_name() << ": EAP authentication starting.";
- is_eap_in_progress_ = true;
- }
-
+ Service::ConnectFailure failure = Service::kFailureUnknown;
+ eap_state_handler_->ParseStatus(status, parameter, &failure);
if (failure != Service::kFailureUnknown) {
- // Avoid a reporting failure twice by clearing eap_in_progress_ early.
+ // Avoid a reporting failure twice by resetting EAP state handler early.
+ eap_state_handler_->Reset();
Error unused_error;
- is_eap_in_progress_ = false;
current_service_->DisconnectWithFailure(failure, &unused_error);
}
}
@@ -1171,7 +1116,8 @@
return true;
}
} else if (service.IsSecurityMatch(flimflam::kSecurity8021x)) {
- if (is_eap_in_progress_ && !service.has_ever_connected()) {
+ if (eap_state_handler_->is_eap_in_progress() &&
+ !service.has_ever_connected()) {
if (failure) {
*failure = Service::kFailureEAPAuthentication;
}
diff --git a/wifi.h b/wifi.h
index 66cbaf3..f90e427 100644
--- a/wifi.h
+++ b/wifi.h
@@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
-#ifndef SHILL_WIFI_
-#define SHILL_WIFI_
+#ifndef SHILL_WIFI_H_
+#define SHILL_WIFI_H_
// A WiFi device represents a wireless network interface implemented as an IEEE
// 802.11 station. An Access Point (AP) (or, more correctly, a Basic Service
@@ -81,6 +81,7 @@
#include <base/callback_forward.h>
#include <base/cancelable_callback.h>
+#include <base/memory/scoped_ptr.h>
#include <base/memory/weak_ptr.h>
#include <dbus-c++/dbus.h>
#include <gtest/gtest_prod.h> // for FRIEND_TEST
@@ -99,6 +100,7 @@
class GeolocationInfo;
class KeyValueStore;
class ProxyFactory;
+class SupplicantEAPStateHandler;
class SupplicantInterfaceProxyInterface;
class SupplicantProcessProxyInterface;
class WiFiProvider;
@@ -369,7 +371,6 @@
WiFiServiceRefPtr pending_service_;
std::string supplicant_state_;
std::string supplicant_bss_;
- std::string supplicant_tls_error_;
// Indicates that we should flush supplicant's BSS cache after the
// next scan completes.
bool need_bss_flush_;
@@ -389,8 +390,8 @@
bool has_already_completed_;
// Indicates that we are debugging a problematic connection.
bool is_debugging_connection_;
- // Indicates that we are in the middle of EAP authentication.
- bool is_eap_in_progress_;
+ // Tracks the process of an EAP negotiation.
+ scoped_ptr<SupplicantEAPStateHandler> eap_state_handler_;
// Properties
std::string bgscan_method_;
@@ -404,4 +405,4 @@
} // namespace shill
-#endif // SHILL_WIFI_
+#endif // SHILL_WIFI_H_
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 7413d6a..49f4cc2 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -46,6 +46,7 @@
#include "shill/mock_rtnl_handler.h"
#include "shill/mock_store.h"
#include "shill/mock_supplicant_bss_proxy.h"
+#include "shill/mock_supplicant_eap_state_handler.h"
#include "shill/mock_supplicant_interface_proxy.h"
#include "shill/mock_supplicant_network_proxy.h"
#include "shill/mock_supplicant_process_proxy.h"
@@ -212,6 +213,7 @@
dhcp_config_(new MockDHCPConfig(&control_interface_,
kDeviceName)),
dbus_manager_(new NiceMock<MockDBusManager>()),
+ eap_state_handler_(new NiceMock<MockSupplicantEAPStateHandler>()),
supplicant_interface_proxy_(
new NiceMock<MockSupplicantInterfaceProxy>(wifi_)),
proxy_factory_(this) {
@@ -225,7 +227,9 @@
WillByDefault(InvokeWithoutArgs(
this, &WiFiObjectTest::CreateSupplicantNetworkProxy));
- manager_.dbus_manager_.reset(dbus_manager_); // Transfers ownership.
+ // Transfers ownership.
+ manager_.dbus_manager_.reset(dbus_manager_);
+ wifi_->eap_state_handler_.reset(eap_state_handler_);
wifi_->provider_ = &wifi_provider_;
wifi_->time_ = &time_;
@@ -715,7 +719,11 @@
scoped_ptr<MockSupplicantBSSProxy> supplicant_bss_proxy_;
MockDHCPProvider dhcp_provider_;
scoped_refptr<MockDHCPConfig> dhcp_config_;
- NiceMock<MockDBusManager> *dbus_manager_;
+
+ // These pointers track mock objects owned by the WiFi device instance
+ // and manager so we can perform expectations against them.
+ MockDBusManager *dbus_manager_;
+ MockSupplicantEAPStateHandler *eap_state_handler_;
private:
scoped_ptr<MockSupplicantInterfaceProxy> supplicant_interface_proxy_;
@@ -1197,6 +1205,7 @@
kPath, WPASupplicant::kDBusAddr))
.WillOnce(Return(network_proxy));
EXPECT_CALL(*network_proxy, SetEnabled(false));
+ EXPECT_CALL(*eap_state_handler_, Reset());
EXPECT_CALL(*GetSupplicantInterfaceProxy(), RemoveNetwork(kPath)).Times(0);
ReportCurrentBSSChanged(WPASupplicant::kCurrentBSSNull);
EXPECT_EQ(NULL, GetCurrentService().get());
@@ -1834,23 +1843,22 @@
TEST_F(WiFiMainTest, SuspectCredentialsEAPInProgress) {
MockWiFiServiceRefPtr service = MakeMockService(flimflam::kSecurity8021x);
- SetCurrentService(service);
+ EXPECT_CALL(*eap_state_handler_, is_eap_in_progress())
+ .WillOnce(Return(false))
+ .WillOnce(Return(true))
+ .WillOnce(Return(false))
+ .WillOnce(Return(true));
service->has_ever_connected_ = false;
EXPECT_FALSE(SuspectCredentials(*service, NULL));
- ReportEAPEvent(WPASupplicant::kEAPStatusStarted, "");
- service->has_ever_connected_ = true;
- EXPECT_FALSE(SuspectCredentials(*service, NULL));
- service->has_ever_connected_ = false;
Service::ConnectFailure failure;
EXPECT_TRUE(SuspectCredentials(*service, &failure));
EXPECT_EQ(Service::kFailureEAPAuthentication, failure);
- ReportEAPEvent(WPASupplicant::kEAPStatusCompletion,
- WPASupplicant::kEAPParameterSuccess);
+ service->has_ever_connected_ = true;
+ EXPECT_FALSE(SuspectCredentials(*service, NULL));
EXPECT_FALSE(SuspectCredentials(*service, NULL));
}
TEST_F(WiFiMainTest, SuspectCredentialsYieldFailureWPA) {
- ScopedMockLog log;
MockWiFiServiceRefPtr service = MakeMockService(flimflam::kSecurityWpa);
SetPendingService(service);
ReportStateChanged(WPASupplicant::kInterfaceState4WayHandshake);
@@ -1859,6 +1867,7 @@
EXPECT_CALL(*service, SetFailure(Service::kFailureBadPassphrase));
EXPECT_CALL(*service, SetFailureSilent(_)).Times(0);
EXPECT_CALL(*service, SetState(_)).Times(0);
+ ScopedMockLog log;
EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
EndsWith(flimflam::kErrorBadPassphrase)));
@@ -1866,18 +1875,23 @@
}
TEST_F(WiFiMainTest, SuspectCredentialsYieldFailureEAP) {
- ScopedMockLog log;
MockWiFiServiceRefPtr service = MakeMockService(flimflam::kSecurity8021x);
SetCurrentService(service);
- ReportEAPEvent(WPASupplicant::kEAPStatusStarted, "");
EXPECT_FALSE(service->has_ever_connected());
- EXPECT_CALL(*service, SetFailure(Service::kFailureEAPAuthentication));
+ ScopedMockLog log;
+ EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
EXPECT_CALL(*service, SetFailureSilent(_)).Times(0);
EXPECT_CALL(*service, SetState(_)).Times(0);
- EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
+ // Ensure that we retrieve is_eap_in_progress() before resetting the
+ // EAP handler's state.
+ InSequence seq;
+ EXPECT_CALL(*eap_state_handler_, is_eap_in_progress())
+ .WillOnce(Return(true));
+ EXPECT_CALL(*service, SetFailure(Service::kFailureEAPAuthentication));
EXPECT_CALL(log, Log(logging::LOG_ERROR, _,
EndsWith(shill::kErrorEapAuthenticationFailed)));
+ EXPECT_CALL(*eap_state_handler_, Reset());
ReportCurrentBSSChanged(WPASupplicant::kCurrentBSSNull);
}
@@ -2005,71 +2019,28 @@
}
TEST_F(WiFiMainTest, EAPEvent) {
- MockWiFiServiceRefPtr service = MakeMockService(flimflam::kSecurity8021x);
- EXPECT_CALL(*service, SetFailure(_)).Times(0);
-
ScopedMockLog log;
EXPECT_CALL(log, Log(logging::LOG_ERROR, _, EndsWith("no current service.")));
- ReportEAPEvent(string(), string());
- Mock::VerifyAndClearExpectations(&log);
-
- SetCurrentService(service);
- const string kEAPMethod("EAP-ROCHAMBEAU");
- EXPECT_CALL(log, Log(logging::LOG_INFO, _,
- EndsWith("accepted EAP method " + kEAPMethod)));
- ReportEAPEvent(WPASupplicant::kEAPStatusAcceptProposedMethod, kEAPMethod);
-
- EXPECT_CALL(log, Log(_, _, EndsWith("Completed authentication."))).Times(1);
- ReportEAPEvent(WPASupplicant::kEAPStatusCompletion,
- WPASupplicant::kEAPParameterSuccess);
-
+ EXPECT_CALL(*eap_state_handler_, ParseStatus(_, _, _)).Times(0);
+ const string kEAPStatus("eap-status");
+ const string kEAPParameter("eap-parameter");
+ ReportEAPEvent(kEAPStatus, kEAPParameter);
Mock::VerifyAndClearExpectations(&log);
EXPECT_CALL(log, Log(_, _, _)).Times(AnyNumber());
- // An EAP failure without a previous TLS indication yields a generic failure.
- Mock::VerifyAndClearExpectations(service.get());
- EXPECT_CALL(*service, DisconnectWithFailure(
- Service::kFailureEAPAuthentication, NotNull()))
- .Times(1);
- ReportEAPEvent(WPASupplicant::kEAPStatusCompletion,
- WPASupplicant::kEAPParameterFailure);
- Mock::VerifyAndClearExpectations(service.get());
-
- // An EAP failure with a previous TLS indication yields a specific failure.
- SetCurrentService(service);
- EXPECT_CALL(*service, DisconnectWithFailure(_, _)).Times(0);
+ MockWiFiServiceRefPtr service = MakeMockService(flimflam::kSecurity8021x);
EXPECT_CALL(*service, SetFailure(_)).Times(0);
- ReportEAPEvent(WPASupplicant::kEAPStatusLocalTLSAlert, string());
- Mock::VerifyAndClearExpectations(service.get());
- EXPECT_CALL(*service,
- DisconnectWithFailure(Service::kFailureEAPLocalTLS, NotNull()))
- .Times(1);
- ReportEAPEvent(WPASupplicant::kEAPStatusCompletion,
- WPASupplicant::kEAPParameterFailure);
- Mock::VerifyAndClearExpectations(service.get());
-
+ EXPECT_CALL(*eap_state_handler_, ParseStatus(kEAPStatus, kEAPParameter, _));
SetCurrentService(service);
- EXPECT_CALL(*service, DisconnectWithFailure(_, _)).Times(0);
- EXPECT_CALL(*service, SetFailure(_)).Times(0);
- ReportEAPEvent(WPASupplicant::kEAPStatusRemoteTLSAlert, string());
- Mock::VerifyAndClearExpectations(service.get());
- EXPECT_CALL(*service,
- DisconnectWithFailure(Service::kFailureEAPRemoteTLS, NotNull()))
- .Times(1);
- ReportEAPEvent(WPASupplicant::kEAPStatusCompletion,
- WPASupplicant::kEAPParameterFailure);
- Mock::VerifyAndClearExpectations(service.get());
+ ReportEAPEvent(kEAPStatus, kEAPParameter);
+ Mock::VerifyAndClearExpectations(service);
+ Mock::VerifyAndClearExpectations(eap_state_handler_);
- SetCurrentService(service);
- EXPECT_CALL(*service, DisconnectWithFailure(_, _)).Times(0);
- EXPECT_CALL(*service, SetFailure(_)).Times(0);
- const string kStrangeParameter("ennui");
- EXPECT_CALL(log, Log(logging::LOG_ERROR, _,EndsWith(
- string("Unexpected ") +
- WPASupplicant::kEAPStatusRemoteCertificateVerification +
- " parameter: " + kStrangeParameter)));
- ReportEAPEvent(WPASupplicant::kEAPStatusRemoteCertificateVerification,
- kStrangeParameter);
+ EXPECT_CALL(*eap_state_handler_, ParseStatus(kEAPStatus, kEAPParameter, _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(Service::kFailureOutOfRange),
+ Return(false)));
+ EXPECT_CALL(*service, DisconnectWithFailure(Service::kFailureOutOfRange, _));
+ ReportEAPEvent(kEAPStatus, kEAPParameter);
}
TEST_F(WiFiMainTest, PendingScanDoesNotCrashAfterStop) {
diff --git a/wpa_supplicant.cc b/wpa_supplicant.cc
index 4a5bce5..7dc9179 100644
--- a/wpa_supplicant.cc
+++ b/wpa_supplicant.cc
@@ -223,4 +223,25 @@
}
}
+// static
+bool WPASupplicant::ExtractRemoteCertification(
+ const std::map<std::string, DBus::Variant> &properties,
+ std::string *subject, uint32 *depth) {
+ map<string, ::DBus::Variant>::const_iterator depth_it =
+ properties.find(WPASupplicant::kInterfacePropertyDepth);
+ if (depth_it == properties.end()) {
+ LOG(ERROR) << __func__ << " no depth parameter.";
+ return false;
+ }
+ map<string, ::DBus::Variant>::const_iterator subject_it =
+ properties.find(WPASupplicant::kInterfacePropertySubject);
+ if (subject_it == properties.end()) {
+ LOG(ERROR) << __func__ << " no subject parameter.";
+ return false;
+ }
+ *depth = depth_it->second.reader().get_uint32();
+ *subject = subject_it->second.reader().get_string();
+ return true;
+}
+
} // namespace shill
diff --git a/wpa_supplicant.h b/wpa_supplicant.h
index 695424d..a47c67d 100644
--- a/wpa_supplicant.h
+++ b/wpa_supplicant.h
@@ -142,6 +142,14 @@
const EapCredentials &eap, CertificateFile *certificate_file,
NSS *nss, const std::vector<char> nss_identifier,
std::map<std::string, DBus::Variant> *params);
+
+ // Retrieve the |subject| and |depth| of an a remote certifying entity,
+ // as contained the the |properties| to a Certification event from
+ // wpa_supplicant. Returns true if an |subject| and |depth| were
+ // extracted successfully, false otherwise.
+ static bool ExtractRemoteCertification(
+ const std::map<std::string, DBus::Variant> &properties,
+ std::string *subject, uint32 *depth);
};
} // namespace shill
diff --git a/wpa_supplicant_unittest.cc b/wpa_supplicant_unittest.cc
index 9eebd21..de8b681 100644
--- a/wpa_supplicant_unittest.cc
+++ b/wpa_supplicant_unittest.cc
@@ -11,12 +11,15 @@
#include "shill/eap_credentials.h"
#include "shill/mock_certificate_file.h"
+#include "shill/mock_log.h"
#include "shill/mock_nss.h"
using base::FilePath;
using std::map;
using std::string;
using std::vector;
+using testing::_;
+using testing::EndsWith;
using testing::Return;
namespace shill {
@@ -27,6 +30,7 @@
virtual ~WPASupplicantTest() {}
protected:
+ typedef std::map<std::string, DBus::Variant> PropertyMap;
void Populate() {
WPASupplicant::Populate8021xProperties(eap_, &certificate_file_,
@@ -108,4 +112,62 @@
}
}
+TEST_F(WPASupplicantTest, ExtractRemoteCertificationEmpty) {
+ string subject;
+ uint32 depth = 0;
+ ScopedMockLog log;
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _, EndsWith("no depth parameter.")));
+ EXPECT_FALSE(WPASupplicant::ExtractRemoteCertification(
+ PropertyMap(), &subject, &depth));
+ EXPECT_EQ("", subject);
+ EXPECT_EQ(0, depth);
+}
+
+TEST_F(WPASupplicantTest, ExtractRemoteCertificationDepthOnly) {
+ string subject;
+ const uint32 kDepthValue = 100;
+ uint32 depth = kDepthValue - 1;
+ PropertyMap property_map;
+ property_map[WPASupplicant::kInterfacePropertyDepth]
+ .writer().append_uint32(kDepthValue);
+ ScopedMockLog log;
+ EXPECT_CALL(log,
+ Log(logging::LOG_ERROR, _, EndsWith("no subject parameter.")));
+ EXPECT_FALSE(WPASupplicant::ExtractRemoteCertification(
+ property_map, &subject, &depth));
+ EXPECT_EQ("", subject);
+ EXPECT_NE(kDepthValue, depth);
+}
+
+TEST_F(WPASupplicantTest, ExtractRemoteCertificationSubjectOnly) {
+ const char kSubjectName[] = "subject-name";
+ string subject;
+ uint32 depth = 0;
+ PropertyMap property_map;
+ property_map[WPASupplicant::kInterfacePropertySubject]
+ .writer().append_string(kSubjectName);
+ ScopedMockLog log;
+ EXPECT_CALL(log, Log(logging::LOG_ERROR, _, EndsWith("no depth parameter.")));
+ EXPECT_FALSE(WPASupplicant::ExtractRemoteCertification(
+ property_map, &subject, &depth));
+ EXPECT_EQ("", subject);
+ EXPECT_EQ(0, depth);
+}
+
+TEST_F(WPASupplicantTest, ExtractRemoteCertificationSubjectAndDepth) {
+ const char kSubjectName[] = "subject-name";
+ string subject;
+ const uint32 kDepthValue = 100;
+ uint32 depth = 0;
+ PropertyMap property_map;
+ property_map[WPASupplicant::kInterfacePropertySubject]
+ .writer().append_string(kSubjectName);
+ property_map[WPASupplicant::kInterfacePropertyDepth]
+ .writer().append_uint32(kDepthValue);
+ EXPECT_TRUE(WPASupplicant::ExtractRemoteCertification(
+ property_map, &subject, &depth));
+ EXPECT_EQ(kSubjectName, subject);
+ EXPECT_EQ(kDepthValue, depth);
+}
+
} // namespace shill