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


The following commit(s) were added to refs/heads/10.1.x by this push:
     new cc96ad9c53 Cleanup and log OpenSSL errors before checking commands
cc96ad9c53 is described below

commit cc96ad9c534551ecc8b599b66f14b6f4941a3ed0
Author: remm <r...@apache.org>
AuthorDate: Tue Jun 18 11:55:01 2024 +0200

    Cleanup and log OpenSSL errors before checking commands
    
    Also fix logic to avoid trying to set "builtin" seed.
---
 .../net/openssl/panama/LocalStrings.properties     |  2 +
 .../util/net/openssl/panama/OpenSSLContext.java    | 73 +++++++---------------
 .../util/net/openssl/panama/OpenSSLEngine.java     |  4 +-
 .../util/net/openssl/panama/OpenSSLLibrary.java    | 39 +++++++++++-
 4 files changed, 65 insertions(+), 53 deletions(-)

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 1a2f3d83fa..307fa3f4a2 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
+++ b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties
@@ -51,6 +51,7 @@ openssl.errCheckConf=Error during OpenSSLConf check
 openssl.errMakeConf=Could not create OpenSSLConf context [{0}]
 openssl.errorAddingCertificate=Error adding certificate to chain: [{0}]
 openssl.errorConfiguringLocations=Error configuring CA certificate locations: 
[{0}]
+openssl.errorInit=Non fatal error initializing certificates or configuration: 
[{0}]
 openssl.errorLoadingCertificate=Error loading certificate: [{0}]
 openssl.errorLoadingCertificateRevocationListWithError=Error loading 
certificate revocation [{0}] with error [{1}]
 openssl.errorLoadingCertificateWithError=Error loading certificate [{0}] with 
error [{1}]
@@ -90,6 +91,7 @@ openssllibrary.initializeFIPSFailed=Failed to enter FIPS mode
 openssllibrary.initializeFIPSSuccess=Successfully entered FIPS mode
 openssllibrary.initializedOpenSSL=OpenSSL successfully initialized using FFM 
[{0}]
 openssllibrary.initializingFIPS=Initializing FIPS mode...
+openssllibrary.errorSettingSSLRandomSeed=Setting random seed [{0}] caused 
error [{1}]
 openssllibrary.requireNotInFIPSMode=The listener is configured to require the 
library to already be in FIPS mode, but it was not in FIPS mode
 openssllibrary.skipFIPSInitialization=Already in FIPS mode; skipping FIPS 
initialization.
 openssllibrary.tooLateForFIPSMode=Cannot setFIPSMode: SSL has already been 
initialized
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 4b21075c5b..9a8ba2ea2b 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLContext.java
@@ -76,8 +76,6 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 
     private static final Cleaner cleaner = Cleaner.create();
 
-    private static final int OPENSSL_ERROR_MESSAGE_BUFFER_SIZE = 256;
-
     private static final String defaultProtocol = "TLS";
 
     private static final int SSL_AIDX_RSA     = 0;
@@ -179,7 +177,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                 }
                 confCtx = SSL_CONF_CTX_new();
                 if (MemorySegment.NULL.equals(confCtx)) {
-                    throw new SSLException(sm.getString("openssl.errMakeConf", 
getLastError()));
+                    throw new SSLException(sm.getString("openssl.errMakeConf", 
OpenSSLLibrary.getLastError()));
                 }
                 SSL_CONF_CTX_set_flags(confCtx, SSL_CONF_FLAG_FILE() |
                         SSL_CONF_FLAG_SERVER() |
@@ -344,7 +342,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                 } else {
                     int code = SSL_CONF_cmd_value_type(state.confCtx, 
localArena.allocateFrom(name));
                     rc = 1;
-                    String errorMessage = getLastError();
+                    String errorMessage = OpenSSLLibrary.getLastError();
                     if (errorMessage != null) {
                         log.error(sm.getString("opensslconf.checkFailed", 
errorMessage));
                         rc = 0;
@@ -416,7 +414,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                 } else {
                     rc = SSL_CONF_cmd(state.confCtx, 
localArena.allocateFrom(name),
                             localArena.allocateFrom(value));
-                    String errorMessage = getLastError();
+                    String errorMessage = OpenSSLLibrary.getLastError();
                     if (rc <= 0 || errorMessage != null) {
                         log.error(sm.getString("opensslconf.commandError", 
name, value, errorMessage));
                         rc = 0;
@@ -592,6 +590,13 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                         SSL_CTX_set_alpn_select_cb$cb.allocate(new 
ALPNSelectCallback(negotiableProtocols), contextArena), state.sslCtx);
             }
 
+            // Log any non fatal init errors
+            String errMessage = OpenSSLLibrary.getLastError();
+            while (errMessage != null) {
+                log.info(sm.getString("openssl.errorInit", errMessage));
+                errMessage = OpenSSLLibrary.getLastError();
+            }
+
             // Apply OpenSSLConfCmd if used
             OpenSSLConf openSslConf = sslHostConfig.getOpenSslConf();
             if (openSslConf != null && 
!MemorySegment.NULL.equals(state.confCtx)) {
@@ -933,7 +938,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
             try {
                 if (BIO_write(certificateBIO, certificateFileBytesNative, 
certificateFileBytes.length) <= 0) {
                     
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                            certificate.getCertificateFile(), getLastError()));
+                            certificate.getCertificateFile(), 
OpenSSLLibrary.getLastError()));
                     return false;
                 }
                 MemorySegment cert = MemorySegment.NULL;
