Skip to content

Commit

Permalink
Merge pull request #6793 from brave/mpilgrim_fix_ua_remote_frame_farb…
Browse files Browse the repository at this point in the history
…ling

Change default in ShouldUseUserAgentOverride for OOPIF
  • Loading branch information
pilgrim-brave authored Oct 8, 2020
2 parents 2badd76 + b8946f9 commit cf18546
Show file tree
Hide file tree
Showing 5 changed files with 80 additions and 3 deletions.
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>

0 comments on commit cf18546

Please sign in to comment.