Fixes for cert name locking, session cache validity, hostname log sanitization, and build/CI hardening

.github/workflows/maven.yml

@@ -85,6 +85,23 @@ jobs:
       - name: Build JNI library
         run: ./java.sh $GITHUB_WORKSPACE/build-dir
 
+      # Resolve declared dependencies into the local Maven repository so
+      # the test dependency bytes can be verified before any test runs.
+      - name: Resolve Maven dependencies
+        run: mvn -B dependency:resolve
+
+      # Verify the resolved JUnit test dependencies against the same
+      # independently trusted SHA-256 digests pinned in the setup-junit
+      # composite action, before mvn package compiles and runs the tests.
+      - name: Verify test dependency checksums
+        shell: bash
+        run: |
+          M2="$HOME/.m2/repository"
+          echo "8e495b634469d64fb8acfa3495a065cbacc8a0fff55ce1e31007be4c16dc57d3  $M2/junit/junit/4.13.2/junit-4.13.2.jar" \
+            | shasum -a 256 -c -
+          echo "4877670629ab96f34f5f90ab283125fcd9acb7e683e66319a68be6eb2cca60de  $M2/org/hamcrest/hamcrest-all/1.3/hamcrest-all-1.3.jar" \
+            | shasum -a 256 -c -
+
       # Maven build. A single mvn package runs the compile, test, and
       # package phases in one lifecycle pass. Separate compile, test,
       # and package steps would run the whole test suite twice, once

build.xml

@@ -103,8 +103,9 @@
 
     <!-- classpath to compiled wolfssl.jar, for running tests -->
     <path id="classpath">
-        <fileset dir="${lib.dir}" includes="*.jar">
+        <fileset dir="${lib.dir}">
             <include name="wolfssl.jar"/>
+            <include name="wolfssl-jsse.jar"/>
         </fileset>
         <fileset dir="${env.JUNIT_HOME}" erroronmissingdir="false">
             <include name="${junit4}"/>

native/com_wolfssl_WolfSSLContext.c

@@ -1022,16 +1022,19 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLContext_memrestoreCertCache
     word32 buffSz = 0;
     WOLFSSL_CTX* ctx = (WOLFSSL_CTX*)(uintptr_t)ctxPtr;
     (void)jcl;
-    (void)sz;
 
     if (jenv == NULL || ctx == NULL || mem == NULL) {
         return (jint)BAD_FUNC_ARG;
     }
 
+    if (sz <= 0 || sz > (*jenv)->GetArrayLength(jenv, mem)) {
+        return (jint)BAD_FUNC_ARG;
+    }
+    buffSz = (word32)sz;
+
     buff = (byte*)(*jenv)->GetByteArrayElements(jenv, mem, NULL);
-    buffSz = (*jenv)->GetArrayLength(jenv, mem);
 
-    if (buff != NULL && buffSz > 0) {
+    if (buff != NULL) {
         ret = wolfSSL_CTX_memrestore_cert_cache(ctx, buff, buffSz);
     }
 

src/java/com/wolfssl/WolfSSLCRL.java

@@ -334,8 +334,10 @@ public int setIssuerName(WolfSSLX509Name name)
         confirmObjectIsActive();
 
         synchronized (crlLock) {
-            return X509_CRL_set_issuer_name(this.crlPtr,
-                name.getNativeX509NamePtr());
+            synchronized (name) {
+                return X509_CRL_set_issuer_name(this.crlPtr,
+                    name.getNativeX509NamePtr());
+            }
         }
     }
 

src/java/com/wolfssl/WolfSSLCertificate.java

