Skip to content
Permalink
Browse files

8238990: java/net/httpclient/HandshakeFailureTest.java failed against…

… TLSv1.3 on Windows

The SSLTube and SSLFlowDelegate are improved to wrap any non-SSL exception that occur during the handshake in an SSLHandshakeException.

Reviewed-by: chegar
  • Loading branch information
dfuch committed Feb 20, 2020
1 parent 23458bf commit f40220f5784922d5df7af3bbb764a205605e3700
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -229,6 +229,10 @@ protected SchedulingAction enterReadScheduling() {
return SchedulingAction.CONTINUE;
}

protected Throwable checkForHandshake(Throwable t) {
return t;
}


/**
* Processing function for incoming data. Pass it thru SSLEngine.unwrap().
@@ -356,6 +360,12 @@ private void addToReadBuf(List<ByteBuffer> buffers, boolean complete) {
}
}

@Override
protected boolean errorCommon(Throwable throwable) {
throwable = SSLFlowDelegate.this.checkForHandshake(throwable);
return super.errorCommon(throwable);
}

void schedule() {
scheduler.runOrSchedule(exec);
}
@@ -479,8 +489,9 @@ else if (this.completing) {
}
}
} catch (IOException ex) {
errorCommon(ex);
handleError(ex);
Throwable cause = checkForHandshake(ex);
errorCommon(cause);
handleError(cause);
return;
}
if (handshaking && !complete) {
@@ -504,6 +515,7 @@ else if (this.completing) {
requestMoreDataIfNeeded();
}
} catch (Throwable ex) {
ex = checkForHandshake(ex);
errorCommon(ex);
handleError(ex);
}
@@ -823,6 +835,7 @@ private void processData() {
writer.addData(HS_TRIGGER);
}
} catch (Throwable ex) {
ex = checkForHandshake(ex);
errorCommon(ex);
handleError(ex);
}
@@ -1127,7 +1140,7 @@ private void executeTasks(List<Runnable> tasks) {
}
resumeActivity();
} catch (Throwable t) {
handleError(t);
handleError(checkForHandshake(t));
}
});
}
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2017, 2019, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2017, 2020, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
@@ -34,6 +34,7 @@
import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Consumer;
import javax.net.ssl.SSLEngine;
import javax.net.ssl.SSLException;
import javax.net.ssl.SSLHandshakeException;
import javax.net.ssl.SSLEngineResult.HandshakeStatus;
import jdk.internal.net.http.common.SubscriberWrapper.SchedulingAction;
@@ -147,6 +148,13 @@ void connect(Flow.Subscriber<? super List<ByteBuffer>> downReader,
// is called.
upstreamWriter().onSubscribe(writeSubscription);
}

// Check whether the given exception should be wrapped
// in SSLHandshakeFailure exception
@Override
protected Throwable checkForHandshake(Throwable t) {
return SSLTube.this.checkForHandshake(t);
}
}

public CompletableFuture<String> getALPN() {
@@ -430,10 +438,10 @@ private void onSubscribeImpl(Flow.Subscription subscription) {
private void complete(DelegateWrapper subscriberImpl, Throwable t) {
try {
if (t == null) subscriberImpl.onComplete();
else subscriberImpl.onError(t);
else subscriberImpl.onError(t = checkForHandshake(t));
if (debug.on()) {
debug.log("subscriber completed %s"
+ ((t == null) ? "normally" : ("with error: " + t)));
debug.log("subscriber completed %s",
((t == null) ? "normally" : ("with error: " + t)));
}
} finally {
// Error or EOF while reading:
@@ -489,7 +497,7 @@ public void onErrorImpl(Throwable throwable) {
// onError before onSubscribe. It also prevents race conditions
// if onError is invoked concurrently with setDelegate.
// See setDelegate.

throwable = checkForHandshake(throwable);
errorRef.compareAndSet(null, throwable);
Throwable failed = errorRef.get();
finished = true;
@@ -517,29 +525,6 @@ public void onError(Throwable throwable) {
onErrorImpl(throwable);
}

private boolean handshaking() {
HandshakeStatus hs = engine.getHandshakeStatus();
return !(hs == NOT_HANDSHAKING || hs == FINISHED);
}

private String handshakeFailed() {
// sslDelegate can be null if we reach here
// during the initial handshake, as that happens
// within the SSLFlowDelegate constructor.
// In that case we will want to raise an exception.
if (handshaking()
&& (sslDelegate == null
|| !sslDelegate.closeNotifyReceived())) {
return "Remote host terminated the handshake";
}
// The initial handshake may not have been started yet.
// In which case - if we are completed before the initial handshake
// is started, we consider this a handshake failure as well.
if ("SSL_NULL_WITH_NULL_NULL".equals(engine.getSession().getCipherSuite()))
return "Remote host closed the channel";
return null;
}

@Override
public void onComplete() {
assert !finished && !onCompleteReceived;
@@ -548,15 +533,9 @@ public void onComplete() {
subscriberImpl = subscribed;
}

String handshakeFailed = handshakeFailed();
Throwable handshakeFailed = checkForHandshake(null);
if (handshakeFailed != null) {
if (debug.on())
debug.log("handshake: %s, inbound done: %s, outbound done: %s: %s",
engine.getHandshakeStatus(),
engine.isInboundDone(),
engine.isOutboundDone(),
handshakeFailed);
onErrorImpl(new SSLHandshakeException(handshakeFailed));
onErrorImpl(handshakeFailed);
} else if (subscriberImpl != null) {
onCompleteReceived = finished = true;
complete(subscriberImpl, null);
@@ -569,6 +548,55 @@ public void onComplete() {
}
}

private boolean handshaking() {
HandshakeStatus hs = engine.getHandshakeStatus();
return !(hs == NOT_HANDSHAKING || hs == FINISHED);
}

private String handshakeFailed() {
// sslDelegate can be null if we reach here
// during the initial handshake, as that happens
// within the SSLFlowDelegate constructor.
// In that case we will want to raise an exception.
if (handshaking()
&& (sslDelegate == null
|| !sslDelegate.closeNotifyReceived())) {
return "Remote host terminated the handshake";
}
// The initial handshake may not have been started yet.
// In which case - if we are completed before the initial handshake
// is started, we consider this a handshake failure as well.
if ("SSL_NULL_WITH_NULL_NULL".equals(engine.getSession().getCipherSuite()))
return "Remote host closed the channel";
return null;
}

/**
* If the stream is completed before the handshake is finished, we want
* to forward an SSLHandshakeException downstream.
* If t is not null an exception will always be returned. If t is null an
* exception will be returned if the engine is handshaking.
* @param t an exception from upstream, or null.
* @return t or an SSLHandshakeException wrapping t, or null.
*/
Throwable checkForHandshake(Throwable t) {
if (t instanceof SSLException) {
return t;
}
String handshakeFailed = handshakeFailed();
if (handshakeFailed == null) return t;
if (debug.on())
debug.log("handshake: %s, inbound done: %s, outbound done: %s: %s",
engine.getHandshakeStatus(),
engine.isInboundDone(),
engine.isOutboundDone(),
handshakeFailed);

SSLHandshakeException e = new SSLHandshakeException(handshakeFailed);
if (t != null) e.initCause(t);
return e;
}

@Override
public void connectFlows(TubePublisher writePub,
TubeSubscriber readSub) {
@@ -627,7 +627,6 @@ java/net/DatagramSocket/SendDatagramToBadAddress.java 7143960 macosx-a

java/net/ServerSocket/AcceptInheritHandle.java 8211854 aix-ppc64

java/net/httpclient/HandshakeFailureTest.java 8238990 windows-all

############################################################################

0 comments on commit f40220f

Please sign in to comment.