Skip to content

Commit fa4e498

Browse files
kannanjgithubkannanjgithub
committed
Use X509TrustManager everywhere instead of TrustManager, and some other review comments.
1 parent 232ca14 commit fa4e498

File tree

12 files changed

+42
-43
lines changed

12 files changed

+42
-43
lines changed

‎core/src/main/java/io/grpc/internal/CertificateUtils.java‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
import java.util.List;
3030
import javax.net.ssl.TrustManager;
3131
import javax.net.ssl.TrustManagerFactory;
32+
import javax.net.ssl.X509TrustManager;
3233
import javax.security.auth.x500.X500Principal;
3334

3435
/**
@@ -85,11 +86,11 @@ public static TrustManager[] createTrustManager(InputStream rootCerts)
8586
return trustManagerFactory.getTrustManagers();
8687
}
8788

88-
public static TrustManager getX509ExtendedTrustManager(List<TrustManager> trustManagers) {
89+
public static X509TrustManager getX509ExtendedTrustManager(List<TrustManager> trustManagers) {
8990
if (x509ExtendedTrustManagerClass != null) {
9091
for (TrustManager trustManager : trustManagers) {
9192
if (x509ExtendedTrustManagerClass.isInstance(trustManager)) {
92-
return trustManager;
93+
return (X509TrustManager) trustManager;
9394
}
9495
}
9596
}

‎netty/src/main/java/io/grpc/netty/InternalProtocolNegotiators.java‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626
import io.netty.handler.ssl.SslContext;
2727
import io.netty.util.AsciiString;
2828
import java.util.concurrent.Executor;
29-
import javax.net.ssl.TrustManager;
3029
import javax.net.ssl.X509TrustManager;
3130

3231
/**
@@ -45,10 +44,10 @@ private InternalProtocolNegotiators() {}
4544
public static InternalProtocolNegotiator.ProtocolNegotiator tls(SslContext sslContext,
4645
ObjectPool<? extends Executor> executorPool,
4746
Optional<Runnable> handshakeCompleteRunnable,
48-
TrustManager extendedX509TrustManager,
47+
X509TrustManager extendedX509TrustManager,
4948
String sni) {
5049
final io.grpc.netty.ProtocolNegotiator negotiator = ProtocolNegotiators.tls(sslContext,
51-
executorPool, handshakeCompleteRunnable, (X509TrustManager) extendedX509TrustManager, sni);
50+
executorPool, handshakeCompleteRunnable, extendedX509TrustManager, sni);
5251
final class TlsNegotiator implements InternalProtocolNegotiator.ProtocolNegotiator {
5352

5453
@Override
@@ -77,7 +76,7 @@ public void close() {
7776
*/
7877
public static InternalProtocolNegotiator.ProtocolNegotiator tls(
7978
SslContext sslContext, String sni,
80-
TrustManager extendedX509TrustManager) {
79+
X509TrustManager extendedX509TrustManager) {
8180
return tls(sslContext, null, Optional.absent(), extendedX509TrustManager, sni);
8281
}
8382

‎netty/src/main/java/io/grpc/netty/NettyChannelBuilder.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -652,8 +652,7 @@ static ProtocolNegotiator createProtocolNegotiatorByType(
652652
case PLAINTEXT_UPGRADE:
653653
return ProtocolNegotiators.plaintextUpgrade();
654654
case TLS:
655-
return ProtocolNegotiators.tls(
656-
sslContext, executorPool, Optional.absent(), null, null);
655+
return ProtocolNegotiators.tls(sslContext, executorPool, Optional.absent(), null, null);
657656
default:
658657
throw new IllegalArgumentException("Unsupported negotiationType: " + negotiationType);
659658
}

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,15 @@
3434
import java.util.ArrayList;
3535
import java.util.List;
3636
import javax.annotation.Nullable;
37-
import javax.net.ssl.TrustManager;
37+
import javax.net.ssl.X509TrustManager;
3838

