Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding support for Conscrypt #6271

Merged
merged 7 commits into from Mar 31, 2017
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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions example/pom.xml
Expand Up @@ -89,6 +89,11 @@
<artifactId>${tcnative.artifactId}</artifactId>
<classifier>${tcnative.classifier}</classifier>
</dependency>
<dependency>
<groupId>${conscrypt.groupId}</groupId>
<artifactId>${conscrypt.artifactId}</artifactId>
<classifier>${conscrypt.classifier}</classifier>
</dependency>
<dependency>
<groupId>org.eclipse.jetty.npn</groupId>
<artifactId>npn-api</artifactId>
Expand Down
5 changes: 5 additions & 0 deletions handler/pom.xml
Expand Up @@ -70,6 +70,11 @@
<artifactId>alpn-api</artifactId>
<optional>true</optional>
</dependency>
<dependency>
<groupId>${conscrypt.groupId}</groupId>
<artifactId>${conscrypt.artifactId}</artifactId>
<classifier>${conscrypt.classifier}</classifier>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Expand Down
174 changes: 174 additions & 0 deletions handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java
@@ -0,0 +1,174 @@
/*
* Copyright 2017 The Netty Project
*
* The Netty Project licenses this file to you under the Apache License,
* version 2.0 (the "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at:
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
* WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
* License for the specific language governing permissions and limitations
* under the License.
*/
package io.netty.handler.ssl;

import static io.netty.handler.ssl.SslUtils.toSSLHandshakeException;
import static io.netty.util.internal.ObjectUtil.checkNotNull;
import static java.lang.Math.min;

import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelectionListener;
import io.netty.handler.ssl.JdkApplicationProtocolNegotiator.ProtocolSelector;
import java.lang.reflect.Method;
import java.nio.ByteBuffer;
import java.util.Collections;
import java.util.LinkedHashSet;
import java.util.List;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLEngineResult;
import javax.net.ssl.SSLException;
import org.conscrypt.Conscrypt;
import org.conscrypt.HandshakeListener;

/**
* A {@link JdkSslEngine} that uses the Conscrypt provider or SSL with ALPN.
*/
abstract class ConscryptAlpnSslEngine extends JdkSslEngine {
private static final Class<?> ENGINES_CLASS = getEnginesClass();

/**
* Indicates whether or not conscrypt is available on the current system.
*/
static boolean isAvailable() {
Copy link
Member

Choose a reason for hiding this comment

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

@nmittler there must be some synchronisation as otherwise it may race.

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved to static initialization

return ENGINES_CLASS != null;
}

static boolean isEngineSupported(SSLEngine engine) {
return isAvailable() && isConscryptEngine(engine, ENGINES_CLASS);
}

static ConscryptAlpnSslEngine newClientEngine(SSLEngine engine,
JdkApplicationProtocolNegotiator applicationNegotiator) {
return new ClientEngine(engine, applicationNegotiator);
}

static ConscryptAlpnSslEngine newServerEngine(SSLEngine engine,
JdkApplicationProtocolNegotiator applicationNegotiator) {
return new ServerEngine(engine, applicationNegotiator);
}

private ConscryptAlpnSslEngine(SSLEngine engine, List<String> protocols) {
super(engine);

// Set the list of supported ALPN protocols on the engine.
Conscrypt.Engines.setAlpnProtocols(engine, protocols.toArray(new String[protocols.size()]));
}

/**
* Calculates the maximum size of the encrypted output buffer required to wrap the given plaintext bytes. Assumes
* as a worst case that there is one TLS record per buffer.
*
* @param plaintextBytes the number of plaintext bytes to be wrapped.
* @param numBuffers the number of buffers that the plaintext bytes are spread across.
* @return the maximum size of the encrypted output buffer required for the wrap operation.
*/
final int calculateOutNetBufSize(int plaintextBytes, int numBuffers) {
// Assuming a max of one frame per component in a composite buffer.
long maxOverhead = (long) Conscrypt.Engines.maxSealOverhead(getWrappedEngine()) * numBuffers;
// TODO(nmittler): update this to use MAX_ENCRYPTED_PACKET_LENGTH instead of Integer.MAX_VALUE
Copy link

Choose a reason for hiding this comment

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

INFO Complete the task associated to this TODO comment. rule

return (int) min(Integer.MAX_VALUE, plaintextBytes + maxOverhead);
}

final SSLEngineResult unwrap(ByteBuffer[] srcs, ByteBuffer[] dests) throws SSLException {
Copy link

Choose a reason for hiding this comment

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

MINOR Consider using varargs for methods or constructors which take an array the last parameter. rule

return Conscrypt.Engines.unwrap(getWrappedEngine(), srcs, dests);
}

private static final class ClientEngine extends ConscryptAlpnSslEngine {
private final ProtocolSelectionListener protocolListener;

ClientEngine(SSLEngine engine,
JdkApplicationProtocolNegotiator applicationNegotiator) {
super(engine, applicationNegotiator.protocols());
// Register for completion of the handshake.
Conscrypt.Engines.setHandshakeListener(engine, new HandshakeListener() {
@Override
public void onHandshakeFinished() throws SSLException {
selectProtocol();
}
});

protocolListener = checkNotNull(applicationNegotiator
.protocolListenerFactory().newListener(this, applicationNegotiator.protocols()),
"protocolListener");
}

private void selectProtocol() throws SSLException {
String protocol = Conscrypt.Engines.getAlpnSelectedProtocol(getWrappedEngine());
try {
protocolListener.selected(protocol);
} catch (Throwable e) {
throw toSSLHandshakeException(e);
}
}
}

private static final class ServerEngine extends ConscryptAlpnSslEngine {
private final ProtocolSelector protocolSelector;

ServerEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator) {
super(engine, applicationNegotiator.protocols());

// Register for completion of the handshake.
Conscrypt.Engines.setHandshakeListener(engine, new HandshakeListener() {
@Override
public void onHandshakeFinished() throws SSLException {
selectProtocol();
}
});

protocolSelector = checkNotNull(applicationNegotiator.protocolSelectorFactory()
.newSelector(this,
new LinkedHashSet<String>(applicationNegotiator.protocols())),
"protocolSelector");
}

private void selectProtocol() throws SSLException {
try {
String protocol = Conscrypt.Engines.getAlpnSelectedProtocol(getWrappedEngine());
protocolSelector.select(protocol != null ? Collections.singletonList(protocol)
: Collections.<String>emptyList());
} catch (Throwable e) {
throw toSSLHandshakeException(e);
}
}
}

