Commit 70753227 authored by eugenis's avatar eugenis Committed by Commit bot

Revert of Make ONCCertificateImporter async. (patchset #5 id:190001 of...

Revert of Make ONCCertificateImporter async. (patchset #5 id:190001 of https://codereview.chromium.org/547553005/)

Reason for revert:
Use-after-free.
https://code.google.com/p/chromium/issues/detail?id=415916

Original issue's description:
> Make ONCCertificateImporter async.
>
> This prepares for the new CertDatabase keyed service, which will have stricter threading restrictions. https://codereview.chromium.org/419013003/
>
> Before, ONCCertificateImporter accessed the NSSCertDatabase from the UI thread and blocked on certificate store operations.
>
> Now, the import itself is asychronous and calls back on completion.
>
> While there, this also removes the GMock of the importer.
>
> BUG=413219
>
> Committed: https://crrev.com/bc656c0e7b7bd67fb28e5a880d21b9510ebd3e3a
> Cr-Commit-Position: refs/heads/master@{#295534}

TBR=joaodasilva@chromium.org,eroman@chromium.org,pneubeck@chromium.org
NOTREECHECKS=true
NOTRY=true
BUG=413219

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

Cr-Commit-Position: refs/heads/master@{#295683}
parent 4a201500
...@@ -15,7 +15,6 @@ ...@@ -15,7 +15,6 @@
#include "chromeos/network/managed_network_configuration_handler.h" #include "chromeos/network/managed_network_configuration_handler.h"
#include "chromeos/network/onc/onc_certificate_importer_impl.h" #include "chromeos/network/onc/onc_certificate_importer_impl.h"
#include "components/user_manager/user.h" #include "components/user_manager/user.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/notification_source.h" #include "content/public/browser/notification_source.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
#include "policy/policy_constants.h" #include "policy/policy_constants.h"
...@@ -85,15 +84,6 @@ void UserNetworkConfigurationUpdater::GetWebTrustedCertificates( ...@@ -85,15 +84,6 @@ void UserNetworkConfigurationUpdater::GetWebTrustedCertificates(
*certs = web_trust_certs_; *certs = web_trust_certs_;
} }
void UserNetworkConfigurationUpdater::OnCertificatesImported(
bool /* unused success */,
const net::CertificateList& onc_trusted_certificates) {
web_trust_certs_.clear();
if (allow_trusted_certificates_from_policy_)
web_trust_certs_ = onc_trusted_certificates;
NotifyTrustAnchorsChanged();
}
void UserNetworkConfigurationUpdater::ImportCertificates( void UserNetworkConfigurationUpdater::ImportCertificates(
const base::ListValue& certificates_onc) { const base::ListValue& certificates_onc) {
// If certificate importer is not yet set, cache the certificate onc. It will // If certificate importer is not yet set, cache the certificate onc. It will
...@@ -103,11 +93,13 @@ void UserNetworkConfigurationUpdater::ImportCertificates( ...@@ -103,11 +93,13 @@ void UserNetworkConfigurationUpdater::ImportCertificates(
return; return;
} }
web_trust_certs_.clear();
certificate_importer_->ImportCertificates( certificate_importer_->ImportCertificates(
certificates_onc, certificates_onc,
onc_source_, onc_source_,
base::Bind(&UserNetworkConfigurationUpdater::OnCertificatesImported, allow_trusted_certificates_from_policy_ ? &web_trust_certs_ : NULL);
base::Unretained(this)));
NotifyTrustAnchorsChanged();
} }
void UserNetworkConfigurationUpdater::ApplyNetworkPolicy( void UserNetworkConfigurationUpdater::ApplyNetworkPolicy(
...@@ -140,10 +132,7 @@ void UserNetworkConfigurationUpdater::CreateAndSetCertificateImporter( ...@@ -140,10 +132,7 @@ void UserNetworkConfigurationUpdater::CreateAndSetCertificateImporter(
net::NSSCertDatabase* database) { net::NSSCertDatabase* database) {
DCHECK(database); DCHECK(database);
SetCertificateImporter(scoped_ptr<chromeos::onc::CertificateImporter>( SetCertificateImporter(scoped_ptr<chromeos::onc::CertificateImporter>(
new chromeos::onc::CertificateImporterImpl( new chromeos::onc::CertificateImporterImpl(database)));
content::BrowserThread::GetMessageLoopProxyForThread(
content::BrowserThread::IO),
database)));
} }
void UserNetworkConfigurationUpdater::SetCertificateImporter( void UserNetworkConfigurationUpdater::SetCertificateImporter(
......
...@@ -95,11 +95,6 @@ class UserNetworkConfigurationUpdater : public NetworkConfigurationUpdater, ...@@ -95,11 +95,6 @@ class UserNetworkConfigurationUpdater : public NetworkConfigurationUpdater,
PolicyService* policy_service, PolicyService* policy_service,
chromeos::ManagedNetworkConfigurationHandler* network_config_handler); chromeos::ManagedNetworkConfigurationHandler* network_config_handler);
// Called by the CertificateImporter when an import finished.
void OnCertificatesImported(
bool success,
const net::CertificateList& onc_trusted_certificates);
// NetworkConfigurationUpdater: // NetworkConfigurationUpdater:
virtual void ImportCertificates( virtual void ImportCertificates(
const base::ListValue& certificates_onc) OVERRIDE; const base::ListValue& certificates_onc) OVERRIDE;
......
...@@ -257,13 +257,6 @@ class NetInternalsMessageHandler ...@@ -257,13 +257,6 @@ class NetInternalsMessageHandler
void ImportONCFileToNSSDB(const std::string& onc_blob, void ImportONCFileToNSSDB(const std::string& onc_blob,
const std::string& passcode, const std::string& passcode,
net::NSSCertDatabase* nssdb); net::NSSCertDatabase* nssdb);
// Called back by the CertificateImporter when a certificate import finished.
// |previous_error| contains earlier errors during this import.
void OnCertificatesImported(
const std::string& previous_error,
bool success,
const net::CertificateList& onc_trusted_certificates);
#endif #endif
private: private:
...@@ -1414,52 +1407,38 @@ void NetInternalsMessageHandler::ImportONCFileToNSSDB( ...@@ -1414,52 +1407,38 @@ void NetInternalsMessageHandler::ImportONCFileToNSSDB(
const std::string& onc_blob, const std::string& onc_blob,
const std::string& passcode, const std::string& passcode,
net::NSSCertDatabase* nssdb) { net::NSSCertDatabase* nssdb) {
std::string error;
user_manager::User* user = chromeos::ProfileHelper::Get()->GetUserByProfile( user_manager::User* user = chromeos::ProfileHelper::Get()->GetUserByProfile(
Profile::FromWebUI(web_ui())); Profile::FromWebUI(web_ui()));
if (!user) { if (user) {
std::string error = "User not found."; onc::ONCSource onc_source = onc::ONC_SOURCE_USER_IMPORT;
SendJavascriptCommand("receivedONCFileParse", new base::StringValue(error));
return; base::ListValue network_configs;
} base::DictionaryValue global_network_config;
base::ListValue certificates;
if (!chromeos::onc::ParseAndValidateOncForImport(onc_blob,
onc_source,
passcode,
&network_configs,
&global_network_config,
&certificates)) {
error = "Errors occurred during the ONC parsing. ";
}
std::string error; chromeos::onc::CertificateImporterImpl cert_importer(nssdb);
onc::ONCSource onc_source = onc::ONC_SOURCE_USER_IMPORT; if (!cert_importer.ImportCertificates(certificates, onc_source, NULL))
base::ListValue network_configs; error += "Some certificates couldn't be imported. ";
base::DictionaryValue global_network_config;
base::ListValue certificates;
if (!chromeos::onc::ParseAndValidateOncForImport(onc_blob,
onc_source,
passcode,
&network_configs,
&global_network_config,
&certificates)) {
error = "Errors occurred during the ONC parsing. ";
}
std::string network_error; std::string network_error;
chromeos::onc::ImportNetworksForUser(user, network_configs, &network_error); chromeos::onc::ImportNetworksForUser(user, network_configs, &network_error);
if (!network_error.empty()) if (!network_error.empty())
error += network_error; error += network_error;
} else {
chromeos::onc::CertificateImporterImpl cert_importer( error = "User not found.";
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO), nssdb); }
cert_importer.ImportCertificates(
certificates,
onc_source,
base::Bind(&NetInternalsMessageHandler::OnCertificatesImported,
AsWeakPtr(),
error));
}
void NetInternalsMessageHandler::OnCertificatesImported(
const std::string& previous_error,
bool success,
const net::CertificateList& /* unused onc_trusted_certificates */) {
std::string error = previous_error;
if (!success)
error += "Some certificates couldn't be imported. ";
LOG_IF(ERROR, !error.empty()) << error;
SendJavascriptCommand("receivedONCFileParse", new base::StringValue(error)); SendJavascriptCommand("receivedONCFileParse", new base::StringValue(error));
} }
......
...@@ -475,6 +475,8 @@ ...@@ -475,6 +475,8 @@
'network/fake_network_device_handler.h', 'network/fake_network_device_handler.h',
'network/mock_managed_network_configuration_handler.cc', 'network/mock_managed_network_configuration_handler.cc',
'network/mock_managed_network_configuration_handler.h', 'network/mock_managed_network_configuration_handler.h',
'network/onc/mock_certificate_importer.cc',
'network/onc/mock_certificate_importer.h',
'network/onc/onc_test_utils.cc', 'network/onc/onc_test_utils.cc',
'network/onc/onc_test_utils.h', 'network/onc/onc_test_utils.h',
'system/mock_statistics_provider.cc', 'system/mock_statistics_provider.cc',
......
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#include "chromeos/network/onc/mock_certificate_importer.h"
namespace chromeos {
namespace onc {
MockCertificateImporter::MockCertificateImporter() {
}
MockCertificateImporter::~MockCertificateImporter() {
}
} // namespace onc
} // namespace chromeos
// Copyright 2013 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
#ifndef CHROMEOS_NETWORK_ONC_MOCK_CERTIFICATE_IMPORTER_H_
#define CHROMEOS_NETWORK_ONC_MOCK_CERTIFICATE_IMPORTER_H_
#include "base/basictypes.h"
#include "base/values.h"
#include "chromeos/chromeos_export.h"
#include "chromeos/network/onc/onc_certificate_importer.h"
#include "testing/gmock/include/gmock/gmock.h"
namespace chromeos {
namespace onc {
class CHROMEOS_EXPORT MockCertificateImporter : public CertificateImporter {
public:
MockCertificateImporter();
virtual ~MockCertificateImporter();
MOCK_METHOD3(ImportCertificates, bool(const base::ListValue&,
::onc::ONCSource,
net::CertificateList*));
private:
DISALLOW_COPY_AND_ASSIGN(MockCertificateImporter);
};
} // namespace onc
} // namespace chromeos
#endif // CHROMEOS_NETWORK_ONC_MOCK_CERTIFICATE_IMPORTER_H_
...@@ -6,7 +6,6 @@ ...@@ -6,7 +6,6 @@
#define CHROMEOS_NETWORK_ONC_ONC_CERTIFICATE_IMPORTER_H_ #define CHROMEOS_NETWORK_ONC_ONC_CERTIFICATE_IMPORTER_H_
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/callback_forward.h"
#include "chromeos/chromeos_export.h" #include "chromeos/chromeos_export.h"
#include "components/onc/onc_constants.h" #include "components/onc/onc_constants.h"
#include "net/cert/x509_certificate.h" #include "net/cert/x509_certificate.h"
...@@ -20,25 +19,20 @@ namespace onc { ...@@ -20,25 +19,20 @@ namespace onc {
class CHROMEOS_EXPORT CertificateImporter { class CHROMEOS_EXPORT CertificateImporter {
public: public:
typedef base::Callback<
void(bool success, const net::CertificateList& onc_trusted_certificates)>
DoneCallback;
CertificateImporter() {} CertificateImporter() {}
virtual ~CertificateImporter() {} virtual ~CertificateImporter() {}
// Import |certificates|, which must be a list of ONC Certificate objects. // Import the |certificates|, which must be a list of ONC Certificate objects.
// Certificates are only imported with web trust for user imports. If the // Certificates are only imported with web trust for user imports. If
// "Remove" field of a certificate is enabled, then removes the certificate // |onc_trusted_certificates| is not NULL, it will be filled with the list
// from the store instead of importing. // of certificates that requested the TrustBit "Web". If the "Remove" field of
// When the import is completed, |done_callback| will be called with |success| // a certificate is enabled, then removes the certificate from the store
// equal to true if all certificates were imported successfully. // instead of importing. Returns true if all certificates were imported
// |onc_trusted_certificates| will contain the list of certificates that // successfully.
// were imported and requested the TrustBit "Web". virtual bool ImportCertificates(
// Never calls |done_callback| after this importer is destructed. const base::ListValue& certificates,
virtual void ImportCertificates(const base::ListValue& certificates, ::onc::ONCSource source,
::onc::ONCSource source, net::CertificateList* onc_trusted_certificates) = 0;
const DoneCallback& done_callback) = 0;
private: private:
DISALLOW_COPY_AND_ASSIGN(CertificateImporter); DISALLOW_COPY_AND_ASSIGN(CertificateImporter);
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "base/basictypes.h" #include "base/basictypes.h"
#include "base/memory/ref_counted.h" #include "base/memory/ref_counted.h"
#include "base/memory/scoped_ptr.h" #include "base/memory/scoped_ptr.h"
#include "base/memory/weak_ptr.h"
#include "chromeos/chromeos_export.h" #include "chromeos/chromeos_export.h"
#include "chromeos/network/onc/onc_certificate_importer.h" #include "chromeos/network/onc/onc_certificate_importer.h"
#include "components/onc/onc_constants.h" #include "components/onc/onc_constants.h"
...@@ -20,8 +19,6 @@ ...@@ -20,8 +19,6 @@
namespace base { namespace base {
class DictionaryValue; class DictionaryValue;
class ListValue; class ListValue;
class SequencedTaskRunner;
class SingleThreadTaskRunner;
} }
namespace net { namespace net {
...@@ -34,36 +31,34 @@ namespace chromeos { ...@@ -34,36 +31,34 @@ namespace chromeos {
namespace onc { namespace onc {
// This class handles certificate imports from ONC (both policy and user // This class handles certificate imports from ONC (both policy and user
// imports) into a certificate store. The GUID of Client certificates is stored // imports) into the certificate store. The GUID of Client certificates is
// together with the certificate as Nickname. In contrast, Server and CA // stored together with the certificate as Nickname. In contrast, Server and CA
// certificates are identified by their PEM and not by GUID. // certificates are identified by their PEM and not by GUID.
// TODO(pneubeck): Replace Nickname by PEM for Client // TODO(pneubeck): Replace Nickname by PEM for Client
// certificates. http://crbug.com/252119 // certificates. http://crbug.com/252119
class CHROMEOS_EXPORT CertificateImporterImpl : public CertificateImporter { class CHROMEOS_EXPORT CertificateImporterImpl : public CertificateImporter {
public: public:
// |io_task_runner| will be used for NSSCertDatabase accesses. typedef std::map<std::string, scoped_refptr<net::X509Certificate> >
CertificateImporterImpl( CertsByGUID;
const scoped_refptr<base::SequencedTaskRunner>& io_task_runner,
net::NSSCertDatabase* target_nssdb_); explicit CertificateImporterImpl(net::NSSCertDatabase* target_nssdb_);
virtual ~CertificateImporterImpl();
// CertificateImporter overrides // CertificateImporter overrides
virtual void ImportCertificates(const base::ListValue& certificates, virtual bool ImportCertificates(
::onc::ONCSource source, const base::ListValue& certificates,
const DoneCallback& done_callback) OVERRIDE; ::onc::ONCSource source,
net::CertificateList* onc_trusted_certificates) OVERRIDE;
// This implements ImportCertificates. Additionally, if
// |imported_server_and_ca_certs| is not NULL, it will be filled with the
// (GUID, Certificate) pairs of all succesfully imported Server and CA
// certificates.
bool ParseAndStoreCertificates(bool allow_trust_imports,
const base::ListValue& onc_certificates,
net::CertificateList* onc_trusted_certificates,
CertsByGUID* imported_server_and_ca_certs);
private: private:
void RunDoneCallback(const CertificateImporter::DoneCallback& callback,
bool success,
const net::CertificateList& onc_trusted_certificates);
// This is the synchronous implementation of ImportCertificates. It is
// executed on the given |io_task_runner_|.
static void ParseAndStoreCertificates(::onc::ONCSource source,
const DoneCallback& done_callback,
base::ListValue* certificates,
net::NSSCertDatabase* nssdb);
// Lists the certificates that have the string |label| as their certificate // Lists the certificates that have the string |label| as their certificate
// nickname (exact match). // nickname (exact match).
static void ListCertsWithNickname(const std::string& label, static void ListCertsWithNickname(const std::string& label,
...@@ -77,36 +72,30 @@ class CHROMEOS_EXPORT CertificateImporterImpl : public CertificateImporter { ...@@ -77,36 +72,30 @@ class CHROMEOS_EXPORT CertificateImporterImpl : public CertificateImporter {
// Parses and stores/removes |certificate| in/from the certificate // Parses and stores/removes |certificate| in/from the certificate
// store. Returns true if the operation succeeded. // store. Returns true if the operation succeeded.
static bool ParseAndStoreCertificate( bool ParseAndStoreCertificate(
bool allow_trust_imports, bool allow_trust_imports,
const base::DictionaryValue& certificate, const base::DictionaryValue& certificate,
net::NSSCertDatabase* nssdb, net::CertificateList* onc_trusted_certificates,
net::CertificateList* onc_trusted_certificates); CertsByGUID* imported_server_and_ca_certs);
// Imports the Server or CA certificate |certificate|. Web trust is only // Imports the Server or CA certificate |certificate|. Web trust is only
// applied if the certificate requests the TrustBits attribute "Web" and if // applied if the certificate requests the TrustBits attribute "Web" and if
// the |allow_trust_imports| permission is granted, otherwise the attribute is // the |allow_trust_imports| permission is granted, otherwise the attribute is
// ignored. // ignored.
static bool ParseServerOrCaCertificate( bool ParseServerOrCaCertificate(
bool allow_trust_imports, bool allow_trust_imports,
const std::string& cert_type, const std::string& cert_type,
const std::string& guid, const std::string& guid,
const base::DictionaryValue& certificate, const base::DictionaryValue& certificate,
net::NSSCertDatabase* nssdb, net::CertificateList* onc_trusted_certificates,
net::CertificateList* onc_trusted_certificates); CertsByGUID* imported_server_and_ca_certs);
static bool ParseClientCertificate(const std::string& guid, bool ParseClientCertificate(const std::string& guid,
const base::DictionaryValue& certificate, const base::DictionaryValue& certificate);
net::NSSCertDatabase* nssdb);
// The task runner to use for NSSCertDatabase accesses.
scoped_refptr<base::SequencedTaskRunner> io_task_runner_;
// The certificate database to which certificates are imported. // The certificate database to which certificates are imported.
net::NSSCertDatabase* target_nssdb_; net::NSSCertDatabase* target_nssdb_;
base::WeakPtrFactory<CertificateImporterImpl> weak_factory_;
DISALLOW_COPY_AND_ASSIGN(CertificateImporterImpl); DISALLOW_COPY_AND_ASSIGN(CertificateImporterImpl);
}; };
......
...@@ -13,12 +13,11 @@ ...@@ -13,12 +13,11 @@
#include "base/bind.h" #include "base/bind.h"
#include "base/logging.h" #include "base/logging.h"
#include "base/strings/string_number_conversions.h" #include "base/strings/string_number_conversions.h"
#include "base/test/test_simple_task_runner.h"
#include "base/thread_task_runner_handle.h"
#include "base/values.h" #include "base/values.h"
#include "chromeos/network/onc/onc_test_utils.h" #include "chromeos/network/onc/onc_test_utils.h"
#include "components/onc/onc_constants.h" #include "components/onc/onc_constants.h"
#include "crypto/scoped_test_nss_db.h" #include "crypto/nss_util_internal.h"
#include "crypto/scoped_test_nss_chromeos_user.h"
#include "net/base/crypto_module.h" #include "net/base/crypto_module.h"
#include "net/cert/cert_type.h" #include "net/cert/cert_type.h"
#include "net/cert/nss_cert_database_chromeos.h" #include "net/cert/nss_cert_database_chromeos.h"
...@@ -64,39 +63,30 @@ net::CertType GetCertType(net::X509Certificate::OSCertHandle cert) { ...@@ -64,39 +63,30 @@ net::CertType GetCertType(net::X509Certificate::OSCertHandle cert) {
class ONCCertificateImporterImplTest : public testing::Test { class ONCCertificateImporterImplTest : public testing::Test {
public: public:
ONCCertificateImporterImplTest() {} ONCCertificateImporterImplTest() : user_("username_hash"),
virtual ~ONCCertificateImporterImplTest() {} private_user_("private_user_hash") {}
virtual void SetUp() OVERRIDE { virtual void SetUp() {
ASSERT_TRUE(public_nssdb_.is_open()); ASSERT_TRUE(user_.constructed_successfully());
ASSERT_TRUE(private_nssdb_.is_open()); ASSERT_TRUE(private_user_.constructed_successfully());
task_runner_ = new base::TestSimpleTaskRunner(); // By default test user will have the same public and private slot.
thread_task_runner_handle_.reset( // Unfortunatelly, ONC importer should care about which slot certificates
new base::ThreadTaskRunnerHandle(task_runner_)); // get imported to. To work around this, we create another NSS user whose
// public slot will act as the private slot.
// TODO(tbarzic): See if there's a better way to achieve this.
test_nssdb_.reset(new net::NSSCertDatabaseChromeOS( test_nssdb_.reset(new net::NSSCertDatabaseChromeOS(
crypto::ScopedPK11Slot(public_nssdb_.slot()), crypto::GetPublicSlotForChromeOSUser(user_.username_hash()),
crypto::ScopedPK11Slot(private_nssdb_.slot()))); crypto::GetPublicSlotForChromeOSUser(private_user_.username_hash())));
// Test db should be empty at start of test. // Test db should be empty at start of test.
EXPECT_TRUE(ListCertsInPublicSlot().empty()); EXPECT_TRUE(ListCertsInPublicSlot().empty());
EXPECT_TRUE(ListCertsInPrivateSlot().empty()); EXPECT_TRUE(ListCertsInPrivateSlot().empty());
} }
virtual void TearDown() OVERRIDE { virtual ~ONCCertificateImporterImplTest() {}
thread_task_runner_handle_.reset();
task_runner_ = NULL;
}
protected: protected:
void OnImportCompleted(bool expected_success,
bool success,
const net::CertificateList& onc_trusted_certificates) {
EXPECT_EQ(expected_success, success);
web_trust_certificates_ = onc_trusted_certificates;
}
void AddCertificatesFromFile(std::string filename, bool expected_success) { void AddCertificatesFromFile(std::string filename, bool expected_success) {
scoped_ptr<base::DictionaryValue> onc = scoped_ptr<base::DictionaryValue> onc =
test_utils::ReadTestDictionary(filename); test_utils::ReadTestDictionary(filename);
...@@ -108,15 +98,14 @@ class ONCCertificateImporterImplTest : public testing::Test { ...@@ -108,15 +98,14 @@ class ONCCertificateImporterImplTest : public testing::Test {
onc_certificates_.reset(certificates); onc_certificates_.reset(certificates);
web_trust_certificates_.clear(); web_trust_certificates_.clear();
CertificateImporterImpl importer(task_runner_, test_nssdb_.get()); imported_server_and_ca_certs_.clear();
importer.ImportCertificates( CertificateImporterImpl importer(test_nssdb_.get());
*certificates, EXPECT_EQ(
::onc::ONC_SOURCE_USER_IMPORT, // allow web trust expected_success,
base::Bind(&ONCCertificateImporterImplTest::OnImportCompleted, importer.ParseAndStoreCertificates(true, // allow web trust
base::Unretained(this), *certificates,
expected_success)); &web_trust_certificates_,
&imported_server_and_ca_certs_));
task_runner_->RunUntilIdle();
public_list_ = ListCertsInPublicSlot(); public_list_ = ListCertsInPublicSlot();
private_list_ = ListCertsInPrivateSlot(); private_list_ = ListCertsInPrivateSlot();
...@@ -130,24 +119,25 @@ class ONCCertificateImporterImplTest : public testing::Test { ...@@ -130,24 +119,25 @@ class ONCCertificateImporterImplTest : public testing::Test {
guid = &guid_temporary; guid = &guid_temporary;
AddCertificatesFromFile(filename, true); AddCertificatesFromFile(filename, true);
ASSERT_EQ(1ul, public_list_.size() + private_list_.size());
if (expected_type == net::SERVER_CERT || expected_type == net::CA_CERT) { if (!public_list_.empty())
ASSERT_EQ(1u, public_list_.size());
EXPECT_EQ(expected_type, GetCertType(public_list_[0]->os_cert_handle())); EXPECT_EQ(expected_type, GetCertType(public_list_[0]->os_cert_handle()));
EXPECT_TRUE(private_list_.empty()); if (!private_list_.empty())
} else { // net::USER_CERT
EXPECT_TRUE(public_list_.empty());
ASSERT_EQ(1u, private_list_.size());
EXPECT_EQ(expected_type, GetCertType(private_list_[0]->os_cert_handle())); EXPECT_EQ(expected_type, GetCertType(private_list_[0]->os_cert_handle()));
}
base::DictionaryValue* certificate = NULL; base::DictionaryValue* certificate = NULL;
onc_certificates_->GetDictionary(0, &certificate); onc_certificates_->GetDictionary(0, &certificate);
certificate->GetStringWithoutPathExpansion(::onc::certificate::kGUID, guid); certificate->GetStringWithoutPathExpansion(::onc::certificate::kGUID, guid);
if (expected_type == net::SERVER_CERT || expected_type == net::CA_CERT) {
EXPECT_EQ(1u, imported_server_and_ca_certs_.size());
EXPECT_TRUE(
imported_server_and_ca_certs_[*guid]->Equals(public_list_[0].get()));
} else { // net::USER_CERT
EXPECT_TRUE(imported_server_and_ca_certs_.empty());
}
} }
scoped_refptr<base::TestSimpleTaskRunner> task_runner_;
scoped_ptr<base::ThreadTaskRunnerHandle> thread_task_runner_handle_;
scoped_ptr<net::NSSCertDatabaseChromeOS> test_nssdb_; scoped_ptr<net::NSSCertDatabaseChromeOS> test_nssdb_;
scoped_ptr<base::ListValue> onc_certificates_; scoped_ptr<base::ListValue> onc_certificates_;
// List of certs in the nssdb's public slot. // List of certs in the nssdb's public slot.
...@@ -155,17 +145,15 @@ class ONCCertificateImporterImplTest : public testing::Test { ...@@ -155,17 +145,15 @@ class ONCCertificateImporterImplTest : public testing::Test {
// List of certs in the nssdb's "private" slot. // List of certs in the nssdb's "private" slot.
net::CertificateList private_list_; net::CertificateList private_list_;
net::CertificateList web_trust_certificates_; net::CertificateList web_trust_certificates_;
CertificateImporterImpl::CertsByGUID imported_server_and_ca_certs_;
crypto::ScopedTestNSSDB public_nssdb_;
crypto::ScopedTestNSSDB private_nssdb_;
private: private:
net::CertificateList ListCertsInPublicSlot() { net::CertificateList ListCertsInPublicSlot() {
return ListCertsInSlot(public_nssdb_.slot()); return ListCertsInSlot(test_nssdb_->GetPublicSlot().get());
} }
net::CertificateList ListCertsInPrivateSlot() { net::CertificateList ListCertsInPrivateSlot() {
return ListCertsInSlot(private_nssdb_.slot()); return ListCertsInSlot(test_nssdb_->GetPrivateSlot().get());
} }
net::CertificateList ListCertsInSlot(PK11SlotInfo* slot) { net::CertificateList ListCertsInSlot(PK11SlotInfo* slot) {
...@@ -183,13 +171,16 @@ class ONCCertificateImporterImplTest : public testing::Test { ...@@ -183,13 +171,16 @@ class ONCCertificateImporterImplTest : public testing::Test {
std::sort(result.begin(), result.end(), net::X509Certificate::LessThan()); std::sort(result.begin(), result.end(), net::X509Certificate::LessThan());
return result; return result;
} }
crypto::ScopedTestNSSChromeOSUser user_;
crypto::ScopedTestNSSChromeOSUser private_user_;
}; };
TEST_F(ONCCertificateImporterImplTest, MultipleCertificates) { TEST_F(ONCCertificateImporterImplTest, MultipleCertificates) {
AddCertificatesFromFile("managed_toplevel2.onc", true); AddCertificatesFromFile("managed_toplevel2.onc", true);
EXPECT_EQ(onc_certificates_->GetSize(), public_list_.size()); EXPECT_EQ(onc_certificates_->GetSize(), public_list_.size());
EXPECT_TRUE(private_list_.empty()); EXPECT_TRUE(private_list_.empty());
EXPECT_EQ(2ul, public_list_.size()); EXPECT_EQ(2ul, imported_server_and_ca_certs_.size());
} }
TEST_F(ONCCertificateImporterImplTest, MultipleCertificatesWithFailures) { TEST_F(ONCCertificateImporterImplTest, MultipleCertificatesWithFailures) {
...@@ -197,6 +188,7 @@ TEST_F(ONCCertificateImporterImplTest, MultipleCertificatesWithFailures) { ...@@ -197,6 +188,7 @@ TEST_F(ONCCertificateImporterImplTest, MultipleCertificatesWithFailures) {
EXPECT_EQ(3ul, onc_certificates_->GetSize()); EXPECT_EQ(3ul, onc_certificates_->GetSize());
EXPECT_EQ(1ul, private_list_.size()); EXPECT_EQ(1ul, private_list_.size());
EXPECT_TRUE(public_list_.empty()); EXPECT_TRUE(public_list_.empty());
EXPECT_TRUE(imported_server_and_ca_certs_.empty());
} }
TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) { TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) {
...@@ -207,7 +199,7 @@ TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) { ...@@ -207,7 +199,7 @@ TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) {
EXPECT_TRUE(public_list_.empty()); EXPECT_TRUE(public_list_.empty());
SECKEYPrivateKeyList* privkey_list = SECKEYPrivateKeyList* privkey_list =
PK11_ListPrivKeysInSlot(private_nssdb_.slot(), NULL, NULL); PK11_ListPrivKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL, NULL);
EXPECT_TRUE(privkey_list); EXPECT_TRUE(privkey_list);
if (privkey_list) { if (privkey_list) {
SECKEYPrivateKeyListNode* node = PRIVKEY_LIST_HEAD(privkey_list); SECKEYPrivateKeyListNode* node = PRIVKEY_LIST_HEAD(privkey_list);
...@@ -224,7 +216,7 @@ TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) { ...@@ -224,7 +216,7 @@ TEST_F(ONCCertificateImporterImplTest, AddClientCertificate) {
} }
SECKEYPublicKeyList* pubkey_list = SECKEYPublicKeyList* pubkey_list =
PK11_ListPublicKeysInSlot(private_nssdb_.slot(), NULL); PK11_ListPublicKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL);
EXPECT_TRUE(pubkey_list); EXPECT_TRUE(pubkey_list);
if (pubkey_list) { if (pubkey_list) {
SECKEYPublicKeyListNode* node = PUBKEY_LIST_HEAD(pubkey_list); SECKEYPublicKeyListNode* node = PUBKEY_LIST_HEAD(pubkey_list);
...@@ -242,11 +234,11 @@ TEST_F(ONCCertificateImporterImplTest, AddServerCertificateWithWebTrust) { ...@@ -242,11 +234,11 @@ TEST_F(ONCCertificateImporterImplTest, AddServerCertificateWithWebTrust) {
AddCertificateFromFile("certificate-server.onc", net::SERVER_CERT, NULL); AddCertificateFromFile("certificate-server.onc", net::SERVER_CERT, NULL);
SECKEYPrivateKeyList* privkey_list = SECKEYPrivateKeyList* privkey_list =
PK11_ListPrivKeysInSlot(private_nssdb_.slot(), NULL, NULL); PK11_ListPrivKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL, NULL);
EXPECT_FALSE(privkey_list); EXPECT_FALSE(privkey_list);
SECKEYPublicKeyList* pubkey_list = SECKEYPublicKeyList* pubkey_list =
PK11_ListPublicKeysInSlot(private_nssdb_.slot(), NULL); PK11_ListPublicKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL);
EXPECT_FALSE(pubkey_list); EXPECT_FALSE(pubkey_list);
ASSERT_EQ(1u, web_trust_certificates_.size()); ASSERT_EQ(1u, web_trust_certificates_.size());
...@@ -260,11 +252,11 @@ TEST_F(ONCCertificateImporterImplTest, AddWebAuthorityCertificateWithWebTrust) { ...@@ -260,11 +252,11 @@ TEST_F(ONCCertificateImporterImplTest, AddWebAuthorityCertificateWithWebTrust) {
AddCertificateFromFile("certificate-web-authority.onc", net::CA_CERT, NULL); AddCertificateFromFile("certificate-web-authority.onc", net::CA_CERT, NULL);
SECKEYPrivateKeyList* privkey_list = SECKEYPrivateKeyList* privkey_list =
PK11_ListPrivKeysInSlot(private_nssdb_.slot(), NULL, NULL); PK11_ListPrivKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL, NULL);
EXPECT_FALSE(privkey_list); EXPECT_FALSE(privkey_list);
SECKEYPublicKeyList* pubkey_list = SECKEYPublicKeyList* pubkey_list =
PK11_ListPublicKeysInSlot(private_nssdb_.slot(), NULL); PK11_ListPublicKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL);
EXPECT_FALSE(pubkey_list); EXPECT_FALSE(pubkey_list);
ASSERT_EQ(1u, web_trust_certificates_.size()); ASSERT_EQ(1u, web_trust_certificates_.size());
...@@ -279,11 +271,11 @@ TEST_F(ONCCertificateImporterImplTest, AddAuthorityCertificateWithoutWebTrust) { ...@@ -279,11 +271,11 @@ TEST_F(ONCCertificateImporterImplTest, AddAuthorityCertificateWithoutWebTrust) {
EXPECT_TRUE(web_trust_certificates_.empty()); EXPECT_TRUE(web_trust_certificates_.empty());
SECKEYPrivateKeyList* privkey_list = SECKEYPrivateKeyList* privkey_list =
PK11_ListPrivKeysInSlot(private_nssdb_.slot(), NULL, NULL); PK11_ListPrivKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL, NULL);
EXPECT_FALSE(privkey_list); EXPECT_FALSE(privkey_list);
SECKEYPublicKeyList* pubkey_list = SECKEYPublicKeyList* pubkey_list =
PK11_ListPublicKeysInSlot(private_nssdb_.slot(), NULL); PK11_ListPublicKeysInSlot(test_nssdb_->GetPrivateSlot().get(), NULL);
EXPECT_FALSE(pubkey_list); EXPECT_FALSE(pubkey_list);
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment