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

remm pushed a commit to branch 10.1.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 625f3c166b46f00661e7a6ac4f0a95c485962ffe
Author: remm <r...@apache.org>
AuthorDate: Thu Dec 14 11:13:45 2023 +0100

    Sync cleanups
---
 .../util/net/openssl/panama/OpenSSLContext.java    |  54 +++----
 .../util/net/openssl/panama/OpenSSLEngine.java     | 166 ++++++++++-----------
 2 files changed, 107 insertions(+), 113 deletions(-)

diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
index ba0c1b2d39..12da6c00c7 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -99,6 +99,8 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     public static final int SSL_PROTOCOL_ALL = (SSL_PROTOCOL_TLSV1 | 
SSL_PROTOCOL_TLSV1_1 | SSL_PROTOCOL_TLSV1_2 |
             SSL_PROTOCOL_TLSV1_3);
 
+    static final int OPTIONAL_NO_CA = 3;
+
     private static final String BEGIN_KEY = "-----BEGIN PRIVATE KEY-----\n";
     private static final Object END_KEY = "\n-----END PRIVATE KEY-----";
 
@@ -135,8 +137,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     private final MemorySegment openSSLCallbackPassword;
 
     private static final ConcurrentHashMap<Long, ContextState> states = new 
ConcurrentHashMap<>();
-
-    static ContextState getState(MemorySegment ctx) {
+    private static ContextState getState(MemorySegment ctx) {
         return states.get(Long.valueOf(ctx.address()));
     }
 
@@ -476,8 +477,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
         return result;
     }
 
-    private static final int OPTIONAL_NO_CA = 3;
-
     /**
      * Setup the SSL_CTX.
      *
@@ -563,9 +562,8 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
             }
 
             // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
-            // Leave this just in case but in Tomcat this is always set again 
by the engine
             SSL_CTX_set_verify(state.sslCtx, value,
-                    SSL_CTX_set_verify$callback.allocate(new VerifyCallback(), 
contextArena));
+                    SSL_CTX_set_verify$callback.allocate(new 
OpenSSLEngine.VerifyCallback(), contextArena));
 
             // Trust and certificate verification
             if (tms != null) {
@@ -709,8 +707,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     // DH *(*tmp_dh_callback)(SSL *ssl, int is_export, int keylength)
     private static class TmpDHCallback implements 
SSL_CTX_set_tmp_dh_callback$dh {
         @Override
-        public MemorySegment apply(MemorySegment ssl, 
@SuppressWarnings("unused") int isExport,
-                @SuppressWarnings("unused") int keylength) {
+        public MemorySegment apply(MemorySegment ssl, int isExport, int 
keylength) {
             var pkey = SSL_get_privatekey(ssl);
             int type = (MemorySegment.NULL.equals(pkey)) ? EVP_PKEY_NONE() : 
EVP_PKEY_base_id(pkey);
             /*
@@ -742,7 +739,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     //        const unsigned char *in, unsigned int inlen, void *arg)
     private static class ALPNSelectCallback implements 
SSL_CTX_set_alpn_select_cb$cb {
         @Override
-        public int apply(@SuppressWarnings("unused") MemorySegment ssl, 
MemorySegment out,
+        public int apply(MemorySegment ssl, MemorySegment out,
                 MemorySegment outlen, MemorySegment in, int inlen, 
MemorySegment arg) {
             ContextState state = getState(arg);
             if (state == null) {
@@ -778,14 +775,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     }
 
 
-    private static class VerifyCallback implements SSL_CTX_set_verify$callback 
{
-        @Override
-        public int apply(int preverify_ok, MemorySegment /*X509_STORE_CTX*/ 
x509ctx) {
-            return OpenSSLEngine.openSSLCallbackVerify(preverify_ok, x509ctx);
-        }
-    }
-
-
     private static class CertVerifyCallback implements 
