Skip to content

Commit f996474

Browse files
kannanjgithubkannanjgithub
committed
Added flag with default false for the per-rpc authority check and removed setting sslEndpointAlgorithm.
1 parent 30b1e14 commit f996474

File tree

3 files changed

+204
-114
lines changed

3 files changed

+204
-114
lines changed

‎okhttp/src/main/java/io/grpc/okhttp/OkHttpClientTransport.java‎

Lines changed: 66 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -132,9 +132,12 @@ class OkHttpClientTransport implements ConnectionClientTransport, TransportExcep
132132
OutboundFlowController.Transport {
133133
private static final Map<ErrorCode, Status> ERROR_CODE_TO_STATUS = buildErrorCodeToStatusMap();
134134
private static final Logger log = Logger.getLogger(OkHttpClientTransport.class.getName());
135+
private static final String GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK =
136+
"GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK";
135137
private final ChannelCredentials channelCredentials;
136138
private Socket sock;
137139
private SSLSession sslSession;
140+
private final Logger logger = Logger.getLogger(OkHttpClientTransport.class.getName());
138141

139142
private static Map<ErrorCode, Status> buildErrorCodeToStatusMap() {
140143
Map<ErrorCode, Status> errorToStatus = new EnumMap<>(ErrorCode.class);
@@ -262,14 +265,14 @@ protected void handleNotInUse() {
262265
SettableFuture<Void> connectedFuture;
263266

264267
public OkHttpClientTransport(
265-
OkHttpTransportFactory transportFactory,
266-
InetSocketAddress address,
267-
String authority,
268-
@Nullable String userAgent,
269-
Attributes eagAttrs,
270-
@Nullable HttpConnectProxiedSocketAddress proxiedAddr,
271-
Runnable tooManyPingsRunnable,
272-
ChannelCredentials channelCredentials) {
268+
OkHttpTransportFactory transportFactory,
269+
InetSocketAddress address,
270+
String authority,
271+
@Nullable String userAgent,
272+
Attributes eagAttrs,
273+
@Nullable HttpConnectProxiedSocketAddress proxiedAddr,
274+
Runnable tooManyPingsRunnable,
275+
ChannelCredentials channelCredentials) {
273276
this(
274277
transportFactory,
275278
address,
@@ -284,16 +287,16 @@ public OkHttpClientTransport(
284287
}
285288

286289
private OkHttpClientTransport(
287-
OkHttpTransportFactory transportFactory,
288-
InetSocketAddress address,
289-
String authority,
290-
@Nullable String userAgent,
291-
Attributes eagAttrs,
292-
Supplier<Stopwatch> stopwatchFactory,
293-
Variant variant,
294-
@Nullable HttpConnectProxiedSocketAddress proxiedAddr,
295-
Runnable tooManyPingsRunnable,
296-
ChannelCredentials channelCredentials) {
290+
OkHttpTransportFactory transportFactory,
291+
InetSocketAddress address,
292+
String authority,
293+
@Nullable String userAgent,
294+
Attributes eagAttrs,
295+
Supplier<Stopwatch> stopwatchFactory,
296+
Variant variant,
297+
@Nullable HttpConnectProxiedSocketAddress proxiedAddr,
298+
Runnable tooManyPingsRunnable,
299+
ChannelCredentials channelCredentials) {
297300
this.address = Preconditions.checkNotNull(address, "address");
298301
this.defaultAuthority = authority;
299302
this.maxMessageSize = transportFactory.maxMessageSize;
@@ -433,48 +436,65 @@ public ClientStream newStream(
433436
if (hostnameVerifier != null && socket instanceof SSLSocket
434437
&& !hostnameVerifier.verify(callOptions.getAuthority(),
435438
((SSLSocket) socket).getSession())) {
436-
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
437-
String.format("HostNameVerifier verification failed for authority '%s'",
438-
callOptions.getAuthority())), tracers);
439+
if (GrpcUtil.getFlag(GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK, false)) {
440+
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
441+
String.format("HostNameVerifier verification failed for authority '%s'",
442+
callOptions.getAuthority())), tracers);
443+
}
444+
logger.warning(String.format("HostNameVerifier verification failed for authority '%s'.",
445+
callOptions.getAuthority()));
439446
}
440447
if (socket instanceof SSLSocket && callOptions.getAuthority() != null
441448
&& channelCredentials != null && channelCredentials instanceof TlsChannelCredentials) {
442-
Status peerVerificationStatus;
449+
Status peerVerificationStatus = null;
443450
if (peerVerificationResults.containsKey(callOptions.getAuthority())) {
444451
peerVerificationStatus = peerVerificationResults.get(callOptions.getAuthority());
445452
} else {
446-
TrustManager x509ExtendedTrustManager;
453+
TrustManager x509ExtendedTrustManager = null;
454+
boolean warningLogged = false;
447455
try {
448456
x509ExtendedTrustManager = getX509ExtendedTrustManager(
449457
(TlsChannelCredentials) channelCredentials);
450458
} catch (GeneralSecurityException e) {
451-
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
452-
"Failure getting X509ExtendedTrustManager from TlsCredentials").withCause(e),
453-
tracers);
459+
if (GrpcUtil.getFlag(GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK, false)) {
460+
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
461+
"Failure getting X509ExtendedTrustManager from TlsCredentials").withCause(e),
462+
tracers);
463+
}
464+
logger.warning(String.format("Failure getting X509ExtendedTrustManager from "
465+
+ "TlsCredentials due to: %s", e.getMessage()));
466+
warningLogged = true;
454467
}
455468
if (x509ExtendedTrustManager == null) {
456-
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
457-
"Can't allow authority override in rpc when X509ExtendedTrustManager is not "
458-
+ "available"), tracers);
459-
}
460-
try {
461-
Certificate[] peerCertificates = sslSession.getPeerCertificates();
462-
X509Certificate[] x509PeerCertificates = new X509Certificate[peerCertificates.length];
463-
for (int i = 0; i < peerCertificates.length; i++) {
464-
x509PeerCertificates[i] = (X509Certificate) peerCertificates[i];
469+
if (GrpcUtil.getFlag(GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK, false)) {
470+
return new FailingClientStream(Status.UNAVAILABLE.withDescription(
471+
"Can't allow authority override in rpc when X509ExtendedTrustManager is not "
472+
+ "available"), tracers);
473+
}
474+
if (!warningLogged) {
475+
logger.warning("Authority override set for rpc when X509ExtendedTrustManager is not "
476+
+ "available.");
477+
}
478+
} else {
479+
try {
480+
Certificate[] peerCertificates = sslSession.getPeerCertificates();
481+
X509Certificate[] x509PeerCertificates = new X509Certificate[peerCertificates.length];
482+
for (int i = 0; i < peerCertificates.length; i++) {
483+
x509PeerCertificates[i] = (X509Certificate) peerCertificates[i];
484+
}
485+
((X509ExtendedTrustManager) x509ExtendedTrustManager).checkServerTrusted(
486+
x509PeerCertificates, "RSA",
487+
new SslSocketWrapper((SSLSocket) socket, callOptions.getAuthority()));
488+
peerVerificationStatus = Status.OK;
489+
} catch (SSLPeerUnverifiedException | CertificateException e) {
490+
peerVerificationStatus = Status.UNAVAILABLE.withDescription(
491+
String.format("Failure in verifying authority '%s' against peer during rpc",
492+
callOptions.getAuthority())).withCause(e);
465493
}
466-
((X509ExtendedTrustManager) x509ExtendedTrustManager).checkServerTrusted(
467-
x509PeerCertificates, "RSA",
468-
new SslSocketWrapper((SSLSocket) socket, callOptions.getAuthority()));
469-
peerVerificationStatus = Status.OK;
470-
} catch (SSLPeerUnverifiedException | CertificateException e) {
471-
peerVerificationStatus = Status.UNAVAILABLE.withDescription(
472-
String.format("Failure in verifying authority '%s' against peer during rpc",
473-
callOptions.getAuthority())).withCause(e);
494+
peerVerificationResults.put(callOptions.getAuthority(), peerVerificationStatus);
474495
}
475-
peerVerificationResults.put(callOptions.getAuthority(), peerVerificationStatus);
476496
}
477-
if (!peerVerificationStatus.isOk()) {
497+
if (peerVerificationStatus != null && !peerVerificationStatus.isOk()) {
478498
return new FailingClientStream(peerVerificationStatus, tracers);
479499
}
480500
}
@@ -1588,9 +1608,7 @@ public boolean isConnected() {
15881608

15891609
@Override
15901610
public SSLParameters getSSLParameters() {
1591-
SSLParameters sslParameters = sslSocket.getSSLParameters();
1592-
sslParameters.setEndpointIdentificationAlgorithm("HTTPS");
1593-
return sslParameters;
1611+
return sslSocket.getSSLParameters();
15941612
}
15951613
}
15961614

