Refactor the client certificate code in chromeos/network/.

This prepares for the next commit, which adds automatic resolution of client certificates which doesn't require a manual trigger by the user anymore.

The only functional is that flimflam::kOpenVPNClientCertSlotProperty is not set anymore, because it was never supported by Shill and leads to errors with Service::SetProperties.

BUG=234983, 126870
R=stevenjb@chromium.org

Review URL: https://codereview.chromium.org/22588002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@216997 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 226 insertions, 163 deletions

diff --git a/chrome/browser/chromeos/cros/network_library.cc b/chrome/browser/chromeos/cros/network_library.cc
index 9943d9e..0a709fd 100644
--- a/chrome/browser/chromeos/cros/network_library.cc
+++ b/chrome/browser/chromeos/cros/network_library.cc
@@ -18,7 +18,7 @@
 #include "chrome/browser/chromeos/enrollment_dialog_view.h"
 #include "chrome/common/net/x509_certificate_model.h"
 #include "chromeos/network/certificate_pattern.h"
-#include "chromeos/network/certificate_pattern_matcher.h"
+#include "chromeos/network/client_cert_util.h"
 #include "chromeos/network/cros_network_functions.h"
 #include "chromeos/network/network_state_handler.h"
 #include "chromeos/network/onc/onc_utils.h"
@@ -768,7 +768,7 @@ void VirtualNetwork::MatchCertificatePattern(bool allow_enroll,
   }
 
   scoped_refptr<net::X509Certificate> matching_cert =
