From e088bedd86682856041e89b5eea4b9d934f3ccd4 Mon Sep 17 00:00:00 2001 From: Ruby Martin Date: Thu, 5 Dec 2024 15:38:33 -0700 Subject: [PATCH 1/7] JSSE: return cached SNI request on Server side --- .../jsse/WolfSSLImplementSSLSession.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java index 1496bcda..94603d12 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java @@ -927,7 +927,7 @@ public String[] getPeerSupportedSignatureAlgorithms() { * Return a list of all SNI server names of the requested Server Name * Indication (SNI) extension. * - * @return non-null immutable List of SNIServerNames. List may be emtpy + * @return non-null immutable List of SNIServerNames. List may be empty * if no SNI names were requested. */ @Override @@ -942,7 +942,20 @@ public synchronized List getRequestedServerNames() } try { - sniRequestArr = this.ssl.getClientSNIRequest(); + /* Return SNI name saved by Client + * or return cached request name from Server + * Currently WolfSSL.WOLFSSL_SNI_HOST_NAME is the only + * supported type */ + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + "calling getRequestedServerNames (" + + this.getSideString() + ")"); + if (this.ssl.getSide() == WolfSSL.WOLFSSL_CLIENT_END){ + sniRequestArr = this.ssl.getClientSNIRequest(); + } else { + sniRequestArr = this.ssl.getSNIRequest((byte)WolfSSL. + WOLFSSL_SNI_HOST_NAME).getBytes(); + } + if (sniRequestArr != null) { SNIHostName sniName = new SNIHostName(sniRequestArr); sniNames.add(sniName); From 67f6fc6aef15f979cba5c324071e145aa26e8a42 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Wed, 13 Nov 2024 11:30:23 -0700 Subject: [PATCH 2/7] JSSE: skip setting ssl to null in Input/OutputStream.close() in case other thread is blocked in ssl.read/write() on select/poll() --- src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index 846d444f..e479507b 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -2443,7 +2443,6 @@ protected void close(boolean closeSocket) throws IOException { } this.socket = null; - this.ssl = null; this.isClosed = true; /* Reset "is closing" state to false, now closed */ @@ -2663,7 +2662,6 @@ protected void close(boolean closeSocket) throws IOException { } this.socket = null; - this.ssl = null; this.isClosed = true; /* Reset "is closing" state to false, now closed */ From b42087ad871aff7dca0379d33594dc56d1a43445 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Tue, 19 Nov 2024 10:03:35 -0700 Subject: [PATCH 3/7] JSSE: skip setting socket to null in Input/OutputStream.close(), so read/write() can detect closure --- src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index e479507b..8a99be1a 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -2442,7 +2442,6 @@ protected void close(boolean closeSocket) throws IOException { } } - this.socket = null; this.isClosed = true; /* Reset "is closing" state to false, now closed */ @@ -2661,7 +2660,6 @@ protected void close(boolean closeSocket) throws IOException { } } - this.socket = null; this.isClosed = true; /* Reset "is closing" state to false, now closed */ From 2da75060c5fb3b3aa5afb77fc27c07181469da23 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 22 Nov 2024 15:25:27 -0700 Subject: [PATCH 4/7] JSSE: fixes for calling SSLSocket methods after SSLSocket.close() has been called --- src/java/com/wolfssl/WolfSSLDebug.java | 2 + .../wolfssl/provider/jsse/WolfSSLEngine.java | 4 +- .../provider/jsse/WolfSSLEngineHelper.java | 4 +- .../wolfssl/provider/jsse/WolfSSLSocket.java | 91 ++++++- .../provider/jsse/test/WolfSSLSocketTest.java | 231 ++++++++++++++++++ .../com/wolfssl/test/WolfSSLSessionTest.java | 6 +- 6 files changed, 323 insertions(+), 15 deletions(-) diff --git a/src/java/com/wolfssl/WolfSSLDebug.java b/src/java/com/wolfssl/WolfSSLDebug.java index 1dc4eb85..1645cd3f 100644 --- a/src/java/com/wolfssl/WolfSSLDebug.java +++ b/src/java/com/wolfssl/WolfSSLDebug.java @@ -65,7 +65,9 @@ public class WolfSSLDebug { * Will be used to determine what string gets put into log messages. */ public enum Component { + /** wolfSSL JNI component */ JNI("wolfJNI"), + /** wolfSSL JSSE component */ JSSE("wolfJSSE"); private final String componentString; diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java index 57d08924..368a3dd5 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java @@ -1394,7 +1394,7 @@ public synchronized boolean isOutboundDone() { public String[] getSupportedCipherSuites() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getSupportedCipherSuites()"); - return this.engineHelper.getAllCiphers(); + return WolfSSLEngineHelper.getAllCiphers(); } @Override @@ -1415,7 +1415,7 @@ public synchronized void setEnabledCipherSuites(String[] suites) { public String[] getSupportedProtocols() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getSupportedProtocols()"); - return this.engineHelper.getAllProtocols(); + return WolfSSLEngineHelper.getAllProtocols(); } @Override diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java index 640f36f4..1b979581 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java @@ -454,7 +454,7 @@ protected synchronized WolfSSLImplementSSLSession getSession() { * * @return String array of all supported cipher suites */ - protected synchronized String[] getAllCiphers() { + protected static synchronized String[] getAllCiphers() { return WolfSSLUtil.sanitizeSuites(WolfSSL.getCiphersIana()); } @@ -551,7 +551,7 @@ protected synchronized String[] getProtocols() { * * @return String array of supported protocols */ - protected synchronized String[] getAllProtocols() { + protected static synchronized String[] getAllProtocols() { return WolfSSLUtil.sanitizeProtocols(WolfSSL.getProtocols()); } diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index 8a99be1a..c56adc26 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -112,6 +112,9 @@ public class WolfSSLSocket extends SSLSocket { /** ALPN selector callback, if set */ protected BiFunction, String> alpnSelector = null; + /* true if client, otherwise false */ + private boolean isClientMode = false; + /** * Create new WolfSSLSocket object * @@ -143,6 +146,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -183,6 +187,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, port, host); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -226,6 +231,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, port, address); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -266,6 +272,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, port, host); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -309,6 +316,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, port, host); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -366,6 +374,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, port, host); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -411,6 +420,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, s.getPort(), s.getInetAddress()); EngineHelper.setUseClientMode(clientMode); + this.isClientMode = clientMode; } catch (WolfSSLException e) { throw new IOException(e); @@ -460,6 +470,7 @@ public WolfSSLSocket(com.wolfssl.WolfSSLContext context, EngineHelper = new WolfSSLEngineHelper(this.ssl, this.authStore, this.params, s.getPort(), s.getInetAddress()); EngineHelper.setUseClientMode(false); + this.isClientMode = false; /* register custom receive callback to read consumed first */ if (consumed != null) { @@ -1030,7 +1041,8 @@ public synchronized String[] getSupportedCipherSuites() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getSupportedCipherSuites()"); - return EngineHelper.getAllCiphers(); + /* getAllCiphers() is a static method, calling directly on class */ + return WolfSSLEngineHelper.getAllCiphers(); } /** @@ -1046,6 +1058,10 @@ public synchronized String[] getEnabledCipherSuites() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getEnabledCipherSuites()"); + if (this.isClosed()) { + return null; + } + return EngineHelper.getCiphers(); } @@ -1064,6 +1080,12 @@ public synchronized void setEnabledCipherSuites(String[] suites) WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered setEnabledCipherSuites()"); + if (this.isClosed()) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + "SSLSocket closed, not setting enabled cipher suites"); + return; + } + /* sets cipher suite(s) to be used for connection */ EngineHelper.setCiphers(suites); WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, @@ -1085,6 +1107,11 @@ public synchronized String getApplicationProtocol() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getApplicationProtocol()"); + /* If socket has been closed, return an empty string */ + if (this.isClosed()) { + return ""; + } + return EngineHelper.getAlpnSelectedProtocolString(); } @@ -1252,8 +1279,9 @@ public synchronized String[] getSupportedProtocols() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getSupportedProtocols()"); - /* returns all protocol version supported by native wolfSSL */ - return EngineHelper.getAllProtocols(); + /* returns all protocol version supported by native wolfSSL. + /* getAllProtocols() is a static method, calling directly on class */ + return WolfSSLEngineHelper.getAllProtocols(); } /** @@ -1267,6 +1295,10 @@ public synchronized String[] getEnabledProtocols() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getEnabledProtocols()"); + if (this.isClosed()) { + return null; + } + /* returns protocols versions enabled for this session */ return EngineHelper.getProtocols(); } @@ -1286,6 +1318,12 @@ public synchronized void setEnabledProtocols(String[] protocols) WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered setEnabledProtocols()"); + if (this.isClosed()) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + "SSLSocket closed, not setting enabled protocols"); + return; + } + /* sets protocol versions to be enabled for use with this session */ EngineHelper.setProtocols(protocols); WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, @@ -1337,6 +1375,15 @@ public synchronized SSLSession getSession() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getSession()"); + if (this.isClosed()) { + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, + "SSLSocket has been closed, returning invalid session"); + + /* return invalid session object with cipher suite + * "SSL_NULL_WITH_NULL_NULL" */ + return new WolfSSLImplementSSLSession(this.authStore); + } + try { /* try to do handshake if not completed yet, * handles synchronization */ @@ -1380,7 +1427,7 @@ public synchronized SSLSession getHandshakeSession() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getHandshakeSession()"); - if (this.handshakeStarted == false) { + if ((this.handshakeStarted == false) || this.isClosed()) { return null; } @@ -1587,7 +1634,11 @@ public synchronized void setUseClientMode(boolean mode) WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered setUseClientMode()"); - EngineHelper.setUseClientMode(mode); + if (!this.isClosed()) { + EngineHelper.setUseClientMode(mode); + } + this.isClientMode = mode; + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "socket client mode set to: " + mode); } @@ -1603,7 +1654,7 @@ public synchronized boolean getUseClientMode() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getUseClientMode()"); - return EngineHelper.getUseClientMode(); + return this.isClientMode; } /** @@ -1621,7 +1672,9 @@ public synchronized void setNeedClientAuth(boolean need) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered setNeedClientAuth(need: " + String.valueOf(need) + ")"); - EngineHelper.setNeedClientAuth(need); + if (!this.isClosed()) { + EngineHelper.setNeedClientAuth(need); + } } /** @@ -1636,6 +1689,12 @@ public synchronized boolean getNeedClientAuth() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getNeedClientAuth()"); + /* When socket is closed, EngineHelper gets set to null. Since we + * don't cache needClientAuth value, return false after closure. */ + if (this.isClosed()) { + return false; + } + return EngineHelper.getNeedClientAuth(); } @@ -1655,7 +1714,9 @@ public synchronized void setWantClientAuth(boolean want) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered setWantClientAuth(want: " + String.valueOf(want) + ")"); - EngineHelper.setWantClientAuth(want); + if (!this.isClosed()) { + EngineHelper.setWantClientAuth(want); + } } /** @@ -1674,6 +1735,12 @@ public synchronized boolean getWantClientAuth() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getWantClientAuth()"); + /* When socket is closed, EngineHelper gets set to null. Since we + * don't cache wantClientAuth value, return false after closure. */ + if (this.isClosed()) { + return false; + } + return EngineHelper.getWantClientAuth(); } @@ -1692,7 +1759,9 @@ public synchronized void setEnableSessionCreation(boolean flag) { "entered setEnableSessionCreation(flag: " + String.valueOf(flag) + ")"); - EngineHelper.setEnableSessionCreation(flag); + if (!this.isClosed()) { + EngineHelper.setEnableSessionCreation(flag); + } } /** @@ -1706,6 +1775,10 @@ public synchronized boolean getEnableSessionCreation() { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered getEnableSessionCreation()"); + if (this.isClosed()) { + return false; + } + return EngineHelper.getEnableSessionCreation(); } diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java index 938e301e..712af895 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSocketTest.java @@ -2746,6 +2746,237 @@ public Void call() throws Exception { System.out.println("\t... passed"); } + @Test + public void testSocketMethodsAfterClose() throws Exception { + + String protocol = null; + + System.out.print("\tTesting methods after close"); + + if (WolfSSL.TLSv12Enabled()) { + protocol = "TLSv1.2"; + } else if (WolfSSL.TLSv11Enabled()) { + protocol = "TLSv1.1"; + } else if (WolfSSL.TLSv1Enabled()) { + protocol = "TLSv1.0"; + } else { + System.out.println("\t... skipped"); + return; + } + + /* create new CTX */ + this.ctx = tf.createSSLContext(protocol, ctxProvider); + + /* create SSLServerSocket first to get ephemeral port */ + SSLServerSocket ss = (SSLServerSocket)ctx.getServerSocketFactory() + .createServerSocket(0); + + SSLSocket cs = (SSLSocket)ctx.getSocketFactory().createSocket(); + cs.connect(new InetSocketAddress(ss.getLocalPort())); + final SSLSocket server = (SSLSocket)ss.accept(); + + ExecutorService es = Executors.newSingleThreadExecutor(); + Future serverFuture = es.submit(new Callable() { + @Override + public Void call() throws Exception { + try { + server.startHandshake(); + + } catch (SSLException e) { + System.out.println("\t... failed"); + fail(); + } + return null; + } + }); + + try { + cs.startHandshake(); + + } catch (SSLHandshakeException e) { + System.out.println("\t... failed"); + fail(); + } + + es.shutdown(); + serverFuture.get(); + cs.close(); + server.close(); + ss.close(); + + /* Test calling public SSLSocket methods after close, make sure + * exception or return value is what we expect. */ + + try { + cs.getApplicationProtocol(); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getApplicationProtocol() exception after close()"); + } + + try { + cs.getEnableSessionCreation(); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getEnableSessionCreation() exception after close()"); + } + + try { + cs.setEnableSessionCreation(true); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setEnableSessionCreation() exception after close()"); + } + + try { + if (cs.getWantClientAuth() != false) { + System.out.println("\t... failed"); + fail("getWantClientAuth() not false after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getWantClientAuth() exception after close()"); + } + + try { + cs.setWantClientAuth(true); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setWantClientAuth() exception after close()"); + } + + try { + if (cs.getNeedClientAuth() != false) { + System.out.println("\t... failed"); + fail("getNeedClientAuth() not false after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getNeedClientAuth() exception after close()"); + } + + try { + cs.setNeedClientAuth(true); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setNeedClientAuth() exception after close()"); + } + + try { + if (cs.getUseClientMode() != true) { + System.out.println("\t... failed"); + fail("getUseClientMode() on client not true after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getUseClientMode() exception after close()"); + } + + try { + cs.setUseClientMode(true); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setUseClientMode() exception after close()"); + } + + try { + if (cs.getHandshakeSession() != null) { + System.out.println("\t... failed"); + fail("getHandshakeSession() not null after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getHandshakeSession() exception after close()"); + } + + try { + SSLSession closeSess = cs.getSession(); + if (closeSess == null || + !closeSess.getCipherSuite().equals("SSL_NULL_WITH_NULL_NULL")) { + System.out.println("\t... failed"); + fail("getSession() null or wrong cipher suite after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getSession() exception after close()"); + } + + try { + if (cs.getEnabledProtocols() != null) { + System.out.println("\t... failed"); + fail("getEnabledProtocols() not null after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getEnabledProtocols() exception after close()"); + } + + try { + cs.setEnabledProtocols(new String[] {"INVALID"}); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setEnabledProtocols() exception after close()"); + } + + try { + cs.setEnabledCipherSuites(new String[] {"INVALID"}); + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("setEnabledCipherSuites() exception after close()"); + } + + try { + String[] suppProtos = cs.getSupportedProtocols(); + if (suppProtos == null || suppProtos.length == 0) { + System.out.println("\t... failed"); + fail("getSupportedProtocols() null or empty after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getSupportedProtocols() exception after close()"); + } + + try { + String[] suppSuites = cs.getSupportedCipherSuites(); + if (suppSuites == null || suppSuites.length == 0) { + System.out.println("\t... failed"); + fail("getSupportedCipherSuites() null or empty after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getSupportedCipherSuites() exception after close()"); + } + + try { + if (cs.getEnabledCipherSuites() != null) { + System.out.println("\t... failed"); + fail("getEnabledCipherSuites() not null after close()"); + } + } catch (Exception e) { + /* should not throw exception */ + System.out.println("\t... failed"); + fail("getEnabledCipherSuites() exception after close()"); + } + + System.out.println("\t... passed"); + } + /** * Inner class used to hold configuration options for * TestServer and TestClient classes. diff --git a/src/test/com/wolfssl/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/test/WolfSSLSessionTest.java index 597ecaf2..966a4917 100644 --- a/src/test/com/wolfssl/test/WolfSSLSessionTest.java +++ b/src/test/com/wolfssl/test/WolfSSLSessionTest.java @@ -1154,11 +1154,13 @@ public Void call() throws Exception { } if (!debugOutput.contains("connect() ret: 1")) { System.out.println("\t... failed"); - fail("Debug output did not contain connect() success"); + fail("Debug output did not contain connect() success:\n" + + debugOutput); } if (!debugOutput.contains("accept() ret: 1")) { System.out.println("\t... failed"); - fail("Debug output did not contain accept() success"); + fail("Debug output did not contain accept() success:\n" + + debugOutput); } } From 1d7a664470333d370ac5f0d724d2b1b1444c7a8f Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Fri, 22 Nov 2024 17:16:26 -0700 Subject: [PATCH 5/7] JSSE: correct reset of closing state in WolfSSLInputStream/OutputStream --- src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java index c56adc26..d6eea669 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLSocket.java @@ -2501,6 +2501,8 @@ protected void close(boolean closeSocket) throws IOException { if (closeSocket) { if (this.socket == null || this.isClosed) { + /* Reset "is closing" state to false and return */ + isClosing.set(false); return; } @@ -2719,6 +2721,8 @@ protected void close(boolean closeSocket) throws IOException { if (closeSocket) { if (this.socket == null || this.isClosed) { + /* Reset "is closing" state to false and return */ + isClosing.set(false); return; } From 2fafa935450587318dcdaa61b041122e9b13e48c Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Mon, 25 Nov 2024 15:02:59 -0700 Subject: [PATCH 6/7] JSSE: add SSLSocket.getSession() tests throughout different times of connection establishment --- .../provider/jsse/WolfSSLAuthStore.java | 10 +- .../provider/jsse/WolfSSLEngineHelper.java | 9 +- .../jsse/WolfSSLImplementSSLSession.java | 18 +- .../jsse/test/WolfSSLSessionTest.java | 182 +++++++++++++++++- .../com/wolfssl/test/WolfSSLSessionTest.java | 13 ++ 5 files changed, 215 insertions(+), 17 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java index ace3d528..c1def31f 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLAuthStore.java @@ -326,7 +326,7 @@ protected synchronized WolfSSLImplementSSLSession getSession( /* Return new session if in server mode, or if host is null */ if (!clientMode || host == null) { - return this.getSession(ssl, clientMode); + return this.getSession(ssl, clientMode, host, port); } WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, @@ -527,18 +527,22 @@ private void printSessionStoreStatus() { } /** Returns a new session, does not check/save for resumption + * * @param ssl WOLFSSL class to reference with new session * @param clientMode true if on client side, false if server + * @param host hostname of peer, or null if not available + * @param port port of peer + * * @return a new SSLSession on success */ protected synchronized WolfSSLImplementSSLSession getSession( - WolfSSLSession ssl, boolean clientMode) { + WolfSSLSession ssl, boolean clientMode, String host, int port) { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "creating new session"); WolfSSLImplementSSLSession ses = - new WolfSSLImplementSSLSession(ssl, this); + new WolfSSLImplementSSLSession(ssl, this, host, port); ses.setValid(true); ses.isFromTable = false; diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java index 1b979581..5a31cf8a 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngineHelper.java @@ -1262,6 +1262,7 @@ protected synchronized int doHandshake(int isSSLEngine, int timeout) int ret, err; byte[] serverId = null; String hostAddress = null; + String sessCacheHostname = this.hostname; if (!modeSet) { throw new SSLException("setUseClientMode has not been called"); @@ -1293,7 +1294,13 @@ protected synchronized int doHandshake(int isSSLEngine, int timeout) return WolfSSL.SSL_HANDSHAKE_FAILURE; } - this.session = this.authStore.getSession(ssl, this.clientMode); + + if (sessCacheHostname == null && this.peerAddr != null) { + sessCacheHostname = this.peerAddr.getHostAddress(); + } + + this.session = this.authStore.getSession(ssl, this.clientMode, + sessCacheHostname, this.port); } if (this.clientMode) { diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java index 94603d12..6880c289 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLImplementSSLSession.java @@ -147,12 +147,14 @@ public WolfSSLImplementSSLSession (WolfSSLSession in, int port, String host, * * @param in WolfSSLSession to be used with this object * @param params WolfSSLAuthStore for this session + * @param host hostname of peer, or null if not available + * @param port port of peer */ public WolfSSLImplementSSLSession (WolfSSLSession in, - WolfSSLAuthStore params) { + WolfSSLAuthStore params, + String host, int port) { this.ssl = in; - this.port = -1; - this.host = null; + this.host = host; this.authStore = params; this.valid = false; /* flag if joining or resuming session is allowed */ this.peerCerts = null; @@ -163,6 +165,12 @@ public WolfSSLImplementSSLSession (WolfSSLSession in, creation = new Date(); accessed = new Date(); + if (port > 0) { + this.port = port; + } else { + this.port = -1; + } + WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "created new session (no host/port yet)"); } @@ -478,7 +486,9 @@ public synchronized Certificate[] getPeerCertificates() X509Certificate exportCert; if (ssl == null) { - throw new SSLPeerUnverifiedException("handshake not complete"); + throw new SSLPeerUnverifiedException( + "internal WolfSSLSession null, handshake not complete or " + + "SSLSocket/Engine closed"); } try { diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSessionTest.java index 5d10de34..84ba4b91 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLSessionTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLSessionTest.java @@ -24,6 +24,13 @@ import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import java.util.concurrent.Callable; +import java.util.concurrent.Future; +import java.util.concurrent.Executors; +import java.util.concurrent.ExecutorService; + +import java.io.InputStream; +import java.io.OutputStream; import java.io.IOException; import java.security.cert.Certificate; import java.security.cert.X509Certificate; @@ -38,20 +45,25 @@ import java.security.cert.CertificateException; import java.util.ArrayList; +import java.net.InetSocketAddress; import javax.net.ssl.SSLContext; import javax.net.ssl.SSLEngine; -import javax.net.ssl.SSLException; -import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLSocket; +import javax.net.ssl.SSLServerSocket; import javax.net.ssl.SSLSession; import javax.net.ssl.SSLSessionBindingEvent; import javax.net.ssl.SSLSessionBindingListener; import javax.net.ssl.SSLSessionContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLPeerUnverifiedException; +import javax.net.ssl.SSLHandshakeException; import org.junit.BeforeClass; import org.junit.Test; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.fail; +import com.wolfssl.WolfSSL; import com.wolfssl.WolfSSLContext; import com.wolfssl.WolfSSLException; import com.wolfssl.provider.jsse.WolfSSLProvider; @@ -63,7 +75,7 @@ public class WolfSSLSessionTest { @BeforeClass public static void testProviderInstallationAtRuntime() - throws NoSuchProviderException { + throws NoSuchProviderException, WolfSSLException { System.out.println("WolfSSLImplementSSLSession Class"); @@ -73,12 +85,8 @@ public static void testProviderInstallationAtRuntime() Provider p = Security.getProvider("wolfJSSE"); assertNotNull(p); - try { - tf = new WolfSSLTestFactory(); - } catch (WolfSSLException e) { - // TODO Auto-generated catch block - e.printStackTrace(); - } + /* Can throw WolfSSLException on error */ + tf = new WolfSSLTestFactory(); } @@ -435,6 +443,162 @@ public void testSessionContext() pass("\t\t... passed"); } + @Test + public void testGetSessionInSocketConnection() throws Exception { + + String protocol = null; + SSLContext ctx = null; + + System.out.print("\tTesting SSLSocket.getSession"); + + if (WolfSSL.TLSv12Enabled()) { + protocol = "TLSv1.2"; + } else if (WolfSSL.TLSv11Enabled()) { + protocol = "TLSv1.1"; + } else if (WolfSSL.TLSv1Enabled()) { + protocol = "TLSv1.0"; + } else { + System.out.println("\t... skipped"); + return; + } + + /* create new CTX */ + ctx = tf.createSSLContext(protocol, engineProvider); + + /* create SSLServerSocket first to get ephemeral port */ + SSLServerSocket ss = (SSLServerSocket)ctx.getServerSocketFactory() + .createServerSocket(0); + + SSLSocket cs = (SSLSocket)ctx.getSocketFactory().createSocket(); + cs.connect(new InetSocketAddress(ss.getLocalPort())); + final SSLSocket server = (SSLSocket)ss.accept(); + server.setNeedClientAuth(true); + + ExecutorService es = Executors.newSingleThreadExecutor(); + Future serverFuture = es.submit(new Callable() { + @Override + public Void call() throws Exception { + try { + testSSLSession(server, false); + server.startHandshake(); + testSSLSession(server, true); + + } catch (SSLException e) { + error("\t... failed"); + fail(); + } + return null; + } + }); + + try { + testSSLSession(cs, false); + cs.startHandshake(); + testSSLSession(cs, true); + + } catch (SSLHandshakeException e) { + error("\t... failed"); + fail(); + } + + es.shutdown(); + serverFuture.get(); + testSSLSession(cs, true); + cs.close(); + testSSLSession(cs, true); + testSSLSession(server, true); + server.close(); + testSSLSession(server, true); + ss.close(); + + pass("\t... passed"); + } + + /** + * Test SSLSocket.getSession() and calling methods on the + * SSLSession retrieved. */ + private void testSSLSession(SSLSocket sock, boolean handshakeDone) + throws Exception { + + int ret; + String val; + Certificate[] certs; + byte[] id; + SSLSession session; + + if (sock == null) { + throw new Exception("SSLSocket was null in testSSLSession"); + } + + session = sock.getSession(); + if (session == null) { + throw new Exception("SSLSocket.getSession() returned null"); + } + + val = session.getCipherSuite(); + if (val == null || val.isEmpty()) { + throw new Exception( + "SSLSession.getCipherSuite() was null or empty"); + } + + val = session.getProtocol(); + if (val == null || val.isEmpty()) { + throw new Exception( + "SSLSession.getProtocol() was null or empty"); + } + + val = session.getPeerHost(); + if (handshakeDone && !sock.isClosed() && + (val == null || val.isEmpty())) { + throw new Exception( + "SSLSession.getPeerHost() was null or empty"); + } + + ret = session.getPeerPort(); + if (ret == 0) { + throw new Exception("SSLSession.getPeerPort() was 0"); + } + + certs = session.getLocalCertificates(); + if (certs == null || certs.length == 0) { + throw new Exception( + "SSLSession.getLocalCertificates() was null or 0 length"); + } + + try { + certs = session.getPeerCertificates(); + if (handshakeDone && (certs == null || certs.length == 0)) { + throw new Exception( + "SSLSession.getPeerCertificates was null or 0 length"); + } + } catch (SSLPeerUnverifiedException e) { + if (handshakeDone && !sock.isClosed()) { + throw new Exception( + "SSLSession.getPeerCertificates threw " + + "SSLPeerUnverifiedException when handshake was done: " + e); + } + } + + id = session.getId(); + if (!sock.isClosed() && (id == null || id.length == 0)) { + throw new Exception("SSLSession.getId() was null or 0 length"); + } + + if (!sock.isClosed() && !session.isValid()) { + throw new Exception("SSLSession.isValid() is false"); + } + + ret = session.getPacketBufferSize(); + if (ret == 0) { + throw new Exception("SSLSession.getPacketBufferSize() is 0"); + } + + ret = session.getApplicationBufferSize(); + if (ret == 0) { + throw new Exception("SSLSession.getApplicationBufferSize() is 0"); + } + } + private void pass(String msg) { WolfSSLTestFactory.pass(msg); diff --git a/src/test/com/wolfssl/test/WolfSSLSessionTest.java b/src/test/com/wolfssl/test/WolfSSLSessionTest.java index 966a4917..2ebaf869 100644 --- a/src/test/com/wolfssl/test/WolfSSLSessionTest.java +++ b/src/test/com/wolfssl/test/WolfSSLSessionTest.java @@ -41,6 +41,8 @@ import java.util.concurrent.Executors; import java.util.concurrent.Future; import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.CountDownLatch; import com.wolfssl.WolfSSL; import com.wolfssl.WolfSSLDebug; @@ -991,6 +993,11 @@ public void test_WolfSSLSession_connectionWithDebug() throws Exception { final WolfSSLContext srvCtx; WolfSSLContext cliCtx; + /* Latch used to wait for server to finish handshake before + * test shuts down. Otherwise, we will sometimes miss debug + * messages from the server side. */ + final CountDownLatch latch = new CountDownLatch(1); + System.out.print("\tTesting wolfssljni.debug"); /* Save original property value, then enable debug. Make sure @@ -1063,6 +1070,8 @@ public Void call() throws Exception { if (server != null) { server.close(); } + + latch.countDown(); } return null; @@ -1129,6 +1138,10 @@ public Void call() throws Exception { } } finally { + + /* Wait for server to finish processing */ + latch.await(10, TimeUnit.SECONDS); + /* Restore original property value */ if (originalProp == null || originalProp.isEmpty()) { System.setProperty("wolfssljni.debug", ""); From 64584fd604e31b12d549e96edac182c76e9f2ad7 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Wed, 27 Nov 2024 15:29:40 -0700 Subject: [PATCH 7/7] JNI: always call wolfSSL_get1_session() inside native JNI getSession(), callers expect to always free returned pointer --- .github/workflows/main.yml | 19 +++++++ native/com_wolfssl_WolfSSLSession.c | 25 ++++---- src/java/com/wolfssl/WolfSSLSession.java | 72 ++++++++++++++++-------- 3 files changed, 78 insertions(+), 38 deletions(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 90c900d3..64236be0 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -101,6 +101,25 @@ jobs: jdk_version: ${{ matrix.jdk_version }} wolfssl_configure: ${{ matrix.wolfssl_configure }} + # -------------------- session cache variant sanity checks ----------------------- + # Only check one Linux and Mac JDK version as a sanity check. + # Using Zulu, but this can be expanded if needed. + linux-zulu-sesscache: + strategy: + matrix: + os: [ 'ubuntu-latest', 'macos-latest' ] + jdk_version: [ '11' ] + wolfssl_configure: [ + '--enable-jni CFLAGS=-DNO_SESSION_CACHE_REF', + ] + name: ${{ matrix.os }} (Zulu JDK ${{ matrix.jdk_version }}, ${{ matrix.wolfssl_configure}}) + uses: ./.github/workflows/linux-common.yml + with: + os: ${{ matrix.os }} + jdk_distro: "zulu" + jdk_version: ${{ matrix.jdk_version }} + wolfssl_configure: ${{ matrix.wolfssl_configure }} + # ------------------ Facebook Infer static analysis ------------------- # Run Facebook infer over PR code, only running on Linux with one # JDK/version for now. diff --git a/native/com_wolfssl_WolfSSLSession.c b/native/com_wolfssl_WolfSSLSession.c index a697ad3e..ed9d909e 100644 --- a/native/com_wolfssl_WolfSSLSession.c +++ b/native/com_wolfssl_WolfSSLSession.c @@ -1644,23 +1644,18 @@ JNIEXPORT jlong JNICALL Java_com_wolfssl_WolfSSLSession_get1Session } } } while (err == SSL_ERROR_WANT_READ); - - sess = wolfSSL_get_session(ssl); - hasTicket = wolfSSL_SESSION_has_ticket((const WOLFSSL_SESSION*)sess); } - /* Only duplicate / save session if not TLS 1.3 (will be using normal - * session IDs), or is TLS 1.3 and we have a session ticket */ - if (version != TLS1_3_VERSION || hasTicket == 1) { - - /* wolfSSL checks ssl for NULL, returns pointer to new WOLFSSL_SESSION, - * Returns new duplicated WOLFSSL_SESSION. Needs to be freed with - * wolfSSL_SESSION_free() when finished with pointer. */ - if (sess != NULL) { - /* Guarantee that we own the WOLFSSL_SESSION, make a copy */ - dup = wolfSSL_SESSION_dup(sess); - } - } + /* Call wolfSSL_get1_session() to increase the ref count of the internal + * WOLFSSL_SESSION struct. This is needed in all build option cases, + * since Java callers of this function expect to explicitly free this + * pointer when finished with use. In some build cases, for example + * NO_CLIENT_CACHE or NO_SESSION_CACHE_REF, the poiner returned by + * wolfSSL_get_session() will be a pointer into the WOLFSSL struct, which + * will be freed with wolfSSL_free(). This can cause issues if the Java + * app expects to hold a valid session pointer for resumption and free + * later on. */ + dup = wolfSSL_get1_session(ssl); if (wc_UnLockMutex(jniSessLock) != 0) { printf("Failed to unlock jniSessLock in get1Session()"); diff --git a/src/java/com/wolfssl/WolfSSLSession.java b/src/java/com/wolfssl/WolfSSLSession.java index d8010c4a..fdeed8a9 100644 --- a/src/java/com/wolfssl/WolfSSLSession.java +++ b/src/java/com/wolfssl/WolfSSLSession.java @@ -1358,15 +1358,25 @@ public int getError(int ret) throws IllegalStateException { } /** - * Sets the session to be used when the SSL object is used to create - * a SSL/TLS connection. - * For session resumption, before calling shutdownSSL() - * with your session object, an application should save the session ID - * from the object with a call to getSession(), which returns - * a pointer to the session. Later, the application should create a new - * SSL session object and assign the saved session with - * setSession(). At this point, the application may call - * connect() and wolfSSL will try to resume the session. + * Sets the session (native WOLFSSL_SESSION) to be used with this object + * for session resumption. + * + * The native WOLFSSL_SESSION pointed to contains all the necessary + * information required to perform a session resumption and reestablishment + * of the connection without a new handshake. + *

+ * To do session resumption, before calling shutdownSSL() + * with your WolfSSLSession object, save the internal session state by + * calling getSession(), which returns a pointer to the + * native WOLFSSL_SESSION session structure. Later, when the application + * is ready to resume a session, it should create a new WolfSSLSession + * object and assign the previously-saved session pointer by passing it + * to the setSession(long session) method. This should be + * done before the handshake is started for the second/resumed time. After + * calling setSession(long session), the application may call + * connect() and wolfSSL will try to resume the session. If + * the session cannot be resumed, a new fresh handshake will be + * established. * * @param session pointer to the native WOLFSSL_SESSION structure used * to set the session for the SSL session object. @@ -1411,25 +1421,35 @@ public int setSession(long session) throws IllegalStateException { } /** - * Returns a pointer to the current session used in the given SSL object. + * Returns a pointer to the current session (native WOLFSSL_SESSION) + * associated with this object, or null if not available. + * * The native WOLFSSL_SESSION pointed to contains all the necessary * information required to perform a session resumption and reestablishment - * the connection without a new handshake. + * of the connection without a new handshake. + *

+ * To do session resumption, before calling shutdownSSL() + * with your WolfSSLSession object, save the internal session state by + * calling getSession(), which returns a pointer to the + * native WOLFSSL_SESSION session structure. Later, when the application + * is ready to resume a session, it should create a new WolfSSLSession + * object and assign the previously-saved session pointer by passing it + * to the setSession(long session) method. This should be + * done before the handshake is started for the second/resumed time. After + * calling setSession(long session), the application may call + * connect() and wolfSSL will try to resume the session. If + * the session cannot be resumed, a new fresh handshake will be + * established. + *

+ * IMPORTANT: *

- * For session resumption, before calling shutdownSSL() - * with your session object, an application should save the session ID - * from the object with a call to getSession(), which returns - * a pointer to the session. Later, the application should create a new - * SSL object and assign the saved session with setSession. - * At this point, the application may call connect() and - * wolfSSL will try to resume the session. - * * The pointer (WOLFSSL_SESSION) returned by this method needs to be freed - * when the application is finished with it, by calling - * freeSession(long). This will release the underlying - * native memory associated with this WOLFSSL_SESSION. + * when the application is finished with it by calling + * freeSession(long session). This will release the underlying + * native memory associated with this WOLFSSL_SESSION. Failing to free + * the session will result in a memory leak. * - * @throws IllegalStateException WolfSSLContext has been freed + * @throws IllegalStateException this WolfSSLSession has been freed * @return a pointer to the current SSL session object on success. * null if ssl is null, * the SSL session cache is disabled, wolfSSL doesn't have @@ -1446,6 +1466,12 @@ public long getSession() throws IllegalStateException { WolfSSLDebug.log(getClass(), WolfSSLDebug.Component.JNI, WolfSSLDebug.INFO, this.sslPtr, "entered getSession()"); + /* Calling get1Session() here as an indication that the native + * JNI level should always return a session pointer that needs + * to be freed by the application. This behavior can change in + * native wolfSSL depending on build options + * (ex: NO_SESSION_CACHE_REF), so JNI layer here will make that + * behavior consistent to the JNI/JSSE callers. */ sessPtr = get1Session(this.sslPtr); WolfSSLDebug.log(getClass(), WolfSSLDebug.Component.JNI,