happy_eyeballs: Validate that additional_address are IP addresses ins…

…tead of crashing when sorting. Signed-off-by: Ryan Hamilton <[email protected]> Signed-off-by: Ryan Northey <[email protected]>
envoyproxy · Dec 18, 2024 · 0d7aa8f · 0d7aa8f
1 parent dc4cc8f
commit 0d7aa8f
Show file tree

Hide file tree

Showing 3 changed files with 42 additions and 1 deletion.
diff --git a/changelogs/current.yaml b/changelogs/current.yaml
@@ -11,6 +11,9 @@ bug_fixes:
 - area: http/1
   change: |
     Fixes sending overload crashes when HTTP/1 request is reset.
+- area: happy_eyeballs
+  change: |
+    Validate that ``additional_address`` are IP addresses instead of crashing when sorting.
 
 removed_config_or_runtime:
 # *Normally occurs at the end of the* :ref:`deprecation period <deprecated>`

diff --git a/source/extensions/clusters/eds/eds.cc b/source/extensions/clusters/eds/eds.cc
@@ -146,7 +146,15 @@ void EdsClusterImpl::BatchUpdateHelper::updateLocalityEndpoints(
   if (!lb_endpoint.endpoint().additional_addresses().empty()) {
     address_list.push_back(address);
     for (const auto& additional_address : lb_endpoint.endpoint().additional_addresses()) {
-      address_list.emplace_back(parent_.resolveProtoAddress(additional_address.address()));
+      Network::Address::InstanceConstSharedPtr address =
+          parent_.resolveProtoAddress(additional_address.address());
+      address_list.emplace_back(address);
+    }
+    for (const Network::Address::InstanceConstSharedPtr& address : address_list) {
+      // All addresses must by IP addresses.
+      if (!address->ip()) {
+        throwEnvoyExceptionOrPanic("additional_addresses must be IP addresses.");
+      }
     }
   }
 

diff --git a/test/extensions/clusters/eds/eds_test.cc b/test/extensions/clusters/eds/eds_test.cc
@@ -435,6 +435,36 @@ TEST_F(EdsTest, DualStackEndpoint) {
   EXPECT_NE(connection, connection_data.connection_.get());
 }
 
+// Verify that non-IP additional addresses are rejected.
+TEST_F(EdsTest, RejectNonIpAdditionalAddresses) {
+  envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment;
+  cluster_load_assignment.set_cluster_name("fare");
+
+  // Add dual stack endpoint
+  auto* endpoints = cluster_load_assignment.add_endpoints();
+  auto* endpoint = endpoints->add_lb_endpoints();
+  endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_address("::1");
+  endpoint->mutable_endpoint()->mutable_address()->mutable_socket_address()->set_port_value(80);
+  endpoint->mutable_endpoint()
+      ->mutable_additional_addresses()
+      ->Add()
+      ->mutable_address()
+      ->mutable_envoy_internal_address()
+      ->set_server_listener_name("internal_address");
+
+  endpoint->mutable_load_balancing_weight()->set_value(30);
+
+  initialize();
+  const auto decoded_resources =
+      TestUtility::decodeResources({cluster_load_assignment}, "cluster_name");
+  try {
+    (void)eds_callbacks_->onConfigUpdate(decoded_resources.refvec_, "");
+    FAIL() << "Invalid address was not rejected";
+  } catch (const EnvoyException& e) {
+    EXPECT_STREQ("additional_addresses must be IP addresses.", e.what());
+  }
+}
+
 // Validate that onConfigUpdate() updates the endpoint metadata.
 TEST_F(EdsTest, EndpointMetadata) {
   envoy::config::endpoint::v3::ClusterLoadAssignment cluster_load_assignment;