Skip to content

Commit 7f7877a

Browse files
kannanjgithubkannanjgithub
committed
TrustManager to server name from SslEngine instead of from ProtocolNegotiator.
1 parent 107cbd8 commit 7f7877a

13 files changed

Lines changed: 140 additions & 80 deletions

‎api/src/main/java/io/grpc/EquivalentAddressGroup.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -55,10 +55,7 @@ public final class EquivalentAddressGroup {
5555
*/
5656
public static final Attributes.Key<String> ATTR_LOCALITY_NAME =
5757
Attributes.Key.create("io.grpc.EquivalentAddressGroup.LOCALITY");
58-
/** Name associated with individual address, if available (e.g., DNS name). */
59-
@Attr
60-
public static final Attributes.Key<String> ATTR_ADDRESS_NAME =
61-
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
58+
6259
private final List<SocketAddress> addrs;
6360
private final Attributes attrs;
6461

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

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@
5353
import io.grpc.xds.client.XdsClient;
5454
import io.grpc.xds.client.XdsLogger;
5555
import io.grpc.xds.client.XdsLogger.XdsLogLevel;
56+
import io.grpc.xds.internal.XdsInternalAttributes;
5657
import io.grpc.xds.internal.security.SecurityProtocolNegotiators;
5758
import io.grpc.xds.internal.security.SslContextProviderSupplier;
5859
import io.grpc.xds.orca.OrcaPerRequestUtil;
@@ -241,9 +242,9 @@ public Subchannel createSubchannel(CreateSubchannelArgs args) {
241242
.set(ATTR_CLUSTER_LOCALITY, localityAtomicReference);
242243
if (GrpcUtil.getFlag("GRPC_EXPERIMENTAL_XDS_AUTHORITY_REWRITE", false)) {
243244
String hostname = args.getAddresses().get(0).getAttributes()
244-
.get(EquivalentAddressGroup.ATTR_ADDRESS_NAME);
245+
.get(XdsInternalAttributes.ATTR_ADDRESS_NAME);
245246
if (hostname != null) {
246-
attrsBuilder.set(EquivalentAddressGroup.ATTR_ADDRESS_NAME, hostname);
247+
attrsBuilder.set(XdsInternalAttributes.ATTR_ADDRESS_NAME, hostname);
247248
}
248249
}
249250
args = args.toBuilder().setAddresses(addresses).setAttributes(attrsBuilder.build()).build();
@@ -439,7 +440,7 @@ public PickResult pickSubchannel(PickSubchannelArgs args) {
439440
result = PickResult.withSubchannel(result.getSubchannel(),
440441
result.getStreamTracerFactory(),
441442
result.getSubchannel().getAttributes().get(
442-
EquivalentAddressGroup.ATTR_ADDRESS_NAME));
443+
XdsInternalAttributes.ATTR_ADDRESS_NAME));
443444
}
444445
}
445446
return result;

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
import io.grpc.xds.client.Locality;
4848
import io.grpc.xds.client.XdsLogger;
4949
import io.grpc.xds.client.XdsLogger.XdsLogLevel;
50+
import io.grpc.xds.internal.XdsInternalAttributes;
5051
import java.net.InetSocketAddress;
5152
import java.net.SocketAddress;
5253
import java.util.ArrayList;
@@ -194,7 +195,7 @@ StatusOr<ClusterResolutionResult> edsUpdateToResult(
194195
.set(XdsAttributes.ATTR_LOCALITY_WEIGHT,
195196
localityLbInfo.localityWeight())
196197
.set(XdsAttributes.ATTR_SERVER_WEIGHT, weight)
197-
.set(EquivalentAddressGroup.ATTR_ADDRESS_NAME, endpoint.hostname())
198+
.set(XdsInternalAttributes.ATTR_ADDRESS_NAME, endpoint.hostname())
198199
.build();
199200
EquivalentAddressGroup eag;
200201
if (config.isHttp11ProxyAvailable()) {

‎xds/src/main/java/io/grpc/xds/internal/XdsInternalAttributes.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
/*
2+
* Copyright 2022 The gRPC Authors
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package io.grpc.xds.internal;
18+
19+
import io.grpc.Attributes;
20+
import io.grpc.EquivalentAddressGroup;
21+
22+
public final class XdsInternalAttributes {
23+
/** Name associated with individual address, if available (e.g., DNS name). */
24+
@EquivalentAddressGroup.Attr
25+
public static final Attributes.Key<String> ATTR_ADDRESS_NAME =
26+
Attributes.Key.create("io.grpc.xds.XdsAttributes.addressName");
27+
}

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
import io.grpc.netty.ProtocolNegotiationEvent;
3434
import io.grpc.xds.EnvoyServerProtoData;
3535
import io.grpc.xds.EnvoyServerProtoData.UpstreamTlsContext;
36+
import io.grpc.xds.internal.XdsInternalAttributes;
3637
import io.grpc.xds.internal.security.trust.CertificateUtils;
3738
import io.netty.channel.ChannelHandler;
3839
import io.netty.channel.ChannelHandlerAdapter;
@@ -150,7 +151,7 @@ public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
150151
return fallbackProtocolNegotiator.newHandler(grpcHandler);
151152
}
152153
return new ClientSecurityHandler(grpcHandler, localSslContextProviderSupplier,
153-
grpcHandler.getEagAttributes().get(EquivalentAddressGroup.ATTR_ADDRESS_NAME));
154+
grpcHandler.getEagAttributes().get(XdsInternalAttributes.ATTR_ADDRESS_NAME));
154155
}
155156

