Author: markt Date: Mon Feb 25 17:38:35 2019 New Revision: 1854326 URL: http://svn.apache.org/viewvc?rev=1854326&view=rev Log: Refactor to a) reduce duplication and b) enable JSSE style config to be used with APR
Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java tomcat/trunk/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java tomcat/trunk/webapps/docs/changelog.xml Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java Mon Feb 25 17:38:35 2019 @@ -314,13 +314,34 @@ public abstract class AbstractEndpoint<S */ protected abstract void createSSLContext(SSLHostConfig sslHostConfig) throws Exception; + + protected void destroySsl() throws Exception { + if (isSSLEnabled()) { + for (SSLHostConfig sslHostConfig : sslHostConfigs.values()) { + releaseSSLContext(sslHostConfig); + } + } + } + + /** * Release the SSLContext, if any, associated with the SSLHostConfig. * * @param sslHostConfig The SSLHostConfig for which the SSLContext should be * released */ - protected abstract void releaseSSLContext(SSLHostConfig sslHostConfig); + protected void releaseSSLContext(SSLHostConfig sslHostConfig) { + for (SSLHostConfigCertificate certificate : sslHostConfig.getCertificates(true)) { + if (certificate.getSslContext() != null) { + SSLContext sslContext = certificate.getSslContext(); + if (sslContext != null) { + sslContext.destroy(); + } + } + } + } + + protected SSLHostConfig getSSLHostConfig(String sniHostName) { SSLHostConfig result = null; Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AbstractJsseEndpoint.java Mon Feb 25 17:38:35 2019 @@ -117,28 +117,6 @@ public abstract class AbstractJsseEndpoi } - protected void destroySsl() throws Exception { - if (isSSLEnabled()) { - for (SSLHostConfig sslHostConfig : sslHostConfigs.values()) { - releaseSSLContext(sslHostConfig); - } - } - } - - - @Override - protected void releaseSSLContext(SSLHostConfig sslHostConfig) { - for (SSLHostConfigCertificate certificate : sslHostConfig.getCertificates(true)) { - if (certificate.getSslContext() != null) { - SSLContext sslContext = certificate.getSslContext(); - if (sslContext != null) { - sslContext.destroy(); - } - } - } - } - - protected SSLEngine createSSLEngine(String sniHostName, List<Cipher> clientRequestedCiphers, List<String> clientRequestedApplicationProtocols) { SSLHostConfig sslHostConfig = getSSLHostConfig(sniHostName); Modified: tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/AprEndpoint.java Mon Feb 25 17:38:35 2019 @@ -24,7 +24,6 @@ import java.nio.ByteBuffer; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ConcurrentHashMap; @@ -43,7 +42,6 @@ import org.apache.tomcat.jni.OS; import org.apache.tomcat.jni.Poll; import org.apache.tomcat.jni.Pool; import org.apache.tomcat.jni.SSL; -import org.apache.tomcat.jni.SSLConf; import org.apache.tomcat.jni.SSLContext; import org.apache.tomcat.jni.SSLContext.SNICallBack; import org.apache.tomcat.jni.SSLSocket; @@ -56,8 +54,8 @@ import org.apache.tomcat.util.collection import org.apache.tomcat.util.net.AbstractEndpoint.Handler.SocketState; import org.apache.tomcat.util.net.Acceptor.AcceptorState; import org.apache.tomcat.util.net.SSLHostConfig.Type; -import org.apache.tomcat.util.net.openssl.OpenSSLConf; -import org.apache.tomcat.util.net.openssl.OpenSSLEngine; +import org.apache.tomcat.util.net.openssl.OpenSSLContext; +import org.apache.tomcat.util.net.openssl.OpenSSLUtil; /** @@ -382,6 +380,14 @@ public class AprEndpoint extends Abstrac Long defaultSSLContext = defaultSSLHostConfig.getOpenSslContext(); sslContext = defaultSSLContext.longValue(); SSLContext.registerDefault(defaultSSLContext, this); + + // For now, sendfile is not supported with SSL + if (getUseSendfile()) { + setUseSendfileInternal(false); + if (useSendFileSet) { + log.warn(sm.getString("endpoint.apr.noSendfileWithSSL")); + } + } } } @@ -389,249 +395,29 @@ public class AprEndpoint extends Abstrac @Override protected void createSSLContext(SSLHostConfig sslHostConfig) throws Exception { + OpenSSLContext sslContext = null; Set<SSLHostConfigCertificate> certificates = sslHostConfig.getCertificates(true); - boolean firstCertificate = true; for (SSLHostConfigCertificate certificate : certificates) { - if (SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()) == null) { - // This is required - throw new Exception(sm.getString("endpoint.apr.noSslCertFile")); - } - if (firstCertificate) { - // TODO: Duplicates code in SSLUtilBase. Consider - // refactoring to reduce duplication - firstCertificate = false; - // Configure the enabled protocols - List<String> enabledProtocols = SSLUtilBase.getEnabled("protocols", log, - true, sslHostConfig.getProtocols(), - OpenSSLEngine.IMPLEMENTED_PROTOCOLS_SET); - sslHostConfig.setEnabledProtocols( - enabledProtocols.toArray(new String[enabledProtocols.size()])); - // Configure the enabled ciphers - List<String> enabledCiphers = SSLUtilBase.getEnabled("ciphers", log, - false, sslHostConfig.getJsseCipherNames(), - OpenSSLEngine.AVAILABLE_CIPHER_SUITES); - sslHostConfig.setEnabledCiphers( - enabledCiphers.toArray(new String[enabledCiphers.size()])); - } - } - if (certificates.size() > 2) { - // TODO: Can this limitation be removed? - throw new Exception(sm.getString("endpoint.apr.tooManyCertFiles")); - } - - // SSL protocol - int value = SSL.SSL_PROTOCOL_NONE; - if (sslHostConfig.getProtocols().size() == 0) { - // Native fallback used if protocols="" - value = SSL.SSL_PROTOCOL_ALL; - } else { - for (String protocol : sslHostConfig.getEnabledProtocols()) { - if (Constants.SSL_PROTO_SSLv2Hello.equalsIgnoreCase(protocol)) { - // NO-OP. OpenSSL always supports SSLv2Hello - } else if (Constants.SSL_PROTO_SSLv2.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_SSLV2; - } else if (Constants.SSL_PROTO_SSLv3.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_SSLV3; - } else if (Constants.SSL_PROTO_TLSv1.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_TLSV1; - } else if (Constants.SSL_PROTO_TLSv1_1.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_TLSV1_1; - } else if (Constants.SSL_PROTO_TLSv1_2.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_TLSV1_2; - } else if (Constants.SSL_PROTO_TLSv1_3.equalsIgnoreCase(protocol)) { - value |= SSL.SSL_PROTOCOL_TLSV1_3; - } else { - // Should not happen since filtering to build - // enabled protocols removes invalid values. - throw new Exception(sm.getString( - "endpoint.apr.invalidSslProtocol", protocol)); - } - } - } - - // Create SSL Context - long ctx = 0; - try { - ctx = SSLContext.make(rootPool, value, SSL.SSL_MODE_SERVER); - } catch (Exception e) { - // If the sslEngine is disabled on the AprLifecycleListener - // there will be an Exception here but there is no way to check - // the AprLifecycleListener settings from here - throw new Exception( - sm.getString("endpoint.apr.failSslContextMake"), e); - } - - if (sslHostConfig.getInsecureRenegotiation()) { - SSLContext.setOptions(ctx, SSL.SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION); - } else { - SSLContext.clearOptions(ctx, SSL.SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION); - } - - // Use server's preference order for ciphers (rather than client's) - if (sslHostConfig.getHonorCipherOrder()) { - SSLContext.setOptions(ctx, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); - } else { - SSLContext.clearOptions(ctx, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE); - } - - // Disable compression if requested - if (sslHostConfig.getDisableCompression()) { - SSLContext.setOptions(ctx, SSL.SSL_OP_NO_COMPRESSION); - } else { - SSLContext.clearOptions(ctx, SSL.SSL_OP_NO_COMPRESSION); - } - - // Disable TLS Session Tickets (RFC4507) to protect perfect forward secrecy - if (sslHostConfig.getDisableSessionTickets()) { - SSLContext.setOptions(ctx, SSL.SSL_OP_NO_TICKET); - } else { - SSLContext.clearOptions(ctx, SSL.SSL_OP_NO_TICKET); - } - - // List the ciphers that the client is permitted to negotiate - SSLContext.setCipherSuite(ctx, sslHostConfig.getCiphers()); - // Load Server key and certificate - int idx = 0; - for (SSLHostConfigCertificate certificate : sslHostConfig.getCertificates(true)) { - SSLContext.setCertificate(ctx, - SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()), - SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()), - certificate.getCertificateKeyPassword(), idx++); - // Set certificate chain file - SSLContext.setCertificateChainFile(ctx, - SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()), false); - } - // Support Client Certificates - SSLContext.setCACertificate(ctx, - SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificateFile()), - SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificatePath())); - // Set revocation - SSLContext.setCARevocation(ctx, - SSLHostConfig.adjustRelativePath( - sslHostConfig.getCertificateRevocationListFile()), - SSLHostConfig.adjustRelativePath( - sslHostConfig.getCertificateRevocationListPath())); - // Client certificate verification - switch (sslHostConfig.getCertificateVerification()) { - case NONE: - value = SSL.SSL_CVERIFY_NONE; - break; - case OPTIONAL: - value = SSL.SSL_CVERIFY_OPTIONAL; - break; - case OPTIONAL_NO_CA: - value = SSL.SSL_CVERIFY_OPTIONAL_NO_CA; - break; - case REQUIRED: - value = SSL.SSL_CVERIFY_REQUIRE; - break; - } - SSLContext.setVerify(ctx, value, sslHostConfig.getCertificateVerificationDepth()); - // For now, sendfile is not supported with SSL - if (getUseSendfile()) { - setUseSendfileInternal(false); - if (useSendFileSet) { - log.warn(sm.getString("endpoint.apr.noSendfileWithSSL")); - } - } - - if (negotiableProtocols.size() > 0) { - List<String> protocols = new ArrayList<>(); - protocols.addAll(negotiableProtocols); - protocols.add("http/1.1"); - String[] protocolsArray = protocols.toArray(new String[0]); - SSLContext.setAlpnProtos(ctx, protocolsArray, SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE); - } - - // If client authentication is being used, OpenSSL requires that - // this is set so always set it in case an app is configured to require - // it - SSLContext.setSessionIdContext(ctx, SSLContext.DEFAULT_SESSION_ID_CONTEXT); + if (sslContext == null) { + SSLUtil sslUtil = new OpenSSLUtil(certificate); + sslHostConfig.setEnabledProtocols(sslUtil.getEnabledProtocols()); + sslHostConfig.setEnabledCiphers(sslUtil.getEnabledCiphers()); - long cctx; - OpenSSLConf openSslConf = sslHostConfig.getOpenSslConf(); - if (openSslConf != null) { - // Create OpenSSLConfCmd context if used - try { - if (log.isDebugEnabled()) - log.debug(sm.getString("endpoint.apr.makeConf")); - cctx = SSLConf.make(rootPool, - SSL.SSL_CONF_FLAG_FILE | - SSL.SSL_CONF_FLAG_SERVER | - SSL.SSL_CONF_FLAG_CERTIFICATE | - SSL.SSL_CONF_FLAG_SHOW_ERRORS); - } catch (Exception e) { - throw new Exception(sm.getString("endpoint.apr.errMakeConf"), e); - } - if (cctx != 0) { - // Check OpenSSLConfCmd if used - if (log.isDebugEnabled()) - log.debug(sm.getString("endpoint.apr.checkConf")); try { - if (!openSslConf.check(cctx)) { - log.error(sm.getString("endpoint.apr.errCheckConf")); - throw new Exception(sm.getString("endpoint.apr.errCheckConf")); - } + sslContext = (OpenSSLContext) sslUtil.createSSLContext(negotiableProtocols); } catch (Exception e) { - throw new Exception(sm.getString("endpoint.apr.errCheckConf"), e); - } - // Apply OpenSSLConfCmd if used - if (log.isDebugEnabled()) - log.debug(sm.getString("endpoint.apr.applyConf")); - try { - if (!openSslConf.apply(cctx, ctx)) { - log.error(sm.getString("endpoint.apr.errApplyConf")); - throw new Exception(sm.getString("endpoint.apr.errApplyConf")); - } - } catch (Exception e) { - throw new Exception(sm.getString("endpoint.apr.errApplyConf"), e); - } - // Reconfigure the enabled protocols - int opts = SSLContext.getOptions(ctx); - List<String> enabled = new ArrayList<>(); - // Seems like there is no way to explicitly disable SSLv2Hello - // in OpenSSL so it is always enabled - enabled.add(Constants.SSL_PROTO_SSLv2Hello); - if ((opts & SSL.SSL_OP_NO_TLSv1) == 0) { - enabled.add(Constants.SSL_PROTO_TLSv1); - } - if ((opts & SSL.SSL_OP_NO_TLSv1_1) == 0) { - enabled.add(Constants.SSL_PROTO_TLSv1_1); - } - if ((opts & SSL.SSL_OP_NO_TLSv1_2) == 0) { - enabled.add(Constants.SSL_PROTO_TLSv1_2); - } - if ((opts & SSL.SSL_OP_NO_SSLv2) == 0) { - enabled.add(Constants.SSL_PROTO_SSLv2); + throw new IllegalArgumentException(e.getMessage(), e); } - if ((opts & SSL.SSL_OP_NO_SSLv3) == 0) { - enabled.add(Constants.SSL_PROTO_SSLv3); - } - sslHostConfig.setEnabledProtocols( - enabled.toArray(new String[enabled.size()])); - // Reconfigure the enabled ciphers - sslHostConfig.setEnabledCiphers(SSLContext.getCiphers(ctx)); + } else { + sslContext.addCertificate(certificate); } - } else { - cctx = 0; - } - - sslHostConfig.setOpenSslConfContext(Long.valueOf(cctx)); - sslHostConfig.setOpenSslContext(Long.valueOf(ctx)); - } + certificate.setSslContext(sslContext); + } - @Override - protected void releaseSSLContext(SSLHostConfig sslHostConfig) { - Long ctx = sslHostConfig.getOpenSslContext(); - if (ctx != null && ctx.longValue() != 0L) { - SSLContext.free(ctx.longValue()); - sslHostConfig.setOpenSslContext(null); - } - Long cctx = sslHostConfig.getOpenSslConfContext(); - if (cctx != null && cctx.longValue() != 0L) { - SSLConf.free(cctx.longValue()); - sslHostConfig.setOpenSslConfContext(null); + if (certificates.size() > 2) { + // TODO: Can this limitation be removed? + throw new Exception(sm.getString("endpoint.apr.tooManyCertFiles")); } } @@ -774,15 +560,7 @@ public class AprEndpoint extends Abstrac } doCloseServerSocket(); - - if (sslContext != 0) { - Long ctx = Long.valueOf(sslContext); - SSLContext.unregisterDefault(ctx); - for (SSLHostConfig sslHostConfig : sslHostConfigs.values()) { - sslHostConfig.setOpenSslContext(null); - } - sslContext = 0; - } + destroySsl(); // Close all APR memory pools and resources if initialised if (rootPool != 0) { Modified: tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties [UTF-8] (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/LocalStrings.properties [UTF-8] Mon Feb 25 17:38:35 2019 @@ -56,7 +56,6 @@ endpoint.apr.makeConf=Creating OpenSSLCo endpoint.apr.maxConnections.running=The APR endpoint does not support the setting of maxConnections while it is running. The existing value of [{0}] will continue to be used. endpoint.apr.maxConnections.unlimited=The APR endpoint does not support unlimited connections. The existing value of [{0}] will continue to be used. endpoint.apr.noSendfileWithSSL=Sendfile is not supported for the APR/native connector when SSL is enabled -endpoint.apr.noSslCertFile=Connector attribute SSLCertificateFile must be defined when using SSL with APR endpoint.apr.pollAddInvalid=Invalid attempt to add a socket [{0}] to the poller endpoint.apr.pollError=Poller failed with error [{0}] : [{1}] endpoint.apr.pollMergeEvents=Merge poller event [{1}] for socket [{0}] to create merged event [{2}] Modified: tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java (original) +++ tomcat/trunk/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java Mon Feb 25 17:38:35 2019 @@ -151,9 +151,8 @@ public class OpenSSLContext implements o } else if (Constants.SSL_PROTO_ALL.equalsIgnoreCase(protocol)) { value |= SSL.SSL_PROTOCOL_ALL; } else { - // Protocol not recognized, fail to start as it is safer than - // continuing with the default which might enable more than the - // is required + // Should not happen since filtering to build + // enabled protocols removes invalid values. throw new Exception(netSm.getString( "endpoint.apr.invalidSslProtocol", protocol)); } Modified: tomcat/trunk/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java (original) +++ tomcat/trunk/test/org/apache/tomcat/util/net/TestSSLHostConfigCompat.java Mon Feb 25 17:38:35 2019 @@ -23,7 +23,6 @@ import java.util.List; import org.junit.Assert; import org.junit.Assume; -import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; @@ -215,7 +214,6 @@ public class TestSSLHostConfigCompat ext @Test - @Ignore // Currently the APR/native connector cannot be configured using a Keystore public void testHostKeystore() throws Exception { sslHostConfig.setCertificateKeystoreFile(getPath(TesterSupport.LOCALHOST_JKS)); doTest(); Modified: tomcat/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1854326&r1=1854325&r2=1854326&view=diff ============================================================================== --- tomcat/trunk/webapps/docs/changelog.xml (original) +++ tomcat/trunk/webapps/docs/changelog.xml Mon Feb 25 17:38:35 2019 @@ -73,6 +73,11 @@ <bug>63182</bug>: Avoid extra notifications when using non container threads on read causing thread safety problems. (remm) </fix> + <fix> + Refactor the APR/Native endpoint TLS configuration code to enable JSSE + style configuration - including JKS keystores - to be used with the + APR/Native connector. (markt) + </fix> </changelog> </subsection> <subsection name="WebSocket"> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org