From 267a8d2c2453b65a9e284edf6886adf463610960 Mon Sep 17 00:00:00 2001 From: Chris Conlon Date: Thu, 30 May 2024 15:32:19 -0600 Subject: [PATCH] SSLEngine: throw SSLException if beginHandshake() called twice by user, renegotiation not supported --- .../wolfssl/provider/jsse/WolfSSLEngine.java | 53 ++++++++++++++++++- .../provider/jsse/test/WolfSSLEngineTest.java | 41 +++++++++++++- 2 files changed, 92 insertions(+), 2 deletions(-) diff --git a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java index 5523cd0..9d55ff8 100644 --- a/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java +++ b/src/java/com/wolfssl/provider/jsse/WolfSSLEngine.java @@ -83,6 +83,9 @@ public class WolfSSLEngine extends SSLEngine { /* closed completely (post shutdown or before handshake) */ private boolean closed = true; + /* handshake started explicitly by user with beginHandshake() */ + private boolean handshakeStartedExplicitly = false; + /* handshake completed */ private boolean handshakeFinished = false; @@ -1395,18 +1398,62 @@ public class WolfSSLEngine extends SSLEngine { return this.engineHelper.getSession(); } + /** + * Explicitly start the SSL/TLS handshake. + * + * This method does not block until handshake is completed, unlike + * the SSLSocket.startHandshake() method. + * + * The handshake may be started implicitly by a call to wrap() or unwrap(), + * if those methods are called and the handshake is not done yet. In that + * case, the user has not explicitly started the handshake themselves + * and may inadvertently call beginHandshake() unknowing that the handshake + * has already started. For that case, we should just return without + * error/exception, since the user is just trying to start the first + * initial handshake. + * + * beginHandshake() may also be called again to initiate renegotiation. + * wolfJSSE does not support renegotiation inside SSLEngine yet. In that + * case, we throw an SSLException to notify callers renegotiation is not + * supported. + * + * @throws SSLException if a problem was encountered while initiating + * a SSL/TLS handshake on this SSLEngine. + * @throws IllegalStateException if the client/server mode has not yet + * been set. + */ @Override public synchronized void beginHandshake() throws SSLException { WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO, "entered beginHandshake()"); + if (!this.clientModeSet) { + throw new IllegalStateException( + "setUseClientMode() has not been called on this SSLEngine"); + } + + if (this.handshakeStartedExplicitly) { + /* Renegotiation (thus calling beginHandshake() multiple times) + * is not supported in wolfJSSE SSLEngine implementation yet. If + * already called once by user, throw SSLException. */ + throw new SSLException("Renegotiation not supported"); + } + else if (!this.needInit && !this.handshakeFinished) { + /* Handshake has started implicitly by wrap() or unwrap(). Simply + * return since this is the first time that the user has called + * beginHandshake() themselves. */ + this.handshakeStartedExplicitly = true; + return; + } + /* No network data source yet */ synchronized (netDataLock) { this.netData = null; } if (outBoundOpen == false) { - throw new SSLException("beginHandshake with closed out bound"); + throw new SSLException( + "beginHandshake() called but outbound closed"); } try { @@ -1428,6 +1475,10 @@ public class WolfSSLEngine extends SSLEngine { int ret = this.engineHelper.doHandshake(1, 0); SetHandshakeStatus(ret); + /* Mark that the user has explicitly started the handshake + * on this SSLEngine by calling beginHandshake() */ + this.handshakeStartedExplicitly = true; + } catch (SocketTimeoutException e) { e.printStackTrace(); throw new SSLException(e); diff --git a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java index 896211a..fd72584 100644 --- a/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java +++ b/src/test/com/wolfssl/provider/jsse/test/WolfSSLEngineTest.java @@ -300,7 +300,8 @@ public class WolfSSLEngineTest { @Test public void testBeginHandshake() - throws NoSuchProviderException, NoSuchAlgorithmException { + throws NoSuchProviderException, NoSuchAlgorithmException, + SSLException { SSLEngine server; SSLEngine client; int ret; @@ -312,6 +313,26 @@ public class WolfSSLEngineTest { server = this.ctx.createSSLEngine(); client = this.ctx.createSSLEngine("wolfSSL begin handshake test", 11111); + /* Calling beginHandshake() before setUseClientMode() should throw + * IllegalStateException */ + try { + server.beginHandshake(); + error("\t\t... failed"); + fail("beginHandshake() before setUseClientMode() should throw " + + "IllegalStateException"); + } catch (IllegalStateException e) { + /* expected */ + } + try { + client.beginHandshake(); + error("\t\t... failed"); + fail("beginHandshake() before setUseClientMode() should throw " + + "IllegalStateException"); + } catch (IllegalStateException e) { + /* expected */ + } + + /* Set client/server mode, disable auth to simplify tests below */ server.setUseClientMode(false); server.setNeedClientAuth(false); client.setUseClientMode(true); @@ -329,6 +350,24 @@ public class WolfSSLEngineTest { error("\t\t... failed"); fail("failed to create engine"); } + + /* Calling beginHandshake() again should throw SSLException + * since renegotiation is not yet supported in wolfJSSE */ + try { + server.beginHandshake(); + error("\t\t... failed"); + fail("beginHandshake() called again should throw SSLException"); + } catch (SSLException e) { + /* expected */ + } + try { + client.beginHandshake(); + error("\t\t... failed"); + fail("beginHandshake() called again should throw SSLException"); + } catch (SSLException e) { + /* expected */ + } + pass("\t\t... passed"); }