Handle connecting to hidden networks correctly

chromeos/network/network_connection_handler.cc

 // Copyright (c) 2013 The Chromium 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 "chromeos/network/network_connection_handler.h"
 
 #include "base/bind.h"
 #include "base/command_line.h"
 #include "base/json/json_reader.h"
 #include "chromeos/chromeos_switches.h"
@@ skipping 213 matching lines @@
   // Clear any existing queued connect request.
   queued_connect_.reset();
   if (HasConnectingNetwork(service_path)) {
     NET_LOG_USER("Connect Request While Pending", service_path);
     InvokeErrorCallback(service_path, error_callback, kErrorConnecting);
     return;
   }
 
   // Check cached network state for connected, connecting, or unactivated
   // networks. These states will not be affected by a recent configuration.
+  // Note: NetworkState may not exist for a network that was recently
+  // configured, in which case these checks do not apply anyway.
   const NetworkState* network =
       network_state_handler_->GetNetworkState(service_path);
-  if (!network) {
-    InvokeErrorCallback(service_path, error_callback, kErrorNotFound);
-    return;
-  }
-  if (network->IsConnectedState()) {
-    InvokeErrorCallback(service_path, error_callback, kErrorConnected);
-    return;
-  }
-  if (network->IsConnectingState()) {
-    InvokeErrorCallback(service_path, error_callback, kErrorConnecting);
-    return;
-  }
-  if (NetworkRequiresActivation(network)) {
-    InvokeErrorCallback(service_path, error_callback, kErrorActivationRequired);
-    return;
-  }
 
