shill: WiFi: Provide wpa_supplicant a PIN if asked for
There are conditions under which wpa_supplicant will clear the
"pin" property of its configuration, even if this was previously
supplied by shill. Under these circumstances it wil request it
again as the EAP negotiation starts. This change provides the
means for the WiFi object to detect when this parameter is being
requested and to supply this value to wpa_supplicant if the
WiFiService has this information on hand.
BUG=chromium:310296
TEST=Unit tests + new autotest:
https://chromium-review.googlesource.com/174170
This autotest still fails the following but shows in the supplicant logs:
CTRL_IFACE: response handle field=PIN
EAPOL: received control response (user input) notification - retrying pending EAP Request
indicating that the PIN has been successfully provided to wpa_supplicant
Change-Id: I91bc3ddd01a335f93d20cca4d47ca497bd631ebe
Reviewed-on: https://chromium-review.googlesource.com/174180
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Queue: Paul Stewart <pstew@chromium.org>
Tested-by: Paul Stewart <pstew@chromium.org>
diff --git a/dbus_bindings/supplicant-interface.xml b/dbus_bindings/supplicant-interface.xml
index d7ca543..d04ae01 100644
--- a/dbus_bindings/supplicant-interface.xml
+++ b/dbus_bindings/supplicant-interface.xml
@@ -43,6 +43,11 @@
</method>
<method name="EAPLogoff"/>
<method name="EAPLogon"/>
+ <method name="NetworkReply">
+ <arg name="network" type="o" direction="in"/>
+ <arg name="field" type="s" direction="in"/>
+ <arg name="value" type="s" direction="in"/>
+ </method>
<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/eap_credentials.h b/eap_credentials.h
index 3ad0ed3..afacd44 100644
--- a/eap_credentials.h
+++ b/eap_credentials.h
@@ -109,6 +109,7 @@
virtual void set_password(const std::string &password) {
password_ = password;
}
+ virtual const std::string &pin() const { return pin_; }
private:
friend class EapCredentialsTest;
diff --git a/mock_eap_credentials.h b/mock_eap_credentials.h
index 43991da..5986714 100644
--- a/mock_eap_credentials.h
+++ b/mock_eap_credentials.h
@@ -35,6 +35,7 @@
MOCK_CONST_METHOD0(identity, const std::string &());
MOCK_CONST_METHOD0(key_management, const std::string &());
MOCK_METHOD1(set_password, void(const std::string &password));
+ MOCK_CONST_METHOD0(pin, const std::string &());
private:
std::string kDefaultKeyManagement;
diff --git a/mock_supplicant_interface_proxy.h b/mock_supplicant_interface_proxy.h
index 849b1e4..19e0849 100644
--- a/mock_supplicant_interface_proxy.h
+++ b/mock_supplicant_interface_proxy.h
@@ -29,6 +29,9 @@
MOCK_METHOD0(EAPLogon, void());
MOCK_METHOD0(Disconnect, void());
MOCK_METHOD1(FlushBSS, void(const uint32_t &age));
+ MOCK_METHOD3(NetworkReply, void(const ::DBus::Path &network,
+ const std::string &field,
+ const std::string &value));
MOCK_METHOD0(Reassociate, void());
MOCK_METHOD0(RemoveAllNetworks, void());
MOCK_METHOD1(RemoveNetwork, void(const ::DBus::Path &network));
diff --git a/service.h b/service.h
index 561a8ab..cb3c3d8 100644
--- a/service.h
+++ b/service.h
@@ -586,6 +586,7 @@
FRIEND_TEST(ServiceTest, Unload);
FRIEND_TEST(WiFiServiceTest, SuspectedCredentialFailure);
FRIEND_TEST(WiFiTimerTest, ReconnectTimer);
+ FRIEND_TEST(WiFiMainTest, EAPEvent); // For eap_.
static const char kAutoConnConnected[];
static const char kAutoConnConnecting[];
diff --git a/supplicant_eap_state_handler.cc b/supplicant_eap_state_handler.cc
index 39c9421..fac9a32 100644
--- a/supplicant_eap_state_handler.cc
+++ b/supplicant_eap_state_handler.cc
@@ -52,9 +52,15 @@
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;
+ if (parameter == WPASupplicant::kEAPRequestedParameterPIN) {
+ // wpa_supplicant could have erased the PIN. Signal to WiFi that
+ // it should supply one if possible.
+ *failure = Service::kFailurePinMissing;
+ } else {
+ 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;
diff --git a/supplicant_eap_state_handler_unittest.cc b/supplicant_eap_state_handler_unittest.cc
index 642c471..cae11dd 100644
--- a/supplicant_eap_state_handler_unittest.cc
+++ b/supplicant_eap_state_handler_unittest.cc
@@ -153,4 +153,15 @@
EXPECT_EQ(Service::kFailureEAPAuthentication, failure_);
}
+TEST_F(SupplicantEAPStateHandlerTest, ParameterNeededPin) {
+ StartEAP();
+ EXPECT_FALSE(handler_.ParseStatus(
+ WPASupplicant::kEAPStatusParameterNeeded,
+ WPASupplicant::kEAPRequestedParameterPIN,
+ &failure_));
+ EXPECT_TRUE(handler_.is_eap_in_progress());
+ EXPECT_EQ("", GetTLSError());
+ EXPECT_EQ(Service::kFailurePinMissing, failure_);
+}
+
} // namespace shill
diff --git a/supplicant_interface_proxy.cc b/supplicant_interface_proxy.cc
index 2ecdbbb..163fb43 100644
--- a/supplicant_interface_proxy.cc
+++ b/supplicant_interface_proxy.cc
@@ -27,7 +27,7 @@
SupplicantInterfaceProxy::~SupplicantInterfaceProxy() {}
::DBus::Path SupplicantInterfaceProxy::AddNetwork(
- const std::map<std::string, ::DBus::Variant> &args) {
+ const map<string, ::DBus::Variant> &args) {
SLOG(DBus, 2) << __func__;
try {
return proxy_.AddNetwork(args);
@@ -88,6 +88,18 @@
}
}
+void SupplicantInterfaceProxy::NetworkReply(const ::DBus::Path &network,
+ const string &field,
+ const string &value) {
+ SLOG(DBus, 2) << __func__;
+ try {
+ return proxy_.NetworkReply(network, field, value);
+ } catch (const DBus::Error &e) {
+ LOG(ERROR) << "DBus exception: " << e.name() << ": " << e.what();
+ throw; // Re-throw the exception.
+ }
+}
+
void SupplicantInterfaceProxy::Reassociate() {
SLOG(DBus, 2) << __func__;
try {
@@ -117,8 +129,7 @@
}
}
-void SupplicantInterfaceProxy::Scan(
- const std::map<std::string, ::DBus::Variant> &args) {
+void SupplicantInterfaceProxy::Scan(const map<string, ::DBus::Variant> &args) {
SLOG(DBus, 2) << __func__;
try {
return proxy_.Scan(args);
diff --git a/supplicant_interface_proxy.h b/supplicant_interface_proxy.h
index b3dab30..c2cce33 100644
--- a/supplicant_interface_proxy.h
+++ b/supplicant_interface_proxy.h
@@ -39,6 +39,9 @@
virtual void EAPLogoff();
virtual void Disconnect();
virtual void FlushBSS(const uint32_t &age);
+ virtual void NetworkReply(const ::DBus::Path &network,
+ const std::string &field,
+ const std::string &value);
virtual void Reassociate();
virtual void RemoveAllNetworks();
virtual void RemoveNetwork(const ::DBus::Path &network);
diff --git a/supplicant_interface_proxy_interface.h b/supplicant_interface_proxy_interface.h
index aa9afcb..02bbc9e 100644
--- a/supplicant_interface_proxy_interface.h
+++ b/supplicant_interface_proxy_interface.h
@@ -25,6 +25,9 @@
virtual void EAPLogon() = 0;
virtual void Disconnect() = 0;
virtual void FlushBSS(const uint32_t &age) = 0;
+ virtual void NetworkReply(const ::DBus::Path &network,
+ const std::string &field,
+ const std::string &value) = 0;
virtual void Reassociate() = 0;
virtual void RemoveAllNetworks() = 0;
virtual void RemoveNetwork(const ::DBus::Path &network) = 0;
diff --git a/wifi.cc b/wifi.cc
index 50e2081..41d01f8 100644
--- a/wifi.cc
+++ b/wifi.cc
@@ -27,6 +27,7 @@
#include "shill/control_interface.h"
#include "shill/dbus_adaptor.h"
#include "shill/device.h"
+#include "shill/eap_credentials.h"
#include "shill/error.h"
#include "shill/file_reader.h"
#include "shill/geolocation_info.h"
@@ -1145,6 +1146,19 @@
}
Service::ConnectFailure failure = Service::kFailureUnknown;
eap_state_handler_->ParseStatus(status, parameter, &failure);
+ if (failure == Service::kFailurePinMissing) {
+ // wpa_supplicant can sometimes forget the PIN on disconnect from the AP.
+ const string &pin = current_service_->eap()->pin();
+ Error unused_error;
+ string rpcid = FindNetworkRpcidForService(current_service_, &unused_error);
+ if (!pin.empty() && !rpcid.empty()) {
+ // We have a PIN configured, so we can provide it back to wpa_supplicant.
+ LOG(INFO) << "Re-supplying PIN parameter to wpa_supplicant.";
+ supplicant_interface_proxy_->NetworkReply(
+ rpcid, WPASupplicant::kEAPRequestedParameterPIN, pin);
+ failure = Service::kFailureUnknown;
+ }
+ }
if (failure != Service::kFailureUnknown) {
// Avoid a reporting failure twice by resetting EAP state handler early.
eap_state_handler_->Reset();
diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 125704d..f6bbe2f 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc
@@ -38,6 +38,7 @@
#include "shill/mock_device_info.h"
#include "shill/mock_dhcp_config.h"
#include "shill/mock_dhcp_provider.h"
+#include "shill/mock_eap_credentials.h"
#include "shill/mock_event_dispatcher.h"
#include "shill/mock_link_monitor.h"
#include "shill/mock_log.h"
@@ -89,6 +90,7 @@
using ::testing::Mock;
using ::testing::NiceMock;
using ::testing::NotNull;
+using ::testing::Ref;
using ::testing::Return;
using ::testing::ReturnRef;
using ::testing::SaveArg;
@@ -710,6 +712,10 @@
void SetPendingService(const WiFiServiceRefPtr &service) {
wifi_->SetPendingService(service);
}
+ void SetServiceNetworkRpcId(
+ const WiFiServiceRefPtr &service, const string &rpcid) {
+ wifi_->rpcid_by_service_[service.get()] = rpcid;
+ }
bool SetScanInterval(uint16_t interval_seconds, Error *error) {
return wifi_->SetScanInterval(interval_seconds, error);
}
@@ -2842,6 +2848,7 @@
}
TEST_F(WiFiMainTest, EAPEvent) {
+ StartWiFi();
ScopedMockLog log;
EXPECT_CALL(log, Log(logging::LOG_ERROR, _, EndsWith("no current service.")));
EXPECT_CALL(*eap_state_handler_, ParseStatus(_, _, _)).Times(0);
@@ -2864,6 +2871,32 @@
Return(false)));
EXPECT_CALL(*service, DisconnectWithFailure(Service::kFailureOutOfRange, _));
ReportEAPEvent(kEAPStatus, kEAPParameter);
+
+ MockEapCredentials *eap = new MockEapCredentials();
+ service->eap_.reset(eap); // Passes ownership.
+ const char kNetworkRpcId[] = "/service/network/rpcid";
+ SetServiceNetworkRpcId(service, kNetworkRpcId);
+ EXPECT_CALL(*eap_state_handler_, ParseStatus(kEAPStatus, kEAPParameter, _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(Service::kFailurePinMissing),
+ Return(false)));
+ // We need a real string object since it will be returned by reference below.
+ const string kEmptyPin;
+ EXPECT_CALL(*eap, pin()).WillOnce(ReturnRef(kEmptyPin));
+ EXPECT_CALL(*service, DisconnectWithFailure(Service::kFailurePinMissing, _));
+ ReportEAPEvent(kEAPStatus, kEAPParameter);
+
+ EXPECT_CALL(*eap_state_handler_, ParseStatus(kEAPStatus, kEAPParameter, _))
+ .WillOnce(DoAll(SetArgumentPointee<2>(Service::kFailurePinMissing),
+ Return(false)));
+ // We need a real string object since it will be returned by reference below.
+ const string kPin("000000");
+ EXPECT_CALL(*eap, pin()).WillOnce(ReturnRef(kPin));
+ EXPECT_CALL(*service, DisconnectWithFailure(_, _)).Times(0);
+ EXPECT_CALL(*GetSupplicantInterfaceProxy(),
+ NetworkReply(StrEq(kNetworkRpcId),
+ StrEq(WPASupplicant::kEAPRequestedParameterPIN),
+ Ref(kPin)));
+ ReportEAPEvent(kEAPStatus, kEAPParameter);
}
TEST_F(WiFiMainTest, PendingScanDoesNotCrashAfterStop) {
diff --git a/wpa_supplicant.cc b/wpa_supplicant.cc
index 10a875b..d7a4acc 100644
--- a/wpa_supplicant.cc
+++ b/wpa_supplicant.cc
@@ -46,6 +46,7 @@
const char WPASupplicant::kEAPParameterAlertUnknownCA[] = "unknown CA";
const char WPASupplicant::kEAPParameterFailure[] = "failure";
const char WPASupplicant::kEAPParameterSuccess[] = "success";
+const char WPASupplicant::kEAPRequestedParameterPIN[] = "PIN";
const char WPASupplicant::kEAPStatusAcceptProposedMethod[] =
"accept proposed method";
const char WPASupplicant::kEAPStatusCompletion[] = "completion";
diff --git a/wpa_supplicant.h b/wpa_supplicant.h
index 466f5bd..9736bc0 100644
--- a/wpa_supplicant.h
+++ b/wpa_supplicant.h
@@ -38,6 +38,7 @@
static const char kEAPParameterAlertUnknownCA[];
static const char kEAPParameterFailure[];
static const char kEAPParameterSuccess[];
+ static const char kEAPRequestedParameterPIN[];
static const char kEAPStatusAcceptProposedMethod[];
static const char kEAPStatusCompletion[];
static const char kEAPStatusLocalTLSAlert[];