This is an automated email from the ASF dual-hosted git repository. markt-asf pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit 36b1897401057e4e8f3b7740234d5e12432ba760 Author: Mark Thomas <[email protected]> AuthorDate: Wed Nov 4 14:30:02 2026 +0000 Review "Check for NULL after SSL_CTX_new() and SSL_new()..." Add some additional NULL checks Co-authored by: Claude / Sonnet 5.5 --- .../net/openssl/panama/LocalStrings.properties | 2 + .../util/net/openssl/panama/OpenSSLEngine.java | 6 +- .../util/net/openssl/panama/OpenSSLLibrary.java | 66 ++++++++++------------ java/org/apache/tomcat/util/openssl/openssl_h.java | 51 +++++++++++++++++ res/openssl/openssl-tomcat.conf | 1 + 5 files changed, 89 insertions(+), 37 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 443bca259e..bc24ea528d 100644 --- a/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties +++ b/java/org/apache/tomcat/util/net/openssl/panama/LocalStrings.properties @@ -25,6 +25,7 @@ certificateLoader.readFailed=Failed to read the file [{0}] engine.emptyCipherSuite=Empty cipher suite engine.engineClosed=Engine is closed +engine.errMakeBIO=Could not create the network BIO pair [{0}] engine.failedCipherList=Some or all of cipher list [{0}] for TLS 1.2- could not be enabled engine.failedCipherSuite=Some or all of cipher suite [{0}] for TLS 1.3+ could not be enabled engine.failedToReadAvailableBytes=There are plain text bytes available to read but no bytes were read @@ -106,6 +107,7 @@ openssllibrary.currentFIPSMode=Current FIPS mode: [{0}] openssllibrary.engineError=Error creating engine openssllibrary.enterAlreadyInFIPSMode=AprLifecycleListener is configured to force entering FIPS mode, but library is already in FIPS mode [{0}] openssllibrary.FIPSProviderNotDefault=The FIPS provider must be configured as the default provider when FIPS mode [{0}] is configured +openssllibrary.errMakeDHParameters=Could not create the DH parameters [{0}] openssllibrary.errorSettingSSLRandomSeed=Setting random seed [{0}] caused error [{1}] openssllibrary.initializeFIPSFailed=Failed to enter FIPS mode openssllibrary.initializeFIPSSuccess=Successfully entered FIPS mode 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 522a107385..80c4f6f9a1 100644 --- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java +++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLEngine.java @@ -207,7 +207,11 @@ public final class OpenSSLEngine extends SSLEngine implements SSLUtil.ProtocolIn try (var localArena = Arena.ofConfined()) { var internalBIOPointer = localArena.allocate(ValueLayout.ADDRESS); var networkBIOPointer = localArena.allocate(ValueLayout.ADDRESS); - BIO_new_bio_pair(internalBIOPointer, 0, networkBIOPointer, 0); + if (BIO_new_bio_pair(internalBIOPointer, 0, networkBIOPointer, 0) <= 0) { + String error = OpenSSLLibrary.getLastError(); + SSL_free(ssl); + throw new IllegalStateException(sm.getString("engine.errMakeBIO", error)); + } var internalBIO = internalBIOPointer.get(ValueLayout.ADDRESS, 0); var networkBIO = networkBIOPointer.get(ValueLayout.ADDRESS, 0); SSL_set_bio(ssl, internalBIO, internalBIO); 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 3f1286edbf..75a885386d 100644 --- a/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java +++ b/java/org/apache/tomcat/util/net/openssl/panama/OpenSSLLibrary.java @@ -107,42 +107,36 @@ public class OpenSSLLibrary { static final DHParam[] dhParameters = new DHParam[6]; private static void initDHParameters() { - var dh = DH_new(); - var p = BN_get_rfc3526_prime_8192(MemorySegment.NULL); - var g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[0] = new DHParam(dh, 6145); - dh = DH_new(); - p = BN_get_rfc3526_prime_6144(MemorySegment.NULL); - g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[1] = new DHParam(dh, 4097); - dh = DH_new(); - p = BN_get_rfc3526_prime_4096(MemorySegment.NULL); - g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[2] = new DHParam(dh, 3073); - dh = DH_new(); - p = BN_get_rfc3526_prime_3072(MemorySegment.NULL); - g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[3] = new DHParam(dh, 2049); - dh = DH_new(); - p = BN_get_rfc3526_prime_2048(MemorySegment.NULL); - g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[4] = new DHParam(dh, 1025); - dh = DH_new(); - p = BN_get_rfc2409_prime_1024(MemorySegment.NULL); - g = BN_new(); - BN_set_word(g, 2); - DH_set0_pqg(dh, p, MemorySegment.NULL, g); - dhParameters[5] = new DHParam(dh, 0); + dhParameters[0] = createDHParam(BN_get_rfc3526_prime_8192(MemorySegment.NULL), 6145); + dhParameters[1] = createDHParam(BN_get_rfc3526_prime_6144(MemorySegment.NULL), 4097); + dhParameters[2] = createDHParam(BN_get_rfc3526_prime_4096(MemorySegment.NULL), 3073); + dhParameters[3] = createDHParam(BN_get_rfc3526_prime_3072(MemorySegment.NULL), 2049); + dhParameters[4] = createDHParam(BN_get_rfc3526_prime_2048(MemorySegment.NULL), 1025); + dhParameters[5] = createDHParam(BN_get_rfc2409_prime_1024(MemorySegment.NULL), 0); + } + + private static DHParam createDHParam(MemorySegment p, int min) { + MemorySegment dh = MemorySegment.NULL; + MemorySegment g = MemorySegment.NULL; + try { + dh = DH_new(); + g = BN_new(); + if (MemorySegment.NULL.equals(dh) || MemorySegment.NULL.equals(p) || MemorySegment.NULL.equals(g) || + BN_set_word(g, 2) == 0 || DH_set0_pqg(dh, p, MemorySegment.NULL, g) == 0) { + throw new IllegalStateException(sm.getString("openssllibrary.errMakeDHParameters", getLastError())); + } + // DH_set0_pqg() has taken ownership of both BIGNUMs. From this point, only DH_free() may free them. + p = MemorySegment.NULL; + g = MemorySegment.NULL; + DHParam result = new DHParam(dh, min); + dh = MemorySegment.NULL; + return result; + } finally { + // On failure free the values whose ownership was not transferred, including if DHParam creation fails. + BN_free(p); + BN_free(g); + DH_free(dh); + } } private static void freeDHParameters() { diff --git a/java/org/apache/tomcat/util/openssl/openssl_h.java b/java/org/apache/tomcat/util/openssl/openssl_h.java index 8bff6604e1..82ecc4cd4e 100644 --- a/java/org/apache/tomcat/util/openssl/openssl_h.java +++ b/java/org/apache/tomcat/util/openssl/openssl_h.java @@ -1799,6 +1799,57 @@ public class openssl_h { } } + private static class BN_free { + public static final FunctionDescriptor DESC = FunctionDescriptor.ofVoid(openssl_h.C_POINTER); + + public static final MemorySegment ADDR = openssl_h.findOrThrow("BN_free"); + + public static final MethodHandle HANDLE = Linker.nativeLinker().downcallHandle(ADDR, DESC); + } + + /** + * Function descriptor for: + * {@snippet lang = c : * void BN_free(BIGNUM *a) + * } + */ + public static FunctionDescriptor BN_free$descriptor() { + return BN_free.DESC; + } + + /** + * Downcall method handle for: + * {@snippet lang = c : * void BN_free(BIGNUM *a) + * } + */ + public static MethodHandle BN_free$handle() { + return BN_free.HANDLE; + } + + /** + * Address for: + * {@snippet lang = c : * void BN_free(BIGNUM *a) + * } + */ + public static MemorySegment BN_free$address() { + return BN_free.ADDR; + } + + /** + * {@snippet lang = c : * void BN_free(BIGNUM *a) + * } + */ + public static void BN_free(MemorySegment a) { + var mh$ = BN_free.HANDLE; + try { + if (TRACE_DOWNCALLS) { + traceDowncall("BN_free", a); + } + mh$.invokeExact(a); + } catch (Throwable ex$) { + throw new AssertionError("should not reach here", ex$); + } + } + private static class BN_new { public static final FunctionDescriptor DESC = FunctionDescriptor.of(openssl_h.C_POINTER); diff --git a/res/openssl/openssl-tomcat.conf b/res/openssl/openssl-tomcat.conf index 4887ebaf4e..065462875b 100644 --- a/res/openssl/openssl-tomcat.conf +++ b/res/openssl/openssl-tomcat.conf @@ -52,6 +52,7 @@ --include-function BN_get_rfc3526_prime_4096 # header: /usr/include/openssl/bn.h --include-function BN_get_rfc3526_prime_6144 # header: /usr/include/openssl/bn.h --include-function BN_get_rfc3526_prime_8192 # header: /usr/include/openssl/bn.h +--include-function BN_free # header: /usr/include/openssl/bn.h --include-function BN_new # header: /usr/include/openssl/bn.h --include-function BN_set_word # header: /usr/include/openssl/bn.h --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
