Migrate Slot ID of client certs in network configuration.

chromeos/network/client_cert_util.cc

Index: chromeos/network/client_cert_util.cc
diff --git a/chromeos/network/client_cert_util.cc b/chromeos/network/client_cert_util.cc
index b831beaf53b99c7bdfb6ddd64817ece339f47e9e..93fcabb6bd70a72612bc77f73ea1afcc85f528ff 100644
--- a/chromeos/network/client_cert_util.cc
+++ b/chromeos/network/client_cert_util.cc
@@ -97,7 +97,9 @@ bool CertPrincipalMatches(const IssuerSubjectPattern& pattern,
   return true;
 }
 
-std::string GetPkcs11IdFromEapCertId(const std::string& cert_id) {
+std::string GetPkcs11AndSlotIdFromEapCertId(const std::string& cert_id,
+                                            int* slot_id) {
+  *slot_id = -1;
   if (cert_id.empty())
     return std::string();
 
@@ -110,9 +112,80 @@ std::string GetPkcs11IdFromEapCertId(const std::string& cert_id) {
     LOG(ERROR) << "Empty PKCS11 id in cert id.";
     return std::string();
   }
+  int parsed_slot_id;
+  if (base::StringToInt(cert_id.substr(0, delimiter_pos), &parsed_slot_id))
+    *slot_id = parsed_slot_id;
+  else
+    LOG(ERROR) << "Slot ID is not an integer. Cert ID is: " << cert_id << ".";
   return cert_id.substr(delimiter_pos + 1);
 }
 
+void GetShillProperties(const base::DictionaryValue& properties,
+                        ConfigType* cert_config_type,
+                        int* tpm_slot,
+                        std::string* pkcs11_id) {
+  *cert_config_type = CONFIG_TYPE_NONE;
+  *tpm_slot = -1;
+  pkcs11_id->clear();
+
+  std::string type;
+  properties.GetStringWithoutPathExpansion(shill::kTypeProperty, &type);
+  std::string security;
+  properties.GetStringWithoutPathExpansion(shill::kSecurityProperty, &security);
+  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).
+    const base::DictionaryValue* provider_properties = NULL;
+    std::string vpn_provider_type;
+    if (!properties.GetDictionaryWithoutPathExpansion(shill::kProviderProperty,
+                                                      &provider_properties)) {
+      LOG(ERROR) << "Missing provider properties.";
+      return;
+    }
+    provider_properties->GetStringWithoutPathExpansion(shill::kTypeProperty,
+                                                       &vpn_provider_type);
+    if (vpn_provider_type == shill::kProviderOpenVpn) {
+      provider_properties->GetStringWithoutPathExpansion(
+          shill::kOpenVPNClientCertIdProperty, pkcs11_id);
+      *cert_config_type = CONFIG_TYPE_OPENVPN;
+    } else if (vpn_provider_type == shill::kProviderL2tpIpsec) {
+      std::string cert_slot;
+      provider_properties->GetStringWithoutPathExpansion(
+          shill::kL2tpIpsecClientCertSlotProperty, &cert_slot);
+      if (!base::StringToInt(cert_slot, tpm_slot)) {
+        LOG(ERROR) << "Cert slot is not an integer: " << cert_slot << ".";
+        return;
+      }
+
+      provider_properties->GetStringWithoutPathExpansion(
+          shill::kL2tpIpsecClientCertIdProperty, pkcs11_id);
+      *cert_config_type = CONFIG_TYPE_IPSEC;
+    } else {
+      LOG(ERROR) << "Unknown VPN type " << vpn_provider_type << ".";
+      return;
+    }
+  } else if (security == shill::kSecurity8021x) {

[Paul Stewart, 2014/08/14 20:09:17]

I think you're missing the 802.1x WEP case?  You also may want to be careful
that you're not in an 802.1x but not TLS case here.  

[pneubeck (no reviews), 2014/08/14 23:06:00]

802.1x WEP is still not supported by Chrome as its priority was never considered
high enough: crbug.com/127070.
I could add a
  if (pkcs11_id.empty())
condition to each assignment to |cert_config_type|.
That would take care of the cases where actually no client cert was configured.

To handle EthernetEAP this would also need a
  || type == shill::kTypeEthernetEap


Alteneratively, I could drop the checks for Type and Security and instead probe
each of the Cert/Key/Slot properties and the first existent will reveal what
kind of network it is.
I.e. something like:

if (provider_properties) {
  // Network must be a VPN

  if
(provider_properties->GetStringWithoutPathExpansion(..OpenVPNClientCertId...)) {
    // Must be a OpenVPN
    *cert_config_type = CONFIG_TYPE_OPENVPN;
  } else if (...Get...(..Ipsec..)) {
    // Must be a Ipsec VPN
    ...
  }
} else if (properties.Get...(kEapCertIdProperty, &cert_id)) {
  // Must be a network with EAP config and client certificate (e.g. WPA 802.1x
or Ethernet 802.1x).
  ...
}

[pneubeck (no reviews), 2014/08/15 13:15:21]

Done.

+    // Shill requires both CertID and KeyID for TLS connections, despite the
+    // fact that by convention they are the same ID, because one identifies
+    // the certificate and the other the private key.
+    std::string cert_id;
+    properties.GetStringWithoutPathExpansion(shill::kEapCertIdProperty,
+                                             &cert_id);
+    std::string key_id;
+    properties.GetStringWithoutPathExpansion(shill::kEapKeyIdProperty, &key_id);
+    // Assume the configuration to be invalid, if the two IDs are not identical.
+    if (cert_id != key_id) {
+      LOG(ERROR) << "EAP CertID differs from KeyID";
+      return;
+    }
+    *pkcs11_id = GetPkcs11AndSlotIdFromEapCertId(cert_id, tpm_slot);
+    *cert_config_type = CONFIG_TYPE_EAP;
+  } else {
+    LOG(ERROR) << "Network type " << type << ".";
+    return;
+  }
+}
+
 void SetShillProperties(const ConfigType cert_config_type,
                         const int tpm_slot,
                         const std::string& pkcs11_id,