Add ONC VPN support for OpenVPN and L2TP/IPsec VPNs

chrome/browser/chromeos/cros/onc_network_parser.cc

 // Copyright (c) 2011 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 "chrome/browser/chromeos/cros/onc_network_parser.h"
 
 #include "base/base64.h"
 #include "base/json/json_value_serializer.h"
+#include "base/json/json_writer.h"  // for debug output only.
 #include "base/stringprintf.h"
 #include "base/values.h"
 #include "chrome/browser/chromeos/cros/native_network_constants.h"
 #include "chrome/browser/chromeos/cros/native_network_parser.h"
 #include "chrome/browser/chromeos/cros/network_library.h"
 #include "net/base/cert_database.h"
 #include "net/base/crypto_module.h"
 #include "net/base/net_errors.h"
 #include "net/base/x509_certificate.h"
 #include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
 // Local constants.
 namespace {
 
-EnumMapper<PropertyIndex>::Pair property_index_table[] = {
-    { "GUID", PROPERTY_INDEX_GUID },
-    { "Name", PROPERTY_INDEX_NAME },
-    { "Remove", PROPERTY_INDEX_REMOVE },
-    { "ProxyURL", PROPERTY_INDEX_PROXY_CONFIG },
-    { "Type", PROPERTY_INDEX_TYPE },
-    { "SSID", PROPERTY_INDEX_SSID },
-    { "Passphrase", PROPERTY_INDEX_PASSPHRASE },
-    { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT },
-    { "HiddenSSID", PROPERTY_INDEX_HIDDEN_SSID },
-    { "Security", PROPERTY_INDEX_SECURITY },
-    { "EAP", PROPERTY_INDEX_EAP },
-    { "Outer", PROPERTY_INDEX_EAP_METHOD },
-    { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH },
-    { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS },
-    { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT },
-    { "ClientCARef", PROPERTY_INDEX_EAP_CLIENT_CERT },
-    { "ClientCertPattern", PROPERTY_INDEX_EAP_CLIENT_CERT_PATTERN },
-    { "Identity", PROPERTY_INDEX_EAP_IDENTITY },
-    { "Password", PROPERTY_INDEX_EAP_PASSWORD },
-    { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY },
+const base::Value::Type TYPE_BOOLEAN = base::Value::TYPE_BOOLEAN;
+const base::Value::Type TYPE_DICTIONARY = base::Value::TYPE_DICTIONARY;
+const base::Value::Type TYPE_INTEGER = base::Value::TYPE_INTEGER;
+const base::Value::Type TYPE_LIST = base::Value::TYPE_LIST;
+const base::Value::Type TYPE_STRING = base::Value::TYPE_STRING;
+
+// Only used currently to keep NetworkParser superclass happy.
+EnumMapper<PropertyIndex>::Pair network_configuration_table[] = {
+  { "GUID", PROPERTY_INDEX_GUID }
+};
+
+OncValueSignature network_configuration_signature[] = {
+  // TODO(crosbug.com/23673): Support Ethernet settings.
+  { "GUID", PROPERTY_INDEX_GUID, TYPE_STRING },
+  { "Name", PROPERTY_INDEX_NAME, TYPE_STRING },
+  // TODO(crosbug.com/23674): Support ProxySettings.
+  // TODO(crosbug.com/23604): Handle removing networks.
+  { "Remove", PROPERTY_INDEX_ONC_REMOVE, TYPE_BOOLEAN },
+  { "Type", PROPERTY_INDEX_TYPE, TYPE_STRING },
+  { "WiFi", PROPERTY_INDEX_ONC_WIFI, TYPE_DICTIONARY },
+  { "VPN", PROPERTY_INDEX_ONC_VPN, TYPE_DICTIONARY }
+};
+
+OncValueSignature wifi_signature[] = {
+  { "AutoConnect", PROPERTY_INDEX_AUTO_CONNECT, TYPE_BOOLEAN },
+  { "EAP", PROPERTY_INDEX_EAP, TYPE_DICTIONARY },
+  { "HiddenSSID", PROPERTY_INDEX_HIDDEN_SSID, TYPE_BOOLEAN },
+  { "Passphrase", PROPERTY_INDEX_PASSPHRASE, TYPE_STRING },
+  { "ProxyURL", PROPERTY_INDEX_PROXY_CONFIG, TYPE_STRING },
+  { "Security", PROPERTY_INDEX_SECURITY, TYPE_STRING },
+  { "SSID", PROPERTY_INDEX_SSID, TYPE_STRING },
+  { NULL }
+};
+
+OncValueSignature eap_signature[] = {
+  { "AnonymousIdentity", PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY, TYPE_STRING },
+  { "ClientCertPattern", PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN,
+    TYPE_DICTIONARY },
+  { "ClientCertRef", PROPERTY_INDEX_ONC_CLIENT_CERT_REF, TYPE_STRING },
+  { "ClientCertType", PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE, TYPE_STRING },
+  { "Identity", PROPERTY_INDEX_EAP_IDENTITY, TYPE_STRING },
+  { "Inner", PROPERTY_INDEX_EAP_PHASE_2_AUTH, TYPE_STRING },
+  { "Outer", PROPERTY_INDEX_EAP_METHOD, TYPE_STRING },
+  { "Password", PROPERTY_INDEX_EAP_PASSWORD, TYPE_STRING },
+  { "ServerCARef", PROPERTY_INDEX_EAP_CA_CERT, TYPE_STRING },
+  { "UseSystemCAs", PROPERTY_INDEX_EAP_USE_SYSTEM_CAS, TYPE_BOOLEAN },
+  { NULL }
+};
+
+OncValueSignature vpn_signature[] = {
+  { "Host", PROPERTY_INDEX_HOST, TYPE_STRING },
+  { "IPsec", PROPERTY_INDEX_ONC_IPSEC, TYPE_DICTIONARY },
+  { "L2TP", PROPERTY_INDEX_ONC_L2TP, TYPE_DICTIONARY },
+  { "OpenVPN", PROPERTY_INDEX_ONC_OPENVPN, TYPE_DICTIONARY },
+  { "Type", PROPERTY_INDEX_PROVIDER_TYPE, TYPE_STRING },
+  { NULL }
+};
+
+OncValueSignature ipsec_signature[] = {
+  { "AuthenticationType", PROPERTY_INDEX_IPSEC_AUTHENTICATIONTYPE,
+    TYPE_STRING },
+  { "Group", PROPERTY_INDEX_L2TPIPSEC_GROUP_NAME, TYPE_STRING },
+  { "IKEVersion", PROPERTY_INDEX_IPSEC_IKEVERSION, TYPE_INTEGER },
+  { "ClientCertPattern", PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN,
+    TYPE_DICTIONARY },
+  { "ClientCertRef", PROPERTY_INDEX_ONC_CLIENT_CERT_REF, TYPE_STRING },
+  { "ClientCertType", PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE, TYPE_STRING },
+  // Note: EAP and XAUTH not yet supported.
+  { "PSK", PROPERTY_INDEX_L2TPIPSEC_PSK, TYPE_STRING },
+  { "SaveCredentials", PROPERTY_INDEX_SAVE_CREDENTIALS, TYPE_BOOLEAN },
+  { "ServerCARef", PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS, TYPE_STRING },
+  { NULL }
+};
+
+OncValueSignature l2tp_signature[] = {
+  { "Password", PROPERTY_INDEX_L2TPIPSEC_PASSWORD, TYPE_STRING },
+  { "SaveCredentials", PROPERTY_INDEX_SAVE_CREDENTIALS, TYPE_BOOLEAN },
+  { "Username", PROPERTY_INDEX_L2TPIPSEC_USER, TYPE_STRING },
+  { NULL }
+};
+
+OncValueSignature openvpn_signature[] = {
+  { "Auth", PROPERTY_INDEX_OPEN_VPN_AUTH, TYPE_STRING },
+  { "AuthRetry", PROPERTY_INDEX_OPEN_VPN_AUTHRETRY, TYPE_STRING },
+  { "AuthNoCache", PROPERTY_INDEX_OPEN_VPN_AUTHNOCACHE, TYPE_BOOLEAN },
+  { "Cipher", PROPERTY_INDEX_OPEN_VPN_CIPHER, TYPE_STRING },
+  { "ClientCertPattern", PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN,
+    TYPE_DICTIONARY },
+  { "ClientCertRef", PROPERTY_INDEX_ONC_CLIENT_CERT_REF, TYPE_STRING },
+  { "ClientCertType", PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE, TYPE_STRING },
+  { "CompLZO", PROPERTY_INDEX_OPEN_VPN_COMPLZO, TYPE_STRING },
+  { "CompNoAdapt", PROPERTY_INDEX_OPEN_VPN_COMPNOADAPT, TYPE_BOOLEAN },
+  { "KeyDirection", PROPERTY_INDEX_OPEN_VPN_KEYDIRECTION, TYPE_STRING },
+  { "NsCertType", PROPERTY_INDEX_OPEN_VPN_NSCERTTYPE, TYPE_STRING },
+  { "Password", PROPERTY_INDEX_OPEN_VPN_PASSWORD, TYPE_STRING },
+  { "Port", PROPERTY_INDEX_OPEN_VPN_PORT, TYPE_INTEGER },
+  { "Proto", PROPERTY_INDEX_OPEN_VPN_PROTO, TYPE_STRING },
+  { "PushPeerInfo", PROPERTY_INDEX_OPEN_VPN_PUSHPEERINFO, TYPE_BOOLEAN },
+  { "RemoteCertEKU", PROPERTY_INDEX_OPEN_VPN_REMOTECERTEKU, TYPE_STRING },
+  { "RemoteCertKU", PROPERTY_INDEX_OPEN_VPN_REMOTECERTKU, TYPE_LIST },
+  { "RemoteCertTLS", PROPERTY_INDEX_OPEN_VPN_REMOTECERTTLS, TYPE_STRING },
+  { "RenegSec", PROPERTY_INDEX_OPEN_VPN_RENEGSEC, TYPE_INTEGER },
+  { "SaveCredentials", PROPERTY_INDEX_SAVE_CREDENTIALS, TYPE_BOOLEAN },
+  { "ServerCARef", PROPERTY_INDEX_OPEN_VPN_CACERT, TYPE_STRING },
+  { "ServerCertRef", PROPERTY_INDEX_OPEN_VPN_CERT, TYPE_STRING },
+  { "ServerPollTimeout", PROPERTY_INDEX_OPEN_VPN_SERVERPOLLTIMEOUT,
+    TYPE_INTEGER },
+  { "Shaper", PROPERTY_INDEX_OPEN_VPN_SHAPER, TYPE_INTEGER },
+  { "StaticChallenge", PROPERTY_INDEX_OPEN_VPN_STATICCHALLENGE, TYPE_STRING },
+  { "TLSAuthContents", PROPERTY_INDEX_OPEN_VPN_TLSAUTHCONTENTS, TYPE_STRING },
+  { "TLSRemote", PROPERTY_INDEX_OPEN_VPN_TLSREMOTE, TYPE_STRING },
+  { "Username", PROPERTY_INDEX_OPEN_VPN_USER, TYPE_STRING },
+  { NULL }
 };
 
 // Serve the singleton mapper instance.
 const EnumMapper<PropertyIndex>* get_onc_mapper() {
   CR_DEFINE_STATIC_LOCAL(const EnumMapper<PropertyIndex>, mapper,
-      (property_index_table,
-       arraysize(property_index_table),
+      (network_configuration_table,
+       arraysize(network_configuration_table),
        PROPERTY_INDEX_UNKNOWN));
   return &mapper;
 }
 
 ConnectionType ParseNetworkType(const std::string& type) {
   static EnumMapper<ConnectionType>::Pair table[] = {
     { "WiFi", TYPE_WIFI },
     { "VPN", TYPE_VPN },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ConnectionType>, parser,
       (table, arraysize(table), TYPE_UNKNOWN));
   return parser.Get(type);
 }
 
 }  // namespace
 
 // -------------------- OncNetworkParser --------------------
 
 OncNetworkParser::OncNetworkParser(const std::string& onc_blob)
     : NetworkParser(get_onc_mapper()),
       network_configs_(NULL),
       certificates_(NULL) {
+  VLOG(2) << __func__ << ": OncNetworkParser called on " << onc_blob;
   JSONStringValueSerializer deserializer(onc_blob);
   deserializer.set_allow_trailing_comma(true);
   scoped_ptr<base::Value> root(deserializer.Deserialize(NULL, &parse_error_));
 
   if (!root.get() || root->GetType() != base::Value::TYPE_DICTIONARY) {
     LOG(WARNING) << "OncNetworkParser received bad ONC file: " << parse_error_;
   } else {
     root_dict_.reset(static_cast<DictionaryValue*>(root.release()));
     // At least one of NetworkConfigurations or Certificates is required.
     if (!root_dict_->GetList("NetworkConfigurations", &network_configs_) &&
@@ skipping 63 matching lines @@
                << " at index " << cert_index;
   return false;
 }
 
 Network* OncNetworkParser::ParseNetwork(int n) {
   if (!network_configs_)
     return NULL;
   DictionaryValue* info = NULL;
   if (!network_configs_->GetDictionary(n, &info))
     return NULL;
-  // Parse Open Network Configuration blob into a temporary Network object.
   return CreateNetworkFromInfo(std::string(), *info);
 }
 
 Network* OncNetworkParser::CreateNetworkFromInfo(
     const std::string& service_path,
     const DictionaryValue& info) {
   ConnectionType type = ParseTypeFromDictionary(info);
   if (type == TYPE_UNKNOWN)  // Return NULL if cannot parse network type.
     return NULL;
   scoped_ptr<Network> network(CreateNewNetwork(type, service_path));
-  // Update property with native value for type.
-  std::string str = NativeNetworkParser::network_type_mapper()->GetKey(type);
-  scoped_ptr<StringValue> val(Value::CreateStringValue(str));
-  network->UpdatePropertyMap(PROPERTY_INDEX_TYPE, *val.get());
-
-  // Get the child dictionary with properties for the network.
-  // And copy all the values from this network type dictionary to parent.
-  DictionaryValue* dict;
-  if (!info.GetDictionary(GetTypeFromDictionary(info), &dict))
-    return NULL;
-
-  // Add GUID from the parent dictionary.
-  dict->SetString("GUID", GetGuidFromDictionary(info));
-
-  UpdateNetworkFromInfo(*dict, network.get());
+  ParseNestedObject(network.get(),
+                    "NetworkConfiguration",
+                    static_cast<const base::Value&>(info),
+                    network_configuration_signature,
+                    ParseNetworkConfigurationValue);
   VLOG(2) << "Created Network '" << network->name()
           << "' from info. Path:" << service_path
           << " Type:" << ConnectionTypeToString(type);
   return network.release();
 }
 
 Network* OncNetworkParser::CreateNewNetwork(
     ConnectionType type, const std::string& service_path) {
   Network* network = NetworkParser::CreateNewNetwork(type, service_path);
   if (network) {
@@ skipping 128 matching lines @@
       module.get(), decoded_pkcs12, string16(), false);
   if (result != net::OK) {
     LOG(WARNING) << "ONC File: Unable to import Client certificate at index "
                  << cert_index
                  << " (error " << net::ErrorToString(result) << ").";
     return false;
   }
   return true;
 }
 
+bool OncNetworkParser::ParseNestedObject(Network* network,
+                                         const std::string& onc_type,
+                                         const base::Value& value,
+                                         OncValueSignature* signature,
+                                         ParserPointer parser) {
+  bool any_errors = false;
+  if (!value.IsType(base::Value::TYPE_DICTIONARY)) {
+    VLOG(1) << network->name() << ": expected object of type " << onc_type;
+    return false;
+  }
+  VLOG(2) << "Parsing nested object of type " << onc_type;
+  const DictionaryValue* dict = NULL;
+  value.GetAsDictionary(&dict);
+  for (DictionaryValue::key_iterator iter = dict->begin_keys();
+       iter != dict->end_keys(); ++iter) {
+    const std::string& key = *iter;
+    base::Value* inner_value = NULL;
+    dict->GetWithoutPathExpansion(key, &inner_value);
+    int field_index;
+    for (field_index = 0; signature[field_index].field != NULL; ++field_index) {
+      if (key == signature[field_index].field)
+        break;
+    }
+    if (signature[field_index].field == NULL) {
+      VLOG(1) << network->name() << ": unexpected field: "
+              << key << ", in type: " << onc_type;
+      any_errors = true;
+      continue;
+    }
+    if (!inner_value->IsType(signature[field_index].type)) {
+      VLOG(1) << network->name() << ": field with wrong type: " << key
+              << ", actual type: " << inner_value->GetType()
+              << ", expected type: " << signature[field_index].type;
+      any_errors = true;
+      continue;
+    }
+    PropertyIndex index = signature[field_index].index;
+    // We need to UpdatePropertyMap now since parser might want to
+    // change the mapped value.
+    network->UpdatePropertyMap(index, *inner_value);
+    if (!parser(this, index, *inner_value, network)) {
+      VLOG(1) << network->name() << ": problem(s) with field: " << key;
+      any_errors = true;
+      continue;
+    }
+    if (VLOG_IS_ON(2)) {
+      std::string value_json;
+      base::JSONWriter::Write(inner_value, true, &value_json);
+      VLOG(2) << network->name() << ": Successfully parsed [" << key
+              << "(" << index << ")] = " << value_json;
+    }
+  }
+  return !any_errors;
+}
+
+bool OncNetworkParser::ParseNetworkConfigurationValue(
+    OncNetworkParser* parser,
+    PropertyIndex index,
+    const base::Value& value,
+    Network* network) {
+  switch (index) {
+    case PROPERTY_INDEX_ONC_WIFI: {
+      if (TYPE_WIFI != network->type()) {
+        VLOG(2) << network->name()
+                << ": WiFi field unexpected for this type network";
+        return false;
+      }
+      return parser->ParseNestedObject(network,
+                                       "WiFi",
+                                       value,
+                                       wifi_signature,
+                                       OncWifiNetworkParser::ParseWifiValue);
+    }
+    case PROPERTY_INDEX_ONC_VPN: {
+      if (TYPE_VPN != network->type()) {
+        VLOG(2) << network->name()
+                << ": VPN field unexpected for this type network";
+        return false;
+      }
+      VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
+      // Got the "VPN" field.  Immediately store the VPN.Type field
+      // value so that we can properly validate fields in the VPN
+      // object based on the type.
+      const DictionaryValue* dict = NULL;
+      CHECK(value.GetAsDictionary(&dict));
+      std::string provider_type_string;
+      if (!dict->GetString("Type", &provider_type_string)) {
+        VLOG(1) << network->name() << ": VPN.Type is missing";
+        return false;
+      }
+      ProviderType provider_type =
+        OncVirtualNetworkParser::ParseProviderType(provider_type_string);
+      virtual_network->set_provider_type(provider_type);
+      return parser->ParseNestedObject(network,
+                                       "VPN",
+                                       value,
+                                       vpn_signature,
+                                       OncVirtualNetworkParser::ParseVPNValue);
+      return true;
+    }
+    case PROPERTY_INDEX_ONC_REMOVE:
+      VLOG(1) << network->name() << ": Remove field not yet implemented";
+      return false;
+    case PROPERTY_INDEX_TYPE: {
+      // Update property with native value for type.
+      std::string str =
+        NativeNetworkParser::network_type_mapper()->GetKey(network->type());
+      scoped_ptr<StringValue> val(Value::CreateStringValue(str));
+      network->UpdatePropertyMap(PROPERTY_INDEX_TYPE, *val.get());
+      return true;
+    }
+    case PROPERTY_INDEX_GUID:
+    case PROPERTY_INDEX_NAME:
+      // Fall back to generic parser for these.
+      return parser->ParseValue(index, value, network);
+    default:
+      break;
+  }
+  return false;
+}
+
 // -------------------- OncWirelessNetworkParser --------------------
 
 OncWirelessNetworkParser::OncWirelessNetworkParser() {}
 OncWirelessNetworkParser::~OncWirelessNetworkParser() {}
 
-bool OncWirelessNetworkParser::ParseValue(PropertyIndex index,
-                                          const base::Value& value,
-                                          Network* network) {
-  DCHECK_NE(TYPE_ETHERNET, network->type());
-  DCHECK_NE(TYPE_VPN, network->type());
-  return OncNetworkParser::ParseValue(index, value, network);
-}
-
 // -------------------- OncWifiNetworkParser --------------------
 
 OncWifiNetworkParser::OncWifiNetworkParser() {}
+
 OncWifiNetworkParser::~OncWifiNetworkParser() {}
 
-bool OncWifiNetworkParser::ParseValue(PropertyIndex index,
-                                      const base::Value& value,
-                                      Network* network) {
-  DCHECK_EQ(TYPE_WIFI, network->type());
+bool OncWifiNetworkParser::ParseWifiValue(OncNetworkParser* parser,
+                                          PropertyIndex index,
+                                          const base::Value& value,
+                                          Network* network) {

[stevenjb, 2011/12/02 21:34:45]

We should keep the check before doing the static cast. Really, we should do a
proper test and return false if |network| is not TYPE_WIFI.

[kmixter1, 2011/12/03 00:20:34]

Done.

   WifiNetwork* wifi_network = static_cast<WifiNetwork*>(network);
+  std::string string_value;
+  bool bool_value = false;

[stevenjb, 2011/12/02 21:34:45]

nit: I'm not a fan of extracting something as two different types. I'd rather
have lightweight wrappers, e.g. std::string ValueAsString(value) and
ValueAsBool() and call those instead.

[kmixter1, 2011/12/03 00:20:34]

Done.

+
+  value.GetAsString(&string_value);
+  value.GetAsBoolean(&bool_value);
   switch (index) {
-    case PROPERTY_INDEX_SSID: {
-      std::string ssid;
-      if (!value.GetAsString(&ssid))
-        break;
-      wifi_network->SetName(ssid);
+    case PROPERTY_INDEX_SSID:
+      wifi_network->SetName(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_GUID: {
-      std::string unique_id;
-      if (!value.GetAsString(&unique_id))
-        break;
-      wifi_network->set_unique_id(unique_id);
-      return true;
-    }
     case PROPERTY_INDEX_SECURITY: {
-      std::string security_string;
-      if (!value.GetAsString(&security_string))
-        break;
-      ConnectionSecurity security = ParseSecurity(security_string);
+      ConnectionSecurity security = ParseSecurity(string_value);
       wifi_network->set_encryption(security);
       // Also update property with native value for security.
       std::string str =
           NativeNetworkParser::network_security_mapper()->GetKey(security);
       scoped_ptr<StringValue> val(Value::CreateStringValue(str));
       wifi_network->UpdatePropertyMap(index, *val.get());
       return true;
     }
-    case PROPERTY_INDEX_PASSPHRASE: {
-      std::string passphrase;
-      if (!value.GetAsString(&passphrase))
-        break;
-      wifi_network->set_passphrase(passphrase);
+    case PROPERTY_INDEX_PASSPHRASE:
+      wifi_network->set_passphrase(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_IDENTITY: {
-      std::string identity;
-      if (!value.GetAsString(&identity))
-        break;
-      wifi_network->set_identity(identity);
+    case PROPERTY_INDEX_IDENTITY:
+      wifi_network->set_identity(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_EAP: {
-      DCHECK_EQ(value.GetType(), Value::TYPE_DICTIONARY);
-      const DictionaryValue& dict = static_cast<const DictionaryValue&>(value);
-      for (DictionaryValue::key_iterator iter = dict.begin_keys();
-           iter != dict.end_keys(); ++iter) {
-        const std::string& key = *iter;
-        base::Value* eap_value;
-        bool res = dict.GetWithoutPathExpansion(key, &eap_value);
-        DCHECK(res);
-        if (res) {
-          PropertyIndex index = mapper().Get(key);
-          wifi_network->UpdatePropertyMap(index, *eap_value);
-          if (!ParseEAPValue(index, *eap_value, wifi_network))
-            VLOG(1) << network->name() << ": EAP unhandled key: " << key
-                    << " Type: " << eap_value->GetType();
-        }
-      }
+    case PROPERTY_INDEX_EAP:
+      parser->ParseNestedObject(wifi_network,
+                                "EAP",
+                                value,
+                                eap_signature,
+                                ParseEAPValue);
       return true;
-    }
+    case PROPERTY_INDEX_AUTO_CONNECT:
+      network->set_auto_connect(bool_value);
+      return true;
+    case PROPERTY_INDEX_HIDDEN_SSID:
+      // Pass this through to connection manager as is.
+      return true;
     default:
-      return OncWirelessNetworkParser::ParseValue(index, value, network);
+      break;
   }
   return false;
 }
 
+bool OncWifiNetworkParser::ParseEAPValue(OncNetworkParser*,
+                                         PropertyIndex index,
+                                         const base::Value& value,
+                                         Network* network) {

[stevenjb, 2011/12/02 21:34:45]

Same comment about static cast.

[kmixter1, 2011/12/03 00:20:34]

Done.

+  WifiNetwork* wifi_network = static_cast<WifiNetwork*>(network);
+  std::string string_value;
+  bool bool_value = false;
 
-bool OncWifiNetworkParser::ParseEAPValue(PropertyIndex index,
-                                         const base::Value& value,
-                                         WifiNetwork* wifi_network) {
+  value.GetAsString(&string_value);
+  value.GetAsBoolean(&bool_value);

[stevenjb, 2011/12/02 21:34:45]

same comment about multiple extraction.

[kmixter1, 2011/12/03 00:20:34]

Done.

+
   switch (index) {
-    case PROPERTY_INDEX_EAP_IDENTITY: {
-      std::string eap_identity;
-      if (!value.GetAsString(&eap_identity))
-        break;
-      wifi_network->set_eap_identity(eap_identity);
+    case PROPERTY_INDEX_EAP_IDENTITY:
+      wifi_network->set_eap_identity(string_value);
       return true;
-    }
     case PROPERTY_INDEX_EAP_METHOD: {
-      std::string eap_method_str;
-      if (!value.GetAsString(&eap_method_str))
-        break;
-      EAPMethod eap_method = ParseEAPMethod(eap_method_str);
+      EAPMethod eap_method = ParseEAPMethod(string_value);
       wifi_network->set_eap_method(eap_method);
       // Also update property with native value for EAP method.
       std::string str =
           NativeNetworkParser::network_eap_method_mapper()->GetKey(eap_method);
       scoped_ptr<StringValue> val(Value::CreateStringValue(str));
       wifi_network->UpdatePropertyMap(index, *val.get());
       return true;
     }
     case PROPERTY_INDEX_EAP_PHASE_2_AUTH: {
-      std::string eap_phase_2_auth_str;
-      if (!value.GetAsString(&eap_phase_2_auth_str))
-        break;
-      EAPPhase2Auth eap_phase_2_auth = ParseEAPPhase2Auth(eap_phase_2_auth_str);
+      EAPPhase2Auth eap_phase_2_auth = ParseEAPPhase2Auth(string_value);
       wifi_network->set_eap_phase_2_auth(eap_phase_2_auth);
       // Also update property with native value for EAP phase 2 auth.
       std::string str = NativeNetworkParser::network_eap_auth_mapper()->GetKey(
           eap_phase_2_auth);
       scoped_ptr<StringValue> val(Value::CreateStringValue(str));
       wifi_network->UpdatePropertyMap(index, *val.get());
       return true;
     }
-    case PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY: {
-      std::string eap_anonymous_identity;
-      if (!value.GetAsString(&eap_anonymous_identity))
-        break;
-      wifi_network->set_eap_anonymous_identity(eap_anonymous_identity);
+    case PROPERTY_INDEX_EAP_ANONYMOUS_IDENTITY:
+      wifi_network->set_eap_anonymous_identity(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_EAP_CERT_ID: {
-      std::string eap_client_cert_pkcs11_id;
-      if (!value.GetAsString(&eap_client_cert_pkcs11_id))
-        break;
-      wifi_network->set_eap_client_cert_pkcs11_id(eap_client_cert_pkcs11_id);
+    case PROPERTY_INDEX_EAP_CERT_ID:
+      wifi_network->set_eap_client_cert_pkcs11_id(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_EAP_CA_CERT_NSS: {
-      std::string eap_server_ca_cert_nss_nickname;
-      if (!value.GetAsString(&eap_server_ca_cert_nss_nickname))
-        break;
-      wifi_network->set_eap_server_ca_cert_nss_nickname(
-          eap_server_ca_cert_nss_nickname);
+    case PROPERTY_INDEX_EAP_CA_CERT_NSS:
+      wifi_network->set_eap_server_ca_cert_nss_nickname(string_value);
       return true;
-    }
-    case PROPERTY_INDEX_EAP_USE_SYSTEM_CAS: {
-      bool eap_use_system_cas;
-      if (!value.GetAsBoolean(&eap_use_system_cas))
-        break;
-      wifi_network->set_eap_use_system_cas(eap_use_system_cas);
+    case PROPERTY_INDEX_EAP_USE_SYSTEM_CAS:
+      wifi_network->set_eap_use_system_cas(bool_value);
       return true;
-    }
-    case PROPERTY_INDEX_EAP_PASSWORD: {
-      std::string eap_passphrase;
-      if (!value.GetAsString(&eap_passphrase))
-        break;
-      wifi_network->set_eap_passphrase(eap_passphrase);
+    case PROPERTY_INDEX_EAP_PASSWORD:
+      wifi_network->set_eap_passphrase(string_value);
       return true;
-    }
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_REF:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE:
+      // TODO(crosbug.com/19409): Support certificate patterns.
+      // Ignore for now.
+      return true;
     default:
       break;
   }
   return false;
 }
 
 ConnectionSecurity OncWifiNetworkParser::ParseSecurity(
     const std::string& security) {
   static EnumMapper<ConnectionSecurity>::Pair table[] = {
     { "None", SECURITY_NONE },
-    { "WEP", SECURITY_WEP },
-    { "WPA", SECURITY_WPA },
-    { "WPA2", SECURITY_8021X },
+    { "WEP-PSK", SECURITY_WEP },
+    { "WPA-PSK", SECURITY_WPA },
+    { "WPA-EAP", SECURITY_8021X },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ConnectionSecurity>, parser,
       (table, arraysize(table), SECURITY_UNKNOWN));
   return parser.Get(security);
 }
 
 EAPMethod OncWifiNetworkParser::ParseEAPMethod(const std::string& method) {
   static EnumMapper<EAPMethod>::Pair table[] = {
     { "PEAP", EAP_METHOD_PEAP },
     { "EAP-TLS", EAP_METHOD_TLS },
@@ skipping 28 matching lines @@
     const DictionaryValue& info,
     Network* network) {
   DCHECK_EQ(TYPE_VPN, network->type());
   VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
   if (!OncNetworkParser::UpdateNetworkFromInfo(info, network))
     return false;
   VLOG(1) << "Updating VPN '" << virtual_network->name()
           << "': Server: " << virtual_network->server_hostname()
           << " Type: "
           << ProviderTypeToString(virtual_network->provider_type());
-  if (virtual_network->provider_type() == PROVIDER_TYPE_L2TP_IPSEC_PSK) {
-    if (!virtual_network->client_cert_id().empty())
-      virtual_network->set_provider_type(PROVIDER_TYPE_L2TP_IPSEC_USER_CERT);
-  }
   return true;
 }
 
-bool OncVirtualNetworkParser::ParseValue(PropertyIndex index,
-                                         const base::Value& value,
-                                         Network* network) {
-  DCHECK_EQ(TYPE_VPN, network->type());
-  VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
-  switch (index) {
-    case PROPERTY_INDEX_PROVIDER: {
-      DCHECK_EQ(value.GetType(), Value::TYPE_DICTIONARY);
-      const DictionaryValue& dict = static_cast<const DictionaryValue&>(value);
-      for (DictionaryValue::key_iterator iter = dict.begin_keys();
-           iter != dict.end_keys(); ++iter) {
-        const std::string& key = *iter;
-        base::Value* provider_value;
-        bool res = dict.GetWithoutPathExpansion(key, &provider_value);
-        DCHECK(res);
-        if (res) {
-          PropertyIndex index = mapper().Get(key);
-          if (!ParseProviderValue(index, *provider_value, virtual_network))
-            VLOG(1) << network->name() << ": Provider unhandled key: " << key
-                    << " Type: " << provider_value->GetType();
-        }
-      }
-      return true;
-    }
-    default:
-      return OncNetworkParser::ParseValue(index, value, network);
+bool OncVirtualNetworkParser::ParseVPNValue(OncNetworkParser* parser,
+                                            PropertyIndex index,
+                                            const base::Value& value,
+                                            Network* network) {
+  VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
+  std::string string_value;
+  value.GetAsString(&string_value);
+  switch (index) {
+    case PROPERTY_INDEX_HOST: {
+      virtual_network->set_server_hostname(string_value);
+      // Flimflam requires a domain property which is unused.
+      scoped_ptr<StringValue> val(Value::CreateStringValue(""));
+      network->UpdatePropertyMap(PROPERTY_INDEX_VPN_DOMAIN, *val.get());
+      return true;
+    }
+    case PROPERTY_INDEX_ONC_IPSEC:
+      if (virtual_network->provider_type() != PROVIDER_TYPE_L2TP_IPSEC_PSK &&
+          virtual_network->provider_type() !=
+          PROVIDER_TYPE_L2TP_IPSEC_USER_CERT) {
+        VLOG(1) << "IPsec field not allowed with this VPN type";
+        return false;
+      }
+      return parser->ParseNestedObject(network,
+                                       "IPsec",
+                                       value,
+                                       ipsec_signature,
+                                       ParseIPsecValue);
+    case PROPERTY_INDEX_ONC_L2TP:
+      if (virtual_network->provider_type() != PROVIDER_TYPE_L2TP_IPSEC_PSK) {
+        VLOG(1) << "L2TP field not allowed with this VPN type";
+        return false;
+      }
+      return parser->ParseNestedObject(network,
+                                       "L2TP",
+                                       value,
+                                       l2tp_signature,
+                                       ParseL2TPValue);
+    case PROPERTY_INDEX_ONC_OPENVPN:
+      if (virtual_network->provider_type() != PROVIDER_TYPE_OPEN_VPN) {
+        VLOG(1) << "OpenVPN field not allowed with this VPN type";
+        return false;
+      }
+      return parser->ParseNestedObject(network,
+                                       "OpenVPN",
+                                       value,
+                                       openvpn_signature,
+                                       ParseOpenVPNValue);
+    case PROPERTY_INDEX_PROVIDER_TYPE: {
+      // Update property with native value for provider type.
+      ProviderType provider_type = GetCanonicalProviderType(
+          virtual_network->provider_type());
+      std::string str =
+        NativeVirtualNetworkParser::provider_type_mapper()->GetKey(
+            provider_type);
+      scoped_ptr<StringValue> val(Value::CreateStringValue(str));
+      network->UpdatePropertyMap(PROPERTY_INDEX_PROVIDER_TYPE, *val.get());
+      return true;
+    }
+    default:
       break;
   }
   return false;
 }
 
-bool OncVirtualNetworkParser::ParseProviderValue(PropertyIndex index,
-                                                 const base::Value& value,
-                                                 VirtualNetwork* network) {
-  switch (index) {
-    case PROPERTY_INDEX_HOST: {
-      std::string server_hostname;
-      if (!value.GetAsString(&server_hostname))
+bool OncVirtualNetworkParser::ParseIPsecValue(OncNetworkParser* parser,
+                                              PropertyIndex index,
+                                              const base::Value& value,
+                                              Network* network) {
+  VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
+  std::string string_value;
+  bool bool_value = false;
+  value.GetAsString(&string_value);
+  value.GetAsBoolean(&bool_value);
+
+  switch (index) {
+    case PROPERTY_INDEX_IPSEC_AUTHENTICATIONTYPE:
+      virtual_network->set_provider_type(
+          UpdateProviderTypeWithAuthType(virtual_network->provider_type(),
+                                         string_value));
+      return true;
+    case PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS:
+      virtual_network->set_ca_cert_nss(string_value);
+      return true;
+    case PROPERTY_INDEX_L2TPIPSEC_PSK:
+      virtual_network->set_psk_passphrase(string_value);
+      return true;
+    case PROPERTY_INDEX_L2TPIPSEC_GROUP_NAME:
+      virtual_network->set_group_name(string_value);
+      return true;
+    case PROPERTY_INDEX_SAVE_CREDENTIALS:
+      // Note that the specification allows different settings for
+      // IPsec credentials (PSK) and L2TP credentials (username and
+      // password) but we merge them in our implementation as is required
+      // with the current connection manager.
+      virtual_network->set_save_credentials(bool_value);
+      return true;
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_REF:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE:
+      // TODO(crosbug.com/19409): Support certificate patterns.
+      // Ignore for now.
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
+ProviderType OncVirtualNetworkParser::UpdateProviderTypeWithAuthType(
+    ProviderType provider,
+    const std::string& auth_type) {
+  switch (provider) {
+  case PROVIDER_TYPE_L2TP_IPSEC_PSK:
+  case PROVIDER_TYPE_L2TP_IPSEC_USER_CERT:
+    if (auth_type == "Cert") {
+      return PROVIDER_TYPE_L2TP_IPSEC_USER_CERT;
+    } else {
+      if (auth_type != "PSK") {
+        VLOG(1) << "Unexpected authentication type " << auth_type;
         break;
-      network->set_server_hostname(server_hostname);
-      return true;
-    }
-    case PROPERTY_INDEX_NAME: {
-      std::string name;
-      if (!value.GetAsString(&name))
-        break;
-      network->set_name(name);
-      return true;
-    }
-    case PROPERTY_INDEX_TYPE: {
-      std::string provider_type_string;
-      if (!value.GetAsString(&provider_type_string))
-        break;
-      network->set_provider_type(ParseProviderType(provider_type_string));
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_CA_CERT_NSS: {
-      std::string ca_cert_nss;
-      if (!value.GetAsString(&ca_cert_nss))
-        break;
-      network->set_ca_cert_nss(ca_cert_nss);
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_PSK: {
-      std::string psk_passphrase;
-      if (!value.GetAsString(&psk_passphrase))
-        break;
-      network->set_psk_passphrase(psk_passphrase);
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_CLIENT_CERT_ID: {
-      std::string client_cert_id;
-      if (!value.GetAsString(&client_cert_id))
-        break;
-      network->set_client_cert_id(client_cert_id);
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_USER: {
-      std::string username;
-      if (!value.GetAsString(&username))
-        break;
-      network->set_username(username);
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_PASSWORD: {
-      std::string user_passphrase;
-      if (!value.GetAsString(&user_passphrase))
-        break;
-      network->set_user_passphrase(user_passphrase);
-      return true;
-    }
-    case PROPERTY_INDEX_L2TPIPSEC_GROUP_NAME: {
-      std::string group_name;
-      if (!value.GetAsString(&group_name))
-        break;
-      network->set_group_name(group_name);
-      return true;
-    }
-    default:
-      break;
-  }
+      }
+      return PROVIDER_TYPE_L2TP_IPSEC_PSK;
+    }
+  default:
+    VLOG(1) << "Unexpected provider type with authentication type "
+            << auth_type;
+    break;
+  }
+  return provider;
+}
+
+ProviderType OncVirtualNetworkParser::GetCanonicalProviderType(
+    ProviderType provider_type) {
+  if (provider_type == PROVIDER_TYPE_L2TP_IPSEC_USER_CERT)
+    return PROVIDER_TYPE_L2TP_IPSEC_PSK;
+  return provider_type;
+}
+
+bool OncVirtualNetworkParser::ParseL2TPValue(OncNetworkParser*,
+                                             PropertyIndex index,
+                                             const base::Value& value,
+                                             Network* network) {
+  VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
+  std::string string_value;
+  bool bool_value = false;
+  value.GetAsString(&string_value);
+  value.GetAsBoolean(&bool_value);
+  switch (index) {
+    case PROPERTY_INDEX_L2TPIPSEC_PASSWORD:
+      virtual_network->set_user_passphrase(string_value);
+      return true;
+    case PROPERTY_INDEX_L2TPIPSEC_USER:
+      virtual_network->set_username(string_value);
+      return true;
+    case PROPERTY_INDEX_SAVE_CREDENTIALS:
+      // Note that the specification allows different settings for
+      // IPsec credentials (PSK) and L2TP credentials (username and
+      // password) but we merge them in our implementation as is required
+      // with the current connection manager.
+      virtual_network->set_save_credentials(bool_value);
+      return true;
+    default:
+      break;
+  }
+  return false;
+}
+
+bool OncVirtualNetworkParser::ParseOpenVPNValue(OncNetworkParser*,
+                                                PropertyIndex index,
+                                                const base::Value& value,
+                                                Network* network) {
+  VirtualNetwork* virtual_network = static_cast<VirtualNetwork*>(network);
+  std::string string_value;
+  bool bool_value = false;
+  value.GetAsString(&string_value);
+  value.GetAsBoolean(&bool_value);
+  switch (index) {
+    case PROPERTY_INDEX_OPEN_VPN_PASSWORD:
+      virtual_network->set_user_passphrase(string_value);
+      return true;
+    case PROPERTY_INDEX_OPEN_VPN_USER:
+      virtual_network->set_username(string_value);
+      return true;
+    case PROPERTY_INDEX_SAVE_CREDENTIALS:
+      virtual_network->set_save_credentials(bool_value);
+      return true;
+    case PROPERTY_INDEX_OPEN_VPN_CACERT:
+      virtual_network->set_ca_cert_nss(string_value);
+      return true;
+    case PROPERTY_INDEX_OPEN_VPN_REMOTECERTKU: {
+      // ONC supports a list of these, but we flimflam supports only one
+      // today.  So extract the first.
+      const base::ListValue* value_list = NULL;
+      value.GetAsList(&value_list);
+      base::Value* first_item = NULL;
+      if (!value_list->Get(0, &first_item) ||
+          !first_item->IsType(base::Value::TYPE_STRING)) {
+        VLOG(1) << "RemoteCertKU must be non-empty list of strings";
+        return false;
+      }
+      virtual_network->UpdatePropertyMap(index, *first_item);
+      return true;
+    }
+    case PROPERTY_INDEX_OPEN_VPN_AUTH:
+    case PROPERTY_INDEX_OPEN_VPN_AUTHRETRY:
+    case PROPERTY_INDEX_OPEN_VPN_AUTHNOCACHE:
+    case PROPERTY_INDEX_OPEN_VPN_CERT:
+    case PROPERTY_INDEX_OPEN_VPN_CIPHER:
+    case PROPERTY_INDEX_OPEN_VPN_COMPLZO:
+    case PROPERTY_INDEX_OPEN_VPN_COMPNOADAPT:
+    case PROPERTY_INDEX_OPEN_VPN_KEYDIRECTION:
+    case PROPERTY_INDEX_OPEN_VPN_NSCERTTYPE:
+    case PROPERTY_INDEX_OPEN_VPN_PORT:
+    case PROPERTY_INDEX_OPEN_VPN_PROTO:
+    case PROPERTY_INDEX_OPEN_VPN_PUSHPEERINFO:
+    case PROPERTY_INDEX_OPEN_VPN_REMOTECERTEKU:
+    case PROPERTY_INDEX_OPEN_VPN_REMOTECERTTLS:
+    case PROPERTY_INDEX_OPEN_VPN_RENEGSEC:
+    case PROPERTY_INDEX_OPEN_VPN_SERVERPOLLTIMEOUT:
+    case PROPERTY_INDEX_OPEN_VPN_SHAPER:
+    case PROPERTY_INDEX_OPEN_VPN_STATICCHALLENGE:
+    case PROPERTY_INDEX_OPEN_VPN_TLSAUTHCONTENTS:
+    case PROPERTY_INDEX_OPEN_VPN_TLSREMOTE:
+      // Pass these through to connection manager as is.
+      return true;
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_PATTERN:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_REF:
+    case PROPERTY_INDEX_ONC_CLIENT_CERT_TYPE:
+      // TODO(crosbug.com/19409): Support certificate patterns.
+      // Ignore for now.
+      return true;
+
+    default:
+      break;
+  }
   return false;
 }
 
 ProviderType OncVirtualNetworkParser::ParseProviderType(
     const std::string& type) {
   static EnumMapper<ProviderType>::Pair table[] = {
-    { flimflam::kProviderL2tpIpsec, PROVIDER_TYPE_L2TP_IPSEC_PSK },
-    { flimflam::kProviderOpenVpn, PROVIDER_TYPE_OPEN_VPN },
+    // We initially map to L2TP-IPsec PSK and then fix this up based
+    // on the value of AuthenticationType.
+    { "L2TP-IPsec", PROVIDER_TYPE_L2TP_IPSEC_PSK },
+    { "OpenVPN", PROVIDER_TYPE_OPEN_VPN },
   };
   CR_DEFINE_STATIC_LOCAL(EnumMapper<ProviderType>, parser,
       (table, arraysize(table), PROVIDER_TYPE_MAX));
   return parser.Get(type);
 }
 
 }  // namespace chromeos