-      certificate_pattern::GetCertificateMatch(client_cert_pattern());
+      client_cert::GetCertificateMatch(client_cert_pattern());
   if (matching_cert.get()) {
     std::string client_cert_id =
         x509_certificate_model::GetPkcs11Id(matching_cert->os_cert_handle());
@@ -1285,7 +1285,7 @@ void WifiNetwork::MatchCertificatePattern(bool allow_enroll,
   }
 
   scoped_refptr<net::X509Certificate> matching_cert =
-      certificate_pattern::GetCertificateMatch(client_cert_pattern());
+      client_cert::GetCertificateMatch(client_cert_pattern());
   if (matching_cert.get()) {
     SetEAPClientCertPkcs11Id(
         x509_certificate_model::GetPkcs11Id(matching_cert->os_cert_handle()));
diff --git a/chrome/browser/chromeos/options/network_connect.cc b/chrome/browser/chromeos/options/network_connect.cc
index a494325..7305e4b 100644
--- a/chrome/browser/chromeos/options/network_connect.cc
+++ b/chrome/browser/chromeos/options/network_connect.cc
@@ -22,7 +22,6 @@
 #include "chrome/common/url_constants.h"
 #include "chromeos/chromeos_switches.h"
 #include "chromeos/network/certificate_pattern.h"
-#include "chromeos/network/certificate_pattern_matcher.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_event_log.h"
 #include "chromeos/network/network_handler.h"
diff --git a/chromeos/cert_loader.cc b/chromeos/cert_loader.cc
index 799815d..964c777 100644
--- a/chromeos/cert_loader.cc
+++ b/chromeos/cert_loader.cc
@@ -110,7 +110,7 @@ void CertLoader::SetCryptoTaskRunner(
 }
 
 void CertLoader::SetSlowTaskRunnerForTest(
-    const scoped_refptr<base::SequencedTaskRunner>& task_runner) {
+    const scoped_refptr<base::TaskRunner>& task_runner) {
   slow_task_runner_for_test_ = task_runner;
 }
 
@@ -238,6 +238,7 @@ void CertLoader::OnPersistentNSSDBOpened() {
   InitializeTokenAndLoadCertificates();
 }
 
+// This is copied from chrome/common/net/x509_certificate_model_nss.cc.
 // For background see this discussion on dev-tech-crypto.lists.mozilla.org:
 // http://web.archiveorange.com/archive/v/6JJW7E40sypfZGtbkzxX
 //
@@ -245,11 +246,9 @@ void CertLoader::OnPersistentNSSDBOpened() {
 // is shared between a certificate and its associated private and public
 // keys.  I tried to implement this with PK11_GetLowLevelKeyIDForCert(),
 // but that always returns NULL on Chrome OS for me.
-std::string CertLoader::GetPkcs11IdForCert(
-    const net::X509Certificate& cert) const {
-  if (!IsHardwareBacked())
-    return std::string();
 
+// static
+std::string CertLoader::GetPkcs11IdForCert(const net::X509Certificate& cert) {
   CERTCertificateStr* cert_handle = cert.os_cert_handle();
   SECKEYPrivateKey *priv_key =
       PK11_FindKeyByAnyCert(cert_handle, NULL /* wincx */);
diff --git a/chromeos/cert_loader.h b/chromeos/cert_loader.h
index ec2d7ce..75d2cd0 100644
--- a/chromeos/cert_loader.h
+++ b/chromeos/cert_loader.h
@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "base/basictypes.h"
 #include "base/memory/ref_counted.h"
 #include "base/memory/scoped_ptr.h"
 #include "base/memory/weak_ptr.h"
@@ -20,6 +21,7 @@
 
 namespace base {
 class SequencedTaskRunner;
+class TaskRunner;
 }
 
 namespace crypto {
@@ -65,6 +67,8 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer,
   // Returns true if the global instance has been initialized.
   static bool IsInitialized();
 
+  static std::string GetPkcs11IdForCert(const net::X509Certificate& cert);
+
   // |crypto_task_runner| is the task runner that any synchronous crypto calls
   // should be made from, e.g. in Chrome this is the IO thread. Must be called
   // after the thread is started. Certificate loading will not happen unless
@@ -75,7 +79,7 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer,
   // Sets the task runner that any slow calls will be made from, e.g. calls
   // to the NSS database. If not set, uses base::WorkerPool.
   void SetSlowTaskRunnerForTest(
-      const scoped_refptr<base::SequencedTaskRunner>& task_runner);
+      const scoped_refptr<base::TaskRunner>& task_runner);
 
   void AddObserver(CertLoader::Observer* observer);
   void RemoveObserver(CertLoader::Observer* observer);
@@ -86,8 +90,6 @@ class CHROMEOS_EXPORT CertLoader : public net::CertDatabase::Observer,
   // Returns true if the TPM is available for hardware-backed certificates.
   bool IsHardwareBacked() const;
 
-  std::string GetPkcs11IdForCert(const net::X509Certificate& cert) const;
-
   bool certificates_loaded() const { return certificates_loaded_; }
 
   // TPM info is only valid once the TPM is available (IsHardwareBacked is
diff --git a/chromeos/chromeos.gyp b/chromeos/chromeos.gyp
index 69d2a67..57fd83b 100644
--- a/chromeos/chromeos.gyp
+++ b/chromeos/chromeos.gyp
@@ -217,8 +217,8 @@
         'login/login_state.h',
         'network/certificate_pattern.cc',
         'network/certificate_pattern.h',
-        'network/certificate_pattern_matcher.cc',
-        'network/certificate_pattern_matcher.h',
+        'network/client_cert_util.cc',
+        'network/client_cert_util.h',
         'network/cros_network_functions.cc',
         'network/cros_network_functions.h',
         'network/device_state.cc',
diff --git a/chromeos/network/certificate_pattern_matcher.h b/chromeos/network/certificate_pattern_matcher.h
deleted file mode 100644
index a79a548..0000000
--- a/chromeos/network/certificate_pattern_matcher.h
+++ /dev/null
@@ -1,30 +0,0 @@
-// Copyright (c) 2012 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.
-
-#ifndef CHROMEOS_NETWORK_CERTIFICATE_PATTERN_MATCHER_H_
-#define CHROMEOS_NETWORK_CERTIFICATE_PATTERN_MATCHER_H_
-
-#include "base/memory/ref_counted.h"
-#include "chromeos/chromeos_export.h"
-
-namespace net {
-class X509Certificate;
-}
-
-namespace chromeos {
-
-class CertificatePattern;
-
-namespace certificate_pattern {
-
-// Fetches the matching certificate that has the latest valid start date.
-// Returns a NULL refptr if there is no such match.
-CHROMEOS_EXPORT scoped_refptr<net::X509Certificate> GetCertificateMatch(
-    const CertificatePattern& pattern);
-
-}  // namespace certificate_pattern
-
-}  // namespace chromeos
-
-#endif  // CHROMEOS_NETWORK_CERTIFICATE_PATTERN_MATCHER_H_
diff --git a/chromeos/network/certificate_pattern_matcher.cc b/chromeos/network/client_cert_util.cc
index 3d7f328..ad2bc45 100644
--- a/chromeos/network/certificate_pattern_matcher.cc
+++ b/chromeos/network/client_cert_util.cc
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-#include "chromeos/network/certificate_pattern_matcher.h"
+#include "chromeos/network/client_cert_util.h"
 
 #include <cert.h>
 #include <pk11pub.h>
@@ -11,53 +11,20 @@
 #include <string>
 #include <vector>
 
+#include "base/values.h"
 #include "chromeos/network/certificate_pattern.h"
 #include "net/base/net_errors.h"
 #include "net/cert/cert_database.h"
 #include "net/cert/nss_cert_database.h"
 #include "net/cert/x509_cert_types.h"
 #include "net/cert/x509_certificate.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
 
 namespace chromeos {
 
-namespace {
+namespace client_cert {
 
-// Returns true only if any fields set in this pattern match exactly with
-// similar fields in the principal.  If organization_ or organizational_unit_
-// are set, then at least one of the organizations or units in the principal
-// must match.
-bool CertPrincipalMatches(const IssuerSubjectPattern& pattern,
-                          const net::CertPrincipal& principal) {
-  if (!pattern.common_name().empty() &&
-      pattern.common_name() != principal.common_name) {
-    return false;
-  }
-
-  if (!pattern.locality().empty() &&
-      pattern.locality() != principal.locality_name) {
-    return false;
-  }
-
-  if (!pattern.organization().empty()) {
-    if (std::find(principal.organization_names.begin(),
-                  principal.organization_names.end(),
-                  pattern.organization()) ==
-        principal.organization_names.end()) {
-      return false;
-    }
-  }
-
-  if (!pattern.organizational_unit().empty()) {
-    if (std::find(principal.organization_unit_names.begin(),
-                  principal.organization_unit_names.end(),
-                  pattern.organizational_unit()) ==
-        principal.organization_unit_names.end()) {
-      return false;
-    }
-  }
-
-  return true;
-}
+namespace {
 
 // Functor to filter out non-matching issuers.
 class IssuerFilter {
@@ -126,7 +93,42 @@ class IssuerCaFilter {
 
 }  // namespace
 
-namespace certificate_pattern {
+// Returns true only if any fields set in this pattern match exactly with
+// similar fields in the principal.  If organization_ or organizational_unit_
+// are set, then at least one of the organizations or units in the principal
+// must match.
+bool CertPrincipalMatches(const IssuerSubjectPattern& pattern,
+                          const net::CertPrincipal& principal) {
+  if (!pattern.common_name().empty() &&
+      pattern.common_name() != principal.common_name) {
+    return false;
+  }
+
+  if (!pattern.locality().empty() &&
+      pattern.locality() != principal.locality_name) {
+    return false;
+  }
+
+  if (!pattern.organization().empty()) {
+    if (std::find(principal.organization_names.begin(),
+                  principal.organization_names.end(),
+                  pattern.organization()) ==
+        principal.organization_names.end()) {
+      return false;
+    }
+  }
+
+  if (!pattern.organizational_unit().empty()) {
+    if (std::find(principal.organization_unit_names.begin(),
+                  principal.organization_unit_names.end(),
+                  pattern.organizational_unit()) ==
+        principal.organization_unit_names.end()) {
+      return false;
+    }
+  }
+
+  return true;
+}
 
 scoped_refptr<net::X509Certificate> GetCertificateMatch(
     const CertificatePattern& pattern) {
@@ -188,6 +190,54 @@ scoped_refptr<net::X509Certificate> GetCertificateMatch(
   return latest;
 }
 
-}  // namespace certificate_pattern
+void SetShillProperties(const client_cert::ConfigType cert_config_type,
+                        const std::string& tpm_slot,
+                        const std::string& tpm_pin,
+                        const std::string* pkcs11_id,
+                        base::DictionaryValue* properties) {
+  const char* tpm_pin_property = NULL;
+  switch (cert_config_type) {
+    case CONFIG_TYPE_NONE: {
+      return;
+    }
+    case CONFIG_TYPE_OPENVPN: {
+      tpm_pin_property = flimflam::kOpenVPNPinProperty;
+      if (pkcs11_id) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kOpenVPNClientCertIdProperty, *pkcs11_id);
+      }
+      break;
+    }
+    case CONFIG_TYPE_IPSEC: {
+      tpm_pin_property = flimflam::kL2tpIpsecPinProperty;
+      if (!tpm_slot.empty()) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecClientCertSlotProperty, tpm_slot);
+      }
+      if (pkcs11_id) {
+        properties->SetStringWithoutPathExpansion(
+            flimflam::kL2tpIpsecClientCertIdProperty, *pkcs11_id);
+      }
+      break;
+    }
+    case CONFIG_TYPE_EAP: {
+      tpm_pin_property = flimflam::kEapPinProperty;
+      if (pkcs11_id) {
+        // Shill requires both CertID and KeyID for TLS connections, despite the
+        // fact that by convention they are the same ID.
+        properties->SetStringWithoutPathExpansion(flimflam::kEapCertIdProperty,
+                                                  *pkcs11_id);
+        properties->SetStringWithoutPathExpansion(flimflam::kEapKeyIdProperty,
+                                                  *pkcs11_id);
+      }
+      break;
+    }
+  }
+  DCHECK(tpm_pin_property);
+  if (!tpm_pin.empty())
+    properties->SetStringWithoutPathExpansion(tpm_pin_property, tpm_pin);
+}
+
+}  // namespace client_cert
 
 }  // namespace chromeos
diff --git a/chromeos/network/client_cert_util.h b/chromeos/network/client_cert_util.h
new file mode 100644
index 0000000..aa3c882
--- /dev/null
+++ b/chromeos/network/client_cert_util.h
@@ -0,0 +1,61 @@
+// Copyright (c) 2012 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.
+
+#ifndef CHROMEOS_NETWORK_CLIENT_CERT_UTIL_H_
+#define CHROMEOS_NETWORK_CLIENT_CERT_UTIL_H_
+
+#include <string>
+
+#include "base/memory/ref_counted.h"
+#include "chromeos/chromeos_export.h"
+
+namespace base {
+class DictionaryValue;
+}
+
+namespace net {
+struct CertPrincipal;
+class X509Certificate;
+}
+
+namespace chromeos {
+
+class CertificatePattern;
+class IssuerSubjectPattern;
+
+namespace client_cert {
+
+enum ConfigType {
+  CONFIG_TYPE_NONE,
+  CONFIG_TYPE_OPENVPN,
+  CONFIG_TYPE_IPSEC,
+  CONFIG_TYPE_EAP
+};
+
+// Returns true only if any fields set in this pattern match exactly with
+// similar fields in the principal.  If organization_ or organizational_unit_
+// are set, then at least one of the organizations or units in the principal
+// must match.
+bool CertPrincipalMatches(const IssuerSubjectPattern& pattern,
+                          const net::CertPrincipal& principal);
+
+// Fetches the matching certificate that has the latest valid start date.
+// Returns a NULL refptr if there is no such match.
+CHROMEOS_EXPORT scoped_refptr<net::X509Certificate> GetCertificateMatch(
+    const CertificatePattern& pattern);
+
+// If not empty, sets the TPM properties in |properties|. If |pkcs11_id| is not
+// NULL, also sets the ClientCertID. |cert_config_type| determines which
+// dictionary entries to set.
+void SetShillProperties(const ConfigType cert_config_type,
+                        const std::string& tpm_slot,
+                        const std::string& tpm_pin,
+                        const std::string* pkcs11_id,
+                        base::DictionaryValue* properties);
+
+}  // namespace client_cert
+
+}  // namespace chromeos
+
+#endif  // CHROMEOS_NETWORK_CLIENT_CERT_UTIL_H_
diff --git a/chromeos/network/network_connection_handler.cc b/chromeos/network/network_connection_handler.cc
index 25e03de..08d233e 100644
--- a/chromeos/network/network_connection_handler.cc
+++ b/chromeos/network/network_connection_handler.cc
@@ -11,7 +11,7 @@
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "chromeos/dbus/shill_manager_client.h"
 #include "chromeos/dbus/shill_service_client.h"
-#include "chromeos/network/certificate_pattern_matcher.h"
+#include "chromeos/network/client_cert_util.h"
 #include "chromeos/network/managed_network_configuration_handler.h"
 #include "chromeos/network/network_configuration_handler.h"
 #include "chromeos/network/network_event_log.h"
@@ -407,82 +407,63 @@ void NetworkConnectionHandler::VerifyConfiguredAndConnect(
     }
   }
 
-  // These will be set if they need to be configured, otherwise they will
-  // be left empty and the properties will not be set.
-  std::string pkcs11_id, tpm_slot, tpm_pin;
-
-  // Check certificate properties in kUIDataProperty if configured.
-  // Note: Wifi/VPNConfigView set these properties explicitly.
-  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);
-      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;
-    }
-
-    // Ensure the certificate is available and configured.
-    if (!CertificateIsConfigured(ui_data.get(), &pkcs11_id)) {
-      ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
-      return;
-    }
-  }
-
-  // The network may not be 'Connectable' because the TPM properties are
-  // not set up, so configure tpm slot/pin before connecting.
-  if (cert_loader_) {
-    tpm_slot = cert_loader_->tpm_token_slot();
-    tpm_pin = cert_loader_->tpm_user_pin();
+  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) {
+    client_cert_type = client_cert::CONFIG_TYPE_EAP;
   }
 
   base::DictionaryValue config_properties;
-
-  if (type == flimflam::kTypeVPN) {
-    if (vpn_provider_type == flimflam::kProviderOpenVpn) {
-      if (!pkcs11_id.empty()) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kOpenVPNClientCertIdProperty, pkcs11_id);
-      }
-      if (!tpm_pin.empty()) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kOpenVPNPinProperty, tpm_pin);
+  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;
       }