@@ -943,7 +948,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     MemorySegment p12 = d2i_PKCS12_bio(certificateBIO, 
MemorySegment.NULL);
                     if (MemorySegment.NULL.equals(p12)) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                certificate.getCertificateFile(), 
getLastError()));
+                                certificate.getCertificateFile(), 
OpenSSLLibrary.getLastError()));
                         return false;
                     }
                     MemorySegment passwordAddress = MemorySegment.NULL;
@@ -955,7 +960,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     if (PKCS12_verify_mac(p12, passwordAddress, 
passwordLength) <= 0) {
                         // Bad password
                         
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                certificate.getCertificateFile(), 
getLastError()));
+                                certificate.getCertificateFile(), 
OpenSSLLibrary.getLastError()));
                         PKCS12_free(p12);
                         return false;
                     }
@@ -963,7 +968,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     MemorySegment keyPointer = 
localArena.allocate(ValueLayout.ADDRESS);
                     if (PKCS12_parse(p12, passwordAddress, keyPointer, 
certPointer, MemorySegment.NULL) <= 0) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                certificate.getCertificateFile(), 
getLastError()));
+                                certificate.getCertificateFile(), 
OpenSSLLibrary.getLastError()));
                         PKCS12_free(p12);
                         return false;
                     }
@@ -986,7 +991,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     try {
                         if (BIO_write(keyBIO, certificateKeyFileBytesNative, 
certificateKeyFileBytes.length) <= 0) {
                             
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                    certificateKeyFileName, getLastError()));
+                                    certificateKeyFileName, 
OpenSSLLibrary.getLastError()));
                             return false;
                         }
                         key = MemorySegment.NULL;
@@ -1012,7 +1017,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     }
                     if (MemorySegment.NULL.equals(key)) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                certificateKeyFileName, getLastError()));
+                                certificateKeyFileName, 
OpenSSLLibrary.getLastError()));
                         return false;
                     }
                     // Load certificate
@@ -1028,7 +1033,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     }
                     if (MemorySegment.NULL.equals(cert)) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                certificate.getCertificateFile(), 
getLastError()));
+                                certificate.getCertificateFile(), 
OpenSSLLibrary.getLastError()));
                         return false;
                     }
                 }
@@ -1110,7 +1115,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     try {
                         if (BIO_write(certificateChainBIO, 
certificateChainBytesNative, certificateChainBytes.length) <= 0) {
                             
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                    certificate.getCertificateChainFile(), 
getLastError()));
+                                    certificate.getCertificateChainFile(), 
OpenSSLLibrary.getLastError()));
                             return false;
                         }
                         MemorySegment certChainEntry =
