Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block .onion addresses outside of Tor windows #8019

Closed
wants to merge 1 commit into from

Conversation

fmarier
Copy link
Member

@fmarier fmarier commented Feb 20, 2021

Resolves brave/brave-browser#14261

Submitter Checklist:

  • There is a ticket for my issue
  • Used Github auto-closing keywords in the PR description above
  • Wrote a good PR/commit description
  • Added appropriate labels (QA/Yes or QA/No; release-notes/include or release-notes/exclude; OS/...) to the associated issue
  • Checked the PR locally: npm run test -- brave_browser_tests, npm run test -- brave_unit_tests, npm run lint, npm run gn_check, npm run tslint
  • Ran git rebase master (if needed)
  • Requested a security/privacy review as needed

Reviewer Checklist:

  • New files have MPL-2.0 license header
  • Adequate test coverage exists to prevent regressions
  • Major classes, functions and non-trivial code blocks are well-commented
  • Changes in component dependencies are properly reflected in gn
  • Code follows the style guide
  • Test plan is specified in PR before merging

After-merge Checklist:

Test Plan:

  1. Start wireshark and filter for dns.
  2. Open a normal window and type https://brave5t5rjjg3s6k.onion/ in the URL bar.
  3. Confirm that you get an error page and that the .onion didn't result in a DNS lookup.
  4. Open a Tor window and type https://brave5t5rjjg3s6k.onion/ in the URL bar.
  5. Confirm that it opens fine.
  6. Open a normal window and type https://brave.com in the URL bar.
  7. Click on the "Open in Tor" link.
  8. Confirm that it opens a Tor window and displays the Brave website.

@fmarier fmarier self-assigned this Feb 20, 2021
@@ -170,6 +186,9 @@ int OnBeforeURLRequest_SiteHacksWork(const ResponseCallback& next_callback,
if (ctx->request_url.has_query()) {
ApplyPotentialQueryStringFilter(ctx);
}
if (ShouldBlockOnionAddress(ctx)) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be done in OnionLocationNavigationThrottle::WillStartRequest to prevent the request from being made

diff --git a/components/tor/onion_location_navigation_throttle.cc b/components/tor/onion_location_navigation_throttle.cc
index 482cea1bf2..cc030ca6da 100644
--- a/components/tor/onion_location_navigation_throttle.cc
+++ b/components/tor/onion_location_navigation_throttle.cc
@@ -97,10 +97,12 @@ OnionLocationNavigationThrottle::WillStartRequest() {
   // Open .onion site in Tor window
   if (!is_tor_profile_) {
     GURL url = navigation_handle()->GetURL();
-    if (url.SchemeIsHTTPOrHTTPS() && url.DomainIs("onion") &&
-        pref_service_->GetBoolean(prefs::kAutoOnionRedirect)) {
-      delegate_->OpenInTorWindow(navigation_handle()->GetWebContents(),
-                                 std::move(url));
+    if (url.SchemeIsHTTPOrHTTPS() && url.DomainIs("onion")) {
+      if (pref_service_->GetBoolean(prefs::kAutoOnionRedirect)) {
+        delegate_->OpenInTorWindow(navigation_handle()->GetWebContents(),
+                                   std::move(url));
+      }
+      return content::NavigationThrottle::BLOCK_REQUEST;
     }
   }
   return content::NavigationThrottle::PROCEED;

if (!ctx->request_url.has_host()) {
return false;
}
// TODO: don't block "Open in Tor" button (unless #if BUILDFLAG(ENABLE_TOR) is false)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

directly adding .onion address doesn't have "Open in Tor" button currently(only onion-location header), we can add that button for this case though.

@darkdh
Copy link
Member

darkdh commented Feb 22, 2021

superseded by #8040

@darkdh darkdh closed this Feb 22, 2021
@fmarier fmarier deleted the disable-onion-requests-14261 branch June 10, 2021 20:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

.onion request in regular window should also avoid DNS leakage
2 participants