‎okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java‎

Lines changed: 24 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -817,18 +817,36 @@ public void perRpcAuthoritySpecified_hostnameVerification_SslSocket_successCase(
817817
@Test
818818
public void perRpcAuthoritySpecified_hostnameVerification_SslSocket_failureCase()
819819
throws Exception {
820+
System.setProperty("GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK", "true");
821+
try {
822+
startTransport(
823+
DEFAULT_START_STREAM_ID, null, true, null,
824+
(hostname, session) -> false, true);
825+
ClientStream clientStream =
826+
clientTransport.newStream(method, new Metadata(),
827+
CallOptions.DEFAULT.withAuthority("some-authority"), tracers);
828+
assertThat(clientStream).isInstanceOf(FailingClientStream.class);
829+
InsightBuilder insightBuilder = new InsightBuilder();
830+
clientStream.appendTimeoutInsight(insightBuilder);
831+
assertThat(insightBuilder.toString()).contains("error=Status{code=UNAVAILABLE, "
832+
+ "description=HostNameVerifier verification failed for authority 'some-authority', "
833+
+ "cause=null}");
834+
shutdownAndVerify();
835+
} finally {
836+
System.clearProperty("GRPC_ENABLE_PER_RPC_AUTHORITY_CHECK");
837+
}
838+
}
839+
840+
@Test
841+
public void perRpcAuthoritySpecified_hostnameVerification_SslSocket_flagDisabled()
842+
throws Exception {
820843
startTransport(
821844
DEFAULT_START_STREAM_ID, null, true, null,
822845
(hostname, session) -> false, true);
823846
ClientStream clientStream =
824847
clientTransport.newStream(method, new Metadata(),
825848
CallOptions.DEFAULT.withAuthority("some-authority"), tracers);
826-
assertThat(clientStream).isInstanceOf(FailingClientStream.class);
827-
InsightBuilder insightBuilder = new InsightBuilder();
828-
clientStream.appendTimeoutInsight(insightBuilder);
829-
assertThat(insightBuilder.toString()).contains("error=Status{code=UNAVAILABLE, "
830-
+ "description=HostNameVerifier verification failed for authority 'some-authority', "
831-
+ "cause=null}");
849+
assertThat(clientStream).isInstanceOf(OkHttpClientStream.class);
832850
shutdownAndVerify();
833851
}
834852

0 commit comments

Comments
 (0)