From 039534e20546221c3466d1ceb663625c59edb0e7 Mon Sep 17 00:00:00 2001 From: Michael Simacek Date: Tue, 11 Jul 2017 13:37:22 +0200 Subject: [PATCH 3/3] Remove conscrypt ALPN --- handler/pom.xml | 6 - .../netty/handler/ssl/ConscryptAlpnSslEngine.java | 176 --------------------- .../ssl/JdkAlpnApplicationProtocolNegotiator.java | 6 +- .../main/java/io/netty/handler/ssl/SslHandler.java | 35 ---- .../ssl/ConscryptJdkSslEngineInteropTest.java | 76 --------- .../io/netty/handler/ssl/Java8SslTestUtils.java | 7 - .../ssl/JdkConscryptSslEngineInteropTest.java | 86 ---------- .../io/netty/handler/ssl/JdkSslEngineTest.java | 2 +- 8 files changed, 2 insertions(+), 392 deletions(-) delete mode 100644 handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java delete mode 100644 handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java delete mode 100644 handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java diff --git a/handler/pom.xml b/handler/pom.xml index 52e63ca..69af32a 100644 --- a/handler/pom.xml +++ b/handler/pom.xml @@ -60,12 +60,6 @@ true - ${conscrypt.groupId} - ${conscrypt.artifactId} - ${conscrypt.classifier} - true - - org.mockito mockito-core diff --git a/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java b/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java deleted file mode 100644 index 8e7a544..0000000 --- a/handler/src/main/java/io/netty/handler/ssl/ConscryptAlpnSslEngine.java +++ /dev/null @@ -1,176 +0,0 @@ -/* - * 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 io.netty.util.internal.PlatformDependent; -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() { - return ENGINES_CLASS != null && PlatformDependent.javaVersion() >= 8; - } - - 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 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 - return (int) min(Integer.MAX_VALUE, plaintextBytes + maxOverhead); - } - - final SSLEngineResult unwrap(ByteBuffer[] srcs, ByteBuffer[] dests) throws SSLException { - 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(applicationNegotiator.protocols())), - "protocolSelector"); - } - - private void selectProtocol() throws SSLException { - try { - String protocol = Conscrypt.Engines.getAlpnSelectedProtocol(getWrappedEngine()); - protocolSelector.select(protocol != null ? Collections.singletonList(protocol) - : Collections.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); - } -} diff --git a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java index f82c7da..9c4ab9e 100644 --- a/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java +++ b/handler/src/main/java/io/netty/handler/ssl/JdkAlpnApplicationProtocolNegotiator.java @@ -21,7 +21,7 @@ import javax.net.ssl.SSLEngine; * The {@link JdkApplicationProtocolNegotiator} to use if you need ALPN and are using {@link SslProvider#JDK}. */ public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicationProtocolNegotiator { - private static final boolean AVAILABLE = ConscryptAlpnSslEngine.isAvailable() || JettyAlpnSslEngine.isAvailable(); + private static final boolean AVAILABLE = JettyAlpnSslEngine.isAvailable(); private static final SslEngineWrapperFactory ALPN_WRAPPER = AVAILABLE ? new AlpnWrapper() : new FailureWrapper(); /** @@ -121,10 +121,6 @@ public final class JdkAlpnApplicationProtocolNegotiator extends JdkBaseApplicati @Override public SSLEngine wrapSslEngine(SSLEngine engine, JdkApplicationProtocolNegotiator applicationNegotiator, boolean isServer) { - if (ConscryptAlpnSslEngine.isEngineSupported(engine)) { - return isServer ? ConscryptAlpnSslEngine.newServerEngine(engine, applicationNegotiator) - : ConscryptAlpnSslEngine.newClientEngine(engine, applicationNegotiator); - } if (JettyAlpnSslEngine.isAvailable()) { return isServer ? JettyAlpnSslEngine.newServerEngine(engine, applicationNegotiator) : JettyAlpnSslEngine.newClientEngine(engine, applicationNegotiator); diff --git a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java index 05c451a..8693011 100644 --- a/handler/src/main/java/io/netty/handler/ssl/SslHandler.java +++ b/handler/src/main/java/io/netty/handler/ssl/SslHandler.java @@ -187,38 +187,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH new ClosedChannelException(), SslHandler.class, "channelInactive(...)"); private enum SslEngineType { - CONSCRYPT(true, COMPOSITE_CUMULATOR) { - @Override - SSLEngineResult unwrap(SslHandler handler, ByteBuf in, int readerIndex, int len, ByteBuf out) - throws SSLException { - int nioBufferCount = in.nioBufferCount(); - int writerIndex = out.writerIndex(); - final SSLEngineResult result; - if (nioBufferCount > 1) { - /* - * Use a special unwrap method without additional memory copies. - */ - try { - handler.singleBuffer[0] = toByteBuffer(out, writerIndex, out.writableBytes()); - result = ((ConscryptAlpnSslEngine) handler.engine).unwrap( - in.nioBuffers(readerIndex, len), - handler.singleBuffer); - } finally { - handler.singleBuffer[0] = null; - } - } else { - result = handler.engine.unwrap(toByteBuffer(in, readerIndex, len), - toByteBuffer(out, writerIndex, out.writableBytes())); - } - out.writerIndex(writerIndex + result.bytesProduced()); - return result; - } - - @Override - int calculateWrapBufferCapacity(SslHandler handler, int pendingBytes, int numComponents) { - return ((ConscryptAlpnSslEngine) handler.engine).calculateOutNetBufSize(pendingBytes, numComponents); - } - }, JDK(false, MERGE_CUMULATOR) { @Override SSLEngineResult unwrap(SslHandler handler, ByteBuf in, int readerIndex, int len, ByteBuf out) @@ -237,9 +205,6 @@ public class SslHandler extends ByteToMessageDecoder implements ChannelOutboundH }; static SslEngineType forEngine(SSLEngine engine) { - if (engine instanceof ConscryptAlpnSslEngine) { - return CONSCRYPT; - } return JDK; } diff --git a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java deleted file mode 100644 index e217136..0000000 --- a/handler/src/test/java/io/netty/handler/ssl/ConscryptJdkSslEngineInteropTest.java +++ /dev/null @@ -1,76 +0,0 @@ -/* - * Copyright 2016 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 java.security.Provider; -import org.junit.BeforeClass; -import org.junit.Ignore; - -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - -import static org.junit.Assume.assumeTrue; - -@RunWith(Parameterized.class) -public class ConscryptJdkSslEngineInteropTest extends SSLEngineTest { - - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); - for (BufferType type: BufferType.values()) { - params.add(type); - } - return params; - } - - public ConscryptJdkSslEngineInteropTest(BufferType type) { - super(type); - } - - @BeforeClass - public static void checkConscrypt() { - assumeTrue(ConscryptAlpnSslEngine.isAvailable()); - } - - @Override - protected SslProvider sslClientProvider() { - return SslProvider.JDK; - } - - @Override - protected SslProvider sslServerProvider() { - return SslProvider.JDK; - } - - @Override - protected Provider clientSslContextProvider() { - return Java8SslTestUtils.conscryptProvider(); - } - - @Ignore /* Does the JDK support a "max certificate chain length"? */ - @Override - public void testMutualAuthValidClientCertChainTooLongFailOptionalClientAuth() throws Exception { - } - - @Ignore /* Does the JDK support a "max certificate chain length"? */ - @Override - public void testMutualAuthValidClientCertChainTooLongFailRequireClientAuth() throws Exception { - } -} diff --git a/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java b/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java index cc2e6c6..f9cf771 100644 --- a/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java +++ b/handler/src/test/java/io/netty/handler/ssl/Java8SslTestUtils.java @@ -16,12 +16,9 @@ package io.netty.handler.ssl; -import org.conscrypt.OpenSSLProvider; - import javax.net.ssl.SNIMatcher; import javax.net.ssl.SNIServerName; import javax.net.ssl.SSLParameters; -import java.security.Provider; import java.util.Collections; final class Java8SslTestUtils { @@ -37,8 +34,4 @@ final class Java8SslTestUtils { }; parameters.setSNIMatchers(Collections.singleton(matcher)); } - - static Provider conscryptProvider() { - return new OpenSSLProvider(); - } } diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java deleted file mode 100644 index 0625f7a..0000000 --- a/handler/src/test/java/io/netty/handler/ssl/JdkConscryptSslEngineInteropTest.java +++ /dev/null @@ -1,86 +0,0 @@ -/* - * 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 java.security.Provider; -import org.junit.BeforeClass; -import org.junit.Ignore; -import org.junit.Test; -import org.junit.runner.RunWith; -import org.junit.runners.Parameterized; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.List; - -import static org.junit.Assume.assumeTrue; - -@RunWith(Parameterized.class) -public class JdkConscryptSslEngineInteropTest extends SSLEngineTest { - - @Parameterized.Parameters(name = "{index}: bufferType = {0}") - public static Collection data() { - List params = new ArrayList(); - for (BufferType type: BufferType.values()) { - params.add(type); - } - return params; - } - - public JdkConscryptSslEngineInteropTest(BufferType type) { - super(type); - } - - @BeforeClass - public static void checkConscrypt() { - assumeTrue(ConscryptAlpnSslEngine.isAvailable()); - } - - @Override - protected SslProvider sslClientProvider() { - return SslProvider.JDK; - } - - @Override - protected SslProvider sslServerProvider() { - return SslProvider.JDK; - } - - @Override - protected Provider serverSslContextProvider() { - return Java8SslTestUtils.conscryptProvider(); - } - - @Override - @Test - @Ignore("TODO: Make this work with Conscrypt") - public void testMutualAuthValidClientCertChainTooLongFailOptionalClientAuth() throws Exception { - super.testMutualAuthValidClientCertChainTooLongFailOptionalClientAuth(); - } - - @Override - @Test - @Ignore("TODO: Make this work with Conscrypt") - public void testMutualAuthValidClientCertChainTooLongFailRequireClientAuth() throws Exception { - super.testMutualAuthValidClientCertChainTooLongFailRequireClientAuth(); - } - - @Override - protected boolean mySetupMutualAuthServerIsValidClientException(Throwable cause) { - // TODO(scott): work around for a JDK issue. The exception should be SSLHandshakeException. - return super.mySetupMutualAuthServerIsValidClientException(cause) || causedBySSLException(cause); - } -} diff --git a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java index 4489b16..e32fa0d 100644 --- a/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java +++ b/handler/src/test/java/io/netty/handler/ssl/JdkSslEngineTest.java @@ -81,7 +81,7 @@ public class JdkSslEngineTest extends SSLEngineTest { @Override boolean isAvailable() { - return ConscryptAlpnSslEngine.isAvailable(); + return false; } @Override -- 2.9.4