3939
/** Base class for dynamic {@link SslContextProvider}s. */
4040
@Internal
4141
public abstract class DynamicSslContextProvider extends SslContextProvider {
4242

4343
protected final List<Callback> pendingCallbacks = new ArrayList<>();
4444
@Nullable protected final CertificateValidationContext staticCertificateValidationContext;
45-
@Nullable protected AbstractMap.SimpleImmutableEntry<SslContext, TrustManager>
45+
@Nullable protected AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
4646
sslContextAndTrustManager;
4747

4848
protected DynamicSslContextProvider(
@@ -52,15 +52,15 @@ protected DynamicSslContextProvider(
5252
}
5353

5454
@Nullable
55-
public AbstractMap.SimpleImmutableEntry<SslContext, TrustManager>
55+
public AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
5656
getSslContextAndTrustManager() {
5757
return sslContextAndTrustManager;
5858
}
5959

6060
protected abstract CertificateValidationContext generateCertificateValidationContext();
6161

6262
/** Gets a server or client side SslContextBuilder. */
63-
protected abstract AbstractMap.SimpleImmutableEntry<SslContextBuilder, TrustManager>
63+
protected abstract AbstractMap.SimpleImmutableEntry<SslContextBuilder, X509TrustManager>
6464
getSslContextBuilderAndTrustManager(
6565
CertificateValidationContext certificateValidationContext)
6666
throws CertificateException, IOException, CertStoreException;
@@ -70,7 +70,7 @@ protected final void updateSslContext() {
7070
try {
7171
CertificateValidationContext localCertValidationContext =
7272
generateCertificateValidationContext();
73-
AbstractMap.SimpleImmutableEntry<SslContextBuilder, TrustManager> sslContextBuilderAndTm =
73+
AbstractMap.SimpleImmutableEntry<SslContextBuilder, X509TrustManager> sslContextBuilderAndTm =
7474
getSslContextBuilderAndTrustManager(localCertValidationContext);
7575
CommonTlsContext commonTlsContext = getCommonTlsContext();
7676
if (commonTlsContext != null && commonTlsContext.getAlpnProtocolsCount() > 0) {
@@ -84,7 +84,7 @@ protected final void updateSslContext() {
8484
sslContextBuilderAndTm.getKey().applicationProtocolConfig(apn);
8585
}
8686
List<Callback> pendingCallbacksCopy;
87-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager>
87+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
8888
sslContextAndExtendedX09TrustManagerCopy;
8989
synchronized (pendingCallbacks) {
9090
sslContextAndTrustManager = new AbstractMap.SimpleImmutableEntry<>(
@@ -101,11 +101,11 @@ protected final void updateSslContext() {
101101

102102
protected final void callPerformCallback(
103103
Callback callback,
104-
final AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTmCopy) {
104+
final AbstractMap.SimpleImmutableEntry<SslContext,X509TrustManager> sslContextAndTmCopy) {
105105
performCallback(
106106
new SslContextGetter() {
107107
@Override
108-
public AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> get() {
108+
public AbstractMap.SimpleImmutableEntry<SslContext,X509TrustManager> get() {
109109
return sslContextAndTmCopy;
110110
}
111111
},
@@ -117,7 +117,7 @@ public AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> get() {
117117
public final void addCallback(Callback callback) {
118118
checkNotNull(callback, "callback");
119119
// if there is a computed sslContext just send it
120-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextCopy = null;
120+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextCopy = null;
121121
synchronized (pendingCallbacks) {
122122
if (sslContextAndTrustManager != null) {
123123
sslContextCopy = sslContextAndTrustManager;
@@ -131,7 +131,7 @@ public final void addCallback(Callback callback) {
131131
}
132132

133133
private final void makePendingCallbacks(
134-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager>
134+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager>
135135
sslContextAndExtendedX509TrustManagerCopy,
136136
List<Callback> pendingCallbacksCopy) {
137137
for (Callback callback : pendingCallbacksCopy) {

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@
4848
import java.util.logging.Level;
4949
import java.util.logging.Logger;
5050
import javax.annotation.Nullable;
51-
import javax.net.ssl.TrustManager;
51+
import javax.net.ssl.X509TrustManager;
5252

5353
/**
5454
* Provides client and server side gRPC {@link ProtocolNegotiator}s to provide the SSL
@@ -237,7 +237,7 @@ protected void handlerAdded0(final ChannelHandlerContext ctx) {
237237

238238
@Override
239239
public void updateSslContextAndExtendedX509TrustManager(
240-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
240+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextAndTm) {
241241
if (ctx.isRemoved()) {
242242
return;
243243
}
@@ -383,7 +383,7 @@ protected void handlerAdded0(final ChannelHandlerContext ctx) {
383383

384384
@Override
385385
public void updateSslContextAndExtendedX509TrustManager(
386-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
386+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextAndTm) {
387387
ChannelHandler handler = InternalProtocolNegotiators.serverTls(
388388
sslContextAndTm.getKey()).newHandler(grpcHandler);
389389

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@
3434
import java.security.cert.CertificateException;
3535
import java.util.AbstractMap;
3636
import java.util.concurrent.Executor;
37-
import javax.net.ssl.TrustManager;
37+
import javax.net.ssl.X509TrustManager;
3838

3939
/**
4040
* A SslContextProvider is a "container" or provider of SslContext. This is used by gRPC-xds to
@@ -60,7 +60,7 @@ protected Callback(Executor executor) {
6060

6161
/** Informs callee of new/updated SslContext. */
6262
@VisibleForTesting public abstract void updateSslContextAndExtendedX509TrustManager(
63-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext);
63+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContext);
6464

6565
/** Informs callee of an exception that was generated. */
6666
@VisibleForTesting protected abstract void onException(Throwable throwable);
@@ -122,7 +122,7 @@ protected final void performCallback(
122122
@Override
123123
public void run() {
124124
try {
125-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm =
125+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextAndTm =
126126
sslContextGetter.get();
127127
callback.updateSslContextAndExtendedX509TrustManager(sslContextAndTm);
128128
} catch (Throwable e) {
@@ -134,6 +134,6 @@ public void run() {
134134

135135
/** Allows implementations to compute or get SslContext. */
136136
protected interface SslContextGetter {
137-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> get() throws Exception;
137+
AbstractMap.SimpleImmutableEntry<SslContext,X509TrustManager> get() throws Exception;
138138
}
139139
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import io.netty.handler.ssl.SslContext;
2828
import java.util.AbstractMap;
2929
import java.util.Objects;
30-
import javax.net.ssl.TrustManager;
30+
import javax.net.ssl.X509TrustManager;
3131

3232
/**
3333
* Enables Client or server side to initialize this object with the received {@link BaseTlsContext}
@@ -70,7 +70,7 @@ public synchronized void updateSslContext(
7070

7171
@Override
7272
public void updateSslContextAndExtendedX509TrustManager(
73-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm) {
73+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextAndTm) {
7474
callback.updateSslContextAndExtendedX509TrustManager(sslContextAndTm);
7575
releaseSslContextProvider(toRelease);
7676
}

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderClientSslContextProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
import java.util.Arrays;
3131
import java.util.Map;
3232
import javax.annotation.Nullable;
33-
import javax.net.ssl.TrustManager;
33+
import javax.net.ssl.X509TrustManager;
3434

3535
/** A client SslContext provider using CertificateProviderInstance to fetch secrets. */
3636
final class CertProviderClientSslContextProvider extends CertProviderSslContextProvider {
@@ -54,7 +54,7 @@ final class CertProviderClientSslContextProvider extends CertProviderSslContextP
5454
}
5555

5656
@Override
57-
protected final AbstractMap.SimpleImmutableEntry<SslContextBuilder, TrustManager>
57+
protected final AbstractMap.SimpleImmutableEntry<SslContextBuilder, X509TrustManager>
5858
getSslContextBuilderAndTrustManager(
5959
CertificateValidationContext certificateValidationContext)
6060
throws CertStoreException {

‎xds/src/main/java/io/grpc/xds/internal/security/certprovider/CertProviderServerSslContextProvider.java‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
import java.util.AbstractMap;
3434
import java.util.Map;
3535
import javax.annotation.Nullable;
36-
import javax.net.ssl.TrustManager;
36+
import javax.net.ssl.X509TrustManager;
3737

3838
/** A server SslContext provider using CertificateProviderInstance to fetch secrets. */
3939
final class CertProviderServerSslContextProvider extends CertProviderSslContextProvider {
@@ -57,7 +57,7 @@ final class CertProviderServerSslContextProvider extends CertProviderSslContextP
5757
}
5858

5959
@Override
60-
protected final AbstractMap.SimpleImmutableEntry<SslContextBuilder, TrustManager>
60+
protected final AbstractMap.SimpleImmutableEntry<SslContextBuilder, X509TrustManager>
6161
getSslContextBuilderAndTrustManager(
6262
CertificateValidationContext certificateValidationContextdationContext)
6363
throws CertStoreException, CertificateException, IOException {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
import java.util.List;
4242
import java.util.concurrent.Executor;
4343
import javax.annotation.Nullable;
44-
import javax.net.ssl.TrustManager;
44+
import javax.net.ssl.X509TrustManager;
4545

4646
/** Utility class for client and server ssl provider tests. */
4747
public class CommonTlsContextTestsUtil {
@@ -405,7 +405,7 @@ private static CommonTlsContext.Builder addNewCertificateValidationContext(
405405

406406
/** Perform some simple checks on sslContext. */
407407
public static void doChecksOnSslContext(boolean server,
408-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContextAndTm,
408+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContextAndTm,
409409
List<String> expectedApnProtos) {
410410
if (server) {
411411
assertThat(sslContextAndTm.getKey().isServer()).isTrue();
@@ -438,7 +438,7 @@ public static TestCallback getValueThruCallback(SslContextProvider provider, Exe
438438

439439
public static class TestCallback extends SslContextProvider.Callback {
440440

441-
public AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> updatedSslContext;
441+
public AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> updatedSslContext;
442442
public Throwable updatedThrowable;
443443

444444
public TestCallback(Executor executor) {
@@ -447,7 +447,7 @@ public TestCallback(Executor executor) {
447447

448448
@Override
449449
public void updateSslContextAndExtendedX509TrustManager(
450-
AbstractMap.SimpleImmutableEntry<SslContext, TrustManager> sslContext) {
450+
AbstractMap.SimpleImmutableEntry<SslContext, X509TrustManager> sslContext) {
451451
updatedSslContext = sslContext;
452452
}
453453

0 commit comments

Comments
 (0)