Commit 38637d36 authored by valenting's avatar valenting
Browse files

Bug 1824091 - Make sure OHTTP config gets reloaded on connectivity change if... 

Bug 1824091 - Make sure OHTTP config gets reloaded on connectivity change if first attempt failed. r=keeler,necko-reviewers,kershaw, a=dsmith

- Make sure the ObliviousHttpService clears the observers at shutdown
- Make the ObliviousHttpService reload the OHTTP config on connectivity change if first attempt failed
- Make the ObliviousHttpService reload the OHTTP config on TRR confirmation-fail if the first attempt failed

Differential Revision: https://phabricator.services.mozilla.com/D173412
parent db55e9d7

netwerk/dns/TRRService.cpp

+11 −0
Original line number Diff line number Diff line
@@ -660,6 +660,17 @@ void TRRService::ConfirmationContext::SetState( 
    enum ConfirmationState aNewState) {

  mState = aNewState;



  if (mState == CONFIRM_FAILED) {

    NS_DispatchToMainThread(

        NS_NewRunnableFunction("TRRService::ConfirmationContextRetry", [] {

          if (nsCOMPtr<nsIObserverService> obs =

                  mozilla::services::GetObserverService()) {

            obs->NotifyObservers(nullptr, "trrservice-confirmation-failed",

                                 nullptr);

          }

        }));

  }



  if (XRE_IsParentProcess()) {

    return;

  }

netwerk/protocol/http/ObliviousHttpService.cpp

+118 −49
Original line number Diff line number Diff line
@@ -10,9 +10,11 @@ 
#include "DNSUtils.h"

#include "ObliviousHttpChannel.h"

#include "mozilla/Base64.h"

#include "mozilla/StaticPrefs_network.h"

#include "nsIObserverService.h"

#include "nsIPrefService.h"

#include "nsNetUtil.h"

#include "nsPrintfCString.h"



namespace mozilla::net {
@@ -21,35 +23,52 @@ NS_IMPL_ISUPPORTS(ObliviousHttpService, nsIObliviousHttpService, nsIObserver, 


ObliviousHttpService::ObliviousHttpService()

    : mTRRConfig(ObliviousHttpConfig(), "ObliviousHttpService::mTRRConfig") {

  MOZ_ASSERT(NS_IsMainThread());



  nsCOMPtr<nsIPrefBranch> prefBranch(do_GetService(NS_PREFSERVICE_CONTRACTID));

  if (prefBranch) {

    prefBranch->AddObserver("network.trr.ohttp", this, false);

  }

  MOZ_ASSERT(NS_IsMainThread());



  if (nsCOMPtr<nsIObserverService> obs =

          mozilla::services::GetObserverService()) {

    obs->AddObserver(this, "xpcom-shutdown", false);

    obs->AddObserver(this, "network:captive-portal-connectivity-changed",

                     false);

    obs->AddObserver(this, "trrservice-confirmation-failed", false);

  }



  ReadPrefs("*"_ns);

}



void ObliviousHttpService::ReadPrefs(const nsACString& whichPref) {

  nsAutoCString relayURIPref("network.trr.ohttp.relay_uri");

  if (whichPref.Equals(relayURIPref) || whichPref.EqualsLiteral("*")) {

    nsAutoCString relayURIString;

    nsresult rv = Preferences::GetCString(relayURIPref.get(), relayURIString);

    if (NS_FAILED(rv)) {

static constexpr nsLiteralCString kTRRohttpConfigURIPref =

    "network.trr.ohttp.config_uri"_ns;

static constexpr nsLiteralCString kTRRohttpRelayURIPref =

    "network.trr.ohttp.relay_uri"_ns;



void ObliviousHttpService::FetchConfig(bool aConfigURIChanged) {

  auto scopeExit = MakeScopeExit([&] {

    nsCOMPtr<nsIObserverService> obs(mozilla::services::GetObserverService());

    if (!obs) {

      return;

    }

    nsCOMPtr<nsIURI> relayURI;

    rv = NS_NewURI(getter_AddRefs(relayURI), relayURIString);

    if (NS_FAILED(rv)) {

    obs->NotifyObservers(nullptr, "ohttp-service-config-loaded", u"no-changes");

  });



  {

    auto trrConfig = mTRRConfig.Lock();

    if (aConfigURIChanged) {

      // If the config URI has changed, we need to clear the config.

      trrConfig->mEncodedConfig.Clear();

    } else if (trrConfig->mEncodedConfig.Length()) {

      // The URI hasn't changed and we already have a config. No need to reload

      return;

    }

    auto trrConfig = mTRRConfig.Lock();

    trrConfig->mRelayURI = relayURI;

  }



  nsAutoCString configURIPref("network.trr.ohttp.config_uri");

  if (whichPref.Equals(configURIPref) || whichPref.EqualsLiteral("*")) {

  nsAutoCString configURIString;

    nsresult rv = Preferences::GetCString(configURIPref.get(), configURIString);

  nsresult rv =

      Preferences::GetCString(kTRRohttpConfigURIPref.get(), configURIString);

  if (NS_FAILED(rv)) {

    return;

  }
@@ -85,11 +104,38 @@ void ObliviousHttpService::ReadPrefs(const nsACString& whichPref) { 
    return;

  }

  rv = httpChannel->AsyncOpen(loader);



  if (NS_SUCCEEDED(rv)) {

    scopeExit.release();

    return;

  }



  nsPrintfCString msg(

      "ObliviousHttpService::FetchConfig AsyncOpen failed rv=%X",

      static_cast<uint32_t>(rv));

  NS_WARNING(msg.get());

}



void ObliviousHttpService::ReadPrefs(const nsACString& whichPref) {

  if (whichPref.Equals(kTRRohttpRelayURIPref) || whichPref.EqualsLiteral("*")) {

    nsAutoCString relayURIString;

    nsresult rv =

        Preferences::GetCString(kTRRohttpRelayURIPref.get(), relayURIString);

    if (NS_FAILED(rv)) {

      return;

    }

    nsCOMPtr<nsIURI> relayURI;

    rv = NS_NewURI(getter_AddRefs(relayURI), relayURIString);

    if (NS_FAILED(rv)) {

      return;

    }

    auto trrConfig = mTRRConfig.Lock();

    trrConfig->mEncodedConfig.Clear();

    trrConfig->mRelayURI = relayURI;

  }



  if (whichPref.Equals(kTRRohttpConfigURIPref) ||

      whichPref.EqualsLiteral("*")) {

    FetchConfig(true);

  }

}
@@ -124,6 +170,13 @@ ObliviousHttpService::GetTRRSettings(nsIURI** relayURI, 
  return NS_OK;

}



NS_IMETHODIMP

ObliviousHttpService::ClearTRRConfig() {

  auto trrConfig = mTRRConfig.Lock();

  trrConfig->mEncodedConfig.Clear();

  return NS_OK;

}



// nsIObserver



NS_IMETHODIMP
@@ -131,6 +184,21 @@ ObliviousHttpService::Observe(nsISupports* subject, const char* topic, 
                              const char16_t* data) {

  if (!strcmp(topic, NS_PREFBRANCH_PREFCHANGE_TOPIC_ID)) {

    ReadPrefs(NS_ConvertUTF16toUTF8(data));

  } else if (!strcmp(topic, "xpcom-shutdown")) {

    if (nsCOMPtr<nsIPrefBranch> prefBranch =

            do_GetService(NS_PREFSERVICE_CONTRACTID)) {

      prefBranch->RemoveObserver("network.trr.ohttp", this);

    }



    if (nsCOMPtr<nsIObserverService> obs =

            mozilla::services::GetObserverService()) {

      obs->RemoveObserver(this, "xpcom-shutdown");

      obs->RemoveObserver(this, "network:captive-portal-connectivity-changed");

      obs->RemoveObserver(this, "trrservice-confirmation-failed");

    }

  } else if (!strcmp(topic, "network:captive-portal-connectivity-changed") ||

             !strcmp(topic, "trrservice-confirmation-failed")) {

    FetchConfig(false);

  }

  return NS_OK;

}
@@ -154,7 +222,8 @@ ObliviousHttpService::OnStreamComplete(nsIStreamLoader* aLoader, 
    return NS_ERROR_FAILURE;

  }

  nsresult rv = observerService->NotifyObservers(

      nullptr, "ohttp-service-config-loaded", nullptr);

      nullptr, "ohttp-service-config-loaded",

      NS_SUCCEEDED(aStatus) ? u"success" : u"failed");

  if (NS_FAILED(rv)) {

    return rv;

  }

netwerk/protocol/http/ObliviousHttpService.h

+3 −2
Original line number Diff line number Diff line
@@ -23,8 +23,8 @@ class ObliviousHttpConfig { 
};



class ObliviousHttpService final : public nsIObliviousHttpService,

                                   nsIObserver,

                                   nsIStreamLoaderObserver {

                                   public nsIObserver,

                                   public nsIStreamLoaderObserver {

 public:

  NS_DECL_THREADSAFE_ISUPPORTS

  NS_DECL_NSIOBLIVIOUSHTTPSERVICE
@@ -36,6 +36,7 @@ class ObliviousHttpService final : public nsIObliviousHttpService, 
 private:

  ~ObliviousHttpService() = default;

  void ReadPrefs(const nsACString& whichPref);

  void FetchConfig(bool aConfigURIChanged);



  DataMutex<ObliviousHttpConfig> mTRRConfig;

};

netwerk/protocol/http/nsIObliviousHttp.idl

+3 −0
Original line number Diff line number Diff line
@@ -72,4 +72,7 @@ interface nsIObliviousHttpService : nsISupports 
  nsIChannel newChannel(in nsIURI relayURI, in nsIURI targetURI, in Array<octet> encodedConfig);



  void getTRRSettings(out nsIURI relayURI, out Array<octet> encodedConfig);



  // Clears the config

  void clearTRRConfig();

};

netwerk/test/unit/test_dooh.js

+102 −3
Original line number Diff line number Diff line
@@ -140,6 +140,7 @@ add_task(async function test_ohttp_not_configured() { 
});



add_task(async function set_ohttp_invalid_prefs() {

  let configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.prefs.setCharPref(

    "network.trr.ohttp.relay_uri",

    "http://nonexistent.test"
@@ -152,7 +153,7 @@ add_task(async function set_ohttp_invalid_prefs() { 
  Cc["@mozilla.org/network/oblivious-http-service;1"].getService(

    Ci.nsIObliviousHttpService

  );

  await TestUtils.topicObserved("ohttp-service-config-loaded");

  await configPromise;

});



// Test that if DNS-over-OHTTP has an invalid configuration, the implementation
@@ -164,6 +165,7 @@ add_task(async function test_ohttp_invalid_prefs_fallback() { 
});



add_task(async function set_ohttp_prefs_500_error() {

  let configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.prefs.setCharPref(

    "network.trr.ohttp.relay_uri",

    `http://localhost:${httpServer.identity.primaryPort}/relay`
@@ -172,7 +174,7 @@ add_task(async function set_ohttp_prefs_500_error() { 
    "network.trr.ohttp.config_uri",

    `http://localhost:${httpServer.identity.primaryPort}/500error`

  );

  await TestUtils.topicObserved("ohttp-service-config-loaded");

  await configPromise;

});



// Test that if DNS-over-OHTTP has an invalid configuration, the implementation
@@ -183,13 +185,110 @@ add_task(async function test_ohttp_500_error_fallback() { 
  await new TRRDNSListener("example.com", "127.0.0.1");

});



add_task(async function retryConfigOnConnectivityChange() {

  Services.prefs.setCharPref("network.trr.confirmationNS", "skip");

  // First we make sure the config is properly loaded

  setModeAndURI(2, "doh?responseIP=2.2.2.2");

  let ohttpService = Cc[

    "@mozilla.org/network/oblivious-http-service;1"

  ].getService(Ci.nsIObliviousHttpService);

  ohttpService.clearTRRConfig();

  ohttpEncodedConfig = bytesToString(ohttpServer.encodedConfig);

  let configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.prefs.setCharPref(

    "network.trr.ohttp.relay_uri",

    `http://localhost:${httpServer.identity.primaryPort}/relay`

  );

  Services.prefs.setCharPref(

    "network.trr.ohttp.config_uri",

    `http://localhost:${httpServer.identity.primaryPort}/config`

  );

  let [, status] = await configPromise;

  equal(status, "success");

  info("retryConfigOnConnectivityChange setup complete");



  ohttpService.clearTRRConfig();



  let port = httpServer.identity.primaryPort;

  // Stop the server so getting the config fails.

  await httpServer.stop();



  configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.obs.notifyObservers(

    null,

    "network:captive-portal-connectivity-changed"

  );

  [, status] = await configPromise;

  equal(status, "failed");



  // Should fallback to native DNS since the config is empty

  Services.dns.clearCache(true);

  await new TRRDNSListener("example.com", "127.0.0.1");



  // Start the server back again.

  httpServer.start(port);

  Assert.equal(

    port,

    httpServer.identity.primaryPort,

    "server should get the same port"

  );



  // Still the config hasn't been reloaded.

  await new TRRDNSListener("example2.com", "127.0.0.1");



  // Signal a connectivity change so we reload the config

  configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.obs.notifyObservers(

    null,

    "network:captive-portal-connectivity-changed"

  );

  [, status] = await configPromise;

  equal(status, "success");



  await new TRRDNSListener("example3.com", "2.2.2.2");



  // Now check that we also reload a missing config if a TRR confirmation fails.

  ohttpService.clearTRRConfig();

  configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.obs.notifyObservers(null, "trrservice-confirmation-failed");

  [, status] = await configPromise;

  equal(status, "success");

  await new TRRDNSListener("example4.com", "2.2.2.2");



  // set the config to an invalid value and check that as long as the URL

  // doesn't change, we dont reload it again on connectivity notifications.

  ohttpEncodedConfig = "not a valid config";

  configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.obs.notifyObservers(

    null,

    "network:captive-portal-connectivity-changed"

  );



  await new TRRDNSListener("example5.com", "2.2.2.2");



  // The change should not cause any config reload because we already have a config.

  [, status] = await configPromise;

  equal(status, "no-changes");



  await new TRRDNSListener("example6.com", "2.2.2.2");

  // Clear the config_uri pref so it gets set to the proper value in the next test.

  configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  Services.prefs.setCharPref("network.trr.ohttp.config_uri", ``);

  await configPromise;

});



add_task(async function set_ohttp_prefs_valid() {

  let ohttpService = Cc[

    "@mozilla.org/network/oblivious-http-service;1"

  ].getService(Ci.nsIObliviousHttpService);

  ohttpService.clearTRRConfig();

  let configPromise = TestUtils.topicObserved("ohttp-service-config-loaded");

  ohttpEncodedConfig = bytesToString(ohttpServer.encodedConfig);

  Services.prefs.setCharPref(

    "network.trr.ohttp.config_uri",

    `http://localhost:${httpServer.identity.primaryPort}/config`

  );

  await TestUtils.topicObserved("ohttp-service-config-loaded");

  await configPromise;

});



add_task(test_A_record);