156157
@Override

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ private static X509Certificate[] getTrustedCaFromCertContext(
130130
static XdsX509TrustManager createX509TrustManager(
131131
X509Certificate[] certs, CertificateValidationContext certContext, String sniForSanMatching)
132132
throws CertStoreException {
133-
return new XdsX509TrustManager(certContext, createTrustManager(certs), sniForSanMatching);
133+
return new XdsX509TrustManager(certContext, createTrustManager(certs));
134134
}
135135

136136
@VisibleForTesting
@@ -144,7 +144,7 @@ static XdsX509TrustManager createX509TrustManager(
144144
delegates.put(entry.getKey(), createTrustManager(
145145
entry.getValue().toArray(new X509Certificate[0])));
146146
}
147-
return new XdsX509TrustManager(certContext, delegates, sniForSanMatching);
147+
return new XdsX509TrustManager(certContext, delegates);
148148
}
149149

150150
private static X509ExtendedTrustManager createTrustManager(X509Certificate[] certs)

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

Lines changed: 28 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import com.google.common.base.Strings;
2424
import com.google.common.collect.ImmutableList;
2525
import com.google.common.collect.ImmutableMap;
26+
import com.google.common.collect.Lists;
2627
import com.google.re2j.Pattern;
2728
import io.envoyproxy.envoy.extensions.transport_sockets.tls.v3.CertificateValidationContext;
2829
import io.envoyproxy.envoy.type.matcher.v3.RegexMatcher;
@@ -32,6 +33,7 @@
3233
import java.security.cert.CertificateException;
3334
import java.security.cert.CertificateParsingException;
3435
import java.security.cert.X509Certificate;
36+
import java.util.ArrayList;
3537
import java.util.Arrays;
3638
import java.util.Collection;
3739
import java.util.HashSet;
@@ -40,6 +42,8 @@
4042
import java.util.Map;
4143
import java.util.Set;
4244
import javax.annotation.Nullable;
45+
import javax.net.ssl.SNIHostName;
46+
import javax.net.ssl.SNIServerName;
4347
import javax.net.ssl.SSLEngine;
4448
import javax.net.ssl.SSLParameters;
4549
import javax.net.ssl.SSLSocket;
@@ -61,30 +65,21 @@ final class XdsX509TrustManager extends X509ExtendedTrustManager implements X509
6165
private final X509ExtendedTrustManager delegate;
6266
private final Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates;
6367
private final CertificateValidationContext certContext;
64-
private final String sniForSanMatching;
6568

6669
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
6770
X509ExtendedTrustManager delegate) {
68-
this(certContext, delegate, null);
69-
}
70-
71-
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
72-
X509ExtendedTrustManager delegate, @Nullable String sniForSanMatching) {
7371
checkNotNull(delegate, "delegate");
7472
this.certContext = certContext;
7573
this.delegate = delegate;
7674
this.spiffeTrustMapDelegates = null;
77-
this.sniForSanMatching = sniForSanMatching;
7875
}
7976

8077
XdsX509TrustManager(@Nullable CertificateValidationContext certContext,
81-
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates,
82-
@Nullable String sniForSanMatching) {
78+
Map<String, X509ExtendedTrustManager> spiffeTrustMapDelegates) {
8379
checkNotNull(spiffeTrustMapDelegates, "spiffeTrustMapDelegates");
8480
this.spiffeTrustMapDelegates = ImmutableMap.copyOf(spiffeTrustMapDelegates);
8581
this.certContext = certContext;
8682
this.delegate = null;
87-
this.sniForSanMatching = sniForSanMatching;
8883
}
8984

