Implement IceConfigFetcherCloud

This CL implements the IceConfigFetcherCloud class, adds tests for it, and wires it up in remoting_me2me_host. I've also updated some of the code in IceConfig to remove a test-only helper which wasn't useful and exposed one of the helpers as a class method so parsing can be moved to the use-case specific files instead of IceConfig having to know about every proto.

Lastly I've also added a variant of the IceConfigFetcher for Corp, I'll implement that in a follow-up.

Bug: 325448647
Change-Id: If957095a6d67977d08a61b02ec3d78a5686cef32
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5924563
Reviewed-by: Yuwei Huang <yuweih@chromium.org>
Commit-Queue: Joe Downing <joedow@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1367807}

Author: joedow-42

remoting/host/remoting_me2me_host.cc

@@ -115,6 +115,8 @@
 #include "remoting/protocol/channel_authenticator.h"
 #include "remoting/protocol/chromium_port_allocator_factory.h"
 #include "remoting/protocol/host_authentication_config.h"
+#include "remoting/protocol/ice_config_fetcher_cloud.h"
+#include "remoting/protocol/ice_config_fetcher_corp.h"
 #include "remoting/protocol/ice_config_fetcher_default.h"
 #include "remoting/protocol/jingle_session_manager.h"
 #include "remoting/protocol/me2me_host_authenticator_factory.h"
