Issue 518: Enabling CRLSets

Revoked certificates don't show certificate error on all platforms. This PR enables CRLSets, a component managed by Google to show certificate errors for domains with revoked certificates. Since, CRLSets is maintained by Google we will be proxying requests for CRLSets through crlsets[n].brave.com, crxdownload.brave.com (resources) and componentupdater.brave.com (component updates) This change: 1. Enables CRLSets 2. Proxies requests for CRLSet resources through brave proxies 3. Removes braveRedirect from brave_common_static_redirect_network_delegate 4. Lint fixes 5. Extension endpoint is set to dev server. Will be reverted after QA-signoff to update go-updater auditors: @bbondy, @bsclifton, @diracdeltas
brave · Feb 25, 2019 · e1903f9 · e1903f9
1 parent 2e81293
commit e1903f9
Show file tree

Hide file tree

Showing 10 changed files with 384 additions and 146 deletions.
diff --git a/browser/net/brave_common_static_redirect_network_delegate_helper.cc b/browser/net/brave_common_static_redirect_network_delegate_helper.cc
@@ -1,29 +1,37 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "brave/browser/net/brave_common_static_redirect_network_delegate_helper.h"
 
+#include <memory>
+#include <string>
+#include <vector>
+
 #include "brave/common/network_constants.h"
 #include "components/component_updater/component_updater_url_constants.h"
 #include "extensions/common/extension_urls.h"
 #include "extensions/common/url_pattern.h"
 
 namespace brave {
 
-// Update server checks happen from the profile context for admin policy installed extensions.
-// Update server checks happen from the system context for normal update operations.
+// Update server checks happen from the profile context for admin policy
+// installed extensions.
+// Update server checks happen from the system context for normal update
+// operations.
 bool IsUpdaterURL(const GURL& gurl) {
-  static std::vector<URLPattern> updater_patterns({
-      URLPattern(URLPattern::SCHEME_HTTPS, std::string(component_updater::kUpdaterDefaultUrl) + "*"),
-      URLPattern(URLPattern::SCHEME_HTTP, std::string(component_updater::kUpdaterFallbackUrl) + "*"),
-      URLPattern(URLPattern::SCHEME_HTTPS, std::string(extension_urls::kChromeWebstoreUpdateURL) + "*")
-  });
-  bool braveRedirect = gurl.query().find("braveRedirect=true") != std::string::npos;
-  return std::any_of(updater_patterns.begin(), updater_patterns.end(),
-      [&gurl, braveRedirect](URLPattern pattern) {
-        return !braveRedirect && pattern.MatchesURL(gurl);
-      });
+  static std::vector<URLPattern> updater_patterns(
+      {URLPattern(URLPattern::SCHEME_HTTPS,
+                  std::string(component_updater::kUpdaterDefaultUrl) + "*"),
+       URLPattern(URLPattern::SCHEME_HTTP,
+                  std::string(component_updater::kUpdaterFallbackUrl) + "*"),
+       URLPattern(
+           URLPattern::SCHEME_HTTPS,
+           std::string(extension_urls::kChromeWebstoreUpdateURL) + "*")});
+  return std::any_of(
+      updater_patterns.begin(), updater_patterns.end(),
+      [&gurl](URLPattern pattern) { return pattern.MatchesURL(gurl); });
 }
 
 int OnBeforeURLRequest_CommonStaticRedirectWork(
@@ -32,7 +40,9 @@ int OnBeforeURLRequest_CommonStaticRedirectWork(
   GURL::Replacements replacements;
   if (IsUpdaterURL(ctx->request_url)) {
     replacements.SetQueryStr(ctx->request_url.query_piece());
-    ctx->new_url_spec = GURL(kBraveUpdatesExtensionsEndpoint).ReplaceComponents(replacements).spec();
+    ctx->new_url_spec = GURL(kBraveUpdatesExtensionsEndpoint)
+                            .ReplaceComponents(replacements)
+                            .spec();
     return net::OK;
   }
   return net::OK;

diff --git a/browser/net/brave_common_static_redirect_network_delegate_helper_unittest.cc b/browser/net/brave_common_static_redirect_network_delegate_helper_unittest.cc
@@ -1,9 +1,13 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "brave/browser/net/brave_common_static_redirect_network_delegate_helper.h"
 
+#include <memory>
+#include <string>
+
 #include "brave/browser/net/url_context.h"
 #include "brave/common/network_constants.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
@@ -13,54 +17,55 @@
 #include "url/gurl.h"
 #include "url/url_constants.h"
 
-
 namespace {
 
-class BraveCommonStaticRedirectNetworkDelegateHelperTest: public testing::Test {
+const char kComponentUpdaterProxy[] = "https://componentupdater.brave.com";
+
+class BraveCommonStaticRedirectNetworkDelegateHelperTest
+    : public testing::Test {
  public:
   BraveCommonStaticRedirectNetworkDelegateHelperTest()
       : thread_bundle_(content::TestBrowserThreadBundle::IO_MAINLOOP),
-        context_(new net::TestURLRequestContext(true)) {
-  }
+        context_(new net::TestURLRequestContext(true)) {}
   ~BraveCommonStaticRedirectNetworkDelegateHelperTest() override {}
-  void SetUp() override {
-    context_->Init();
-  }
+  void SetUp() override { context_->Init(); }
   net::TestURLRequestContext* context() { return context_.get(); }
 
  private:
   content::TestBrowserThreadBundle thread_bundle_;
   std::unique_ptr<net::TestURLRequestContext> context_;
 };
 
-TEST_F(BraveCommonStaticRedirectNetworkDelegateHelperTest, ModifyComponentUpdaterURL) {
+TEST_F(BraveCommonStaticRedirectNetworkDelegateHelperTest,
+       ModifyComponentUpdaterURL) {
   net::TestDelegate test_delegate;
   std::string query_string("?foo=bar");
   GURL url(std::string(component_updater::kUpdaterDefaultUrl) + query_string);
-  std::unique_ptr<net::URLRequest> request =
-      context()->CreateRequest(url, net::IDLE, &test_delegate,
-                             TRAFFIC_ANNOTATION_FOR_TESTS);
-  std::shared_ptr<brave::BraveRequestInfo>
-      before_url_context(new brave::BraveRequestInfo());
-  brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
+  std::unique_ptr<net::URLRequest> request = context()->CreateRequest(
+      url, net::IDLE, &test_delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
+  std::shared_ptr<brave::BraveRequestInfo> before_url_context(
+      new brave::BraveRequestInfo());
+  brave::BraveRequestInfo::FillCTXFromRequest(request.get(),
+                                              before_url_context);
   brave::ResponseCallback callback;
-  GURL expected_url(std::string(kBraveUpdatesExtensionsEndpoint + query_string));
+  GURL expected_url(
+      std::string(kBraveUpdatesExtensionsEndpoint + query_string));
   int ret =
       OnBeforeURLRequest_CommonStaticRedirectWork(callback, before_url_context);
   EXPECT_EQ(GURL(before_url_context->new_url_spec), expected_url);
   EXPECT_EQ(ret, net::OK);
 }
 
-TEST_F(BraveCommonStaticRedirectNetworkDelegateHelperTest, NoModifyComponentUpdaterURL) {
+TEST_F(BraveCommonStaticRedirectNetworkDelegateHelperTest,
+       NoModifyComponentUpdaterURL) {
   net::TestDelegate test_delegate;
-  std::string query_string("?braveRedirect=true");
-  GURL url(std::string(component_updater::kUpdaterDefaultUrl) + query_string);
-  std::unique_ptr<net::URLRequest> request =
-      context()->CreateRequest(url, net::IDLE, &test_delegate,
-                             TRAFFIC_ANNOTATION_FOR_TESTS);
-  std::shared_ptr<brave::BraveRequestInfo>
-      before_url_context(new brave::BraveRequestInfo());
-  brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
+  GURL url(kComponentUpdaterProxy);
+  std::unique_ptr<net::URLRequest> request = context()->CreateRequest(
+      url, net::IDLE, &test_delegate, TRAFFIC_ANNOTATION_FOR_TESTS);
+  std::shared_ptr<brave::BraveRequestInfo> before_url_context(
+      new brave::BraveRequestInfo());
+  brave::BraveRequestInfo::FillCTXFromRequest(request.get(),
+                                              before_url_context);
   brave::ResponseCallback callback;
   GURL expected_url;
   int ret =

diff --git a/browser/net/brave_static_redirect_network_delegate_helper.cc b/browser/net/brave_static_redirect_network_delegate_helper.cc
@@ -1,9 +1,13 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 #include "brave/browser/net/brave_static_redirect_network_delegate_helper.h"
 
+#include <memory>
+#include <vector>
+
 #include "brave/common/network_constants.h"
 #include "extensions/common/url_pattern.h"
 
@@ -14,7 +18,16 @@ int OnBeforeURLRequest_StaticRedirectWork(
     std::shared_ptr<BraveRequestInfo> ctx) {
   GURL::Replacements replacements;
   static URLPattern geo_pattern(URLPattern::SCHEME_HTTPS, kGeoLocationsPattern);
-  static URLPattern safeBrowsing_pattern(URLPattern::SCHEME_HTTPS, kSafeBrowsingPrefix);
+  static URLPattern safeBrowsing_pattern(URLPattern::SCHEME_HTTPS,
+                                         kSafeBrowsingPrefix);
+  static URLPattern crlSet_pattern1(
+      URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kCRLSetPrefix1);
+  static URLPattern crlSet_pattern2(
+      URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kCRLSetPrefix2);
+  static URLPattern crlSet_pattern3(
+      URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kCRLSetPrefix3);
+  static URLPattern crxDownload_pattern(
+      URLPattern::SCHEME_HTTP | URLPattern::SCHEME_HTTPS, kCRXDownloadPrefix);
 
   if (geo_pattern.MatchesURL(ctx->request_url)) {
     ctx->new_url_spec = GURL(GOOGLEAPIS_ENDPOINT GOOGLEAPIS_API_KEY).spec();
@@ -27,58 +40,107 @@ int OnBeforeURLRequest_StaticRedirectWork(
     return net::OK;
   }
 
+  if (crxDownload_pattern.MatchesHost(ctx->request_url)) {
+    replacements.SetSchemeStr("https");
+    replacements.SetHostStr("crxdownload.brave.com");
+    ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
+    return net::OK;
+  }
+
+  if (crlSet_pattern1.MatchesHost(ctx->request_url)) {
+    replacements.SetSchemeStr("https");
+    replacements.SetHostStr("crlsets.brave.com");
+    ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
+    return net::OK;
+  }
+
+  if (crlSet_pattern2.MatchesHost(ctx->request_url)) {
+    replacements.SetSchemeStr("https");
+    replacements.SetHostStr("crlsets.brave.com");
+    ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
+    return net::OK;
+  }
+
+  if (crlSet_pattern3.MatchesHost(ctx->request_url)) {
+    replacements.SetSchemeStr("https");
+    replacements.SetHostStr("crlsets.brave.com");
+    ctx->new_url_spec = ctx->request_url.ReplaceComponents(replacements).spec();
+    return net::OK;
+  }
+
 #if !defined(NDEBUG)
   GURL gurl = ctx->request_url;
   static std::vector<URLPattern> allowed_patterns({
-    // Brave updates
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://go-updater.brave.com/*"),
-    // Brave promo referrals, production and staging (laptop-updates
-    // proxies to promo-services)
-    // TODO: In the future, we may want to specify the value of the
-    // BRAVE_REFERRALS_SERVER environment variable rather than
-    // hardcoding the server name here
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://laptop-updates.brave.com/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://laptop-updates-staging.herokuapp.com/*"),
-    // CRX file download
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://brave-core-ext.s3.brave.com/release/*"),
-    // Safe Browsing and other files
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://static.brave.com/*"),
-    // We do allow redirects to the Google update server for extensions we don't support
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://update.googleapis.com/service/update2"),
-
-    // Rewards URLs
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://ledger.mercury.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://balance.mercury.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://publishers.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://publishers-distro.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://ledger-staging.mercury.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://balance-staging.mercury.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://publishers-staging.basicattentiontoken.org/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://publishers-staging-distro.basicattentiontoken.org/*"),
-
-    // Safe browsing
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://safebrowsing.brave.com/v4/*"),
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://ssl.gstatic.com/safebrowsing/*"),
-
-    // Will be removed when https://github.com/brave/brave-browser/issues/663 is fixed
-    URLPattern(URLPattern::SCHEME_HTTPS, "https://www.gstatic.com/*"),
+      // Brave updates
+      URLPattern(URLPattern::SCHEME_HTTPS, "https://go-updater.brave.com/*"),
+      // Brave promo referrals, production and staging (laptop-updates
+      // proxies to promo-services)
+      // TODO(@emerick): In the future, we may want to specify the value of the
+      // BRAVE_REFERRALS_SERVER environment variable rather than
+      // hardcoding the server name here
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://laptop-updates.brave.com/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://laptop-updates-staging.herokuapp.com/*"),
+      // CRX file download
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://brave-core-ext.s3.brave.com/release/*"),
+      // Safe Browsing and other files
+      URLPattern(URLPattern::SCHEME_HTTPS, "https://static.brave.com/*"),
+      // We do allow redirects to the Google update server for extensions we
+      // don't
+      // support
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://update.googleapis.com/service/update2"),
+
+      // Rewards URLs
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://ledger.mercury.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://balance.mercury.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://publishers.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://publishers-distro.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://ledger-staging.mercury.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://balance-staging.mercury.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://publishers-staging.basicattentiontoken.org/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://publishers-staging-distro.basicattentiontoken.org/*"),
+
+      // Safe browsing
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://safebrowsing.brave.com/v4/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS,
+                 "https://ssl.gstatic.com/safebrowsing/*"),
+
+      URLPattern(URLPattern::SCHEME_HTTPS, "https://crlsets.brave.com/*"),
+      URLPattern(URLPattern::SCHEME_HTTPS, "https://crxdownload.brave.com/*"),
   });
-  // Check to make sure the URL being requested matches at least one of the allowed patterns
-  bool is_url_allowed = std::any_of(allowed_patterns.begin(), allowed_patterns.end(),
-    [&gurl](URLPattern pattern) {
-      if (pattern.MatchesURL(gurl)) {
-        return true;
-      }
-      return false;
-    });
+
+  // Check to make sure the URL being requested matches at least one of the
+  // allowed patterns
+  bool is_url_allowed =
+      std::any_of(allowed_patterns.begin(), allowed_patterns.end(),
+                  [&gurl](URLPattern pattern) {
+                    if (pattern.MatchesURL(gurl)) {
+                      return true;
+                    }
+                    return false;
+                  });
   if (!is_url_allowed) {
     LOG(ERROR) << "URL not allowed from system network delegate: " << gurl;
   }
-  // TODO: Before we can turn this into DCHECK we have to find a way to allow these, I think they are for Chrome Cast
+  // TODO(@bbondy): Before we can turn this into DCHECK we have to find a way to
+  // allow these, I think they are for Chrome Cast
   // http://192.168.0.13:8008/ssdp/device-desc.xml
-  // http://192.168.0.27:60000/upnp/dev/e16bf493-ed87-5798-ffff-ffffeb4f1c34/desc
-  // And also I don't know where they're from, but there's always 3 requests similar to this:
-  // http://vijscbncpv/
+  // http://192.168.0.27:60000/upnp/dev/e16bf493-ed87-5798-ffff-ffffeb4f1c34
+  // /desc
+  // And also I don't know where they're from, but there's always 3 requests
+  // similar to this: http://vijscbncpv/
 #endif
 
   return net::OK;

diff --git a/browser/net/brave_static_redirect_network_delegate_helper.h b/browser/net/brave_static_redirect_network_delegate_helper.h
@@ -1,9 +1,12 @@
-/* This Source Code Form is subject to the terms of the Mozilla Public
+/* Copyright (c) 2019 The Brave Authors. All rights reserved.
+ * This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this file,
  * You can obtain one at http://mozilla.org/MPL/2.0/. */
 
-#ifndef BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_H_
-#define BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_H_
+#ifndef BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_HELPER_H_
+#define BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_HELPER_H_
+
+#include <memory>
 
 #include "brave/browser/net/url_context.h"
 
@@ -17,4 +20,4 @@ int OnBeforeURLRequest_StaticRedirectWork(
 
 }  // namespace brave
 
-#endif  // BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_H_
+#endif  // BRAVE_BROWSER_NET_BRAVE_STATIC_REDIRECT_NETWORK_DELEGATE_HELPER_H_