Skip to content

Commit 5e2e22e

Browse files
kannanjgithubkannanjgithub
committed
Netty authority verify against peer host in server cert.
1 parent 0152478 commit 5e2e22e

File tree

3 files changed

+227
-43
lines changed

3 files changed

+227
-43
lines changed

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

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -81,17 +81,13 @@
8181
import java.security.cert.CertificateException;
8282
import java.security.cert.X509Certificate;
8383
import java.util.Arrays;
84-
import java.util.Collections;
8584
import java.util.EnumSet;
86-
import java.util.List;
8785
import java.util.Optional;
8886
import java.util.Set;
8987
import java.util.concurrent.Executor;
9088
import java.util.logging.Level;
9189
import java.util.logging.Logger;
9290
import javax.annotation.Nullable;
93-
import javax.net.ssl.ExtendedSSLSession;
94-
import javax.net.ssl.SNIServerName;
9591
import javax.net.ssl.SSLEngine;
9692
import javax.net.ssl.SSLEngineResult;
9793
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
@@ -197,28 +193,6 @@ public static FromChannelCredentialsResult from(ChannelCredentials creds) {
197193
}
198194
}
199195

200-
private static Optional<TrustManager> getX509ExtendedTrustManager(InputStream rootCerts)
201-
throws GeneralSecurityException {
202-
KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
203-
try {
204-
ks.load(null, null);
205-
} catch (IOException ex) {
206-
// Shouldn't really happen, as we're not loading any data.
207-
throw new GeneralSecurityException(ex);
208-
}
209-
X509Certificate[] certs = CertificateUtils.getX509Certificates(rootCerts);
210-
for (X509Certificate cert : certs) {
211-
X500Principal principal = cert.getSubjectX500Principal();
212-
ks.setCertificateEntry(principal.getName("RFC2253"), cert);
213-
}
214-
215-
TrustManagerFactory trustManagerFactory =
216-
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
217-
trustManagerFactory.init(ks);
218-
return Arrays.stream(trustManagerFactory.getTrustManagers())
219-
.filter(trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
220-
}
221-
222196
public static FromServerCredentialsResult from(ServerCredentials creds) {
223197
if (creds instanceof TlsServerCredentials) {
224198
TlsServerCredentials tlsCreds = (TlsServerCredentials) creds;
@@ -297,6 +271,28 @@ public static FromServerCredentialsResult from(ServerCredentials creds) {
297271
}
298272
}
299273

274+
private static Optional<TrustManager> getX509ExtendedTrustManager(InputStream rootCerts)
275+
throws GeneralSecurityException {
276+
KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
277+
try {
278+
ks.load(null, null);
279+
} catch (IOException ex) {
280+
// Shouldn't really happen, as we're not loading any data.
281+
throw new GeneralSecurityException(ex);
282+
}
283+
X509Certificate[] certs = CertificateUtils.getX509Certificates(rootCerts);
284+
for (X509Certificate cert : certs) {
285+
X500Principal principal = cert.getSubjectX500Principal();
286+
ks.setCertificateEntry(principal.getName("RFC2253"), cert);
287+
}
288+
289+
TrustManagerFactory trustManagerFactory =
290+
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
291+
trustManagerFactory.init(ks);
292+
return Arrays.stream(trustManagerFactory.getTrustManagers())
293+
.filter(trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
294+
}
295+
300296
public static final class FromChannelCredentialsResult {
301297
public final ProtocolNegotiator.ClientFactory negotiator;
302298
public final CallCredentials callCredentials;
@@ -650,7 +646,7 @@ boolean canVerifyAuthorityOverride() {
650646

651647
public void verifyAuthorityAllowedForPeerCert(String authority)
652648
throws SSLPeerUnverifiedException, CertificateException {
653-
SSLEngine sslEngineWrapper = new SSLEngineWrapper(sslEngine, authority);
649+
SSLEngine sslEngineWrapper = new SslEngineWrapper(sslEngine, authority);
654650
// The typecasting of Certificate to X509Certificate should work because this method will only
655651
// be called when there is a X509ExtendedTrustManager available.
656652
Certificate[] peerCertificates = sslEngine.getSession().getPeerCertificates();
@@ -664,6 +660,11 @@ public void verifyAuthorityAllowedForPeerCert(String authority)
664660
public void setSslEngine(SSLEngine sslEngine) {
665661
this.sslEngine = sslEngine;
666662
}
663+
664+
@VisibleForTesting
665+
boolean hasX509ExtendedTrustManager() {
666+
return x509ExtendedTrustManager != null;
667+
}
667668
}
668669

669670
static final class ClientTlsHandler extends ProtocolNegotiationHandler {
@@ -1204,12 +1205,12 @@ protected final void fireProtocolNegotiationEvent(ChannelHandlerContext ctx) {
12041205
}
12051206
}
12061207

1207-
static final class SSLEngineWrapper extends SSLEngine {
1208+
static final class SslEngineWrapper extends SSLEngine {
12081209

12091210
private final SSLEngine sslEngine;
12101211
private final String peerHost;
12111212

1212-
SSLEngineWrapper(SSLEngine sslEngine, String peerHost) {
1213+
SslEngineWrapper(SSLEngine sslEngine, String peerHost) {
12131214
this.sslEngine = sslEngine;
12141215
this.peerHost = peerHost;
12151216
}
@@ -1221,7 +1222,7 @@ public String getPeerHost() {
12211222

12221223
@Override
12231224
public SSLSession getHandshakeSession() {
1224-
return new FakeSSLSession(peerHost);
1225+
return new FakeSslSession(peerHost);
12251226
}
12261227

12271228
@Override
@@ -1352,10 +1353,10 @@ public boolean getEnableSessionCreation() {
13521353
}
13531354
}
13541355

1355-
static class FakeSSLSession implements SSLSession {
1356+
static class FakeSslSession implements SSLSession {
13561357
private final String peerHost;
13571358

1358-
FakeSSLSession(String peerHost) {
1359+
FakeSslSession(String peerHost) {
13591360
this.peerHost = peerHost;
13601361
}
13611362

@@ -1370,8 +1371,9 @@ public SSLSessionContext getSessionContext() {
13701371
}
13711372

13721373
@Override
1373-
public javax.security.cert.X509Certificate[] getPeerCertificateChain() throws SSLPeerUnverifiedException {
1374-
throw new UnsupportedOperationException("This method is deprecated and marked for removal. Use the getPeerCertificates() method instead.");
1374+
public javax.security.cert.X509Certificate[] getPeerCertificateChain() {
1375+
throw new UnsupportedOperationException("This method is deprecated and marked for removal. "
1376+
+ "Use the getPeerCertificates() method instead.");
13751377
}
13761378

13771379
@Override

‎netty/src/test/java/io/grpc/netty/NettyClientTransportTest.java‎

Lines changed: 133 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,11 @@
5959
import io.grpc.internal.ClientStream;
6060
import io.grpc.internal.ClientStreamListener;
6161
import io.grpc.internal.ClientTransport;
62+
import io.grpc.internal.FailingClientStream;
6263
import io.grpc.internal.FakeClock;
6364
import io.grpc.internal.FixedObjectPool;
6465
import io.grpc.internal.GrpcUtil;
66+
import io.grpc.internal.InsightBuilder;
6567
import io.grpc.internal.ManagedClientTransport;
6668
import io.grpc.internal.ServerListener;
6769
import io.grpc.internal.ServerStream;
@@ -73,6 +75,7 @@
7375
import io.grpc.netty.NettyChannelBuilder.LocalSocketPicker;
7476
import io.grpc.netty.NettyTestUtil.TrackingObjectPoolForTest;
7577
import io.grpc.testing.TlsTesting;
78+
import io.grpc.util.CertificateUtils;
7679
import io.netty.buffer.ByteBuf;
7780
import io.netty.channel.Channel;
7881
import io.netty.channel.ChannelConfig;
@@ -100,7 +103,12 @@
100103
import java.net.InetSocketAddress;
101104
import java.net.SocketAddress;
102105
import java.nio.charset.StandardCharsets;
106+
import java.security.GeneralSecurityException;
107+
import java.security.KeyStore;
108+
import java.security.cert.CertificateException;
109+
import java.security.cert.X509Certificate;
103110
import java.util.ArrayList;
111+
import java.util.Arrays;
104112
import java.util.Collections;
105113
import java.util.HashMap;
106114
import java.util.List;
@@ -114,6 +122,11 @@
114122
import javax.annotation.Nullable;
115123
import javax.net.ssl.SSLException;
116124
import javax.net.ssl.SSLHandshakeException;
125+
import javax.net.ssl.TrustManager;
126+
import javax.net.ssl.TrustManagerFactory;
127+
import javax.net.ssl.X509ExtendedTrustManager;
128+
import javax.net.ssl.X509TrustManager;
129+
import javax.security.auth.x500.X500Principal;
117130
import org.junit.After;
118131
import org.junit.Before;
119132
import org.junit.Rule;
@@ -199,7 +212,7 @@ public void addDefaultUserAgent() throws Exception {
199212
}
200213

201214
@Test
202-
public void setSoLingerChannelOption() throws IOException {
215+
public void setSoLingerChannelOption() throws IOException, GeneralSecurityException {
203216
startServer();
204217
Map<ChannelOption<?>, Object> channelOptions = new HashMap<>();
205218
// set SO_LINGER option
@@ -817,17 +830,86 @@ public void tlsNegotiationServerExecutorShouldSucceed() throws Exception {
817830
assertEquals(false, serverExecutorPool.isInUse());
818831
}
819832

833+
@Test
834+
public void authorityOverrideInCallOptions_doesntMatchServerPeerHost_newStreamCreationFails()
835+
throws IOException, InterruptedException, GeneralSecurityException {
836+
startServer();
837+
NettyClientTransport transport = newTransport(newNegotiator());
838+
FakeClientTransportListener fakeClientTransportListener = new FakeClientTransportListener();
839+
callMeMaybe(transport.start(fakeClientTransportListener));
840+
synchronized (fakeClientTransportListener) {
841+
fakeClientTransportListener.wait(10000);
842+
}
843+
assertThat(fakeClientTransportListener.isConnected).isTrue();
844+
845+
ClientStream stream = transport.newStream(
846+
Rpc.METHOD, new Metadata(), CallOptions.DEFAULT.withAuthority("foo.test.google.in"),
847+
new ClientStreamTracer[]{new ClientStreamTracer() {
848+
}});
849+
850+
assertThat(stream).isInstanceOf(FailingClientStream.class);
851+
InsightBuilder insightBuilder = new InsightBuilder();
852+
stream.appendTimeoutInsight(insightBuilder);
853+
assertThat(insightBuilder.toString()).contains(
854+
"Status{code=INTERNAL, description=Peer hostname verification failed for authority, "
855+
+ "cause=null}");
856+
}
857+
858+
@Test
859+
public void authorityOverrideInCallOptions_matchesServerPeerHost_newStreamCreationSucceeds()
860+
throws IOException, InterruptedException, GeneralSecurityException {
861+
startServer();
862+
NettyClientTransport transport = newTransport(newNegotiator());
863+
FakeClientTransportListener fakeClientTransportListener = new FakeClientTransportListener();
864+
callMeMaybe(transport.start(fakeClientTransportListener));
865+
synchronized (fakeClientTransportListener) {
866+
fakeClientTransportListener.wait(10000);
867+
}
868+
assertThat(fakeClientTransportListener.isConnected).isTrue();
869+
870+
ClientStream stream = transport.newStream(
871+
Rpc.METHOD, new Metadata(), CallOptions.DEFAULT.withAuthority("zoo.test.google.fr"),
872+
new ClientStreamTracer[]{new ClientStreamTracer() {
873+
}});
874+
875+
assertThat(stream).isNotInstanceOf(FailingClientStream.class);
876+
}
877+
820878
private Throwable getRootCause(Throwable t) {
821879
if (t.getCause() == null) {
822880
return t;
823881
}
824882
return getRootCause(t.getCause());
825883
}
826884

827-
private ProtocolNegotiator newNegotiator() throws IOException {
885+
private ProtocolNegotiator newNegotiator() throws IOException, GeneralSecurityException {
828886
InputStream caCert = TlsTesting.loadCert("ca.pem");
829887
SslContext clientContext = GrpcSslContexts.forClient().trustManager(caCert).build();
830-
return ProtocolNegotiators.tls(clientContext, null);
888+
return ProtocolNegotiators.tls(clientContext,
889+
(X509ExtendedTrustManager) getX509ExtendedTrustManager(
890+
TlsTesting.loadCert("ca.pem")).get());
891+
}
892+
893+
private static Optional<TrustManager> getX509ExtendedTrustManager(InputStream rootCerts)
894+
throws GeneralSecurityException {
895+
KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
896+
try {
897+
ks.load(null, null);
898+
} catch (IOException ex) {
899+
// Shouldn't really happen, as we're not loading any data.
900+
throw new GeneralSecurityException(ex);
901+
}
902+
X509Certificate[] certs = CertificateUtils.getX509Certificates(rootCerts);
903+
for (X509Certificate cert : certs) {
904+
X500Principal principal = cert.getSubjectX500Principal();
905+
ks.setCertificateEntry(principal.getName("RFC2253"), cert);
906+
}
907+
908+
TrustManagerFactory trustManagerFactory =
909+
TrustManagerFactory.getInstance(TrustManagerFactory.getDefaultAlgorithm());
910+
trustManagerFactory.init(ks);
911+
return Arrays.stream(trustManagerFactory.getTrustManagers())
912+
.filter(trustManager -> trustManager instanceof X509ExtendedTrustManager).findFirst();
831913
}
832914

833915
private NettyClientTransport newTransport(ProtocolNegotiator negotiator) {
@@ -951,7 +1033,7 @@ private static class Rpc {
9511033

9521034
Rpc(NettyClientTransport transport, Metadata headers) {
9531035
stream = transport.newStream(
954-
METHOD, headers, CallOptions.DEFAULT.withAuthority("wrong-authority"),
1036+
METHOD, headers, CallOptions.DEFAULT,
9551037
new ClientStreamTracer[]{ new ClientStreamTracer() {} });
9561038
stream.start(listener);
9571039
stream.request(1);
@@ -1144,4 +1226,51 @@ public void log(ChannelLogLevel level, String message) {}
11441226
@Override
11451227
public void log(ChannelLogLevel level, String messageFormat, Object... args) {}
11461228
}
1229+
1230+
static class FakeTrustManager implements X509TrustManager {
1231+
1232+
@Override
1233+
public void checkClientTrusted(X509Certificate[] x509Certificates, String s)
1234+
throws CertificateException {
1235+
1236+
}
1237+
1238+
@Override
1239+
public void checkServerTrusted(X509Certificate[] x509Certificates, String s)
1240+
throws CertificateException {
1241+
1242+
}
1243+
1244+
@Override
1245+
public X509Certificate[] getAcceptedIssuers() {
1246+
return new X509Certificate[0];
1247+
}
1248+
}
1249+
1250+
static class FakeClientTransportListener implements ManagedClientTransport.Listener {
1251+
private boolean isConnected = false;
1252+
1253+
@Override
1254+
public void transportShutdown(Status s) {
1255+
1256+
}
1257+
1258+
@Override
1259+
public void transportTerminated() {
1260+
1261+
}
1262+
1263+
@Override
1264+
public void transportReady() {
1265+
isConnected = true;
1266+
synchronized (this) {
1267+
notify();
1268+
}
1269+
}
1270+
1271+
@Override
1272+
public void transportInUse(boolean inUse) {
1273+
1274+
}
1275+
}
11471276
}

0 commit comments

Comments
 (0)