Skip to content

xds: Don't allow hostnames in address field #12123

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

Merged
merged 2 commits into from
Jun 5, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion xds/src/main/java/io/grpc/xds/Endpoints.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import com.google.common.annotations.VisibleForTesting;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.net.InetAddresses;
import io.grpc.EquivalentAddressGroup;
import java.net.InetSocketAddress;
import java.util.List;
Expand Down Expand Up @@ -78,7 +79,8 @@ static LbEndpoint create(EquivalentAddressGroup eag, int loadBalancingWeight,
@VisibleForTesting
static LbEndpoint create(String address, int port, int loadBalancingWeight, boolean isHealthy,
String hostname, ImmutableMap<String, Object> endpointMetadata) {
return LbEndpoint.create(new EquivalentAddressGroup(new InetSocketAddress(address, port)),
return LbEndpoint.create(
new EquivalentAddressGroup(new InetSocketAddress(InetAddresses.forString(address), port)),
loadBalancingWeight, isHealthy, hostname, endpointMetadata);
}
}
Expand Down
13 changes: 10 additions & 3 deletions xds/src/main/java/io/grpc/xds/XdsEndpointResource.java
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@
import io.grpc.xds.client.Locality;
import io.grpc.xds.client.XdsClient.ResourceUpdate;
import io.grpc.xds.client.XdsResourceType;
import java.net.InetAddress;
import java.net.InetSocketAddress;
import java.util.ArrayList;
import java.util.Collections;
Expand Down Expand Up @@ -245,10 +246,16 @@ static StructOrError<LocalityLbEndpoints> parseLocalityLbEndpoints(
proto.getPriority(), localityMetadata));
}