-    } else {
-      if (!pkcs11_id.empty()) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecClientCertIdProperty, pkcs11_id);
-      }
-      if (!tpm_slot.empty()) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecClientCertSlotProperty, tpm_slot);
+
+      // If certificates have not been loaded yet, queue the connect request.
+      if (!certificates_loaded_) {
+        ConnectRequest* request = pending_request(service_path);
+        DCHECK(request);
+        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;
       }
-      if (!tpm_pin.empty()) {
-        config_properties.SetStringWithoutPathExpansion(
-            flimflam::kL2tpIpsecPinProperty, tpm_pin);
+
+      pkcs11_id = CertificateIsConfigured(ui_data.get());
+      // Ensure the certificate is available and configured.
+      if (!cert_loader_->IsHardwareBacked() || pkcs11_id.empty()) {
+        ErrorCallbackForPendingRequest(service_path, kErrorCertificateRequired);
+        return;
       }
     }
-  } else if (type == flimflam::kTypeWifi) {
-    if (!pkcs11_id.empty()) {
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapCertIdProperty, pkcs11_id);
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapKeyIdProperty, pkcs11_id);
-    }
-    if (!tpm_pin.empty()) {
-      config_properties.SetStringWithoutPathExpansion(
-          flimflam::kEapPinProperty, tpm_pin);
+
+    // The network may not be 'Connectable' because the TPM properties are not
+    // set up, so configure tpm slot/pin before connecting.
+    if (cert_loader_ && cert_loader_->IsHardwareBacked()) {
+      // Pass NULL if pkcs11_id is empty, so that it doesn't clear any
+      // previously configured client cert.
+      client_cert::SetShillProperties(client_cert_type,
+                                      cert_loader_->tpm_token_slot(),
+                                      cert_loader_->tpm_user_pin(),
+                                      pkcs11_id.empty() ? NULL : &pkcs11_id,
+                                      &config_properties);
     }
   }
 
