Merge pull request #1679 from brave/riastradh-3349-blockiftorhosed

Block URL requests in Tor windows if Tor is not set up right.
brave · Feb 20, 2019 · a48af9c · a48af9c
2 parents 9ffaf67 + 0471a50
commit a48af9c
Show file tree

Hide file tree

Showing 9 changed files with 140 additions and 50 deletions.
diff --git a/browser/net/brave_tor_network_delegate_helper.cc b/browser/net/brave_tor_network_delegate_helper.cc
@@ -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/. */
 
 #include "brave/browser/net/brave_tor_network_delegate_helper.h"
 
+#include <memory>
+
 #include "brave/browser/renderer_host/brave_navigation_ui_data.h"
 #include "brave/browser/tor/tor_profile_service.h"
 #include "content/public/browser/browser_thread.h"
@@ -40,17 +43,18 @@ int OnBeforeURLRequest_TorWork(
     return net::OK;
   }
 
-  if (!(ctx->request_url.SchemeIsHTTPOrHTTPS() ||
-        ctx->request_url.SchemeIs(content::kChromeUIScheme) ||
-        ctx->request_url.SchemeIs(extensions::kExtensionScheme) ||
-        ctx->request_url.SchemeIs(content::kChromeDevToolsScheme))) {
+  auto& request_url = ctx->request_url;
+  if (request_url.SchemeIsHTTPOrHTTPS()) {
+    auto* proxy_service = ctx->request->context()->proxy_resolution_service();
+    return tor_profile_service->SetProxy(proxy_service, request_url, false);
+  } else if (request_url.SchemeIs(content::kChromeUIScheme) ||
+             request_url.SchemeIs(extensions::kExtensionScheme) ||
+             request_url.SchemeIs(content::kChromeDevToolsScheme)) {
+    // No proxy for internal schemes.
+    return net::OK;
+  } else {
     return net::ERR_DISALLOWED_URL_SCHEME;
   }
-
-  auto* proxy_service = ctx->request->context()->proxy_resolution_service();
-  tor_profile_service->SetProxy(proxy_service, ctx->request_url, false);
-
-  return net::OK;
 }
 
 }  // namespace brave
