Commit 6cd49e63 authored by Daniel Murphy's avatar Daniel Murphy Committed by Commit Bot

[SessionStorageS13N] Save process id per bound namespace

Previously, the process id was just the last-bound namespace process.
With site-isolation turned on, multiple processes bind to the same
namespace, so when 'old' ones opened up their storage area, they would
check the wrong process id and fail.

This stores the process id as the 'Context' on the binding set.

Bug: 848281
Change-Id: I5d4a082494f618c361f0ce845a641679330841fe
Reviewed-on: https://chromium-review.googlesource.com/1103553Reviewed-by: default avatarMarijn Kruisselbrink <mek@chromium.org>
Commit-Queue: Marijn Kruisselbrink <mek@chromium.org>
Cr-Commit-Position: refs/heads/master@{#567871}
parent 12c66048
...@@ -79,7 +79,6 @@ void SessionStorageNamespaceImplMojo::PopulateAsClone( ...@@ -79,7 +79,6 @@ void SessionStorageNamespaceImplMojo::PopulateAsClone(
void SessionStorageNamespaceImplMojo::Reset() { void SessionStorageNamespaceImplMojo::Reset() {
namespace_entry_ = SessionStorageMetadata::NamespaceEntry(); namespace_entry_ = SessionStorageMetadata::NamespaceEntry();
process_id_ = ChildProcessHost::kInvalidUniqueID;
database_ = nullptr; database_ = nullptr;
waiting_on_clone_population_ = false; waiting_on_clone_population_ = false;
bind_waiting_on_clone_population_ = false; bind_waiting_on_clone_population_ = false;
...@@ -100,8 +99,7 @@ void SessionStorageNamespaceImplMojo::Bind( ...@@ -100,8 +99,7 @@ void SessionStorageNamespaceImplMojo::Bind(
return; return;
} }
DCHECK(IsPopulated()); DCHECK(IsPopulated());
process_id_ = process_id; bindings_.AddBinding(this, std::move(request), process_id);
bindings_.AddBinding(this, std::move(request));
bind_waiting_on_clone_population_ = false; bind_waiting_on_clone_population_ = false;
} }
...@@ -138,9 +136,9 @@ void SessionStorageNamespaceImplMojo::OpenArea( ...@@ -138,9 +136,9 @@ void SessionStorageNamespaceImplMojo::OpenArea(
blink::mojom::StorageAreaAssociatedRequest database) { blink::mojom::StorageAreaAssociatedRequest database) {
DCHECK(IsPopulated()); DCHECK(IsPopulated());
DCHECK(!bindings_.empty()); DCHECK(!bindings_.empty());
DCHECK_NE(process_id_, ChildProcessHost::kInvalidUniqueID); int process_id = bindings_.dispatch_context();
if (!ChildProcessSecurityPolicy::GetInstance()->CanAccessDataForOrigin( if (!ChildProcessSecurityPolicy::GetInstance()->CanAccessDataForOrigin(
process_id_, origin.GetURL())) { process_id, origin.GetURL())) {
bindings_.ReportBadMessage("Access denied for sessionStorage request"); bindings_.ReportBadMessage("Access denied for sessionStorage request");
return; return;
} }
......
...@@ -12,7 +12,6 @@ ...@@ -12,7 +12,6 @@
#include "content/browser/dom_storage/session_storage_area_impl.h" #include "content/browser/dom_storage/session_storage_area_impl.h"
#include "content/browser/dom_storage/session_storage_data_map.h" #include "content/browser/dom_storage/session_storage_data_map.h"
#include "content/browser/dom_storage/session_storage_metadata.h" #include "content/browser/dom_storage/session_storage_metadata.h"
#include "content/public/common/child_process_host.h"
#include "mojo/public/cpp/bindings/binding.h" #include "mojo/public/cpp/bindings/binding.h"
#include "mojo/public/cpp/bindings/binding_set.h" #include "mojo/public/cpp/bindings/binding_set.h"
#include "mojo/public/cpp/bindings/interface_ptr_set.h" #include "mojo/public/cpp/bindings/interface_ptr_set.h"
...@@ -137,7 +136,6 @@ class CONTENT_EXPORT SessionStorageNamespaceImplMojo final ...@@ -137,7 +136,6 @@ class CONTENT_EXPORT SessionStorageNamespaceImplMojo final
private: private:
const std::string namespace_id_; const std::string namespace_id_;
SessionStorageMetadata::NamespaceEntry namespace_entry_; SessionStorageMetadata::NamespaceEntry namespace_entry_;
int process_id_ = ChildProcessHost::kInvalidUniqueID;
leveldb::mojom::LevelDBDatabase* database_; leveldb::mojom::LevelDBDatabase* database_;
SessionStorageDataMap::Listener* data_map_listener_; SessionStorageDataMap::Listener* data_map_listener_;
...@@ -150,7 +148,8 @@ class CONTENT_EXPORT SessionStorageNamespaceImplMojo final ...@@ -150,7 +148,8 @@ class CONTENT_EXPORT SessionStorageNamespaceImplMojo final
bool populated_ = false; bool populated_ = false;
OriginAreas origin_areas_; OriginAreas origin_areas_;
mojo::BindingSet<blink::mojom::SessionStorageNamespace> bindings_; // The context is the process id.
mojo::BindingSet<blink::mojom::SessionStorageNamespace, int> bindings_;
}; };
} // namespace content } // namespace content
......
...@@ -448,5 +448,46 @@ TEST_F(SessionStorageNamespaceImplMojoTest, PurgeUnused) { ...@@ -448,5 +448,46 @@ TEST_F(SessionStorageNamespaceImplMojoTest, PurgeUnused) {
namespaces_.clear(); namespaces_.clear();
} }
TEST_F(SessionStorageNamespaceImplMojoTest, NamespaceBindingPerOrigin) {
// Tries to open an area with a process that is locked to a different origin
// and verifies the bad message callback.
SessionStorageNamespaceImplMojo* namespace_impl =
CreateSessionStorageNamespaceImplMojo(test_namespace_id1_);
EXPECT_CALL(listener_,
OnDataMapCreation(StdStringToUint8Vector("0"), testing::_))
.Times(1);
namespace_impl->PopulateFromMetadata(
&database_, metadata_.GetOrCreateNamespaceEntry(test_namespace_id1_),
std::map<std::vector<uint8_t>, SessionStorageDataMap*>());
blink::mojom::SessionStorageNamespacePtr ss_namespace_o1;
namespace_impl->Bind(mojo::MakeRequest(&ss_namespace_o1),
kTestProcessIdOrigin1);
blink::mojom::StorageAreaAssociatedPtr leveldb_1;
ss_namespace_o1->OpenArea(test_origin1_, mojo::MakeRequest(&leveldb_1));
ss_namespace_o1.FlushForTesting();
EXPECT_FALSE(bad_message_called_);
EXPECT_CALL(listener_,
OnDataMapCreation(StdStringToUint8Vector("1"), testing::_))
.Times(1);
blink::mojom::SessionStorageNamespacePtr ss_namespace_o2;
namespace_impl->Bind(mojo::MakeRequest(&ss_namespace_o2),
kTestProcessIdOrigin3);
blink::mojom::StorageAreaAssociatedPtr leveldb_2;
ss_namespace_o2->OpenArea(test_origin3_, mojo::MakeRequest(&leveldb_2));
ss_namespace_o2.FlushForTesting();
EXPECT_FALSE(bad_message_called_);
EXPECT_CALL(listener_, OnDataMapDestruction(StdStringToUint8Vector("0")))
.Times(1);
EXPECT_CALL(listener_, OnDataMapDestruction(StdStringToUint8Vector("1")))
.Times(1);
namespaces_.clear();
}
} // namespace } // namespace
} // namespace content } // namespace content
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