SSL_CTX_set_cert_verify_callback$cb {
         @Override
         public int apply(MemorySegment /*X509_STORE_CTX*/ x509_ctx, 
MemorySegment param) {
@@ -919,8 +908,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 
     private static class PasswordCallback implements 
SSL_CTX_set_default_passwd_cb$cb {
         @Override
-        public int apply(MemorySegment /*char **/ buf, int bufsiz,
-                @SuppressWarnings("unused") int verify, 
@SuppressWarnings("unused") MemorySegment /*void **/ cb) {
+        public int apply(MemorySegment /* char **/ buf, int bufsiz, int 
verify, MemorySegment /* void **/ cb) {
             if (log.isDebugEnabled()) {
                 log.debug("Return password for certificate");
             }
@@ -943,7 +931,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
     }
 
 
-    @SuppressWarnings("deprecation")
     private boolean addCertificate(SSLHostConfigCertificate certificate, Arena 
localArena) throws Exception {
         int index = getCertificateIndex(certificate);
         // Load Server key and certificate
@@ -1376,10 +1363,26 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 
 
     private static void logLastError(SegmentAllocator allocator, String 
string) {
-        var buf = allocator.allocate(ValueLayout.JAVA_BYTE, 128);
-        ERR_error_string(ERR_get_error(), buf);
-        String err = buf.getString(0);
-        log.error(sm.getString(string, err));
+        String sslError = null;
+        long error = ERR_get_error();
+        if (error != SSL_ERROR_NONE()) {
+            do {
+                // Loop until getLastErrorNumber() returns SSL_ERROR_NONE
+                var buf = allocator.allocate(ValueLayout.JAVA_BYTE, 128);
+                ERR_error_string(error, buf);
+                String err = buf.getString(0);
+                if (sslError == null) {
+                    sslError = err;
+                }
+                if (log.isDebugEnabled()) {
+                    log.debug(sm.getString("engine.openSSLError", 
Long.toString(error), err));
+                }
+            } while ((error = ERR_get_error()) != SSL_ERROR_NONE());
+        }
+        if (sslError == null) {
+            sslError = "";
+        }
+        log.error(sm.getString(string, sslError));
     }
 
 
@@ -1461,8 +1464,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                             @Override
                             public void accept(MemorySegment t) {
                                 SSL_CONF_CTX_free(t);
-                            }
-                });
+                            }});
             } else {
                 this.confCtx = null;
             }
diff --git 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
index 9bbd9881e9..9f6593cc42 100644
--- 
a/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ 
b/modules/openssl-foreign/src/main/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -62,6 +62,7 @@ import org.apache.tomcat.util.buf.Asn1Parser;
 import org.apache.tomcat.util.net.Constants;
 import org.apache.tomcat.util.net.SSLUtil;
 import 
org.apache.tomcat.util.net.openssl.ciphers.OpenSSLCipherConfigurationParser;
+import org.apache.tomcat.util.openssl.SSL_CTX_set_verify$callback;
 import org.apache.tomcat.util.openssl.SSL_set_info_callback$cb;
 import org.apache.tomcat.util.openssl.SSL_set_verify$callback;
 import org.apache.tomcat.util.res.StringManager;
@@ -101,15 +102,10 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
     private static final int MAX_COMPRESSED_LENGTH = MAX_PLAINTEXT_LENGTH + 
1024;
     private static final int MAX_CIPHERTEXT_LENGTH = MAX_COMPRESSED_LENGTH + 
1024;
 
-    // Protocols
-    static final int VERIFY_DEPTH = 10;
-
     // Header (5) + Data (2^14) + Compression (1024) + Encryption (1024) + MAC 
(20) + Padding (256)
-    static final int MAX_ENCRYPTED_PACKET_LENGTH = MAX_CIPHERTEXT_LENGTH + 5 + 
20 + 256;
-
-    static final int MAX_ENCRYPTION_OVERHEAD_LENGTH = 
MAX_ENCRYPTED_PACKET_LENGTH - MAX_PLAINTEXT_LENGTH;
+    private static final int MAX_ENCRYPTED_PACKET_LENGTH = 
MAX_CIPHERTEXT_LENGTH + 5 + 20 + 256;
 
