Skip to content

Commit 42058ea

Browse files
kannanjgithubkannanjgithub
committed
Revert unintended changes.
1 parent 1d6f46f commit 42058ea

File tree

6 files changed

+76
-52
lines changed

6 files changed

+76
-52
lines changed

‎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:1.70.0" }
48+
grpc { artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}" }
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:1.70.0" }
37+
grpc { artifact = "io.grpc:protoc-gen-grpc-java:${grpcVersion}" }
3838
}
3939
generateProtoTasks {
4040
all()*.plugins { grpc {} }

‎examples/example-tls/src/main/java/io/grpc/examples/helloworldtls/HelloWorldClientTls.java‎

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616

1717
package io.grpc.examples.helloworldtls;
1818

19-
import static io.grpc.examples.helloworld.GreeterGrpc.getSayHelloMethod;
20-
21-
import io.grpc.CallOptions;
2219
import io.grpc.Channel;
2320
import io.grpc.Grpc;
2421
import io.grpc.ManagedChannel;
@@ -28,7 +25,6 @@
2825
import io.grpc.examples.helloworld.HelloReply;
2926
import io.grpc.examples.helloworld.HelloRequest;
3027
import java.io.File;
31-
import java.io.IOException;
3228
import java.util.concurrent.TimeUnit;
3329
import java.util.logging.Level;
3430
import java.util.logging.Logger;
@@ -56,11 +52,7 @@ public void greet(String name) {
5652
HelloRequest request = HelloRequest.newBuilder().setName(name).build();
5753
HelloReply response;
5854
try {
59-
CallOptions callOptions = blockingStub.getCallOptions()
60-
.withAuthority("moo.test.goog.fr");
61-
response = io.grpc.stub.ClientCalls.blockingUnaryCall(
62-
blockingStub.getChannel(), getSayHelloMethod(),
63-
callOptions, request);
55+
response = blockingStub.sayHello(request);
6456
} catch (StatusRuntimeException e) {
6557
logger.log(Level.WARNING, "RPC failed: {0}", e.getStatus());
6658
return;

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

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,17 @@ protected boolean isGracefulShutdownComplete() {
586586
&& ((StreamBufferingEncoder) encoder()).numBufferedStreams() == 0;
587587
}
588588

589+
private String getAuthorityPseudoHeader(Http2Headers http2Headers) {
590+
if (http2Headers instanceof GrpcHttp2OutboundHeaders) {
591+
return ((GrpcHttp2OutboundHeaders) http2Headers).getAuthority();
592+
}
593+
try {
594+
return http2Headers.authority().toString();
595+
} catch (UnsupportedOperationException e) {
596+
return null;
597+
}
598+
}
599+
589600
/**
590601
* Attempts to create a new stream from the given command. If there are too many active streams,
591602
* the creation request is queued.
@@ -602,15 +613,16 @@ private void createStream(CreateStreamCommand command, ChannelPromise promise)
602613
return;
603614
}
604615

605-
String authority = ((GrpcHttp2OutboundHeaders) command.headers()).getAuthority();
616+
String authority = getAuthorityPseudoHeader(command.headers());
606617
if (authority != null) {
607618
Status authorityVerificationStatus = peerVerificationResults.get(authority);
608-
if (authorityVerificationStatus == null) {
619+
if (authorityVerificationStatus == null && attributes != null
620+
&& attributes.get(GrpcAttributes.ATTR_AUTHORITY_VERIFIER) != null) {
609621
authorityVerificationStatus = attributes.get(GrpcAttributes.ATTR_AUTHORITY_VERIFIER)
610622
.verifyAuthority(((GrpcHttp2OutboundHeaders) command.headers()).getAuthority());
611623
peerVerificationResults.put(authority, authorityVerificationStatus);
612624
}
613-
if (!authorityVerificationStatus.isOk()) {
625+
if (authorityVerificationStatus != null && !authorityVerificationStatus.isOk()) {
614626
logger.log(Level.WARNING, String.format("%s.%s",
615627
authorityVerificationStatus.getDescription(), enablePerRpcAuthorityCheck
616628
? "" : "This will be an error in the future."),

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,9 @@ public Status verifyAuthority(@Nonnull String authority) {
6565
}
6666
Status peerVerificationStatus;
6767
try {
68-
verifyAuthorityAllowedForPeerCert(authority);
68+
// Because the authority pseudo-header can contain a port number:
69+
// https://www.rfc-editor.org/rfc/rfc7540#section-8.1.2.3com
70+
verifyAuthorityAllowedForPeerCert(removeAnyPortNumber(authority));
6971
peerVerificationStatus = Status.OK;
7072
} catch (SSLPeerUnverifiedException | CertificateException | InvocationTargetException
7173
| IllegalAccessException | IllegalStateException e) {
@@ -76,6 +78,15 @@ public Status verifyAuthority(@Nonnull String authority) {
7678
return peerVerificationStatus;
7779
}
7880

81+
private String removeAnyPortNumber(String authority) {
82+
int closingSquareBracketIndex = authority.lastIndexOf(']');
83+
int portNumberSeperatorColonIndex = authority.lastIndexOf(':');
84+
if (portNumberSeperatorColonIndex > closingSquareBracketIndex) {
85+
return authority.substring(0, portNumberSeperatorColonIndex);
86+
}
87+
return authority;
88+
}
89+
7990
private void verifyAuthorityAllowedForPeerCert(String authority)
8091
throws SSLPeerUnverifiedException, CertificateException, InvocationTargetException,
8192
IllegalAccessException {

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

Lines changed: 46 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -37,9 +37,7 @@
3737
import static org.junit.Assert.assertTrue;
3838
import static org.junit.Assert.fail;
3939
import static org.mockito.ArgumentMatchers.any;
40-
import static org.mockito.Mockito.mock;
4140
import static org.mockito.Mockito.timeout;
42-
import static org.mockito.Mockito.times;
4341
import static org.mockito.Mockito.verify;
4442
import static org.mockito.Mockito.when;
4543

@@ -139,8 +137,6 @@
139137
import org.junit.Test;
140138
import org.junit.runner.RunWith;
141139
import org.junit.runners.JUnit4;
142-
import org.mockito.AdditionalAnswers;
143-
import org.mockito.ArgumentCaptor;
144140
import org.mockito.Mock;
145141
import org.mockito.junit.MockitoJUnit;
146142
import org.mockito.junit.MockitoRule;
@@ -955,47 +951,58 @@ public void authorityOverrideInCallOptions_matchesServerPeerHost_newStreamCreati
955951
}
956952
}
957953

954+
// Without removing the port number part that {@link X509AuthorityVerifier} does, there will be a
955+
// java.security.cert.CertificateException: Illegal given domain name: foo.test.google.fr:12345
958956
@Test
959-
public void authorityOverrideInCallOptions_lruCache()
960-
throws IOException, InterruptedException, GeneralSecurityException, ExecutionException,
961-
TimeoutException {
957+
public void authorityOverrideInCallOptions_portNumberInAuthority_isStrippedForPeerVerification()
958+
throws IOException, InterruptedException, GeneralSecurityException, ExecutionException,
959+
TimeoutException {
962960
NettyClientHandler.enablePerRpcAuthorityCheck = true;
963961
try {
964962
startServer();
965-
ProtocolNegotiator mockNegotiator =
966-
mock(ProtocolNegotiator.class, AdditionalAnswers.delegatesTo(newNegotiator()));
967-
NettyClientTransport transport = newTransport(mockNegotiator);
963+
NettyClientTransport transport = newTransport(newNegotiator());
968964
SettableFuture<Void> connected = SettableFuture.create();
969965
FakeClientTransportListener fakeClientTransportListener =
970-
new FakeClientTransportListener(connected);
966+
new FakeClientTransportListener(connected);
971967
callMeMaybe(transport.start(fakeClientTransportListener));
972968
connected.get(10, TimeUnit.SECONDS);
973969
assertThat(fakeClientTransportListener.isConnected()).isTrue();
974970

975-
ArgumentCaptor<String> authorityArgumentCaptor = ArgumentCaptor.forClass(String.class);
976-
new Rpc(transport, new Metadata(), "foo-0.test.google.fr").halfClose()
977-
.waitForResponse();
978-
// Should use cache.
979-
new Rpc(transport, new Metadata(), "foo-0.test.google.fr").waitForResponse();
980-
for (int i = 1; i < 100; i++) {
981-
// Should call verifyAuthority each time here and the cache grows with each call.
982-
new Rpc(transport, new Metadata(), "foo-" + i + ".test.google.fr").halfClose()
983-
.waitForResponse();
984-
}
985-
// Cache is full at this point. Eviction occurs here for foo-0.test.google.fr.
986-
new Rpc(transport, new Metadata(), "foo-100.test.google.fr").halfClose()
987-
.waitForResponse();
988-
// verifyAuthority call happens for foo-0.test.google.fr.
989-
new Rpc(transport, new Metadata(), "foo-0.test.google.fr").halfClose()
990-
.waitForResponse();
991-
verify(mockNegotiator, times(102)).verifyAuthority(
992-
authorityArgumentCaptor.capture());
993-
List<String> authorityValues = authorityArgumentCaptor.getAllValues();
994-
for (int i = 0; i < 100; i++) {
995-
assertThat(authorityValues.get(i)).isEqualTo("foo-" + i + ".test.google.fr");
996-
}
997-
assertThat(authorityValues.get(100)).isEqualTo("foo-100.test.google.fr");
998-
assertThat(authorityValues.get(101)).isEqualTo("foo-0.test.google.fr");
971+
new Rpc(transport, new Metadata(), "foo.test.google.fr:12345").waitForResponse();
972+
} finally {
973+
NettyClientHandler.enablePerRpcAuthorityCheck = false;;
974+
}
975+
}
976+
977+
@Test
978+
public void authorityOverrideInCallOptions_portNumberAndIpv6_isStrippedForPeerVerification()
979+
throws IOException, InterruptedException, GeneralSecurityException, ExecutionException,
980+
TimeoutException {
981+
NettyClientHandler.enablePerRpcAuthorityCheck = true;
982+
try {
983+
startServer();
984+
NettyClientTransport transport = newTransport(newNegotiator());
985+
SettableFuture<Void> connected = SettableFuture.create();
986+
FakeClientTransportListener fakeClientTransportListener =
987+
new FakeClientTransportListener(connected);
988+
callMeMaybe(transport.start(fakeClientTransportListener));
989+
connected.get(10, TimeUnit.SECONDS);
990+
assertThat(fakeClientTransportListener.isConnected()).isTrue();
991+
992+
new Rpc(transport, new Metadata(), "[2001:db8:3333:4444:5555:6666:1.2.3.4]:12345")
993+
.waitForResponse();
994+
} catch (ExecutionException ex) {
995+
Status status = ((StatusException) ex.getCause()).getStatus();
996+
assertThat(status.getDescription()).isEqualTo("Peer hostname verification during rpc "
997+
+ "failed for authority '[2001:db8:3333:4444:5555:6666:1.2.3.4]:12345'");
998+
assertThat(status.getCode()).isEqualTo(Code.UNAVAILABLE);
999+
assertThat(((InvocationTargetException) ex.getCause().getCause()).getTargetException())
1000+
.isInstanceOf(CertificateException.class);
1001+
// Port number is removed by {@link X509AuthorityVerifier}.
1002+
assertThat(((InvocationTargetException) ex.getCause().getCause()).getTargetException()
1003+
.getMessage()).isEqualTo(
1004+
"No subject alternative names matching IP address 2001:db8:3333:4444:5555:6666:1.2.3.4 "
1005+
+ "found");
9991006
} finally {
10001007
NettyClientHandler.enablePerRpcAuthorityCheck = false;;
10011008
}
@@ -1182,9 +1189,11 @@ private static class Rpc {
11821189

11831190
Rpc(NettyClientTransport transport, Metadata headers, String authorityOverride) {
11841191
stream = transport.newStream(
1185-
METHOD, headers, authorityOverride != null
1186-
? CallOptions.DEFAULT.withAuthority(authorityOverride) : CallOptions.DEFAULT,
1192+
METHOD, headers, CallOptions.DEFAULT,
11871193
new ClientStreamTracer[]{ new ClientStreamTracer() {} });
1194+
if (authorityOverride != null) {
1195+
stream.setAuthority(authorityOverride);
1196+
}
11881197
stream.start(listener);
11891198
stream.request(1);
11901199
stream.writeMessage(new ByteArrayInputStream(MESSAGE.getBytes(UTF_8)));

0 commit comments

Comments
 (0)