private static InetSocketAddress getInetSocketAddress(Address address) {
private static InetSocketAddress getInetSocketAddress(Address address)
throws ResourceInvalidException {
io.envoyproxy.envoy.config.core.v3.SocketAddress socketAddress = address.getSocketAddress();

return new InetSocketAddress(socketAddress.getAddress(), socketAddress.getPortValue());
InetAddress parsedAddress;
try {
parsedAddress = InetAddresses.forString(socketAddress.getAddress());
} catch (IllegalArgumentException ex) {
throw new ResourceInvalidException("Address is not an IP", ex);
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add a negative test in GrpcXdsClientImplDataTest with a non I.p. address in the Endpoint resource proto.

}
return new InetSocketAddress(parsedAddress, socketAddress.getPortValue());
}

static final class EdsUpdate implements ResourceUpdate {
Expand Down
23 changes: 9 additions & 14 deletions xds/src/test/java/io/grpc/xds/ControlPlaneRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,17 +266,17 @@ static Cluster buildCluster(String clusterName, String edsName) {
/**
* Builds a new default EDS configuration.
*/
static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, String endpointHostname,
int port) {
return buildClusterLoadAssignment(hostName, endpointHostname, port, EDS_NAME);
static ClusterLoadAssignment buildClusterLoadAssignment(
String hostAddress, String endpointHostname, int port) {
return buildClusterLoadAssignment(hostAddress, endpointHostname, port, EDS_NAME);
}

static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, String endpointHostname,
int port, String edsName) {
static ClusterLoadAssignment buildClusterLoadAssignment(
String hostAddress, String endpointHostname, int port, String edsName) {

Address address = Address.newBuilder()
.setSocketAddress(
SocketAddress.newBuilder().setAddress(hostName).setPortValue(port).build()).build();
SocketAddress.newBuilder().setAddress(hostAddress).setPortValue(port).build()).build();
LocalityLbEndpoints endpoints = LocalityLbEndpoints.newBuilder()
.setLoadBalancingWeight(UInt32Value.of(10))
.setPriority(0)
Expand All @@ -297,17 +297,12 @@ static ClusterLoadAssignment buildClusterLoadAssignment(String hostName, String
* Builds a new client listener.
*/
static Listener buildClientListener(String name) {
return buildClientListener(name, "terminal-filter");
return buildClientListener(name, RDS_NAME);
}


static Listener buildClientListener(String name, String identifier) {
return buildClientListener(name, identifier, RDS_NAME);
}

static Listener buildClientListener(String name, String identifier, String rdsName) {
static Listener buildClientListener(String name, String rdsName) {
HttpFilter httpFilter = HttpFilter.newBuilder()
.setName(identifier)
.setName("terminal-filter")
.setTypedConfig(Any.pack(Router.newBuilder().build()))
.setIsOptional(true)
.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@
import io.grpc.MethodDescriptor;
import io.grpc.Status;
import io.grpc.StatusOr;
import io.grpc.inprocess.InProcessServerBuilder;
import io.grpc.testing.TestMethodDescriptors;
import io.grpc.xds.Endpoints.LbEndpoint;
import io.grpc.xds.Endpoints.LocalityLbEndpoints;
Expand Down Expand Up @@ -84,7 +83,6 @@
public class GcpAuthenticationFilterTest {
private static final GcpAuthenticationFilter.Provider FILTER_PROVIDER =
new GcpAuthenticationFilter.Provider();
private static final String serverName = InProcessServerBuilder.generateName();
private static final LdsUpdate ldsUpdate = getLdsUpdate();
private static final EdsUpdate edsUpdate = getEdsUpdate();
private static final RdsUpdate rdsUpdate = getRdsUpdate();
Expand Down Expand Up @@ -461,15 +459,15 @@ public void testLruCacheEvictionOnResize() throws IOException, ResourceInvalidEx

private static LdsUpdate getLdsUpdate() {
Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig(
serverName, RouterFilter.ROUTER_CONFIG);
"router", RouterFilter.ROUTER_CONFIG);
HttpConnectionManager httpConnectionManager = HttpConnectionManager.forRdsName(
0L, RDS_NAME, Collections.singletonList(routerFilterConfig));
return XdsListenerResource.LdsUpdate.forApiListener(httpConnectionManager);
}

private static RdsUpdate getRdsUpdate() {
RouteConfiguration routeConfiguration =
buildRouteConfiguration(serverName, RDS_NAME, CLUSTER_NAME);
buildRouteConfiguration("my-server", RDS_NAME, CLUSTER_NAME);
XdsResourceType.Args args = new XdsResourceType.Args(null, "0", "0", null, null, null);
try {
return XdsRouteConfigureResource.getInstance().doParse(args, routeConfiguration);
Expand All @@ -481,7 +479,7 @@ private static RdsUpdate getRdsUpdate() {
private static EdsUpdate getEdsUpdate() {
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
LbEndpoint lbEndpoint = LbEndpoint.create(
serverName, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
"127.0.0.5", ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
lbEndpointsMap.put(
Locality.create("", "", ""),
LocalityLbEndpoints.create(ImmutableList.of(lbEndpoint), 10, 0, ImmutableMap.of()));
Expand Down
24 changes: 24 additions & 0 deletions xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,30 @@ public void parseLocalityLbEndpoints_withHealthyEndpoints() throws ResourceInval
100, 1, ImmutableMap.of()));
}

@Test
public void parseLocalityLbEndpoints_onlyPermitIp() {
io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints proto =
io.envoyproxy.envoy.config.endpoint.v3.LocalityLbEndpoints.newBuilder()
.setLocality(Locality.newBuilder()
.setRegion("region-foo").setZone("zone-foo").setSubZone("subZone-foo"))
.setLoadBalancingWeight(UInt32Value.newBuilder().setValue(100)) // locality weight
.setPriority(1)
.addLbEndpoints(io.envoyproxy.envoy.config.endpoint.v3.LbEndpoint.newBuilder()
.setEndpoint(Endpoint.newBuilder()
.setAddress(Address.newBuilder()
.setSocketAddress(
SocketAddress.newBuilder()
.setAddress("example.com").setPortValue(8888))))
.setHealthStatus(io.envoyproxy.envoy.config.core.v3.HealthStatus.HEALTHY)
.setLoadBalancingWeight(UInt32Value.newBuilder().setValue(20))) // endpoint weight
.build();
ResourceInvalidException ex = assertThrows(
ResourceInvalidException.class,
() -> XdsEndpointResource.parseLocalityLbEndpoints(proto));
assertThat(ex.getMessage()).contains("IP");
assertThat(ex.getMessage()).contains("example.com");
}

@Test
public void parseLocalityLbEndpoints_treatUnknownHealthAsHealthy()
throws ResourceInvalidException {
Expand Down
8 changes: 4 additions & 4 deletions xds/src/test/java/io/grpc/xds/XdsClientFallbackTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,10 +84,10 @@ public class XdsClientFallbackTest {
private static final String FALLBACK_EDS_NAME = "fallback-" + EDS_NAME;
private static final HttpConnectionManager MAIN_HTTP_CONNECTION_MANAGER =
HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of(
new Filter.NamedFilterConfig(MAIN_SERVER, RouterFilter.ROUTER_CONFIG)));
new Filter.NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG)));
private static final HttpConnectionManager FALLBACK_HTTP_CONNECTION_MANAGER =
HttpConnectionManager.forRdsName(0, RDS_NAME, ImmutableList.of(
new Filter.NamedFilterConfig(FALLBACK_SERVER, RouterFilter.ROUTER_CONFIG)));
HttpConnectionManager.forRdsName(0, FALLBACK_RDS_NAME, ImmutableList.of(
new Filter.NamedFilterConfig("terminal-filter", RouterFilter.ROUTER_CONFIG)));
private ObjectPool<XdsClient> xdsClientPool;
private XdsClient xdsClient;
private boolean originalEnableXdsFallback;
Expand Down Expand Up @@ -201,7 +201,7 @@ private static void setAdsConfig(ControlPlaneRule controlPlane, String serverNam
String edsName = isMainServer ? EDS_NAME : FALLBACK_EDS_NAME;

controlPlane.setLdsConfig(ControlPlaneRule.buildServerListener(),
ControlPlaneRule.buildClientListener(MAIN_SERVER, serverName));
ControlPlaneRule.buildClientListener(MAIN_SERVER, rdsName));

controlPlane.setRdsConfig(rdsName,
XdsTestUtils.buildRouteConfiguration(MAIN_SERVER, rdsName, clusterName));
Expand Down
14 changes: 7 additions & 7 deletions xds/src/test/java/io/grpc/xds/XdsDependencyManagerTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ public class XdsDependencyManagerTest {
private Server xdsServer;

private final FakeClock fakeClock = new FakeClock();
private final String serverName = InProcessServerBuilder.generateName();
private final String serverName = "the-service-name";
private final Queue<XdsTestUtils.LrsRpcCall> loadReportCalls = new ArrayDeque<>();
private final AtomicBoolean adsEnded = new AtomicBoolean(true);
private final AtomicBoolean lrsEnded = new AtomicBoolean(true);
Expand Down Expand Up @@ -153,7 +153,7 @@ public class XdsDependencyManagerTest {
@Before
public void setUp() throws Exception {
xdsServer = cleanupRule.register(InProcessServerBuilder
.forName(serverName)
.forName("control-plane")
.addService(controlPlaneService)
.addService(lrsService)
.directExecutor()
Expand All @@ -163,7 +163,7 @@ public void setUp() throws Exception {
XdsTestUtils.setAdsConfig(controlPlaneService, serverName);

channel = cleanupRule.register(
InProcessChannelBuilder.forName(serverName).directExecutor().build());
InProcessChannelBuilder.forName("control-plane").directExecutor().build());
XdsTransportFactory xdsTransportFactory =
ignore -> new GrpcXdsTransportFactory.GrpcXdsTransport(channel);

Expand Down Expand Up @@ -351,10 +351,10 @@ public void testMissingCdsAndEds() {
// Update config so that one of the 2 "valid" clusters has an EDS resource, the other does not
// and there is an EDS that doesn't have matching clusters
ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
serverName, ENDPOINT_HOSTNAME, ENDPOINT_PORT, XdsTestUtils.EDS_NAME + 0);
"127.0.1.1", ENDPOINT_HOSTNAME, ENDPOINT_PORT, XdsTestUtils.EDS_NAME + 0);
edsMap.put(XdsTestUtils.EDS_NAME + 0, clusterLoadAssignment);
clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
serverName, ENDPOINT_HOSTNAME, ENDPOINT_PORT, "garbageEds");
"127.0.1.2", ENDPOINT_HOSTNAME, ENDPOINT_PORT, "garbageEds");
edsMap.put("garbageEds", clusterLoadAssignment);
controlPlaneService.setXdsConfig(ADS_TYPE_URL_EDS, edsMap);

Expand Down Expand Up @@ -421,7 +421,7 @@ public void testTcpListenerErrors() {
@Test
public void testMissingRds() {
String rdsName = "badRdsName";
Listener clientListener = ControlPlaneRule.buildClientListener(serverName, serverName, rdsName);
Listener clientListener = ControlPlaneRule.buildClientListener(serverName, rdsName);
controlPlaneService.setXdsConfig(ADS_TYPE_URL_LDS,
ImmutableMap.of(serverName, clientListener));

Expand Down Expand Up @@ -578,7 +578,7 @@ public void testMultipleCdsReferToSameEds() {

Map<String, Message> edsMap = new HashMap<>();
ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
serverName, ENDPOINT_HOSTNAME, ENDPOINT_PORT, edsName);
"127.0.1.4", ENDPOINT_HOSTNAME, ENDPOINT_PORT, edsName);
edsMap.put(edsName, clusterLoadAssignment);

RouteConfiguration routeConfig =
Expand Down
2 changes: 1 addition & 1 deletion xds/src/test/java/io/grpc/xds/XdsNameResolverTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -1228,7 +1228,7 @@ private static void createAndDeliverClusterUpdates(
.roundRobinLbPolicy();
xdsClient.deliverCdsUpdate(clusterName, forEds.build());
EdsUpdate edsUpdate = new EdsUpdate(clusterName,
XdsTestUtils.createMinimalLbEndpointsMap("host"), Collections.emptyList());
XdsTestUtils.createMinimalLbEndpointsMap("127.0.0.3"), Collections.emptyList());
xdsClient.deliverEdsUpdate(clusterName, edsUpdate);
}
}
Expand Down
20 changes: 10 additions & 10 deletions xds/src/test/java/io/grpc/xds/XdsTestUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ static void setAdsConfig(XdsTestControlPlaneService service, String serverName,
int endpointPort) {

Listener serverListener = ControlPlaneRule.buildServerListener();
Listener clientListener = ControlPlaneRule.buildClientListener(serverName, serverName, rdsName);
Listener clientListener = ControlPlaneRule.buildClientListener(serverName, rdsName);
service.setXdsConfig(ADS_TYPE_URL_LDS,
ImmutableMap.of(SERVER_LISTENER, serverListener, serverName, clientListener));

Expand All @@ -148,7 +148,7 @@ static void setAdsConfig(XdsTestControlPlaneService service, String serverName,
service.setXdsConfig(ADS_TYPE_URL_CDS, ImmutableMap.<String, Message>of(clusterName, cluster));

ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
serverName, endpointHostname, endpointPort, edsName);
"127.0.0.11", endpointHostname, endpointPort, edsName);
service.setXdsConfig(ADS_TYPE_URL_EDS,
ImmutableMap.<String, Message>of(edsName, clusterLoadAssignment));

Expand Down Expand Up @@ -186,7 +186,7 @@ static void setAggregateCdsConfig(XdsTestControlPlaneService service, String ser
Map<String, Message> edsMap = new HashMap<>();
for (String child : children) {
ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
serverName, ENDPOINT_HOSTNAME, ENDPOINT_PORT, getEdsNameForCluster(child));
"127.0.0.16", ENDPOINT_HOSTNAME, ENDPOINT_PORT, getEdsNameForCluster(child));
edsMap.put(getEdsNameForCluster(child), clusterLoadAssignment);
}
service.setXdsConfig(ADS_TYPE_URL_EDS, edsMap);
Expand Down Expand Up @@ -225,7 +225,7 @@ static void addAggregateToExistingConfig(XdsTestControlPlaneService service, Str
continue;
}
ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
child, ENDPOINT_HOSTNAME, ENDPOINT_PORT, getEdsNameForCluster(child));
"127.0.0.15", ENDPOINT_HOSTNAME, ENDPOINT_PORT, getEdsNameForCluster(child));
edsMap.put(getEdsNameForCluster(child), clusterLoadAssignment);
}
service.setXdsConfig(ADS_TYPE_URL_EDS, edsMap);
Expand All @@ -236,7 +236,7 @@ static XdsConfig getDefaultXdsConfig(String serverHostName)
XdsConfig.XdsConfigBuilder builder = new XdsConfig.XdsConfigBuilder();

Filter.NamedFilterConfig routerFilterConfig = new Filter.NamedFilterConfig(
serverHostName, RouterFilter.ROUTER_CONFIG);
"terminal-filter", RouterFilter.ROUTER_CONFIG);

HttpConnectionManager httpConnectionManager = HttpConnectionManager.forRdsName(
0L, RDS_NAME, Collections.singletonList(routerFilterConfig));
Expand All @@ -257,7 +257,7 @@ static XdsConfig getDefaultXdsConfig(String serverHostName)
// Need to create endpoints to create locality endpoints map to create edsUpdate
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
LbEndpoint lbEndpoint = LbEndpoint.create(
serverHostName, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
"127.0.0.11", ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
lbEndpointsMap.put(
Locality.create("", "", ""),
LocalityLbEndpoints.create(ImmutableList.of(lbEndpoint), 10, 0, ImmutableMap.of()));
Expand All @@ -280,10 +280,10 @@ static XdsConfig getDefaultXdsConfig(String serverHostName)
return builder.build();
}

static Map<Locality, LocalityLbEndpoints> createMinimalLbEndpointsMap(String serverHostName) {
static Map<Locality, LocalityLbEndpoints> createMinimalLbEndpointsMap(String serverAddress) {
Map<Locality, LocalityLbEndpoints> lbEndpointsMap = new HashMap<>();
LbEndpoint lbEndpoint = LbEndpoint.create(
serverHostName, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
serverAddress, ENDPOINT_PORT, 0, true, ENDPOINT_HOSTNAME, ImmutableMap.of());
lbEndpointsMap.put(
Locality.create("", "", ""),
LocalityLbEndpoints.create(ImmutableList.of(lbEndpoint), 10, 0, ImmutableMap.of()));
Expand Down Expand Up @@ -338,15 +338,15 @@ static void addEdsClusters(Map<String, Message> clusterMap, Map<String, Message>
clusterMap.put(clusterName, cluster);

ClusterLoadAssignment clusterLoadAssignment = ControlPlaneRule.buildClusterLoadAssignment(
clusterName, ENDPOINT_HOSTNAME, ENDPOINT_PORT, edsName);
"127.0.0.13", ENDPOINT_HOSTNAME, ENDPOINT_PORT, edsName);
edsMap.put(edsName, clusterLoadAssignment);
}
}

static Listener buildInlineClientListener(String rdsName, String clusterName, String serverName) {
HttpFilter
httpFilter = HttpFilter.newBuilder()
.setName(serverName)
.setName("terminal-filter")
.setTypedConfig(Any.pack(Router.newBuilder().build()))
.setIsOptional(true)
.build();
Expand Down