Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,8 @@ subprojects {
netty_proxy_handler: "io.netty:netty-handler-proxy:${nettyVersion}",
netty_tcnative: 'io.netty:netty-tcnative-boringssl-static:2.0.7.Final',

conscrypt: 'org.conscrypt:conscrypt-openjdk-uber:1.0.1',

// Test dependencies.
junit: 'junit:junit:4.12',
mockito: 'org.mockito:mockito-core:1.9.5',
Expand Down
2 changes: 1 addition & 1 deletion core/src/main/java/io/grpc/internal/MoreThrowables.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@

/** Utility functions when interacting with {@link Throwable}s. */
// TODO(ejona): Delete this once we've upgraded to Guava 20 or later.
final class MoreThrowables {
public final class MoreThrowables {
/**
* Throws {code t} if it is an instance of {@link RuntimeException} or {@link Error}.
*
Expand Down
4 changes: 2 additions & 2 deletions interop-testing/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ dependencies {
project(':grpc-testing'),
libraries.junit,
libraries.mockito,
libraries.netty_tcnative,
libraries.oauth_client,
libraries.truth
runtime libraries.opencensus_impl
runtime libraries.opencensus_impl,
libraries.netty_tcnative
testCompile project(':grpc-context').sourceSets.test.output
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,8 @@ public class TestServiceClient {
* The main application allowing this client to be launched from the command line.
*/
public static void main(String[] args) throws Exception {
// Let OkHttp use Conscrypt if it is available.
Util.installConscryptIfAvailable();
// Let Netty or OkHttp use Conscrypt if it is available.
TestUtils.installConscryptIfAvailable();
final TestServiceClient client = new TestServiceClient();
client.parseArgs(args);
client.setUp();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@
public class TestServiceServer {
/** The main application allowing this server to be launched from the command line. */
public static void main(String[] args) throws Exception {
// Let Netty use Conscrypt if it is available.
TestUtils.installConscryptIfAvailable();
final TestServiceServer server = new TestServiceServer();
server.parseArgs(args);
if (server.useTls) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@
import com.google.protobuf.MessageLite;
import io.grpc.Metadata;
import io.grpc.protobuf.ProtoUtils;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.security.Provider;
import java.security.Security;
import java.util.List;
import org.junit.Assert;

Expand Down Expand Up @@ -66,41 +62,4 @@ public static void assertEquals(List<? extends MessageLite> expected,
}
}
}

private static boolean conscryptInstallAttempted;

/**
* Add Conscrypt to the list of security providers, if it is available. If it appears to be
* available but fails to load, this method will throw an exception. Since the list of security
* providers is static, this method does nothing if the provider is not available or succeeded
* previously.
*/
public static void installConscryptIfAvailable() {
if (conscryptInstallAttempted) {
return;
}
Class<?> conscrypt;
try {
conscrypt = Class.forName("org.conscrypt.Conscrypt");
} catch (ClassNotFoundException ex) {
conscryptInstallAttempted = true;
return;
}
Method newProvider;
try {
newProvider = conscrypt.getMethod("newProvider");
} catch (NoSuchMethodException ex) {
throw new RuntimeException("Could not find newProvider method on Conscrypt", ex);
}
Provider provider;
try {
provider = (Provider) newProvider.invoke(null);
} catch (IllegalAccessException ex) {
throw new RuntimeException("Could not invoke Conscrypt.newProvider", ex);
} catch (InvocationTargetException ex) {
throw new RuntimeException("Could not invoke Conscrypt.newProvider", ex);
}
Security.addProvider(provider);
conscryptInstallAttempted = true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@
import io.grpc.netty.NettyChannelBuilder;
import io.grpc.netty.NettyServerBuilder;
import io.netty.handler.ssl.ClientAuth;
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.SupportedCipherSuiteFilter;
import java.io.IOException;
import java.net.InetAddress;
Expand All @@ -53,7 +52,6 @@ protected AbstractServerImplBuilder<?> getServerBuilder() {
.clientAuth(ClientAuth.REQUIRE)
.trustManager(TestUtils.loadCert("ca.pem"))
.ciphers(TestUtils.preferredTestCiphers(), SupportedCipherSuiteFilter.INSTANCE)
.sslProvider(SslProvider.OPENSSL)
.build());
} catch (IOException ex) {
throw new RuntimeException(ex);
Expand All @@ -72,7 +70,6 @@ protected ManagedChannel createChannel() {
.keyManager(TestUtils.loadCert("client.pem"), TestUtils.loadCert("client.key"))
.trustManager(TestUtils.loadX509Cert("ca.pem"))
.ciphers(TestUtils.preferredTestCiphers(), SupportedCipherSuiteFilter.INSTANCE)
.sslProvider(SslProvider.OPENSSL)
.build());
io.grpc.internal.TestingAccessor.setStatsImplementation(
builder, createClientCensusStatsModule());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public class Http2OkHttpTest extends AbstractInteropTest {
public static void loadConscrypt() throws Exception {
// Load conscrypt if it is available. Either Conscrypt or Jetty ALPN needs to be available for
// OkHttp to negotiate.
Util.installConscryptIfAvailable();
TestUtils.installConscryptIfAvailable();
}

@Override
Expand Down
10 changes: 8 additions & 2 deletions netty/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@ dependencies {

// Tests depend on base class defined by core module.
testCompile project(':grpc-core').sourceSets.test.output,
project(':grpc-testing')
testRuntime libraries.netty_tcnative
project(':grpc-testing'),
project(':grpc-testing-proto')
testRuntime libraries.netty_tcnative,
libraries.conscrypt
signature "org.codehaus.mojo.signature:java17:1.0@signature"
}

Expand All @@ -29,6 +31,10 @@ project.sourceSets {
}
}

test {
// Allow testing Jetty ALPN in TlsTest
jvmArgs "-javaagent:" + configurations.alpnagent.asPath
}

jmh {
// Workaround
Expand Down
155 changes: 123 additions & 32 deletions netty/src/main/java/io/grpc/netty/GrpcSslContexts.java
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

import com.google.errorprone.annotations.CanIgnoreReturnValue;
import io.grpc.ExperimentalApi;
import io.grpc.internal.MoreThrowables;
import io.netty.handler.codec.http2.Http2SecurityUtil;
import io.netty.handler.ssl.ApplicationProtocolConfig;
import io.netty.handler.ssl.ApplicationProtocolConfig.Protocol;
Expand All @@ -30,16 +31,24 @@
import io.netty.handler.ssl.SslProvider;
import io.netty.handler.ssl.SupportedCipherSuiteFilter;
import java.io.File;
import java.lang.reflect.InvocationTargetException;
import java.lang.reflect.Method;
import java.security.Provider;
import java.security.Security;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;

/**
* Utility for configuring SslContext for gRPC.
*/
@SuppressWarnings("deprecation")
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1784")
public class GrpcSslContexts {
private static final Logger logger = Logger.getLogger(GrpcSslContexts.class.getName());

private GrpcSslContexts() {}

/*
Expand Down Expand Up @@ -84,6 +93,22 @@ private GrpcSslContexts() {}
SelectedListenerFailureBehavior.ACCEPT,
NEXT_PROTOCOL_VERSIONS);

private static final String SUN_PROVIDER_NAME = "SunJSSE";
private static final Method IS_CONSCRYPT_PROVIDER;

static {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be wrapped in a function to provide an easier to understand stack trace if this ends up throwing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I could, although I find it more confusing to have static methods that are called before static initialization completes. It's necessary and helpful at times, but when unnecessary I try to avoid it as I find it to be bug-prone.

Method method = null;
try {
Class<?> conscryptClass = Class.forName("org.conscrypt.Conscrypt");
method = conscryptClass.getMethod("isConscrypt", Provider.class);
} catch (ClassNotFoundException ex) {
logger.log(Level.FINE, "Conscrypt class not found. Not using Conscrypt", ex);
} catch (NoSuchMethodException ex) {
throw new AssertionError(ex);
}
IS_CONSCRYPT_PROVIDER = method;
}

/**
* Creates a SslContextBuilder with ciphers and APN appropriate for gRPC.
*
Expand Down Expand Up @@ -131,49 +156,115 @@ public static SslContextBuilder configure(SslContextBuilder builder) {
@ExperimentalApi("https://github.com/grpc/grpc-java/issues/1784")
@CanIgnoreReturnValue
public static SslContextBuilder configure(SslContextBuilder builder, SslProvider provider) {
return builder.sslProvider(provider)
.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
.applicationProtocolConfig(selectApplicationProtocolConfig(provider));
switch (provider) {
case JDK:
{
Provider jdkProvider = findJdkProvider();
if (jdkProvider == null) {
throw new IllegalArgumentException(
"Could not find Jetty NPN/ALPN or Conscrypt as installed JDK providers");
}
return configure(builder, jdkProvider);
}
case OPENSSL:
{
ApplicationProtocolConfig apc;
if (OpenSsl.isAlpnSupported()) {
apc = NPN_AND_ALPN;
} else {
apc = NPN;
}
return builder
.sslProvider(SslProvider.OPENSSL)
.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
.applicationProtocolConfig(apc);
}
default:
throw new IllegalArgumentException("Unsupported provider: " + provider);
}
}

/**
* Returns OpenSSL if available, otherwise returns the JDK provider.
* Set ciphers and APN appropriate for gRPC. Precisely what is set is permitted to change, so if
* an application requires particular settings it should override the options set here.
*/
private static SslProvider defaultSslProvider() {
return OpenSsl.isAvailable() ? SslProvider.OPENSSL : SslProvider.JDK;
@CanIgnoreReturnValue
public static SslContextBuilder configure(SslContextBuilder builder, Provider jdkProvider) {
ApplicationProtocolConfig apc;
if (SUN_PROVIDER_NAME.equals(jdkProvider.getName())) {
// Jetty ALPN/NPN only supports one of NPN or ALPN
if (JettyTlsUtil.isJettyAlpnConfigured()) {
apc = ALPN;
} else if (JettyTlsUtil.isJettyNpnConfigured()) {
apc = NPN;
} else {
throw new IllegalArgumentException(
SUN_PROVIDER_NAME + " selected, but Jetty NPN/ALPN unavailable");
}
} else if (isConscrypt(jdkProvider)) {
apc = ALPN;
} else {
throw new IllegalArgumentException("Unknown provider; can't configure: " + jdkProvider);
}
return builder
.sslProvider(SslProvider.JDK)
.ciphers(Http2SecurityUtil.CIPHERS, SupportedCipherSuiteFilter.INSTANCE)
.applicationProtocolConfig(apc)
.sslContextProvider(jdkProvider);
}

/**
* Attempts to select the best {@link ApplicationProtocolConfig} for the given
* {@link SslProvider}.
* Returns OpenSSL if available, otherwise returns the JDK provider.
*/
private static ApplicationProtocolConfig selectApplicationProtocolConfig(SslProvider provider) {
switch (provider) {
case JDK: {
if (JettyTlsUtil.isJettyAlpnConfigured()) {
return ALPN;
}
if (JettyTlsUtil.isJettyNpnConfigured()) {
return NPN;
}
if (JettyTlsUtil.isJava9AlpnAvailable()) {
return ALPN;
private static SslProvider defaultSslProvider() {
if (OpenSsl.isAvailable()) {
logger.log(Level.FINE, "Selecting OPENSSL");
return SslProvider.OPENSSL;
}
Provider provider = findJdkProvider();
if (provider != null) {
logger.log(Level.FINE, "Selecting JDK with provider {0}", provider);
return SslProvider.JDK;
}
logger.log(Level.INFO, "netty-tcnative unavailable (this may be normal)",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we expect the IllegalStateException to be caught somewhere? It looks like in the current flow this propagates back through the NettyChannelBuilder, in which case it seems a little confusing to say "(this may be normal)" for all of these when we're about to fail. I think the intent is that it might be normal for two out of the three to be missing, but the current message seems to imply it's ok for all of them to be missing. Maybe add an initial "One of the following TLS ALPN providers must be available:" log statement and drop the "(this may be normal)" text?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spoke offline. The main reason I didn't do that is because then the error is reported in two places: the log message and the exception. Right now the log messages are just informational and it's only after you get the exception that you can look at them and think "these might be useful."

OpenSsl.unavailabilityCause());
logger.log(Level.INFO, "Conscrypt not found (this may be normal)");
logger.log(Level.INFO, "Jetty ALPN unavailable (this may be normal)",
JettyTlsUtil.getJettyAlpnUnavailabilityCause());
throw new IllegalStateException(
"Could not find TLS ALPN provider; "
+ "no working netty-tcnative, Conscrypt, or Jetty NPN/ALPN available");
}

private static Provider findJdkProvider() {
for (Provider provider : Security.getProviders("SSLContext.TLS")) {
if (SUN_PROVIDER_NAME.equals(provider.getName())) {
if (JettyTlsUtil.isJettyAlpnConfigured()
|| JettyTlsUtil.isJettyNpnConfigured()
|| JettyTlsUtil.isJava9AlpnAvailable()) {
return provider;
}
// Use the ALPN cause since it is prefered.
throw new IllegalArgumentException(
"ALPN is not configured properly. See https://github.com/grpc/grpc-java/blob/master/SECURITY.md#troubleshooting"
+ " for more information.",
JettyTlsUtil.getJettyAlpnUnavailabilityCause());
} else if (isConscrypt(provider)) {
return provider;
}
case OPENSSL: {
if (!OpenSsl.isAvailable()) {
throw new IllegalArgumentException(
"OpenSSL is not installed on the system.", OpenSsl.unavailabilityCause());
}
return OpenSsl.isAlpnSupported() ? NPN_AND_ALPN : NPN;
}
return null;
}

private static boolean isConscrypt(Provider provider) {
if (IS_CONSCRYPT_PROVIDER == null) {
return false;
}
try {
return (Boolean) IS_CONSCRYPT_PROVIDER.invoke(null, provider);
} catch (IllegalAccessException ex) {
throw new AssertionError(ex);
} catch (InvocationTargetException ex) {
if (ex.getCause() != null) {
MoreThrowables.throwIfUnchecked(ex.getCause());
// If checked, just wrap up everything.
}
default:
throw new IllegalArgumentException("Unsupported provider: " + provider);
throw new AssertionError(ex);
}
}

Expand Down
Loading