-  if (check_error_state) {
-    const std::string& error = network->error();
-    if (error == flimflam::kErrorConnectFailed) {
-      InvokeErrorCallback(
-          service_path, error_callback, kErrorPassphraseRequired);
+  if (network) {
+    // For existing networks, perform some immediate consistency checks.
+    if (network->IsConnectedState()) {
+      InvokeErrorCallback(service_path, error_callback, kErrorConnected);
       return;
     }
-    if (error == flimflam::kErrorBadPassphrase) {
-      InvokeErrorCallback(
-          service_path, error_callback, kErrorPassphraseRequired);
+    if (network->IsConnectingState()) {
+      InvokeErrorCallback(service_path, error_callback, kErrorConnecting);
+      return;
+    }
+    if (NetworkRequiresActivation(network)) {
+      InvokeErrorCallback(service_path, error_callback,
+                          kErrorActivationRequired);
       return;
     }
 
-    if (IsAuthenticationError(error)) {
-      InvokeErrorCallback(
-          service_path, error_callback, kErrorAuthenticationRequired);
-      return;
+    if (check_error_state) {
+      const std::string& error = network->error();
+      if (error == flimflam::kErrorConnectFailed) {
+        InvokeErrorCallback(
+            service_path, error_callback, kErrorPassphraseRequired);
+        return;
+      }
+      if (error == flimflam::kErrorBadPassphrase) {
+        InvokeErrorCallback(
+            service_path, error_callback, kErrorPassphraseRequired);
+        return;
+      }
+      if (IsAuthenticationError(error)) {
+        InvokeErrorCallback(
+            service_path, error_callback, kErrorAuthenticationRequired);
+        return;
+      }
     }
   }
 
   // All synchronous checks passed, add |service_path| to connecting list.
   pending_requests_.insert(std::make_pair(
       service_path,
       ConnectRequest(service_path, success_callback, error_callback)));
 
-  // Connect immediately to 'connectable' networks.
-  // TODO(stevenjb): Shill needs to properly set Connectable for VPN.
-  if (network->connectable() && network->type() != flimflam::kTypeVPN) {
-    CallShillConnect(service_path);
-    return;
-  }
-
-  if (network->type() == flimflam::kTypeCellular ||
-      (network->type() == flimflam::kTypeWifi &&
-       network->security() == flimflam::kSecurityNone)) {
-    NET_LOG_ERROR("Network has no security but is not 'Connectable'",
-                  service_path);
-    CallShillConnect(service_path);
-    return;
+  if (network) {
+    // Connect immediately to 'connectable' networks.
+    // TODO(stevenjb): Shill needs to properly set Connectable for VPN.
+    if (network->connectable() && network->type() != flimflam::kTypeVPN) {

[pneubeck (no reviews), 2013/08/12 18:32:44]

NIT: combine the two ifs

+      CallShillConnect(service_path);
+      return;
+    }
   }
 
   // Request additional properties to check. VerifyConfiguredAndConnect will
   // use only these properties, not cached properties, to ensure that they
   // are up to date after any recent configuration.
   network_configuration_handler_->GetProperties(
-      network->path(),
+      service_path,
       base::Bind(&NetworkConnectionHandler::VerifyConfiguredAndConnect,
                  AsWeakPtr(), check_error_state),
       base::Bind(&NetworkConnectionHandler::HandleConfigurationFailure,
                  AsWeakPtr(), service_path));
 }
 
 void NetworkConnectionHandler::DisconnectNetwork(
     const std::string& service_path,
     const base::Closure& success_callback,
     const network_handler::ErrorCallback& error_callback) {
@@ skipping 35 matching lines @@
   CheckAllPendingRequests();
 }
 
 void NetworkConnectionHandler::NetworkPropertiesUpdated(
     const NetworkState* network) {
   if (HasConnectingNetwork(network->path()))
     CheckPendingRequest(network->path());
 }
 
 NetworkConnectionHandler::ConnectRequest*
-NetworkConnectionHandler::pending_request(
-    const std::string& service_path) {
+NetworkConnectionHandler::GetPendingRequest(const std::string& service_path) {
   std::map<std::string, ConnectRequest>::iterator iter =
       pending_requests_.find(service_path);
   return iter != pending_requests_.end() ? &(iter->second) : NULL;
 }
 
 // ConnectToNetwork implementation
 
 void NetworkConnectionHandler::VerifyConfiguredAndConnect(
     bool check_error_state,
     const std::string& service_path,
     const base::DictionaryValue& service_properties) {
   NET_LOG_EVENT("VerifyConfiguredAndConnect", service_path);
 
   // If 'passphrase_required' is still true, then the 'Passphrase' property
   // has not been set to a minimum length value.
   bool passphrase_required = false;
   service_properties.GetBooleanWithoutPathExpansion(
       flimflam::kPassphraseRequiredProperty, &passphrase_required);
   if (passphrase_required) {
     ErrorCallbackForPendingRequest(service_path, kErrorPassphraseRequired);
     return;
   }
 
-  std::string type;
+  std::string type, security;
   service_properties.GetStringWithoutPathExpansion(
       flimflam::kTypeProperty, &type);
+  service_properties.GetStringWithoutPathExpansion(
+      flimflam::kSecurityProperty, &security);
+  bool connectable = false;
+  service_properties.GetBooleanWithoutPathExpansion(
+      flimflam::kConnectableProperty, &connectable);
+
+  // In case NetworkState was not available in ConnectToNetwork (e.g. it had
+  // been recently configured), we need to check Connectable again.
+  if (connectable && type != flimflam::kTypeVPN) {
+    // TODO(stevenjb): Shill needs to properly set Connectable for VPN.
+    CallShillConnect(service_path);
+    return;
+  }
 
   // Get VPN provider type and host (required for configuration) and ensure
   // that required VPN non-cert properties are set.
   std::string vpn_provider_type, vpn_provider_host;
   if (type == flimflam::kTypeVPN) {
     // VPN Provider values are read from the "Provider" dictionary, not the
     // "Provider.Type", etc keys (which are used only to set the values).
     const base::DictionaryValue* provider_properties;
     if (service_properties.GetDictionaryWithoutPathExpansion(
             flimflam::kProviderProperty, &provider_properties)) {
@@ skipping 10 matching lines @@
     if (!VPNIsConfigured(
             service_path, vpn_provider_type, *provider_properties)) {
       NET_LOG_ERROR("VPN Not Configured", service_path);
       ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
       return;
     }
   }
 
   client_cert::ConfigType client_cert_type = client_cert::CONFIG_TYPE_NONE;
   if (type == flimflam::kTypeVPN) {
-      if (vpn_provider_type == flimflam::kProviderOpenVpn)
-        client_cert_type = client_cert::CONFIG_TYPE_OPENVPN;
-      else
-        client_cert_type = client_cert::CONFIG_TYPE_IPSEC;
-  } else if (type == flimflam::kTypeWifi) {
+    if (vpn_provider_type == flimflam::kProviderOpenVpn)
+      client_cert_type = client_cert::CONFIG_TYPE_OPENVPN;
+    else
+      client_cert_type = client_cert::CONFIG_TYPE_IPSEC;
+  } else if (type == flimflam::kTypeWifi &&
+             security == flimflam::kSecurity8021x) {
     client_cert_type = client_cert::CONFIG_TYPE_EAP;
   }
 
   base::DictionaryValue config_properties;
   if (client_cert_type != client_cert::CONFIG_TYPE_NONE) {
     // If the client certificate must be configured, this will be set to a
     // non-empty string.
     std::string pkcs11_id;
 
     // Check certificate properties in kUIDataProperty if configured.
     // Note: Wifi/VPNConfigView set these properties explicitly, in which case
     //   only the TPM must be configured.
     scoped_ptr<NetworkUIData> ui_data =
         ManagedNetworkConfigurationHandler::GetUIData(service_properties);
     if (ui_data && ui_data->certificate_type() == CLIENT_CERT_TYPE_PATTERN) {
       // User must be logged in to connect to a network requiring a certificate.
       if (!logged_in_ || !cert_loader_) {
         ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
         return;
       }
 
       // If certificates have not been loaded yet, queue the connect request.
       if (!certificates_loaded_) {
-        ConnectRequest* request = pending_request(service_path);
-        DCHECK(request);
+        ConnectRequest* request = GetPendingRequest(service_path);
+        if (!request) {
+          NET_LOG_ERROR("No pending request to queue", service_path);
+          return;
+        }
         NET_LOG_EVENT("Connect Request Queued", service_path);
         queued_connect_.reset(new ConnectRequest(
             service_path, request->success_callback, request->error_callback));
         pending_requests_.erase(service_path);
         return;
       }
 
       pkcs11_id = CertificateIsConfigured(ui_data.get());
       // Ensure the certificate is available and configured.
       if (!cert_loader_->IsHardwareBacked() || pkcs11_id.empty()) {
@@ skipping 24 matching lines @@
     CopyStringFromDictionary(service_properties, flimflam::kNameProperty,
                              &config_properties);
     CopyStringFromDictionary(service_properties, flimflam::kGuidProperty,
                              &config_properties);
     if (type == flimflam::kTypeVPN) {
       config_properties.SetStringWithoutPathExpansion(
           flimflam::kProviderTypeProperty, vpn_provider_type);
       config_properties.SetStringWithoutPathExpansion(
           flimflam::kProviderHostProperty, vpn_provider_host);
     } else if (type == flimflam::kTypeWifi) {
-      CopyStringFromDictionary(service_properties, flimflam::kSecurityProperty,
-                               &config_properties);
+      config_properties.SetStringWithoutPathExpansion(
+          flimflam::kSecurityProperty, security);
     }
 
     network_configuration_handler_->SetProperties(
         service_path,
         config_properties,
         base::Bind(&NetworkConnectionHandler::CallShillConnect,
                    AsWeakPtr(), service_path),
         base::Bind(&NetworkConnectionHandler::HandleConfigurationFailure,
                    AsWeakPtr(), service_path));
    return;
@@ skipping 17 matching lines @@
       base::Bind(&NetworkConnectionHandler::HandleShillConnectSuccess,
                  AsWeakPtr(), service_path),
       base::Bind(&NetworkConnectionHandler::HandleShillConnectFailure,
                  AsWeakPtr(), service_path));
 }
 
 void NetworkConnectionHandler::HandleConfigurationFailure(
     const std::string& service_path,
     const std::string& error_name,
     scoped_ptr<base::DictionaryValue> error_data) {
-  ConnectRequest* request = pending_request(service_path);
-  DCHECK(request);
+  ConnectRequest* request = GetPendingRequest(service_path);
+  if (!request) {
+    NET_LOG_ERROR("HandleConfigurationFailure called with no pending request.",
+                  service_path);
+    return;
+  }
   network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
   if (!error_callback.is_null())
     error_callback.Run(kErrorConfigureFailed, error_data.Pass());
 }
 
 void NetworkConnectionHandler::HandleShillConnectSuccess(
     const std::string& service_path) {
-  ConnectRequest* request = pending_request(service_path);
-  DCHECK(request);
+  ConnectRequest* request = GetPendingRequest(service_path);
+  if (!request) {
+    NET_LOG_ERROR("HandleShillConnectSuccess called with no pending request.",
+                  service_path);
+    return;
+  }
   request->connect_state = ConnectRequest::CONNECT_STARTED;
   NET_LOG_EVENT("Connect Request Acknowledged", service_path);
   // Do not call success_callback here, wait for one of the following
   // conditions:
   // * State transitions to a non connecting state indicating succes or failure
   // * Network is no longer in the visible list, indicating failure
   CheckPendingRequest(service_path);
 }
 
 void NetworkConnectionHandler::HandleShillConnectFailure(
     const std::string& service_path,
     const std::string& dbus_error_name,
     const std::string& dbus_error_message) {
-  ConnectRequest* request = pending_request(service_path);
-  DCHECK(request);
+  ConnectRequest* request = GetPendingRequest(service_path);
+  if (!request) {
+    NET_LOG_ERROR("HandleShillConnectFailure called with no pending request.",
+                  service_path);
+    return;
+  }
   network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
   network_handler::ShillErrorCallbackFunction(
       kErrorConnectFailed, service_path, error_callback,
       dbus_error_name, dbus_error_message);
 }
 
 void NetworkConnectionHandler::CheckPendingRequest(
     const std::string service_path) {
-  ConnectRequest* request = pending_request(service_path);
+  ConnectRequest* request = GetPendingRequest(service_path);
   DCHECK(request);
   if (request->connect_state == ConnectRequest::CONNECT_REQUESTED)
     return;  // Request has not started, ignore update
   const NetworkState* network =
       network_state_handler_->GetNetworkState(service_path);
-  if (!network) {
-    ErrorCallbackForPendingRequest(service_path, kErrorNotFound);
-    return;
-  }
+  if (!network)
+    return;  // NetworkState may not be be updated yet.
+
   if (network->IsConnectingState()) {
     request->connect_state = ConnectRequest::CONNECT_CONNECTING;
     return;
   }
   if (network->IsConnectedState()) {
     NET_LOG_EVENT("Connect Request Succeeded", service_path);
     if (!request->success_callback.is_null())
       request->success_callback.Run();
     pending_requests_.erase(service_path);
     return;
@@ skipping 48 matching lines @@
   scoped_refptr<net::X509Certificate> matching_cert =
       client_cert::GetCertificateMatch(ui_data->certificate_pattern());
   if (!matching_cert.get())
     return std::string();
   return CertLoader::GetPkcs11IdForCert(*matching_cert.get());
 }
 
 void NetworkConnectionHandler::ErrorCallbackForPendingRequest(
     const std::string& service_path,
     const std::string& error_name) {
-  ConnectRequest* request = pending_request(service_path);
-  DCHECK(request);
+  ConnectRequest* request = GetPendingRequest(service_path);
+  if (!request) {
+    NET_LOG_ERROR("ErrorCallbackForPendingRequest with no pending request.",
+                  service_path);
+    return;
+  }
   // Remove the entry before invoking the callback in case it triggers a retry.
   network_handler::ErrorCallback error_callback = request->error_callback;
   pending_requests_.erase(service_path);
   InvokeErrorCallback(service_path, error_callback, error_name);
 }
 
 // Disconnect
 
 void NetworkConnectionHandler::CallShillDisconnect(
     const std::string& service_path,
@@ skipping 35 matching lines @@
 
 void NetworkConnectionHandler::HandleShillActivateSuccess(
     const std::string& service_path,
     const base::Closure& success_callback) {
   NET_LOG_EVENT("Activate Request Sent", service_path);
   if (!success_callback.is_null())
     success_callback.Run();
 }
 
 }  // namespace chromeos