diff --git a/browser/net/brave_tor_network_delegate_helper_unittest.cc b/browser/net/brave_tor_network_delegate_helper_unittest.cc
@@ -1,16 +1,22 @@
-/* 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_tor_network_delegate_helper.h"
 
+#include <memory>
+#include <string>
+#include <utility>
+
 #include "base/files/file_path.h"
 #include "base/files/scoped_temp_dir.h"
 #include "brave/browser/net/url_context.h"
 #include "brave/browser/profiles/brave_profile_manager.h"
 #include "brave/browser/profiles/tor_unittest_profile_manager.h"
 #include "brave/browser/renderer_host/brave_navigation_ui_data.h"
 #include "brave/browser/tor/mock_tor_profile_service_factory.h"
+#include "brave/common/tor/tor_common.h"
 #include "brave/common/tor/tor_test_constants.h"
 #include "chrome/test/base/chrome_render_view_host_test_harness.h"
 #include "chrome/test/base/scoped_testing_local_state.h"
@@ -58,6 +64,7 @@ class BraveTorNetworkDelegateHelperTest: public testing::Test {
   content::MockResourceContext* resource_context() {
     return resource_context_.get();
   }
+
  protected:
   // The path to temporary directory used to contain the test operations.
   base::ScopedTempDir temp_dir_;
@@ -78,7 +85,8 @@ TEST_F(BraveTorNetworkDelegateHelperTest, NotTorProfile) {
                              TRAFFIC_ANNOTATION_FOR_TESTS);
   std::shared_ptr<brave::BraveRequestInfo>
       before_url_context(new brave::BraveRequestInfo());
-  brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
+  brave::BraveRequestInfo::FillCTXFromRequest(request.get(),
+                                              before_url_context);
   brave::ResponseCallback callback;
 
   std::unique_ptr<BraveNavigationUIData> navigation_ui_data =
@@ -117,7 +125,8 @@ TEST_F(BraveTorNetworkDelegateHelperTest, TorProfile) {
                              TRAFFIC_ANNOTATION_FOR_TESTS);
   std::shared_ptr<brave::BraveRequestInfo>
       before_url_context(new brave::BraveRequestInfo());
-  brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
+  brave::BraveRequestInfo::FillCTXFromRequest(request.get(),
+                                              before_url_context);
   brave::ResponseCallback callback;
 
   std::unique_ptr<BraveNavigationUIData> navigation_ui_data =
@@ -163,7 +172,8 @@ TEST_F(BraveTorNetworkDelegateHelperTest, TorProfileBlockFile) {
                              TRAFFIC_ANNOTATION_FOR_TESTS);
   std::shared_ptr<brave::BraveRequestInfo>
       before_url_context(new brave::BraveRequestInfo());
-  brave::BraveRequestInfo::FillCTXFromRequest(request.get(), before_url_context);
+  brave::BraveRequestInfo::FillCTXFromRequest(request.get(),
+                                              before_url_context);
   brave::ResponseCallback callback;
 
   std::unique_ptr<BraveNavigationUIData> navigation_ui_data =
@@ -183,3 +193,48 @@ TEST_F(BraveTorNetworkDelegateHelperTest, TorProfileBlockFile) {
   EXPECT_TRUE(before_url_context->new_url_spec.empty());
   EXPECT_EQ(ret, net::ERR_DISALLOWED_URL_SCHEME);
 }
+
+TEST_F(BraveTorNetworkDelegateHelperTest, TorProfileBlockIfHosed) {
+  ProfileManager* profile_manager = g_browser_process->profile_manager();
+  base::FilePath tor_path = BraveProfileManager::GetTorProfilePath();
+
+  Profile* profile = profile_manager->GetProfile(tor_path);
+  ASSERT_TRUE(profile);
+
+  net::TestDelegate test_delegate;
+  GURL url("https://check.torproject.org/");
+  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;
+
+  std::unique_ptr<BraveNavigationUIData> navigation_ui_data =
+    std::make_unique<BraveNavigationUIData>();
+  BraveNavigationUIData* navigation_ui_data_ptr = navigation_ui_data.get();
+  content::ResourceRequestInfo::AllocateForTesting(
+    request.get(), content::RESOURCE_TYPE_MAIN_FRAME, resource_context(),
+    kRenderProcessId, /*render_view_id=*/-1, kRenderFrameId,
+    /*is_main_frame=*/true, /*allow_download=*/false, /*is_async=*/true,
+    content::PREVIEWS_OFF, std::move(navigation_ui_data));
+
+  MockTorProfileServiceFactory::SetTorNavigationUIData(profile,
+                                                       navigation_ui_data_ptr);
+
+  // `Relaunch' tor with broken config.
+  {
+    auto* tor_profile_service = navigation_ui_data_ptr->GetTorProfileService();
+    base::FilePath path(tor::kTestBrokenTorPath);
+    std::string proxy(tor::kTestTorProxy);
+    tor_profile_service->ReLaunchTor(tor::TorConfig(path, proxy));
+  }
+
+  int ret =
+    brave::OnBeforeURLRequest_TorWork(callback,
+                                      before_url_context);
+  EXPECT_TRUE(before_url_context->new_url_spec.empty());
+  EXPECT_NE(ret, net::OK);
+}
diff --git a/browser/tor/mock_tor_profile_service_impl.cc b/browser/tor/mock_tor_profile_service_impl.cc
@@ -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/. */
 
 #include "brave/browser/tor/mock_tor_profile_service_impl.h"
 
+#include <string>
+
 #include "brave/browser/tor/tor_proxy_config_service.h"
 #include "brave/common/tor/tor_test_constants.h"
 #include "chrome/browser/profiles/profile.h"
@@ -27,7 +30,9 @@ MockTorProfileServiceImpl::~MockTorProfileServiceImpl() {}
 
 void MockTorProfileServiceImpl::LaunchTor(const TorConfig& config) {}
 
