Merge pull request #975 from brave/breaking-extensions

Disable shields for extensions

browser/brave_content_browser_client.cc

@@ -99,9 +99,11 @@ bool BraveContentBrowserClient::AllowAccessCookie(const GURL& url, const GURL& f
       BraveShieldsWebContentsObserver::GetTabURLFromRenderFrameInfo(
           render_process_id, render_frame_id).GetOrigin();
   ProfileIOData* io_data = ProfileIOData::FromResourceContext(context);
-  bool allow_brave_shields = brave_shields::IsAllowContentSettingWithIOData(
-      io_data, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
-      brave_shields::kBraveShields);
+  bool allow_brave_shields =
+      brave_shields::IsAllowContentSettingWithIOData(
+          io_data, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
+          brave_shields::kBraveShields) &&
+      !first_party.SchemeIs(kChromeExtensionScheme);
   bool allow_1p_cookies = brave_shields::IsAllowContentSettingWithIOData(
       io_data, tab_origin, GURL("https://firstParty/"),
       CONTENT_SETTINGS_TYPE_PLUGINS, brave_shields::kCookies);

browser/extensions/brave_extension_functional_test.cc

@@ -15,7 +15,8 @@
 
 namespace extensions {
 
-void ExtensionFunctionalTest::InstallExtensionSilently(ExtensionService* service,
+const Extension* ExtensionFunctionalTest::InstallExtensionSilently(
+    ExtensionService* service,
     const base::FilePath& path) {
   ExtensionRegistry* registry = ExtensionRegistry::Get(profile());
   size_t num_before = registry->enabled_extensions().size();
@@ -39,6 +40,7 @@ void ExtensionFunctionalTest::InstallExtensionSilently(ExtensionService* service
 
   const Extension* extension = registry_observer.WaitForExtensionReady();
   EXPECT_TRUE(extension);
+  return extension;
 }
 
 void ExtensionFunctionalTest::SetUp() {

browser/extensions/brave_extension_functional_test.h

@@ -10,8 +10,8 @@ namespace extensions {
 
 class ExtensionFunctionalTest : public ExtensionBrowserTest {
  public:
-  void InstallExtensionSilently(ExtensionService* service,
-                                const base::FilePath& path);
+  const Extension* InstallExtensionSilently(ExtensionService* service,
+                                            const base::FilePath& path);
   void SetUp() override;
   void InitEmbeddedTestServer();
   void GetTestDataDir(base::FilePath* test_data_dir);

browser/extensions/brave_extension_provider_browsertest.cc

@@ -8,6 +8,8 @@
 #include "brave/browser/brave_browser_process_impl.h"
 #include "brave/browser/extensions/brave_extension_functional_test.h"
 #include "brave/common/brave_paths.h"
+#include "brave/common/pref_names.h"
+#include "brave/common/url_constants.h"
 #include "brave/components/brave_shields/browser/https_everywhere_service.h"
 #include "chrome/browser/extensions/crx_installer.h"
 #include "chrome/browser/extensions/extension_browsertest.h"
@@ -65,4 +67,68 @@ IN_PROC_BROWSER_TEST_F(BraveExtensionProviderTest, PDFJSInstalls) {
   ASSERT_TRUE(pdfjs_exists);
 }
 
+// Load an extension page with an ad image, and make sure it is NOT blocked.
+// It would otherwise be blocked though if it wasn't an extension.
+IN_PROC_BROWSER_TEST_F(BraveExtensionProviderTest, AdsNotBlockedByDefaultBlockerInExtension) {
+  base::FilePath test_data_dir;
+  GetTestDataDir(&test_data_dir);
+  const extensions::Extension* extension = InstallExtensionSilently(extension_service(),
+      test_data_dir.AppendASCII("extension-compat-test-extension.crx"));
+  GURL url = GURL(std::string(kChromeExtensionScheme) + "://" + extension->id() + "/blocking.html");
+
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(content::WaitForLoadStop(contents));
+  EXPECT_EQ(url, contents->GetURL());
+
+  bool as_expected = false;
+  ASSERT_TRUE(ExecuteScriptAndExtractBool(
+      contents,
+      "setExpectations(1, 0, 0, 0);"
+      "addImage('ad_banner.png')",
+      &as_expected));
+  EXPECT_TRUE(as_expected);
+  EXPECT_EQ(browser()->profile()->GetPrefs()->GetUint64(kAdsBlocked), 0ULL);
+}
+
+IN_PROC_BROWSER_TEST_F(BraveExtensionProviderTest, ExtensionsCanGetCookies) {
+  base::FilePath test_data_dir;
+  GetTestDataDir(&test_data_dir);
+  const extensions::Extension* extension = InstallExtensionSilently(extension_service(),
+      test_data_dir.AppendASCII("extension-compat-test-extension.crx"));
+  GURL url = GURL(std::string(kChromeExtensionScheme) + "://" + extension->id() + "/blocking.html");
+
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(content::WaitForLoadStop(contents));
+  EXPECT_EQ(url, contents->GetURL());
+
+  bool as_expected = false;
+  ASSERT_TRUE(ExecuteScriptAndExtractBool(
+      contents,
+      "canGetCookie('test', 'http://a.com')",
+      &as_expected));
+  EXPECT_TRUE(as_expected);
+}
+
+IN_PROC_BROWSER_TEST_F(BraveExtensionProviderTest, ExtensionsCanSetCookies) {
+  base::FilePath test_data_dir;
+  GetTestDataDir(&test_data_dir);
+  const extensions::Extension* extension = InstallExtensionSilently(extension_service(),
+      test_data_dir.AppendASCII("extension-compat-test-extension.crx"));
+  GURL url = GURL(std::string(kChromeExtensionScheme) + "://" + extension->id() + "/blocking.html");
+
+  ui_test_utils::NavigateToURL(browser(), url);
+  content::WebContents* contents = browser()->tab_strip_model()->GetActiveWebContents();
+  ASSERT_TRUE(content::WaitForLoadStop(contents));
+  EXPECT_EQ(url, contents->GetURL());
+
+  bool as_expected = false;
+  ASSERT_TRUE(ExecuteScriptAndExtractBool(
+      contents,
+      "canSetCookie('test', 'testval', 'http://a.com')",
+      &as_expected));
+  EXPECT_TRUE(as_expected);
+}
+
 }  // namespace extnesions

browser/net/brave_ad_block_tp_network_delegate_helper.cc

@@ -11,7 +11,6 @@
 #include "brave/browser/brave_browser_process_impl.h"
 #include "brave/common/network_constants.h"
 #include "brave/common/shield_exceptions.h"
-#include "brave/common/url_util.h"
 #include "brave/components/brave_shields/browser/ad_block_regional_service.h"
 #include "brave/components/brave_shields/browser/ad_block_service.h"
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
@@ -99,12 +98,7 @@ void OnBeforeURLRequestAdBlockTPOnTaskRunner(std::shared_ptr<BraveRequestInfo> c
   }
   DCHECK(ctx->request_identifier != 0);
 
-  // For PDFJS we don't want to treat 2 different URLs as 3p for the main pdf
-  // load which is meant to be top level
-  // chrome-extension://oemmndcbldboiebfnladdacbdfmadadm/https://developer.att.com/.../file.pdf
-  // https://developer.att.com/.../file.pdf
-  // So if the tab origin is chrome-extension, set it to that of the PDF only for PDFJS
-  std::string tab_host = brave::GetURLOrPDFURL(ctx->tab_url).host();
+  std::string tab_host = ctx->tab_origin.host();
   if (!g_brave_browser_process->tracking_protection_service()->
       ShouldStartRequest(ctx->request_url, ctx->resource_type, tab_host)) {
     ctx->new_url_spec = GetBlankDataURLForResourceType(ctx->resource_type).spec();

browser/net/brave_site_hacks_network_delegate_helper.cc

@@ -10,6 +10,7 @@
 #include "base/strings/string_util.h"
 #include "brave/common/network_constants.h"
 #include "brave/common/shield_exceptions.h"
+#include "brave/common/url_constants.h"
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
 #include "brave/components/brave_shields/browser/brave_shields_web_contents_observer.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
@@ -28,6 +29,9 @@ bool ApplyPotentialReferrerBlock(net::URLRequest* request) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   GURL target_origin = GURL(request->url()).GetOrigin();
   GURL tab_origin = request->site_for_cookies().GetOrigin();
+  if (tab_origin.SchemeIs(kChromeExtensionScheme)) {
+    return false;
+  }
   bool allow_referrers = brave_shields::IsAllowContentSettingFromIO(
       request, tab_origin, tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
       brave_shields::kReferrers);

browser/net/brave_site_hacks_network_delegate_helper_unittest.cc

@@ -271,4 +271,32 @@ TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerCleared) {
   });
 }
 
+TEST_F(BraveSiteHacksNetworkDelegateHelperTest, ReferrerWouldBeClearedButExtensionSite) {
+  std::vector<GURL> urls({
+    GURL("https://digg.com/7"),
+    GURL("https://slashdot.org/5"),
+    GURL("https://bondy.brian.org")
+  });
+  std::for_each(urls.begin(), urls.end(),
+      [this](GURL url){
+    net::TestDelegate test_delegate;
+    std::unique_ptr<net::URLRequest> request =
+        context()->CreateRequest(url, net::IDLE, &test_delegate,
+                                 TRAFFIC_ANNOTATION_FOR_TESTS);
+    request->set_site_for_cookies(GURL("chrome-extension://aemmndcbldboiebfnladdacbdfmadadm/test.html"));
+    std::string original_referrer = "https://hello.brianbondy.com/about";
+    request->SetReferrer(original_referrer);
+
+    std::shared_ptr<brave::BraveRequestInfo>
+        brave_request_info(new brave::BraveRequestInfo());
+    brave::BraveRequestInfo::FillCTXFromRequest(request.get(), brave_request_info);
+    brave::ResponseCallback callback;
+    int ret = brave::OnBeforeURLRequest_SiteHacksWork(callback, brave_request_info);
+    EXPECT_EQ(ret, net::OK);
+    // new_url should not be set
+    EXPECT_TRUE(brave_request_info->new_url_spec.empty());
+    EXPECT_STREQ(request->referrer().c_str(), original_referrer.c_str());
+  });
+}
+
 }  // namespace

browser/net/url_context.cc

@@ -7,6 +7,7 @@
 
 #include <string>
 
+#include "brave/common/url_constants.h"
 #include "brave/components/brave_shields/browser/brave_shields_util.h"
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "content/public/browser/resource_request_info.h"
@@ -23,7 +24,6 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
     std::shared_ptr<brave::BraveRequestInfo> ctx) {
   ctx->request_identifier = request->identifier();
   ctx->request_url = request->url();
-  ctx->tab_url = request->site_for_cookies();
   ctx->tab_origin = request->site_for_cookies().GetOrigin();
   auto* request_info = content::ResourceRequestInfo::ForRequest(request);
   if (request_info) {
@@ -33,7 +33,8 @@ void BraveRequestInfo::FillCTXFromRequest(const net::URLRequest* request,
       &ctx->frame_tree_node_id);
   ctx->allow_brave_shields = brave_shields::IsAllowContentSettingFromIO(
       request, ctx->tab_origin, ctx->tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
-      brave_shields::kBraveShields);
+      brave_shields::kBraveShields) &&
+    !request->site_for_cookies().SchemeIs(kChromeExtensionScheme);
   ctx->allow_ads = brave_shields::IsAllowContentSettingFromIO(
       request, ctx->tab_origin, ctx->tab_origin, CONTENT_SETTINGS_TYPE_PLUGINS,
       brave_shields::kAds);

browser/net/url_context.h

@@ -51,7 +51,6 @@ struct BraveRequestInfo {
   ~BraveRequestInfo();
   GURL request_url;
   GURL tab_origin;
-  GURL tab_url;
   std::string new_url_spec;
   bool allow_brave_shields = true;
   bool allow_ads = false;

common/BUILD.gn

@@ -51,8 +51,6 @@ source_set("common") {
       "shield_exceptions.h",
       "url_constants.cc",
       "url_constants.h",
-      "url_util.cc",
-      "url_util.h",
     ]
 
     public_deps = [

common/url_constants.cc

@@ -4,6 +4,7 @@
 
 #include "brave/common/url_constants.h"
 
+const char kChromeExtensionScheme[] = "chrome-extension";
 const char kBraveUIScheme[] = "brave";
 const char kMagnetScheme[] = "magnet";
 const char kWidevineMoreInfoURL[] = "https://www.eff.org/issues/drm";

common/url_constants.h

@@ -5,6 +5,7 @@
 #ifndef BRAVE_COMMON_URL_CONSTANTS_H_
 #define BRAVE_COMMON_URL_CONSTANTS_H_
 
+extern const char kChromeExtensionScheme[];
 extern const char kBraveUIScheme[];
 extern const char kMagnetScheme[];
 extern const char kWidevineMoreInfoURL[];

components/content_settings/core/browser/brave_cookie_settings.cc

@@ -6,6 +6,7 @@
 
 #include "brave/components/brave_shields/common/brave_shield_constants.h"
 #include "brave/common/brave_cookie_blocking.h"
+#include "extensions/buildflags/buildflags.h"
 #include "net/base/registry_controlled_domains/registry_controlled_domain.h"
 #include "url/gurl.h"
 
@@ -36,6 +37,24 @@ void BraveCookieSettings::GetCookieSetting(const GURL& url,
     content_settings::SettingSource* source,
     ContentSetting* cookie_setting) const {
   DCHECK(cookie_setting);
+
+  // Auto-allow in extensions or for WebUI embedded in a secure origin.
+  // This matches an early return case in CookieSettings::GetCookieSetting
+  if (first_party_url.SchemeIs(kChromeUIScheme) &&
+      url.SchemeIsCryptographic()) {
+    *cookie_setting = CONTENT_SETTING_ALLOW;
+    return;
+  }
+
+  // This matches an early return case in CookieSettings::GetCookieSetting
+#if BUILDFLAG(ENABLE_EXTENSIONS)
+  if (url.SchemeIs(extension_scheme_) &&
+      first_party_url.SchemeIs(extension_scheme_)) {
+    *cookie_setting = CONTENT_SETTING_ALLOW;
+    return;
+  }
+#endif
+
   CookieSettings::GetCookieSetting(url, first_party_url, source,
       cookie_setting);
   if (*cookie_setting == CONTENT_SETTING_BLOCK) {

test/BUILD.gn

@@ -59,7 +59,6 @@ test("brave_unit_tests") {
     "//brave/common/shield_exceptions_unittest.cc",
     "//brave/common/tor/tor_test_constants.cc",
     "//brave/common/tor/tor_test_constants.h",
-    "//brave/common/url_util_unittest.cc",
     "//brave/components/assist_ranker/ranker_model_loader_impl_unittest.cc",
     "//brave/components/brave_shields/browser/ad_block_regional_service_unittest.cc",
     "//brave/components/brave_sync/bookmark_order_util_unittest.cc",