@@ -1118,7 +1123,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                         while (!MemorySegment.NULL.equals(certChainEntry)) {
                             if (SSL_CTX_add0_chain_cert(state.sslCtx, 
certChainEntry) <= 0) {
                                 
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                        certificate.getCertificateChainFile(), 
getLastError()));
+                                        certificate.getCertificateChainFile(), 
OpenSSLLibrary.getLastError()));
                             }
                             certChainEntry =
                                     PEM_read_bio_X509_AUX(certificateChainBIO, 
MemorySegment.NULL, MemorySegment.NULL, MemorySegment.NULL);
@@ -1128,7 +1133,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                             ERR_clear_error();
                         } else {
                             
log.error(sm.getString("openssl.errorLoadingCertificateWithError",
-                                    certificate.getCertificateChainFile(), 
getLastError()));
+                                    certificate.getCertificateChainFile(), 
OpenSSLLibrary.getLastError()));
                         }
                     } finally {
                         BIO_free(certificateChainBIO);
@@ -1143,7 +1148,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     if (X509_LOOKUP_load_file(x509Lookup, 
certificateRevocationListFileNative,
                             X509_FILETYPE_PEM()) <= 0) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateRevocationListWithError",
-                                
sslHostConfig.getCertificateRevocationListFile(), getLastError()));
+                                
sslHostConfig.getCertificateRevocationListFile(), 
OpenSSLLibrary.getLastError()));
                     }
                 }
                 if (sslHostConfig.getCertificateRevocationListPath() != null) {
@@ -1153,7 +1158,7 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
                     if (X509_LOOKUP_add_dir(x509Lookup, 
certificateRevocationListPathNative,
                             X509_FILETYPE_PEM()) <= 0) {
                         
log.error(sm.getString("openssl.errorLoadingCertificateRevocationListWithError",
-                                
sslHostConfig.getCertificateRevocationListPath(), getLastError()));
+                                
sslHostConfig.getCertificateRevocationListPath(), 
OpenSSLLibrary.getLastError()));
                     }
                 }
                 X509_STORE_set_flags(certificateStore, X509_V_FLAG_CRL_CHECK() 
| X509_V_FLAG_CRL_CHECK_ALL());
@@ -1305,45 +1310,13 @@ public class OpenSSLContext implements 
org.apache.tomcat.util.net.SSLContext {
 
 
     private static void logLastError(String string) {
-        String message = getLastError();
+        String message = OpenSSLLibrary.getLastError();
         if (message != null) {
             log.error(sm.getString(string, message));
         }
     }
 
 
-    /**
-     * Many calls to SSL methods do not check the last error. Those that do
-     * check the last error need to ensure that any previously ignored error is
-     * cleared prior to the method call else errors may be falsely reported.
-     * Ideally, before any SSL_read, SSL_write, clearLastError should always
-     * be called, and getLastError should be called after on any negative or
-     * zero result.
-     * @return the first error in the stack
-     */
-    static String getLastError() {
-        String sslError = null;
-        long error = ERR_get_error();
-        if (error != SSL_ERROR_NONE()) {
-            try (var localArena = Arena.ofConfined()) {
-                do {
-                    // Loop until getLastErrorNumber() returns SSL_ERROR_NONE
-                    var buf = localArena.allocate(ValueLayout.JAVA_BYTE, 
OPENSSL_ERROR_MESSAGE_BUFFER_SIZE);
-                    ERR_error_string_n(error, buf, 
OPENSSL_ERROR_MESSAGE_BUFFER_SIZE);
-                    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());
-            }
-        }
-        return sslError;
-    }
-
-
     @Override
     public SSLSessionContext getServerSessionContext() {
         return sessionContext;
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 d80b48d2c4..5d3f5b0e8a 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java
@@ -944,7 +944,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
     }
 
     private void checkLastError() throws SSLException {
-        String sslError = OpenSSLContext.getLastError();
+        String sslError = OpenSSLLibrary.getLastError();
         if (sslError != null) {
             // Many errors can occur during handshake and need to be reported
             if (!handshakeFinished) {
@@ -960,7 +960,7 @@ public final class OpenSSLEngine extends SSLEngine 
implements SSLUtil.ProtocolIn
      * Clear out any errors, but log a warning.
      */
     private static void clearLastError() {
-        OpenSSLContext.getLastError();
+        OpenSSLLibrary.getLastError();
     }
 
     private SSLEngineResult.Status getEngineStatus() {
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 262e97b671..2ce3a201dd 100644
--- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
+++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java
@@ -218,9 +218,12 @@ public class OpenSSLLibrary {
 
                 // Set the random seed, translated to the Java way
                 boolean seedDone = false;
-                if (SSLRandomSeed != null || SSLRandomSeed.length() != 0 || 
!"builtin".equals(SSLRandomSeed)) {
+                if (SSLRandomSeed != null && SSLRandomSeed.length() != 0 && 
!"builtin".equals(SSLRandomSeed)) {
                     var randomSeed = memorySession.allocateFrom(SSLRandomSeed);
                     seedDone = RAND_load_file(randomSeed, 128) > 0;
+                    if (!seedDone) {
+                        
log.warn(sm.getString("openssllibrary.errorSettingSSLRandomSeed", 
SSLRandomSeed, OpenSSLLibrary.getLastError()));
+                    }
                 }
                 if (!seedDone) {
                     // Use a regular random to get some bytes
@@ -456,4 +459,38 @@ public class OpenSSLLibrary {
         return ciphers.toArray(new String[0]);
     }
 
+    private static final int OPENSSL_ERROR_MESSAGE_BUFFER_SIZE = 256;
+
+    /**
+     * Many calls to SSL methods do not check the last error. Those that do
+     * check the last error need to ensure that any previously ignored error is
+     * cleared prior to the method call else errors may be falsely reported.
+     * Ideally, before any SSL_read, SSL_write, clearLastError should always
+     * be called, and getLastError should be called after on any negative or
+     * zero result.
+     * @return the first error in the stack
+     */
+    static String getLastError() {
+        String sslError = null;
+        long error = ERR_get_error();
+        if (error != SSL_ERROR_NONE()) {
+            try (var localArena = Arena.ofConfined()) {
+                do {
+                    // Loop until getLastErrorNumber() returns SSL_ERROR_NONE
+                    var buf = localArena.allocate(ValueLayout.JAVA_BYTE, 
OPENSSL_ERROR_MESSAGE_BUFFER_SIZE);
+                    ERR_error_string_n(error, buf, 
OPENSSL_ERROR_MESSAGE_BUFFER_SIZE);
+                    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());
+            }
+        }
+        return sslError;
+    }
+
+
 }


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

Reply via email to