Skip to content

Commit 8902dbd

Browse files
committed
Address review comments.
1 parent 862b95b commit 8902dbd

File tree

5 files changed

+20
-24
lines changed

5 files changed

+20
-24
lines changed

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,21 +26,19 @@
2626
import java.security.cert.X509Certificate;
2727
import java.util.Arrays;
2828
import java.util.Collection;
29-
import java.util.Optional;
29+
import java.util.List;
3030
import javax.net.ssl.TrustManager;
3131
import javax.net.ssl.TrustManagerFactory;
32-
import javax.net.ssl.X509ExtendedTrustManager;
3332
import javax.security.auth.x500.X500Principal;
3433

3534
/**
3635
* Contains certificate/key PEM file utility method(s) for internal usage.
3736
*/
3837
public class CertificateUtils {
3938
/**
40-
* Creates a X509ExtendedTrustManager using the provided CA certs if applicable for the
41-
* certificate type.
39+
* Creates a X509TrustManagers using the provided CA certs.
4240
*/
43-
public static Optional<TrustManager> getX509ExtendedTrustManager(InputStream rootCerts)
41+
public static List<TrustManager> getTrustManagers(InputStream rootCerts)
4442
throws GeneralSecurityException {
4543
KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
4644
try {
@@ -58,8 +56,7 @@ public static Optional<TrustManager> getX509ExtendedTrustManager(InputStream roo
5856
TrustManagerFactory trustManagerFactory =
5957
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
6058
trustManagerFactory.init(ks);
61-
return Arrays.stream(trustManagerFactory.getTrustManagers())
62-
.filter(trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
59+
return Arrays.asList(trustManagerFactory.getTrustManagers());
6360
}
6461

6562
private static X509Certificate[] getX509Certificates(InputStream inputStream)

netty/src/main/java/io/grpc/netty/NettyClientTransport.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -196,7 +196,7 @@ public ClientStream newStream(
196196
}
197197
if (callOptions.getAuthority() != null) {
198198
Status verificationStatus = negotiator.verifyAuthority(callOptions.getAuthority());
199-
if (!verificationStatus.equals(Status.OK)) {
199+
if (!verificationStatus.isOk()) {
200200
return new FailingClientStream(verificationStatus, tracers);
201201
}
202202
}

netty/src/main/java/io/grpc/netty/NoopSslEngine.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626
* A no-op implementation of SslEngine, to facilitate overriding only the required methods in
2727
* specific implementations.
2828
*/
29-
public class NoopSslEngine extends SSLEngine {
29+
class NoopSslEngine extends SSLEngine {
3030
@Override
3131
public SSLEngineResult wrap(ByteBuffer[] srcs, int offset, int length, ByteBuffer dst)
3232
throws SSLException {

netty/src/main/java/io/grpc/netty/ProtocolNegotiator.java

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ interface ServerFactory {
6969
* Verify the authority against peer if applicable depending on the transport credential type.
7070
*/
7171
default Status verifyAuthority(String authority) {
72-
return Status.UNAVAILABLE;
72+
return Status.UNAVAILABLE.withDescription("Per-rpc authority verification not implemented by "
73+
+ "the protocol negotiator");
7374
}
7475
}

netty/src/main/java/io/grpc/netty/ProtocolNegotiators.java

Lines changed: 12 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import static com.google.common.base.Preconditions.checkNotNull;
2020
import static com.google.common.base.Preconditions.checkState;
21-
import static io.grpc.internal.CertificateUtils.getX509ExtendedTrustManager;
2221

2322
import com.google.common.annotations.VisibleForTesting;
2423
import com.google.common.base.Preconditions;
@@ -42,6 +41,7 @@
4241
import io.grpc.Status;
4342
import io.grpc.TlsChannelCredentials;
4443
import io.grpc.TlsServerCredentials;
44+
import io.grpc.internal.CertificateUtils;
4545
import io.grpc.internal.GrpcAttributes;
4646
import io.grpc.internal.GrpcUtil;
4747
import io.grpc.internal.ObjectPool;
@@ -79,6 +79,7 @@
7979
import java.util.Arrays;
8080
import java.util.EnumSet;
8181
import java.util.LinkedHashMap;
82+
import java.util.List;
8283
import java.util.Map;
8384
import java.util.Optional;
8485
import java.util.Set;
@@ -130,27 +131,24 @@ public static FromChannelCredentialsResult from(ChannelCredentials creds) {
130131
new ByteArrayInputStream(tlsCreds.getPrivateKey()),
131132
tlsCreds.getPrivateKeyPassword());
132133
}
133-
Optional<TrustManager> x509ExtendedTrustManager;
134134
try {
135+
List<TrustManager> trustManagers;
135136
if (tlsCreds.getTrustManagers() != null) {
136-
builder.trustManager(new FixedTrustManagerFactory(tlsCreds.getTrustManagers()));
137-
x509ExtendedTrustManager = tlsCreds.getTrustManagers().stream().filter(
138-
trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
137+
trustManagers = tlsCreds.getTrustManagers();
139138
} else if (tlsCreds.getRootCertificates() != null) {
140-
builder.trustManager(new ByteArrayInputStream(tlsCreds.getRootCertificates()));
141-
x509ExtendedTrustManager = getX509ExtendedTrustManager(new ByteArrayInputStream(
142-
tlsCreds.getRootCertificates()));
139+
trustManagers = CertificateUtils.getTrustManagers(
140+
new ByteArrayInputStream(tlsCreds.getRootCertificates()));
143141
} else { // else use system default
144142
TrustManagerFactory tmf = TrustManagerFactory.getInstance(
145143
TrustManagerFactory.getDefaultAlgorithm());
146144
tmf.init((KeyStore) null);
147-
x509ExtendedTrustManager = Arrays.stream(tmf.getTrustManagers())
148-
.filter(trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
145+
trustManagers = Arrays.asList(tmf.getTrustManagers());
149146
}
147+
builder.trustManager(new FixedTrustManagerFactory(trustManagers));
148+
Optional<TrustManager> x509ExtendedTrustManager = trustManagers.stream().filter(
149+
trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
150150
return FromChannelCredentialsResult.negotiator(tlsClientFactory(builder.build(),
151-
x509ExtendedTrustManager.isPresent()
152-
? (X509ExtendedTrustManager) x509ExtendedTrustManager.get()
153-
: null));
151+
(X509ExtendedTrustManager) x509ExtendedTrustManager.orElse(null)));
154152
} catch (SSLException | GeneralSecurityException ex) {
155153
log.log(Level.FINE, "Exception building SslContext", ex);
156154
return FromChannelCredentialsResult.error(
@@ -1233,7 +1231,7 @@ public SSLParameters getSSLParameters() {
12331231
}
12341232
}
12351233

1236-
static class FakeSslSession extends NoopSslSession {
1234+
static final class FakeSslSession extends NoopSslSession {
12371235
private final String peerHost;
12381236

12391237
FakeSslSession(String peerHost) {

0 commit comments

Comments
 (0)