diff options
author | nharper <nharper@chromium.org> | 2015-06-01 13:29:23 -0700 |
---|---|---|
committer | Commit bot <commit-bot@chromium.org> | 2015-06-01 20:29:53 +0000 |
commit | 2e171cf4d960a6c723a529b2a4cbceb075bcd306 (patch) | |
tree | fa49c0a4c35465a257f2ab4c3dabf990151e98c7 /chrome | |
parent | a1caf96152e70ce6ccaebf3ad8fb770712e30b3d (diff) | |
download | chromium_src-2e171cf4d960a6c723a529b2a4cbceb075bcd306.zip chromium_src-2e171cf4d960a6c723a529b2a4cbceb075bcd306.tar.gz chromium_src-2e171cf4d960a6c723a529b2a4cbceb075bcd306.tar.bz2 |
Remove certificates from Channel ID
When creating a new Channel ID, an X.509 certificate is created, even though
only the keypair is used. This CL removes the generation and storage of
certificates for Channel ID and updates the callsites to use the modified
API that only exposes a crypto::ECPrivateKey.
BUG=457566
Review URL: https://codereview.chromium.org/1076063002
Cr-Commit-Position: refs/heads/master@{#332260}
Diffstat (limited to 'chrome')
7 files changed, 87 insertions, 112 deletions
diff --git a/chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc b/chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc index 13c1a78..ca5c8da 100644 --- a/chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc +++ b/chrome/browser/browsing_data/browsing_data_channel_id_helper_unittest.cc @@ -5,10 +5,12 @@ #include "chrome/browser/browsing_data/browsing_data_channel_id_helper.h" #include "base/bind.h" +#include "base/memory/scoped_ptr.h" #include "base/run_loop.h" #include "chrome/test/base/testing_profile.h" #include "content/public/browser/browser_thread.h" #include "content/public/test/test_browser_thread_bundle.h" +#include "crypto/ec_private_key.h" #include "net/ssl/channel_id_service.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_getter.h" @@ -38,12 +40,14 @@ class BrowsingDataChannelIDHelperTest testing_profile_->GetRequestContext()->GetURLRequestContext(); net::ChannelIDStore* channel_id_store = context->channel_id_service()->GetChannelIDStore(); - channel_id_store->SetChannelID("https://www.google.com:443", - base::Time(), base::Time(), - "key", "cert"); - channel_id_store->SetChannelID("https://www.youtube.com:443", - base::Time(), base::Time(), - "key", "cert"); + channel_id_store->SetChannelID( + make_scoped_ptr(new net::ChannelIDStore::ChannelID( + "https://www.google.com:443", base::Time(), + make_scoped_ptr(crypto::ECPrivateKey::Create())))); + channel_id_store->SetChannelID( + make_scoped_ptr(new net::ChannelIDStore::ChannelID( + "https://www.youtube.com:443", base::Time(), + make_scoped_ptr(crypto::ECPrivateKey::Create())))); } void FetchCallback( @@ -137,7 +141,7 @@ TEST_F(BrowsingDataChannelIDHelperTest, CannedEmpty) { ASSERT_TRUE(helper->empty()); helper->AddChannelID(net::ChannelIDStore::ChannelID( - origin, base::Time(), base::Time(), "key", "cert")); + origin, base::Time(), make_scoped_ptr(crypto::ECPrivateKey::Create()))); ASSERT_FALSE(helper->empty()); helper->Reset(); ASSERT_TRUE(helper->empty()); diff --git a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc index 35d2b98..0531d87 100644 --- a/chrome/browser/browsing_data/browsing_data_remover_unittest.cc +++ b/chrome/browser/browsing_data/browsing_data_remover_unittest.cc @@ -13,6 +13,7 @@ #include "base/files/file_path.h" #include "base/files/file_util.h" #include "base/guid.h" +#include "base/memory/scoped_ptr.h" #include "base/message_loop/message_loop.h" #include "base/prefs/testing_pref_service.h" #include "base/run_loop.h" @@ -332,29 +333,23 @@ class RemoveChannelIDTester : public net::SSLConfigService::Observer { ssl_config_service_->RemoveObserver(this); } - int ChannelIDCount() { - return channel_id_service_->cert_count(); - } + int ChannelIDCount() { return channel_id_service_->channel_id_count(); } // Add a server bound cert for |server| with specific creation and expiry // times. The cert and key data will be filled with dummy values. void AddChannelIDWithTimes(const std::string& server_identifier, - base::Time creation_time, - base::Time expiration_time) { - GetChannelIDStore()->SetChannelID(server_identifier, - creation_time, - expiration_time, - "a", - "b"); + base::Time creation_time) { + GetChannelIDStore()->SetChannelID( + make_scoped_ptr(new net::ChannelIDStore::ChannelID( + server_identifier, creation_time, + make_scoped_ptr(crypto::ECPrivateKey::Create())))); } // Add a server bound cert for |server|, with the current time as the // creation time. The cert and key data will be filled with dummy values. void AddChannelID(const std::string& server_identifier) { base::Time now = base::Time::Now(); - AddChannelIDWithTimes(server_identifier, - now, - now + base::TimeDelta::FromDays(1)); + AddChannelIDWithTimes(server_identifier, now); } void GetChannelIDList(net::ChannelIDStore::ChannelIDList* channel_ids) { @@ -1059,8 +1054,7 @@ TEST_F(BrowsingDataRemoverTest, RemoveChannelIDLastHour) { base::Time now = base::Time::Now(); tester.AddChannelID(kTestOrigin1); tester.AddChannelIDWithTimes(kTestOrigin2, - now - base::TimeDelta::FromHours(2), - now); + now - base::TimeDelta::FromHours(2)); EXPECT_EQ(0, tester.ssl_config_changed_count()); EXPECT_EQ(2, tester.ChannelIDCount()); diff --git a/chrome/browser/browsing_data/mock_browsing_data_channel_id_helper.cc b/chrome/browser/browsing_data/mock_browsing_data_channel_id_helper.cc index dd20a8c..b219008 100644 --- a/chrome/browser/browsing_data/mock_browsing_data_channel_id_helper.cc +++ b/chrome/browser/browsing_data/mock_browsing_data_channel_id_helper.cc @@ -30,9 +30,9 @@ void MockBrowsingDataChannelIDHelper::DeleteChannelID( void MockBrowsingDataChannelIDHelper::AddChannelIDSample( const std::string& server_id) { ASSERT_TRUE(channel_ids_.find(server_id) == channel_ids_.end()); + scoped_ptr<crypto::ECPrivateKey> key(crypto::ECPrivateKey::Create()); channel_id_list_.push_back( - net::ChannelIDStore::ChannelID( - server_id, base::Time(), base::Time(), "key", "cert")); + net::ChannelIDStore::ChannelID(server_id, base::Time(), key.Pass())); channel_ids_[server_id] = true; } diff --git a/chrome/browser/chromeos/login/profile_auth_data_unittest.cc b/chrome/browser/chromeos/login/profile_auth_data_unittest.cc index d2385fa..50fc4ca 100644 --- a/chrome/browser/chromeos/login/profile_auth_data_unittest.cc +++ b/chrome/browser/chromeos/login/profile_auth_data_unittest.cc @@ -27,6 +27,7 @@ #include "net/http/http_transaction_factory.h" #include "net/ssl/channel_id_service.h" #include "net/ssl/channel_id_store.h" +#include "net/test/channel_id_test_util.h" #include "net/url_request/url_request_context.h" #include "net/url_request/url_request_context_getter.h" #include "testing/gtest/include/gtest/gtest.h" @@ -51,10 +52,6 @@ const char kGAIACookieDomain[] = "google.com"; const char kSAMLIdPCookieDomain[] = "example.com"; const char kChannelIDServerIdentifier[] = "server"; -const char kChannelIDPrivateKey1[] = "private key 1"; -const char kChannelIDPrivateKey2[] = "private key 2"; -const char kChannelIDCert1[] = "cert 1"; -const char kChannelIDCert2[] = "cert 2"; } // namespace @@ -75,15 +72,17 @@ class ProfileAuthDataTest : public testing::Test { void VerifyTransferredUserProxyAuthEntry(); void VerifyUserCookies(const std::string& expected_gaia_cookie_value, const std::string& expected_saml_idp_cookie_value); - void VerifyUserChannelID(const std::string& expected_private_key, - const std::string& expected_cert); + void VerifyUserChannelID(crypto::ECPrivateKey* expected_key); + + protected: + scoped_ptr<crypto::ECPrivateKey> channel_id_key1_; + scoped_ptr<crypto::ECPrivateKey> channel_id_key2_; private: void PopulateBrowserContext(content::BrowserContext* browser_context, const std::string& proxy_auth_password, const std::string& cookie_value, - const std::string& channel_id_private_key, - const std::string& channel_id_cert); + scoped_ptr<crypto::ECPrivateKey> channel_id_key); net::URLRequestContext* GetRequestContext( content::BrowserContext* browser_context); @@ -108,19 +107,17 @@ class ProfileAuthDataTest : public testing::Test { }; void ProfileAuthDataTest::SetUp() { - PopulateBrowserContext(&login_browser_context_, - kProxyAuthPassword1, + channel_id_key1_.reset(crypto::ECPrivateKey::Create()); + channel_id_key2_.reset(crypto::ECPrivateKey::Create()); + PopulateBrowserContext(&login_browser_context_, kProxyAuthPassword1, kCookieValue1, - kChannelIDPrivateKey1, - kChannelIDCert1); + make_scoped_ptr(channel_id_key1_->Copy())); } void ProfileAuthDataTest::PopulateUserBrowserContext() { - PopulateBrowserContext(&user_browser_context_, - kProxyAuthPassword2, + PopulateBrowserContext(&user_browser_context_, kProxyAuthPassword2, kCookieValue2, - kChannelIDPrivateKey2, - kChannelIDCert2); + make_scoped_ptr(channel_id_key2_->Copy())); } void ProfileAuthDataTest::Transfer( @@ -190,22 +187,19 @@ void ProfileAuthDataTest::VerifyUserCookies( } void ProfileAuthDataTest::VerifyUserChannelID( - const std::string& expected_private_key, - const std::string& expected_cert) { + crypto::ECPrivateKey* expected_key) { net::ChannelIDStore::ChannelIDList user_channel_ids = GetUserChannelIDs(); ASSERT_EQ(1u, user_channel_ids.size()); net::ChannelIDStore::ChannelID* channel_id = &user_channel_ids.front(); EXPECT_EQ(kChannelIDServerIdentifier, channel_id->server_identifier()); - EXPECT_EQ(expected_private_key, channel_id->private_key()); - EXPECT_EQ(expected_cert, channel_id->cert()); + EXPECT_TRUE(net::KeysEqual(expected_key, channel_id->key())); } void ProfileAuthDataTest::PopulateBrowserContext( content::BrowserContext* browser_context, const std::string& proxy_auth_password, const std::string& cookie_value, - const std::string& channel_id_private_key, - const std::string& channel_id_cert) { + scoped_ptr<crypto::ECPrivateKey> channel_id_key) { GetProxyAuth(browser_context)->Add( GURL(kProxyAuthURL), kProxyAuthRealm, @@ -233,11 +227,9 @@ void ProfileAuthDataTest::PopulateBrowserContext( false, net::COOKIE_PRIORITY_DEFAULT)); cookies->ImportCookies(cookie_list); - GetChannelIDs(browser_context)->SetChannelID(kChannelIDServerIdentifier, - base::Time(), - base::Time(), - channel_id_private_key, - channel_id_cert); + GetChannelIDs(browser_context) + ->SetChannelID(make_scoped_ptr(new net::ChannelIDStore::ChannelID( + kChannelIDServerIdentifier, base::Time(), channel_id_key.Pass()))); } net::URLRequestContext* ProfileAuthDataTest::GetRequestContext( @@ -296,7 +288,7 @@ TEST_F(ProfileAuthDataTest, TransferOnFirstLoginWithNewProfile) { VerifyTransferredUserProxyAuthEntry(); VerifyUserCookies(kCookieValue1, kCookieValue1); - VerifyUserChannelID(kChannelIDPrivateKey1, kChannelIDCert1); + VerifyUserChannelID(channel_id_key1_.get()); } // Verifies that even if the transfer of auth cookies and channel IDs on first @@ -309,7 +301,7 @@ TEST_F(ProfileAuthDataTest, TransferOnFirstLoginWithExistingProfile) { VerifyTransferredUserProxyAuthEntry(); VerifyUserCookies(kCookieValue2, kCookieValue2); - VerifyUserChannelID(kChannelIDPrivateKey2, kChannelIDCert2); + VerifyUserChannelID(channel_id_key2_.get()); } // Verifies that when the transfer of auth cookies set by a SAML IdP on @@ -322,7 +314,7 @@ TEST_F(ProfileAuthDataTest, TransferOnSubsequentLogin) { VerifyTransferredUserProxyAuthEntry(); VerifyUserCookies(kCookieValue2, kCookieValue1); - VerifyUserChannelID(kChannelIDPrivateKey2, kChannelIDCert2); + VerifyUserChannelID(channel_id_key2_.get()); } } // namespace chromeos diff --git a/chrome/browser/extensions/api/messaging/message_property_provider.cc b/chrome/browser/extensions/api/messaging/message_property_provider.cc index a1eb7acf..1d6603b 100644 --- a/chrome/browser/extensions/api/messaging/message_property_provider.cc +++ b/chrome/browser/extensions/api/messaging/message_property_provider.cc @@ -11,6 +11,7 @@ #include "base/values.h" #include "chrome/browser/profiles/profile.h" #include "content/public/browser/browser_thread.h" +#include "crypto/ec_private_key.h" #include "extensions/common/api/runtime.h" #include "net/base/completion_callback.h" #include "net/cert/asn1_util.h" @@ -46,8 +47,7 @@ void MessagePropertyProvider::GetChannelID(Profile* profile, // ChannelIDService::GetChannelID to the callback provided to // MessagePropertyProvider::GetChannelID. struct MessagePropertyProvider::GetChannelIDOutput { - std::string domain_bound_private_key; - std::string domain_bound_cert; + scoped_ptr<crypto::ECPrivateKey> channel_id_key; net::ChannelIDService::RequestHandle request_handle; }; @@ -67,12 +67,9 @@ void MessagePropertyProvider::GetChannelIDOnIOThread( original_task_runner, base::Owned(output), reply); - int status = channel_id_service->GetChannelID( - host, - &output->domain_bound_private_key, - &output->domain_bound_cert, - net_completion_callback, - &output->request_handle); + int status = channel_id_service->GetChannelID(host, &output->channel_id_key, + net_completion_callback, + &output->request_handle); if (status == net::ERR_IO_PENDING) return; GotChannelID(original_task_runner, output, reply, status); @@ -89,11 +86,13 @@ void MessagePropertyProvider::GotChannelID( original_task_runner->PostTask(FROM_HERE, no_tls_channel_id_closure); return; } - base::StringPiece spki; - if (!net::asn1::ExtractSPKIFromDERCert(output->domain_bound_cert, &spki)) { + std::vector<uint8> spki_vector; + if (!output->channel_id_key->ExportPublicKey(&spki_vector)) { original_task_runner->PostTask(FROM_HERE, no_tls_channel_id_closure); return; } + base::StringPiece spki(reinterpret_cast<char*>(vector_as_array(&spki_vector)), + spki_vector.size()); base::DictionaryValue jwk_value; if (!net::JwkSerializer::ConvertSpkiFromDerToJwk(spki, &jwk_value)) { original_task_runner->PostTask(FROM_HERE, no_tls_channel_id_closure); diff --git a/chrome/browser/extensions/extension_messages_apitest.cc b/chrome/browser/extensions/extension_messages_apitest.cc index 9edda30..39c87265 100644 --- a/chrome/browser/extensions/extension_messages_apitest.cc +++ b/chrome/browser/extensions/extension_messages_apitest.cc @@ -973,24 +973,21 @@ class ExternallyConnectableMessagingWithTlsChannelIdTest : std::string CreateTlsChannelId() { scoped_refptr<net::URLRequestContextGetter> request_context_getter( profile()->GetRequestContext()); - std::string channel_id_private_key; - std::string channel_id_cert; - net::ChannelIDService::RequestHandle request_handle; + scoped_ptr<crypto::ECPrivateKey> channel_id_key; + net::ChannelIDService::RequestHandle request_handle; content::BrowserThread::PostTask( - content::BrowserThread::IO, - FROM_HERE, - base::Bind( - &ExternallyConnectableMessagingWithTlsChannelIdTest:: - CreateDomainBoundCertOnIOThread, - base::Unretained(this), - base::Unretained(&channel_id_private_key), - base::Unretained(&channel_id_cert), - base::Unretained(&request_handle), - request_context_getter)); + content::BrowserThread::IO, FROM_HERE, + base::Bind(&ExternallyConnectableMessagingWithTlsChannelIdTest:: + CreateDomainBoundCertOnIOThread, + base::Unretained(this), base::Unretained(&channel_id_key), + base::Unretained(&request_handle), request_context_getter)); tls_channel_id_created_.Wait(); // Create the expected value. - base::StringPiece spki; - net::asn1::ExtractSPKIFromDERCert(channel_id_cert, &spki); + std::vector<uint8> spki_vector; + if (!channel_id_key->ExportPublicKey(&spki_vector)) + return std::string(); + base::StringPiece spki(reinterpret_cast<char*>( + vector_as_array(&spki_vector)), spki_vector.size()); base::DictionaryValue jwk_value; net::JwkSerializer::ConvertSpkiFromDerToJwk(spki, &jwk_value); std::string tls_channel_id_value; @@ -1000,8 +997,7 @@ class ExternallyConnectableMessagingWithTlsChannelIdTest : private: void CreateDomainBoundCertOnIOThread( - std::string* channel_id_private_key, - std::string* channel_id_cert, + scoped_ptr<crypto::ECPrivateKey>* channel_id_key, net::ChannelIDService::RequestHandle* request_handle, scoped_refptr<net::URLRequestContextGetter> request_context_getter) { DCHECK_CURRENTLY_ON(content::BrowserThread::IO); @@ -1009,11 +1005,9 @@ class ExternallyConnectableMessagingWithTlsChannelIdTest : request_context_getter->GetURLRequestContext()-> channel_id_service(); int status = channel_id_service->GetOrCreateChannelID( - chromium_org_url().host(), - channel_id_private_key, - channel_id_cert, + chromium_org_url().host(), channel_id_key, base::Bind(&ExternallyConnectableMessagingWithTlsChannelIdTest:: - GotDomainBoundCert, + GotDomainBoundCert, base::Unretained(this)), request_handle); if (status == net::ERR_IO_PENDING) diff --git a/chrome/browser/net/quota_policy_channel_id_store_unittest.cc b/chrome/browser/net/quota_policy_channel_id_store_unittest.cc index dda262e..670bf88 100644 --- a/chrome/browser/net/quota_policy_channel_id_store_unittest.cc +++ b/chrome/browser/net/quota_policy_channel_id_store_unittest.cc @@ -20,6 +20,7 @@ #include "net/cookies/cookie_util.h" #include "net/ssl/ssl_client_cert_type.h" #include "net/test/cert_test_util.h" +#include "net/test/channel_id_test_util.h" #include "sql/statement.h" #include "testing/gtest/include/gtest/gtest.h" @@ -55,13 +56,6 @@ class QuotaPolicyChannelIDStoreTest : public testing::Test { ScopedVector<net::DefaultChannelIDStore::ChannelID> channel_ids; Load(&channel_ids); ASSERT_EQ(0u, channel_ids.size()); - // Make sure the store gets written at least once. - store_->AddChannelID( - net::DefaultChannelIDStore::ChannelID("google.com", - base::Time::FromInternalValue(1), - base::Time::FromInternalValue(2), - "a", - "b")); } void TearDown() override { @@ -77,12 +71,14 @@ class QuotaPolicyChannelIDStoreTest : public testing::Test { // Test if data is stored as expected in the QuotaPolicy database. TEST_F(QuotaPolicyChannelIDStoreTest, TestPersistence) { - store_->AddChannelID( - net::DefaultChannelIDStore::ChannelID("foo.com", - base::Time::FromInternalValue(3), - base::Time::FromInternalValue(4), - "c", - "d")); + scoped_ptr<crypto::ECPrivateKey> goog_key(crypto::ECPrivateKey::Create()); + scoped_ptr<crypto::ECPrivateKey> foo_key(crypto::ECPrivateKey::Create()); + store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( + "google.com", base::Time::FromInternalValue(1), + make_scoped_ptr(goog_key->Copy()))); + store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( + "foo.com", base::Time::FromInternalValue(3), + make_scoped_ptr(foo_key->Copy()))); ScopedVector<net::DefaultChannelIDStore::ChannelID> channel_ids; // Replace the store effectively destroying the current one and forcing it @@ -109,15 +105,11 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPersistence) { foo_channel_id = channel_ids[0]; } ASSERT_EQ("google.com", goog_channel_id->server_identifier()); - ASSERT_STREQ("a", goog_channel_id->private_key().c_str()); - ASSERT_STREQ("b", goog_channel_id->cert().c_str()); + EXPECT_TRUE(net::KeysEqual(goog_key.get(), goog_channel_id->key())); ASSERT_EQ(1, goog_channel_id->creation_time().ToInternalValue()); - ASSERT_EQ(2, goog_channel_id->expiration_time().ToInternalValue()); ASSERT_EQ("foo.com", foo_channel_id->server_identifier()); - ASSERT_STREQ("c", foo_channel_id->private_key().c_str()); - ASSERT_STREQ("d", foo_channel_id->cert().c_str()); + EXPECT_TRUE(net::KeysEqual(foo_key.get(), foo_channel_id->key())); ASSERT_EQ(3, foo_channel_id->creation_time().ToInternalValue()); - ASSERT_EQ(4, foo_channel_id->expiration_time().ToInternalValue()); // Now delete the channel ID and check persistence again. store_->DeleteChannelID(*channel_ids[0]); @@ -138,12 +130,12 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPersistence) { // Test if data is stored as expected in the QuotaPolicy database. TEST_F(QuotaPolicyChannelIDStoreTest, TestPolicy) { - store_->AddChannelID( - net::DefaultChannelIDStore::ChannelID("nonpersistent.com", - base::Time::FromInternalValue(3), - base::Time::FromInternalValue(4), - "c", - "d")); + store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( + "google.com", base::Time::FromInternalValue(1), + make_scoped_ptr(crypto::ECPrivateKey::Create()))); + store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( + "nonpersistent.com", base::Time::FromInternalValue(3), + make_scoped_ptr(crypto::ECPrivateKey::Create()))); ScopedVector<net::DefaultChannelIDStore::ChannelID> channel_ids; // Replace the store effectively destroying the current one and forcing it @@ -172,10 +164,10 @@ TEST_F(QuotaPolicyChannelIDStoreTest, TestPolicy) { // being committed to disk. store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( "nonpersistent.com", base::Time::FromInternalValue(5), - base::Time::FromInternalValue(6), "e", "f")); + make_scoped_ptr(crypto::ECPrivateKey::Create()))); store_->AddChannelID(net::DefaultChannelIDStore::ChannelID( "persistent.com", base::Time::FromInternalValue(7), - base::Time::FromInternalValue(8), "g", "h")); + make_scoped_ptr(crypto::ECPrivateKey::Create()))); // Now close the store, and the nonpersistent.com channel IDs should be // deleted according to policy. |