-void MockTorProfileServiceImpl::ReLaunchTor(const TorConfig& config) {}
+void MockTorProfileServiceImpl::ReLaunchTor(const TorConfig& config) {
+  config_ = config;
+}
 
 
 void MockTorProfileServiceImpl::SetNewTorCircuit(const GURL& request_url,
@@ -39,15 +44,21 @@ const TorConfig& MockTorProfileServiceImpl::GetTorConfig() {
 
 int64_t MockTorProfileServiceImpl::GetTorPid() { return -1; }
 
-void MockTorProfileServiceImpl::SetProxy(
+int MockTorProfileServiceImpl::SetProxy(
     net::ProxyResolutionService* service, const GURL& request_url,
     bool new_circuit) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   GURL url = SiteInstance::GetSiteForURL(profile_, request_url);
-  if (url.host().empty() || config_.empty())
-    return;
+  if (config_.empty()) {
+    // No tor config => we absolutely cannot talk to the network.
+    // This might mean that there was a problem trying to initialize
+    // Tor.
+    LOG(ERROR) << "Tor not configured -- blocking connection";
+    return net::ERR_SOCKS_CONNECTION_FAILED;
+  }
   TorProxyConfigService::TorSetProxy(service, config_.proxy_string(),
                                      url.host(), nullptr, new_circuit);
+  return net::OK;
 }
 
 }  // namespace tor
diff --git a/browser/tor/mock_tor_profile_service_impl.h b/browser/tor/mock_tor_profile_service_impl.h
@@ -1,9 +1,10 @@
-/* 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_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_
-#define BRAVE_BROWSER_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_
+#ifndef BRAVE_BROWSER_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_H_
+#define BRAVE_BROWSER_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_H_
 
 #include "brave/browser/tor/tor_profile_service.h"
 
@@ -13,7 +14,7 @@ namespace tor {
 
 class MockTorProfileServiceImpl : public TorProfileService {
  public:
-  MockTorProfileServiceImpl(Profile* profile);
+  explicit MockTorProfileServiceImpl(Profile* profile);
   ~MockTorProfileServiceImpl() override;
 
   // TorProfileService:
@@ -23,8 +24,9 @@ class MockTorProfileServiceImpl : public TorProfileService {
   const TorConfig& GetTorConfig() override;
   int64_t GetTorPid() override;
 
-  void SetProxy(net::ProxyResolutionService*, const GURL& request_url,
-                bool new_circuit) override;
+  int SetProxy(net::ProxyResolutionService*,
+               const GURL& request_url,
+               bool new_circuit) override;
 
  private:
   Profile* profile_;  // NOT OWNED
@@ -34,4 +36,4 @@ class MockTorProfileServiceImpl : public TorProfileService {
 
 }  // namespace tor
 
-#endif  // BRAVE_BROWSER_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_
+#endif  // BRAVE_BROWSER_TOR_MOCK_TOR_PROFILE_SERVICE_IMPL_H_
diff --git a/browser/tor/tor_profile_service.h b/browser/tor/tor_profile_service.h
@@ -1,9 +1,10 @@
-/* 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_TOR_TOR_PROFILE_SERVICE_
-#define BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_
+#ifndef BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_H_
+#define BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_H_
 
 #include "base/macros.h"
 #include "base/observer_list.h"
@@ -40,8 +41,9 @@ class TorProfileService : public KeyedService {
   virtual const TorConfig& GetTorConfig() = 0;
   virtual int64_t GetTorPid() = 0;
 
-  virtual void SetProxy(net::ProxyResolutionService*, const GURL& request_url,
-                        bool new_circuit) = 0;
+  virtual int SetProxy(net::ProxyResolutionService*,
+                       const GURL& request_url,
+                       bool new_circuit) = 0;
 
   void AddObserver(TorLauncherServiceObserver* observer);
   void RemoveObserver(TorLauncherServiceObserver* observer);
@@ -55,4 +57,4 @@ class TorProfileService : public KeyedService {
 
 }  // namespace tor
 
-#endif  // BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_
+#endif  // BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_H_
diff --git a/browser/tor/tor_profile_service_impl.cc b/browser/tor/tor_profile_service_impl.cc
@@ -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/. */
 
 #include "brave/browser/tor/tor_profile_service_impl.h"
 
+#include <string>
+
 #include "base/bind.h"
 #include "base/task/post_task.h"
 #include "brave/browser/tor/tor_launcher_service_observer.h"
@@ -81,7 +84,6 @@ void TorProfileServiceImpl::SetNewTorCircuit(const GURL& request_url,
                  base::WrapRefCounted(url_request_context_getter),
                  url.host()),
     callback);
