Fenrir fixes for session, cipher, CRL, and X509 handling

native/com_wolfssl_WolfSSLSession.c

@@ -3639,7 +3639,7 @@ JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_getCurrentCipher
 
     if (ssl == NULL) {
         throwWolfSSLException(jenv,
-            "Input WolfSSLSession object was null in getVersion");
+            "Input WolfSSLSession object was null in getCurrentCipher");
         return SSL_FAILURE;
     }
 
@@ -4691,15 +4691,17 @@ JNIEXPORT jint JNICALL Java_com_wolfssl_WolfSSLSession_setTlsHmacInner
 
     ret = wolfSSL_SetTlsHmacInner(ssl, hmacInner, (long)sz, content, verify);
 
-    /* copy hmacInner back into inner jbyteArray */
-    (*jenv)->SetByteArrayRegion(jenv, inner, 0, WOLFSSL_TLS_HMAC_INNER_SZ,
-            (jbyte*)hmacInner);
-    if ((*jenv)->ExceptionOccurred(jenv)) {
-        (*jenv)->ExceptionDescribe(jenv);
-        (*jenv)->ExceptionClear(jenv);
-        throwWolfSSLException(jenv,
-            "Failed to set byte region in native setTlsHmacInner");
-        return -1;
+    /* Copy hmacInner back only on success, else it is uninitialized. */
+    if (ret == 0) {
+        (*jenv)->SetByteArrayRegion(jenv, inner, 0, WOLFSSL_TLS_HMAC_INNER_SZ,
+                (jbyte*)hmacInner);
+        if ((*jenv)->ExceptionOccurred(jenv)) {
+            (*jenv)->ExceptionDescribe(jenv);
+            (*jenv)->ExceptionClear(jenv);
+            throwWolfSSLException(jenv,
+                "Failed to set byte region in native setTlsHmacInner");
+            return -1;
+        }
     }
 
     return ret;

src/java/com/wolfssl/WolfSSLSession.java

@@ -4615,19 +4615,20 @@ public int getCipherType() throws IllegalStateException {
 
     /**
      * Allows caller to set the Hmac Inner vector for message sending/receiving.
-     * The result is written to <b>inner</b> which should be at least
-     * getHmacSize() bytes. The size of the message is specified by <b>sz</b>,
-     * <b>content</b> is the type of message, and <b>verify</b> specifies
-     * whether this is a verification of a peer message. Valid for cipher
-     * types excluding <b>WOLFSSL_AEAD_TYPE</b>.
+     * The result is written to <b>inner</b>, which must be at least
+     * WOLFSSL_TLS_HMAC_INNER_SZ (13) bytes. The size of the message is
+     * specified by <b>sz</b>, <b>content</b> is the type of message, and
+     * <b>verify</b> specifies whether this is a verification of a peer
+     * message. Valid for cipher types excluding <b>WOLFSSL_AEAD_TYPE</b>.
      *
      * @param   inner    inner HMAC vector to set
      * @param   sz       size of the message, in bytes
      * @param   content  type of the message
      * @param   verify   specifies if this is a verification of a peer message.
      *
-     * @return  <b><code>1</code></b> upon success,
-     *          <b><code>BAD_FUNC_ARG</code></b> for an error state.
+     * @return  <b><code>0</code></b> on success, or a negative error code
+     *          such as <b><code>BAD_FUNC_ARG</code></b> on error. On error
+     *          <b>inner</b> is left unmodified.
      * @throws IllegalStateException WolfSSLContext has been freed
      * @see    #getBulkCipher()
      * @see    #getHmacType()

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

@@ -376,12 +376,19 @@ protected WolfSSLImplementSSLSession getSession(
             /* Try getting session out of Java store */
             ses = store.get(cacheKey);
 
-            /* Remove old entry from table. TLS 1.3 binder changes between
-             * resumptions and stored session should only be used to
-             * resume once. New session structure/object will be cached
-             * after the resumed session completes the handshake, for
-             * subsequent resumption attempts to use. */
-            store.remove(cacheKey);
+            /* A server-side entry is not usable for client resumption, so leave
+             * it in the table and fall through to create a new session. */
+            if (ses != null && ses.getSide() != WolfSSL.WOLFSSL_CLIENT_END) {
+                ses = null;
+            }
+            else {
+                /* Remove old entry from table. TLS 1.3 binder changes between
+                 * resumptions and stored session should only be used to
+                 * resume once. New session structure/object will be cached
+                 * after the resumed session completes the handshake, for
+                 * subsequent resumption attempts to use. */
+                store.remove(cacheKey);
+            }
         }
 
         /* Check conditions where we need to create a new new session:
@@ -684,9 +691,11 @@ protected int addSession(WolfSSLImplementSSLSession session) {
             }
         }
 
-        /* Only store session into cache if we have a usable key. If a session
-         * already exists for cacheKey, it will be overwritten with the new
-         * version. */
+        /* Only store session into cache if we have a usable key. An existing
+         * entry for cacheKey is overwritten, including one from the opposite
+         * side since client and server share the host:port key namespace.
+         * getSession() guards the read side against reusing a server-side
+         * entry for client resumption. */
         if (haveKey) {
             WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
                 () -> "stored session in cache table (host: " +

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

@@ -623,48 +623,54 @@ public boolean hasUnsupportedCriticalExtension() {
     }
 
 
-    public Set<String> getCriticalExtensionOIDs() {
+    /* Shared impl for the critical/non-critical extension OID getters below.
+     * wantExtSet selects which to collect: 2 critical, 1 non-critical (per
+     * WolfSSLCertificate.getExtensionSet()). Per the X509Certificate contract,
+     * returns null only when no extensions are present, else a possibly-empty
+     * Set. Presence is only detected for the extensionOid list above, so a
+     * cert carrying only other extensions is treated as having none. */
+    private Set<String> getExtensionOIDs(int wantExtSet) {
         int i;
+        int extCount = 0;
         Set<String> ret = new TreeSet<String>();
 
-        WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "entered getCriticalExtensionOIDs()");
-
         if (this.cert == null) {
             return null;
         }
 
         for (i = 0; i < this.extensionOid.length; i++) {
-            if (this.cert.getExtensionSet(this.extensionOid[i]) == 2) {
+            int extSet = this.cert.getExtensionSet(this.extensionOid[i]);
+            if (extSet == 1 || extSet == 2) {
+                extCount++;
+            }
+            if (extSet == wantExtSet) {
                 ret.add(this.extensionOid[i]);
             }
         }
 
-        if (ret.size() == 0)
+        if (extCount == 0) {
             return null;
+        }
 
         return ret;
     }
 
 
-    public Set<String> getNonCriticalExtensionOIDs() {
-        int i;
-        Set<String> ret = new TreeSet<String>();
+    public Set<String> getCriticalExtensionOIDs() {
 
         WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
-            () -> "entered getNonCriticalExtensionOIDs()");
+            () -> "entered getCriticalExtensionOIDs()");
 
-        if (this.cert == null) {
-            return null;
-        }
+        return getExtensionOIDs(2);
+    }
 
-        for (i = 0; i < this.extensionOid.length; i++) {
-            if (this.cert.getExtensionSet(this.extensionOid[i]) == 1) {
-                ret.add(this.extensionOid[i]);
-            }
-        }
 
-        return ret;
+    public Set<String> getNonCriticalExtensionOIDs() {
+
+        WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
+            () -> "entered getNonCriticalExtensionOIDs()");
+
+        return getExtensionOIDs(1);
     }
 
 

src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java

@@ -88,6 +88,7 @@ class WolfSSLTestFactory {
 
     protected String googleCACert;
     protected String exampleComCert;
+    protected String clientCertDer;
 
     protected final static String jksPassStr = "wolfSSL test";
     protected final static char[] jksPass = jksPassStr.toCharArray();
@@ -136,6 +137,7 @@ protected WolfSSLTestFactory() throws WolfSSLException {
         /* External CA certificate files */
         googleCACert     = "examples/certs/ca-google-root.der";
         exampleComCert   = "examples/certs/example-com.der";
+        clientCertDer    = "examples/certs/client-cert.der";
 
         /* test if running from IDE directory */
         File f = new File(serverJKS);
@@ -174,6 +176,7 @@ private void setPaths(String in) {
 
         googleCACert = in.concat(googleCACert);
         exampleComCert = in.concat(exampleComCert);
+        clientCertDer = in.concat(clientCertDer);
     }
 
     private boolean isIDEFile() {

src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java

@@ -228,6 +228,35 @@ public void testExtensions() {
         }
     }
 
+    @Test
+    public void testCriticalExtensionOIDsEmptyNotNull() {
+
+        WolfSSLX509 x509;
+        Set<String> crit;
+        Set<String> nonCrit;
+
+        /* skip if wolfSSL compiled with NO_FILESYSTEM */
+        Assume.assumeTrue(WolfSSL.FileSystemEnabled());
+
+        try {
+            /* client-cert.der has non-critical extensions present but none
+             * marked critical. A cert that has extensions must return a
+             * possibly-empty Set from getCriticalExtensionOIDs(), not null. */
+            x509 = new WolfSSLX509(tf.clientCertDer);
+
+            crit = x509.getCriticalExtensionOIDs();
+            assertNotNull(crit);
+            assertTrue(crit.isEmpty());
+
+            nonCrit = x509.getNonCriticalExtensionOIDs();
+            assertNotNull(nonCrit);
+            assertFalse(nonCrit.isEmpty());
+
+        } catch (Exception ex) {
+            fail("unexpected exception found");
+        }
+    }
+
     @Test
     public void testX509XValidity() {
         WolfSSLX509X x509;

src/test/com/wolfssl/test/WolfSSLSessionTest.java

@@ -5552,5 +5552,38 @@ public Void call() throws Exception {
             }
         }
     }
+
+    @Test
+    public void test_WolfSSLSession_setTlsHmacInnerErrorNoBufferCopy()
+        throws WolfSSLJNIException, WolfSSLException {
+
+        /* WOLFSSL_TLS_HMAC_INNER_SZ from native wolfSSL */
+        final int hmacInnerSz = 13;
+
+        /* dtls12_cid content type forces wolfSSL_SetTlsHmacInner() to return
+         * an error before it writes the inner buffer */
+        final int dtls12Cid = 25;
+
+        WolfSSLSession ssl = new WolfSSLSession(ctx);
+
+        try {
+            /* Prefill inner with a sentinel so we can detect any overwrite */
+            byte[] inner = new byte[hmacInnerSz];
+            byte[] expected = new byte[hmacInnerSz];
+            for (int i = 0; i < hmacInnerSz; i++) {
+                inner[i] = (byte)0xAA;
+                expected[i] = (byte)0xAA;
+            }
+
+            int ret = ssl.setTlsHmacInner(inner, 0, dtls12Cid, 0);
+
+            /* Error return expected, buffer must be left untouched so no
+             * uninitialized native data is handed back to the caller */
+            assertNotEquals(0, ret);
+            assertArrayEquals(expected, inner);
+        } finally {
+            ssl.freeSSL();
+        }
+    }
 }