Skip to content

Commit c8d1e6e

Browse files
shivaspeaksshivaspeaks
authored
xds: listener type validation (#11933)
1 parent 84c7713 commit c8d1e6e

11 files changed

+271
-25
lines changed

‎xds/src/main/java/io/grpc/xds/EnvoyServerProtoData.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import com.google.common.annotations.VisibleForTesting;
2323
import com.google.common.collect.ImmutableList;
2424
import com.google.protobuf.util.Durations;
25+
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
2526
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CommonTlsContext;
2627
import io.grpc.Internal;
2728
import io.grpc.xds.client.EnvoyProtoData;
@@ -248,13 +249,17 @@ abstract static class Listener {
248249
@Nullable
249250
abstract FilterChain defaultFilterChain();
250251

252+
@Nullable
253+
abstract Protocol protocol();
254+
251255
static Listener create(
252256
String name,
253257
@Nullable String address,
254258
ImmutableList<FilterChain> filterChains,
255-
@Nullable FilterChain defaultFilterChain) {
259+
@Nullable FilterChain defaultFilterChain,
260+
@Nullable Protocol protocol) {
256261
return new AutoValue_EnvoyServerProtoData_Listener(name, address, filterChains,
257-
defaultFilterChain);
262+
defaultFilterChain, protocol);
258263
}
259264
}
260265

‎xds/src/main/java/io/grpc/xds/XdsListenerResource.java‎

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -162,13 +162,16 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
162162
}
163163

164164
String address = null;
165+
SocketAddress socketAddress = null;
165166
if (proto.getAddress().hasSocketAddress()) {
166-
SocketAddress socketAddress = proto.getAddress().getSocketAddress();
167+
socketAddress = proto.getAddress().getSocketAddress();
167168
address = socketAddress.getAddress();
169+
if (address.isEmpty()) {
170+
throw new ResourceInvalidException("Invalid address: Empty address is not allowed.");
171+
}
168172
switch (socketAddress.getPortSpecifierCase()) {
169173
case NAMED_PORT:
170-
address = address + ":" + socketAddress.getNamedPort();
171-
break;
174+
throw new ResourceInvalidException("NAMED_PORT is not supported in gRPC.");
172175
case PORT_VALUE:
173176
address = address + ":" + socketAddress.getPortValue();
174177
break;
@@ -209,8 +212,8 @@ static EnvoyServerProtoData.Listener parseServerSideListener(
209212
null, certProviderInstances, args);
210213
}
211214

212-
return EnvoyServerProtoData.Listener.create(
213-
proto.getName(), address, filterChains.build(), defaultFilterChain);
215+
return EnvoyServerProtoData.Listener.create(proto.getName(), address, filterChains.build(),
216+
defaultFilterChain, socketAddress == null ? null : socketAddress.getProtocol());
214217
}
215218

216219
@VisibleForTesting

