From 3fd952f233865cb230ca3e868e0a3adab5e0492b Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Tue, 21 Apr 2020 14:33:52 -0700 Subject: [PATCH 1/2] Revert "okhttp: Skip enabling SNI and session ticket for fake/test host names (#6949)" This reverts commit eb8e31409ea1bb0db4eb947a5df6aff267ba1f4b. --- .../io/grpc/okhttp/OkHttpProtocolNegotiator.java | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java index 5443675549a..40d19db0e26 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java @@ -19,7 +19,6 @@ import static com.google.common.base.Preconditions.checkNotNull; import com.google.common.annotations.VisibleForTesting; -import io.grpc.internal.GrpcUtil; import io.grpc.okhttp.internal.OptionalMethod; import io.grpc.okhttp.internal.Platform; import io.grpc.okhttp.internal.Platform.TlsExtensionType; @@ -236,11 +235,7 @@ protected void configureTlsExtensions( SSLParameters sslParams = sslSocket.getSSLParameters(); try { // Enable SNI and session tickets. - // Hostname is normally validated in the builder (see checkAuthority) and it should - // virtually always succeed. Check again here to avoid troubles (e.g., hostname with - // underscore) enabling SNI, which works around cases where checkAuthority is disabled. - // See b/154375837. - if (hostname != null && isValidHostName(hostname)) { + if (hostname != null) { if (SSL_SOCKETS_IS_SUPPORTED_SOCKET != null && (boolean) SSL_SOCKETS_IS_SUPPORTED_SOCKET.invoke(null, sslSocket)) { SSL_SOCKETS_SET_USE_SESSION_TICKET.invoke(null, sslSocket, true); @@ -361,13 +356,4 @@ private static String[] protocolIds(List protocols) { } return result.toArray(new String[0]); } - - private static boolean isValidHostName(String name) { - try { - GrpcUtil.checkAuthority(name); - return true; - } catch (IllegalArgumentException e) { - return false; - } - } } From 948f01f3096f998dddf675a59e3b34db187e2374 Mon Sep 17 00:00:00 2001 From: Chengyuan Zhang Date: Tue, 21 Apr 2020 14:34:06 -0700 Subject: [PATCH 2/2] Revert "okhttp: use new APIs for configuring TLS whenever possible (Android Q+) (#6912)" This reverts commit 5803dfd9dcca0fba4483fccf724efc1c853a3ada. --- .../grpc/okhttp/OkHttpProtocolNegotiator.java | 159 +----------------- 1 file changed, 5 insertions(+), 154 deletions(-) diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java index 40d19db0e26..d981ee2845f 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpProtocolNegotiator.java @@ -25,18 +25,11 @@ import io.grpc.okhttp.internal.Protocol; import io.grpc.okhttp.internal.Util; import java.io.IOException; -import java.lang.reflect.Constructor; -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; import java.net.Socket; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collections; import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import javax.annotation.Nullable; -import javax.net.ssl.SSLParameters; import javax.net.ssl.SSLSocket; /** @@ -140,69 +133,6 @@ static final class AndroidNegotiator extends OkHttpProtocolNegotiator { private static final OptionalMethod SET_NPN_PROTOCOLS = new OptionalMethod<>(null, "setNpnProtocols", byte[].class); - // Non-null on Android 10.0+. - // SSLSockets.isSupportedSocket(SSLSocket) - private static final Method SSL_SOCKETS_IS_SUPPORTED_SOCKET; - // SSLSockets.setUseSessionTickets(SSLSocket, boolean) - private static final Method SSL_SOCKETS_SET_USE_SESSION_TICKET; - // SSLParameters.setApplicationProtocols(String[]) - private static final Method SET_APPLICATION_PROTOCOLS; - // SSLParameters.getApplicationProtocols() - private static final Method GET_APPLICATION_PROTOCOLS; - // SSLSocket.getApplicationProtocol() - private static final Method GET_APPLICATION_PROTOCOL; - - // Non-null on Android 7.0+. - // SSLParameters.setServerNames(List) - private static final Method SET_SERVER_NAMES; - // SNIHostName(String) - private static final Constructor SNI_HOST_NAME; - - static { - // Attempt to find Android 10.0+ APIs. - Method setApplicationProtocolsMethod = null; - Method getApplicationProtocolsMethod = null; - Method getApplicationProtocolMethod = null; - Method sslSocketsIsSupportedSocketMethod = null; - Method sslSocketsSetUseSessionTicketsMethod = null; - try { - Class sslParameters = SSLParameters.class; - setApplicationProtocolsMethod = - sslParameters.getMethod("setApplicationProtocols", String[].class); - getApplicationProtocolsMethod = sslParameters.getMethod("getApplicationProtocols"); - getApplicationProtocolMethod = SSLSocket.class.getMethod("getApplicationProtocol"); - Class sslSockets = Class.forName("android.net.ssl.SSLSockets"); - sslSocketsIsSupportedSocketMethod = - sslSockets.getMethod("isSupportedSocket", SSLSocket.class); - sslSocketsSetUseSessionTicketsMethod = - sslSockets.getMethod("setUseSessionTickets", SSLSocket.class, boolean.class); - } catch (ClassNotFoundException e) { - logger.log(Level.FINER, "Failed to find Android 10.0+ APIs", e); - } catch (NoSuchMethodException e) { - logger.log(Level.FINER, "Failed to find Android 10.0+ APIs", e); - } - SET_APPLICATION_PROTOCOLS = setApplicationProtocolsMethod; - GET_APPLICATION_PROTOCOLS = getApplicationProtocolsMethod; - GET_APPLICATION_PROTOCOL = getApplicationProtocolMethod; - SSL_SOCKETS_IS_SUPPORTED_SOCKET = sslSocketsIsSupportedSocketMethod; - SSL_SOCKETS_SET_USE_SESSION_TICKET = sslSocketsSetUseSessionTicketsMethod; - - // Attempt to find Android 7.0+ APIs. - Method setServerNamesMethod = null; - Constructor sniHostNameConstructor = null; - try { - setServerNamesMethod = SSLParameters.class.getMethod("setServerNames", List.class); - sniHostNameConstructor = - Class.forName("javax.net.ssl.SNIHostName").getConstructor(String.class); - } catch (ClassNotFoundException e) { - logger.log(Level.FINER, "Failed to find Android 7.0+ APIs", e); - } catch (NoSuchMethodException e) { - logger.log(Level.FINER, "Failed to find Android 7.0+ APIs", e); - } - SET_SERVER_NAMES = setServerNamesMethod; - SNI_HOST_NAME = sniHostNameConstructor; - } - AndroidNegotiator(Platform platform) { super(platform); } @@ -222,75 +152,21 @@ public String negotiate(SSLSocket sslSocket, String hostname, List pro /** * Override {@link Platform}'s configureTlsExtensions for Android older than 5.0, since OkHttp * (2.3+) only support such function for Android 5.0+. - * - *

Note: Prior to Android Q, the standard way of accessing some Conscrypt features was to - * use reflection to call hidden APIs. Beginning in Q, there is public API for all of these - * features. We attempt to use the public API where possible. Otherwise, fall back to use the - * old reflective API. */ @Override protected void configureTlsExtensions( SSLSocket sslSocket, String hostname, List protocols) { - String[] protocolNames = protocolIds(protocols); - SSLParameters sslParams = sslSocket.getSSLParameters(); - try { - // Enable SNI and session tickets. - if (hostname != null) { - if (SSL_SOCKETS_IS_SUPPORTED_SOCKET != null - && (boolean) SSL_SOCKETS_IS_SUPPORTED_SOCKET.invoke(null, sslSocket)) { - SSL_SOCKETS_SET_USE_SESSION_TICKET.invoke(null, sslSocket, true); - } else { - SET_USE_SESSION_TICKETS.invokeOptionalWithoutCheckedException(sslSocket, true); - } - if (SET_SERVER_NAMES != null && SNI_HOST_NAME != null) { - SET_SERVER_NAMES - .invoke(sslParams, Collections.singletonList(SNI_HOST_NAME.newInstance(hostname))); - } else { - SET_HOSTNAME.invokeOptionalWithoutCheckedException(sslSocket, hostname); - } - } - boolean alpnEnabled = false; - if (GET_APPLICATION_PROTOCOL != null) { - try { - // If calling SSLSocket.getApplicationProtocol() throws UnsupportedOperationException, - // the underlying provider does not implement operations for enabling - // ALPN in the fashion of SSLParameters.setApplicationProtocols(). Fall back to - // use old hidden methods. - GET_APPLICATION_PROTOCOL.invoke(sslSocket); - SET_APPLICATION_PROTOCOLS.invoke(sslParams, (Object) protocolNames); - alpnEnabled = true; - } catch (InvocationTargetException e) { - Throwable targetException = e.getTargetException(); - if (targetException instanceof UnsupportedOperationException) { - logger.log(Level.FINER, "setApplicationProtocol unsupported, will try old methods"); - } else { - throw e; - } - } - } - sslSocket.setSSLParameters(sslParams); - // Check application protocols are configured correctly. If not, configure again with - // old methods. - // Workaround for Conscrypt bug: https://github.com/google/conscrypt/issues/832 - if (alpnEnabled && GET_APPLICATION_PROTOCOLS != null) { - String[] configuredProtocols = - (String[]) GET_APPLICATION_PROTOCOLS.invoke(sslSocket.getSSLParameters()); - if (Arrays.equals(protocolNames, configuredProtocols)) { - return; - } - } - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } catch (InvocationTargetException e) { - throw new RuntimeException(e); - } catch (InstantiationException e) { - throw new RuntimeException(e); + // Enable SNI and session tickets. + if (hostname != null) { + SET_USE_SESSION_TICKETS.invokeOptionalWithoutCheckedException(sslSocket, true); + SET_HOSTNAME.invokeOptionalWithoutCheckedException(sslSocket, hostname); } Object[] parameters = {Platform.concatLengthPrefixed(protocols)}; if (platform.getTlsExtensionType() == TlsExtensionType.ALPN_AND_NPN) { SET_ALPN_PROTOCOLS.invokeWithoutCheckedException(sslSocket, parameters); } + if (platform.getTlsExtensionType() != TlsExtensionType.NONE) { SET_NPN_PROTOCOLS.invokeWithoutCheckedException(sslSocket, parameters); } else { @@ -301,23 +177,6 @@ protected void configureTlsExtensions( @Override public String getSelectedProtocol(SSLSocket socket) { - if (GET_APPLICATION_PROTOCOL != null) { - try { - return (String) GET_APPLICATION_PROTOCOL.invoke(socket); - } catch (IllegalAccessException e) { - throw new RuntimeException(e); - } catch (InvocationTargetException e) { - Throwable targetException = e.getTargetException(); - if (targetException instanceof UnsupportedOperationException) { - logger.log( - Level.FINER, - "Socket unsupported for getApplicationProtocol, will try old methods"); - } else { - throw new RuntimeException(e); - } - } - } - if (platform.getTlsExtensionType() == TlsExtensionType.ALPN_AND_NPN) { try { byte[] alpnResult = @@ -348,12 +207,4 @@ public String getSelectedProtocol(SSLSocket socket) { return null; } } - - private static String[] protocolIds(List protocols) { - List result = new ArrayList<>(); - for (Protocol protocol : protocols) { - result.add(protocol.toString()); - } - return result.toArray(new String[0]); - } }