Skip to content

Commit 15c8161

Browse files
kannanjgithubkannanjgithub
committed
In-progress changes to move authority verification to the ChannelHandler instead of the Protocol negotiator.
1 parent b8b70bf commit 15c8161

File tree

8 files changed

+105
-70
lines changed

8 files changed

+105
-70
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package io.grpc.internal;
2+
3+
import io.grpc.Status;
4+
5+
public interface AuthorityVerifier {
6+
Status verifyAuthority(String authority);
7+
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,5 +42,7 @@ public final class GrpcAttributes {
4242
public static final Attributes.Key<Attributes> ATTR_CLIENT_EAG_ATTRS =
4343
Attributes.Key.create("io.grpc.internal.GrpcAttributes.clientEagAttrs");
4444

45+
public static final Attributes.Key<AuthorityVerifier> ATTR_AUTHORITY_VERIFIER =
46+
Attributes.Key.create("io.grpc.internal.GrpcAttributes.authorityVerifier");
4547
private GrpcAttributes() {}
4648
}

‎examples/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ dependencies {
4545
protobuf {
4646
protoc { artifact = "com.google.protobuf:protoc:${protocVersion}" }
4747
plugins {
48-
grpc { artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}" }
48+
grpc { artifact = "io.grpc:protoc-gen-grpc-java:1.70.0" }
4949
}
5050
generateProtoTasks {
5151
all()*.plugins { grpc {} }

‎examples/example-tls/build.gradle‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ dependencies {
3434
protobuf {
3535
protoc { artifact = "com.google.protobuf:protoc:${protocVersion}" }
3636
plugins {
37-
grpc { artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}" }
37+
grpc { artifact = "io.grpc:protoc-gen-grpc-java:1.70.0" }
3838
}
3939
generateProtoTasks {
4040
all()*.plugins { grpc {} }

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -170,7 +170,7 @@ public static ChannelHandler clientTlsHandler(
170170
ChannelHandler next, SslContext sslContext, String authority,
171171
ChannelLogger negotiationLogger) {
172172
return new ClientTlsHandler(next, sslContext, authority, null, negotiationLogger,
173-
Optional.absent(), null);
173+
Optional.absent(), null, null);
174174
}
175175

176176
public static class ProtocolNegotiationHandler

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

Lines changed: 12 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -585,25 +585,6 @@ protected void userEventTriggered0(ChannelHandlerContext ctx, Object evt) throws
585585
}
586586

587587
static final class ClientTlsProtocolNegotiator implements ProtocolNegotiator {
588-
private static final Method checkServerTrustedMethod;
589-
590-
static {
591-
Method method = null;
592-
try {
593-
Class<?> x509ExtendedTrustManagerClass =
594-
Class.forName("javax.net.ssl.X509ExtendedTrustManager");
595-
method = x509ExtendedTrustManagerClass.getMethod("checkServerTrusted",
596-
X509Certificate[].class, String.class, SSLEngine.class);
597-
} catch (ClassNotFoundException e) {
598-
// Per-rpc authority overriding via call options will be disallowed.
599-
} catch (NoSuchMethodException e) {
600-
// Should never happen since X509ExtendedTrustManager was introduced in Android API level 24
601-
// along with checkServerTrusted.
602-
}
603-
checkServerTrustedMethod = method;
604-
}
605-
606-
private SSLEngine sslEngine;
607588

608589
public ClientTlsProtocolNegotiator(SslContext sslContext,
609590
ObjectPool<? extends Executor> executorPool, Optional<Runnable> handshakeCompleteRunnable,
@@ -633,7 +614,8 @@ public ChannelHandler newHandler(GrpcHttp2ConnectionHandler grpcHandler) {
633614
ChannelHandler gnh = new GrpcNegotiationHandler(grpcHandler);
634615
ChannelLogger negotiationLogger = grpcHandler.getNegotiationLogger();
635616
ChannelHandler cth = new ClientTlsHandler(gnh, sslContext, grpcHandler.getAuthority(),
636-
this.executor, negotiationLogger, handshakeCompleteRunnable, this);
617+
this.executor, negotiationLogger, handshakeCompleteRunnable, this,
618+
x509ExtendedTrustManager);
637619
return new WaitUntilActiveHandler(cth, negotiationLogger);
638620
}
639621

@@ -644,47 +626,6 @@ public void close() {
644626
}
645627
}
646628

647-
@Override
648-
public Status verifyAuthority(@Nonnull String authority) {
649-
// sslEngine won't be set when creating ClientTlsHandler from InternalProtocolNegotiators
650-
// for example.
651-
if (sslEngine == null || x509ExtendedTrustManager == null) {
652-
return Status.FAILED_PRECONDITION.withDescription(
653-
"Can't allow authority override in rpc when SslEngine or X509ExtendedTrustManager"
654-
+ " is not available");
655-
}
656-
Status peerVerificationStatus;
657-
try {
658-
verifyAuthorityAllowedForPeerCert(authority);
659-
peerVerificationStatus = Status.OK;
660-
} catch (SSLPeerUnverifiedException | CertificateException | InvocationTargetException
661-
| IllegalAccessException | IllegalStateException e) {
662-
peerVerificationStatus = Status.UNAVAILABLE.withDescription(
663-
String.format("Peer hostname verification during rpc failed for authority '%s'",
664-
authority)).withCause(e);
665-
}
666-
return peerVerificationStatus;
667-
}
668-
669-
public void setSslEngine(SSLEngine sslEngine) {
670-
this.sslEngine = sslEngine;
671-
}
672-
673-
private void verifyAuthorityAllowedForPeerCert(String authority)
674-
throws SSLPeerUnverifiedException, CertificateException, InvocationTargetException,
675-
IllegalAccessException {
676-
SSLEngine sslEngineWrapper = new SslEngineWrapper(sslEngine, authority);
677-
// The typecasting of Certificate to X509Certificate should work because this method will only
678-
// be called when using TLS and thus X509.
679-
Certificate[] peerCertificates = sslEngine.getSession().getPeerCertificates();
680-
X509Certificate[] x509PeerCertificates = new X509Certificate[peerCertificates.length];
681-
for (int i = 0; i < peerCertificates.length; i++) {
682-
x509PeerCertificates[i] = (X509Certificate) peerCertificates[i];
683-
}
684-
checkServerTrustedMethod.invoke(
685-
x509ExtendedTrustManager, x509PeerCertificates, "RSA", sslEngineWrapper);
686-
}
687-
688629
@VisibleForTesting
689630
boolean hasX509ExtendedTrustManager() {
690631
return x509ExtendedTrustManager != null;
@@ -699,11 +640,13 @@ static final class ClientTlsHandler extends ProtocolNegotiationHandler {
699640
private final ClientTlsProtocolNegotiator clientTlsProtocolNegotiator;
700641
private Executor executor;
701642
private final Optional<Runnable> handshakeCompleteRunnable;
643+
private final X509TrustManager x509ExtendedTrustManager;
702644

703645
ClientTlsHandler(ChannelHandler next, SslContext sslContext, String authority,
704646
Executor executor, ChannelLogger negotiationLogger,
705647
Optional<Runnable> handshakeCompleteRunnable,
706-
ClientTlsProtocolNegotiator clientTlsProtocolNegotiator) {
648+
ClientTlsProtocolNegotiator clientTlsProtocolNegotiator,
649+
X509TrustManager x509ExtendedTrustManager) {
707650
super(next, negotiationLogger);
708651
this.sslContext = checkNotNull(sslContext, "sslContext");
709652
HostPort hostPort = parseAuthority(authority);
@@ -712,6 +655,7 @@ static final class ClientTlsHandler extends ProtocolNegotiationHandler {
712655
this.executor = executor;
713656
this.handshakeCompleteRunnable = handshakeCompleteRunnable;
714657
this.clientTlsProtocolNegotiator = clientTlsProtocolNegotiator;
658+
this.x509ExtendedTrustManager = x509ExtendedTrustManager;
715659
}
716660

717661
@Override
@@ -724,7 +668,12 @@ protected void handlerAdded0(ChannelHandlerContext ctx) {
724668
ctx.pipeline().addBefore(ctx.name(), /* name= */ null, this.executor != null
725669
? new SslHandler(sslEngine, false, this.executor)
726670
: new SslHandler(sslEngine, false));
727-
clientTlsProtocolNegotiator.setSslEngine(sslEngine);
671+
ProtocolNegotiationEvent existingPne = getProtocolNegotiationEvent();
672+
Attributes attrs = existingPne.getAttributes().toBuilder()
673+
.set(GrpcAttributes.ATTR_AUTHORITY_VERIFIER, new X509AuthorityVerifier(
674+
sslEngine, x509ExtendedTrustManager))
675+
.build();
676+
replaceProtocolNegotiationEvent(existingPne.withAttributes(attrs));
728677
}
729678

730679
@Override

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

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
package io.grpc.netty;
2+
3+
import io.grpc.Status;
4+
import io.grpc.internal.AuthorityVerifier;
5+
import java.lang.reflect.InvocationTargetException;
6+
import java.lang.reflect.Method;
7+
import java.security.cert.Certificate;
8+
import java.security.cert.CertificateException;
9+
import java.security.cert.X509Certificate;
10+
import javax.annotation.Nonnull;
11+
import javax.net.ssl.SSLEngine;
12+
import javax.net.ssl.SSLPeerUnverifiedException;
13+
import javax.net.ssl.X509TrustManager;
14+
15+
public class X509AuthorityVerifier implements AuthorityVerifier {
16+
private final SSLEngine sslEngine;
17+
private final X509TrustManager x509ExtendedTrustManager;
18+
19+
private static final Method checkServerTrustedMethod;
20+
21+
static {
22+
Method method = null;
23+
try {
24+
Class<?> x509ExtendedTrustManagerClass =
25+
Class.forName("javax.net.ssl.X509ExtendedTrustManager");
26+
method = x509ExtendedTrustManagerClass.getMethod("checkServerTrusted",
27+
X509Certificate[].class, String.class, SSLEngine.class);
28+
} catch (ClassNotFoundException e) {
29+
// Per-rpc authority overriding via call options will be disallowed.
30+
} catch (NoSuchMethodException e) {
31+
// Should never happen since X509ExtendedTrustManager was introduced in Android API level 24
32+
// along with checkServerTrusted.
33+
}
34+
checkServerTrustedMethod = method;
35+
}
36+
37+
public X509AuthorityVerifier(SSLEngine sslEngine, X509TrustManager x509ExtendedTrustManager) {
38+
this.sslEngine = sslEngine;
39+
this.x509ExtendedTrustManager = x509ExtendedTrustManager;
40+
}
41+
42+
public Status verifyAuthority(@Nonnull String authority) {
43+
// sslEngine won't be set when creating ClientTlsHandler from InternalProtocolNegotiators
44+
// for example.
45+
if (sslEngine == null || x509ExtendedTrustManager == null) {
46+
return Status.FAILED_PRECONDITION.withDescription(
47+
"Can't allow authority override in rpc when SslEngine or X509ExtendedTrustManager"
48+
+ " is not available");
49+
}
50+
Status peerVerificationStatus;
51+
try {
52+
verifyAuthorityAllowedForPeerCert(authority);
53+
peerVerificationStatus = Status.OK;
54+
} catch (SSLPeerUnverifiedException | CertificateException | InvocationTargetException
55+
| IllegalAccessException | IllegalStateException e) {
56+
peerVerificationStatus = Status.UNAVAILABLE.withDescription(
57+
String.format("Peer hostname verification during rpc failed for authority '%s'",
58+
authority)).withCause(e);
59+
}
60+
return peerVerificationStatus;
61+
}
62+
63+
private void verifyAuthorityAllowedForPeerCert(String authority)
64+
throws SSLPeerUnverifiedException, CertificateException, InvocationTargetException,
65+
IllegalAccessException {
66+
SSLEngine sslEngineWrapper = new ProtocolNegotiators.SslEngineWrapper(sslEngine, authority);
67+
// The typecasting of Certificate to X509Certificate should work because this method will only
68+
// be called when using TLS and thus X509.
69+
Certificate[] peerCertificates = sslEngine.getSession().getPeerCertificates();
70+
X509Certificate[] x509PeerCertificates = new X509Certificate[peerCertificates.length];
71+
for (int i = 0; i < peerCertificates.length; i++) {
72+
x509PeerCertificates[i] = (X509Certificate) peerCertificates[i];
73+
}
74+
checkServerTrustedMethod.invoke(
75+
x509ExtendedTrustManager, x509PeerCertificates, "RSA", sslEngineWrapper);
76+
}
77+
}

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -921,7 +921,7 @@ public String applicationProtocol() {
921921

922922
ClientTlsHandler handler = new ClientTlsHandler(grpcHandler, sslContext,
923923
"authority", elg, noopLogger, Optional.absent(),
924-
getClientTlsProtocolNegotiator());
924+
getClientTlsProtocolNegotiator(), null);
925925
pipeline.addLast(handler);
926926
pipeline.replace(SslHandler.class, null, goodSslHandler);
927927
pipeline.fireUserEventTriggered(ProtocolNegotiationEvent.DEFAULT);
@@ -960,7 +960,7 @@ public String applicationProtocol() {
960960

961961
ClientTlsHandler handler = new ClientTlsHandler(grpcHandler, sslContext,
962962
"authority", elg, noopLogger, Optional.absent(),
963-
getClientTlsProtocolNegotiator());
963+
getClientTlsProtocolNegotiator(), null);
964964
pipeline.addLast(handler);
965965
pipeline.replace(SslHandler.class, null, goodSslHandler);
966966
pipeline.fireUserEventTriggered(ProtocolNegotiationEvent.DEFAULT);
@@ -985,7 +985,7 @@ public String applicationProtocol() {
985985

986986
ClientTlsHandler handler = new ClientTlsHandler(grpcHandler, sslContext,
987987
"authority", elg, noopLogger, Optional.absent(),
988-
getClientTlsProtocolNegotiator());
988+
getClientTlsProtocolNegotiator(), null);
989989
pipeline.addLast(handler);
990990

991991
final AtomicReference<Throwable> error = new AtomicReference<>();
@@ -1014,7 +1014,7 @@ public void exceptionCaught(ChannelHandlerContext ctx, Throwable cause) {
10141014
public void clientTlsHandler_closeDuringNegotiation() throws Exception {
10151015
ClientTlsHandler handler = new ClientTlsHandler(grpcHandler, sslContext,
10161016
"authority", null, noopLogger, Optional.absent(),
1017-
getClientTlsProtocolNegotiator());
1017+
getClientTlsProtocolNegotiator(), null);
10181018
pipeline.addLast(new WriteBufferingAndExceptionHandler(handler));
10191019
ChannelFuture pendingWrite = channel.writeAndFlush(NettyClientHandler.NOOP_MESSAGE);
10201020

0 commit comments

Comments
 (0)