This is an automated email from the ASF dual-hosted git repository.

markt-asf pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 020347b626 Length validation for ALPN name. More robust certificate 
verification.
020347b626 is described below

commit 020347b626b5bff5b4dc3ba93e3ece8d2a2288cd
Author: Mark Thomas <[email protected]>
AuthorDate: Wed Sep 2 17:51:28 2026 +0100

    Length validation for ALPN name. More robust certificate verification.
---
 .../apache/tomcat/util/net/AbstractEndpoint.java   |  4 +++
 .../apache/tomcat/util/net/LocalStrings.properties |  1 +
 .../net/openssl/panama/LocalStrings.properties     |  1 +
 .../util/net/openssl/panama/OpenSSLContext.java    | 37 ++++++++++++++--------
 .../util/net/openssl/panama/OpenSSLEngine.java     |  7 ++--
 .../util/net/openssl/panama/OpenSSLLibrary.java    | 18 +++++++----
 webapps/docs/changelog.xml                         |  6 ++++
 7 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java 
b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 925744c8ef..c475b03c94 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -22,6 +22,7 @@ import java.net.InetAddress;
 import java.net.InetSocketAddress;
 import java.net.NetworkInterface;
 import java.net.SocketException;
+import java.nio.charset.StandardCharsets;
 import java.security.MessageDigest;
 import java.security.NoSuchAlgorithmException;
 import java.security.cert.CertificateEncodingException;
@@ -1544,6 +1545,9 @@ public abstract class AbstractEndpoint<S, U> {
      * @param negotiableProtocol The protocol to add
      */
     public void addNegotiatedProtocol(String negotiableProtocol) {
+        if (negotiableProtocol.getBytes(StandardCharsets.UTF_8).length > 255) {
+            throw new 
IllegalArgumentException(sm.getString("abstractEndpoint.alpn.tooLong", 
negotiableProtocol));
+        }
         negotiableProtocols.add(negotiableProtocol);
     }
 
diff --git a/java/org/apache/tomcat/util/net/LocalStrings.properties 
b/java/org/apache/tomcat/util/net/LocalStrings.properties
index cf6cc07acc..f6459c4d9d 100644
--- a/java/org/apache/tomcat/util/net/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/LocalStrings.properties
@@ -51,6 +51,7 @@ channel.nio.ssl.wrapFail=Unable to wrap data, invalid status 
[{0}]
 
 endpoint.accept.fail=Socket accept failed
 endpoint.alpn.fail=Failed to configure endpoint for ALPN using [{0}]
+endpoint.alpn.tooLong=The negotiated protocol name [{0}] is too long (more 
than 255 bytes)
 endpoint.alpn.negotiated=Negotiated [{0}] protocol using ALPN
 endpoint.apr.applyConf=Applying OpenSSLConfCmd to SSL context
 endpoint.apr.checkConf=Checking OpenSSLConf
diff --git 
a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties 
b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
index 4034c3659f..bd73d69ba2 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
@@ -46,6 +46,7 @@ engine.unverifiedPeer=Peer unverified
 
 openssl.X509FactoryError=Error getting X509 factory instance
 openssl.addedClientCaCert=Added client CA cert: [{0}]
+openssl.alpn.tooLong=The ALPN protocol [{0}] is too long as it is more than 
255 bytes
 openssl.applyConf=Applying OpenSSLConfCmd to SSL context
 openssl.certificateVerificationFailed=Certificate verification failed
 openssl.checkConf=Checking OpenSSLConf
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 516baec515..976985472c 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -52,6 +52,7 @@ import static 
org.apache.tomcat.util.openssl.openssl_h_Compatibility.*;
 import static org.apache.tomcat.util.openssl.openssl_h_Macros.*;
 import org.apache.juli.logging.Log;
 import org.apache.juli.logging.LogFactory;
+import org.apache.tomcat.util.ExceptionUtils;
 import org.apache.tomcat.util.file.ConfigFileLoader;
 import org.apache.tomcat.util.file.ConfigurationSource.Resource;
 import org.apache.tomcat.util.net.Constants;
@@ -290,7 +291,11 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                 alpn = true;
                 negotiableProtocolsBytes = new 
ArrayList<>(negotiableProtocols.size() + 1);
                 for (String negotiableProtocol : negotiableProtocols) {
-                    
negotiableProtocolsBytes.add(negotiableProtocol.getBytes(StandardCharsets.ISO_8859_1));
+                    byte[] negotiableProtocolBytes = 
negotiableProtocol.getBytes(StandardCharsets.UTF_8);
+                    if (negotiableProtocolBytes.length > 255) {
+                        throw new 
IllegalArgumentException(sm.getString("openssl.alpn.tooLong", 
negotiableProtocol));
+                    }
+                    negotiableProtocolsBytes.add(negotiableProtocolBytes);
                 }
                 negotiableProtocolsBytes.add(HTTP_11_PROTOCOL);
             } else {
@@ -803,24 +808,28 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
             if (MemorySegment.NULL.equals(param)) {
                 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_h_Compatibility.OPENSSL_sk_num(sk);
-            byte[][] certificateChain = new byte[len][];
             try (var localArena = Arena.ofConfined()) {
-                OpenSSLLibrary.populateCertificateChain(localArena, sk, 
certificateChain);
+                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);
+                if (MemorySegment.NULL.equals(sk)) {
+                    return 0;
+                }
+                int len = openssl_h_Compatibility.OPENSSL_sk_num(sk);
+                byte[][] certificateChain = new byte[len][];
+                if (!OpenSSLLibrary.populateCertificateChain(localArena, sk, 
certificateChain)) {
+                    return 0;
+                }
                 MemorySegment cipher = SSL_get_current_cipher(ssl);
                 String authMethod = (MemorySegment.NULL.equals(cipher)) ? 
"UNKNOWN" :
                         
getCipherAuthenticationMethod(SSL_CIPHER_get_auth_nid(cipher), 
SSL_CIPHER_get_kx_nid(cipher));
                 X509Certificate[] peerCerts = certificates(certificateChain);
-                try {
-                    x509TrustManager.checkClientTrusted(peerCerts, authMethod);
-                    OpenSSLEngine.markPostHandshakeAuthComplete(ssl);
-                    return 1;
-                } catch (Exception e) {
-                    if (log.isDebugEnabled()) {
-                        
log.debug(sm.getString("openssl.certificateVerificationFailed"), e);
-                    }
+                x509TrustManager.checkClientTrusted(peerCerts, authMethod);
+                OpenSSLEngine.markPostHandshakeAuthComplete(ssl);
+                return 1;
+            } catch (Throwable t) {
+                ExceptionUtils.handleThrowable(t);
+                if (log.isDebugEnabled()) {
+                    
log.debug(sm.getString("openssl.certificateVerificationFailed"), t);
                 }
             }
             return 0;
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java 
b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index 83acba6166..5b969886ea 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -31,6 +31,7 @@ import java.net.URISyntaxException;
 import java.net.URL;
 import java.nio.ByteBuffer;
 import java.nio.ReadOnlyBufferException;
+import java.nio.charset.StandardCharsets;
 import java.security.Principal;
 import java.security.cert.Certificate;
 import java.util.ArrayList;
@@ -860,7 +861,9 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
         }
         byte[][] certificateChain = new byte[len][];
         try (var localArena = Arena.ofConfined()) {
-            OpenSSLLibrary.populateCertificateChain(localArena, sk, 
certificateChain);
+            if (!OpenSSLLibrary.populateCertificateChain(localArena, sk, 
certificateChain)) {
+                return null;
+            }
             return certificateChain;
         }
     }