@@ -407,6 +407,10 @@ protected long getX509Ptr() throws IllegalStateException {
     public void setSubjectName(WolfSSLX509Name name)
         throws IllegalStateException, WolfSSLException {
 
+        if (name == null) {
+            throw new WolfSSLException("Subject name is null");
+        }
+
         int ret;
 
         confirmObjectIsActive();
@@ -416,9 +420,10 @@ public void setSubjectName(WolfSSLX509Name name)
                 WolfSSLDebug.INFO, this.x509Ptr,
                 () -> "entering setSubjectName(" + name + ")");
 
-            /* TODO somehow lock WolfSSLX509Name object while using pointer? */
-            ret = X509_set_subject_name(this.x509Ptr,
+            synchronized (name) {
+                ret = X509_set_subject_name(this.x509Ptr,
                     name.getNativeX509NamePtr());
+            }
         }
 
         if (ret != WolfSSL.SSL_SUCCESS) {
@@ -443,6 +448,10 @@ public void setSubjectName(WolfSSLX509Name name)
     public void setIssuerName(WolfSSLX509Name name)
         throws IllegalStateException, WolfSSLException {
 
+        if (name == null) {
+            throw new WolfSSLException("Issuer name is null");
+        }
+
         int ret;
 
         confirmObjectIsActive();
@@ -452,9 +461,10 @@ public void setIssuerName(WolfSSLX509Name name)
                 WolfSSLDebug.INFO, this.x509Ptr,
                 () -> "entering setIssuerName(" + name + ")");
 
-            /* TODO somehow lock WolfSSLX509Name object while using pointer? */
-            ret = X509_set_issuer_name(this.x509Ptr,
+            synchronized (name) {
+                ret = X509_set_issuer_name(this.x509Ptr,
                     name.getNativeX509NamePtr());
+            }
         }
 
         if (ret != WolfSSL.SSL_SUCCESS) {

src/java/com/wolfssl/WolfSSLContext.java

@@ -811,11 +811,14 @@ WolfSSLDebug.INFO, getContextPtr(),
      *
      * @param mem   memory buffer containing the stored certificate cache
      *              to restore
-     * @param sz    size of the input memory buffer, <b>mem</b>
+     * @param sz    number of bytes from <b>mem</b> to restore, must be
+     *              greater than zero and no larger than the <b>mem</b>
+     *              array length
      * @return      <b><code>SSL_SUCCESS</code></b> upon success,
      *              <b><code>SSL_FAILURE</code></b> upon general failure,
-     *              <b><code>BAD_FUNC_ARG</code></b> if null or negative
-     *              parameters are passed in,
+     *              <b><code>BAD_FUNC_ARG</code></b> if <b>mem</b> is
+     *              null, or <b>sz</b> is not positive or larger than
+     *              the <b>mem</b> array length,
      *              <b><code>BUFFER_E</code></b> if the certificate cache
      *              memory buffer is too small,
      *              <b><code>CACHE_MATCH_ERROR</code></b> if the cert cache

src/java/com/wolfssl/WolfSSLDebug.java

@@ -28,6 +28,7 @@
 import java.util.logging.Formatter;
 import java.util.logging.Handler;
 import java.util.function.Supplier;
+import java.util.regex.Pattern;
 
 /**
  * Central location for all debugging messages
@@ -179,6 +180,30 @@ private static void configureLoggers() {
         jsseLogger.setUseParentHandlers(false);
     }
 
+    /**
+     * Control characters (including CR/LF), double quote, and backslash,
+     * which could forge log lines or break out of a JSON string field.
+     */
+    private static final Pattern LOG_UNSAFE_CHARS =
+        Pattern.compile("[\\x00-\\x1F\\x7F\"\\\\]");
+
+    /**
+     * Sanitize String for log, replacing characters that could forge log lines
+     * (CR/LF) or break out of a JSON string field (double quote, backslash)
+     * with '_'.
+     *
+     * @param in string to sanitize, may be null
+     * @return sanitized string, or null if input was null
+     */
+    public static String sanitizeForLog(String in) {
+
+        if (in == null) {
+            return null;
+        }
+
+        return LOG_UNSAFE_CHARS.matcher(in).replaceAll("_");
+    }
+
     /**
      * Custom formatter for wolfSSL logs
      */

src/java/com/wolfssl/WolfSSLX509Name.java

@@ -793,14 +793,17 @@ private void confirmObjectIsActive()
     /**
      * For package use only, return native WOLFSSL_X509_NAME pointer.
      *
+     * The returned pointer is only valid while this object is not freed.
+     * Callers must synchronize on this WolfSSLX509Name while using the
+     * returned pointer so a concurrent free() cannot release it.
+     *
      * @return native WOLFSSL_X509_POINTER value
      * @throws IllegalStateException if WolfSSLX509Name has been freed.
      */
     protected long getNativeX509NamePtr() throws IllegalStateException {
 
         confirmObjectIsActive();
 
-        /* TODO lock around x509NamePtr for caller use */
         synchronized (x509NameLock) {
             return this.x509NamePtr;
         }

src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java

@@ -358,8 +358,8 @@ protected WolfSSLImplementSSLSession getSession(
         }
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "attempting to look up session (host: " + host +
-            ", port: " + port + ")");
+            () -> "attempting to look up session (host: " +
+            WolfSSLDebug.sanitizeForLog(host) + ", port: " + port + ")");
 
         /* Print current size and contents of SessionStore / LinkedHashMap.
          * Synchronizes on storeLock internally. */
@@ -386,12 +386,12 @@ protected WolfSSLImplementSSLSession getSession(
 
         /* Check conditions where we need to create a new new session:
          *   1. Session not found in cache
-         *   2. Session marked as not resumable
-         *   3. Original session cipher suite not available
-         *   4. Original session protocol version not available
+         *   2. Session has been invalidated
+         *   3. Session marked as not resumable
+         *   4. Original session cipher suite not available
+         *   5. Original session protocol version not available
          */
-        if (ses == null ||
-            !ses.isResumable() ||
+        if (ses == null || !ses.isValid() || !ses.isResumable() ||
             !sessionCipherSuiteAvailable(ses, enabledCipherSuites) ||
             !sessionProtocolAvailable(ses, enabledProtocols)) {
             needNewSession = true;
@@ -403,6 +403,11 @@ protected WolfSSLImplementSSLSession getSession(
                     () -> "session not found in cache table, " +
                     "creating new session");
             }
+            else if (!ses.isValid()) {
+                WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
+                    () -> "cached session has been invalidated, " +
+                    "creating new session");
+            }
             else if (!ses.isResumable()) {
                 WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                     () -> "native WOLFSSL_SESSION not resumable, " +
@@ -554,7 +559,9 @@ private void printSessionStoreStatus() {
                     () -> "    values: ");
                 for (WolfSSLImplementSSLSession s : values) {
                     WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-                        () -> "        " + s.getHost() + ": " + s.getPort());
+                        () -> "        " +
+                        WolfSSLDebug.sanitizeForLog(s.getHost()) +
+                        ": " + s.getPort());
                 }
             }
             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
@@ -690,12 +697,18 @@ protected int addSession(WolfSSLImplementSSLSession session) {
         if (haveKey) {
             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                 () -> "stored session in cache table (host: " +
-                session.getPeerHost() + ", port: " +
-                session.getPeerPort() + ") " + "cacheKey = " + cacheKey +
+                WolfSSLDebug.sanitizeForLog(session.getPeerHost()) +
+                ", port: " +
+                session.getPeerPort() + ") cacheKey = " +
+                WolfSSLDebug.sanitizeForLog(cacheKey) +
                 " side = " + session.getSideString());
 
             /* Lock access to store while adding new session, store is global */
             synchronized (storeLock) {
+                /* Skip caching a session invalidated during setup. */
+                if (!session.isValid()) {
+                    return WolfSSL.SSL_FAILURE;
+                }
                 session.isInTable = true;
                 store.put(cacheKey, session);
             }

src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java

@@ -2172,6 +2172,15 @@ public synchronized String[] getEnabledCipherSuites() {
         return this.engineHelper.getCiphers();
     }
 
+    /**
+     * Sets the cipher suites enabled for this SSLEngine.
+     *
+     * Note: the jdk.tls.disabledAlgorithms security property is applied to
+     * protocols and key sizes, but not to cipher suites. Cipher suites
+     * disabled only through that property may still be negotiated.
+     *
+     * @param suites array of cipher suites to enable for this SSLEngine
+     */
     @Override
     public synchronized void setEnabledCipherSuites(String[] suites) {
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,

src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java

@@ -196,7 +196,7 @@ protected WolfSSLEngineHelper(WolfSSLSession ssl, WolfSSLAuthStore store,
         this.authStore = store;
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
             () -> "created new WolfSSLEngineHelper(peer port: " + port +
-            ", peer hostname: " + hostname + ")");
+            ", peer hostname: " + WolfSSLDebug.sanitizeForLog(hostname) + ")");
     }
 
     /**

src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java

@@ -169,7 +169,7 @@ public WolfSSLImplementSSLSession (WolfSSLSession in, int port, String host,
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
             () -> "created new session (port: " + port + ", host: " +
-            host + ")");
+            WolfSSLDebug.sanitizeForLog(host) + ")");
     }
 
     /**

src/java/com/wolfssl/provider/jsse/WolfSSLServerSocket.java

@@ -163,6 +163,18 @@ synchronized public String[] getEnabledCipherSuites() {
         return WolfSSLUtil.sanitizeSuites(params.getCipherSuites(), false);
     }
 
+    /**
+     * Sets the cipher suites enabled for this SSLServerSocket.
+     *
+     * Note: the jdk.tls.disabledAlgorithms security property is applied to
+     * protocols and key sizes, but not to cipher suites. Cipher suites
+     * disabled only through that property may still be negotiated.
+     *
+     * @param suites array of cipher suites to enable for this ServerSocket
+     *
+     * @throws IllegalArgumentException when suites array contains
+     *         cipher suites unsupported by native wolfSSL
+     */
     @Override
     synchronized public void setEnabledCipherSuites(String[] suites)
         throws IllegalArgumentException {

src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java

@@ -268,7 +268,8 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context,
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
             () -> "creating new WolfSSLSocket(clientMode: " +
-            String.valueOf(clientMode) + ", host: " + host + ", port: " +
+            String.valueOf(clientMode) + ", host: " +
+            WolfSSLDebug.sanitizeForLog(host) + ", port: " +
             port + ")");
 
         try {
@@ -312,7 +313,8 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context,
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
             () -> "creating new WolfSSLSocket(clientMode: " +
-            String.valueOf(clientMode) + ", host: " + host + ", port: " +
+            String.valueOf(clientMode) + ", host: " +
+            WolfSSLDebug.sanitizeForLog(host) + ", port: " +
             port + ", InetAddress, locaPort: " + localPort + ")");
 
         try {
@@ -360,7 +362,8 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context,
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
             () -> "creating new WolfSSLSocket(clientMode: " +
-            String.valueOf(clientMode) + ", Socket, host: " + host +
+            String.valueOf(clientMode) + ", Socket, host: " +
+            WolfSSLDebug.sanitizeForLog(host) +
             ", port: " + port + ", autoClose: " +
             String.valueOf(autoClose) + ")");
 
@@ -1079,6 +1082,10 @@ public synchronized String[] getEnabledCipherSuites() {
     /**
      * Sets the cipher suites enabled for this SSLSocket.
      *
+     * Note: the jdk.tls.disabledAlgorithms security property is applied to
+     * protocols and key sizes, but not to cipher suites. Cipher suites
+     * disabled only through that property may still be negotiated.
+     *
      * @param suites array of cipher suites to enable for this Socket
      *
      * @throws IllegalArgumentException when suites array contains

src/java/com/wolfssl/provider/jsse/WolfSSLSocketFactory.java

@@ -238,7 +238,8 @@ public Socket createSocket(String host, int port)
         throws IOException, UnknownHostException {
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "entered createSocket(host: " + host + ", port: " +
+            () -> "entered createSocket(host: " +
+            WolfSSLDebug.sanitizeForLog(host) + ", port: " +
             port + ")");
 
         try {
@@ -268,7 +269,8 @@ public Socket createSocket(String host, int port, InetAddress localHost,
         int localPort) throws IOException, UnknownHostException {
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "entered createSocket(host: " + host + ", port: " + port +
+            () -> "entered createSocket(host: " +
+            WolfSSLDebug.sanitizeForLog(host) + ", port: " + port +
             ", InetAddress localHost, localPort: " + localPort + ")");
 
         try {
@@ -299,8 +301,9 @@ public Socket createSocket(Socket s, String host, int port,
         boolean autoClose) throws IOException {
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "entered createSocket(Socket: " + s.getClass() + ", host: " +
-            host + ", port: " + port + ", autoClose: " +
+            () -> "entered createSocket(Socket: " + s.getClass() +
+            ", host: " + WolfSSLDebug.sanitizeForLog(host) +
+            ", port: " + port + ", autoClose: " +
             String.valueOf(autoClose) + ")");
 
         try {