private static Class<?> getEnginesClass() {
try {
// Always use bootstrap class loader.
Class<?> engineClass = Class.forName("org.conscrypt.Conscrypt$Engines", true,
ConscryptAlpnSslEngine.class.getClassLoader());
// Ensure that it also has the isConscrypt method.
getIsConscryptMethod(engineClass);
return engineClass;
} catch (Throwable ignore) {
// Conscrypt was not loaded.
return null;
}
}

private static boolean isConscryptEngine(SSLEngine engine, Class<?> enginesClass) {
try {
Method method = getIsConscryptMethod(enginesClass);
return (Boolean) method.invoke(null, engine);
} catch (Throwable ignore) {
return false;
}
}

private static Method getIsConscryptMethod(Class<?> enginesClass) throws NoSuchMethodException {
return enginesClass.getMethod("isConscrypt", SSLEngine.class);
}
}
Expand Up @@ -21,20 +21,8 @@
* The {@link JdkApplicationProtocolNegotiator} to use if you need ALPN and are using {@link SslProvider#JDK}.
*/
public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicationProtocolNegotiator {
private static final SslEngineWrapperFactory ALPN_WRAPPER = new SslEngineWrapperFactory() {
{
if (!JdkAlpnSslEngine.isAvailable()) {
throw new RuntimeException("ALPN unsupported. Is your classpatch configured correctly?"
+ " See http://www.eclipse.org/jetty/documentation/current/alpn-chapter.html#alpn-starting");
}
}

@Override
public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator,
boolean isServer) {
return new JdkAlpnSslEngine(engine, applicationNegotiator, isServer);
}
};
private static final boolean AVAILABLE = ConscryptAlpnSslEngine.isAvailable() || JettyAlpnSslEngine.isAvailable();
private static final SslEngineWrapperFactory ALPN_WRAPPER = AVAILABLE ? new AlpnWrapper() : new FailureWrapper();

/**
* Create a new instance.
Expand Down Expand Up @@ -117,4 +105,31 @@ public JdkAlpnApplicationProtocolNegotiator(ProtocolSelectorFactory selectorFact
ProtocolSelectionListenerFactory listenerFactory, String... protocols) {
super(ALPN_WRAPPER, selectorFactory, listenerFactory, protocols);
}

private static final class FailureWrapper implements SslEngineWrapperFactory {
@Override
public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator,
boolean isServer) {
throw new RuntimeException("ALPN unsupported. Is your classpatch configured correctly?"
Copy link
Contributor

Choose a reason for hiding this comment

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

Mention Conscrypt too?

Copy link
Member Author

Choose a reason for hiding this comment

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

done.

+ " For Conscrypt, add the appropriate Conscrypt JAR to classpath and set the security provider."
+ " For Jetty-ALPN, see "
+ "http://www.eclipse.org/jetty/documentation/current/alpn-chapter.html#alpn-starting");
}
}

private static final class AlpnWrapper implements SslEngineWrapperFactory {
@Override
public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator,
boolean isServer) {
if (ConscryptAlpnSslEngine.isEngineSupported(engine)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

so Conscrypt is priority, if I have both on the classpath am I able to pick which one to use?

Copy link
Member Author

Choose a reason for hiding this comment

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

Without some sort of configuration option, Conscrypt would have to take precedence here. @normanmaurer @Scottmitch any thoughts on this? Shall I add a system property?

Copy link
Member

@Scottmitch Scottmitch Mar 6, 2017

Choose a reason for hiding this comment

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

yes my preference would be to make this configurable (default to conscrypt) if its based upon class path resolution.

Copy link
Member

Choose a reason for hiding this comment

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

+1

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, I just realized that we don't need a system property to configure this. Conscrypt will only be selected if the java security Provider was set to org.conscrypt.OpenSSLProvider. So the user has to be explicit when building the SslContext ... it's not magic.

return isServer ? ConscryptAlpnSslEngine.newServerEngine(engine, applicationNegotiator)
: ConscryptAlpnSslEngine.newClientEngine(engine, applicationNegotiator);
}
if (JettyAlpnSslEngine.isAvailable()) {
return isServer ? JettyAlpnSslEngine.newServerEngine(engine, applicationNegotiator)
: JettyAlpnSslEngine.newClientEngine(engine, applicationNegotiator);
}
throw new RuntimeException("Unable to wrap SSLEngine of type " + engine.getClass().getName());
}
}
}
126 changes: 0 additions & 126 deletions handler/src/main/java/io/netty/handler/ssl/JdkAlpnSslEngine.java

This file was deleted.