From d12e67b6586196ce631dbb7b671d22299bdd1fed Mon Sep 17 00:00:00 2001 From: riastradh-brave <34034191+riastradh-brave@users.noreply.github.com> Date: Tue, 26 Feb 2019 19:08:53 +0000 Subject: [PATCH] Block URL requests in Tor windows if Tor is not set up right. (uplift to 0.62.x) (#1741) * Test network request in Tor profile with broken tor config. Should fail, but currently this is broken (#3058, #3349). * Block all external URL requests if tor is not set up right. Allow chrome, chrome-extension, and chrome-devtools URLs. This way we don't have to rely on other logic to prevent usage of the Tor profile before the daemon is configured, which turned out not be as reliable as hoped. fix #3058 fix #3349 * Appease lint and avoid C++ pitfalls. - Fix copyright headings to appease lint. - Fix header include guards to appease lint. - Fix missing includes. - Use explicit for unary constructors. - Fix whitespace to appease lint. --- .../net/brave_tor_network_delegate_helper.cc | 24 ++++--- ...ve_tor_network_delegate_helper_unittest.cc | 63 +++++++++++++++++-- browser/tor/mock_tor_profile_service_impl.cc | 21 +++++-- browser/tor/mock_tor_profile_service_impl.h | 16 ++--- browser/tor/tor_profile_service.h | 14 +++-- browser/tor/tor_profile_service_impl.cc | 21 +++++-- browser/tor/tor_profile_service_impl.h | 22 ++++--- common/tor/tor_test_constants.cc | 5 +- common/tor/tor_test_constants.h | 4 +- 9 files changed, 140 insertions(+), 50 deletions(-) diff --git a/browser/net/brave_tor_network_delegate_helper.cc b/browser/net/brave_tor_network_delegate_helper.cc index 6c5b34d951bc..5f536037a2df 100644 --- 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 + #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 index 44392ce9d646..b514fd0ab6b4 100644 --- a/browser/net/brave_tor_network_delegate_helper_unittest.cc +++ b/browser/net/brave_tor_network_delegate_helper_unittest.cc @@ -1,9 +1,14 @@ -/* 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 +#include +#include + #include "base/files/file_path.h" #include "base/files/scoped_temp_dir.h" #include "brave/browser/net/url_context.h" @@ -11,6 +16,7 @@ #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 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 navigation_ui_data = @@ -117,7 +125,8 @@ TEST_F(BraveTorNetworkDelegateHelperTest, TorProfile) { TRAFFIC_ANNOTATION_FOR_TESTS); std::shared_ptr 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 navigation_ui_data = @@ -163,7 +172,8 @@ TEST_F(BraveTorNetworkDelegateHelperTest, TorProfileBlockFile) { TRAFFIC_ANNOTATION_FOR_TESTS); std::shared_ptr 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 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 request = + context()->CreateRequest(url, net::IDLE, &test_delegate, + TRAFFIC_ANNOTATION_FOR_TESTS); + std::shared_ptr + before_url_context(new brave::BraveRequestInfo()); + brave::BraveRequestInfo::FillCTXFromRequest(request.get(), + before_url_context); + brave::ResponseCallback callback; + + std::unique_ptr navigation_ui_data = + std::make_unique(); + 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 index e2eb4c8962f4..e473b8771ee4 100644 --- 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 + #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 index 9812d0bac379..17edfaadc93d 100644 --- 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 index fbd617d004b7..295148bf5833 100644 --- 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 index 4a0a05acf27f..f4b64f9ff0f3 100644 --- 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 + #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 index a5934bff1dc3..44e8152ad706 100644 --- 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 + #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,8 +38,9 @@ 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(); @@ -44,17 +48,17 @@ class TorProfileServiceImpl : public TorProfileService, void NotifyTorLauncherCrashed(); void NotifyTorCrashed(int64_t pid); void NotifyTorLaunched(bool result, int64_t pid); - private: + private: void SetNewTorCircuitOnIOThread( const scoped_refptr&, 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_ diff --git a/common/tor/tor_test_constants.cc b/common/tor/tor_test_constants.cc index 900314271662..5daa7ed2dfc0 100644 --- a/common/tor/tor_test_constants.cc +++ b/common/tor/tor_test_constants.cc @@ -1,4 +1,5 @@ -/* 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/. */ @@ -11,6 +12,6 @@ namespace tor { const char kTestTorProxy[] = "socks5://127.0.0.1:9999"; const char kTestTorPacString[] = "SOCKS5 127.0.0.1:9999"; const base::FilePath::CharType kTestTorPath[] = FPL("."); - +const base::FilePath::CharType kTestBrokenTorPath[] = FPL(""); } // namespace tor diff --git a/common/tor/tor_test_constants.h b/common/tor/tor_test_constants.h index db8d620b1049..5c54f8652a2b 100644 --- a/common/tor/tor_test_constants.h +++ b/common/tor/tor_test_constants.h @@ -1,4 +1,5 @@ -/* 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/. */ @@ -12,6 +13,7 @@ namespace tor { extern const char kTestTorProxy[]; extern const char kTestTorPacString[]; extern const base::FilePath::CharType kTestTorPath[]; +extern const base::FilePath::CharType kTestBrokenTorPath[]; } // namespace tor