summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authormek@chromium.org <mek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-10-09 19:12:33 +0000
committermek@chromium.org <mek@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2013-10-09 19:12:33 +0000
commitf872276f5fa8a98097199ea61a188e97bf5de800 (patch)
tree2931809cc18d67efd85aa6da5fa05a1e492441b5
parent68126b10f0faa3e5ff8463eecb5b76c4848a77a4 (diff)
downloadchromium_src-f872276f5fa8a98097199ea61a188e97bf5de800.zip
chromium_src-f872276f5fa8a98097199ea61a188e97bf5de800.tar.gz
chromium_src-f872276f5fa8a98097199ea61a188e97bf5de800.tar.bz2
Revert 227772 "Clears the expired identities from the WebRTC ide..."
This fails on ASAN tests: http://build.chromium.org/p/chromium.memory/builders/Linux%20ASAN%20Tests%20%282%29/builds/18341/steps/content_unittests/logs/stdio ==23976==ERROR: AddressSanitizer: heap-use-after-free on address 0x60b00002ae68 at pc 0x2281510 bp 0x7fff24edac90 sp 0x7fff24edac88 READ of size 8 at 0x60b00002ae68 thread T0 #0 0x228150f in content::WebRTCIdentityStoreBackend::FindIdentity(GURL const&, std::string const&, std::string const&, base::Callback<void (int, std::string const&, std::string const&)> const&) content/browser/media/webrtc_identity_store_backend.cc:225 #1 0x228340f in content::WebRTCIdentityStoreBackend::OnLoaded(scoped_ptr<std::map<content::WebRTCIdentityStoreBackend::IdentityKey, content::WebRTCIdentityStoreBackend::Identity, std::less<content::WebRTCIdentityStoreBackend::IdentityKey>, std::allocator<std::pair<content::WebRTCIdentityStoreBackend::IdentityKey const, content::WebRTCIdentityStoreBackend::Identity> > >, base::DefaultDeleter<std::map<content::WebRTCIdentityStoreBackend::IdentityKey, content::WebRTCIdentityStoreBackend::Identity, std::less<content::WebRTCIdentityStoreBackend::IdentityKey>, std::allocator<std::pair<content::WebRTCIdentityStoreBackend::IdentityKey const, content::WebRTCIdentityStoreBackend::Identity> > > > >) content/browser/media/webrtc_identity_store_backend.cc:353 #2 0x2289f53 in Run base/bind_internal.h:190 .... > Clears the expired identities from the WebRTC identity cache. > We now check the creation time of an identity before returning for WebRTCIdentityStoreBackend::FindIdentity and remove the expired identity from the in-memory cache. > The expired identities are also removed from the on-disk database before it's loaded to the in-memory cache. > The identities have 30-day expiration time by default for now. > > BUG=304398 > > Review URL: https://codereview.chromium.org/26082002 TBR=jiayl@chromium.org Review URL: https://codereview.chromium.org/26567009 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@227776 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat
-rw-r--r--content/browser/media/webrtc_identity_store.cc46
-rw-r--r--content/browser/media/webrtc_identity_store.h5
-rw-r--r--content/browser/media/webrtc_identity_store_backend.cc140
-rw-r--r--content/browser/media/webrtc_identity_store_backend.h13
-rw-r--r--content/browser/media/webrtc_identity_store_unittest.cc77
5 files changed, 72 insertions, 209 deletions
diff --git a/content/browser/media/webrtc_identity_store.cc b/content/browser/media/webrtc_identity_store.cc
index 5e93b09..67a2585 100644
--- a/content/browser/media/webrtc_identity_store.cc
+++ b/content/browser/media/webrtc_identity_store.cc
@@ -31,10 +31,9 @@ struct WebRTCIdentityRequestResult {
std::string private_key;
};
-// Generates a new identity using |common_name| which expires after
-// |validity_period| and returns the result in |result|.
+// Generates a new identity using |common_name| and returns the result in
+// |result|.
static void GenerateIdentityWorker(const std::string& common_name,
- base::TimeDelta validity_period,
WebRTCIdentityRequestResult* result) {
result->error = net::OK;
int serial_number = base::RandInt(0, std::numeric_limits<int>::max());
@@ -47,12 +46,13 @@ static void GenerateIdentityWorker(const std::string& common_name,
}
base::Time now = base::Time::Now();
- bool success = net::x509_util::CreateSelfSignedCert(key.get(),
- "CN=" + common_name,
- serial_number,
- now,
- now + validity_period,
- &result->certificate);
+ bool success =
+ net::x509_util::CreateSelfSignedCert(key.get(),
+ "CN=" + common_name,
+ serial_number,
+ now,
+ now + base::TimeDelta::FromDays(30),
+ &result->certificate);
if (!success) {
DLOG(ERROR) << "Unable to create x509 cert for client";
result->error = net::ERR_SELF_SIGNED_CERT_GENERATION_FAILED;
@@ -176,10 +176,8 @@ class WebRTCIdentityRequestHandle {
WebRTCIdentityStore::WebRTCIdentityStore(const base::FilePath& path,
quota::SpecialStoragePolicy* policy)
- : validity_period_(base::TimeDelta::FromDays(30)),
- task_runner_(base::WorkerPool::GetTaskRunner(true)),
- backend_(new WebRTCIdentityStoreBackend(path, policy, validity_period_)) {
- }
+ : task_runner_(base::WorkerPool::GetTaskRunner(true)),
+ backend_(new WebRTCIdentityStoreBackend(path, policy)) {}
WebRTCIdentityStore::~WebRTCIdentityStore() { backend_->Close(); }
@@ -228,13 +226,6 @@ void WebRTCIdentityStore::DeleteBetween(base::Time delete_begin,
backend_->DeleteBetween(delete_begin, delete_end, callback);
}
-void WebRTCIdentityStore::SetValidityPeriodForTesting(
- base::TimeDelta validity_period) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- validity_period_ = validity_period;
- backend_->SetValidityPeriodForTesting(validity_period);
-}
-
void WebRTCIdentityStore::SetTaskRunnerForTesting(
const scoped_refptr<base::TaskRunner>& task_runner) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
@@ -256,15 +247,12 @@ void WebRTCIdentityStore::BackendFindCallback(WebRTCIdentityRequest* request,
WebRTCIdentityRequestResult* result =
new WebRTCIdentityRequestResult(0, "", "");
if (!task_runner_->PostTaskAndReply(
- FROM_HERE,
- base::Bind(&GenerateIdentityWorker,
- request->common_name_,
- validity_period_,
- result),
- base::Bind(&WebRTCIdentityStore::GenerateIdentityCallback,
- this,
- request,
- base::Owned(result)))) {
+ FROM_HERE,
+ base::Bind(&GenerateIdentityWorker, request->common_name_, result),
+ base::Bind(&WebRTCIdentityStore::GenerateIdentityCallback,
+ this,
+ request,
+ base::Owned(result)))) {
// Completes the request with error if failed to post the task.
WebRTCIdentityRequestResult result(net::ERR_UNEXPECTED, "", "");
PostRequestResult(request, result);
diff --git a/content/browser/media/webrtc_identity_store.h b/content/browser/media/webrtc_identity_store.h
index 3602b70..c2b523e 100644
--- a/content/browser/media/webrtc_identity_store.h
+++ b/content/browser/media/webrtc_identity_store.h
@@ -83,7 +83,6 @@ class CONTENT_EXPORT WebRTCIdentityStore
friend class base::RefCountedThreadSafe<WebRTCIdentityStore>;
friend class WebRTCIdentityStoreTest;
- void SetValidityPeriodForTesting(base::TimeDelta validity_period);
void SetTaskRunnerForTesting(
const scoped_refptr<base::TaskRunner>& task_runner);
@@ -99,12 +98,8 @@ class CONTENT_EXPORT WebRTCIdentityStore
void PostRequestResult(WebRTCIdentityRequest* request,
const WebRTCIdentityRequestResult& result);
- // The validity period of the certificates.
- base::TimeDelta validity_period_;
-
// The TaskRunner for doing work on a worker thread.
scoped_refptr<base::TaskRunner> task_runner_;
-
// Weak references of the in flight requests. Used to join identical external
// requests.
std::vector<WebRTCIdentityRequest*> in_flight_requests_;
diff --git a/content/browser/media/webrtc_identity_store_backend.cc b/content/browser/media/webrtc_identity_store_backend.cc
index 641e335..9ec73e9 100644
--- a/content/browser/media/webrtc_identity_store_backend.cc
+++ b/content/browser/media/webrtc_identity_store_backend.cc
@@ -6,7 +6,6 @@
#include "base/file_util.h"
#include "base/files/file_path.h"
-#include "base/timer/timer.h"
#include "content/public/browser/browser_thread.h"
#include "net/base/net_errors.h"
#include "sql/error_delegate_util.h"
@@ -108,10 +107,9 @@ struct WebRTCIdentityStoreBackend::PendingFindRequest {
class WebRTCIdentityStoreBackend::SqlLiteStorage
: public base::RefCountedThreadSafe<SqlLiteStorage> {
public:
- SqlLiteStorage(base::TimeDelta validity_period,
- const base::FilePath& path,
+ SqlLiteStorage(const base::FilePath& path,
quota::SpecialStoragePolicy* policy)
- : validity_period_(validity_period), special_storage_policy_(policy) {
+ : special_storage_policy_(policy) {
if (!path.empty())
path_ = path.Append(kWebRTCIdentityStoreDirectory);
}
@@ -124,13 +122,9 @@ class WebRTCIdentityStoreBackend::SqlLiteStorage
void DeleteIdentity(const GURL& origin,
const std::string& identity_name,
const Identity& identity);
- void DeleteBetween(base::Time delete_begin, base::Time delete_end);
-
- void SetValidityPeriodForTesting(base::TimeDelta validity_period) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
- DCHECK(!db_.get());
- validity_period_ = validity_period;
- }
+ void DeleteBetween(base::Time delete_begin,
+ base::Time delete_end,
+ const base::Closure& callback);
private:
friend class base::RefCountedThreadSafe<SqlLiteStorage>;
@@ -164,7 +158,6 @@ class WebRTCIdentityStoreBackend::SqlLiteStorage
const Identity& identity);
void Commit();
- base::TimeDelta validity_period_;
// The file path of the DB. Empty if temporary.
base::FilePath path_;
scoped_refptr<quota::SpecialStoragePolicy> special_storage_policy_;
@@ -177,11 +170,9 @@ class WebRTCIdentityStoreBackend::SqlLiteStorage
WebRTCIdentityStoreBackend::WebRTCIdentityStoreBackend(
const base::FilePath& path,
- quota::SpecialStoragePolicy* policy,
- base::TimeDelta validity_period)
- : validity_period_(validity_period),
- state_(NOT_STARTED),
- sql_lite_storage_(new SqlLiteStorage(validity_period, path, policy)) {}
+ quota::SpecialStoragePolicy* policy)
+ : state_(NOT_STARTED),
+ sql_lite_storage_(new SqlLiteStorage(path, policy)) {}
bool WebRTCIdentityStoreBackend::FindIdentity(
const GURL& origin,
@@ -221,21 +212,14 @@ bool WebRTCIdentityStoreBackend::FindIdentity(
IdentityKey key(origin, identity_name);
IdentityMap::iterator iter = identities_.find(key);
- base::TimeDelta age = base::Time::Now() - base::Time::FromInternalValue(
- iter->second.creation_time);
if (iter != identities_.end() && iter->second.common_name == common_name) {
- if (age < validity_period_) {
- // Identity found.
- return BrowserThread::PostTask(BrowserThread::IO,
- FROM_HERE,
- base::Bind(callback,
- net::OK,
- iter->second.certificate,
- iter->second.private_key));
- }
- // Removes the expired identity from the in-memory cache. The copy in the
- // database will be removed on the next load.
- identities_.erase(iter);
+ // Identity found.
+ return BrowserThread::PostTask(BrowserThread::IO,
+ FROM_HERE,
+ base::Bind(callback,
+ net::OK,
+ iter->second.certificate,
+ iter->second.private_key));
}
return BrowserThread::PostTask(
@@ -302,50 +286,30 @@ void WebRTCIdentityStoreBackend::DeleteBetween(base::Time delete_begin,
base::Time delete_end,
const base::Closure& callback) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- if (state_ == CLOSED)
- return;
-
// Delete the in-memory cache.
IdentityMap::iterator it = identities_.begin();
while (it != identities_.end()) {
if (it->second.creation_time >= delete_begin.ToInternalValue() &&
- it->second.creation_time <= delete_end.ToInternalValue()) {
+ it->second.creation_time <= delete_end.ToInternalValue())
identities_.erase(it++);
- } else {
- ++it;
- }
+ else
+ it++;
}
+
BrowserThread::PostTaskAndReply(BrowserThread::DB,
FROM_HERE,
base::Bind(&SqlLiteStorage::DeleteBetween,
sql_lite_storage_,
delete_begin,
- delete_end),
+ delete_end,
+ callback),
callback);
}
-void WebRTCIdentityStoreBackend::SetValidityPeriodForTesting(
- base::TimeDelta validity_period) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
- validity_period_ = validity_period;
- BrowserThread::PostTask(
- BrowserThread::DB,
- FROM_HERE,
- base::Bind(&SqlLiteStorage::SetValidityPeriodForTesting,
- sql_lite_storage_,
- validity_period));
-}
-
WebRTCIdentityStoreBackend::~WebRTCIdentityStoreBackend() {}
void WebRTCIdentityStoreBackend::OnLoaded(scoped_ptr<IdentityMap> out_map) {
DCHECK(BrowserThread::CurrentlyOn(BrowserThread::IO));
-
- if (state_ != LOADING)
- return;
-
- DVLOG(2) << "WebRTC identity store has loaded.";
-
state_ = LOADED;
identities_.swap(*out_map);
@@ -393,9 +357,6 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Load(IdentityMap* out_map) {
db_->Preload();
- // Delete expired identities.
- DeleteBetween(base::Time(), base::Time::Now() - validity_period_);
-
// Slurp all the identities into the out_map.
sql::Statement stmt(db_->GetUniqueStatement(
"SELECT origin, identity_name, common_name, "
@@ -450,35 +411,6 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::DeleteIdentity(
BatchOperation(DELETE_IDENTITY, origin, identity_name, identity);
}
-void WebRTCIdentityStoreBackend::SqlLiteStorage::DeleteBetween(
- base::Time delete_begin,
- base::Time delete_end) {
- DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
- if (!db_.get())
- return;
-
- // Commit pending operations first.
- Commit();
-
- sql::Statement del_stmt(db_->GetCachedStatement(
- SQL_FROM_HERE,
- "DELETE FROM webrtc_identity_store"
- " WHERE creation_time >= ? AND creation_time <= ?"));
- CHECK(del_stmt.is_valid());
-
- del_stmt.BindInt64(0, delete_begin.ToInternalValue());
- del_stmt.BindInt64(1, delete_end.ToInternalValue());
-
- sql::Transaction transaction(db_.get());
- if (!transaction.Begin()) {
- DLOG(ERROR) << "Failed to begin the transaction.";
- return;
- }
-
- CHECK(del_stmt.Run());
- transaction.Commit();
-}
-
void WebRTCIdentityStoreBackend::SqlLiteStorage::OnDatabaseError(
int error,
sql::Statement* stmt) {
@@ -581,4 +513,34 @@ void WebRTCIdentityStoreBackend::SqlLiteStorage::Commit() {
pending_operations_.clear();
}
+void WebRTCIdentityStoreBackend::SqlLiteStorage::DeleteBetween(
+ base::Time delete_begin,
+ base::Time delete_end,
+ const base::Closure& callback) {
+ DCHECK(BrowserThread::CurrentlyOn(BrowserThread::DB));
+ if (!db_.get())
+ return;
+
+ // Commit pending operations first.
+ Commit();
+
+ sql::Statement del_stmt(db_->GetCachedStatement(
+ SQL_FROM_HERE,
+ "DELETE FROM webrtc_identity_store"
+ " WHERE creation_time >= ? AND creation_time <= ?"));
+ CHECK(del_stmt.is_valid());
+
+ del_stmt.BindInt64(0, delete_begin.ToInternalValue());
+ del_stmt.BindInt64(1, delete_end.ToInternalValue());
+
+ sql::Transaction transaction(db_.get());
+ if (!transaction.Begin()) {
+ DLOG(ERROR) << "Failed to begin the transaction.";
+ return;
+ }
+
+ CHECK(del_stmt.Run());
+ transaction.Commit();
+}
+
} // namespace content
diff --git a/content/browser/media/webrtc_identity_store_backend.h b/content/browser/media/webrtc_identity_store_backend.h
index 92b68b6..ab4e1ed 100644
--- a/content/browser/media/webrtc_identity_store_backend.h
+++ b/content/browser/media/webrtc_identity_store_backend.h
@@ -35,11 +35,9 @@ class WebRTCIdentityStoreBackend
const std::string& private_key)>
FindIdentityCallback;
- // No data is saved on disk if |path| is empty. Identites older than
- // |validity_period| will be removed lazily.
+ // No data is saved on disk if |path| is empty.
WebRTCIdentityStoreBackend(const base::FilePath& path,
- quota::SpecialStoragePolicy* policy,
- base::TimeDelta validity_period);
+ quota::SpecialStoragePolicy* policy);
// Finds the identity with |origin|, |identity_name|, and |common_name| from
// the DB.
@@ -79,10 +77,6 @@ class WebRTCIdentityStoreBackend
base::Time delete_end,
const base::Closure& callback);
- // Changes the validity period. Should be called before the database is
- // loaded into memory.
- void SetValidityPeriodForTesting(base::TimeDelta validity_period);
-
private:
friend class base::RefCountedThreadSafe<WebRTCIdentityStoreBackend>;
class SqlLiteStorage;
@@ -101,9 +95,6 @@ class WebRTCIdentityStoreBackend
void OnLoaded(scoped_ptr<IdentityMap> out_map);
-
- // Identities expires after |validity_period_|.
- base::TimeDelta validity_period_;
// In-memory copy of the identities.
IdentityMap identities_;
// "Find identity" requests waiting for the DB to load.
diff --git a/content/browser/media/webrtc_identity_store_unittest.cc b/content/browser/media/webrtc_identity_store_unittest.cc
index 129fa8d..81c25d7 100644
--- a/content/browser/media/webrtc_identity_store_unittest.cc
+++ b/content/browser/media/webrtc_identity_store_unittest.cc
@@ -3,7 +3,6 @@
// found in the LICENSE file.
#include "base/bind.h"
-#include "base/files/scoped_temp_dir.h"
#include "base/memory/scoped_ptr.h"
#include "base/test/sequenced_worker_pool_owner.h"
#include "content/browser/media/webrtc_identity_store.h"
@@ -16,7 +15,8 @@
namespace content {
// TODO(jiayl): the tests fail on Android since the openssl version of
-// CreateSelfSignedCert is not implemented.
+// CreateSelfSignedCert is not implemented. We should mock out this dependency
+// and remove the if-defined.
#if !defined(OS_ANDROID)
static const char* kFakeOrigin = "http://foo.com";
@@ -55,10 +55,6 @@ class WebRTCIdentityStoreTest : public testing::Test {
pool_owner_->pool()->Shutdown();
}
- void SetValidityPeriod(base::TimeDelta validity_period) {
- webrtc_identity_store_->SetValidityPeriodForTesting(validity_period);
- }
-
void RunUntilIdle() {
RunAllPendingInMessageLoop(BrowserThread::DB);
RunAllPendingInMessageLoop(BrowserThread::IO);
@@ -82,11 +78,6 @@ class WebRTCIdentityStoreTest : public testing::Test {
return cancel_callback;
}
- void Restart(const base::FilePath& path) {
- webrtc_identity_store_ = new WebRTCIdentityStore(path, NULL);
- webrtc_identity_store_->SetTaskRunnerForTesting(pool_owner_->pool());
- }
-
protected:
TestBrowserThreadBundle browser_thread_bundle_;
scoped_ptr<base::SequencedWorkerPoolOwner> pool_owner_;
@@ -283,69 +274,5 @@ TEST_F(WebRTCIdentityStoreTest, DeleteDataAndGenerateNewIdentity) {
EXPECT_NE(key_1, key_2);
}
-TEST_F(WebRTCIdentityStoreTest, ExpiredIdentityDeleted) {
- // The identities will expire immediately after creation.
- SetValidityPeriod(base::TimeDelta::FromMilliseconds(0));
-
- bool completed_1 = false;
- bool completed_2 = false;
- std::string cert_1, cert_2, key_1, key_2;
-
- base::Closure cancel_callback_1 =
- RequestIdentityAndRunUtilIdle(kFakeOrigin,
- kFakeIdentityName1,
- kFakeCommonName1,
- &completed_1,
- &cert_1,
- &key_1);
- EXPECT_TRUE(completed_1);
-
- // Check that the old identity is not returned.
- base::Closure cancel_callback_2 =
- RequestIdentityAndRunUtilIdle(kFakeOrigin,
- kFakeIdentityName1,
- kFakeCommonName1,
- &completed_2,
- &cert_2,
- &key_2);
- EXPECT_TRUE(completed_2);
- EXPECT_NE(cert_1, cert_2);
- EXPECT_NE(key_1, key_2);
-}
-
-TEST_F(WebRTCIdentityStoreTest, IdentityPersistentAcrossRestart) {
- base::ScopedTempDir temp_dir;
- ASSERT_TRUE(temp_dir.CreateUniqueTempDir());
- Restart(temp_dir.path());
-
- bool completed_1 = false;
- bool completed_2 = false;
- std::string cert_1, cert_2, key_1, key_2;
-
- // Creates an identity.
- base::Closure cancel_callback_1 =
- RequestIdentityAndRunUtilIdle(kFakeOrigin,
- kFakeIdentityName1,
- kFakeCommonName1,
- &completed_1,
- &cert_1,
- &key_1);
- EXPECT_TRUE(completed_1);
-
- Restart(temp_dir.path());
-
- // Check that the same identity is returned after the restart.
- base::Closure cancel_callback_2 =
- RequestIdentityAndRunUtilIdle(kFakeOrigin,
- kFakeIdentityName1,
- kFakeCommonName1,
- &completed_2,
- &cert_2,
- &key_2);
- EXPECT_TRUE(completed_2);
- EXPECT_EQ(cert_1, cert_2);
- EXPECT_EQ(key_1, key_2);
-}
-
#endif
} // namespace content
generated by cgit v1.1 at 2025-04-04 09:33:36 +0000