Skip checking certificate properties for L2TP/IPsec VPN using pre-shared key.

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 "base/location.h"
@@ skipping 356 matching lines @@
   // been recently configured), we need to check Connectable again.
   if (connectable && type != shill::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.
   const base::DictionaryValue* provider_properties = NULL;
-  std::string vpn_provider_type, vpn_provider_host;
+  std::string vpn_provider_type, vpn_provider_host, vpn_client_cert_id;
   if (type == shill::kTypeVPN) {
     // VPN Provider values are read from the "Provider" dictionary, not the
     // "Provider.Type", etc keys (which are used only to set the values).
     if (service_properties.GetDictionaryWithoutPathExpansion(
             shill::kProviderProperty, &provider_properties)) {
       provider_properties->GetStringWithoutPathExpansion(
           shill::kTypeProperty, &vpn_provider_type);
       provider_properties->GetStringWithoutPathExpansion(
           shill::kHostProperty, &vpn_provider_host);
+      provider_properties->GetStringWithoutPathExpansion(
+          shill::kL2tpIpsecClientCertIdProperty, &vpn_client_cert_id);
     }
     if (vpn_provider_type.empty() || vpn_provider_host.empty()) {
       ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
       return;
     }
   }
 
+  scoped_ptr<NetworkUIData> ui_data =
+      shill_property_util::GetUIDataFromProperties(service_properties);
+  bool has_certificate_in_onc =
+      ui_data && (ui_data->certificate_type() != CLIENT_CERT_TYPE_NONE);

[stevenjb, 2014/02/13 21:15:33]

It looks like this bool is only used for kTypeVPN. We should probably move it
there, and even better do something like this:

if (vpn_provider_type == shill::kProviderOpenVpn) {
  client_cert_type = client_cert::CONFIG_TYPE_OPENVPN;
} else {
  // L2TP/IPSec only requires a certificate if one is specified in ONC
  // or one was configured by the UI. Otherwise it is L2TP/IPSec with
  // PSK and doesn't require a certificate.
  // TODO(benchan)...
  if (!vpn_client_cert_id.empty() ||
      (ui_data && ui_data->certificate_type() != CLIENT_CERT_TYPE_NONE))
    client_cert_type = client_cert::CONFIG_TYPE_IPSEC;
}

[Ben Chan, 2014/02/13 21:30:58]

Done.

+
   client_cert::ConfigType client_cert_type = client_cert::CONFIG_TYPE_NONE;
   if (type == shill::kTypeVPN) {
     if (vpn_provider_type == shill::kProviderOpenVpn)
       client_cert_type = client_cert::CONFIG_TYPE_OPENVPN;
-    else
+    else if (has_certificate_in_onc || !vpn_client_cert_id.empty()) {
+      // If UIData doesn't contain a certificate (in ONC case), or if
+      // |vpn_client_cert_id| is empty (in non-ONC case), it's L2TP/IPsec with
+      // PSK and doesn't require a certificate.
+
+      // TODO(benchan): Modify shill to specify the authentication type via the
+      // kL2tpIpsecAuthenticationType property, so that Chrome doesn't need to
+      // deduce the authentication type based on the
+      // kL2tpIpsecClientCertIdProperty here (and also in VPNConfigView).
       client_cert_type = client_cert::CONFIG_TYPE_IPSEC;
+    }
   } else if (type == shill::kTypeWifi && security == shill::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 =
-        shill_property_util::GetUIDataFromProperties(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_) {
         NET_LOG_EVENT("Certificates not loaded", "");
@@ skipping 41 matching lines @@
   if (type == shill::kTypeVPN) {
     // VPN may require a username, and/or passphrase to be set. (Check after
     // ensuring that any required certificates are configured).
     DCHECK(provider_properties);
     if (VPNRequiresCredentials(
             service_path, vpn_provider_type, *provider_properties)) {
       NET_LOG_USER("VPN Requires Credentials", service_path);
       ErrorCallbackForPendingRequest(service_path, kErrorConfigurationRequired);
       return;
     }
+
+    // If it's L2TP/IPsec PSK, there is no properties to configure, so proceed
+    // to connect.
+    if (client_cert_type == client_cert::CONFIG_TYPE_NONE) {
+      CallShillConnect(service_path);
+      return;
+    }
   }
 
   if (!config_properties.empty()) {
     NET_LOG_EVENT("Configuring Network", service_path);
     network_configuration_handler_->SetProperties(
         service_path,
         config_properties,
         base::Bind(&NetworkConnectionHandler::CallShillConnect,
                    AsWeakPtr(),
                    service_path),
@@ skipping 190 matching lines @@
 
 void NetworkConnectionHandler::HandleShillDisconnectSuccess(
     const std::string& service_path,
     const base::Closure& success_callback) {
   NET_LOG_EVENT("Disconnect Request Sent", service_path);
   if (!success_callback.is_null())
     success_callback.Run();
 }
 
 }  // namespace chromeos