@@ -882,7 +885,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
             if (log.isTraceEnabled()) {
                 log.trace("Protocol negotiated [" + new String(name) + "]");
             }
-            return new String(name);
+            return new String(name, StandardCharsets.UTF_8);
         }
     }
 
diff --git a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java 
b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
index acf9c9566f..b8691992d2 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
@@ -517,19 +517,25 @@ public class OpenSSLLibrary {
         return sslError;
     }
 
-    static void populateCertificateChain(Arena localArena, MemorySegment /* 
STACK_OF(X509) */ sk,
+    static boolean populateCertificateChain(Arena localArena, MemorySegment /* 
STACK_OF(X509) */ sk,
             byte[][] certificateChain) {
         for (int i = 0; i < certificateChain.length; i++) {
             MemorySegment/* (X509*) */ x509 = 
openssl_h_Compatibility.OPENSSL_sk_value(sk, i);
             MemorySegment bufPointer = 
localArena.allocateFrom(ValueLayout.ADDRESS, MemorySegment.NULL);
             int length = i2d_X509(x509, bufPointer);
+            MemorySegment buf = bufPointer.get(ValueLayout.ADDRESS, 0);
             if (length <= 0) {
-                certificateChain[i] = new byte[0];
-                continue;
+                if (!MemorySegment.NULL.equals(buf)) {
+                    OPENSSL_free(buf);
+                }
+                return false;
+            }
+            try {
+                certificateChain[i] = buf.reinterpret(length, localArena, 
null).toArray(ValueLayout.JAVA_BYTE);
+            } finally {
+                OPENSSL_free(buf);
             }
-            MemorySegment buf = bufPointer.get(ValueLayout.ADDRESS, 0);
-            certificateChain[i] = buf.reinterpret(length, localArena, 
null).toArray(ValueLayout.JAVA_BYTE);
-            OPENSSL_free(buf);
         }
+        return true;
     }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index b55d17d692..e7bc27c5e5 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -216,6 +216,12 @@
       <fix>
         Implement stricter ALPN matching for Connectors using FFM. (markt)
       </fix>
+      <add>
+        Add length validation for ALPN protocol names. (markt)
+      </add>
+      <fix>
+        Make FFM certificate verification more robust. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to