This is an automated email from the ASF dual-hosted git repository.
remm 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 714236a Switch again to shared scope for the context
714236a is described below
commit 714236af3458f030ec598660fa5fa886cd63dbd0
Author: remm <[email protected]>
AuthorDate: Mon Dec 20 17:22:58 2021 +0100
Switch again to shared scope for the context
Following further feedback from Panama.
The problem is the implicit scope isn't explicitly closed on JVM
shutdown at the moment. OTOH, this change (once everything is done
properly, there were simply too many places when convenient references
could be kept, preventing GC) to a shared scope works fine both with GC
(like the implicit scope does) and JVM shutdown (the explicit clean call
on shutdown does the cleanup there).
The engine should be able to keep its implicit scope for optimal safety
(GC will likely come soon enough for engines).
---
.../util/net/openssl/panama/OpenSSLContext.java | 67 +++++++++++++---------
webapps/docs/changelog.xml | 4 ++
2 files changed, 43 insertions(+), 28 deletions(-)
diff --git
a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index be94914..2755a0d 100644
---
a/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++
b/modules/openssl-java17/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -30,6 +30,8 @@ import java.io.File;
import java.lang.invoke.MethodHandle;
import java.lang.invoke.MethodHandles;
import java.lang.invoke.MethodType;
+import java.lang.ref.Cleaner;
+import java.lang.ref.Cleaner.Cleanable;
import java.nio.charset.StandardCharsets;
import java.security.PrivateKey;
import java.security.SecureRandom;
@@ -72,6 +74,8 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
private static final StringManager netSm =
StringManager.getManager(AbstractEndpoint.class);
private static final StringManager sm =
StringManager.getManager(OpenSSLContext.class);
+ private static final Cleaner cleaner = Cleaner.create();
+
private static final String defaultProtocol = "TLS";
private static final int SSL_AIDX_RSA = 0;
@@ -167,7 +171,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
private final ContextState state;
- private final ResourceScope contextScope;
+ private final Cleanable cleanable;
private static String[] getCiphers(MemoryAddress sslCtx) {
MemoryAddress sk = SSL_CTX_get_ciphers(sslCtx);
@@ -198,7 +202,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
this.sslHostConfig = certificate.getSSLHostConfig();
this.certificate = certificate;
- contextScope = ResourceScope.newImplicitScope();
+ ResourceScope contextScope = ResourceScope.newSharedScope();
MemoryAddress sslCtx = MemoryAddress.NULL;
MemoryAddress confCtx = MemoryAddress.NULL;
@@ -335,7 +339,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
} catch(Exception e) {
throw new SSLException(sm.getString("openssl.errorSSLCtxInit"), e);
} finally {
- state = new ContextState(sslCtx, confCtx,
negotiableProtocolsBytes);
+ state = new ContextState(contextScope, sslCtx, confCtx,
negotiableProtocolsBytes);
/*
* When an SSLHostConfig is replaced at runtime, it is not
possible to
* call destroy() on the associated OpenSSLContext since it is
likely
@@ -347,7 +351,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
* and the implicit scope will ensure that the associated native
* resources are cleaned up.
*/
- contextScope.addCloseAction(state);
+ cleanable = cleaner.register(this, state);
if (!success) {
destroy();
@@ -368,6 +372,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public void destroy() {
+ cleanable.clean();
}
@@ -554,7 +559,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
// List the ciphers that the client is permitted to negotiate
- if (SSL_CTX_set_cipher_list(state.sslCtx,
CLinker.toCString(sslHostConfig.getCiphers(), contextScope)) <= 0) {
+ if (SSL_CTX_set_cipher_list(state.sslCtx,
CLinker.toCString(sslHostConfig.getCiphers(), state.contextScope)) <= 0) {
log.warn(sm.getString("engine.failedCipherSuite",
sslHostConfig.getCiphers()));
}
@@ -590,18 +595,18 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
// Set int verify_callback(int preverify_ok, X509_STORE_CTX
*x509_ctx) callback
MemoryAddress openSSLCallbackVerify =
CLinker.getInstance().upcallStub(openSSLCallbackVerifyHandle,
- openSSLCallbackVerifyFunctionDescriptor, contextScope);
+ openSSLCallbackVerifyFunctionDescriptor,
state.contextScope);
// Leave this just in case but in Tomcat this is always set again
by the engine
SSL_CTX_set_verify(state.sslCtx, value, openSSLCallbackVerify);
// Trust and certificate verification
- var allocator = SegmentAllocator.ofScope(contextScope);
+ var allocator = SegmentAllocator.ofScope(state.contextScope);
if (tms != null) {
// Client certificate verification based on custom trust
managers
state.x509TrustManager = chooseTrustManager(tms);
MemoryAddress openSSLCallbackCertVerify =
CLinker.getInstance().upcallStub(openSSLCallbackCertVerifyHandle,
- openSSLCallbackCertVerifyFunctionDescriptor,
contextScope);
+ openSSLCallbackCertVerifyFunctionDescriptor,
state.contextScope);
SSL_CTX_set_cert_verify_callback(state.sslCtx,
openSSLCallbackCertVerify, state.sslCtx);
// Pass along the DER encoded certificates of the accepted
client
@@ -627,9 +632,9 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
//
SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificateFile()),
//
SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificatePath()));
MemorySegment caCertificateFileNative =
sslHostConfig.getCaCertificateFile() != null
- ?
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificateFile()),
contextScope) : null;
+ ?
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificateFile()),
state.contextScope) : null;
MemorySegment caCertificatePathNative =
sslHostConfig.getCaCertificatePath() != null
- ?
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificatePath()),
contextScope) : null;
+ ?
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCaCertificatePath()),
state.contextScope) : null;
if (SSL_CTX_load_verify_locations(state.sslCtx,
caCertificateFileNative == null ? MemoryAddress.NULL :
caCertificateFileNative,
caCertificatePathNative == null ?
MemoryAddress.NULL : caCertificatePathNative) <= 0) {
@@ -654,7 +659,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
// No CA certificates configured. Reject all client
certificates.
MemoryAddress openSSLCallbackCertVerify =
CLinker.getInstance().upcallStub(openSSLCallbackCertVerifyHandle,
- openSSLCallbackCertVerifyFunctionDescriptor,
contextScope);
+ openSSLCallbackCertVerifyFunctionDescriptor,
state.contextScope);
SSL_CTX_set_cert_verify_callback(state.sslCtx,
openSSLCallbackCertVerify, MemoryAddress.NULL);
}
@@ -663,7 +668,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
// MemoryAddress in, int inlen, MemoryAddress arg
MemoryAddress openSSLCallbackAlpnSelectProto =
CLinker.getInstance().upcallStub(openSSLCallbackAlpnSelectProtoHandle,
- openSSLCallbackAlpnSelectProtoFunctionDescriptor,
contextScope);
+ openSSLCallbackAlpnSelectProtoFunctionDescriptor,
state.contextScope);
SSL_CTX_set_alpn_select_cb(state.sslCtx,
openSSLCallbackAlpnSelectProto, state.sslCtx);
// Skip NPN (annoying and likely not useful anymore)
//SSLContext.setNpnProtos(state.ctx, protocolsArray,
SSL.SSL_SELECTOR_FAILURE_NO_ADVERTISE);
@@ -965,7 +970,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
private void addCertificate(SSLHostConfigCertificate certificate) throws
Exception {
- var allocator = SegmentAllocator.ofScope(contextScope);
+ var allocator = SegmentAllocator.ofScope(state.contextScope);
int index = getCertificateIndex(certificate);
// Load Server key and certificate
if (certificate.getCertificateFile() != null) {
@@ -974,9 +979,9 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
//
SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()),
//
SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()),
// certificate.getCertificateKeyPassword(),
getCertificateIndex(certificate));
- var certificateFileNative =
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()),
contextScope);
+ var certificateFileNative =
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateFile()),
state.contextScope);
var certificateKeyFileNative =
(certificate.getCertificateKeyFile() == null) ? certificateFileNative
- :
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()),
contextScope);
+ :
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateKeyFile()),
state.contextScope);
MemoryAddress bio;
MemoryAddress cert = MemoryAddress.NULL;
MemoryAddress key = MemoryAddress.NULL;
@@ -1000,7 +1005,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
int passwordLength = 0;
String callbackPassword =
certificate.getCertificateKeyPassword();
if (callbackPassword != null && callbackPassword.length() > 0)
{
- MemorySegment password =
CLinker.toCString(callbackPassword, contextScope);
+ MemorySegment password =
CLinker.toCString(callbackPassword, state.contextScope);
passwordAddress = password.address();
passwordLength = (int) (password.byteSize() - 1);
}
@@ -1104,7 +1109,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
// Try to read DH parameters from the (first) SSLCertificateFile
if (index == SSL_AIDX_RSA) {
- bio = BIO_new_file(certificateFileNative,
CLinker.toCString("r", contextScope));
+ bio = BIO_new_file(certificateFileNative,
CLinker.toCString("r", state.contextScope));
var dh = PEM_read_bio_DHparams(bio, MemoryAddress.NULL,
MemoryAddress.NULL, MemoryAddress.NULL);
BIO_free(bio);
// # define SSL_CTX_set_tmp_dh(sslCtx,dh) \
@@ -1115,7 +1120,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
}
// Similarly, try to read the ECDH curve name from
SSLCertificateFile...
- bio = BIO_new_file(certificateFileNative, CLinker.toCString("r",
contextScope));
+ bio = BIO_new_file(certificateFileNative, CLinker.toCString("r",
state.contextScope));
var ecparams = PEM_read_bio_ECPKParameters(bio,
MemoryAddress.NULL, MemoryAddress.NULL, MemoryAddress.NULL);
BIO_free(bio);
if (!MemoryAddress.NULL.equals(ecparams)) {
@@ -1129,12 +1134,12 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
// Set callback for DH parameters
MemoryAddress openSSLCallbackTmpDH =
CLinker.getInstance().upcallStub(openSSLCallbackTmpDHHandle,
- openSSLCallbackTmpDHFunctionDescriptor, contextScope);
+ openSSLCallbackTmpDHFunctionDescriptor,
state.contextScope);
SSL_CTX_set_tmp_dh_callback(state.sslCtx, openSSLCallbackTmpDH);
// Set certificate chain file
if (certificate.getCertificateChainFile() != null) {
var certificateChainFileNative =
-
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()),
contextScope);
+
CLinker.toCString(SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()),
state.contextScope);
// SSLContext.setCertificateChainFile(state.ctx,
//
SSLHostConfig.adjustRelativePath(certificate.getCertificateChainFile()), false);
if (SSL_CTX_use_certificate_chain_file(state.sslCtx,
certificateChainFileNative) <= 0) {
@@ -1151,7 +1156,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
if (sslHostConfig.getCertificateRevocationListFile() != null) {
MemoryAddress x509Lookup =
X509_STORE_add_lookup(certificateStore, X509_LOOKUP_file());
var certificateRevocationListFileNative =
-
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCertificateRevocationListFile()),
contextScope);
+
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCertificateRevocationListFile()),
state.contextScope);
//X509_LOOKUP_ctrl(lookup,X509_L_FILE_LOAD,file,type,NULL)
if (X509_LOOKUP_ctrl(x509Lookup, X509_L_FILE_LOAD(),
certificateRevocationListFileNative,
X509_FILETYPE_PEM(), MemoryAddress.NULL) <= 0) {
@@ -1161,7 +1166,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
if (sslHostConfig.getCertificateRevocationListPath() != null) {
MemoryAddress x509Lookup =
X509_STORE_add_lookup(certificateStore, X509_LOOKUP_hash_dir());
var certificateRevocationListPathNative =
-
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCertificateRevocationListPath()),
contextScope);
+
CLinker.toCString(SSLHostConfig.adjustRelativePath(sslHostConfig.getCertificateRevocationListPath()),
state.contextScope);
//X509_LOOKUP_ctrl(lookup,X509_L_ADD_DIR,path,type,NULL)
if (X509_LOOKUP_ctrl(x509Lookup, X509_L_ADD_DIR(),
certificateRevocationListPathNative,
X509_FILETYPE_PEM(), MemoryAddress.NULL) <= 0) {
@@ -1217,7 +1222,7 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
}
// Set callback for DH parameters
MemoryAddress openSSLCallbackTmpDH =
CLinker.getInstance().upcallStub(openSSLCallbackTmpDHHandle,
- openSSLCallbackTmpDHFunctionDescriptor, contextScope);
+ openSSLCallbackTmpDHFunctionDescriptor,
state.contextScope);
SSL_CTX_set_tmp_dh_callback(state.sslCtx, openSSLCallbackTmpDH);
for (int i = 1; i < chain.length; i++) {
//SSLContext.addChainCertificateRaw(state.ctx,
chain[i].getEncoded());
@@ -1362,14 +1367,16 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
private static class ContextState implements Runnable {
+ private final ResourceScope contextScope;
private final MemoryAddress sslCtx;
private final MemoryAddress confCtx;
private final List<byte[]> negotiableProtocols;
private X509TrustManager x509TrustManager = null;
- private ContextState(MemoryAddress sslCtx, MemoryAddress confCtx,
List<byte[]> negotiableProtocols) {
+ private ContextState(ResourceScope contextScope, MemoryAddress sslCtx,
MemoryAddress confCtx, List<byte[]> negotiableProtocols) {
states.put(Long.valueOf(sslCtx.toRawLongValue()), this);
+ this.contextScope = contextScope;
this.sslCtx = sslCtx;
this.confCtx = confCtx;
this.negotiableProtocols = negotiableProtocols;
@@ -1377,10 +1384,14 @@ public class OpenSSLContext implements
org.apache.tomcat.util.net.SSLContext {
@Override
public void run() {
- states.remove(Long.valueOf(sslCtx.toRawLongValue()));
- SSL_CTX_free(sslCtx);
- if (!MemoryAddress.NULL.equals(confCtx)) {
- SSL_CONF_CTX_free(confCtx);
+ try {
+ states.remove(Long.valueOf(sslCtx.toRawLongValue()));
+ SSL_CTX_free(sslCtx);
+ if (!MemoryAddress.NULL.equals(confCtx)) {
+ SSL_CONF_CTX_free(confCtx);
+ }
+ } finally {
+ contextScope.close();
}
}
}
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 9e3f380..4153ffd 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -129,6 +129,10 @@
Revert the previous fix for <bug>65714</bug> and implement a more
comprehensive fix. (markt)
</fix>
+ <fix>
+ Allow freeing up context on JVM shutdown in the OpenSSL Panama module
+ by properly using a shared scope. (remm)
+ </fix>
</changelog>
</subsection>
<subsection name="Jasper">
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