@@ -640,18 +621,16 @@ void NetworkConnectionHandler::CheckAllPendingRequests() {
   }
 }
 
-bool NetworkConnectionHandler::CertificateIsConfigured(NetworkUIData* ui_data,
-                                                       std::string* pkcs11_id) {
+std::string NetworkConnectionHandler::CertificateIsConfigured(
+    NetworkUIData* ui_data) {
   if (ui_data->certificate_pattern().Empty())
-    return false;
-
+    return std::string();
   // Find the matching certificate.
   scoped_refptr<net::X509Certificate> matching_cert =
-      certificate_pattern::GetCertificateMatch(ui_data->certificate_pattern());
+      client_cert::GetCertificateMatch(ui_data->certificate_pattern());
   if (!matching_cert.get())
-    return false;
-  *pkcs11_id = cert_loader_->GetPkcs11IdForCert(*matching_cert.get());
-  return true;
+    return std::string();
+  return CertLoader::GetPkcs11IdForCert(*matching_cert.get());
 }
 
 void NetworkConnectionHandler::ErrorCallbackForPendingRequest(
diff --git a/chromeos/network/network_connection_handler.h b/chromeos/network/network_connection_handler.h
index 90f91e2..b1855c8 100644
--- a/chromeos/network/network_connection_handler.h
+++ b/chromeos/network/network_connection_handler.h
@@ -167,7 +167,10 @@ class CHROMEOS_EXPORT NetworkConnectionHandler
 
   void CheckPendingRequest(const std::string service_path);
   void CheckAllPendingRequests();
-  bool CertificateIsConfigured(NetworkUIData* ui_data, std::string* pkcs11_id);
+
+  // Returns the PKCS#11 ID of a cert matching the certificate pattern in
+  // |ui_data|. Returns empty string otherwise.
+  std::string CertificateIsConfigured(NetworkUIData* ui_data);
   void ErrorCallbackForPendingRequest(const std::string& service_path,
                                       const std::string& error_name);