9085
private static boolean verifyDnsNameInPattern(
@@ -213,15 +208,10 @@ private static void verifySubjectAltNameInLeaf(
213208
* This is called from various check*Trusted methods.
214209
*/
215210
@VisibleForTesting
216-
void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws CertificateException {
211+
void verifySubjectAltNameInChain(X509Certificate[] peerCertChain, List<StringMatcher> verifyList) throws CertificateException {
217212
if (certContext == null) {
218213
return;
219214
}
220-
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
221-
List<StringMatcher> verifyList =
222-
CertificateUtils.isXdsSniEnabled && !Strings.isNullOrEmpty(sniForSanMatching)
223-
? ImmutableList.of(StringMatcher.newBuilder().setExact(sniForSanMatching).build())
224-
: certContext.getMatchSubjectAltNamesList();
225215
if (verifyList.isEmpty()) {
226216
return;
227217
}
@@ -233,24 +223,27 @@ void verifySubjectAltNameInChain(X509Certificate[] peerCertChain) throws Certifi
233223
}
234224

235225
@Override
226+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
236227
public void checkClientTrusted(X509Certificate[] chain, String authType, Socket socket)
237228
throws CertificateException {
238229
chooseDelegate(chain).checkClientTrusted(chain, authType, socket);
239-
verifySubjectAltNameInChain(chain);
230+
verifySubjectAltNameInChain(chain, certContext.getMatchSubjectAltNamesList());
240231
}
241232

242233
@Override
234+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
243235
public void checkClientTrusted(X509Certificate[] chain, String authType, SSLEngine sslEngine)
244236
throws CertificateException {
245237
chooseDelegate(chain).checkClientTrusted(chain, authType, sslEngine);
246-
verifySubjectAltNameInChain(chain);
238+
verifySubjectAltNameInChain(chain, certContext.getMatchSubjectAltNamesList());
247239
}
248240

249241
@Override
242+
@SuppressWarnings("deprecation") // gRFC A29 predates match_typed_subject_alt_names
250243
public void checkClientTrusted(X509Certificate[] chain, String authType)
251244
throws CertificateException {
252245
chooseDelegate(chain).checkClientTrusted(chain, authType);
253-
verifySubjectAltNameInChain(chain);
246+
verifySubjectAltNameInChain(chain, certContext.getMatchSubjectAltNamesList());
254247
}
255248

256249
@Override
@@ -265,7 +258,22 @@ public void checkServerTrusted(X509Certificate[] chain, String authType, Socket
265258
}
266259
}
267260
chooseDelegate(chain).checkServerTrusted(chain, authType, socket);
268-
verifySubjectAltNameInChain(chain);
261+
List<StringMatcher> sniMatchers = null;
262+
if (CertificateUtils.isXdsSniEnabled) {
263+
List<SNIServerName> serverNames = ((SSLSocket) socket).getSSLParameters().getServerNames();
264+
if (serverNames != null) {
265+
for (SNIServerName name : serverNames) {
266+
if (name instanceof SNIHostName) {
267+
sniMatchers = new ArrayList<>();
268+
sniMatchers.add(StringMatcher.newBuilder().setExact(((SNIHostName) name).getAsciiName()).build());
269+
}
270+
}
271+
}
272+
}
273+
if (sniMatchers == null) {
274+
sniMatchers = certContext.getMatchSubjectAltNamesList();
275+
}
276+
verifySubjectAltNameInChain(chain, sniMatchers);
269277
}
270278

271279
@Override

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

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@
7777
import io.grpc.xds.client.Stats.ClusterStats;
7878
import io.grpc.xds.client.Stats.UpstreamLocalityStats;
7979
import io.grpc.xds.client.XdsClient;
80+
import io.grpc.xds.internal.XdsInternalAttributes;
8081
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
8182
import io.grpc.xds.internal.security.SecurityProtocolNegotiators;
8283
import io.grpc.xds.internal.security.SslContextProvider;
@@ -811,10 +812,10 @@ public void endpointAddressesAttachedWithClusterName() {
811812
new FixedResultPicker(PickResult.withSubchannel(subchannel)));
812813
}
813814
});
814-
assertThat(subchannel.getAttributes().get(EquivalentAddressGroup.ATTR_ADDRESS_NAME)).isEqualTo(
815+
assertThat(subchannel.getAttributes().get(XdsInternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo(
815816
"authority-host-name");
816817
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
817-
assertThat(eag.getAttributes().get(EquivalentAddressGroup.ATTR_ADDRESS_NAME))
818+
assertThat(eag.getAttributes().get(XdsInternalAttributes.ATTR_ADDRESS_NAME))
818819
.isEqualTo("authority-host-name");
819820
}
820821

@@ -863,9 +864,9 @@ public void endpointAddressesAttachedWithClusterName() {
863864
}
864865
});
865866
// Sub Channel wrapper args won't have the address name although addresses will.
866-
assertThat(subchannel.getAttributes().get(EquivalentAddressGroup.ATTR_ADDRESS_NAME)).isNull();
867+
assertThat(subchannel.getAttributes().get(XdsInternalAttributes.ATTR_ADDRESS_NAME)).isNull();
867868
for (EquivalentAddressGroup eag : subchannel.getAllAddresses()) {
868-
assertThat(eag.getAttributes().get(EquivalentAddressGroup.ATTR_ADDRESS_NAME))
869+
assertThat(eag.getAttributes().get(XdsInternalAttributes.ATTR_ADDRESS_NAME))
869870
.isEqualTo("authority-host-name");
870871
}
871872

