diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index 9875fe0..f93201d 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -2709,120 +2709,132 @@ public class WolfSSLSocket extends SSLSocket { throw new NullPointerException("Input array is null"); } - /* check if socket is closing */ - if (isClosing.get()) { - throw new SocketException( - "InputStream in process of being closed"); - } - - /* check if socket is closed */ - if (this.isClosed || socket == null || socket.isClosed()) { + if (socket == null) { throw new SocketException("Socket is closed"); } - /* check if connection has already been closed/shutdown */ - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "trying to get socket.handshakeLock (read)"); - - synchronized (socket.handshakeLock) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "thread got socket.handshakeLock (read)"); - - if (socket.connectionClosed == true) { - throw new SocketException("Connection already shutdown"); - } - } - - /* do handshake if not completed yet, handles synchronization */ - try { - /* do handshake if not completed yet, handles synchronization */ - if (socket.handshakeComplete == false && - socket.handshakeStarted == false) { - socket.startHandshake(); - } - } catch (SocketTimeoutException e) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "got socket timeout in read()"); - throw e; - } - - if (b.length == 0 || len == 0) { - return 0; - } - - if (off < 0 || len < 0 || len > (b.length - off)) { - throw new IndexOutOfBoundsException( - "Array index out of bounds"); - } - - /* Enter I/O operation to prevent use-after-free */ + /* Enter I/O op before state checks below, so close() either + * defers ssl.freeSSL() until this read exits or the checks + * below throw before the SSL session is used */ socket.enterIOOperation(); try { + /* check if socket is closing */ + if (isClosing.get()) { + throw new SocketException( + "InputStream in process of being closed"); + } + + /* check if socket is closed */ + if (this.isClosed || socket.isClosed()) { + throw new SocketException("Socket is closed"); + } + + /* check if connection has already been closed/shutdown */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "trying to get socket.handshakeLock (read)"); + + synchronized (socket.handshakeLock) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "thread got socket.handshakeLock (read)"); + + if (socket.connectionClosed == true) { + throw new SocketException( + "Connection already shutdown"); + } + } + + try { + /* do handshake if not completed yet, handles + * synchronization */ + if (socket.handshakeComplete == false && + socket.handshakeStarted == false) { + socket.startHandshake(); + } + } catch (SocketTimeoutException e) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "got socket timeout in read()"); + throw e; + } + + if (b.length == 0 || len == 0) { + return 0; + } + + if (off < 0 || len < 0 || len > (b.length - off)) { + throw new IndexOutOfBoundsException( + "Array index out of bounds"); + } + int err; int timeout = socket.getSoTimeout(); WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "ssl.read() socket timeout = " + timeout); - ret = ssl.read(b, off, len, timeout); - err = ssl.getError(ret); + try { + ret = ssl.read(b, off, len, timeout); + err = ssl.getError(ret); - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "ssl.read(off: " + off + ", len: " + len + - ") ret = " + ret + ", err = " + err); - - /* check for end of stream */ - if ((err == WolfSSL.SSL_ERROR_ZERO_RETURN) || - ((err == WolfSSL.SSL_ERROR_SOCKET_PEER_CLOSED) && - (ret == 0))) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "ssl.read() got SSL_ERROR_ZERO_RETURN, " + err + - ", end of stream"); + () -> "ssl.read(off: " + off + ", len: " + len + + ") ret = " + ret + ", err = " + err); - /* End of stream */ - return -1; - } - - if (ret < 0) { - /* other errors besides end of stream or WANT_READ - * are treated as I/O errors and throw an exception */ - String errStr = WolfSSL.getErrorString(err); - if (err == WolfSSL.SOCKET_ERROR_E) { - /* Socket error, indicate to caller by returning - * end of stream */ + /* check for end of stream */ + if ((err == WolfSSL.SSL_ERROR_ZERO_RETURN) || + ((err == WolfSSL.SSL_ERROR_SOCKET_PEER_CLOSED) && + (ret == 0))) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "Native wolfSSL_read() error: " + errStr + - " (error code: " + err + "ret: " + ret + - "), end of stream"); + () -> "ssl.read() got SSL_ERROR_ZERO_RETURN, " + + err + ", end of stream"); + + /* End of stream */ return -1; - - } else { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "Native wolfSSL_read() error: " + errStr + - " (error code: " + err + ", ret: " + ret + ")"); - throw new IOException("Native wolfSSL_read() " + - "error: " + errStr + - " (error code: " + err + ", ret: " + ret + ")"); } - } - } catch (SocketException e) { - /* ssl.read() can throw SocketException from poll() if fd - * closed or peer shut down connection */ - if (e.getMessage().contains("fd closed during poll") || - e.getMessage().contains("disconnected during poll")) { - /* end of stream */ - return -1; - } - throw e; + if (ret < 0) { + /* other errors besides end of stream or WANT_READ + * are treated as I/O errors and throw an exception */ + String errStr = WolfSSL.getErrorString(err); + if (err == WolfSSL.SOCKET_ERROR_E) { + /* Socket error, indicate to caller by returning + * end of stream */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "Native wolfSSL_read() error: " + + errStr + " (error code: " + err + "ret: " + + ret + "), end of stream"); + return -1; - } catch (IllegalStateException e) { - /* SSLSocket.close() may have already called freeSSL(), - * thus causing a 'WolfSSLSession object has been freed' - * IllegalStateException to be thrown from - * WolfSSLSession.read(). Return as a SocketException here. */ - throw new SocketException(e.getMessage()); + } else { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "Native wolfSSL_read() error: " + + errStr + " (error code: " + err + ", ret: " + + ret + ")"); + throw new IOException("Native wolfSSL_read() " + + "error: " + errStr + " (error code: " + err + + ", ret: " + ret + ")"); + } + } + + } catch (SocketException e) { + /* ssl.read() can throw SocketException from poll() if fd + * closed or peer shut down connection */ + String msg = e.getMessage(); + if (msg != null && + (msg.contains("fd closed during poll") || + msg.contains("disconnected during poll"))) { + /* end of stream */ + return -1; + } + throw e; + + } catch (IllegalStateException e) { + /* SSLSocket.close() may have already called freeSSL(), + * thus causing a 'WolfSSLSession object has been freed' + * IllegalStateException to be thrown from + * WolfSSLSession.read(). Return as a SocketException. */ + throw new SocketException(e.getMessage()); + } } finally { /* Exit I/O operation */ socket.exitIOOperation(); @@ -2931,91 +2943,104 @@ public class WolfSSLSocket extends SSLSocket { throw new NullPointerException("Input array is null"); } - /* check if socket is closing */ - if (isClosing.get()) { - throw new SocketException( - "OutputStream in process of being closed"); - } - - /* check if socket is closed */ - if (this.isClosed || socket == null || socket.isClosed()) { + if (socket == null) { throw new SocketException("Socket is closed"); } - /* check if connection has already been closed/shutdown */ - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "trying to get socket.handshakeLock (write)"); - - synchronized (socket.handshakeLock) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "thread got socket.handshakeLock (write)"); - if (socket.connectionClosed == true) { - throw new SocketException( - "Connection already shutdown"); - } - } - - try { - /* do handshake if not completed yet, handles synchronization */ - if (socket.handshakeComplete == false && - socket.handshakeStarted == false) { - socket.startHandshake(); - } - } catch (SocketTimeoutException e) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "got socket timeout in write()"); - throw e; - } - - if (off < 0 || len < 0 || (off + len) > b.length) { - throw new IndexOutOfBoundsException( - "Array index out of bounds"); - } - - /* Enter I/O operation to prevent use-after-free */ + /* Enter I/O op before state checks below, so close() either + * defers ssl.freeSSL() until this write exits or the checks + * below throw before the SSL session is used */ socket.enterIOOperation(); try { + /* check if socket is closing */ + if (isClosing.get()) { + throw new SocketException( + "OutputStream in process of being closed"); + } + + /* check if socket is closed */ + if (this.isClosed || socket.isClosed()) { + throw new SocketException("Socket is closed"); + } + + /* check if connection has already been closed/shutdown */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "trying to get socket.handshakeLock (write)"); + + synchronized (socket.handshakeLock) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "thread got socket.handshakeLock (write)"); + if (socket.connectionClosed == true) { + throw new SocketException( + "Connection already shutdown"); + } + } + + try { + /* do handshake if not completed yet, handles + * synchronization */ + if (socket.handshakeComplete == false && + socket.handshakeStarted == false) { + socket.startHandshake(); + } + } catch (SocketTimeoutException e) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "got socket timeout in write()"); + throw e; + } + + if (off < 0 || len < 0 || (off + len) > b.length) { + throw new IndexOutOfBoundsException( + "Array index out of bounds"); + } + int err; int timeout = socket.getSoTimeout(); WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, () -> "ssl.write() socket timeout = " + timeout); - ret = ssl.write(b, off, len, timeout); - err = ssl.getError(ret); + /* Scope the freed-session catch to ssl.write() only, so a + * close() racing an implicit handshake above propagates + * unchanged */ + try { + ret = ssl.write(b, off, len, timeout); + err = ssl.getError(ret); - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "ssl.write(off: " + off + ", len: " + len + - ") returned ret = " + ret + ", err = " + err); - - /* check for end of stream */ - if (err == WolfSSL.SSL_ERROR_ZERO_RETURN) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "ssl.write() got SSL_ERROR_ZERO_RETURN, " + - "end of stream"); + () -> "ssl.write(off: " + off + ", len: " + len + + ") returned ret = " + ret + ", err = " + err); - /* check to see if we received a close notify alert. - * if so, throw SocketException since peer has closed - * the connection */ - if (ssl.gotCloseNotify() == true) { - throw new SocketException("Peer closed connection"); + /* check for end of stream */ + if (err == WolfSSL.SSL_ERROR_ZERO_RETURN) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "ssl.write() got SSL_ERROR_ZERO_RETURN, " + + "end of stream"); + + /* check to see if we received a close notify alert. + * if so, throw SocketException since peer has closed + * the connection */ + if (ssl.gotCloseNotify() == true) { + throw new SocketException( + "Peer closed connection"); + } } - } - if (ret < 0) { - /* print error description string */ - String errStr = WolfSSL.getErrorString(err); - throw new IOException("Native wolfSSL_write() error: " + - errStr + " (ret: " + ret + ", error code: " + - err + ")"); - } + if (ret < 0) { + /* print error description string */ + String errStr = WolfSSL.getErrorString(err); + throw new IOException( + "Native wolfSSL_write() error: " + errStr + + " (ret: " + ret + ", error code: " + err + ")"); + } - } catch (IllegalStateException e) { - WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, - () -> "got IllegalStateException: " + e + - ", throwing IOException"); - throw new IOException(e); + } catch (IllegalStateException e) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + () -> "got IllegalStateException: " + e + + ", throwing IOException"); + throw new IOException(e); + } } finally { /* Exit I/O operation */ socket.exitIOOperation(); diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java index 4a74ae9..237110a 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java @@ -42,6 +42,8 @@ import java.util.concurrent.ExecutorService; import java.util.concurrent.TimeoutException; import java.util.concurrent.CountDownLatch; import java.util.concurrent.atomic.AtomicIntegerArray; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.locks.LockSupport; import java.io.InputStream; import java.io.OutputStream; @@ -133,6 +135,8 @@ import com.wolfssl.WolfSSLException; public void testSocketConnectException(); public void testSocketCloseInterruptsWrite(); public void testSocketCloseInterruptsRead(); + public void testSocketCloseDuringConcurrentWrite(); + public void testSocketCloseDuringConcurrentRead(); public void testSSLHandshakeExceptionCauseChain(); */ public class WolfSSLSocketTest { @@ -3522,8 +3526,7 @@ public class WolfSSLSocketTest { String msg = e.getMessage(); if (msg == null || (!msg.contains("Socket is closed") && - !msg.contains("Connection already shutdown") && - !msg.contains("object has been freed"))) { + !msg.contains("Connection already shutdown"))) { e.printStackTrace(); fail("Incorrect SocketException thrown by client"); throw e; @@ -3547,6 +3550,338 @@ public class WolfSSLSocketTest { } } + /* Close a resource, ignoring exceptions, for deterministic + * per-iteration cleanup in the stress tests below. */ + private static void closeQuietly(java.io.Closeable c) { + if (c != null) { + try { + c.close(); + } catch (Exception e) { + /* ignore */ + } + } + } + + /* Races close() against OutputStream.write() under CPU load, verifying + * write() never uses a WolfSSLSession that close() has freed. Bounded by + * iteration count and wall clock budget to keep runtime predictable on + * slow CI runners, 120 sec timeout is a backstop. */ + @Test(timeout = 120000) + public void testSocketCloseDuringConcurrentWrite() throws Exception { + + int i; + String protocol = null; + final int maxIterations = 300; + final long budgetMs = 5000; + + /* pipe() interrupt mechanism not implemented for Windows yet since + * Windows does not support Unix/Linux pipe(). Re-enable this test + * for Windows when that support has been added */ + Assume.assumeFalse(WolfSSLTestFactory.isWindows()); + + if (WolfSSL.TLSv12Enabled()) { + protocol = "TLSv1.2"; + } else if (WolfSSL.TLSv13Enabled()) { + protocol = "TLSv1.3"; + } + Assume.assumeNotNull(protocol); + + /* create new CTX */ + this.ctx = tf.createSSLContext(protocol, ctxProvider); + + ExecutorService es = Executors.newCachedThreadPool(); + + /* Busy spinners force thread preemption inside write()/close() */ + final AtomicBoolean spinStop = new AtomicBoolean(false); + int numSpin = Runtime.getRuntime().availableProcessors() * 2; + for (i = 0; i < numSpin; i++) { + Thread spin = new Thread(new Runnable() { + @Override + public void run() { + while (!spinStop.get()) { + /* busy loop to create CPU load */ + } + } + }); + spin.setDaemon(true); + spin.start(); + } + + try { + long startMs = System.currentTimeMillis(); + + for (i = 0; (i < maxIterations) && + ((System.currentTimeMillis() - startMs) < budgetMs); + i++) { + + SSLServerSocket ss = null; + Socket plain = null; + SSLSocket cs = null; + SSLSocket server = null; + + try { + ss = (SSLServerSocket)ctx + .getServerSocketFactory().createServerSocket(0); + + /* autoClose false keeps underlying Socket open across + * SSLSocket.close() */ + plain = new Socket(); + plain.connect(new InetSocketAddress("127.0.0.1", + ss.getLocalPort())); + cs = (SSLSocket)ctx.getSocketFactory() + .createSocket(plain, "127.0.0.1", + ss.getLocalPort(), false); + server = (SSLSocket)ss.accept(); + + /* Server thread reads until end of stream or Exception */ + final SSLSocket serverConn = server; + Future serverFuture = es.submit( + new Callable() { + @Override + public Void call() throws Exception { + try { + serverConn.startHandshake(); + byte[] tmp = new byte[8192]; + InputStream in = + serverConn.getInputStream(); + while (in.read(tmp) >= 0) { + /* discard data */ + } + } catch (Exception e) { + /* expected when connection closed */ + } + return null; + } + }); + + cs.startHandshake(); + + final OutputStream out = cs.getOutputStream(); + final Throwable[] writeExc = new Throwable[1]; + + /* Writer thread loops until close() causes Exception */ + Future writeFuture = es.submit( + new Callable() { + @Override + public Void call() throws Exception { + byte[] one = new byte[1]; + try { + while (true) { + out.write(one); + } + } catch (Throwable t) { + writeExc[0] = t; + } + return null; + } + }); + + /* sweep delay 0 - ~490 us to vary close() alignment */ + LockSupport.parkNanos((i % 50) * 10000L); + + /* close socket while writer thread is running */ + cs.close(); + + writeFuture.get(30, TimeUnit.SECONDS); + + /* Unblock the server reader before joining it, so a + * lost close_notify cannot stall the iteration */ + server.close(); + ss.close(); + plain.close(); + serverFuture.get(30, TimeUnit.SECONDS); + + /* fail if write() used a freed WolfSSLSession */ + Throwable t = writeExc[0]; + while (t != null) { + String msg = t.getMessage(); + if (msg != null && msg.contains("has been freed")) { + fail("write() used freed WOLFSSL session, " + + "iteration " + i + ": " + t); + } + t = t.getCause(); + } + } + finally { + closeQuietly(cs); + closeQuietly(server); + closeQuietly(ss); + closeQuietly(plain); + } + } + } + finally { + spinStop.set(true); + /* shutdownNow() interrupts tasks left running on a failure or + * timeout, awaitTermination() bounds the wait */ + es.shutdownNow(); + es.awaitTermination(30, TimeUnit.SECONDS); + } + } + + /* Races close() against InputStream.read() under CPU load, verifying + * read() never uses a WolfSSLSession that close() has freed. Server sends + * one byte at a time so read() returns and loops rapidly through its + * internal state checks. Bounded by iteration count and wall clock budget + * to keep runtime predictable, 120 sec timeout backstop. */ + @Test(timeout = 120000) + public void testSocketCloseDuringConcurrentRead() throws Exception { + + int i; + String protocol = null; + final int maxIterations = 500; + final long budgetMs = 5000; + + /* pipe() interrupt mechanism not implemented for Windows yet since + * Windows does not support Unix/Linux pipe(). Re-enable this test + * for Windows when that support has been added */ + Assume.assumeFalse(WolfSSLTestFactory.isWindows()); + + if (WolfSSL.TLSv12Enabled()) { + protocol = "TLSv1.2"; + } else if (WolfSSL.TLSv13Enabled()) { + protocol = "TLSv1.3"; + } + Assume.assumeNotNull(protocol); + + /* create new CTX */ + this.ctx = tf.createSSLContext(protocol, ctxProvider); + + ExecutorService es = Executors.newCachedThreadPool(); + + /* Busy spinners force thread preemption inside read()/close() */ + final AtomicBoolean spinStop = new AtomicBoolean(false); + int numSpin = Runtime.getRuntime().availableProcessors() * 2; + for (i = 0; i < numSpin; i++) { + Thread spin = new Thread(new Runnable() { + @Override + public void run() { + while (!spinStop.get()) { + /* busy loop to create CPU load */ + } + } + }); + spin.setDaemon(true); + spin.start(); + } + + try { + long startMs = System.currentTimeMillis(); + + for (i = 0; (i < maxIterations) && + ((System.currentTimeMillis() - startMs) < budgetMs); + i++) { + + SSLServerSocket ss = null; + Socket plain = null; + SSLSocket cs = null; + SSLSocket server = null; + + /* Server thread floods data until stopped */ + final AtomicBoolean serverStop = new AtomicBoolean(false); + + try { + ss = (SSLServerSocket)ctx + .getServerSocketFactory().createServerSocket(0); + + /* autoClose false keeps underlying Socket open across + * SSLSocket.close() */ + plain = new Socket(); + plain.connect(new InetSocketAddress("127.0.0.1", + ss.getLocalPort())); + cs = (SSLSocket)ctx.getSocketFactory() + .createSocket(plain, "127.0.0.1", + ss.getLocalPort(), false); + server = (SSLSocket)ss.accept(); + + final SSLSocket serverConn = server; + Future serverFuture = es.submit( + new Callable() { + @Override + public Void call() throws Exception { + try { + serverConn.startHandshake(); + byte[] tmp = new byte[1]; + OutputStream out = + serverConn.getOutputStream(); + while (!serverStop.get()) { + out.write(tmp); + } + } catch (Exception e) { + /* expected when connection closed */ + } + return null; + } + }); + + cs.startHandshake(); + + final InputStream in = cs.getInputStream(); + final Throwable[] readExc = new Throwable[1]; + + /* Reader thread loops until close() causes Exception */ + Future readFuture = es.submit( + new Callable() { + @Override + public Void call() throws Exception { + byte[] buf = new byte[1]; + try { + while (in.read(buf) >= 0) { + /* discard data */ + } + } catch (Throwable t) { + readExc[0] = t; + } + return null; + } + }); + + /* sweep delay 0 - ~490 us to vary close() alignment */ + LockSupport.parkNanos((i % 50) * 10000L); + + /* close socket while reader thread is running */ + cs.close(); + + readFuture.get(30, TimeUnit.SECONDS); + + /* Stop the flooding server thread. Closing the Socket + * breaks TCP so a write() blocked on a full send buffer + * errors out */ + serverStop.set(true); + server.close(); + ss.close(); + plain.close(); + serverFuture.get(30, TimeUnit.SECONDS); + + /* fail if read() used a freed WolfSSLSession */ + Throwable t = readExc[0]; + while (t != null) { + String msg = t.getMessage(); + if (msg != null && msg.contains("has been freed")) { + fail("read() used freed WOLFSSL session, " + + "iteration " + i + ": " + t); + } + t = t.getCause(); + } + } + finally { + serverStop.set(true); + closeQuietly(cs); + closeQuietly(server); + closeQuietly(ss); + closeQuietly(plain); + } + } + } + finally { + spinStop.set(true); + /* shutdownNow() interrupts tasks left running on a failure or + * timeout, awaitTermination() bounds the wait */ + es.shutdownNow(); + es.awaitTermination(30, TimeUnit.SECONDS); + } + } + @Test public void testSocketMethodsAfterClose() throws Exception {