Skip to content

Commit

Permalink
Encrypt all stored cookies on selected operating systems.
Browse files Browse the repository at this point in the history
As part of the goal of protecting private user information, this encrypts the cookie values on operating systems with user-specific crypto APIs and that do not otherwise protect this data.

Performance tests indicate a penalty of about 1ms per cookie  (regardless of size) on a Mac and 0.1ms to 0.7ms (depending on the size) under Windows.  This will be higher on older hardware but still insignificant.

Encrypted data is binary (with an overhead of 128 bytes on Windows) and binary data must be stored in a BLOB so only one of two fields ("value" or "encrypted_value") will have data with the other being empty.  Both values, however, need to be read & written when accessing a cookie because they are marked "non null").

BUG=313323

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234855

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@240684 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
bcwhite@chromium.org committed Dec 13, 2013
1 parent 4c5d7c9 commit d0d400e
Show file tree
Hide file tree
Showing 9 changed files with 320 additions and 68 deletions.
4 changes: 3 additions & 1 deletion android_webview/native/cookie_manager.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "base/threading/thread_restrictions.h"
#include "content/public/browser/browser_context.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_crypto_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/common/url_constants.h"
#include "jni/AwCookieManager_jni.h"
Expand Down Expand Up @@ -190,7 +191,8 @@ void CookieManager::CreateCookieMonster(
NULL,
NULL,
client_task_runner,
background_task_runner);
background_task_runner,
scoped_ptr<content::CookieCryptoDelegate>());
cookie_monster_ = cookie_store->GetCookieMonster();
cookie_monster_->SetPersistSessionCookies(true);
SetAcceptFileSchemeCookiesLocked(kDefaultFileSchemeAllowed);
Expand Down
47 changes: 44 additions & 3 deletions chrome/browser/profiles/profile_impl_io_data.cc
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@
#include "chrome/common/chrome_switches.h"
#include "chrome/common/pref_names.h"
#include "chrome/common/url_constants.h"
#include "components/webdata/encryptor/encryptor.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_crypto_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/browser/notification_service.h"
#include "content/public/browser/resource_context.h"
Expand All @@ -52,6 +54,42 @@

namespace {

// Use the operating system's mechanisms to encrypt cookies before writing
// them to persistent store. Currently this only is done with desktop OS's
// because ChromeOS and Android already protect the entire profile contents.
//
// TODO(bcwhite): Enable on MACOSX -- requires all Cookie tests to call
// Encryptor::UseMockKeychain or will hang waiting for user input.
#if defined(OS_WIN) || defined(OS_LINUX) // || defined(OS_MACOSX)
class CookieOSCryptoDelegate : public content::CookieCryptoDelegate {
public:
virtual bool EncryptString(const std::string& plaintext,
std::string* ciphertext) OVERRIDE;
virtual bool DecryptString(const std::string& ciphertext,
std::string* plaintext) OVERRIDE;
};

bool CookieOSCryptoDelegate::EncryptString(const std::string& plaintext,
std::string* ciphertext) {
return Encryptor::EncryptString(plaintext, ciphertext);
}

bool CookieOSCryptoDelegate::DecryptString(const std::string& ciphertext,
std::string* plaintext) {
return Encryptor::DecryptString(ciphertext, plaintext);
}

scoped_ptr<content::CookieCryptoDelegate> CreateCookieCryptoIfUseful() {
return scoped_ptr<content::CookieCryptoDelegate>(
new CookieOSCryptoDelegate);
}
#else
scoped_ptr<content::CookieCryptoDelegate> CreateCookieCryptoIfUseful() {
return scoped_ptr<content::CookieCryptoDelegate>();
}
#endif


net::BackendType ChooseCacheBackendType() {
const CommandLine& command_line = *CommandLine::ForCurrentProcess();
if (command_line.HasSwitch(switches::kUseSimpleCacheBackend)) {
Expand Down Expand Up @@ -404,7 +442,8 @@ void ProfileImplIOData::InitializeInternal(
lazy_params_->cookie_path,
lazy_params_->restore_old_session_cookies,
lazy_params_->special_storage_policy.get(),
profile_params->cookie_monster_delegate.get());
profile_params->cookie_monster_delegate.get(),
CreateCookieCryptoIfUseful());
cookie_store->GetCookieMonster()->SetPersistSessionCookies(true);
}

Expand Down Expand Up @@ -502,7 +541,8 @@ void ProfileImplIOData::
lazy_params_->extensions_cookie_path,
lazy_params_->restore_old_session_cookies,
NULL,
NULL);
NULL,
CreateCookieCryptoIfUseful());
// Enable cookies for devtools and extension URLs.
const char* schemes[] = {chrome::kChromeDevToolsScheme,
extensions::kExtensionScheme};
Expand Down Expand Up @@ -588,7 +628,8 @@ ProfileImplIOData::InitializeAppRequestContext(
cookie_path,
false,
NULL,
NULL);
NULL,
CreateCookieCryptoIfUseful());
}