-
 }
 
 const TorConfig& TorProfileServiceImpl::GetTorConfig() {
@@ -92,17 +94,24 @@ int64_t TorProfileServiceImpl::GetTorPid() {
   return tor_launcher_factory_->GetTorPid();
 }
 
-void TorProfileServiceImpl::SetProxy(net::ProxyResolutionService* service,
-                                     const GURL& request_url,bool new_circuit) {
+int TorProfileServiceImpl::SetProxy(net::ProxyResolutionService* service,
+                                    const GURL& request_url,
+                                    bool new_circuit) {
   DCHECK_CURRENTLY_ON(BrowserThread::IO);
   const TorConfig tor_config = tor_launcher_factory_->GetTorConfig();
   GURL url = SiteInstance::GetSiteForURL(profile_, request_url);
-  if (url.host().empty() || tor_config.empty())
-    return;
+  if (tor_config.empty()) {
+    // No tor config => we absolutely cannot talk to the network.
+    // This might mean that there was a problem trying to initialize
+    // Tor.
+    LOG(ERROR) << "Tor not configured -- blocking connection";
+    return net::ERR_SOCKS_CONNECTION_FAILED;
+  }
   base::PostTaskWithTraits(FROM_HERE, {BrowserThread::IO},
       base::Bind(&TorProxyConfigService::TorSetProxy,
       service, tor_config.proxy_string(),
       url.host(), &tor_proxy_map_, new_circuit));
+  return net::OK;
 }
 
 void TorProfileServiceImpl::KillTor() {

diff --git a/browser/tor/tor_profile_service_impl.h b/browser/tor/tor_profile_service_impl.h
@@ -1,12 +1,15 @@
-/* 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_TOR_TOR_PROFILE_SERVICE_IMPL_
-#define BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_IMPL_
+#ifndef BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_IMPL_H_
+#define BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_IMPL_H_
 
 #include "brave/browser/tor/tor_profile_service.h"
 
+#include <string>
+
 #include "base/memory/scoped_refptr.h"
 #include "brave/browser/tor/tor_launcher_factory.h"
 #include "brave/browser/tor/tor_proxy_config_service.h"
@@ -22,7 +25,7 @@ namespace tor {
 class TorProfileServiceImpl : public TorProfileService,
                               public base::CheckedObserver {
  public:
-  TorProfileServiceImpl(Profile* profile);
+  explicit TorProfileServiceImpl(Profile* profile);
   ~TorProfileServiceImpl() override;
 
   // KeyedService:
@@ -35,26 +38,27 @@ class TorProfileServiceImpl : public TorProfileService,
   const TorConfig& GetTorConfig() override;
   int64_t GetTorPid() override;
 
-  void SetProxy(net::ProxyResolutionService*, const GURL& request_url,
-                bool new_circuit) override;
+  int SetProxy(net::ProxyResolutionService*,
+               const GURL& request_url,
+               bool new_circuit) override;
 
   void KillTor();
 
   // For internal observer
   void NotifyTorLauncherCrashed();
   void NotifyTorCrashed(int64_t pid);
   void NotifyTorLaunched(bool result, int64_t pid);
- private:
 
+ private:
   void SetNewTorCircuitOnIOThread(
       const scoped_refptr<net::URLRequestContextGetter>&, std::string);
 
   Profile* profile_;  // NOT OWNED
-  TorLauncherFactory* tor_launcher_factory_; // Singleton
+  TorLauncherFactory* tor_launcher_factory_;  // Singleton
   TorProxyConfigService::TorProxyMap tor_proxy_map_;
   DISALLOW_COPY_AND_ASSIGN(TorProfileServiceImpl);
 };
 
 }  // namespace tor
 
-#endif  // BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_IMPL_
+#endif  // BRAVE_BROWSER_TOR_TOR_PROFILE_SERVICE_IMPL_H_