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 aa50220a06 Optimize state for contexts aa50220a06 is described below commit aa50220a06b4548b31ea185ffbd3289988adf4b2 Author: remm <r...@apache.org> AuthorDate: Thu Jan 11 16:17:37 2024 +0100 Optimize state for contexts This technique is likely not useful for the engine though. At best they need holders and hassle for everything, and the map is still needed for one callback. --- .../util/net/openssl/panama/OpenSSLContext.java | 55 +++++++++------------- webapps/docs/changelog.xml | 4 ++ 2 files changed, 26 insertions(+), 33 deletions(-) diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java index c79726fc76..2a3dfec8dd 100644 --- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java +++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java @@ -36,7 +36,6 @@ import java.util.Arrays; import java.util.Base64; import java.util.Iterator; import java.util.List; -import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import javax.net.ssl.KeyManager; @@ -128,17 +127,14 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { private final boolean alpn; private final int minTlsVersion; private final int maxTlsVersion; + private final List<byte[]> negotiableProtocols; private OpenSSLSessionContext sessionContext; private String enabledProtocol; private boolean initialized = false; private boolean noOcspCheck = false; - - private static final ConcurrentHashMap<Long, ContextState> states = new ConcurrentHashMap<>(); - private static ContextState getState(MemorySegment ctx) { - return states.get(Long.valueOf(ctx.address())); - } + private X509TrustManager x509TrustManager; private final ContextState state; private final Arena contextArena; @@ -292,7 +288,8 @@ 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); + this.negotiableProtocols = negotiableProtocolsBytes; + state = new ContextState(sslCtx, confCtx); /* * When an SSLHostConfig is replaced at runtime, it is not possible to * call destroy() on the associated OpenSSLContext since it is likely @@ -546,15 +543,15 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { // Trust and certificate verification if (tms != null) { // Client certificate verification based on custom trust managers - state.x509TrustManager = chooseTrustManager(tms); + x509TrustManager = chooseTrustManager(tms); SSL_CTX_set_cert_verify_callback(state.sslCtx, - SSL_CTX_set_cert_verify_callback$cb.allocate(new CertVerifyCallback(), contextArena), state.sslCtx); + SSL_CTX_set_cert_verify_callback$cb.allocate(new CertVerifyCallback(x509TrustManager), contextArena), state.sslCtx); // Pass along the DER encoded certificates of the accepted client // certificate issuers, so that their subjects can be presented // by the server during the handshake to allow the client choosing // an acceptable certificate - for (X509Certificate caCert : state.x509TrustManager.getAcceptedIssuers()) { + for (X509Certificate caCert : x509TrustManager.getAcceptedIssuers()) { var rawCACertificate = localArena.allocateFrom(ValueLayout.JAVA_BYTE, caCert.getEncoded()); var rawCACertificatePointer = localArena.allocateFrom(ValueLayout.ADDRESS, rawCACertificate); var x509CACert = d2i_X509(MemorySegment.NULL, rawCACertificatePointer, rawCACertificate.byteSize()); @@ -596,9 +593,9 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { } } - if (state.negotiableProtocols != null && state.negotiableProtocols.size() > 0) { + if (negotiableProtocols != null && negotiableProtocols.size() > 0) { SSL_CTX_set_alpn_select_cb(state.sslCtx, - SSL_CTX_set_alpn_select_cb$cb.allocate(new ALPNSelectCallback(), contextArena), state.sslCtx); + SSL_CTX_set_alpn_select_cb$cb.allocate(new ALPNSelectCallback(negotiableProtocols), contextArena), state.sslCtx); } // Apply OpenSSLConfCmd if used @@ -710,18 +707,17 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { // int SSL_callback_alpn_select_proto(SSL* ssl, const unsigned char **out, unsigned char *outlen, // const unsigned char *in, unsigned int inlen, void *arg) private static class ALPNSelectCallback implements SSL_CTX_set_alpn_select_cb$cb { + private final List<byte[]> negotiableProtocols; + ALPNSelectCallback(List<byte[]> negotiableProtocols) { + this.negotiableProtocols = negotiableProtocols; + } @Override public int apply(MemorySegment ssl, MemorySegment out, MemorySegment outlen, MemorySegment in, int inlen, MemorySegment arg) { - ContextState state = getState(arg); - if (state == null) { - log.warn(sm.getString("context.noSSL", Long.valueOf(arg.address()))); - return SSL_TLSEXT_ERR_NOACK(); - } try (var localArena = Arena.ofConfined()) { MemorySegment inSeg = in.reinterpret(inlen, localArena, null); byte[] advertisedBytes = inSeg.toArray(ValueLayout.JAVA_BYTE); - for (byte[] negotiableProtocolBytes : state.negotiableProtocols) { + for (byte[] negotiableProtocolBytes : negotiableProtocols) { for (int i = 0; i <= advertisedBytes.length - negotiableProtocolBytes.length; i++) { if (advertisedBytes[i] == negotiableProtocolBytes[0]) { for (int j = 0; j < negotiableProtocolBytes.length; j++) { @@ -748,6 +744,10 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { private static class CertVerifyCallback implements SSL_CTX_set_cert_verify_callback$cb { + private final X509TrustManager x509TrustManager; + CertVerifyCallback(X509TrustManager x509TrustManager) { + this.x509TrustManager = x509TrustManager; + } @Override public int apply(MemorySegment /*X509_STORE_CTX*/ x509_ctx, MemorySegment param) { if (log.isDebugEnabled()) { @@ -756,11 +756,6 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { if (MemorySegment.NULL.equals(param)) { return 0; } - ContextState state = getState(param); - if (state == null) { - log.warn(sm.getString("context.noSSL", Long.valueOf(param.address()))); - return 0; - } MemorySegment ssl = X509_STORE_CTX_get_ex_data(x509_ctx, SSL_get_ex_data_X509_STORE_CTX_idx()); MemorySegment /*STACK_OF(X509)*/ sk = X509_STORE_CTX_get0_untrusted(x509_ctx); int len = OPENSSL_sk_num(sk); @@ -783,7 +778,7 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { : getCipherAuthenticationMethod(SSL_CIPHER_get_auth_nid(cipher), SSL_CIPHER_get_kx_nid(cipher)); X509Certificate[] peerCerts = certificates(certificateChain); try { - state.x509TrustManager.checkClientTrusted(peerCerts, authMethod); + x509TrustManager.checkClientTrusted(peerCerts, authMethod); return 1; } catch (Exception e) { log.debug(sm.getString("openssl.certificateVerificationFailed"), e); @@ -1411,8 +1406,8 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { @Override public X509Certificate[] getAcceptedIssuers() { X509Certificate[] acceptedCerts = null; - if (state.x509TrustManager != null) { - acceptedCerts = state.x509TrustManager.getAcceptedIssuers(); + if (x509TrustManager != null) { + acceptedCerts = x509TrustManager.getAcceptedIssuers(); } return acceptedCerts; } @@ -1423,13 +1418,8 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { private final Arena stateArena = Arena.ofShared(); private final MemorySegment sslCtx; private final MemorySegment confCtx; - private final List<byte[]> negotiableProtocols; - - private X509TrustManager x509TrustManager = null; - private ContextState(MemorySegment sslCtx, MemorySegment confCtx, List<byte[]> negotiableProtocols) { - states.put(Long.valueOf(sslCtx.address()), this); - this.negotiableProtocols = negotiableProtocols; + private ContextState(MemorySegment sslCtx, MemorySegment confCtx) { // Use another arena to avoid keeping a reference through segments // This also allows making further accesses to the main pointers safer this.sslCtx = sslCtx.reinterpret(ValueLayout.ADDRESS.byteSize(), stateArena, @@ -1452,7 +1442,6 @@ public class OpenSSLContext implements org.apache.tomcat.util.net.SSLContext { @Override public void run() { - states.remove(Long.valueOf(sslCtx.address())); stateArena.close(); } } diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml index 5732a67b37..4149652677 100644 --- a/webapps/docs/changelog.xml +++ b/webapps/docs/changelog.xml @@ -122,6 +122,10 @@ Setting a <code>null</code> value for a cookie attribute should remove the attribute. (markt) </fix> + <fix> + Optimize state handling for OpenSSL context callbacks with the FFM API. + (remm) + </fix> </changelog> </subsection> </section> --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org