Skip to content

Commit f12bc61

Browse files
kannanjgithubkannanjgithub
committed
save changed
1 parent 42c9df0 commit f12bc61

11 files changed

+70
-63
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -241,9 +241,9 @@ public Subchannel createSubchannel(CreateSubchannelArgs args) {
241241
.set(ATTR_CLUSTER_LOCALITY, localityAtomicReference);
242242
if (GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE", false)) {
243243
String hostname = args.getAddresses().get(0).getAttributes()
244-
.get(XdsAttributes.ATTR_ADDRESS_NAME);
244+
.get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME);
245245
if (hostname != null) {
246-
attrsBuilder.set(XdsAttributes.ATTR_ADDRESS_NAME, hostname);
246+
attrsBuilder.set(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME, hostname);
247247
}
248248
}
249249
args = args.toBuilder().setAddresses(addresses).setAttributes(attrsBuilder.build()).build();
@@ -438,7 +438,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) {
438438
result = PickResult.withSubchannel(result.getSubchannel(),
439439
result.getStreamTracerFactory(),
440440
result.getSubchannel().getAttributes().get(
441-
XdsAttributes.ATTR_ADDRESS_NAME));
441+
SecurityProtocolNegotiators.ATTR_ADDRESS_NAME));
442442
}
443443
}
444444
return result;

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,7 @@
6161
import io.grpc.xds.client.XdsClient.ResourceWatcher;
6262
import io.grpc.xds.client.XdsLogger;
6363
import io.grpc.xds.client.XdsLogger.XdsLogLevel;
64+
import io.grpc.xds.internal.security.SecurityProtocolNegotiators;
6465
import java.net.InetSocketAddress;
6566
import java.net.SocketAddress;
6667
import java.net.URI;
@@ -432,7 +433,7 @@ public void run() {
432433
.set(XdsAttributes.ATTR_LOCALITY_WEIGHT,
433434
localityLbInfo.localityWeight())
434435
.set(XdsAttributes.ATTR_SERVER_WEIGHT, weight)
435-
.set(XdsAttributes.ATTR_ADDRESS_NAME, endpoint.hostname())
436+
.set(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME, endpoint.hostname())
436437
.build();
437438

438439
EquivalentAddressGroup eag;
@@ -680,7 +681,7 @@ public Status onResult2(final ResolutionResult resolutionResult) {
680681
Attributes attr = eag.getAttributes().toBuilder()
681682
.set(XdsAttributes.ATTR_LOCALITY, LOGICAL_DNS_CLUSTER_LOCALITY)
682683
.set(XdsAttributes.ATTR_LOCALITY_NAME, localityName)
683-
.set(XdsAttributes.ATTR_ADDRESS_NAME, dnsHostName)
684+
.set(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME, dnsHostName)
684685
.build();
685686
eag = new EquivalentAddressGroup(eag.getAddresses(), attr);
686687
eag = AddressFilter.setPathFilter(eag, Arrays.asList(priorityName, localityName));

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

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,11 +95,6 @@ final class XdsAttributes {
9595
static final Attributes.Key<Long> ATTR_SERVER_WEIGHT =
9696
Attributes.Key.create("io.grpc.xds.XdsAttributes.serverWeight");
9797

98-
/** Name associated with individual address, if available (e.g., DNS name). */
99-
@EquivalentAddressGroup.Attr
100-
static final Attributes.Key<String> ATTR_ADDRESS_NAME =
101-
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
102-
10398
/**
10499
* Filter chain match for network filters.
105100
*/

‎xds/src/main/java/io/grpc/xds/internal/security/SecurityProtocolNegotiators.java‎

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@
2020

2121
import com.google.common.annotations.VisibleForTesting;
2222
import io.grpc.Attributes;
23+
import io.grpc.EquivalentAddressGroup;
2324
import io.grpc.Grpc;
2425
import io.grpc.internal.GrpcUtil;
2526
import io.grpc.internal.ObjectPool;
@@ -50,6 +51,11 @@
5051
@VisibleForTesting
5152
public final class SecurityProtocolNegotiators {
5253

54+
/** Name associated with individual address, if available (e.g., DNS name). */
55+
@EquivalentAddressGroup.Attr
56+
public static final Attributes.Key<String> ATTR_ADDRESS_NAME =
57+
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
58+
5359
// Prevent instantiation.
5460
private SecurityProtocolNegotiators() {
5561
}
@@ -142,7 +148,8 @@ public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
142148
fallbackProtocolNegotiator, "No TLS config and no fallbackProtocolNegotiator!");
143149
return fallbackProtocolNegotiator.newHandler(grpcHandler);
144150
}
145-
return new ClientSecurityHandler(grpcHandler, localSslContextProviderSupplier);
151+
return new ClientSecurityHandler(grpcHandler, localSslContextProviderSupplier,
152+
grpcHandler.getEagAttributes().get(ATTR_ADDRESS_NAME));
146153
}
147154

148155
@Override
@@ -185,10 +192,12 @@ static final class ClientSecurityHandler
185192
extends InternalProtocolNegotiators.ProtocolNegotiationHandler {
186193
private final GrpcHttp2ConnectionHandler grpcHandler;
187194
private final SslContextProviderSupplier sslContextProviderSupplier;
195+
private final String hostname;
188196

189197
ClientSecurityHandler(
190198
GrpcHttp2ConnectionHandler grpcHandler,
191-
SslContextProviderSupplier sslContextProviderSupplier) {
199+
SslContextProviderSupplier sslContextProviderSupplier,
200+
String hostname) {
192201
super(
193202
// superclass (InternalProtocolNegotiators.ProtocolNegotiationHandler) expects 'next'
194203
// handler but we don't have a next handler _yet_. So we "disable" superclass's behavior
@@ -202,6 +211,7 @@ public void handlerAdded(ChannelHandlerContext ctx) throws Exception {
202211
checkNotNull(grpcHandler, "grpcHandler");
203212
this.grpcHandler = grpcHandler;
204213
this.sslContextProviderSupplier = sslContextProviderSupplier;
214+
this.hostname = hostname;
205215
}
206216

207217
@Override
@@ -210,7 +220,7 @@ protected void handlerAdded0(final ChannelHandlerContext ctx) {
210220
ctx.pipeline().addBefore(ctx.name(), null, bufferReads);
211221

212222
sslContextProviderSupplier.updateSslContext(
213-
new SslContextProvider.Callback(ctx.executor()) {
223+
new SslContextProvider.Callback(ctx.executor(), hostname) {
214224

215225
@Override
216226
public void updateSslContext(SslContext sslContext, String sni) {

‎xds/src/main/java/io/grpc/xds/internal/security/SslContextProvider.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -48,19 +48,16 @@ public abstract class SslContextProvider implements Closeable {
4848
@VisibleForTesting public abstract static class Callback {
4949
private final Executor executor;
5050
private final String hostname;
51-
private final boolean isClientSide;
5251

5352
protected Callback(Executor executor) {
5453
this.executor = executor;
5554
this.hostname = null;
56-
this.isClientSide = false;
5755
}
5856

5957
// Only for client SslContextProvider.
6058
protected Callback(Executor executor, String hostname) {
6159
this.executor = executor;
6260
this.hostname = hostname;
63-
this.isClientSide = true;
6461
}
6562

6663
@VisibleForTesting public Executor getExecutor() {
@@ -71,10 +68,6 @@ protected String getHostname() {
7168
return hostname;
7269
}
7370

74-
public boolean isClientSide() {
75-
return isClientSide;
76-
}
77-
7871
/** Informs callee of new/updated SslContext. */
7972
@VisibleForTesting public abstract void updateSslContext(SslContext sslContext, String sni);
8073

‎xds/src/main/java/io/grpc/xds/internal/security/SslContextProviderSupplier.java‎

Lines changed: 16 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ public synchronized void updateSslContext(final SslContextProvider.Callback call
6262
checkNotNull(callback, "callback");
6363
try {
6464
String sni;
65-
if (callback.isClientSide()) {
65+
if (tlsContext instanceof UpstreamTlsContext) {
6666
UpstreamTlsContext upstreamTlsContext = ((UpstreamTlsContext) tlsContext);
6767
sni = upstreamTlsContext.getAutoHostSni() ? callback.getHostname() : upstreamTlsContext.getSni();
6868
} else {
@@ -75,33 +75,21 @@ public synchronized void updateSslContext(final SslContextProvider.Callback call
7575
}
7676
// we want to increment the ref-count so call findOrCreate again...
7777
final SslContextProvider toRelease = getSslContextProvider(sni);
78-
if (toRelease instanceof CertProviderClientSslContextProvider
79-
&& ((CertProviderClientSslContextProvider) toRelease).isUsingSystemRootCerts()) {
80-
callback.getExecutor().execute(() -> {
81-
try {
82-
callback.updateSslContext(GrpcSslContexts.forClient().build(), sni);
83-
releaseSslContextProvider(toRelease, sni);
84-
} catch (SSLException e) {
85-
callback.onException(e);
86-
}
87-
});
88-
} else {
89-
toRelease.addCallback(
90-
new SslContextProvider.Callback(callback.getExecutor()) {
91-
92-
@Override
93-
public void updateSslContext(SslContext sslContext, String sni) {
94-
callback.updateSslContext(sslContext, sni);
95-
releaseSslContextProvider(toRelease, sni);
96-
}
97-
98-
@Override
99-
public void onException(Throwable throwable) {
100-
callback.onException(throwable);
101-
releaseSslContextProvider(toRelease, sni);
102-
}
103-
});
104-
};
78+
toRelease.addCallback(
79+
new SslContextProvider.Callback(callback.getExecutor()) {
80+
81+
@Override
82+
public void updateSslContext(SslContext sslContext, String sni) {
83+
callback.updateSslContext(sslContext, sni);
84+
releaseSslContextProvider(toRelease, sni);
85+
}
86+
87+
@Override
88+
public void onException(Throwable throwable) {
89+
callback.onException(throwable);
90+
releaseSslContextProvider(toRelease, sni);
91+
}
92+
});
10593
} catch (final Throwable throwable) {
10694
callback.getExecutor().execute(new Runnable() {
10795
@Override

‎xds/src/main/java/io/grpc/xds/internal/security/trust/XdsX509TrustManager.java‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
import com.google.common.annotations.VisibleForTesting;
2222
import com.google.common.base.Optional;
2323
import com.google.common.base.Strings;
24+
import com.google.common.collect.ImmutableList;
2425
import com.google.common.collect.ImmutableMap;
2526
import com.google.re2j.Pattern;
2627
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
@@ -60,21 +61,34 @@ final class XdsX509TrustManager extends X509ExtendedTrustManager implements X509
6061
private final X509ExtendedTrustManager delegate;
6162
private final Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates;
6263
private final CertificateValidationContext certContext;
64+
private final String sni;
6365

6466
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
6567
X509ExtendedTrustManager delegate) {
68+
this(certContext, delegate, null);
69+
}
70+
71+
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
72+
X509ExtendedTrustManager delegate, @Nullable String sni) {
6673
checkNotNull(delegate, "delegate");
6774
this.certContext = certContext;
6875
this.delegate = delegate;
6976
this.spiffeTrustMapDelegates = null;
77+
this.sni = sni;
78+
}
79+
80+
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
81+
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates) {
82+
this(certContext, spiffeTrustMapDelegates, null);
7083
}
7184

7285
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
73-
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates) {
86+
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates, @Nullable String sni) {
7487
checkNotNull(spiffeTrustMapDelegates, "spiffeTrustMapDelegates");
7588
this.spiffeTrustMapDelegates = ImmutableMap.copyOf(spiffeTrustMapDelegates);
7689
this.certContext = certContext;
7790
this.delegate = null;
91+
this.sni = sni;
7892
}
7993

8094
private static boolean verifyDnsNameInPattern(
@@ -208,7 +222,7 @@ void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws Certifi
208222
return;
209223
}
210224
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
211-
List<StringMatcher> verifyList = certContext.getMatchSubjectAltNamesList();
225+
List<StringMatcher> verifyList = sni != null? ImmutableList.of(StringMatcher.newBuilder().setExact(sni).build()) : certContext.getMatchSubjectAltNamesList();
212226
if (verifyList.isEmpty()) {
213227
return;
214228
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -811,10 +811,10 @@ public void endpointAddressesAttachedWithClusterName() {
811811
new FixedResultPicker(PickResult.withSubchannel(subchannel)));
812812
}
813813
});
814-
assertThat(subchannel.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo(
814+
assertThat(subchannel.getAttributes().get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME)).isEqualTo(
815815
"authority-host-name");
816816
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
817-
assertThat(eag.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME))
817+
assertThat(eag.getAttributes().get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME))
818818
.isEqualTo("authority-host-name");
819819
}
820820

@@ -863,9 +863,9 @@ public void endpointAddressesAttachedWithClusterName() {
863863
}
864864
});
865865
// Sub Channel wrapper args won't have the address name although addresses will.
866-
assertThat(subchannel.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME)).isNull();
866+
assertThat(subchannel.getAttributes().get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME)).isNull();
867867
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
868-
assertThat(eag.getAttributes().get(XdsAttributes.ATTR_ADDRESS_NAME))
868+
assertThat(eag.getAttributes().get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME))
869869
.isEqualTo("authority-host-name");
870870
}
871871

@@ -1019,7 +1019,7 @@ public String toString() {
10191019
// Unique but arbitrary string
10201020
.set(XdsAttributes.ATTR_LOCALITY_NAME, locality.toString());
10211021
if (authorityHostname != null) {
1022-
attributes.set(XdsAttributes.ATTR_ADDRESS_NAME, authorityHostname);
1022+
attributes.set(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME, authorityHostname);
10231023
}
10241024
EquivalentAddressGroup eag = new EquivalentAddressGroup(new FakeSocketAddress(name),
10251025
attributes.build());

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
import io.grpc.xds.client.XdsClient;
8585
import io.grpc.xds.client.XdsResourceType;
8686
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
87+
import io.grpc.xds.internal.security.SecurityProtocolNegotiators;
8788
import java.net.InetSocketAddress;
8889
import java.net.SocketAddress;
8990
import java.net.URI;
@@ -378,7 +379,7 @@ public void edsClustersEndpointHostname_addedToAddressAttribute() {
378379

379380
assertThat(
380381
childBalancer.addresses.get(0).getAttributes()
381-
.get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
382+
.get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
382383
}
383384

384385
@Test
@@ -864,9 +865,9 @@ void do_onlyLogicalDnsCluster_endpointsResolved() {
864865
Collections.<DropOverload>emptyList(), "pick_first");
865866
assertAddressesEqual(Arrays.asList(endpoint1, endpoint2), childBalancer.addresses);
866867
assertThat(childBalancer.addresses.get(0).getAttributes()
867-
.get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME);
868+
.get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME);
868869
assertThat(childBalancer.addresses.get(1).getAttributes()
869-
.get(XdsAttributes.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME);
870+
.get(SecurityProtocolNegotiators.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME);
870871
}
871872

872873
@Test

‎xds/src/test/java/io/grpc/xds/internal/security/SecurityProtocolNegotiatorsTest.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,8 @@
8686
@RunWith(JUnit4.class)
8787
public class SecurityProtocolNegotiatorsTest {
8888

89+
private static final String HOSTNAME = "hostname";
90+
8991
private final GrpcHttp2ConnectionHandler grpcHandler =
9092
FakeGrpcHttp2ConnectionHandler.newHandler();
9193

@@ -157,7 +159,7 @@ public void clientSecurityHandler_addLast()
157159
new SslContextProviderSupplier(upstreamTlsContext,
158160
new TlsContextManagerImpl(bootstrapInfoForClient));
159161
ClientSecurityHandler clientSecurityHandler =
160-
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier);
162+
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME);
161163
pipeline.addLast(clientSecurityHandler);
162164
channelHandlerCtx = pipeline.context(clientSecurityHandler);
163165
assertNotNull(channelHandlerCtx);
@@ -369,7 +371,7 @@ public void clientSecurityProtocolNegotiatorNewHandler_fireProtocolNegotiationEv
369371
new SslContextProviderSupplier(upstreamTlsContext,
370372
new TlsContextManagerImpl(bootstrapInfoForClient));
371373
ClientSecurityHandler clientSecurityHandler =
372-
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier);
374+
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME);
373375

374376
pipeline.addLast(clientSecurityHandler);
375377
channelHandlerCtx = pipeline.context(clientSecurityHandler);
@@ -420,7 +422,7 @@ public void clientSecurityProtocolNegotiatorNewHandler_handleHandlerRemoved() {
420422
new SslContextProviderSupplier(upstreamTlsContext,
421423
new TlsContextManagerImpl(bootstrapInfoForClient));
422424
ClientSecurityHandler clientSecurityHandler =
423-
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier);
425+
new ClientSecurityHandler(grpcHandler, sslContextProviderSupplier, HOSTNAME);
424426

425427
pipeline.addLast(clientSecurityHandler);
426428
channelHandlerCtx = pipeline.context(clientSecurityHandler);

0 commit comments

Comments
 (0)