diff --git a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java index 2f6f6e80cc..b5e207f5e6 100644 --- a/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java +++ b/interop-testing/src/test/java/io/grpc/testing/integration/Http2OkHttpTest.java @@ -23,7 +23,6 @@ import static org.junit.Assert.assertTrue; import com.google.common.base.Throwables; import com.google.protobuf.EmptyProtos.Empty; import com.squareup.okhttp.ConnectionSpec; -import com.squareup.okhttp.TlsVersion; import io.grpc.ManagedChannel; import io.grpc.internal.AbstractServerImplBuilder; import io.grpc.internal.GrpcUtil; @@ -94,9 +93,8 @@ public class Http2OkHttpTest extends AbstractInteropTest { private OkHttpChannelBuilder createChannelBuilder() { OkHttpChannelBuilder builder = OkHttpChannelBuilder.forAddress("localhost", getPort()) .maxInboundMessageSize(AbstractInteropTest.MAX_MESSAGE_SIZE) - .connectionSpec(new ConnectionSpec.Builder(OkHttpChannelBuilder.DEFAULT_CONNECTION_SPEC) + .connectionSpec(new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS) .cipherSuites(TestUtils.preferredTestCiphers().toArray(new String[0])) - .tlsVersions(ConnectionSpec.MODERN_TLS.tlsVersions().toArray(new TlsVersion[0])) .build()) .overrideAuthority(GrpcUtil.authorityFromHostAndPort( TestUtils.TEST_SERVER_HOST, getPort())); diff --git a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java index d74defcc8c..2ac8ea3537 100644 --- a/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java +++ b/okhttp/src/main/java/io/grpc/okhttp/OkHttpChannelBuilder.java @@ -23,9 +23,6 @@ import static io.grpc.internal.GrpcUtil.KEEPALIVE_TIME_NANOS_DISABLED; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.squareup.okhttp.CipherSuite; -import com.squareup.okhttp.ConnectionSpec; -import com.squareup.okhttp.TlsVersion; import io.grpc.Attributes; import io.grpc.ExperimentalApi; import io.grpc.Internal; @@ -40,7 +37,10 @@ import io.grpc.internal.ProxyParameters; import io.grpc.internal.SharedResourceHolder; import io.grpc.internal.SharedResourceHolder.Resource; import io.grpc.internal.TransportTracer; +import io.grpc.okhttp.internal.CipherSuite; +import io.grpc.okhttp.internal.ConnectionSpec; import io.grpc.okhttp.internal.Platform; +import io.grpc.okhttp.internal.TlsVersion; import java.net.InetSocketAddress; import java.net.SocketAddress; import java.security.GeneralSecurityException; @@ -62,7 +62,39 @@ import javax.net.ssl.TrustManagerFactory; public class OkHttpChannelBuilder extends AbstractManagedChannelImplBuilder { - public static final ConnectionSpec DEFAULT_CONNECTION_SPEC = + /** + * ConnectionSpec closely matching the default configuration that could be used as a basis for + * modification. + * + *

Since this field is the only reference in gRPC to ConnectionSpec that may not be ProGuarded, + * we are removing the field to reduce method count. We've been unable to find any existing users + * of the field, and any such user would highly likely at least be changing the cipher suites, + * which is sort of the only part that's non-obvious. Any existing user should instead create + * their own spec from scratch or base it off ConnectionSpec.MODERN_TLS if believed to be + * necessary. If this was providing you with value and don't want to see it removed, open a GitHub + * issue to discuss keeping it. + * + * @deprecated Deemed of little benefit and users weren't using it. Just define one yourself + */ + @Deprecated + public static final com.squareup.okhttp.ConnectionSpec DEFAULT_CONNECTION_SPEC = + new com.squareup.okhttp.ConnectionSpec.Builder(com.squareup.okhttp.ConnectionSpec.MODERN_TLS) + .cipherSuites( + // The following items should be sync with Netty's Http2SecurityUtil.CIPHERS. + com.squareup.okhttp.CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, + com.squareup.okhttp.CipherSuite.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, + com.squareup.okhttp.CipherSuite.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, + com.squareup.okhttp.CipherSuite.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, + com.squareup.okhttp.CipherSuite.TLS_DHE_RSA_WITH_AES_128_GCM_SHA256, + com.squareup.okhttp.CipherSuite.TLS_DHE_DSS_WITH_AES_128_GCM_SHA256, + com.squareup.okhttp.CipherSuite.TLS_DHE_RSA_WITH_AES_256_GCM_SHA384, + com.squareup.okhttp.CipherSuite.TLS_DHE_DSS_WITH_AES_256_GCM_SHA384) + .tlsVersions(com.squareup.okhttp.TlsVersion.TLS_1_2) + .supportsTlsExtensions(true) + .build(); + + @VisibleForTesting + static final ConnectionSpec INTERNAL_DEFAULT_CONNECTION_SPEC = new ConnectionSpec.Builder(ConnectionSpec.MODERN_TLS) .cipherSuites( // The following items should be sync with Netty's Http2SecurityUtil.CIPHERS. @@ -110,7 +142,7 @@ public class OkHttpChannelBuilder extends private SSLSocketFactory sslSocketFactory; private HostnameVerifier hostnameVerifier; - private ConnectionSpec connectionSpec = DEFAULT_CONNECTION_SPEC; + private ConnectionSpec connectionSpec = INTERNAL_DEFAULT_CONNECTION_SPEC; private NegotiationType negotiationType = NegotiationType.TLS; private long keepAliveTimeNanos = KEEPALIVE_TIME_NANOS_DISABLED; private long keepAliveTimeoutNanos = DEFAULT_KEEPALIVE_TIMEOUT_NANOS; @@ -272,7 +304,7 @@ public class OkHttpChannelBuilder extends * For secure connection, provides a ConnectionSpec to specify Cipher suite and * TLS versions. * - *

By default {@link #DEFAULT_CONNECTION_SPEC} will be used. + *

By default a modern, HTTP/2-compatible spec will be used. * *

This method is only used when building a secure connection. For plaintext * connection, use {@link #usePlaintext()} instead. @@ -280,9 +312,10 @@ public class OkHttpChannelBuilder extends * @throws IllegalArgumentException * If {@code connectionSpec} is not with TLS */ - public final OkHttpChannelBuilder connectionSpec(ConnectionSpec connectionSpec) { + public final OkHttpChannelBuilder connectionSpec( + com.squareup.okhttp.ConnectionSpec connectionSpec) { Preconditions.checkArgument(connectionSpec.isTls(), "plaintext ConnectionSpec is not accepted"); - this.connectionSpec = connectionSpec; + this.connectionSpec = Utils.convertSpec(connectionSpec); return this; } @@ -481,7 +514,7 @@ public class OkHttpChannelBuilder extends executor, socketFactory, hostnameVerifier, - Utils.convertSpec(connectionSpec), + connectionSpec, maxMessageSize, proxy, tooManyPingsRunnable, diff --git a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java index bac6a8e67c..5be80b1370 100644 --- a/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java +++ b/okhttp/src/test/java/io/grpc/okhttp/OkHttpClientTransportTest.java @@ -228,7 +228,7 @@ public class OkHttpClientTransportTest { executor, sslSocketFactory, hostnameVerifier, - Utils.convertSpec(OkHttpChannelBuilder.DEFAULT_CONNECTION_SPEC), + OkHttpChannelBuilder.INTERNAL_DEFAULT_CONNECTION_SPEC, DEFAULT_MAX_MESSAGE_SIZE, NO_PROXY, tooManyPingsRunnable,