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

Reply via email to