You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2021/08/23 11:01:23 UTC
[tomcat] branch main updated: Refactor JSSE/OpenSSL integration to
avoid use of finalize()
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/main by this push:
new e45117f Refactor JSSE/OpenSSL integration to avoid use of finalize()
e45117f is described below
commit e45117fd14f987c3c7199d1e87c26a02e8fec0e7
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Aug 23 12:01:14 2021 +0100
Refactor JSSE/OpenSSL integration to avoid use of finalize()
---
.../tomcat/util/net/openssl/OpenSSLContext.java | 146 ++++++++++---------
.../tomcat/util/net/openssl/OpenSSLEngine.java | 157 +++++++++++----------
webapps/docs/changelog.xml | 4 +
3 files changed, 169 insertions(+), 138 deletions(-)
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
index 7e6d198..0ecc6f2 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLContext.java
@@ -16,6 +16,8 @@
*/
package org.apache.tomcat.util.net.openssl;
+import java.lang.ref.Cleaner;
+import java.lang.ref.Cleaner.Cleanable;
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey;
import java.security.SecureRandom;
@@ -27,7 +29,6 @@ import java.util.Arrays;
import java.util.Base64;
import java.util.Iterator;
import java.util.List;
-import java.util.concurrent.atomic.AtomicInteger;
import javax.net.ssl.KeyManager;
import javax.net.ssl.SSLEngine;
@@ -76,27 +77,27 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
}
}
+ private static final Cleaner cleaner = Cleaner.create();
+
private final SSLHostConfig sslHostConfig;
private final SSLHostConfigCertificate certificate;
private final List<String> negotiableProtocols;
- private final long aprPool;
- private final AtomicInteger aprPoolDestroyed = new AtomicInteger(0);
- // OpenSSLConfCmd context
- protected final long cctx;
- // SSL context
- protected final long ctx;
private OpenSSLSessionContext sessionContext;
private X509TrustManager x509TrustManager;
private String enabledProtocol;
private boolean initialized = false;
+ private final OpenSSLState state;
+ private final Cleanable cleanable;
public OpenSSLContext(SSLHostConfigCertificate certificate, List<String> negotiableProtocols)
throws SSLException {
this.sslHostConfig = certificate.getSSLHostConfig();
this.certificate = certificate;
- aprPool = Pool.create(0);
+ long aprPool = Pool.create(0);
+ long cctx = 0;
+ long ctx = 0;
boolean success = false;
try {
// Create OpenSSLConfCmd context if used
@@ -114,8 +115,6 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
} catch (Exception e) {
throw new SSLException(sm.getString("openssl.errMakeConf"), e);
}
- } else {
- cctx = 0;
}
sslHostConfig.setOpenSslConfContext(Long.valueOf(cctx));
@@ -163,6 +162,20 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
} catch(Exception e) {
throw new SSLException(sm.getString("openssl.errorSSLCtxInit"), e);
} finally {
+ state = new OpenSSLState(aprPool, cctx, ctx);
+ /*
+ * When an SSLHostConfig is replaced at runtime, it is not possible to
+ * call destroy() on the associated OpenSSLContext since it is likely
+ * that there will be in-progress connections using the OpenSSLContext.
+ * A reference chain has been deliberately established (see
+ * OpenSSLSessionContext) to ensure that the OpenSSLContext remains
+ * ineligible for GC while those connections are alive. Once those
+ * connections complete, the OpenSSLContext will become eligible for GC
+ * and this method will ensure that the associated native resources are
+ * cleaned up.
+ */
+ cleanable = cleaner.register(this, state);
+
if (!success) {
destroy();
}
@@ -182,20 +195,10 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
@Override
public synchronized void destroy() {
- // Guard against multiple destroyPools() calls triggered by construction exception and finalize() later
- if (aprPoolDestroyed.compareAndSet(0, 1)) {
- if (ctx != 0) {
- SSLContext.free(ctx);
- }
- if (cctx != 0) {
- SSLConf.free(cctx);
- }
- if (aprPool != 0) {
- Pool.destroy(aprPool);
- }
- }
+ cleanable.clean();
}
+
/**
* Setup the SSL_CTX.
*
@@ -213,35 +216,35 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
}
try {
if (sslHostConfig.getInsecureRenegotiation()) {
- SSLContext.setOptions(ctx, SSL.SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
+ SSLContext.setOptions(state.ctx, SSL.SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
} else {
- SSLContext.clearOptions(ctx, SSL.SSL_OP_ALLOW_UNSAFE_LEGACY_RENEGOTIATION);
+ SSLContext.clearOptions(state.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);
+ SSLContext.setOptions(state.ctx, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE);
} else {
- SSLContext.clearOptions(ctx, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE);
+ SSLContext.clearOptions(state.ctx, SSL.SSL_OP_CIPHER_SERVER_PREFERENCE);
}
// Disable compression if requested
if (sslHostConfig.getDisableCompression()) {
- SSLContext.setOptions(ctx, SSL.SSL_OP_NO_COMPRESSION);
+ SSLContext.setOptions(state.ctx, SSL.SSL_OP_NO_COMPRESSION);
} else {
- SSLContext.clearOptions(ctx, SSL.SSL_OP_NO_COMPRESSION);
+ SSLContext.clearOptions(state.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);
+ SSLContext.setOptions(state.ctx, SSL.SSL_OP_NO_TICKET);
} else {
- SSLContext.clearOptions(ctx, SSL.SSL_OP_NO_TICKET);
+ SSLContext.clearOptions(state.ctx, SSL.SSL_OP_NO_TICKET);
}
// List the ciphers that the client is permitted to negotiate
- SSLContext.setCipherSuite(ctx, sslHostConfig.getCiphers());
+ SSLContext.setCipherSuite(state.ctx, sslHostConfig.getCiphers());
if (certificate.getCertificateFile() == null) {
certificate.setCertificateKeyManager(OpenSSLUtil.chooseKeyManager(kms));
@@ -265,12 +268,12 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
value = SSL.SSL_CVERIFY_REQUIRE;
break;
}
- SSLContext.setVerify(ctx, value, sslHostConfig.getCertificateVerificationDepth());
+ SSLContext.setVerify(state.ctx, value, sslHostConfig.getCertificateVerificationDepth());
if (tms != null) {
// Client certificate verification based on custom trust managers
x509TrustManager = chooseTrustManager(tms);
- SSLContext.setCertVerifyCallback(ctx, new CertificateVerifier() {
+ SSLContext.setCertVerifyCallback(state.ctx, new CertificateVerifier() {
@Override
public boolean verify(long ssl, byte[][] chain, String auth) {
X509Certificate[] peerCerts = certificates(chain);
@@ -288,14 +291,14 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
// by the server during the handshake to allow the client choosing
// an acceptable certificate
for (X509Certificate caCert : x509TrustManager.getAcceptedIssuers()) {
- SSLContext.addClientCACertificateRaw(ctx, caCert.getEncoded());
+ SSLContext.addClientCACertificateRaw(state.ctx, caCert.getEncoded());
if (log.isDebugEnabled()) {
log.debug(sm.getString("openssl.addedClientCaCert", caCert.toString()));
}
}
} else {
// Client certificate verification based on trusted CA files and dirs
- SSLContext.setCACertificate(ctx,
+ SSLContext.setCACertificate(state.ctx,
SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificateFile()),
SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificatePath()));
}
@@ -304,19 +307,19 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
List<String> protocols = new ArrayList<>(negotiableProtocols);
protocols.add("http/1.1");
String[] protocolsArray = protocols.toArray(new String[0]);
- SSLContext.setAlpnProtos(ctx, protocolsArray, SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE);
- SSLContext.setNpnProtos(ctx, protocolsArray, SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE);
+ SSLContext.setAlpnProtos(state.ctx, protocolsArray, SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE);
+ SSLContext.setNpnProtos(state.ctx, protocolsArray, SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE);
}
// Apply OpenSSLConfCmd if used
OpenSSLConf openSslConf = sslHostConfig.getOpenSslConf();
- if (openSslConf != null && cctx != 0) {
+ if (openSslConf != null && state.cctx != 0) {
// Check OpenSSLConfCmd if used
if (log.isDebugEnabled()) {
log.debug(sm.getString("openssl.checkConf"));
}
try {
- if (!openSslConf.check(cctx)) {
+ if (!openSslConf.check(state.cctx)) {
log.error(sm.getString("openssl.errCheckConf"));
throw new Exception(sm.getString("openssl.errCheckConf"));
}
@@ -327,7 +330,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
log.debug(sm.getString("openssl.applyConf"));
}
try {
- if (!openSslConf.apply(cctx, ctx)) {
+ if (!openSslConf.apply(state.cctx, state.ctx)) {
log.error(sm.getString("openssl.errApplyConf"));
throw new SSLException(sm.getString("openssl.errApplyConf"));
}
@@ -335,7 +338,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
throw new SSLException(sm.getString("openssl.errApplyConf"), e);
}
// Reconfigure the enabled protocols
- int opts = SSLContext.getOptions(ctx);
+ int opts = SSLContext.getOptions(state.ctx);
List<String> enabled = new ArrayList<>();
// Seems like there is no way to explicitly disable SSLv2Hello
// in OpenSSL so it is always enabled
@@ -358,7 +361,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
sslHostConfig.setEnabledProtocols(
enabled.toArray(new String[0]));
// Reconfigure the enabled ciphers
- sslHostConfig.setEnabledCiphers(SSLContext.getCiphers(ctx));
+ sslHostConfig.setEnabledCiphers(SSLContext.getCiphers(state.ctx));
}
sessionContext = new OpenSSLSessionContext(this);
@@ -366,7 +369,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
// this is set so always set it in case an app is configured to
// require it
sessionContext.setSessionIdContext(SSLContext.DEFAULT_SESSION_ID_CONTEXT);
- sslHostConfig.setOpenSslContext(Long.valueOf(ctx));
+ sslHostConfig.setOpenSslContext(Long.valueOf(state.ctx));
initialized = true;
} catch (Exception e) {
log.warn(sm.getString("openssl.errorSSLCtxInit"), e);
@@ -379,15 +382,15 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
// Load Server key and certificate
if (certificate.getCertificateFile() != null) {
// Set certificate
- SSLContext.setCertificate(ctx,
+ SSLContext.setCertificate(state.ctx,
SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()),
SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()),
certificate.getCertificateKeyPassword(), getCertificateIndex(certificate));
// Set certificate chain file
- SSLContext.setCertificateChainFile(ctx,
+ SSLContext.setCertificateChainFile(state.ctx,
SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()), false);
// Set revocation
- SSLContext.setCARevocation(ctx,
+ SSLContext.setCARevocation(state.ctx,
SSLHostConfig.adjustRelativePath(
sslHostConfig.getCertificateRevocationListFile()),
SSLHostConfig.adjustRelativePath(
@@ -407,11 +410,11 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
StringBuilder sb = new StringBuilder(BEGIN_KEY);
sb.append(Base64.getMimeEncoder(64, new byte[] {'\n'}).encodeToString(key.getEncoded()));
sb.append(END_KEY);
- SSLContext.setCertificateRaw(ctx, chain[0].getEncoded(),
+ SSLContext.setCertificateRaw(state.ctx, chain[0].getEncoded(),
sb.toString().getBytes(StandardCharsets.US_ASCII),
getCertificateIndex(certificate));
for (int i = 1; i < chain.length; i++) {
- SSLContext.addChainCertificateRaw(ctx, chain[i].getEncoded());
+ SSLContext.addChainCertificateRaw(state.ctx, chain[i].getEncoded());
}
}
}
@@ -480,7 +483,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
long getSSLContextID() {
- return ctx;
+ return state.ctx;
}
@@ -491,7 +494,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
@Override
public SSLEngine createSSLEngine() {
- return new OpenSSLEngine(ctx, defaultProtocol, false, sessionContext,
+ return new OpenSSLEngine(cleaner, state.ctx, defaultProtocol, false, sessionContext,
(negotiableProtocols != null && negotiableProtocols.size() > 0), initialized,
sslHostConfig.getCertificateVerificationDepth(),
sslHostConfig.getCertificateVerification() == CertificateVerification.OPTIONAL_NO_CA);
@@ -534,23 +537,32 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext {
return acceptedCerts;
}
- @Override
- protected void finalize() throws Throwable {
- /*
- * When an SSLHostConfig is replaced at runtime, it is not possible to
- * call destroy() on the associated OpenSSLContext since it is likely
- * that there will be in-progress connections using the OpenSSLContext.
- * A reference chain has been deliberately established (see
- * OpenSSLSessionContext) to ensure that the OpenSSLContext remains
- * ineligible for GC while those connections are alive. Once those
- * connections complete, the OpenSSLContext will become eligible for GC
- * and this method will ensure that the associated native resources are
- * cleaned up.
- */
- try {
- destroy();
- } finally {
- super.finalize();
+
+ private static class OpenSSLState implements Runnable {
+
+ final long aprPool;
+ // OpenSSLConfCmd context
+ final long cctx;
+ // SSL context
+ final long ctx;
+
+ private OpenSSLState(long aprPool, long cctx, long ctx) {
+ this.aprPool = aprPool;
+ this.cctx = cctx;
+ this.ctx = ctx;
+ }
+
+ @Override
+ public void run() {
+ if (ctx != 0) {
+ SSLContext.free(ctx);
+ }
+ if (cctx != 0) {
+ SSLConf.free(cctx);
+ }
+ if (aprPool != 0) {
+ Pool.destroy(aprPool);
+ }
}
}
}
diff --git a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
index 0a9b463..ed48e7a 100644
--- a/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/OpenSSLEngine.java
@@ -16,6 +16,8 @@
*/
package org.apache.tomcat.util.net.openssl;
+import java.lang.ref.Cleaner;
+import java.lang.ref.Cleaner.Cleanable;
import java.nio.ByteBuffer;
import java.nio.ReadOnlyBufferException;
import java.security.Principal;
@@ -132,9 +134,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
private static final long EMPTY_ADDR = Buffer.address(ByteBuffer.allocate(0));
- // OpenSSL state
- private final long ssl;
- private final long networkBIO;
+ private final OpenSSLState state;
+ private final Cleanable cleanable;
private enum Accepted { NOT, IMPLICIT, EXPLICIT }
private Accepted accepted = Accepted.NOT;
@@ -175,6 +176,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
/**
* Creates a new instance
*
+ * @param cleaner Used to clean up references to instances before they are
+ * garbage collected
* @param sslCtx an OpenSSL {@code SSL_CTX} object
* @param fallbackApplicationProtocol the fallback application protocol
* @param clientMode {@code true} if this is used for clients, {@code false}
@@ -189,7 +192,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
* @param certificateVerificationOptionalNoCA Skip CA verification in
* optional mode
*/
- OpenSSLEngine(long sslCtx, String fallbackApplicationProtocol,
+ OpenSSLEngine(Cleaner cleaner, long sslCtx, String fallbackApplicationProtocol,
boolean clientMode, OpenSSLSessionContext sessionContext, boolean alpn,
boolean initialized, int certificateVerificationDepth,
boolean certificateVerificationOptionalNoCA) {
@@ -197,8 +200,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
throw new IllegalArgumentException(sm.getString("engine.noSSLContext"));
}
session = new OpenSSLSession();
- ssl = SSL.newSSL(sslCtx, !clientMode);
- networkBIO = SSL.makeNetworkBIO(ssl);
+ long ssl = SSL.newSSL(sslCtx, !clientMode);
+ long networkBIO = SSL.makeNetworkBIO(ssl);
+ state = new OpenSSLState(ssl, networkBIO);
+ cleanable = cleaner.register(this, state);
this.fallbackApplicationProtocol = fallbackApplicationProtocol;
this.clientMode = clientMode;
this.sessionContext = sessionContext;
@@ -219,12 +224,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
public synchronized void shutdown() {
if (!destroyed) {
destroyed = true;
- if (networkBIO != 0) {
- SSL.freeBIO(networkBIO);
- }
- if (ssl != 0) {
- SSL.freeSSL(ssl);
- }
+ cleanable.clean();
// internal errors can cause shutdown without marking the engine closed
isInboundDone = isOutboundDone = engineClosed = true;
}
@@ -450,7 +450,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
int pendingNet;
// Check for pending data in the network BIO
- pendingNet = SSL.pendingWrittenBytesInBIO(networkBIO);
+ pendingNet = SSL.pendingWrittenBytesInBIO(state.networkBIO);
if (pendingNet > 0) {
// Do we have enough room in destination to write encrypted data?
int capacity = dst.remaining();
@@ -460,7 +460,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// Write the pending data from the network BIO into the dst buffer
try {
- bytesProduced = readEncryptedData(networkBIO, dst, pendingNet);
+ bytesProduced = readEncryptedData(state.networkBIO, dst, pendingNet);
} catch (Exception e) {
throw new SSLException(e);
}
@@ -487,13 +487,13 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// Write plain text application data to the SSL engine
try {
- bytesConsumed += writePlaintextData(ssl, src);
+ bytesConsumed += writePlaintextData(state.ssl, src);
} catch (Exception e) {
throw new SSLException(e);
}
// Check to see if the engine wrote data into the network BIO
- pendingNet = SSL.pendingWrittenBytesInBIO(networkBIO);
+ pendingNet = SSL.pendingWrittenBytesInBIO(state.networkBIO);
if (pendingNet > 0) {
// Do we have enough room in dst to write encrypted data?
int capacity = dst.remaining();
@@ -504,7 +504,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// Write the pending data from the network BIO into the dst buffer
try {
- bytesProduced += readEncryptedData(networkBIO, dst, pendingNet);
+ bytesProduced += readEncryptedData(state.networkBIO, dst, pendingNet);
} catch (Exception e) {
throw new SSLException(e);
}
@@ -571,7 +571,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// Write encrypted data to network BIO
int written = 0;
try {
- written = writeEncryptedData(networkBIO, src);
+ written = writeEncryptedData(state.networkBIO, src);
} catch (Exception e) {
throw new SSLException(e);
}
@@ -610,7 +610,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
int bytesRead;
try {
- bytesRead = readPlaintextData(ssl, dst);
+ bytesRead = readPlaintextData(state.ssl, dst);
} catch (Exception e) {
throw new SSLException(e);
}
@@ -637,7 +637,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
}
// Check to see if we received a close_notify message from the peer
- if (!receivedShutdown && (SSL.getShutdown(ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) {
+ if (!receivedShutdown && (SSL.getShutdown(state.ssl) & SSL.SSL_RECEIVED_SHUTDOWN) == SSL.SSL_RECEIVED_SHUTDOWN) {
receivedShutdown = true;
closeOutbound();
closeInbound();
@@ -655,13 +655,13 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// SSL_pending will return 0 if OpenSSL has not started the current TLS record
// See https://www.openssl.org/docs/manmaster/man3/SSL_pending.html
clearLastError();
- int lastPrimingReadResult = SSL.readFromSSL(ssl, EMPTY_ADDR, 0); // priming read
+ int lastPrimingReadResult = SSL.readFromSSL(state.ssl, EMPTY_ADDR, 0); // priming read
// check if SSL_read returned <= 0. In this case we need to check the error and see if it was something
// fatal.
if (lastPrimingReadResult <= 0) {
checkLastError();
}
- int pendingReadableBytesInSSL = SSL.pendingReadableBytesInSSL(ssl);
+ int pendingReadableBytesInSSL = SSL.pendingReadableBytesInSSL(state.ssl);
// TLS 1.0 needs additional handling
// TODO Figure out why this is necessary and if a simpler / better
@@ -669,11 +669,11 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (Constants.SSL_PROTO_TLSv1.equals(version) && lastPrimingReadResult == 0 &&
pendingReadableBytesInSSL == 0) {
// Perform another priming read
- lastPrimingReadResult = SSL.readFromSSL(ssl, EMPTY_ADDR, 0);
+ lastPrimingReadResult = SSL.readFromSSL(state.ssl, EMPTY_ADDR, 0);
if (lastPrimingReadResult <= 0) {
checkLastError();
}
- pendingReadableBytesInSSL = SSL.pendingReadableBytesInSSL(ssl);
+ pendingReadableBytesInSSL = SSL.pendingReadableBytesInSSL(state.ssl);
}
return pendingReadableBytesInSSL;
@@ -716,9 +716,9 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
engineClosed = true;
if (accepted != Accepted.NOT && !destroyed) {
- int mode = SSL.getShutdown(ssl);
+ int mode = SSL.getShutdown(state.ssl);
if ((mode & SSL.SSL_SENT_SHUTDOWN) != SSL.SSL_SENT_SHUTDOWN) {
- SSL.shutdownSSL(ssl);
+ SSL.shutdownSSL(state.ssl);
}
} else {
// engine closing before initial handshake
@@ -742,7 +742,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (destroyed) {
return new String[0];
}
- String[] enabled = SSL.getCiphers(ssl);
+ String[] enabled = SSL.getCiphers(state.ssl);
if (enabled == null) {
return new String[0];
} else {
@@ -791,7 +791,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
final String cipherSuiteSpec = buf.toString();
try {
- SSL.setCipherSuites(ssl, cipherSuiteSpec);
+ SSL.setCipherSuites(state.ssl, cipherSuiteSpec);
} catch (Exception e) {
throw new IllegalStateException(sm.getString("engine.failedCipherSuite", cipherSuiteSpec), e);
}
@@ -810,7 +810,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
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);
- int opts = SSL.getOptions(ssl);
+ int opts = SSL.getOptions(state.ssl);
if ((opts & SSL.SSL_OP_NO_TLSv1) == 0) {
enabled.add(Constants.SSL_PROTO_TLSv1);
}
@@ -868,22 +868,22 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
}
}
// Enable all and then disable what we not want
- SSL.setOptions(ssl, SSL.SSL_OP_ALL);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_ALL);
if (!sslv2) {
- SSL.setOptions(ssl, SSL.SSL_OP_NO_SSLv2);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_NO_SSLv2);
}
if (!sslv3) {
- SSL.setOptions(ssl, SSL.SSL_OP_NO_SSLv3);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_NO_SSLv3);
}
if (!tlsv1) {
- SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_NO_TLSv1);
}
if (!tlsv1_1) {
- SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1_1);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_NO_TLSv1_1);
}
if (!tlsv1_2) {
- SSL.setOptions(ssl, SSL.SSL_OP_NO_TLSv1_2);
+ SSL.setOptions(state.ssl, SSL.SSL_OP_NO_TLSv1_2);
}
}
@@ -923,16 +923,16 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
}
private void handshake() throws SSLException {
- currentHandshake = SSL.getHandshakeCount(ssl);
+ currentHandshake = SSL.getHandshakeCount(state.ssl);
clearLastError();
- int code = SSL.doHandshake(ssl);
+ int code = SSL.doHandshake(state.ssl);
if (code <= 0) {
checkLastError();
} else {
if (alpn) {
- selectedProtocol = SSL.getAlpnSelected(ssl);
+ selectedProtocol = SSL.getAlpnSelected(state.ssl);
if (selectedProtocol == null) {
- selectedProtocol = SSL.getNextProtoNegotiated(ssl);
+ selectedProtocol = SSL.getNextProtoNegotiated(state.ssl);
}
}
session.lastAccessedTime = System.currentTimeMillis();
@@ -945,10 +945,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
private synchronized void renegotiate() throws SSLException {
clearLastError();
int code;
- if (SSL.getVersion(ssl).equals(Constants.SSL_PROTO_TLSv1_3)) {
- code = SSL.verifyClientPostHandshake(ssl);
+ if (SSL.getVersion(state.ssl).equals(Constants.SSL_PROTO_TLSv1_3)) {
+ code = SSL.verifyClientPostHandshake(state.ssl);
} else {
- code = SSL.renegotiate(ssl);
+ code = SSL.renegotiate(state.ssl);
}
if (code <= 0) {
checkLastError();
@@ -956,8 +956,8 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
handshakeFinished = false;
peerCerts = null;
x509PeerCerts = null;
- currentHandshake = SSL.getHandshakeCount(ssl);
- int code2 = SSL.doHandshake(ssl);
+ currentHandshake = SSL.getHandshakeCount(state.ssl);
+ int code2 = SSL.doHandshake(state.ssl);
if (code2 <= 0) {
checkLastError();
}
@@ -1022,7 +1022,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (!handshakeFinished) {
// There is pending data in the network BIO -- call wrap
- if (sendHandshakeError || SSL.pendingWrittenBytesInBIO(networkBIO) != 0) {
+ if (sendHandshakeError || SSL.pendingWrittenBytesInBIO(state.networkBIO) != 0) {
if (sendHandshakeError) {
// After a last wrap, consider it is going to be done
sendHandshakeError = false;
@@ -1064,17 +1064,17 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// No pending data to be sent to the peer
// Check to see if we have finished handshaking
- int handshakeCount = SSL.getHandshakeCount(ssl);
- if (handshakeCount != currentHandshake && SSL.renegotiatePending(ssl) == 0 &&
- (SSL.getPostHandshakeAuthInProgress(ssl) == 0)) {
+ int handshakeCount = SSL.getHandshakeCount(state.ssl);
+ if (handshakeCount != currentHandshake && SSL.renegotiatePending(state.ssl) == 0 &&
+ (SSL.getPostHandshakeAuthInProgress(state.ssl) == 0)) {
if (alpn) {
- selectedProtocol = SSL.getAlpnSelected(ssl);
+ selectedProtocol = SSL.getAlpnSelected(state.ssl);
if (selectedProtocol == null) {
- selectedProtocol = SSL.getNextProtoNegotiated(ssl);
+ selectedProtocol = SSL.getNextProtoNegotiated(state.ssl);
}
}
session.lastAccessedTime = System.currentTimeMillis();
- version = SSL.getVersion(ssl);
+ version = SSL.getVersion(state.ssl);
handshakeFinished = true;
return SSLEngineResult.HandshakeStatus.FINISHED;
}
@@ -1088,7 +1088,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
// Check if we are in the shutdown phase
if (engineClosed) {
// Waiting to send the close_notify message
- if (SSL.pendingWrittenBytesInBIO(networkBIO) != 0) {
+ if (SSL.pendingWrittenBytesInBIO(state.networkBIO) != 0) {
return SSLEngineResult.HandshakeStatus.NEED_WRAP;
}
@@ -1142,13 +1142,13 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
}
switch (mode) {
case NONE:
- SSL.setVerify(ssl, SSL.SSL_CVERIFY_NONE, certificateVerificationDepth);
+ SSL.setVerify(state.ssl, SSL.SSL_CVERIFY_NONE, certificateVerificationDepth);
break;
case REQUIRE:
- SSL.setVerify(ssl, SSL.SSL_CVERIFY_REQUIRE, certificateVerificationDepth);
+ SSL.setVerify(state.ssl, SSL.SSL_CVERIFY_REQUIRE, certificateVerificationDepth);
break;
case OPTIONAL:
- SSL.setVerify(ssl,
+ SSL.setVerify(state.ssl,
certificateVerificationOptionalNoCA ? SSL.SSL_CVERIFY_OPTIONAL_NO_CA : SSL.SSL_CVERIFY_OPTIONAL,
certificateVerificationDepth);
break;
@@ -1170,12 +1170,6 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
return true;
}
- @Override
- protected void finalize() throws Throwable {
- super.finalize();
- // Call shutdown as the user may have created the OpenSslEngine and not used it at all.
- shutdown();
- }
private class OpenSSLSession implements SSLSession {
@@ -1190,7 +1184,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
byte[] id = null;
synchronized (OpenSSLEngine.this) {
if (!destroyed) {
- id = SSL.getSessionId(ssl);
+ id = SSL.getSessionId(state.ssl);
}
}
@@ -1208,7 +1202,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
long creationTime = 0;
synchronized (OpenSSLEngine.this) {
if (!destroyed) {
- creationTime = SSL.getTime(ssl);
+ creationTime = SSL.getTime(state.ssl);
}
}
return creationTime * 1000L;
@@ -1296,16 +1290,16 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
byte[] clientCert;
byte[][] chain;
synchronized (OpenSSLEngine.this) {
- if (destroyed || SSL.isInInit(ssl) != 0) {
+ if (destroyed || SSL.isInInit(state.ssl) != 0) {
throw new SSLPeerUnverifiedException(sm.getString("engine.unverifiedPeer"));
}
- chain = SSL.getPeerCertChain(ssl);
+ chain = SSL.getPeerCertChain(state.ssl);
if (!clientMode) {
// if used on the server side SSL_get_peer_cert_chain(...) will not include the remote peer certificate.
// We use SSL_get_peer_certificate to get it in this case and add it to our array later.
//
// See https://www.openssl.org/docs/ssl/SSL_get_peer_cert_chain.html
- clientCert = SSL.getPeerCertificate(ssl);
+ clientCert = SSL.getPeerCertificate(state.ssl);
} else {
clientCert = null;
}
@@ -1353,10 +1347,10 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (c == null) {
byte[][] chain;
synchronized (OpenSSLEngine.this) {
- if (destroyed || SSL.isInInit(ssl) != 0) {
+ if (destroyed || SSL.isInInit(state.ssl) != 0) {
throw new SSLPeerUnverifiedException(sm.getString("engine.unverifiedPeer"));
}
- chain = SSL.getPeerCertChain(ssl);
+ chain = SSL.getPeerCertChain(state.ssl);
}
if (chain == null) {
throw new SSLPeerUnverifiedException(sm.getString("engine.unverifiedPeer"));
@@ -1408,7 +1402,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (destroyed) {
return INVALID_CIPHER;
}
- ciphers = SSL.getCipherForSSL(ssl);
+ ciphers = SSL.getCipherForSSL(state.ssl);
}
String c = OpenSSLCipherConfigurationParser.openSSLToJsse(ciphers);
if (c != null) {
@@ -1424,7 +1418,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
if (applicationProtocol == null) {
synchronized (OpenSSLEngine.this) {
if (!destroyed) {
- applicationProtocol = SSL.getNextProtoNegotiated(ssl);
+ applicationProtocol = SSL.getNextProtoNegotiated(state.ssl);
}
}
if (applicationProtocol == null) {
@@ -1439,7 +1433,7 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
String version = null;
synchronized (OpenSSLEngine.this) {
if (!destroyed) {
- version = SSL.getVersion(ssl);
+ version = SSL.getVersion(state.ssl);
}
}
if (applicationProtocol.isEmpty()) {
@@ -1473,4 +1467,25 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn
}
+
+ private static class OpenSSLState implements Runnable {
+
+ private final long ssl;
+ private final long networkBIO;
+
+ private OpenSSLState(long ssl, long networkBIO) {
+ this.ssl = ssl;
+ this.networkBIO = networkBIO;
+ }
+
+ @Override
+ public void run() {
+ if (networkBIO != 0) {
+ SSL.freeBIO(networkBIO);
+ }
+ if (ssl != 0) {
+ SSL.freeSSL(ssl);
+ }
+ }
+ }
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ab56c02..ba8a5c4 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -154,6 +154,10 @@
APR/Native library that are not used by the OpenSSL integration for the
NIO and NIO2 connectors. (markt)
</scode>
+ <scode>
+ Refactor the JSSE/OpenSSL integration to avoid the use of
+ <code>finalize()</code>. (markt)
+ </scode>
</changelog>
</subsection>
<subsection name="WebSocket">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org