From ffa348273ada1edbe55a627e982a02269d1fb729 Mon Sep 17 00:00:00 2001 From: Trustin Lee Date: Thu, 22 May 2014 10:12:42 +0900 Subject: [PATCH] Future compatibility with TLS ALPN Motivation: According to TLS ALPN draft-05, a client sends the list of the supported protocols and a server responds with the selected protocol, which is different from NPN. Therefore, ApplicationProtocolSelector won't work with ALPN Modifications: - Use Iterable to list the supported protocols on the client side, rather than using ApplicationProtocolSelector - Remove ApplicationProtocolSelector Result: Future compatibility with TLS ALPN --- .../ssl/ApplicationProtocolSelector.java | 34 ------ .../handler/ssl/JdkSslClientContext.java | 40 +++---- .../netty/handler/ssl/JdkSslContext.java | 8 +- .../handler/ssl/JdkSslServerContext.java | 15 --- .../netty/handler/ssl/JettyNpnSslEngine.java | 80 ++++++------- .../handler/ssl/OpenSslServerContext.java | 22 ++-- .../jboss/netty/handler/ssl/SslContext.java | 108 ++---------------- 7 files changed, 79 insertions(+), 228 deletions(-) delete mode 100644 src/main/java/org/jboss/netty/handler/ssl/ApplicationProtocolSelector.java diff --git a/src/main/java/org/jboss/netty/handler/ssl/ApplicationProtocolSelector.java b/src/main/java/org/jboss/netty/handler/ssl/ApplicationProtocolSelector.java deleted file mode 100644 index b93b4624c6..0000000000 --- a/src/main/java/org/jboss/netty/handler/ssl/ApplicationProtocolSelector.java +++ /dev/null @@ -1,34 +0,0 @@ -/* - * Copyright 2014 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 org.jboss.netty.handler.ssl; - -import java.util.List; - -/** - * Selects an application layer protocol in TLS NPN - * (Next Protocol Negotiation) or ALPN - * (Application Layer Protocol Negotiation). - */ -public interface ApplicationProtocolSelector { - /** - * Invoked to select a protocol from the list of specified application layer protocols. - * - * @param protocols the list of application layer protocols sent by the server. - * The list is empty if the server supports neither NPN nor ALPM. - * @return the selected protocol. {@code null} if no protocol was selected. - */ - String selectProtocol(List protocols) throws Exception; -} diff --git a/src/main/java/org/jboss/netty/handler/ssl/JdkSslClientContext.java b/src/main/java/org/jboss/netty/handler/ssl/JdkSslClientContext.java index 5234af8e00..06786063bf 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/JdkSslClientContext.java +++ b/src/main/java/org/jboss/netty/handler/ssl/JdkSslClientContext.java @@ -20,7 +20,6 @@ import org.jboss.netty.buffer.ChannelBuffer; import org.jboss.netty.buffer.ChannelBufferInputStream; import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSessionContext; import javax.net.ssl.TrustManagerFactory; @@ -29,6 +28,7 @@ import java.io.File; import java.security.KeyStore; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; +import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -38,7 +38,7 @@ import java.util.List; public final class JdkSslClientContext extends JdkSslContext { private final SSLContext ctx; - private final ApplicationProtocolSelector nextProtocolSelector; + private final List nextProtocols; /** * Creates a new instance. @@ -93,9 +93,8 @@ public final class JdkSslClientContext extends JdkSslContext { * {@code null} to use the default. * @param ciphers the cipher suites to enable, in the order of preference. * {@code null} to use the default cipher suites. - * @param nextProtocolSelector the {@link ApplicationProtocolSelector} that chooses one of the application layer - * protocols returned by a TLS server. - * {@code null} to disable TLS NPN/ALPN extension. + * @param nextProtocols the application layer protocols to accept, in the order of preference. + * {@code null} to disable TLS NPN/ALPN extension. * @param sessionCacheSize the size of the cache used for storing SSL session objects. * {@code 0} to use the default value. * @param sessionTimeout the timeout for the cached SSL session objects, in seconds. @@ -103,16 +102,23 @@ public final class JdkSslClientContext extends JdkSslContext { */ public JdkSslClientContext( SslBufferPool bufPool, File certChainFile, TrustManagerFactory trustManagerFactory, - Iterable ciphers, ApplicationProtocolSelector nextProtocolSelector, + Iterable ciphers, Iterable nextProtocols, long sessionCacheSize, long sessionTimeout) throws SSLException { super(bufPool, ciphers); - if (nextProtocolSelector != null && !JettyNpnSslEngine.isAvailable()) { - throw new SSLException("NPN/ALPN unsupported: " + nextProtocolSelector); + if (nextProtocols != null && nextProtocols.iterator().hasNext() && !JettyNpnSslEngine.isAvailable()) { + throw new SSLException("NPN/ALPN unsupported: " + nextProtocols); } - this.nextProtocolSelector = nextProtocolSelector; + List nextProtoList = new ArrayList(); + for (String p: nextProtocols) { + if (p == null) { + break; + } + nextProtoList.add(p); + } + this.nextProtocols = Collections.unmodifiableList(nextProtoList); try { if (certChainFile == null) { @@ -162,27 +168,13 @@ public final class JdkSslClientContext extends JdkSslContext { return true; } - @Override - public ApplicationProtocolSelector nextProtocolSelector() { - return nextProtocolSelector; - } - @Override public List nextProtocols() { - return Collections.emptyList(); + return nextProtocols; } @Override public SSLContext context() { return ctx; } - - @Override - SSLEngine wrapEngine(SSLEngine engine) { - if (nextProtocolSelector == null) { - return engine; - } else { - return new JettyNpnSslEngine(engine, nextProtocolSelector); - } - } } diff --git a/src/main/java/org/jboss/netty/handler/ssl/JdkSslContext.java b/src/main/java/org/jboss/netty/handler/ssl/JdkSslContext.java index 7c30801748..dfe846f9ce 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/JdkSslContext.java +++ b/src/main/java/org/jboss/netty/handler/ssl/JdkSslContext.java @@ -161,7 +161,13 @@ public abstract class JdkSslContext extends SslContext { return wrapEngine(engine); } - abstract SSLEngine wrapEngine(SSLEngine engine); + private SSLEngine wrapEngine(SSLEngine engine) { + if (nextProtocols().isEmpty()) { + return engine; + } else { + return new JettyNpnSslEngine(engine, nextProtocols(), isServer()); + } + } private static String[] toCipherSuiteArray(Iterable ciphers) { if (ciphers == null) { diff --git a/src/main/java/org/jboss/netty/handler/ssl/JdkSslServerContext.java b/src/main/java/org/jboss/netty/handler/ssl/JdkSslServerContext.java index a71fa21e9e..607433f5c2 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/JdkSslServerContext.java +++ b/src/main/java/org/jboss/netty/handler/ssl/JdkSslServerContext.java @@ -21,7 +21,6 @@ import org.jboss.netty.buffer.ChannelBufferInputStream; import javax.net.ssl.KeyManagerFactory; import javax.net.ssl.SSLContext; -import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLException; import javax.net.ssl.SSLSessionContext; import java.io.File; @@ -178,11 +177,6 @@ public final class JdkSslServerContext extends JdkSslContext { return false; } - @Override - public ApplicationProtocolSelector nextProtocolSelector() { - return null; - } - @Override public List nextProtocols() { return nextProtocols; @@ -192,13 +186,4 @@ public final class JdkSslServerContext extends JdkSslContext { public SSLContext context() { return ctx; } - - @Override - SSLEngine wrapEngine(SSLEngine engine) { - if (nextProtocols.isEmpty()) { - return engine; - } else { - return new JettyNpnSslEngine(engine, nextProtocols); - } - } } diff --git a/src/main/java/org/jboss/netty/handler/ssl/JettyNpnSslEngine.java b/src/main/java/org/jboss/netty/handler/ssl/JettyNpnSslEngine.java index 9b4621577e..79b134688a 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/JettyNpnSslEngine.java +++ b/src/main/java/org/jboss/netty/handler/ssl/JettyNpnSslEngine.java @@ -19,8 +19,6 @@ package org.jboss.netty.handler.ssl; import org.eclipse.jetty.npn.NextProtoNego; import org.eclipse.jetty.npn.NextProtoNego.ClientProvider; import org.eclipse.jetty.npn.NextProtoNego.ServerProvider; -import org.jboss.netty.logging.InternalLogger; -import org.jboss.netty.logging.InternalLoggerFactory; import javax.net.ssl.SSLEngine; import javax.net.ssl.SSLEngineResult; @@ -33,8 +31,6 @@ import java.util.List; final class JettyNpnSslEngine extends SSLEngine { - private static final InternalLogger logger = InternalLoggerFactory.getInstance(JettyNpnSslEngine.class); - private static boolean available; static boolean isAvailable() { @@ -64,51 +60,49 @@ final class JettyNpnSslEngine extends SSLEngine { private final SSLEngine engine; private final JettyNpnSslSession session; - JettyNpnSslEngine(SSLEngine engine, final List nextProtocols) { + JettyNpnSslEngine(SSLEngine engine, final List nextProtocols, boolean server) { assert !nextProtocols.isEmpty(); this.engine = engine; session = new JettyNpnSslSession(engine); - NextProtoNego.put(engine, new ServerProvider() { - public void unsupported() { - getSession().setApplicationProtocol(nextProtocols.get(nextProtocols.size() - 1)); - } - - public List protocols() { - return nextProtocols; - } - - public void protocolSelected(String protocol) { - getSession().setApplicationProtocol(protocol); - } - }); - } - - JettyNpnSslEngine(SSLEngine engine, final ApplicationProtocolSelector nextProtocolSelector) { - this.engine = engine; - session = new JettyNpnSslSession(engine); - - NextProtoNego.put(engine, new ClientProvider() { - public boolean supports() { - return true; - } - - public void unsupported() { - session.setApplicationProtocol(null); - } - - public String selectProtocol(List protocols) { - String p = null; - try { - p = nextProtocolSelector.selectProtocol(protocols); - } catch (Exception e) { - logger.warn("Failed to select the next protocol:", e); + if (server) { + NextProtoNego.put(engine, new ServerProvider() { + public void unsupported() { + getSession().setApplicationProtocol(nextProtocols.get(nextProtocols.size() - 1)); } - session.setApplicationProtocol(p); - return p; - } - }); + + public List protocols() { + return nextProtocols; + } + + public void protocolSelected(String protocol) { + getSession().setApplicationProtocol(protocol); + } + }); + } else { + final String[] list = nextProtocols.toArray(new String[nextProtocols.size()]); + final String fallback = list[list.length - 1]; + + NextProtoNego.put(engine, new ClientProvider() { + public boolean supports() { + return true; + } + + public void unsupported() { + session.setApplicationProtocol(null); + } + + public String selectProtocol(List protocols) { + for (String p: list) { + if (protocols.contains(p)) { + return p; + } + } + return fallback; + } + }); + } } @Override diff --git a/src/main/java/org/jboss/netty/handler/ssl/OpenSslServerContext.java b/src/main/java/org/jboss/netty/handler/ssl/OpenSslServerContext.java index dc1709f4c4..d05a9d2aed 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/OpenSslServerContext.java +++ b/src/main/java/org/jboss/netty/handler/ssl/OpenSslServerContext.java @@ -64,8 +64,7 @@ public final class OpenSslServerContext extends SslContext { private final List unmodifiableCiphers = Collections.unmodifiableList(ciphers); private final long sessionCacheSize; private final long sessionTimeout; - private final List nextProtocols = new ArrayList(); - private final List unmodifiableNextProtocols = Collections.unmodifiableList(nextProtocols); + private final List nextProtocols; /** The OpenSSL SSL_CTX object */ private final long ctx; @@ -151,12 +150,14 @@ public final class OpenSslServerContext extends SslContext { this.ciphers.add(c); } + List nextProtoList = new ArrayList(); for (String p: nextProtocols) { if (p == null) { break; } - this.nextProtocols.add(p); + nextProtoList.add(p); } + this.nextProtocols = Collections.unmodifiableList(nextProtoList); // Allocate a new APR pool. aprPool = Pool.create(0); @@ -221,10 +222,10 @@ public final class OpenSslServerContext extends SslContext { } /* Set next protocols for next protocol negotiation extension, if specified */ - if (!this.nextProtocols.isEmpty()) { + if (!nextProtoList.isEmpty()) { // Convert the protocol list into a comma-separated string. StringBuilder nextProtocolBuf = new StringBuilder(); - for (String p: this.nextProtocols) { + for (String p: nextProtoList) { nextProtocolBuf.append(p); nextProtocolBuf.append(','); } @@ -290,14 +291,9 @@ public final class OpenSslServerContext extends SslContext { return sessionTimeout; } - @Override - public ApplicationProtocolSelector nextProtocolSelector() { - return null; - } - @Override public List nextProtocols() { - return unmodifiableNextProtocols; + return nextProtocols; } /** @@ -319,11 +315,11 @@ public final class OpenSslServerContext extends SslContext { */ @Override public SSLEngine newEngine() { - if (unmodifiableNextProtocols.isEmpty()) { + if (nextProtocols.isEmpty()) { return new OpenSslEngine(ctx, bufferPool(), null); } else { return new OpenSslEngine( - ctx, bufferPool(), unmodifiableNextProtocols.get(unmodifiableNextProtocols.size() - 1)); + ctx, bufferPool(), nextProtocols.get(nextProtocols.size() - 1)); } } diff --git a/src/main/java/org/jboss/netty/handler/ssl/SslContext.java b/src/main/java/org/jboss/netty/handler/ssl/SslContext.java index 461e368e95..d8db67153c 100644 --- a/src/main/java/org/jboss/netty/handler/ssl/SslContext.java +++ b/src/main/java/org/jboss/netty/handler/ssl/SslContext.java @@ -26,7 +26,6 @@ import javax.net.ssl.SSLException; import javax.net.ssl.TrustManager; import javax.net.ssl.TrustManagerFactory; import java.io.File; -import java.util.ArrayList; import java.util.List; /** @@ -265,9 +264,8 @@ public abstract class SslContext { * {@code null} to use the default. * @param ciphers the cipher suites to enable, in the order of preference. * {@code null} to use the default cipher suites. - * @param nextProtocolSelector the {@link ApplicationProtocolSelector} that chooses one of the application layer - * protocols returned by a TLS server. - * {@code null} to disable TLS NPN/ALPN extension. + * @param nextProtocols the application layer protocols to accept, in the order of preference. + * {@code null} to disable TLS NPN/ALPN extension. * @param sessionCacheSize the size of the cache used for storing SSL session objects. * {@code 0} to use the default value. * @param sessionTimeout the timeout for the cached SSL session objects, in seconds. @@ -278,11 +276,11 @@ public abstract class SslContext { public static SslContext newClientContext( SslBufferPool bufPool, File certChainFile, TrustManagerFactory trustManagerFactory, - Iterable ciphers, ApplicationProtocolSelector nextProtocolSelector, + Iterable ciphers, Iterable nextProtocols, long sessionCacheSize, long sessionTimeout) throws SSLException { return newClientContext( null, bufPool, certChainFile, trustManagerFactory, - ciphers, nextProtocolSelector, sessionCacheSize, sessionTimeout); + ciphers, nextProtocols, sessionCacheSize, sessionTimeout); } /** @@ -359,9 +357,8 @@ public abstract class SslContext { * {@code null} to use the default. * @param ciphers the cipher suites to enable, in the order of preference. * {@code null} to use the default cipher suites. - * @param nextProtocolSelector the {@link ApplicationProtocolSelector} that chooses one of the application layer - * protocols returned by a TLS server. - * {@code null} to disable TLS NPN/ALPN extension. + * @param nextProtocols the application layer protocols to accept, in the order of preference. + * {@code null} to disable TLS NPN/ALPN extension. * @param sessionCacheSize the size of the cache used for storing SSL session objects. * {@code 0} to use the default value. * @param sessionTimeout the timeout for the cached SSL session objects, in seconds. @@ -372,7 +369,7 @@ public abstract class SslContext { public static SslContext newClientContext( SslProvider provider, SslBufferPool bufPool, File certChainFile, TrustManagerFactory trustManagerFactory, - Iterable ciphers, ApplicationProtocolSelector nextProtocolSelector, + Iterable ciphers, Iterable nextProtocols, long sessionCacheSize, long sessionTimeout) throws SSLException { if (provider != null && provider != SslProvider.JDK) { @@ -381,83 +378,7 @@ public abstract class SslContext { return new JdkSslClientContext( bufPool, certChainFile, trustManagerFactory, - ciphers, nextProtocolSelector, sessionCacheSize, sessionTimeout); - } - - /** - * Creates a simple client-side {@link ApplicationProtocolSelector} that selects the most preferred protocol - * among the application protocols sent by the server. If there is no match, it chooses the least preferred one. - * - * @param nextProtocols the list of the supported client-side application protocols, in the order of preference - * @return the new {@link ApplicationProtocolSelector}. - * {@code null} if the specified {@code nextProtocols} does not contain any elements. - * - */ - public static ApplicationProtocolSelector newApplicationProtocolSelector(String... nextProtocols) { - if (nextProtocols == null) { - throw new NullPointerException("nextProtocols"); - } - - final List list = new ArrayList(); - for (String p: nextProtocols) { - if (p == null) { - break; - } - list.add(p); - } - - if (list.isEmpty()) { - return null; - } - - return newApplicationProtocolSelector(list); - } - - private static ApplicationProtocolSelector newApplicationProtocolSelector(final List list) { - return new ApplicationProtocolSelector() { - public String selectProtocol(List protocols) throws Exception { - for (String p: list) { - if (protocols.contains(p)) { - return p; - } - } - return list.get(list.size() - 1); - } - - @Override - public String toString() { - return "ApplicationProtocolSelector(" + list + ')'; - } - }; - } - - /** - * Creates a simple client-side {@link ApplicationProtocolSelector} that selects the most preferred protocol - * among the application protocols sent by the server. If there is no match, it chooses the least preferred one. - * - * @param nextProtocols the list of the supported client-side application protocols, in the order of preference - * @return the new {@link ApplicationProtocolSelector}. - * {@code null} if the specified {@code nextProtocols} does not contain any elements. - * - */ - public static ApplicationProtocolSelector newApplicationProtocolSelector(Iterable nextProtocols) { - if (nextProtocols == null) { - throw new NullPointerException("nextProtocols"); - } - - final List list = new ArrayList(); - for (String p: nextProtocols) { - if (p == null) { - break; - } - list.add(p); - } - - if (list.isEmpty()) { - return null; - } - - return newApplicationProtocolSelector(list); + ciphers, nextProtocols, sessionCacheSize, sessionTimeout); } private final SslBufferPool bufferPool; @@ -505,18 +426,9 @@ public abstract class SslContext { public abstract long sessionTimeout(); /** - * Returns the client-side {@link ApplicationProtocolSelector} for the TLS NPN/ALPN extension. + * Returns the list of application layer protocols for the TLS NPN/ALPN extension, in the order of preference. * - * @return the client-side {@link ApplicationProtocolSelector}. - * {@code null} if NPN/ALPN extension has been disabled. - */ - public abstract ApplicationProtocolSelector nextProtocolSelector(); - - /** - * Returns the list of server-side application layer protocols for the TLS NPN/ALPN extension, - * in the order of preference. - * - * @return the list of server-side application layer protocols. + * @return the list of application layer protocols. * {@code null} if NPN/ALPN extension has been disabled. */ public abstract List nextProtocols();