Navigation Predictor: Disable same-origin preconnect for non-public IPs.

Bug: 1129002
Change-Id: I5edbab61ab01498557ffa030f5bd5fd7a362346a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2413548
Commit-Queue: Tarun Bansal <tbansal@chromium.org>
Reviewed-by: Ryan Sturm <ryansturm@chromium.org>
Cr-Commit-Position: refs/heads/master@{#808153}

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.cc

@@ -9,6 +9,7 @@
 #include "base/bind.h"
 #include "base/feature_list.h"
 #include "base/metrics/field_trial_params.h"
+#include "base/metrics/histogram_macros.h"
 #include "base/time/time.h"
 #include "build/build_config.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_features.h"
@@ -23,6 +24,7 @@
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/web_contents.h"
 #include "net/base/features.h"
+#include "net/base/ip_address.h"
 
 namespace {
 
@@ -44,8 +46,22 @@ NavigationPredictorPreconnectClient::~NavigationPredictorPreconnectClient() =
 void NavigationPredictorPreconnectClient::DidFinishNavigation(
     content::NavigationHandle* navigation_handle) {
   if (!navigation_handle->IsInMainFrame() ||
-      !navigation_handle->HasCommitted() ||
-      (!base::FeatureList::IsEnabled(
+      !navigation_handle->HasCommitted()) {
+    return;
+  }
+
+  if (!navigation_handle->IsSameDocument()) {
+    is_publicly_routable_ = false;
+
+    base::Optional<bool> is_publicly_routable =
+        IsPubliclyRoutable(navigation_handle);
+
+    if (is_publicly_routable) {
+      is_publicly_routable_ = is_publicly_routable.value();
+    }
+  }
+
+  if ((!base::FeatureList::IsEnabled(
            features::
                kNavigationPredictorEnablePreconnectOnSameDocumentNavigations) &&
        navigation_handle->IsSameDocument())) {
@@ -141,6 +157,14 @@ void NavigationPredictorPreconnectClient::MaybePreconnectNow(
     return;
   }
 
+  UMA_HISTOGRAM_BOOLEAN("NavigationPredictor.IsPubliclyRoutable",
+                        is_publicly_routable_);
+
+  // Disable preconnecting to servers that are not publicly routable. These
+  // could likely be small IoT servers that may not support extra traffic.
+  if (!is_publicly_routable_)
+    return;
+
   auto* loading_predictor = predictors::LoadingPredictorFactory::GetForProfile(
       Profile::FromBrowserContext(browser_context_));
   GURL preconnect_url_serialized(preconnect_origin.Serialize());
@@ -163,7 +187,7 @@ void NavigationPredictorPreconnectClient::MaybePreconnectNow(
       FROM_HERE,
       base::TimeDelta::FromSeconds(base::GetFieldTrialParamByFeatureAsInt(
           net::features::kNetUnusedIdleSocketTimeout,
-          "unused_idle_socket_timeout_seconds", 60)) +
+          "unused_idle_socket_timeout_seconds", 10)) +
           base::TimeDelta::FromMilliseconds(retry_delay_ms),
       base::BindOnce(&NavigationPredictorPreconnectClient::MaybePreconnectNow,
                      base::Unretained(this), preconnects_attempted + 1));
@@ -178,4 +202,27 @@ bool NavigationPredictorPreconnectClient::IsSearchEnginePage() const {
       web_contents()->GetLastCommittedURL());
 }
 
+base::Optional<bool> NavigationPredictorPreconnectClient::IsPubliclyRoutable(
+    content::NavigationHandle* navigation_handle) const {
+  net::IPEndPoint remote_endpoint = navigation_handle->GetSocketAddress();
+  net::IPAddress page_ip_address_ = remote_endpoint.address();
+
+  // Sometimes the IP address may not be set (e.g., if the socket is being
+  // reused).
+  if (!page_ip_address_.IsValid()) {
+    return base::nullopt;
+  }
+
+  if (!enable_preconnects_for_local_ips_for_testing_) {
+    if (page_ip_address_.IsLoopback() ||
+        !page_ip_address_.IsPubliclyRoutable()) {
+      return false;
+    }
+  }
+  return true;
+}
+
+bool NavigationPredictorPreconnectClient::
+    enable_preconnects_for_local_ips_for_testing_ = false;
+
 WEB_CONTENTS_USER_DATA_KEY_IMPL(NavigationPredictorPreconnectClient)

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h

@@ -25,6 +25,12 @@ class NavigationPredictorPreconnectClient
  public:
   ~NavigationPredictorPreconnectClient() override;
 
+  static void EnablePreconnectsForLocalIPsForTesting(
+      bool enable_preconnects_for_local_ips) {
+    enable_preconnects_for_local_ips_for_testing_ =
+        enable_preconnects_for_local_ips;
+  }
+
  private:
   friend class content::WebContentsUserData<
       NavigationPredictorPreconnectClient>;
@@ -47,15 +53,26 @@ class NavigationPredictorPreconnectClient
   // MaybePreconnectNow preconnects to an origin server if it's allowed.
   void MaybePreconnectNow(size_t preconnects_attempted);
 
+  // Returns true if the origin is publicly routable.
+  base::Optional<bool> IsPubliclyRoutable(
+      content::NavigationHandle* navigation_handle) const;
+
   // Used to get keyed services.
   content::BrowserContext* const browser_context_;
 
+  // Set to true only if preconnects are allowed to local IPs. Defaulted to
+  // false. Set to true only for testing.
+  static bool enable_preconnects_for_local_ips_for_testing_;
+
   // Current visibility state of the web contents.
   content::Visibility current_visibility_;
 
   // Used to preconnect regularly.
   base::OneShotTimer timer_;
 
+  // Set to true if the origin is publicly routable.
+  bool is_publicly_routable_ = true;
+
   SEQUENCE_CHECKER(sequence_checker_);
 
   WEB_CONTENTS_USER_DATA_KEY_DECL();

chrome/browser/navigation_predictor/navigation_predictor_preconnect_client_browsertest.cc

@@ -11,6 +11,7 @@
 #include "chrome/browser/navigation_predictor/navigation_predictor_features.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service.h"
 #include "chrome/browser/navigation_predictor/navigation_predictor_keyed_service_factory.h"
+#include "chrome/browser/navigation_predictor/navigation_predictor_preconnect_client.h"
 #include "chrome/browser/navigation_predictor/search_engine_preconnector.h"
 #include "chrome/browser/predictors/loading_predictor.h"
 #include "chrome/browser/predictors/loading_predictor_factory.h"
@@ -56,6 +57,8 @@ class NavigationPredictorPreconnectClientBrowserTest
   void SetUpOnMainThread() override {
     subresource_filter::SubresourceFilterBrowserTest::SetUpOnMainThread();
     host_resolver()->ClearRules();
+    NavigationPredictorPreconnectClient::EnablePreconnectsForLocalIPsForTesting(
+        true);
 
     auto* loading_predictor =
         predictors::LoadingPredictorFactory::GetForProfile(
@@ -131,6 +134,7 @@ IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientBrowserTest,
 
 IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientBrowserTest,
                        PreconnectNotSearch) {
+  base::HistogramTester histogram_tester;
   const GURL& url = GetTestURL("/anchors_different_area.html");
 
   ui_test_utils::NavigateToURL(browser(), url);
@@ -138,6 +142,8 @@ IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientBrowserTest,
   // client.
   WaitForPreresolveCount(2);
   EXPECT_EQ(2, preresolve_done_count_);
+  histogram_tester.ExpectUniqueSample("NavigationPredictor.IsPubliclyRoutable",
+                                      true, 1);
 }
 
 IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientBrowserTest,
@@ -409,4 +415,32 @@ IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientBrowserTestWithSearch,
   EXPECT_EQ(4, preresolve_done_count_);
 }
 
+class NavigationPredictorPreconnectClientLocalURLBrowserTest
+    : public NavigationPredictorPreconnectClientBrowserTest {
+ public:
+  NavigationPredictorPreconnectClientLocalURLBrowserTest() = default;
+
+ private:
+  void SetUpOnMainThread() override {
+    NavigationPredictorPreconnectClientBrowserTest::SetUpOnMainThread();
+    NavigationPredictorPreconnectClient::EnablePreconnectsForLocalIPsForTesting(
+        false);
+  }
+
+  DISALLOW_COPY_AND_ASSIGN(
+      NavigationPredictorPreconnectClientLocalURLBrowserTest);
+};
+
+IN_PROC_BROWSER_TEST_F(NavigationPredictorPreconnectClientLocalURLBrowserTest,
+                       NoPreconnectSearch) {
+  base::HistogramTester histogram_tester;
+  const GURL& url = GetTestURL("/anchors_different_area.html");
+
+  ui_test_utils::NavigateToURL(browser(), url);
+
+  // There should not be any preconnects to non-public addresses.
+  histogram_tester.ExpectUniqueSample("NavigationPredictor.IsPubliclyRoutable",
+                                      false, 1);
+}
+
 }  // namespace

tools/metrics/histograms/histograms.xml

@@ -94445,6 +94445,17 @@ reviews. Googlers can read more about this at go/gwsq-gerrit.
   </summary>
 </histogram>
 
+<histogram name="NavigationPredictor.IsPubliclyRoutable" enum="Boolean"
+    expires_after="M90">
+  <owner>tbansal@chromium.org</owner>
+  <owner>ryansturm@chromium.org</owner>
+  <summary>
+    Set to true if the IP address of the origin of the main frame URL is
+    publically routable. Recorded everytime a preconnect attempt is made by the
+    navigation predictor.
+  </summary>
+</histogram>
+
 <histogram base="true" name="NavigationPredictor.LinkClickedPrerenderResult"
     enum="NavigationPredictorLinkClickedPrerenderResult" expires_after="M85">
   <owner>ryansturm@chromium.org</owner>