// Transfer ownership of the cookies and cache to AppRequestContext.
Expand Down
4 changes: 3 additions & 1 deletion chrome/browser/safe_browsing/safe_browsing_service.cc
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
#include "chrome/common/url_constants.h"
#include "components/startup_metric_utils/startup_metric_utils.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_crypto_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "content/public/browser/notification_service.h"
#include "net/cookies/cookie_monster.h"
Expand Down Expand Up @@ -309,7 +310,8 @@ void SafeBrowsingService::InitURLRequestContextOnIOThread(
CookieFilePath(),
false,
NULL,
NULL));
NULL,
scoped_ptr<content::CookieCryptoDelegate>()));

url_request_context_.reset(new net::URLRequestContext);
// |system_url_request_context_getter| may be NULL during tests.
Expand Down
129 changes: 94 additions & 35 deletions content/browser/net/sqlite_persistent_cookie_store.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
#include "base/threading/sequenced_worker_pool.h"
#include "base/time/time.h"
#include "content/public/browser/browser_thread.h"
#include "content/public/browser/cookie_crypto_delegate.h"
#include "content/public/browser/cookie_store_factory.h"
#include "net/base/registry_controlled_domains/registry_controlled_domain.h"
#include "net/cookies/canonical_cookie.h"
Expand Down Expand Up @@ -73,7 +74,8 @@ class SQLitePersistentCookieStore::Backend
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
bool restore_old_session_cookies,
quota::SpecialStoragePolicy* special_storage_policy)
quota::SpecialStoragePolicy* special_storage_policy,
scoped_ptr<CookieCryptoDelegate> crypto_delegate)
: path_(path),
num_pending_(0),
force_keep_session_state_(false),
Expand All @@ -85,7 +87,8 @@ class SQLitePersistentCookieStore::Backend
client_task_runner_(client_task_runner),
background_task_runner_(background_task_runner),
num_priority_waiting_(0),
total_priority_requests_(0) {}
total_priority_requests_(0),
crypto_(crypto_delegate.Pass()) {}

// Creates or loads the SQLite database.
void Load(const LoadedCallback& loaded_callback);
Expand Down Expand Up @@ -268,6 +271,9 @@ class SQLitePersistentCookieStore::Backend
// The cumulative duration of time when |num_priority_waiting_| was greater
// than 1.
base::TimeDelta priority_wait_duration_;
// Class with functions that do cryptographic operations (for protecting
// cookies stored persistently).
scoped_ptr<CookieCryptoDelegate> crypto_;