@@ -1819,9 +1821,21 @@ void HostProcess::StartHost() {
 
   InitializeSignaling();
 
-  // TODO: joedow - Create Cloud/Corp/Me2Me fetchers based on host config hint.
-  auto ice_config_fetcher = std::make_unique<protocol::IceConfigFetcherDefault>(
-      context_->url_loader_factory(), oauth_token_getter_.get());
+  // Create the appropriate API service client (corp, cloud, or me2me) for the
+  // IceConfigFetcher.
+  std::unique_ptr<protocol::IceConfigFetcher> ice_config_fetcher;
+  if (is_cloud_host_) {
+    ice_config_fetcher = std::make_unique<protocol::IceConfigFetcherCloud>(
+        context_->url_loader_factory(), oauth_token_getter_.get());
+    // TODO: joedow - Implement IceConfigFetcherCorp.
+    // } else if (is_corp_host_) {
+    // ice_config_fetcher = std::make_unique<protocol::IceConfigFetcherCorp>(
+    //     context_->url_loader_factory(), oauth_token_getter_.get());
+  } else {
+    ice_config_fetcher = std::make_unique<protocol::IceConfigFetcherDefault>(
+        context_->url_loader_factory(), oauth_token_getter_.get());
+  }
+
   scoped_refptr<protocol::TransportContext> transport_context =
       new protocol::TransportContext(
           std::make_unique<protocol::ChromiumPortAllocatorFactory>(),

remoting/protocol/BUILD.gn

@@ -99,6 +99,10 @@ static_library("protocol") {
     "ice_config.cc",
     "ice_config.h",
     "ice_config_fetcher.h",
+    "ice_config_fetcher_cloud.cc",
+    "ice_config_fetcher_cloud.h",
+    "ice_config_fetcher_corp.cc",
+    "ice_config_fetcher_corp.h",
     "ice_config_fetcher_default.cc",
     "ice_config_fetcher_default.h",
     "ice_connection_to_host.cc",
@@ -249,6 +253,7 @@ static_library("protocol") {
     "//remoting/base:session_policies",
     "//remoting/codec:decoder",
     "//remoting/proto:internal_structs",
+    "//remoting/proto/google/internal/remoting/cloud/v1alpha:messages",
     "//remoting/proto/remoting/v1:network_traversal_proto",
     "//remoting/signaling",
     "//services/network/public/cpp:cpp",
@@ -397,6 +402,7 @@ source_set("unit_tests") {
     "data_channel_manager_unittest.cc",
     "fractional_input_filter_unittest.cc",
     "frame_stats_unittest.cc",
+    "ice_config_fetcher_cloud_unittest.cc",
     "ice_config_fetcher_default_unittest.cc",
     "ice_config_unittest.cc",
     "ice_transport_unittest.cc",
@@ -446,6 +452,7 @@ source_set("unit_tests") {
     "//remoting/base:test_support",
     "//remoting/codec:encoder",
     "//remoting/proto:internal_structs",
+    "//remoting/proto/google/internal/remoting/cloud/v1alpha:messages",
     "//remoting/proto/remoting/v1:network_traversal_proto",
     "//remoting/signaling:test_support",
     "//services/network/public/cpp:cpp",

remoting/protocol/ice_config.cc

@@ -11,6 +11,7 @@
 #include "base/logging.h"
 #include "base/notreached.h"
 #include "base/strings/string_number_conversions.h"
+#include "base/strings/string_split.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "net/base/url_util.h"
@@ -34,69 +35,6 @@ bool ParseLifetime(const std::string& string, base::TimeDelta* result) {
   return true;
 }
 
-// Parses url in form of <stun|turn|turns>:<host>[:<port>][?transport=<udp|tcp>]
-// and adds an entry to the |config|.
-bool AddServerToConfig(std::string url,
-                       const std::string& username,
-                       const std::string& password,
-                       IceConfig* config) {
-  cricket::ProtocolType turn_transport_type = cricket::PROTO_LAST;
-
-  const char kTcpTransportSuffix[] = "?transport=tcp";
-  const char kUdpTransportSuffix[] = "?transport=udp";
-  if (base::EndsWith(url, kTcpTransportSuffix,
-                     base::CompareCase::INSENSITIVE_ASCII)) {
-    turn_transport_type = cricket::PROTO_TCP;
-    url.resize(url.size() - strlen(kTcpTransportSuffix));
-  } else if (base::EndsWith(url, kUdpTransportSuffix,
-                            base::CompareCase::INSENSITIVE_ASCII)) {
-    turn_transport_type = cricket::PROTO_UDP;
-    url.resize(url.size() - strlen(kUdpTransportSuffix));
-  }
-
-  size_t colon_pos = url.find(':');
-  if (colon_pos == std::string::npos) {
-    return false;
-  }
-
-  std::string protocol = url.substr(0, colon_pos);
-
-  std::string host;
-  int port;
-  if (!net::ParseHostAndPort(url.substr(colon_pos + 1), &host, &port)) {
-    return false;
-  }
-
-  if (protocol == "stun") {
-    if (port == -1) {
-      port = kDefaultStunTurnPort;
-    }
-    config->stun_servers.emplace_back(host, port);
-  } else if (protocol == "turn") {
-    if (port == -1) {
-      port = kDefaultStunTurnPort;
-    }
-    if (turn_transport_type == cricket::PROTO_LAST) {
-      turn_transport_type = cricket::PROTO_UDP;
-    }
-    config->turn_servers.emplace_back(host, port, username, password,
-                                      turn_transport_type, false);
-  } else if (protocol == "turns") {
-    if (port == -1) {
-      port = kDefaultTurnsPort;
-    }
-    if (turn_transport_type == cricket::PROTO_LAST) {
-      turn_transport_type = cricket::PROTO_TCP;
-    }
-    config->turn_servers.emplace_back(host, port, username, password,
-                                      turn_transport_type, true);
-  } else {
-    return false;
-  }
-
-  return true;
-}
-
 // Returns the smallest specified value, or 0 if neither is specified.
 // A value is "specified" if it is greater than 0.
 int MinimumSpecified(int value1, int value2) {
@@ -120,6 +58,11 @@ IceConfig::~IceConfig() = default;
 
 // static
 IceConfig IceConfig::Parse(const base::Value::Dict& dictionary) {
+  const base::Value::Dict* data = dictionary.FindDict("data");
+  if (data) {
+    return Parse(*data);
+  }
+
   const base::Value::List* ice_servers_list = dictionary.FindList("iceServers");
   if (!ice_servers_list) {
     return IceConfig();
@@ -185,7 +128,7 @@ IceConfig IceConfig::Parse(const base::Value::Dict& dictionary) {
         errors_found = true;
         continue;
       }
-      if (!AddServerToConfig(*url_str, username, password, &ice_config)) {
+      if (!ice_config.AddServer(*url_str, username, password)) {
         LOG(ERROR) << "Invalid ICE server URL: " << *url_str;
       }
     }
@@ -210,30 +153,6 @@ IceConfig IceConfig::Parse(const base::Value::Dict& dictionary) {
   return ice_config;
 }
 
-// static
-IceConfig IceConfig::Parse(const std::string& config_json) {
-  std::optional<base::Value> json = base::JSONReader::Read(config_json);
-  if (!json) {
-    return IceConfig();
-  }
-
-  base::Value::Dict* dictionary = json->GetIfDict();
-  if (!dictionary) {
-    return IceConfig();
-  }
-
-  // Handle the case when the config is wrapped in 'data', i.e. as {'data': {
-  // 'iceServers': {...} }}.
-  if (!dictionary->Find("iceServers")) {
-    base::Value::Dict* data_dictionary = dictionary->FindDict("data");
-    if (data_dictionary) {
-      return Parse(*data_dictionary);
-    }
-  }
-
-  return Parse(*dictionary);
-}
-
 // static
 IceConfig IceConfig::Parse(const apis::v1::GetIceConfigResponse& config) {
   IceConfig ice_config;
@@ -255,8 +174,7 @@ IceConfig IceConfig::Parse(const apis::v1::GetIceConfigResponse& config) {
         MinimumSpecified(ice_config.max_bitrate_kbps, server.max_rate_kbps());
 
     for (const auto& url : server.urls()) {
-      if (!AddServerToConfig(url, server.username(), server.credential(),
-                             &ice_config)) {
+      if (!ice_config.AddServer(url, server.username(), server.credential())) {
         LOG(ERROR) << "Invalid ICE server URL: " << url;
       }
     }
@@ -271,4 +189,68 @@ IceConfig IceConfig::Parse(const apis::v1::GetIceConfigResponse& config) {
   return ice_config;
 }
 
+bool IceConfig::AddStunServer(std::string_view url) {
+  CHECK(url.starts_with("stun:"));
+  return AddServer(url, /*username=*/"", /*password=*/"");
+}
+
+bool IceConfig::AddServer(std::string_view url,
+                          const std::string& username,
+                          const std::string& password) {
+  cricket::ProtocolType turn_transport_type = cricket::PROTO_LAST;
+
+  const char kTcpTransportSuffix[] = "?transport=tcp";
+  const char kUdpTransportSuffix[] = "?transport=udp";
+  if (base::EndsWith(url, kTcpTransportSuffix,
+                     base::CompareCase::INSENSITIVE_ASCII)) {
+    turn_transport_type = cricket::PROTO_TCP;
+    url.remove_suffix(strlen(kTcpTransportSuffix));
+  } else if (base::EndsWith(url, kUdpTransportSuffix,
+                            base::CompareCase::INSENSITIVE_ASCII)) {
+    turn_transport_type = cricket::PROTO_UDP;
+    url.remove_suffix(strlen(kUdpTransportSuffix));
+  }
+
+  auto parts = base::SplitStringOnce(url, ':');
+  if (!parts) {
+    return false;
+  }
+
+  auto [protocol, host_and_port] = *parts;
+  std::string host;
+  int port;
+  if (!net::ParseHostAndPort(host_and_port, &host, &port)) {
+    return false;
+  }
+
+  if (protocol == "stun") {
+    if (port == -1) {
+      port = kDefaultStunTurnPort;
+    }
+    stun_servers.emplace_back(host, port);
+  } else if (protocol == "turn") {
+    if (port == -1) {
+      port = kDefaultStunTurnPort;
+    }
+    if (turn_transport_type == cricket::PROTO_LAST) {
+      turn_transport_type = cricket::PROTO_UDP;
+    }
+    turn_servers.emplace_back(host, port, username, password,
+                              turn_transport_type, false);
+  } else if (protocol == "turns") {
+    if (port == -1) {
+      port = kDefaultTurnsPort;
+    }
+    if (turn_transport_type == cricket::PROTO_LAST) {
+      turn_transport_type = cricket::PROTO_TCP;
+    }
+    turn_servers.emplace_back(host, port, username, password,
+                              turn_transport_type, true);
+  } else {
+    return false;
+  }
+
+  return true;
+}
+
 }  // namespace remoting::protocol

remoting/protocol/ice_config.h

@@ -5,7 +5,9 @@
 #ifndef REMOTING_PROTOCOL_ICE_CONFIG_H_
 #define REMOTING_PROTOCOL_ICE_CONFIG_H_
 
+#include <optional>
 #include <string>
+#include <string_view>
 #include <vector>
 
 #include "base/time/time.h"
@@ -15,11 +17,9 @@
 
 namespace remoting {
 
-namespace apis {
-namespace v1 {
+namespace apis::v1 {
 class GetIceConfigResponse;
-}  // namespace v1
-}  // namespace apis
+}  // namespace apis::v1
 
 namespace protocol {
 
@@ -33,9 +33,19 @@ struct IceConfig {
   // Parses JSON representation of the config. Returns null config if parsing
   // fails.
   static IceConfig Parse(const base::Value::Dict& dictionary);
-  static IceConfig Parse(const std::string& config_json);
   static IceConfig Parse(const apis::v1::GetIceConfigResponse& config);
 
+  // Parses a |url| in the form of stun:<host>[:<port>][?transport=<udp|tcp>]
+  // and adds an entry to this instance.
+  bool AddStunServer(std::string_view url);
+
+  // Parses a |url| in the form of
+  // <stun|turn|turns>:<host>[:<port>][?transport=<udp|tcp>]
+  // and adds an entry to this instance.
+  bool AddServer(std::string_view url,
+                 const std::string& username,
+                 const std::string& password);
+
   // Time when the config will stop being valid and need to be refreshed.
   base::Time expiration_time;
 
@@ -44,8 +54,8 @@ struct IceConfig {
   // Standard TURN servers
   std::vector<cricket::RelayServerConfig> turn_servers;
 
-  // If greater than 0, the max bandwidth used for relayed connections should
-  // be set to this value.
+  // If greater than 0, the max bandwidth used for relayed connections should be
+  // set to this value.
   int max_bitrate_kbps = 0;
 };