‎xds/src/main/java/io/grpc/xds/XdsNameResolver.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -684,6 +684,13 @@ public void onUpdate(StatusOr<XdsConfig> updateOrStatus) {
684684
// Process Route
685685
XdsConfig update = updateOrStatus.getValue();
686686
HttpConnectionManager httpConnectionManager = update.getListener().httpConnectionManager();
687+
if (httpConnectionManager == null) {
688+
logger.log(XdsLogLevel.INFO, "API Listener: httpConnectionManager does not exist.");
689+
updateActiveFilters(null);
690+
cleanUpRoutes(updateOrStatus.getStatus());
691+
return;
692+
}
693+
687694
VirtualHost virtualHost = update.getVirtualHost();
688695
ImmutableList<NamedFilterConfig> filterConfigs = httpConnectionManager.httpFilterConfigs();
689696
long streamDurationNano = httpConnectionManager.httpMaxStreamDurationNano();

‎xds/src/main/java/io/grpc/xds/XdsServerWrapper.java‎

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,10 @@
2424
import com.google.common.annotations.VisibleForTesting;
2525
import com.google.common.collect.ImmutableList;
2626
import com.google.common.collect.ImmutableMap;
27+
import com.google.common.net.HostAndPort;
28+
import com.google.common.net.InetAddresses;
2729
import com.google.common.util.concurrent.SettableFuture;
30+
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
2831
import io.grpc.Attributes;
2932
import io.grpc.InternalServerInterceptors;
3033
import io.grpc.Metadata;
@@ -57,6 +60,7 @@
5760
import io.grpc.xds.client.XdsClient.ResourceWatcher;
5861
import io.grpc.xds.internal.security.SslContextProviderSupplier;
5962
import java.io.IOException;
63+
import java.net.InetAddress;
6064
import java.net.SocketAddress;
6165
import java.util.ArrayList;
6266
import java.util.HashMap;
@@ -383,7 +387,21 @@ public void onChanged(final LdsUpdate update) {
383387
return;
384388
}
385389
logger.log(Level.FINEST, "Received Lds update {0}", update);
386-
checkNotNull(update.listener(), "update");
390+
if (update.listener() == null) {
391+
onResourceDoesNotExist("Non-API");
392+
return;
393+
}
394+
395+
String ldsAddress = update.listener().address();
396+
if (ldsAddress == null || update.listener().protocol() != Protocol.TCP
397+
|| !ipAddressesMatch(ldsAddress)) {
398+
handleConfigNotFoundOrMismatch(
399+
Status.UNKNOWN.withDescription(
400+
String.format(
401+
"Listener address mismatch: expected %s, but got %s.",
402+
listenerAddress, ldsAddress)).asException());
403+
return;
404+
}
387405
if (!pendingRds.isEmpty()) {
388406
// filter chain state has not yet been applied to filterChainSelectorManager and there
389407
// are two sets of sslContextProviderSuppliers, so we release the old ones.
@@ -432,6 +450,18 @@ public void onChanged(final LdsUpdate update) {
432450
}
433451
}
434452

453+
private boolean ipAddressesMatch(String ldsAddress) {
454+
HostAndPort ldsAddressHnP = HostAndPort.fromString(ldsAddress);
455+
HostAndPort listenerAddressHnP = HostAndPort.fromString(listenerAddress);
456+
if (!ldsAddressHnP.hasPort() || !listenerAddressHnP.hasPort()
457+
|| ldsAddressHnP.getPort() != listenerAddressHnP.getPort()) {
458+
return false;
459+
}
460+
InetAddress listenerIp = InetAddresses.forString(listenerAddressHnP.getHost());
461+
InetAddress ldsIp = InetAddresses.forString(ldsAddressHnP.getHost());
462+
return listenerIp.equals(ldsIp);
463+
}
464+
435465
@Override
436466
public void onResourceDoesNotExist(final String resourceName) {
437467
if (stopped) {
@@ -440,7 +470,7 @@ public void onResourceDoesNotExist(final String resourceName) {
440470
StatusException statusException = Status.UNAVAILABLE.withDescription(
441471
String.format("Listener %s unavailable, xDS node ID: %s", resourceName,
442472
xdsClient.getBootstrapInfo().node().getId())).asException();
443-
handleConfigNotFound(statusException);
473+
handleConfigNotFoundOrMismatch(statusException);
444474
}
445475

446476
@Override
@@ -674,7 +704,7 @@ public <ReqT, RespT> Listener<ReqT> interceptCall(ServerCall<ReqT, RespT> call,
674704
};
675705
}
676706

677-
private void handleConfigNotFound(StatusException exception) {
707+
private void handleConfigNotFoundOrMismatch(StatusException exception) {
678708
cleanUpRouteDiscoveryStates();
679709
shutdownActiveFilters();
680710
List<SslContextProviderSupplier> toRelease = getSuppliersInUse();

‎xds/src/test/java/io/grpc/xds/ControlPlaneRule.java‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -366,10 +366,14 @@ static Listener buildServerListener() {
366366
.setFilterChainMatch(filterChainMatch)
367367
.addFilters(filter)
368368
.build();
369+
Address address = Address.newBuilder()
370+
.setSocketAddress(SocketAddress.newBuilder().setAddress("0.0.0.0").setPortValue(0))
371+
.build();
369372
return Listener.newBuilder()
370373
.setName(SERVER_LISTENER_TEMPLATE_NO_REPLACEMENT)
371374
.setTrafficDirection(TrafficDirection.INBOUND)
372375
.addFilterChains(filterChain)
376+
.setAddress(address)
373377
.build();
374378
}
375379
}

‎xds/src/test/java/io/grpc/xds/GrpcXdsClientImplDataTest.java‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2661,6 +2661,41 @@ public void parseServerSideListener_useOriginalDst() throws ResourceInvalidExcep
26612661
listener,null, filterRegistry, null, getXdsResourceTypeArgs(true));
26622662
}
26632663

2664+
@Test
2665+
public void parseServerSideListener_emptyAddress() throws ResourceInvalidException {
2666+
Listener listener =
2667+
Listener.newBuilder()
2668+
.setName("listener1")
2669+
.setTrafficDirection(TrafficDirection.INBOUND)
2670+
.setAddress(Address.newBuilder()
2671+
.setSocketAddress(
2672+
SocketAddress.newBuilder()))
2673+
.build();
2674+
thrown.expect(ResourceInvalidException.class);
2675+
thrown.expectMessage("Invalid address: Empty address is not allowed.");
2676+
2677+
XdsListenerResource.parseServerSideListener(
2678+
listener,null, filterRegistry, null, getXdsResourceTypeArgs(true));
2679+
}
2680+
2681+
@Test
2682+
public void parseServerSideListener_namedPort() throws ResourceInvalidException {
2683+
Listener listener =
2684+
Listener.newBuilder()
2685+
.setName("listener1")
2686+
.setTrafficDirection(TrafficDirection.INBOUND)
2687+
.setAddress(Address.newBuilder()
2688+
.setSocketAddress(
2689+
SocketAddress.newBuilder()
2690+
.setAddress("172.14.14.5").setNamedPort("")))
2691+
.build();
2692+
thrown.expect(ResourceInvalidException.class);
2693+
thrown.expectMessage("NAMED_PORT is not supported in gRPC.");
2694+
2695+
XdsListenerResource.parseServerSideListener(
2696+
listener,null, filterRegistry, null, getXdsResourceTypeArgs(true));
2697+
}
2698+
26642699
@Test
26652700
public void parseServerSideListener_nonUniqueFilterChainMatch() throws ResourceInvalidException {
26662701
Filter filter1 = buildHttpConnectionManagerFilter(

‎xds/src/test/java/io/grpc/xds/XdsClientWrapperForServerSdsTestMisc.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232

3333
import com.google.common.collect.ImmutableList;
3434
import com.google.common.util.concurrent.SettableFuture;
35+
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
3536
import io.grpc.Server;
3637
import io.grpc.ServerBuilder;
3738
import io.grpc.Status;
@@ -165,9 +166,10 @@ public void run() {
165166
EnvoyServerProtoData.Listener tcpListener =
166167
EnvoyServerProtoData.Listener.create(
167168
"listener1",
168-
"10.1.2.3",
169+
"0.0.0.0:7000",
169170
ImmutableList.of(),
170-
null);
171+
null,
172+
Protocol.TCP);
171173
LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(tcpListener);
172174
xdsClient.ldsWatcher.onChanged(listenerUpdate);
173175
verify(listener, timeout(5000)).onServing();

‎xds/src/test/java/io/grpc/xds/XdsSecurityClientServerTest.java‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import com.google.common.collect.ImmutableList;
3636
import com.google.common.collect.ImmutableMap;
3737
import com.google.common.util.concurrent.SettableFuture;
38+
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
3839
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
3940
import io.grpc.Attributes;
4041
import io.grpc.EquivalentAddressGroup;
@@ -497,7 +498,7 @@ public void mtlsClientServer_changeServerContext_expectException()
497498
DownstreamTlsContext downstreamTlsContext =
498499
CommonTlsContextTestsUtil.buildDownstreamTlsContext(
499500
"cert-instance-name2", true, true);
500-
EnvoyServerProtoData.Listener listener = buildListener("listener1", "0.0.0.0",
501+
EnvoyServerProtoData.Listener listener = buildListener("listener1", "0.0.0.0:0",
501502
downstreamTlsContext,
502503
tlsContextManagerForServer);
503504
xdsClient.deliverLdsUpdate(LdsUpdate.forTcpListener(listener));
@@ -601,7 +602,7 @@ private void buildServer(
601602
tlsContextManagerForServer = new TlsContextManagerImpl(bootstrapInfoForServer);
602603
XdsServerWrapper xdsServer = (XdsServerWrapper) builder.build();
603604
SettableFuture<Throwable> startFuture = startServerAsync(xdsServer);
604-
EnvoyServerProtoData.Listener listener = buildListener("listener1", "10.1.2.3",
605+
EnvoyServerProtoData.Listener listener = buildListener("listener1", "0.0.0.0:0",
605606
downstreamTlsContext, tlsContextManagerForServer);
606607
LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(listener);
607608
xdsClient.deliverLdsUpdate(listenerUpdate);
@@ -642,7 +643,7 @@ static EnvoyServerProtoData.Listener buildListener(
642643
"filter-chain-foo", filterChainMatch, httpConnectionManager, tlsContext,
643644
tlsContextManager);
644645
EnvoyServerProtoData.Listener listener = EnvoyServerProtoData.Listener.create(
645-
name, address, ImmutableList.of(defaultFilterChain), null);
646+
name, address, ImmutableList.of(defaultFilterChain), null, Protocol.TCP);
646647
return listener;
647648
}
648649

‎xds/src/test/java/io/grpc/xds/XdsServerBuilderTest.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package io.grpc.xds;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static io.grpc.xds.XdsServerTestHelper.buildTestListener;
2021
import static org.junit.Assert.fail;
2122
import static org.mockito.Mockito.any;
2223
import static org.mockito.Mockito.mock;
@@ -26,13 +27,15 @@
2627
import static org.mockito.Mockito.verify;
2728
import static org.mockito.Mockito.when;
2829

30+
import com.google.common.collect.ImmutableList;
2931
import com.google.common.util.concurrent.SettableFuture;
3032
import io.grpc.BindableService;
3133
import io.grpc.InsecureServerCredentials;
3234
import io.grpc.ServerServiceDefinition;
3335
import io.grpc.Status;
3436
import io.grpc.StatusException;
3537
import io.grpc.testing.GrpcCleanupRule;
38+
import io.grpc.xds.XdsListenerResource.LdsUpdate;
3639
import io.grpc.xds.XdsServerTestHelper.FakeXdsClient;
3740
import io.grpc.xds.XdsServerTestHelper.FakeXdsClientPoolFactory;
3841
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
@@ -221,10 +224,13 @@ public void xdsServer_startError()
221224
buildServer(mockXdsServingStatusListener);
222225
Future<Throwable> future = startServerAsync();
223226
// create port conflict for start to fail
224-
XdsServerTestHelper.generateListenerUpdate(
225-
xdsClient,
227+
EnvoyServerProtoData.Listener listener = buildTestListener(
228+
"listener1", "0.0.0.0:" + port, ImmutableList.of(),
226229
CommonTlsContextTestsUtil.buildTestInternalDownstreamTlsContext("CERT1", "VA1"),
227-
tlsContextManager);
230+
null, tlsContextManager);
231+
LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(listener);
232+
xdsClient.deliverLdsUpdate(listenerUpdate);
233+
228234
Throwable exception = future.get(5, TimeUnit.SECONDS);
229235
assertThat(exception).isInstanceOf(IOException.class);
230236
assertThat(exception).hasMessageThat().contains("Failed to bind");

‎xds/src/test/java/io/grpc/xds/XdsServerTestHelper.java‎

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.collect.ImmutableList;
2222
import com.google.common.collect.ImmutableMap;
2323
import com.google.common.util.concurrent.SettableFuture;
24+
import io.envoyproxy.envoy.config.core.v3.SocketAddress.Protocol;
2425
import io.grpc.InsecureChannelCredentials;
2526
import io.grpc.MetricRecorder;
2627
import io.grpc.internal.ObjectPool;
@@ -74,7 +75,7 @@ public class XdsServerTestHelper {
7475
static void generateListenerUpdate(FakeXdsClient xdsClient,
7576
EnvoyServerProtoData.DownstreamTlsContext tlsContext,
7677
TlsContextManager tlsContextManager) {
77-
EnvoyServerProtoData.Listener listener = buildTestListener("listener1", "10.1.2.3",
78+
EnvoyServerProtoData.Listener listener = buildTestListener("listener1", "0.0.0.0:0",
7879
ImmutableList.of(), tlsContext, null, tlsContextManager);
7980
LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(listener);
8081
xdsClient.deliverLdsUpdate(listenerUpdate);
@@ -85,7 +86,8 @@ static void generateListenerUpdate(
8586
EnvoyServerProtoData.DownstreamTlsContext tlsContext,
8687
EnvoyServerProtoData.DownstreamTlsContext tlsContextForDefaultFilterChain,
8788
TlsContextManager tlsContextManager) {
88-
EnvoyServerProtoData.Listener listener = buildTestListener("listener1", "10.1.2.3", sourcePorts,
89+
EnvoyServerProtoData.Listener listener = buildTestListener(
90+
"listener1", "0.0.0.0:7000", sourcePorts,
8991
tlsContext, tlsContextForDefaultFilterChain, tlsContextManager);
9092
LdsUpdate listenerUpdate = LdsUpdate.forTcpListener(listener);
9193
xdsClient.deliverLdsUpdate(listenerUpdate);
@@ -130,7 +132,7 @@ static EnvoyServerProtoData.Listener buildTestListener(
130132
tlsContextForDefaultFilterChain, tlsContextManager);
131133
EnvoyServerProtoData.Listener listener =
132134
EnvoyServerProtoData.Listener.create(
133-
name, address, ImmutableList.of(filterChain1), defaultFilterChain);
135+
name, address, ImmutableList.of(filterChain1), defaultFilterChain, Protocol.TCP);
134136
return listener;
135137
}
136138

@@ -290,15 +292,23 @@ private String awaitLdsResource(Duration timeout) {
290292
}
291293
}
292294

295+
void deliverLdsUpdateWithApiListener(long httpMaxStreamDurationNano,
296+
List<VirtualHost> virtualHosts) {
297+
execute(() -> {
298+
ldsWatcher.onChanged(LdsUpdate.forApiListener(HttpConnectionManager.forVirtualHosts(
299+
httpMaxStreamDurationNano, virtualHosts, null)));
300+
});
301+
}
302+
293303
void deliverLdsUpdate(LdsUpdate ldsUpdate) {
294304
execute(() -> ldsWatcher.onChanged(ldsUpdate));
295305
}
296306

297307
void deliverLdsUpdate(
298308
List<FilterChain> filterChains,
299309
@Nullable FilterChain defaultFilterChain) {
300-
deliverLdsUpdate(LdsUpdate.forTcpListener(Listener.create(
301-
"listener", "0.0.0.0:1", ImmutableList.copyOf(filterChains), defaultFilterChain)));
310+
deliverLdsUpdate(LdsUpdate.forTcpListener(Listener.create("listener", "0.0.0.0:1",
311+
ImmutableList.copyOf(filterChains), defaultFilterChain, Protocol.TCP)));
302312
}
303313

304314
void deliverLdsUpdate(FilterChain filterChain, @Nullable FilterChain defaultFilterChain) {

0 commit comments

Comments
 (0)