shill: WiFi: Disable gateway ARP for static IP Even when a static IP address is configured for a WiFi service, we perform a DHCP lease negotiation in the interest of gaining other parameters associated with the connection (DNS servers, proxy configuration, etc). Shortly thereafter, we release such a lease. Since we never intend on actually using the IP address provided by the DHCP server, the validity of the IP address it provides is not of any interest. As such, it's reasonable to disable the "gateway ARP" address validation. BUG=chromium:377990 TEST=New unit tests. Note that an DHCP autotest using pseudoethernet won't help validate this change since it is not WiFi, and as such gateway ARP is disabled across-the-board. Change-Id: I7f3e096370189698f900f85d78592fd42017ffbd Reviewed-on: https://chromium-review.googlesource.com/202176 Reviewed-by: Paul Stewart <pstew@chromium.org> Commit-Queue: Paul Stewart <pstew@chromium.org> Tested-by: Paul Stewart <pstew@chromium.org>

diff --git a/device.cc b/device.cc
index 806ce00..639529e 100644
--- a/device.cc
+++ b/device.cc

@@ -440,6 +440,13 @@
   return selected_service_ && selected_service_->ShouldUseMinimalDHCPConfig();
 }
 
+bool Device::IsUsingStaticIP() const {
+  if (!selected_service_) {
+    return false;
+  }
+  return selected_service_->HasStaticIPAddress();
+}
+
 bool Device::AcquireIPConfig() {
   return AcquireIPConfigWithLeaseName(string());
 }
@@ -513,9 +520,7 @@
     return;
   }
 
-  const StaticIPParameters &static_ip_parameters =
-      selected_service_->static_ip_parameters();
-  if (static_ip_parameters.ContainsAddress()) {
+  if (IsUsingStaticIP()) {
     SLOG(Device, 2) << __func__ << " " << " configuring static IP parameters.";
     // If the parameters contain an IP address, apply them now and bring
     // the interface up.  When DHCP information arrives, it will supplement
@@ -536,7 +541,7 @@
   if (selected_service_) {
     ipconfig->ApplyStaticIPParameters(
         selected_service_->mutable_static_ip_parameters());
-    if (selected_service_->static_ip_parameters().ContainsAddress()) {
+    if (IsUsingStaticIP()) {
       // If we are using a statically configured IP address instead
       // of a leased IP address, release any acquired lease so it may
       // be used by others.  This allows us to merge other non-leased
@@ -578,7 +583,7 @@
   if (selected_service_) {
     selected_service_->OnDHCPFailure();
 
-    if (selected_service_->static_ip_parameters().ContainsAddress()) {
+    if (IsUsingStaticIP()) {
       // Consider three cases:
       //
       // 1. We're here because DHCP failed while starting up. There

diff --git a/device.h b/device.h
index e1408db..7a4dc08 100644
--- a/device.h
+++ b/device.h

@@ -466,6 +466,9 @@
   // to alleviate issues with receiving a reply.
   bool ShouldUseMinimalDHCPConfig() const;
 
+  // Indicates if the selected service is configured with a static IP address.
+  bool IsUsingStaticIP() const;
+
   const ServiceRefPtr &selected_service() const { return selected_service_; }
 
   void HelpRegisterConstDerivedString(

diff --git a/mock_wifi_service.h b/mock_wifi_service.h
index 241c19e..9449fbc 100644
--- a/mock_wifi_service.h
+++ b/mock_wifi_service.h

@@ -54,6 +54,7 @@
   MOCK_METHOD0(ResetWiFi, void());
   MOCK_CONST_METHOD0(GetSupplicantConfigurationParameters, DBusPropertiesMap());
   MOCK_CONST_METHOD1(IsAutoConnectable, bool(const char **reason));
+  MOCK_CONST_METHOD0(HasStaticIPAddress, bool());
 
  private:
   DISALLOW_COPY_AND_ASSIGN(MockWiFiService);

diff --git a/service.cc b/service.cc
index 2aa4467..ca2b09a 100644
--- a/service.cc
+++ b/service.cc

@@ -765,6 +765,10 @@
   remote_certification_.clear();
 }
 
+bool Service::HasStaticIPAddress() const {
+  return static_ip_parameters().ContainsAddress();
+}
+
 void Service::SetAutoConnect(bool connect) {
   if (auto_connect() == connect) {
     return;

diff --git a/service.h b/service.h
index 6ff3e03..5e89583 100644
--- a/service.h
+++ b/service.h

@@ -310,6 +310,10 @@
   // Clear all EAP certification elements.
   virtual void ClearEAPCertification();
 
+  // Returns true if this service contains a IP address in its static IP
+  // parameters, false otherwise.
+  virtual bool HasStaticIPAddress() const;
+
   // The inherited class that needs to send metrics after the service has
   // transitioned to the ready state should override this method.
   // |time_resume_to_ready_milliseconds| holds the elapsed time from when

diff --git a/wifi.cc b/wifi.cc
index 8e5dcc1..3054695 100644
--- a/wifi.cc
+++ b/wifi.cc

@@ -1577,7 +1577,7 @@
 }
 
 bool WiFi::ShouldUseArpGateway() const {
-  return true;
+  return !IsUsingStaticIP();
 }
 
 void WiFi::DisassociateFromService(const WiFiServiceRefPtr &service) {

diff --git a/wifi_unittest.cc b/wifi_unittest.cc
index 1aa66ec..fdd3864 100644
--- a/wifi_unittest.cc
+++ b/wifi_unittest.cc

@@ -1218,9 +1218,31 @@
 }
 
 TEST_F(WiFiMainTest, UseArpGateway) {
+  StartWiFi();
+
+  // With no selected service.
+  EXPECT_TRUE(wifi()->ShouldUseArpGateway());
   EXPECT_CALL(dhcp_provider_, CreateConfig(kDeviceName, _, _, true, false))
       .WillOnce(Return(dhcp_config_));
   const_cast<WiFi *>(wifi().get())->AcquireIPConfig();
+
+  MockWiFiServiceRefPtr service = MakeMockService(kSecurityNone);
+  InitiateConnect(service);
+
+  // Selected service that does not have a static IP address.
+  EXPECT_CALL(*service, HasStaticIPAddress()).WillRepeatedly(Return(false));
+  EXPECT_TRUE(wifi()->ShouldUseArpGateway());
+  EXPECT_CALL(dhcp_provider_, CreateConfig(kDeviceName, _, _, true, false))
+      .WillOnce(Return(dhcp_config_));
+  const_cast<WiFi *>(wifi().get())->AcquireIPConfig();
+  Mock::VerifyAndClearExpectations(service);
+
+  // Selected service that has a static IP address.
+  EXPECT_CALL(*service, HasStaticIPAddress()).WillRepeatedly(Return(true));
+  EXPECT_FALSE(wifi()->ShouldUseArpGateway());
+  EXPECT_CALL(dhcp_provider_, CreateConfig(kDeviceName, _, _, false, false))
+      .WillOnce(Return(dhcp_config_));
+  const_cast<WiFi *>(wifi().get())->AcquireIPConfig();
 }
 
 TEST_F(WiFiMainTest, RemoveNetworkWhenSupplicantReturnsInvalidArgs) {