DISALLOW_COPY_AND_ASSIGN(Backend);
};
Expand All @@ -276,6 +282,13 @@ namespace {

// Version number of the database.
//
// Version 7 adds encrypted values. Old values will continue to be used but
// all new values written will be encrypted on selected operating systems. New
// records read by old clients will simply get an empty cookie value while old
// records read by new clients will continue to operate with the unencrypted
// version. New and old clients alike will always write/update records with
// what they support.
//
// Version 6 adds cookie priorities. This allows developers to influence the
// order in which cookies are evicted in order to meet domain cookie limits.
//
Expand All @@ -292,7 +305,7 @@ namespace {
// Version 3 updated the database to include the last access time, so we can
// expire them in decreasing order of use when we've reached the maximum
// number of cookies.
const int kCurrentVersionNumber = 6;
const int kCurrentVersionNumber = 7;
const int kCompatibleVersionNumber = 5;

// Possible values for the 'priority' column.
Expand Down Expand Up @@ -369,7 +382,8 @@ bool InitTable(sql::Connection* db) {
"last_access_utc INTEGER NOT NULL, "
"has_expires INTEGER NOT NULL DEFAULT 1, "
"persistent INTEGER NOT NULL DEFAULT 1,"
"priority INTEGER NOT NULL DEFAULT %d)",
"priority INTEGER NOT NULL DEFAULT %d,"
"encrypted_value BLOB DEFAULT '')",
CookiePriorityToDBCookiePriority(net::COOKIE_PRIORITY_DEFAULT)));
if (!db->Execute(stmt.c_str()))
return false;
Expand Down Expand Up @@ -678,15 +692,16 @@ bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(
if (restore_old_session_cookies_) {
smt.Assign(db_->GetCachedStatement(
SQL_FROM_HERE,
"SELECT creation_utc, host_key, name, value, path, expires_utc, "
"secure, httponly, last_access_utc, has_expires, persistent, priority "
"FROM cookies WHERE host_key = ?"));
"SELECT creation_utc, host_key, name, value, encrypted_value, path, "
"expires_utc, secure, httponly, last_access_utc, has_expires, "
"persistent, priority FROM cookies WHERE host_key = ?"));
} else {
smt.Assign(db_->GetCachedStatement(
SQL_FROM_HERE,
"SELECT creation_utc, host_key, name, value, path, expires_utc, "
"secure, httponly, last_access_utc, has_expires, persistent, priority "
"FROM cookies WHERE host_key = ? AND persistent = 1"));
"SELECT creation_utc, host_key, name, value, encrypted_value, path, "
"expires_utc, secure, httponly, last_access_utc, has_expires, "
"persistent, priority FROM cookies WHERE host_key = ? "
"AND persistent = 1"));
}
if (!smt.is_valid()) {
smt.Clear(); // Disconnect smt_ref from db_.
Expand All @@ -700,20 +715,28 @@ bool SQLitePersistentCookieStore::Backend::LoadCookiesForDomains(
for (; it != domains.end(); ++it) {
smt.BindString(0, *it);
while (smt.Step()) {
std::string value;
std::string encrypted_value = smt.ColumnString(4);
if (!encrypted_value.empty() && crypto_.get()) {
crypto_->DecryptString(encrypted_value, &value);
} else {
DCHECK(encrypted_value.empty());
value = smt.ColumnString(3);
}
scoped_ptr<net::CanonicalCookie> cc(new net::CanonicalCookie(
// The "source" URL is not used with persisted cookies.
GURL(), // Source
smt.ColumnString(2), // name
smt.ColumnString(3), // value
value, // value
smt.ColumnString(1), // domain
smt.ColumnString(4), // path
smt.ColumnString(5), // path
Time::FromInternalValue(smt.ColumnInt64(0)), // creation_utc
Time::FromInternalValue(smt.ColumnInt64(5)), // expires_utc
Time::FromInternalValue(smt.ColumnInt64(8)), // last_access_utc
smt.ColumnInt(6) != 0, // secure
smt.ColumnInt(7) != 0, // httponly
Time::FromInternalValue(smt.ColumnInt64(6)), // expires_utc
Time::FromInternalValue(smt.ColumnInt64(9)), // last_access_utc
smt.ColumnInt(7) != 0, // secure
smt.ColumnInt(8) != 0, // httponly
DBCookiePriorityToCookiePriority(
static_cast<DBCookiePriority>(smt.ColumnInt(11))))); // priority
static_cast<DBCookiePriority>(smt.ColumnInt(12))))); // priority
DLOG_IF(WARNING,
cc->CreationDate() > Time::Now()) << L"CreationDate too recent";
cookies_per_origin_[CookieOrigin(cc->Domain(), cc->IsSecure())]++;
Expand Down Expand Up @@ -836,6 +859,26 @@ bool SQLitePersistentCookieStore::Backend::EnsureDatabaseVersion() {
base::TimeTicks::Now() - start_time);
}