@@ -1019,7 +1020,7 @@ public String toString() {
10191020
// Unique but arbitrary string
10201021
.set(EquivalentAddressGroup.ATTR_LOCALITY_NAME, locality.toString());
10211022
if (authorityHostname != null) {
1022-
attributes.set(EquivalentAddressGroup.ATTR_ADDRESS_NAME, authorityHostname);
1023+
attributes.set(XdsInternalAttributes.ATTR_ADDRESS_NAME, authorityHostname);
10231024
}
10241025
EquivalentAddressGroup eag = new EquivalentAddressGroup(new FakeSocketAddress(name),
10251026
attributes.build());

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,6 +102,7 @@
102102
import io.grpc.xds.WrrLocalityLoadBalancer.WrrLocalityConfig;
103103
import io.grpc.xds.client.Bootstrapper.ServerInfo;
104104
import io.grpc.xds.client.XdsClient;
105+
import io.grpc.xds.internal.XdsInternalAttributes;
105106
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
106107
import java.net.InetSocketAddress;
107108
import java.net.URI;
@@ -408,7 +409,7 @@ public void edsClustersEndpointHostname_addedToAddressAttribute() {
408409

409410
assertThat(
410411
childBalancer.addresses.get(0).getAttributes()
411-
.get(EquivalentAddressGroup.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
412+
.get(XdsInternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo("hostname1");
412413
}
413414

414415
@Test
@@ -897,7 +898,7 @@ public void onlyLogicalDnsCluster_endpointsResolved() {
897898
newInetSocketAddress("127.0.2.1", 9000), newInetSocketAddress("127.0.2.2", 9000)))),
898899
childBalancer.addresses);
899900
assertThat(childBalancer.addresses.get(0).getAttributes()
900-
.get(EquivalentAddressGroup.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME + ":9000");
901+
.get(XdsInternalAttributes.ATTR_ADDRESS_NAME)).isEqualTo(DNS_HOST_NAME + ":9000");
901902
}
902903

903904
@Test

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,7 @@
7171
import io.grpc.xds.client.Bootstrapper;
7272
import io.grpc.xds.client.CommonBootstrapperTestUtils;
7373
import io.grpc.xds.internal.Matchers.HeaderMatcher;
74+
import io.grpc.xds.internal.XdsInternalAttributes;
7475
import io.grpc.xds.internal.security.CommonTlsContextTestsUtil;
7576
import io.grpc.xds.internal.security.SecurityProtocolNegotiators;
7677
import io.grpc.xds.internal.security.SslContextProviderSupplier;
@@ -839,7 +840,7 @@ private SimpleServiceGrpc.SimpleServiceBlockingStub getBlockingStub(
839840
upstreamTlsContext, tlsContextManagerForClient))
840841
: Attributes.newBuilder();
841842
if (addrNameAttribute != null) {
842-
sslContextAttributesBuilder.set(EquivalentAddressGroup.ATTR_ADDRESS_NAME, addrNameAttribute);
843+
sslContextAttributesBuilder.set(XdsInternalAttributes.ATTR_ADDRESS_NAME, addrNameAttribute);
843844
}
844845
sslContextAttributes = sslContextAttributesBuilder.build();
845846
fakeNameResolverFactory.setServers(

0 commit comments

Comments
 (0)