Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change default in ShouldUseUserAgentOverride for OOPIF #6793

Merged
merged 1 commit into from
Oct 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions browser/farbling/brave_navigator_useragent_farbling_browsertest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "brave/common/pref_names.h"
#include "brave/components/brave_component_updater/browser/local_data_files_service.h"
#include "brave/components/brave_shields/browser/brave_shields_util.h"
#include "chrome/browser/chrome_content_browser_client.h"
#include "chrome/browser/content_settings/host_content_settings_map_factory.h"
#include "chrome/browser/extensions/extension_browsertest.h"
#include "chrome/browser/ui/browser.h"
Expand All @@ -31,6 +32,7 @@ using brave_shields::ControlType;

const char kUserAgentScript[] =
"domAutomationController.send(navigator.userAgent);";
const char kTitleScript[] = "domAutomationController.send(document.title);";

class BraveNavigatorUserAgentFarblingBrowserTest : public InProcessBrowserTest {
public:
Expand Down Expand Up @@ -115,17 +117,19 @@ IN_PROC_BROWSER_TEST_F(BraveNavigatorUserAgentFarblingBrowserTest,
std::string domain_z = "z.com";
GURL url_b = embedded_test_server()->GetURL(domain_b, "/simple.html");
GURL url_z = embedded_test_server()->GetURL(domain_z, "/simple.html");

// Farbling level: off
// get real navigator.userAgent
std::string real_ua = ::GetUserAgent();
// Farbling level: off
AllowFingerprinting(domain_b);
NavigateToURLUntilLoadStop(url_b);
std::string off_ua_b = ExecScriptGetStr(kUserAgentScript, contents());
// user agent should be the same as the real user agent
EXPECT_EQ(off_ua_b, real_ua);
AllowFingerprinting(domain_z);
NavigateToURLUntilLoadStop(url_z);
std::string off_ua_z = ExecScriptGetStr(kUserAgentScript, contents());
// user agent should be the same on every domain if farbling is off
EXPECT_EQ(off_ua_b, off_ua_z);
EXPECT_EQ(off_ua_z, real_ua);

// Farbling level: default
// navigator.userAgent may be farbled, but the farbling is not
Expand All @@ -144,6 +148,7 @@ IN_PROC_BROWSER_TEST_F(BraveNavigatorUserAgentFarblingBrowserTest,
// farbling level, further suffixed by a pseudo-random number of spaces based
// on domain and session key
BlockFingerprinting(domain_b);
// test known values
NavigateToURLUntilLoadStop(url_b);
std::string max_ua_b = ExecScriptGetStr(kUserAgentScript, contents());
EXPECT_EQ(max_ua_b, default_ua_b + " ");
Expand All @@ -152,6 +157,17 @@ IN_PROC_BROWSER_TEST_F(BraveNavigatorUserAgentFarblingBrowserTest,
std::string max_ua_z = ExecScriptGetStr(kUserAgentScript, contents());
EXPECT_EQ(max_ua_z, default_ua_z + " ");

// test that iframes also inherit the farbled user agent
// (farbling level is still maximum)
NavigateToURLUntilLoadStop(embedded_test_server()->GetURL(
domain_b, "/navigator/ua-local-iframe.html"));
std::string local_iframe_ua = ExecScriptGetStr(kTitleScript, contents());
EXPECT_EQ(local_iframe_ua, "pass");
NavigateToURLUntilLoadStop(embedded_test_server()->GetURL(
domain_b, "/navigator/ua-remote-iframe.html"));
std::string remote_iframe_ua = ExecScriptGetStr(kTitleScript, contents());
EXPECT_EQ(remote_iframe_ua, "pass");

// Farbling level: off
// verify that user agent is reset properly after having been farbled
AllowFingerprinting(domain_b);
Expand Down
13 changes: 13 additions & 0 deletions patches/content-renderer-render_frame_impl.cc.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
diff --git a/content/renderer/render_frame_impl.cc b/content/renderer/render_frame_impl.cc
index c110db6a32bba72c7a89137fa39377c58f0061d8..bd82ce78d6be98b8369e8dbeb5c364fbd24c4578 100644
--- a/content/renderer/render_frame_impl.cc
+++ b/content/renderer/render_frame_impl.cc
@@ -5030,7 +5030,7 @@ bool RenderFrameImpl::ShouldUseUserAgentOverride() const {
// Temporarily return early and fix properly as part of
// https://crbug.com/426555.
if (render_view_->GetWebView()->MainFrame()->IsWebRemoteFrame())
- return false;
+ return true;
WebLocalFrame* main_frame =
render_view_->GetWebView()->MainFrame()->ToWebLocalFrame();

12 changes: 12 additions & 0 deletions test/data/navigator/ua-iframe.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
<!DOCTYPE HTML>
<!-- navigator.userAgent iframe test -->
<html>
<head>
<title></title>
</head>
<body>
<script>
window.parent.postMessage(navigator.userAgent, '*');
</script>
</body>
</html>
17 changes: 17 additions & 0 deletions test/data/navigator/ua-local-iframe.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<!DOCTYPE HTML>
<!-- navigator.userAgent iframe test -->
<html>
<head>
<title></title>
</head>
<body>
<script>
window.onmessage = function(event) {
if (navigator.userAgent == event.data) {
document.title = "pass";
}
}
</script>
<iframe src="ua-iframe.html" id="test"></iframe>
</body>
</html>
19 changes: 19 additions & 0 deletions test/data/navigator/ua-remote-iframe.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE HTML>
<!-- navigator.userAgent iframe test -->
<html>
<head>
<title></title>
</head>
<body>
<script>
window.onmessage = function(event) {
if (navigator.userAgent == event.data) {
document.title = "pass";
}
}
var iframe = document.createElement("iframe");
iframe.src = document.location.href.replace("b.com", "z.com").replace("remote-", "");
document.body.appendChild(iframe);
</script>
</body>
</html>