diff --git a/native/com_wolfssl_WolfSSLSession.c b/native/com_wolfssl_WolfSSLSession.c index d404dea8..f3409166 100644 --- a/native/com_wolfssl_WolfSSLSession.c +++ b/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; diff --git a/src/java/com/wolfssl/WolfSSLSession.java b/src/java/com/wolfssl/WolfSSLSession.java index 65ecb484..110b2f2a 100644 --- a/src/java/com/wolfssl/WolfSSLSession.java +++ b/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 inner which should be at least - * getHmacSize() bytes. The size of the message is specified by sz, - * content is the type of message, and verify specifies - * whether this is a verification of a peer message. Valid for cipher - * types excluding WOLFSSL_AEAD_TYPE. + * The result is written to inner, which must be at least + * WOLFSSL_TLS_HMAC_INNER_SZ (13) bytes. The size of the message is + * specified by sz, content is the type of message, and + * verify specifies whether this is a verification of a peer + * message. Valid for cipher types excluding WOLFSSL_AEAD_TYPE. * * @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 1 upon success, - * BAD_FUNC_ARG for an error state. + * @return 0 on success, or a negative error code + * such as BAD_FUNC_ARG on error. On error + * inner is left unmodified. * @throws IllegalStateException WolfSSLContext has been freed * @see #getBulkCipher() * @see #getHmacType() diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java index 7f36590c..f3c7086a 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java +++ b/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: " + diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLX509.java b/src/java/com/wolfssl/provider/jsse/WolfSSLX509.java index c375af7c..bdcde001 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLX509.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLX509.java @@ -623,48 +623,54 @@ public boolean hasUnsupportedCriticalExtension() { } - public Set 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 getExtensionOIDs(int wantExtSet) { int i; + int extCount = 0; Set ret = new TreeSet(); - 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 getNonCriticalExtensionOIDs() { - int i; - Set ret = new TreeSet(); + public Set 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 getNonCriticalExtensionOIDs() { + + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "entered getNonCriticalExtensionOIDs()"); + + return getExtensionOIDs(1); } diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java index 35a662d2..a9ecca99 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLTestFactory.java +++ b/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() { diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java index e334ac59..c12b2cbf 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLX509Test.java @@ -228,6 +228,35 @@ public void testExtensions() { } } + @Test + public void testCriticalExtensionOIDsEmptyNotNull() { + + WolfSSLX509 x509; + Set crit; + Set 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; diff --git a/src/test/com/wolfssl/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/test/WolfSSLSessionTest.java index 2cb4eb73..edafc378 100644 --- a/src/test/com/wolfssl/test/WolfSSLSessionTest.java +++ b/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(); + } + } }