if (cur_version == 6) {
const base::TimeTicks start_time = base::TimeTicks::Now();
sql::Transaction transaction(db_.get());
if (!transaction.Begin())
return false;
// Alter the table to add empty "encrypted value" column.
if (!db_->Execute("ALTER TABLE cookies "
"ADD COLUMN encrypted_value BLOB DEFAULT ''")) {
LOG(WARNING) << "Unable to update cookie database to version 7.";
return false;
}
++cur_version;
meta_table_.SetVersionNumber(cur_version);
meta_table_.SetCompatibleVersionNumber(
std::min(cur_version, kCompatibleVersionNumber));
transaction.Commit();
UMA_HISTOGRAM_TIMES("Cookie.TimeDatabaseMigrationToV7",
base::TimeTicks::Now() - start_time);
}

// Put future migration cases here.

if (cur_version < kCurrentVersionNumber) {
Expand Down Expand Up @@ -920,10 +963,10 @@ void SQLitePersistentCookieStore::Backend::Commit() {
return;

sql::Statement add_smt(db_->GetCachedStatement(SQL_FROM_HERE,
"INSERT INTO cookies (creation_utc, host_key, name, value, path, "
"expires_utc, secure, httponly, last_access_utc, has_expires, "
"persistent, priority) "
"VALUES (?,?,?,?,?,?,?,?,?,?,?,?)"));
"INSERT INTO cookies (creation_utc, host_key, name, value, "
"encrypted_value, path, expires_utc, secure, httponly, last_access_utc, "
"has_expires, persistent, priority) "
"VALUES (?,?,?,?,?,?,?,?,?,?,?,?,?)"));
if (!add_smt.is_valid())
return;

Expand Down Expand Up @@ -953,16 +996,26 @@ void SQLitePersistentCookieStore::Backend::Commit() {
add_smt.BindInt64(0, po->cc().CreationDate().ToInternalValue());
add_smt.BindString(1, po->cc().Domain());
add_smt.BindString(2, po->cc().Name());
add_smt.BindString(3, po->cc().Value());
add_smt.BindString(4, po->cc().Path());
add_smt.BindInt64(5, po->cc().ExpiryDate().ToInternalValue());
add_smt.BindInt(6, po->cc().IsSecure());
add_smt.BindInt(7, po->cc().IsHttpOnly());
add_smt.BindInt64(8, po->cc().LastAccessDate().ToInternalValue());
add_smt.BindInt(9, po->cc().IsPersistent());
if (crypto_.get()) {
std::string encrypted_value;
add_smt.BindCString(3, ""); // value
crypto_->EncryptString(po->cc().Value(), &encrypted_value);
// BindBlob() immediately makes an internal copy of the data.
add_smt.BindBlob(4, encrypted_value.data(),
static_cast<int>(encrypted_value.length()));
} else {
add_smt.BindString(3, po->cc().Value());
add_smt.BindBlob(4, "", 0); // encrypted_value
}
add_smt.BindString(5, po->cc().Path());
add_smt.BindInt64(6, po->cc().ExpiryDate().ToInternalValue());
add_smt.BindInt(7, po->cc().IsSecure());
add_smt.BindInt(8, po->cc().IsHttpOnly());
add_smt.BindInt64(9, po->cc().LastAccessDate().ToInternalValue());
add_smt.BindInt(10, po->cc().IsPersistent());
add_smt.BindInt(11, po->cc().IsPersistent());
add_smt.BindInt(
11, CookiePriorityToDBCookiePriority(po->cc().Priority()));
12, CookiePriorityToDBCookiePriority(po->cc().Priority()));
if (!add_smt.Run())
NOTREACHED() << "Could not add a cookie to the DB.";
break;
Expand Down Expand Up @@ -1148,12 +1201,14 @@ SQLitePersistentCookieStore::SQLitePersistentCookieStore(
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
bool restore_old_session_cookies,
quota::SpecialStoragePolicy* special_storage_policy)
quota::SpecialStoragePolicy* special_storage_policy,
scoped_ptr<CookieCryptoDelegate> crypto_delegate)
: backend_(new Backend(path,
client_task_runner,
background_task_runner,
restore_old_session_cookies,
special_storage_policy)) {
special_storage_policy,
crypto_delegate.Pass())) {
}

