Add DumpWithoutCrashing and crash keys to get more context
for RFMF_SET_COOKIE_BAD_ORIGIN and RFMF_GET_COOKIES_BAD_ORIGIN
renderer kills.
This is all temporary code, which will be reverted after we understand
the issue.
BUG=600441
TEST=RenderFrameMessageFilterBrowserTest.CrossSiteCookieSecurityEnforcement
CQ_INCLUDE_TRYBOTS=tryserver.chromium.linux:linux_site_isolation
Review-Url: https://codereview.chromium.org/1855383002
Cr-Commit-Position: refs/heads/master@{#391396}
diff --git a/chrome/common/crash_keys.cc b/chrome/common/crash_keys.cc
index 99efb58..ef74b63 100644
--- a/chrome/common/crash_keys.cc
+++ b/chrome/common/crash_keys.cc
@@ -183,6 +183,11 @@
{ "initrf_root_is_in_same_site_instance_as_parent", kSmallSize},
{ "initrf_root_process_is_live", kSmallSize},
{ "initrf_root_proxy_is_live", kSmallSize},
+
+ // Temporary for https://crbug.com/600441
+ { "cookie_url", kSmallSize },
+ { "cookie_first_party", kSmallSize },
+ { "security_policy_origin_lock", kSmallSize },
};
// This dynamic set of keys is used for sets of key value pairs when gathering
diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc
index 34bad5ad..ed946fd 100644
--- a/content/browser/child_process_security_policy_impl.cc
+++ b/content/browser/child_process_security_policy_impl.cc
@@ -10,6 +10,7 @@
#include "base/files/file_path.h"
#include "base/logging.h"
#include "base/macros.h"
+#include "base/memory/ptr_util.h"
#include "base/metrics/histogram.h"
#include "base/stl_util.h"
#include "base/strings/string_util.h"
@@ -237,6 +238,9 @@
return origin_lock_ == site_gurl;
}
+ // TODO(nick): Remove this once we understand http://crbug.com/600441
+ GURL GetOriginLock() { return origin_lock_; }
+
void LockToOrigin(const GURL& gurl) {
origin_lock_ = gurl;
}
@@ -822,6 +826,18 @@
return state->second->CanAccessDataForOrigin(gurl);
}
+// TODO(nick): Remove this once we understand http://crbug.com/600441
+std::unique_ptr<base::debug::ScopedCrashKey>
+ChildProcessSecurityPolicyImpl::GetOriginLockCrashKey(int child_id) {
+ base::AutoLock lock(lock_);
+ SecurityStateMap::iterator state = security_state_.find(child_id);
+ return base::WrapUnique(new base::debug::ScopedCrashKey(
+ "security_policy_origin_lock",
+ state == security_state_.end()
+ ? "not-found"
+ : state->second->GetOriginLock().possibly_invalid_spec()));
+}
+
void ChildProcessSecurityPolicyImpl::LockToOrigin(int child_id,
const GURL& gurl) {
// "gurl" can be currently empty in some cases, such as file://blah.
diff --git a/content/browser/child_process_security_policy_impl.h b/content/browser/child_process_security_policy_impl.h
index 46d2d7a..025eb50 100644
--- a/content/browser/child_process_security_policy_impl.h
+++ b/content/browser/child_process_security_policy_impl.h
@@ -5,12 +5,13 @@
#ifndef CONTENT_BROWSER_CHILD_PROCESS_SECURITY_POLICY_IMPL_H_
#define CONTENT_BROWSER_CHILD_PROCESS_SECURITY_POLICY_IMPL_H_
-
#include <map>
+#include <memory>
#include <set>
#include <string>
#include "base/compiler_specific.h"
+#include "base/debug/crash_logging.h"
#include "base/gtest_prod_util.h"
#include "base/macros.h"
#include "base/memory/singleton.h"
@@ -78,6 +79,10 @@
void GrantSendMidiSysExMessage(int child_id) override;
bool CanAccessDataForOrigin(int child_id, const GURL& url) override;
+ // TODO(nick): Remove this once we understand http://crbug.com/600441
+ std::unique_ptr<base::debug::ScopedCrashKey> GetOriginLockCrashKey(
+ int child_id);
+
// Pseudo schemes are treated differently than other schemes because they
// cannot be requested like normal URLs. There is no mechanism for revoking
// pseudo schemes.
diff --git a/content/browser/frame_host/render_frame_message_filter.cc b/content/browser/frame_host/render_frame_message_filter.cc
index 98bd0fd..cb526235 100644
--- a/content/browser/frame_host/render_frame_message_filter.cc
+++ b/content/browser/frame_host/render_frame_message_filter.cc
@@ -5,6 +5,8 @@
#include "content/browser/frame_host/render_frame_message_filter.h"
#include "base/command_line.h"
+#include "base/debug/crash_logging.h"
+#include "base/debug/dump_without_crashing.h"
#include "base/macros.h"
#include "base/metrics/field_trial.h"
#include "base/strings/string_util.h"
@@ -249,6 +251,16 @@
ChildProcessSecurityPolicyImpl* policy =
ChildProcessSecurityPolicyImpl::GetInstance();
if (!policy->CanAccessDataForOrigin(render_process_id_, url)) {
+ {
+ // TODO(nick): Remove this once we understand http://crbug.com/600441
+ auto origin_lock = policy->GetOriginLockCrashKey(render_process_id_);
+ base::debug::ScopedCrashKey("cookie_url", url.possibly_invalid_spec());
+ base::debug::ScopedCrashKey(
+ "cookie_first_party",
+ first_party_for_cookies.possibly_invalid_spec());
+ base::debug::DumpWithoutCrashing();
+ }
+
bad_message::ReceivedBadMessage(this,
bad_message::RFMF_SET_COOKIE_BAD_ORIGIN);
return;
@@ -282,6 +294,16 @@
ChildProcessSecurityPolicyImpl* policy =
ChildProcessSecurityPolicyImpl::GetInstance();
if (!policy->CanAccessDataForOrigin(render_process_id_, url)) {
+ {
+ // TODO(nick): Remove this once we understand http://crbug.com/600441
+ auto origin_lock = policy->GetOriginLockCrashKey(render_process_id_);
+ base::debug::ScopedCrashKey("cookie_url", url.possibly_invalid_spec());
+ base::debug::ScopedCrashKey(
+ "cookie_first_party",
+ first_party_for_cookies.possibly_invalid_spec());
+ base::debug::DumpWithoutCrashing();
+ }
+
bad_message::ReceivedBadMessage(this,
bad_message::RFMF_GET_COOKIES_BAD_ORIGIN);
delete reply_msg;
