From b1f3f527f1c81d52be8cb8e8ca5499de9ca88275 Mon Sep 17 00:00:00 2001 From: "pneubeck@chromium.org" Date: Mon, 12 Aug 2013 15:22:49 +0000 Subject: 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 --- chrome/browser/chromeos/cros/network_library.cc | 6 +- chrome/browser/chromeos/options/network_connect.cc | 1 - chromeos/cert_loader.cc | 9 +- chromeos/cert_loader.h | 8 +- chromeos/chromeos.gyp | 4 +- chromeos/network/certificate_pattern_matcher.cc | 193 ---------------- chromeos/network/certificate_pattern_matcher.h | 30 --- chromeos/network/client_cert_util.cc | 243 +++++++++++++++++++++ chromeos/network/client_cert_util.h | 61 ++++++ chromeos/network/network_connection_handler.cc | 135 +++++------- chromeos/network/network_connection_handler.h | 5 +- 11 files changed, 379 insertions(+), 316 deletions(-) delete mode 100644 chromeos/network/certificate_pattern_matcher.cc delete mode 100644 chromeos/network/certificate_pattern_matcher.h create mode 100644 chromeos/network/client_cert_util.cc create mode 100644 chromeos/network/client_cert_util.h 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 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 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& task_runner) { + const scoped_refptr& 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 +#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& task_runner); + const scoped_refptr& 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.cc b/chromeos/network/certificate_pattern_matcher.cc deleted file mode 100644 index 3d7f328..0000000 --- a/chromeos/network/certificate_pattern_matcher.cc +++ /dev/null @@ -1,193 +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. - -#include "chromeos/network/certificate_pattern_matcher.h" - -#include -#include - -#include -#include -#include - -#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" - -namespace chromeos { - -namespace { - -// 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; -} - -// Functor to filter out non-matching issuers. -class IssuerFilter { - public: - explicit IssuerFilter(const IssuerSubjectPattern& issuer) - : issuer_(issuer) {} - bool operator()(const scoped_refptr& cert) const { - return !CertPrincipalMatches(issuer_, cert.get()->issuer()); - } - private: - const IssuerSubjectPattern& issuer_; -}; - -// Functor to filter out non-matching subjects. -class SubjectFilter { - public: - explicit SubjectFilter(const IssuerSubjectPattern& subject) - : subject_(subject) {} - bool operator()(const scoped_refptr& cert) const { - return !CertPrincipalMatches(subject_, cert.get()->subject()); - } - private: - const IssuerSubjectPattern& subject_; -}; - -// Functor to filter out certs that don't have private keys, or are invalid. -class PrivateKeyFilter { - public: - explicit PrivateKeyFilter(net::CertDatabase* cert_db) : cert_db_(cert_db) {} - bool operator()(const scoped_refptr& cert) const { - return cert_db_->CheckUserCert(cert.get()) != net::OK; - } - private: - net::CertDatabase* cert_db_; -}; - -// Functor to filter out certs that don't have an issuer in the associated -// IssuerCAPEMs list. -class IssuerCaFilter { - public: - explicit IssuerCaFilter(const std::vector& issuer_ca_pems) - : issuer_ca_pems_(issuer_ca_pems) {} - bool operator()(const scoped_refptr& cert) const { - // Find the certificate issuer for each certificate. - // TODO(gspencer): this functionality should be available from - // X509Certificate or NSSCertDatabase. - CERTCertificate* issuer_cert = CERT_FindCertIssuer( - cert.get()->os_cert_handle(), PR_Now(), certUsageAnyCA); - - if (!issuer_cert) - return true; - - std::string pem_encoded; - if (!net::X509Certificate::GetPEMEncoded(issuer_cert, &pem_encoded)) { - LOG(ERROR) << "Couldn't PEM-encode certificate."; - return true; - } - - return (std::find(issuer_ca_pems_.begin(), issuer_ca_pems_.end(), - pem_encoded) == - issuer_ca_pems_.end()); - } - private: - const std::vector& issuer_ca_pems_; -}; - -} // namespace - -namespace certificate_pattern { - -scoped_refptr GetCertificateMatch( - const CertificatePattern& pattern) { - typedef std::list > CertificateStlList; - - // Start with all the certs, and narrow it down from there. - net::CertificateList all_certs; - CertificateStlList matching_certs; - net::NSSCertDatabase::GetInstance()->ListCerts(&all_certs); - - if (all_certs.empty()) - return NULL; - - for (net::CertificateList::iterator iter = all_certs.begin(); - iter != all_certs.end(); ++iter) { - matching_certs.push_back(*iter); - } - - // Strip off any certs that don't have the right issuer and/or subject. - if (!pattern.issuer().Empty()) { - matching_certs.remove_if(IssuerFilter(pattern.issuer())); - if (matching_certs.empty()) - return NULL; - } - - if (!pattern.subject().Empty()) { - matching_certs.remove_if(SubjectFilter(pattern.subject())); - if (matching_certs.empty()) - return NULL; - } - - if (!pattern.issuer_ca_pems().empty()) { - matching_certs.remove_if(IssuerCaFilter(pattern.issuer_ca_pems())); - if (matching_certs.empty()) - return NULL; - } - - // Eliminate any certs that don't have private keys associated with - // them. The CheckUserCert call in the filter is a little slow (because of - // underlying PKCS11 calls), so we do this last to reduce the number of times - // we have to call it. - PrivateKeyFilter private_filter(net::CertDatabase::GetInstance()); - matching_certs.remove_if(private_filter); - - if (matching_certs.empty()) - return NULL; - - // We now have a list of certificates that match the pattern we're - // looking for. Now we find the one with the latest start date. - scoped_refptr latest(NULL); - - // Iterate over the rest looking for the one that was issued latest. - for (CertificateStlList::iterator iter = matching_certs.begin(); - iter != matching_certs.end(); ++iter) { - if (!latest.get() || (*iter)->valid_start() > latest->valid_start()) - latest = *iter; - } - - return latest; -} - -} // namespace certificate_pattern - -} // namespace chromeos 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 GetCertificateMatch( - const CertificatePattern& pattern); - -} // namespace certificate_pattern - -} // namespace chromeos - -#endif // CHROMEOS_NETWORK_CERTIFICATE_PATTERN_MATCHER_H_ diff --git a/chromeos/network/client_cert_util.cc b/chromeos/network/client_cert_util.cc new file mode 100644 index 0000000..ad2bc45 --- /dev/null +++ b/chromeos/network/client_cert_util.cc @@ -0,0 +1,243 @@ +// 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. + +#include "chromeos/network/client_cert_util.h" + +#include +#include + +#include +#include +#include + +#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 client_cert { + +namespace { + +// Functor to filter out non-matching issuers. +class IssuerFilter { + public: + explicit IssuerFilter(const IssuerSubjectPattern& issuer) + : issuer_(issuer) {} + bool operator()(const scoped_refptr& cert) const { + return !CertPrincipalMatches(issuer_, cert.get()->issuer()); + } + private: + const IssuerSubjectPattern& issuer_; +}; + +// Functor to filter out non-matching subjects. +class SubjectFilter { + public: + explicit SubjectFilter(const IssuerSubjectPattern& subject) + : subject_(subject) {} + bool operator()(const scoped_refptr& cert) const { + return !CertPrincipalMatches(subject_, cert.get()->subject()); + } + private: + const IssuerSubjectPattern& subject_; +}; + +// Functor to filter out certs that don't have private keys, or are invalid. +class PrivateKeyFilter { + public: + explicit PrivateKeyFilter(net::CertDatabase* cert_db) : cert_db_(cert_db) {} + bool operator()(const scoped_refptr& cert) const { + return cert_db_->CheckUserCert(cert.get()) != net::OK; + } + private: + net::CertDatabase* cert_db_; +}; + +// Functor to filter out certs that don't have an issuer in the associated +// IssuerCAPEMs list. +class IssuerCaFilter { + public: + explicit IssuerCaFilter(const std::vector& issuer_ca_pems) + : issuer_ca_pems_(issuer_ca_pems) {} + bool operator()(const scoped_refptr& cert) const { + // Find the certificate issuer for each certificate. + // TODO(gspencer): this functionality should be available from + // X509Certificate or NSSCertDatabase. + CERTCertificate* issuer_cert = CERT_FindCertIssuer( + cert.get()->os_cert_handle(), PR_Now(), certUsageAnyCA); + + if (!issuer_cert) + return true; + + std::string pem_encoded; + if (!net::X509Certificate::GetPEMEncoded(issuer_cert, &pem_encoded)) { + LOG(ERROR) << "Couldn't PEM-encode certificate."; + return true; + } + + return (std::find(issuer_ca_pems_.begin(), issuer_ca_pems_.end(), + pem_encoded) == + issuer_ca_pems_.end()); + } + private: + const std::vector& issuer_ca_pems_; +}; + +} // namespace + +// 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 GetCertificateMatch( + const CertificatePattern& pattern) { + typedef std::list > CertificateStlList; + + // Start with all the certs, and narrow it down from there. + net::CertificateList all_certs; + CertificateStlList matching_certs; + net::NSSCertDatabase::GetInstance()->ListCerts(&all_certs); + + if (all_certs.empty()) + return NULL; + + for (net::CertificateList::iterator iter = all_certs.begin(); + iter != all_certs.end(); ++iter) { + matching_certs.push_back(*iter); + } + + // Strip off any certs that don't have the right issuer and/or subject. + if (!pattern.issuer().Empty()) { + matching_certs.remove_if(IssuerFilter(pattern.issuer())); + if (matching_certs.empty()) + return NULL; + } + + if (!pattern.subject().Empty()) { + matching_certs.remove_if(SubjectFilter(pattern.subject())); + if (matching_certs.empty()) + return NULL; + } + + if (!pattern.issuer_ca_pems().empty()) { + matching_certs.remove_if(IssuerCaFilter(pattern.issuer_ca_pems())); + if (matching_certs.empty()) + return NULL; + } + + // Eliminate any certs that don't have private keys associated with + // them. The CheckUserCert call in the filter is a little slow (because of + // underlying PKCS11 calls), so we do this last to reduce the number of times + // we have to call it. + PrivateKeyFilter private_filter(net::CertDatabase::GetInstance()); + matching_certs.remove_if(private_filter); + + if (matching_certs.empty()) + return NULL; + + // We now have a list of certificates that match the pattern we're + // looking for. Now we find the one with the latest start date. + scoped_refptr latest(NULL); + + // Iterate over the rest looking for the one that was issued latest. + for (CertificateStlList::iterator iter = matching_certs.begin(); + iter != matching_certs.end(); ++iter) { + if (!latest.get() || (*iter)->valid_start() > latest->valid_start()) + latest = *iter; + } + + return latest; +} + +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 + +#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 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 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 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 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); -- cgit v1.1