void SQLitePersistentCookieStore::Load(const LoadedCallback& loaded_callback) {
Expand Down Expand Up @@ -1199,30 +1254,34 @@ net::CookieStore* CreatePersistentCookieStore(
quota::SpecialStoragePolicy* storage_policy,
net::CookieMonster::Delegate* cookie_monster_delegate,
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner) {
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
scoped_ptr<CookieCryptoDelegate> crypto_delegate) {
SQLitePersistentCookieStore* persistent_store =
new SQLitePersistentCookieStore(
path,
client_task_runner,
background_task_runner,
restore_old_session_cookies,
storage_policy);
storage_policy,
crypto_delegate.Pass());
return new net::CookieMonster(persistent_store, cookie_monster_delegate);
}

net::CookieStore* CreatePersistentCookieStore(
const base::FilePath& path,
bool restore_old_session_cookies,
quota::SpecialStoragePolicy* storage_policy,
net::CookieMonster::Delegate* cookie_monster_delegate) {
net::CookieMonster::Delegate* cookie_monster_delegate,
scoped_ptr<CookieCryptoDelegate> crypto_delegate) {
return CreatePersistentCookieStore(
path,
restore_old_session_cookies,
storage_policy,
cookie_monster_delegate,
BrowserThread::GetMessageLoopProxyForThread(BrowserThread::IO),
BrowserThread::GetBlockingPool()->GetSequencedTaskRunner(
BrowserThread::GetBlockingPool()->GetSequenceToken()));
BrowserThread::GetBlockingPool()->GetSequenceToken()),
crypto_delegate.Pass());
}

} // namespace content
4 changes: 3 additions & 1 deletion content/browser/net/sqlite_persistent_cookie_store.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ class SpecialStoragePolicy;
}

namespace content {
class CookieCryptoDelegate;

// Implements the PersistentCookieStore interface in terms of a SQLite database.
// For documentation about the actual member functions consult the documentation
Expand All @@ -49,7 +50,8 @@ class CONTENT_EXPORT SQLitePersistentCookieStore
const scoped_refptr<base::SequencedTaskRunner>& client_task_runner,
const scoped_refptr<base::SequencedTaskRunner>& background_task_runner,
bool restore_old_session_cookies,
quota::SpecialStoragePolicy* special_storage_policy);
quota::SpecialStoragePolicy* special_storage_policy,
scoped_ptr<CookieCryptoDelegate> crypto_delegate);

// net::CookieMonster::PersistentCookieStore:
virtual void Load(const LoadedCallback& loaded_callback) OVERRIDE;
Expand Down
Loading

0 comments on commit d0d400e

Please sign in to comment.