diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java index aa82f0cb..0132a66b 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java @@ -145,7 +145,8 @@ private void initKeyManager(KeyManager[] in) if (managers[i] instanceof X509KeyManager) { km = (X509KeyManager)managers[i]; WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "located X509KeyManager instance: " + km); + () -> "located X509KeyManager instance: " + + km.getClass().getName()); break; } } @@ -185,7 +186,8 @@ private void initTrustManager(TrustManager[] in) if (managers[i] instanceof X509TrustManager) { tm = (X509TrustManager)managers[i]; WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "located X509TrustManager instance: " + tm); + () -> "located X509TrustManager instance: " + + tm.getClass().getName()); break; } } @@ -201,8 +203,9 @@ private void initSecureRandom(SecureRandom random) { if (random == null) { sr = new SecureRandom(); + } else { + sr = random; } - sr = random; } /** @@ -616,6 +619,7 @@ protected int addSession(WolfSSLImplementSSLSession session) { String toHash; final int hashCode; + final boolean haveKey; /* Don't store session if invalid (or not complete with sesPtr * if on client side, or not resumable). Server-side still needs to @@ -639,6 +643,7 @@ protected int addSession(WolfSSLImplementSSLSession session) { toHash = session.getPeerHost().concat(Integer.toString( session.getPeerPort())); hashCode = toHash.hashCode(); + haveKey = true; } else { /* If no peer host is available then create hash key from @@ -647,15 +652,18 @@ protected int addSession(WolfSSLImplementSSLSession session) { if (sessionId != null && sessionId.length > 0 && (idAllZeros(sessionId) == false)) { hashCode = Arrays.toString(session.getId()).hashCode(); + haveKey = true; } else { hashCode = 0; + haveKey = false; } } - /* Always try to store session into cache table, as long as we - * have a hashCode. If session already exists for hashCode, it - * will be overwritten with new/refreshed version */ - if (hashCode != 0) { + /* Only store session into cache if we have a usable key. If session + * already exists for hashCode, it will be overwritten with the new + * version. Note that hashCode == 0 is a legitimate hash value, so + * we use haveKey rather than hashCode == 0 to gate caching. */ + if (haveKey) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "stored session in cache table (host: " + session.getPeerHost() + ", port: " + @@ -669,8 +677,12 @@ protected int addSession(WolfSSLImplementSSLSession session) { } printSessionStoreStatus(); + return WolfSSL.SSL_SUCCESS; } + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "Session not cached: no peer host and no usable session ID"); + return WolfSSL.SSL_SUCCESS; } @@ -746,11 +758,13 @@ protected void updateTimeouts(int in, int side) { diff = (now - current.creation.getTime()) / 1000; if (diff < 0) { - /* session is from the future ... */ /* TODO */ - + /* Session creation time in the future. Invalidate so + * it cannot persist indefinitely past the timeout. */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "Invalidating session with future creation"); + current.invalidate(); } - - if (in > 0 && diff >= in) { + else if (in > 0 && diff >= in) { current.invalidate(); } try { diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLContext.java b/src/java/com/wolfssl/provider/jsse/WolfSSLContext.java index b459d6e7..6d2e28f0 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLContext.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLContext.java @@ -26,7 +26,6 @@ import java.security.cert.CertificateEncodingException; import java.security.cert.X509Certificate; import javax.security.auth.x500.X500Principal; -import java.util.Arrays; import javax.net.ssl.KeyManager; import javax.net.ssl.SSLContextSpi; @@ -310,7 +309,7 @@ private void LoadTrustedRootCerts() { } WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "Using X509TrustManager: " + tm.toString()); + () -> "Using X509TrustManager: " + tm.getClass().getName()); /* We only need to load trusted CA certificates into our native * WOLFSSL_CTX if we are doing internal verification with wolfSSL @@ -364,10 +363,12 @@ private void LoadTrustedRootCerts() { /* skip loading if encoding error is encountered */ WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "skipped loading CA, encoding error"); + continue; } catch (WolfSSLJNIException we) { /* skip loading if wolfSSL fails to load der encoding */ WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "skipped loading CA, JNI exception"); + continue; } final String sigAltName = caList[i].getSigAlgName(); @@ -385,6 +386,24 @@ private void LoadTrustedRootCerts() { } } + /* Format an array of objects as "[Class1, Class2, ...]" for debug + * logging without invoking caller-supplied toString() on user-provided + * KeyManager/TrustManager objects. */ + private static String classNames(Object[] arr) { + if (arr == null) { + return "null"; + } + StringBuilder sb = new StringBuilder("["); + for (int i = 0; i < arr.length; i++) { + if (i > 0) { + sb.append(", "); + } + sb.append(arr[i] == null ? "null" : arr[i].getClass().getName()); + } + sb.append("]"); + return sb.toString(); + } + /** * Initializes a SSLContext. * @@ -406,8 +425,9 @@ protected void engineInit(KeyManager[] km, TrustManager[] tm, SecureRandom sr) throws KeyManagementException { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "entered engineInit(km=" + Arrays.toString(km) + ", tm=" + - Arrays.toString(tm) + ", sr=" + sr +")"); + () -> "entered engineInit(km=" + classNames(km) + ", tm=" + + classNames(tm) + ", sr=" + + (sr == null ? "null" : sr.getClass().getName()) + ")"); try { authStore = new WolfSSLAuthStore(km, tm, sr, currentVersion, @@ -620,7 +640,7 @@ protected void finalize() throws Throwable { * @return String array of enabled SSL/TLS protocols for this * WolfSSLContext object */ - public String[] getProtocolsMask(long noOpt) { + private String[] getProtocolsMask(long noOpt) { if (ctx != null) { ctx.setOptions(noOpt); } diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java index 31485143..819cda0f 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java @@ -134,6 +134,14 @@ public class WolfSSLEngine extends SSLEngine { * inside SendAppData, of size SSLSession.getApplicationBufferSize() */ private ByteBuffer staticAppDataBuf = null; + /* Largest plaintext byte count written into staticAppDataBuf by the most + * recent SendAppData() call. Used to bound the post-write zeroing range + * so residual plaintext from a prior larger write is wiped even when the + * current write is smaller. Bytes past max(sendSz, lastSendSz) never need + * zeroing here: ByteBuffer.allocateDirect() zero-initializes the buffer, + * and SendAppData() is the only writer of plaintext into it. */ + private int lastSendSz = 0; + /* Stashed decrypted data when output buffer too small. * Served on next unwrap() without calling ssl_read(). */ private byte[] pendingAppData = null; @@ -780,14 +788,31 @@ private synchronized int SendAppData(ByteBuffer[] in, int ofst, int len) WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "calling ssl.write() with size: " + tmpSendSz); - synchronized (ioLock) { - ret = this.ssl.write(dataArr, sendSz); - } - if (ret <= 0) { - /* error, reset in[] positions for next call */ - for (i = ofst; i < ofst + len; i++) { - in[i].position(pos[i - ofst]); + try { + synchronized (ioLock) { + ret = this.ssl.write(dataArr, sendSz); } + if (ret <= 0) { + /* error, reset in[] positions for next call */ + for (i = ofst; i < ofst + len; i++) { + in[i].position(pos[i - ofst]); + } + } + } finally { + /* Zero plaintext from heap and direct buffers even on exception. + * Zero up to max(sendSz, lastSendSz) bytes so bytes from a prior + * larger write are wiped if needed. */ + Arrays.fill(dataArr, (byte)0); + int zeroSz = Math.max(sendSz, this.lastSendSz); + /* clear() resets position/limit only, does not zero bytes */ + this.staticAppDataBuf.clear(); + /* Bulk put faster than a per-byte loop on DirectByteBuffer */ + this.staticAppDataBuf.put(dataArr, 0, sendSz); + /* Loop wipes [sendSz, lastSendSz) when previous write was larger */ + for (int j = sendSz; j < zeroSz; j++) { + this.staticAppDataBuf.put((byte)0); + } + this.lastSendSz = sendSz; } final int tmpRet = ret; @@ -1077,17 +1102,19 @@ else if (produced == 0) { * @return number of available/remaining bytes in array of ByteBuffers * @throws IllegalArgumentException if readonly buffer found */ - private static synchronized int getTotalOutputSize(ByteBuffer[] out, - int ofst, int length) { + private static int getTotalOutputSize(ByteBuffer[] out, + int ofst, int length) { + int i = 0; int maxOutSz = 0; for (i = 0; i < length; i++) { - if (out[i + ofst] == null || out[i + ofst].isReadOnly()) { + ByteBuffer buf = out[i + ofst]; + if (buf == null || buf.isReadOnly()) { throw new IllegalArgumentException( "null or readonly out buffer found"); } - maxOutSz += out[i + ofst].remaining(); + maxOutSz += buf.remaining(); } return maxOutSz; @@ -2301,7 +2328,9 @@ else if (!this.needInit && !this.handshakeFinished) { this.handshakeStartedExplicitly = true; } catch (SocketTimeoutException e) { - e.printStackTrace(); + WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR, + () -> "doHandshake() timed out in beginHandshake(): " + + e.getMessage()); throw new SSLException(e); } finally { @@ -2813,11 +2842,16 @@ protected synchronized void finalize() throws Throwable { this.ssl.freeSSL(); this.ssl = null; } - /* Clear our reference to static application direct ByteBuffer */ + /* Zero only the byte range that held plaintext and drop our reference + * to the buffer. */ if (this.staticAppDataBuf != null) { this.staticAppDataBuf.clear(); + for (int j = 0; j < this.lastSendSz; j++) { + this.staticAppDataBuf.put((byte)0); + } this.staticAppDataBuf = null; } + this.lastSendSz = 0; super.finalize(); } } diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java index aa248c80..a36b9ddb 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java @@ -516,7 +516,7 @@ protected synchronized Exception getLastVerifyException() { * * @return String array of all supported cipher suites */ - protected static synchronized String[] getAllCiphers() { + protected static String[] getAllCiphers() { return WolfSSLUtil.sanitizeSuites(WolfSSL.getCiphersIana(), false); } @@ -638,7 +638,7 @@ protected synchronized String[] getProtocols() { * * @return String array of supported protocols */ - protected static synchronized String[] getAllProtocols() { + protected static String[] getAllProtocols() { return WolfSSLUtil.sanitizeProtocols( WolfSSL.getProtocols(), WolfSSL.TLS_VERSION.INVALID); } @@ -1696,7 +1696,7 @@ protected synchronized int doHandshake(int isSSLEngine, int timeout) /* send CloseNotify */ /* TODO: SunJSSE sends a Handshake Failure alert instead here */ this.ssl.shutdownSSL(); - } catch (SocketException e) { + } catch (SocketException | SocketTimeoutException e) { throw new SSLException(e); } @@ -1712,6 +1712,13 @@ protected synchronized int doHandshake(int isSSLEngine, int timeout) WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "session creation not allowed"); + try { + /* send CloseNotify so peer is not left hanging */ + this.ssl.shutdownSSL(); + } catch (SocketException | SocketTimeoutException e) { + throw new SSLException(e); + } + return WolfSSL.SSL_HANDSHAKE_FAILURE; } diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java index 58c2998a..c2b43345 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java @@ -35,7 +35,7 @@ import java.util.Date; import java.util.List; import java.util.ArrayList; -import java.util.HashMap; +import java.util.concurrent.ConcurrentHashMap; import java.util.Objects; import java.util.logging.Level; import java.util.logging.Logger; @@ -62,7 +62,7 @@ public class WolfSSLImplementSSLSession extends ExtendedSSLSession { private final WolfSSLAuthStore authStore; private WolfSSLSessionContext ctx = null; private boolean valid = false; - private final HashMap binding; + private final ConcurrentHashMap binding; private final int port; private final String host; String protocol = null; @@ -162,7 +162,7 @@ public WolfSSLImplementSSLSession (WolfSSLSession in, int port, String host, this.peerCerts = null; this.sesPtr = 0; this.protocol = this.nullProtocol; - binding = new HashMap(); + binding = new ConcurrentHashMap(); creation = new Date(); accessed = new Date(); @@ -190,7 +190,7 @@ public WolfSSLImplementSSLSession (WolfSSLSession in, this.peerCerts = null; this.sesPtr = 0; this.protocol = this.nullProtocol; - binding = new HashMap(); + binding = new ConcurrentHashMap(); creation = new Date(); accessed = new Date(); @@ -218,7 +218,7 @@ public WolfSSLImplementSSLSession (WolfSSLAuthStore params) { this.peerCerts = null; this.sesPtr = 0; this.protocol = this.nullProtocol; - binding = new HashMap(); + binding = new ConcurrentHashMap(); creation = new Date(); accessed = new Date(); @@ -282,11 +282,11 @@ public WolfSSLImplementSSLSession (WolfSSLImplementSSLSession orig) { this.sesPtr = orig.sesPtr; this.sesPtrUpdatedAfterTable = false; - /* Copy binding HashMap so session values are preserved */ + /* Copy binding map so session values are preserved */ if (orig.binding != null) { - this.binding = new HashMap(orig.binding); + this.binding = new ConcurrentHashMap(orig.binding); } else { - this.binding = new HashMap(); + this.binding = new ConcurrentHashMap(); } WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, @@ -321,8 +321,8 @@ public synchronized byte[] getId() { return null; } catch (WolfSSLJNIException e) { - /* print stack trace of native JNI error for debugging */ - e.printStackTrace(); + WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR, + () -> "In getId(), JNI error: " + e.getMessage()); return null; } } @@ -430,24 +430,19 @@ protected boolean sessionPointerSet() { * @throws IllegalArgumentException if input name is null */ public void putValue(String name, Object obj) { - Object old; - - if (name == null) { + if (name == null || obj == null) { throw new IllegalArgumentException(); } - /* check if Object should be notified */ + Object old = binding.put(name, obj); + if (obj instanceof SSLSessionBindingListener) { ((SSLSessionBindingListener) obj).valueBound( new SSLSessionBindingEvent(this, name)); } - - old = binding.put(name, obj); - if (old != null) { - if (old instanceof SSLSessionBindingListener) { - ((SSLSessionBindingListener) old).valueUnbound( - new SSLSessionBindingEvent(this, name)); - } + if (old instanceof SSLSessionBindingListener) { + ((SSLSessionBindingListener) old).valueUnbound( + new SSLSessionBindingEvent(this, name)); } } @@ -470,20 +465,16 @@ public Object getValue(String name) { * @throws IllegalArgumentException if input name is null */ public void removeValue(String name) { - Object obj; - if (name == null) { throw new IllegalArgumentException(); } - obj = binding.get(name); - if (obj != null) { - /* check if Object should be notified */ - if (obj instanceof SSLSessionBindingListener) { - ((SSLSessionBindingListener) obj).valueUnbound( - new SSLSessionBindingEvent(this, name)); - } - binding.remove(name); + /* Use ConcurrentHashMap.remove() atomic fetch-and-remove so only + * the thread that actually removed the entry fires valueUnbound(). */ + Object old = binding.remove(name); + if (old instanceof SSLSessionBindingListener) { + ((SSLSessionBindingListener) old).valueUnbound( + new SSLSessionBindingEvent(this, name)); } } @@ -663,11 +654,12 @@ public Certificate[] getLocalCertificates() { } catch (IllegalStateException | WolfSSLJNIException | WolfSSLException ex) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + WolfSSLDebug.log(getClass(), WolfSSLDebug.ERROR, () -> "Error getting peer certificate chain: " + ex.getMessage()); + throw new SSLPeerUnverifiedException( + "Error getting peer certificate chain: " + ex.getMessage()); } - return null; } @Override @@ -1167,7 +1159,7 @@ public synchronized List getRequestedServerNames() sniNames.add(sniName); this.sniServerNames = new ArrayList(sniNames); - return sniNames; + return Collections.unmodifiableList(sniNames); } } catch (IllegalArgumentException e) { throw new UnsupportedOperationException(e);