-    enum ClientAuthMode {
+    private enum ClientAuthMode {
         NONE,
         OPTIONAL,
         REQUIRE,
@@ -924,7 +920,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
         }
     }
 
-    private synchronized void renegotiate() throws SSLException {
+    private void renegotiate() throws SSLException {
         if (log.isDebugEnabled()) {
             log.debug("Start renegotiate");
         }
@@ -964,7 +960,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
     /**
      * Clear out any errors, but log a warning.
      */
-    private void clearLastError() {
+    private static void clearLastError() {
         getLastError();
     }
 
@@ -977,7 +973,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
      * zero result.
      * @return the first error in the stack
      */
-    private String getLastError() {
+    private static String getLastError() {
         String sslError = null;
         long error = ERR_get_error();
         if (error != SSL_ERROR_NONE()) {
@@ -1118,8 +1114,6 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
         return clientAuth == ClientAuthMode.OPTIONAL;
     }
 
-    private static final int OPTIONAL_NO_CA = 3;
-
     private void setClientAuth(ClientAuthMode mode) {
         if (clientMode) {
             return;
@@ -1131,7 +1125,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
             state.certificateVerifyMode = switch (mode) {
                 case NONE -> SSL_VERIFY_NONE();
                 case REQUIRE -> SSL_VERIFY_FAIL_IF_NO_PEER_CERT();
-                case OPTIONAL -> certificateVerificationOptionalNoCA ? 
OPTIONAL_NO_CA : SSL_VERIFY_PEER();
+                case OPTIONAL -> certificateVerificationOptionalNoCA ? 
OpenSSLContext.OPTIONAL_NO_CA : SSL_VERIFY_PEER();
             };
             // SSL.setVerify(state.ssl, value, certificateVerificationDepth);
             // Set int verify_callback(int preverify_ok, X509_STORE_CTX 
*x509_ctx) callback
@@ -1140,6 +1134,8 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
                 case REQUIRE -> SSL_VERIFY_PEER() | 
SSL_VERIFY_FAIL_IF_NO_PEER_CERT();
                 case OPTIONAL -> SSL_VERIFY_PEER();
             };
+            // Note: Since a callback is always set by the context, the 
callback here could in theory
+            // be set to NULL (at the time of creation of the SSL, the SSL_CTX 
will have a non null callback)
             SSL_set_verify(state.ssl, value, 
SSL_set_verify$callback.allocate(new VerifyCallback(), engineArena));
             clientAuth = mode;
         }
@@ -1147,7 +1143,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
 
     private static class InfoCallback implements SSL_set_info_callback$cb {
         @Override
-        public void apply(MemorySegment ssl, int where, 
@SuppressWarnings("unused") int ret) {
+        public void apply(MemorySegment ssl, int where, int ret) {
             EngineState state = getState(ssl);
             if (state == null) {
                 log.warn(sm.getString("engine.noSSL", 
Long.valueOf(ssl.address())));
@@ -1159,94 +1155,90 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
         }
     }
 
-    private static class VerifyCallback implements SSL_set_verify$callback {
+    static class VerifyCallback implements SSL_set_verify$callback, 
SSL_CTX_set_verify$callback {
         @Override
         public int apply(int preverify_ok, MemorySegment /*X509_STORE_CTX*/ 
x509ctx) {
-            return openSSLCallbackVerify(preverify_ok, x509ctx);
-        }
-    }
-
-    public static int openSSLCallbackVerify(int preverify_ok, MemorySegment 
/*X509_STORE_CTX*/ x509ctx) {
-        MemorySegment ssl = X509_STORE_CTX_get_ex_data(x509ctx, 
SSL_get_ex_data_X509_STORE_CTX_idx());
-        EngineState state = getState(ssl);
-        if (state == null) {
-            log.warn(sm.getString("engine.noSSL", 
Long.valueOf(ssl.address())));
-            return 0;
-        }
-        if (log.isDebugEnabled()) {
-            log.debug("Verification in engine with mode [" + 
state.certificateVerifyMode + "] for " + state.ssl);
-        }
-        int ok = preverify_ok;
-        int errnum = X509_STORE_CTX_get_error(x509ctx);
-        int errdepth = X509_STORE_CTX_get_error_depth(x509ctx);
-        state.phaState = PHAState.COMPLETE;
-        if (state.certificateVerifyMode == -1 /*SSL_CVERIFY_UNSET*/ || 
state.certificateVerifyMode == SSL_VERIFY_NONE()) {
-            return 1;
-        }
-        /*SSL_VERIFY_ERROR_IS_OPTIONAL(errnum) -> ((errnum == 
X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT)
+            MemorySegment ssl = X509_STORE_CTX_get_ex_data(x509ctx, 
SSL_get_ex_data_X509_STORE_CTX_idx());
+            EngineState state = getState(ssl);
+            if (state == null) {
+                log.warn(sm.getString("engine.noSSL", 
Long.valueOf(ssl.address())));
+                return 0;
+            }
+            if (log.isDebugEnabled()) {
+                log.debug("Verification in engine with mode [" + 
state.certificateVerifyMode + "] for " + state.ssl);
+            }
+            int ok = preverify_ok;
+            int errnum = X509_STORE_CTX_get_error(x509ctx);
+            int errdepth = X509_STORE_CTX_get_error_depth(x509ctx);
+            state.phaState = PHAState.COMPLETE;
+            if (state.certificateVerifyMode == -1 /*SSL_CVERIFY_UNSET*/ || 
state.certificateVerifyMode == SSL_VERIFY_NONE()) {
+                return 1;
+            }
+            /*SSL_VERIFY_ERROR_IS_OPTIONAL(errnum) -> ((errnum == 
X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT)
                 || (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN)
                 || (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY)
                 || (errnum == X509_V_ERR_CERT_UNTRUSTED)
                 || (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE))*/
-        boolean verifyErrorIsOptional = (errnum == 
X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT())
-                || (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN())
-                || (errnum == X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY())
-                || (errnum == X509_V_ERR_CERT_UNTRUSTED())
-                || (errnum == X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE());
-        if (verifyErrorIsOptional && (state.certificateVerifyMode == 
OPTIONAL_NO_CA)) {
-            ok = 1;
-            SSL_set_verify_result(state.ssl, X509_V_OK());
-        }
-        /*
-         * Expired certificates vs. "expired" CRLs: by default, OpenSSL
-         * turns X509_V_ERR_CRL_HAS_EXPIRED into a "certificate_expired(45)"
-         * SSL alert, but that's not really the message we should convey to the
-         * peer (at the very least, it's confusing, and in many cases, it's 
also
-         * inaccurate, as the certificate itself may very well not have expired
-         * yet). We set the X509_STORE_CTX error to something which OpenSSL's
-         * s3_both.c:ssl_verify_alarm_type() maps to 
SSL_AD_CERTIFICATE_UNKNOWN,
-         * i.e. the peer will receive a "certificate_unknown(46)" alert.
-         * We do not touch errnum, though, so that later on we will still log
-         * the "real" error, as returned by OpenSSL.
-         */
-        if (ok == 0 && errnum == X509_V_ERR_CRL_HAS_EXPIRED()) {
-            X509_STORE_CTX_set_error(x509ctx, -1);
-        }
-
-        // OCSP
-        if (!state.noOcspCheck && (ok > 0)) {
-            /* If there was an optional verification error, it's not
-             * possible to perform OCSP validation since the issuer may be
-             * missing/untrusted.  Fail in that case.
+            boolean verifyErrorIsOptional = (errnum == 
X509_V_ERR_DEPTH_ZERO_SELF_SIGNED_CERT())
+                    || (errnum == X509_V_ERR_SELF_SIGNED_CERT_IN_CHAIN())
+                    || (errnum == 
X509_V_ERR_UNABLE_TO_GET_ISSUER_CERT_LOCALLY())
+                    || (errnum == X509_V_ERR_CERT_UNTRUSTED())
+                    || (errnum == 
X509_V_ERR_UNABLE_TO_VERIFY_LEAF_SIGNATURE());
+            if (verifyErrorIsOptional && (state.certificateVerifyMode == 
OpenSSLContext.OPTIONAL_NO_CA)) {
+                ok = 1;
+                SSL_set_verify_result(state.ssl, X509_V_OK());
+            }
+            /*
+             * Expired certificates vs. "expired" CRLs: by default, OpenSSL
+             * turns X509_V_ERR_CRL_HAS_EXPIRED into a 
"certificate_expired(45)"
+             * SSL alert, but that's not really the message we should convey 
to the
+             * peer (at the very least, it's confusing, and in many cases, 
it's also
+             * inaccurate, as the certificate itself may very well not have 
expired
+             * yet). We set the X509_STORE_CTX error to something which 
OpenSSL's
+             * s3_both.c:ssl_verify_alarm_type() maps to 
SSL_AD_CERTIFICATE_UNKNOWN,
+             * i.e. the peer will receive a "certificate_unknown(46)" alert.
+             * We do not touch errnum, though, so that later on we will still 
log
+             * the "real" error, as returned by OpenSSL.
              */
-            if (verifyErrorIsOptional) {
-                if (state.certificateVerifyMode != OPTIONAL_NO_CA) {
-                    X509_STORE_CTX_set_error(x509ctx, 
X509_V_ERR_APPLICATION_VERIFICATION());
-                    errnum = X509_V_ERR_APPLICATION_VERIFICATION();
-                    ok = 0;
-                }
-            } else {
-                int ocspResponse = processOCSP(x509ctx);
-                if (ocspResponse == V_OCSP_CERTSTATUS_REVOKED()) {
-                    ok = 0;
-                    errnum = X509_STORE_CTX_get_error(x509ctx);
-                } else if (ocspResponse == V_OCSP_CERTSTATUS_UNKNOWN()) {
-                    errnum = X509_STORE_CTX_get_error(x509ctx);
-                    if (errnum <= 0) {
+            if (ok == 0 && errnum == X509_V_ERR_CRL_HAS_EXPIRED()) {
+                X509_STORE_CTX_set_error(x509ctx, -1);
+            }
+
+            // OCSP
+            if (!state.noOcspCheck && (ok > 0)) {
+                /* If there was an optional verification error, it's not
+                 * possible to perform OCSP validation since the issuer may be
+                 * missing/untrusted.  Fail in that case.
+                 */
+                if (verifyErrorIsOptional) {
+                    if (state.certificateVerifyMode != 
OpenSSLContext.OPTIONAL_NO_CA) {
+                        X509_STORE_CTX_set_error(x509ctx, 
X509_V_ERR_APPLICATION_VERIFICATION());
+                        errnum = X509_V_ERR_APPLICATION_VERIFICATION();
                         ok = 0;
                     }
+                } else {
+                    int ocspResponse = processOCSP(x509ctx);
+                    if (ocspResponse == V_OCSP_CERTSTATUS_REVOKED()) {
+                        ok = 0;
+                        errnum = X509_STORE_CTX_get_error(x509ctx);
+                    } else if (ocspResponse == V_OCSP_CERTSTATUS_UNKNOWN()) {
+                        errnum = X509_STORE_CTX_get_error(x509ctx);
+                        if (errnum <= 0) {
+                            ok = 0;
+                        }
+                    }
                 }
             }
-        }
 
-        if (errdepth > state.certificateVerificationDepth) {
-            // Certificate Verification: Certificate Chain too long
-            ok = 0;
+            if (errdepth > state.certificateVerificationDepth) {
+                // Certificate Verification: Certificate Chain too long
+                ok = 0;
+            }
+            return ok;
         }
-        return ok;
     }
 
-    static int processOCSP(MemorySegment /*X509_STORE_CTX*/ x509ctx) {
+    private static int processOCSP(MemorySegment /*X509_STORE_CTX*/ x509ctx) {
         int ocspResponse = V_OCSP_CERTSTATUS_UNKNOWN();
         // ocspResponse = ssl_verify_OCSP(x509_ctx);
         MemorySegment x509 = X509_STORE_CTX_get_current_cert(x509ctx);


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to