Merge pull request #389 from cconlon/fenrirJuly22_2
Fixes for WolfSSLSocket handshake coordination, cert request locking, example and build hardeningpull/390/head
commit
f0faf42441
5
Makefile
5
Makefile
|
|
@ -1,5 +1,5 @@
|
|||
NAME = wolfssl-jni-jsse
|
||||
VERSION = $(shell grep 'name="implementation.version"' build.xml | sed -re 's/.*value="(.+)".*/\1/')
|
||||
VERSION = $(shell grep 'name="implementation.version"' build.xml | sed -nre 's/.*value="([0-9A-Za-z._-]+)".*/\1/p')
|
||||
DIST_FILES = build.xml COPYING examples IDE java.sh LICENSING Makefile native platform \
|
||||
README.md rpm src
|
||||
|
||||
|
|
@ -151,6 +151,9 @@ uninstall:
|
|||
rm -f $(INSTALL_DIR)/$(LIBDIR)/wolfssl-jsse.jar
|
||||
|
||||
dist:
|
||||
@test -n "$(VERSION)" || { \
|
||||
echo "ERROR: could not extract VERSION from build.xml" >&2 ; \
|
||||
exit 1 ; }
|
||||
@mkdir -p "$(NAME)-$(VERSION)"
|
||||
@cp -pr $(DIST_FILES) "$(NAME)-$(VERSION)"
|
||||
tar -zcf "$(NAME)-$(VERSION).tar.gz" "$(NAME)-$(VERSION)"
|
||||
|
|
|
|||
|
|
@ -446,6 +446,11 @@ public class Client {
|
|||
sslCtx.setRsaDecCb(rsaDec);
|
||||
}
|
||||
|
||||
/* Verify the peer hostname for non-loopback hosts. The example
|
||||
* certs carry no loopback name. */
|
||||
boolean checkDomain = (verifyPeer != 0) &&
|
||||
!InetAddress.getByName(host).isLoopbackAddress();
|
||||
|
||||
if (benchmark != 0) {
|
||||
int times = benchmark;
|
||||
int i = 0;
|
||||
|
|
@ -466,6 +471,15 @@ public class Client {
|
|||
ssl = new WolfSSLSession(sslCtx);
|
||||
ssl.setFd(sock);
|
||||
|
||||
if (checkDomain) {
|
||||
ret = ssl.checkDomainName(host);
|
||||
if (ret != WolfSSL.SSL_SUCCESS) {
|
||||
System.out.println(
|
||||
"failed to set domain name check!");
|
||||
System.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
do {
|
||||
ret = ssl.connect();
|
||||
err = ssl.getError(ret);
|
||||
|
|
@ -648,6 +662,14 @@ public class Client {
|
|||
ssl.setRsaDecCtx(rsaDecCtx);
|
||||
}
|
||||
|
||||
if (checkDomain) {
|
||||
ret = ssl.checkDomainName(host);
|
||||
if (ret != WolfSSL.SSL_SUCCESS) {
|
||||
System.out.println("failed to set domain name check!");
|
||||
System.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
/* call wolfSSL_connect */
|
||||
do {
|
||||
ret = ssl.connect();
|
||||
|
|
@ -822,6 +844,15 @@ public class Client {
|
|||
/* restore saved WOLFSSL_SESSION */
|
||||
ssl.setSession(session);
|
||||
|
||||
if (checkDomain) {
|
||||
ret = ssl.checkDomainName(host);
|
||||
if (ret != WolfSSL.SSL_SUCCESS) {
|
||||
System.out.println(
|
||||
"failed to set domain name check!");
|
||||
System.exit(1);
|
||||
}
|
||||
}
|
||||
|
||||
/* call wolfSSL_connect */
|
||||
do {
|
||||
ret = ssl.connect();
|
||||
|
|
|
|||
|
|
@ -127,7 +127,8 @@ public class WolfSSLCertRequest {
|
|||
* @param name Initialized and populated WolfSSLX509 name to be set into
|
||||
* Subject Name of WolfSSLCertRequest for cert generation.
|
||||
*
|
||||
* @throws IllegalStateException if WolfSSLCertRequest has been freed.
|
||||
* @throws IllegalStateException if WolfSSLCertRequest has been freed, or
|
||||
* if the provided WolfSSLX509Name has been freed.
|
||||
* @throws WolfSSLException if native JNI error occurs.
|
||||
*/
|
||||
public void setSubjectName(WolfSSLX509Name name)
|
||||
|
|
@ -142,9 +143,12 @@ public class WolfSSLCertRequest {
|
|||
WolfSSLDebug.INFO, this.x509ReqPtr,
|
||||
() -> "entered setSubjectName(" + name + ")");
|
||||
|
||||
/* TODO somehow lock WolfSSLX509Name object while using pointer? */
|
||||
ret = X509_REQ_set_subject_name(this.x509ReqPtr,
|
||||
/* Synchronize on the name so its free() can't release the native
|
||||
* pointer during the call below. */
|
||||
synchronized (name) {
|
||||
ret = X509_REQ_set_subject_name(this.x509ReqPtr,
|
||||
name.getNativeX509NamePtr());
|
||||
}
|
||||
}
|
||||
|
||||
if (ret != WolfSSL.SSL_SUCCESS) {
|
||||
|
|
|
|||
|
|
@ -1583,7 +1583,9 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
} catch (SSLHandshakeException e){
|
||||
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
|
||||
() -> "got SSLHandshakeException in doHandshake()");
|
||||
close();
|
||||
throw e;
|
||||
|
||||
} catch (SSLException e) {
|
||||
final int tmpErr = err;
|
||||
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
|
||||
|
|
@ -1592,6 +1594,7 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
Thread.currentThread().getId() + ")");
|
||||
close();
|
||||
throw e;
|
||||
|
||||
} catch (WolfSSLException e) {
|
||||
/* close socket if the handshake is unsuccessful */
|
||||
close();
|
||||
|
|
@ -1623,8 +1626,10 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
synchronized (handshakeLock) {
|
||||
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
|
||||
() -> "thread got handshakeLock (handshakeComplete)");
|
||||
/* mark handshake completed */
|
||||
/* Mark handshake completed, then wake read()/write() waiters so
|
||||
* they stop waiting on the in-progress handshake */
|
||||
handshakeComplete = true;
|
||||
handshakeLock.notifyAll();
|
||||
}
|
||||
|
||||
/* notify handshake completed listeners */
|
||||
|
|
@ -1650,6 +1655,66 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Block until any in-progress handshake finishes or the connection
|
||||
* closes, bounded by SO_TIMEOUT, then report whether this thread must
|
||||
* start the handshake itself. Called by stream read()/write() so native
|
||||
* I/O is never dispatched while a handshake runs on the same WOLFSSL*.
|
||||
* The start decision is made under handshakeLock together with the wait,
|
||||
* so a handshake cannot slip in between. startHandshake() is left to the
|
||||
* caller (after the lock is released) to avoid a lock-order inversion
|
||||
* with close(), which takes the socket monitor before handshakeLock.
|
||||
*
|
||||
* @return true if the caller should invoke startHandshake()
|
||||
* @throws SocketException if the connection is already closed or the
|
||||
* wait is interrupted
|
||||
* @throws SocketTimeoutException if SO_TIMEOUT elapses while waiting
|
||||
* @throws IOException if SO_TIMEOUT cannot be read
|
||||
*/
|
||||
private boolean waitForHandshakeThenCheckStart() throws IOException {
|
||||
|
||||
long remaining;
|
||||
|
||||
/* Read SO_TIMEOUT before locking to keep lock order consistent. */
|
||||
int soTimeout = getSoTimeout();
|
||||
long endTime = (soTimeout > 0) ?
|
||||
System.currentTimeMillis() + soTimeout : 0;
|
||||
|
||||
synchronized (handshakeLock) {
|
||||
if (this.connectionClosed == true) {
|
||||
throw new SocketException("Connection already shutdown");
|
||||
}
|
||||
|
||||
while (this.handshakeStarted && !this.handshakeComplete &&
|
||||
!this.connectionClosed) {
|
||||
|
||||
try {
|
||||
if (soTimeout > 0) {
|
||||
remaining = endTime - System.currentTimeMillis();
|
||||
if (remaining <= 0) {
|
||||
throw new SocketTimeoutException(
|
||||
"Timed out waiting for handshake");
|
||||
}
|
||||
handshakeLock.wait(remaining);
|
||||
} else {
|
||||
handshakeLock.wait();
|
||||
}
|
||||
} catch (InterruptedException e) {
|
||||
Thread.currentThread().interrupt();
|
||||
throw new SocketException(
|
||||
"Interrupted waiting for handshake");
|
||||
}
|
||||
}
|
||||
|
||||
if (this.connectionClosed == true) {
|
||||
throw new SocketException("Connection already shutdown");
|
||||
}
|
||||
|
||||
return (this.handshakeComplete == false &&
|
||||
this.handshakeStarted == false);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Sets the SSLSocket to use client or server mode.
|
||||
*
|
||||
|
|
@ -2071,6 +2136,9 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
|
||||
() -> "Socket already closed, skipping " +
|
||||
"TLS shutdown");
|
||||
/* Mark closed and wake any read()/write() waiters */
|
||||
this.connectionClosed = true;
|
||||
handshakeLock.notifyAll();
|
||||
return;
|
||||
}
|
||||
|
||||
|
|
@ -2138,8 +2206,6 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
WolfSSLDebug.log(getClass(), WolfSSLDebug.INFO,
|
||||
() -> "thread got handshakeLock");
|
||||
|
||||
this.connectionClosed = true;
|
||||
|
||||
/* Release native verify callback (JNI global) */
|
||||
if (this.EngineHelper != null) {
|
||||
this.EngineHelper.unsetVerifyCallback();
|
||||
|
|
@ -2216,6 +2282,14 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
}
|
||||
}
|
||||
|
||||
/* Mark closed on every teardown path, before clearing
|
||||
* EngineHelper, so a later startHandshake() won't NPE.
|
||||
* Wake any read()/write() waiting on the handshake. */
|
||||
synchronized (handshakeLock) {
|
||||
this.connectionClosed = true;
|
||||
handshakeLock.notifyAll();
|
||||
}
|
||||
|
||||
/* Reset internal WolfSSLEngineHelper to null */
|
||||
if (this.EngineHelper != null) {
|
||||
this.EngineHelper.clearObjectState();
|
||||
|
|
@ -2730,25 +2804,12 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
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");
|
||||
}
|
||||
}
|
||||
|
||||
/* Wait for any in-progress handshake to finish, then drive
|
||||
* one here if needed. Keeps ssl.read() off a WOLFSSL* whose
|
||||
* handshake is still running under ioLock. The helper throws
|
||||
* if the connection has already been closed. */
|
||||
try {
|
||||
/* do handshake if not completed yet, handles
|
||||
* synchronization */
|
||||
if (socket.handshakeComplete == false &&
|
||||
socket.handshakeStarted == false) {
|
||||
if (socket.waitForHandshakeThenCheckStart()) {
|
||||
socket.startHandshake();
|
||||
}
|
||||
} catch (SocketTimeoutException e) {
|
||||
|
|
@ -2964,24 +3025,12 @@ public class WolfSSLSocket extends SSLSocket {
|
|||
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");
|
||||
}
|
||||
}
|
||||
|
||||
/* Wait for any in-progress handshake to finish, then drive
|
||||
* one here if needed. Keeps ssl.write() off a WOLFSSL* whose
|
||||
* handshake is still running under ioLock. The helper throws
|
||||
* if the connection has already been closed. */
|
||||
try {
|
||||
/* do handshake if not completed yet, handles
|
||||
* synchronization */
|
||||
if (socket.handshakeComplete == false &&
|
||||
socket.handshakeStarted == false) {
|
||||
if (socket.waitForHandshakeThenCheckStart()) {
|
||||
socket.startHandshake();
|
||||
}
|
||||
} catch (SocketTimeoutException e) {
|
||||
|
|
|
|||
|
|
@ -4289,6 +4289,113 @@ public class WolfSSLSocketTest {
|
|||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* A handshake that fails by throwing SSLHandshakeException from
|
||||
* doHandshake() leaves handshakeStarted true and handshakeComplete
|
||||
* false. A later read()/write() must not wait forever on that state.
|
||||
* This drives the server-side SNI mismatch path, which throws
|
||||
* SSLHandshakeException after the native handshake completes, then
|
||||
* confirms a subsequent read() returns instead of hanging.
|
||||
*/
|
||||
@Test
|
||||
public void testReadDoesNotHangAfterFailedHandshake()
|
||||
throws Exception {
|
||||
|
||||
/* SNI matcher rejection requires wolfSSL 5.7.6 or later, matching
|
||||
* testSNIMatchers above. */
|
||||
long libVerHex = WolfSSL.getLibVersionHex();
|
||||
Assume.assumeTrue(libVerHex >= 0x05007006L);
|
||||
|
||||
this.ctx = tf.createSSLContext("TLS", ctxProvider);
|
||||
|
||||
final SSLServerSocket ss = (SSLServerSocket)ctx
|
||||
.getServerSocketFactory().createServerSocket(0);
|
||||
|
||||
/* Server accepts only SNI www.example.com */
|
||||
SNIMatcher matcher =
|
||||
SNIHostName.createSNIMatcher("www\\.example\\.com");
|
||||
Collection<SNIMatcher> matchers = new ArrayList<>();
|
||||
matchers.add(matcher);
|
||||
SSLParameters sp = ss.getSSLParameters();
|
||||
sp.setSNIMatchers(matchers);
|
||||
ss.setSSLParameters(sp);
|
||||
|
||||
ExecutorService es = Executors.newSingleThreadExecutor();
|
||||
SSLSocket cs = null;
|
||||
SSLSocket server = null;
|
||||
|
||||
try {
|
||||
cs = (SSLSocket)ctx.getSocketFactory().createSocket();
|
||||
cs.connect(new InetSocketAddress(ss.getLocalPort()));
|
||||
|
||||
/* Non-matching SNI makes the server throw SSLHandshakeException
|
||||
* after the native handshake completes. */
|
||||
SNIHostName serverName = new SNIHostName("www.example.org");
|
||||
List<SNIServerName> serverNames = new ArrayList<>();
|
||||
serverNames.add(serverName);
|
||||
SSLParameters cp = cs.getSSLParameters();
|
||||
cp.setServerNames(serverNames);
|
||||
cs.setSSLParameters(cp);
|
||||
|
||||
server = (SSLSocket)ss.accept();
|
||||
/* No SO_TIMEOUT on purpose: a timeout would turn a hung
|
||||
* handshake wait into a SocketTimeoutException and mask the
|
||||
* regression this test detects. */
|
||||
|
||||
final SSLSocket srv = server;
|
||||
Future<Void> serverFuture = es.submit(new Callable<Void>() {
|
||||
@Override
|
||||
public Void call() throws Exception {
|
||||
try {
|
||||
srv.startHandshake();
|
||||
fail("Server handshake should fail on bad SNI");
|
||||
} catch (SSLHandshakeException e) {
|
||||
/* expected */
|
||||
}
|
||||
|
||||
/* Must not block on the handshake wait. Any return or
|
||||
* exception is acceptable, only a hang is a failure. */
|
||||
try {
|
||||
srv.getInputStream().read(new byte[16]);
|
||||
} catch (Exception e) {
|
||||
/* SocketException from closed connection expected */
|
||||
}
|
||||
return null;
|
||||
}
|
||||
});
|
||||
|
||||
try {
|
||||
cs.startHandshake();
|
||||
} catch (SSLHandshakeException e) {
|
||||
/* Client may or may not throw, both are acceptable */
|
||||
}
|
||||
|
||||
try {
|
||||
serverFuture.get(20, TimeUnit.SECONDS);
|
||||
} catch (TimeoutException e) {
|
||||
fail("read() hung after a failed handshake");
|
||||
}
|
||||
|
||||
} finally {
|
||||
es.shutdownNow();
|
||||
if (cs != null) {
|
||||
try {
|
||||
cs.close();
|
||||
} catch (Exception e) {
|
||||
/* ignore close error during cleanup */
|
||||
}
|
||||
}
|
||||
if (server != null) {
|
||||
try {
|
||||
server.close();
|
||||
} catch (Exception e) {
|
||||
/* ignore close error during cleanup */
|
||||
}
|
||||
}
|
||||
ss.close();
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Inner class used to hold configuration options for
|
||||
* TestServer and TestClient classes.
|
||||
|
|
|
|||
|
|
@ -32,6 +32,8 @@ import java.io.File;
|
|||
import java.io.IOException;
|
||||
import java.nio.file.Files;
|
||||
import java.nio.file.Paths;
|
||||
import java.util.concurrent.CountDownLatch;
|
||||
import java.util.concurrent.atomic.AtomicReference;
|
||||
|
||||
import java.security.PublicKey;
|
||||
import java.security.PrivateKey;
|
||||
|
|
@ -725,4 +727,64 @@ public class WolfSSLCertRequestTest {
|
|||
throws IOException {
|
||||
Files.write(new File(path).toPath(), csr);
|
||||
}
|
||||
|
||||
/* setSubjectName() holds the WolfSSLX509Name lock across the native call
|
||||
* so a concurrent free() cannot free the pointer mid-call. Races the two
|
||||
* operations to check concurrency safety and deadlock-freedom of the added
|
||||
* locking, tolerating the expected exceptions when free() wins the race. */
|
||||
@Test(timeout = 60000)
|
||||
public void testSetSubjectNameFreeRace()
|
||||
throws WolfSSLException, WolfSSLJNIException, InterruptedException {
|
||||
|
||||
Assume.assumeTrue(WolfSSL.certReqEnabled());
|
||||
|
||||
final int iterations = 200;
|
||||
final AtomicReference<Throwable> failure =
|
||||
new AtomicReference<Throwable>();
|
||||
|
||||
for (int i = 0; i < iterations && failure.get() == null; i++) {
|
||||
|
||||
final WolfSSLCertRequest req = new WolfSSLCertRequest();
|
||||
final WolfSSLX509Name name = GenerateTestSubjectName();
|
||||
final CountDownLatch start = new CountDownLatch(1);
|
||||
|
||||
Thread setter = new Thread(new Runnable() {
|
||||
public void run() {
|
||||
try {
|
||||
start.await();
|
||||
req.setSubjectName(name);
|
||||
} catch (IllegalStateException | WolfSSLException e) {
|
||||
/* expected if free() won the race */
|
||||
} catch (Throwable t) {
|
||||
failure.compareAndSet(null, t);
|
||||
}
|
||||
}
|
||||
});
|
||||
Thread freer = new Thread(new Runnable() {
|
||||
public void run() {
|
||||
try {
|
||||
start.await();
|
||||
name.free();
|
||||
} catch (Throwable t) {
|
||||
failure.compareAndSet(null, t);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
setter.start();
|
||||
freer.start();
|
||||
start.countDown();
|
||||
setter.join();
|
||||
freer.join();
|
||||
|
||||
name.free();
|
||||
req.free();
|
||||
}
|
||||
|
||||
if (failure.get() != null) {
|
||||
throw new AssertionError(
|
||||
"unexpected error during setSubjectName/free race",
|
||||
failure.get());
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in New Issue