Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Adjust SSLDriver behavior for JDK11 changes #32145

Merged
merged 3 commits into from
Jul 18, 2018
Merged

Adjust SSLDriver behavior for JDK11 changes #32145

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
35bff1f
Fix all tests
Tim-Brooks Jul 17, 2018
7cee171
Fix comments
Tim-Brooks Jul 17, 2018
3937942
Remove fallthrough
Tim-Brooks Jul 17, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions ...ugin/security/src/main/java/org/elasticsearch/xpack/security/transport/nio/SSLDriver.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,10 @@ private void handshake() throws SSLException {
if (hasFlushPending() == false) {
handshakeStatus = wrap(EMPTY_BUFFER_ARRAY).getHandshakeStatus();
}
continueHandshaking = false;
// If we need NEED_TASK we should run the tasks immediately
if (handshakeStatus != SSLEngineResult.HandshakeStatus.NEED_TASK) {
continueHandshaking = false;
}
break;
case NEED_TASK:
runTasks();
Expand Down Expand Up @@ -432,8 +435,16 @@ private void runTasks() {
}

private void maybeFinishHandshake() {
// We only acknowledge that we are done handshaking if there are no bytes that need to be written
if (hasFlushPending() == false) {
if (engine.isOutboundDone() || engine.isInboundDone()) {
// If the engine is partially closed, immediate transition to close mode.
if (currentMode.isHandshake()) {
currentMode = new CloseMode(true);
} else {
String message = "Expected to be in handshaking mode. Instead in non-handshaking mode: " + currentMode;
throw new AssertionError(message);
}
} else if (hasFlushPending() == false) {
// We only acknowledge that we are done handshaking if there are no bytes that need to be written
if (currentMode.isHandshake()) {
currentMode = new ApplicationMode();
} else {
Expand Down Expand Up @@ -510,7 +521,7 @@ private CloseMode(boolean isHandshaking) {
if (isHandshaking && engine.isInboundDone() == false) {
// If we attempt to close during a handshake either we are sending an alert and inbound
// should already be closed or we are sending a close_notify. If we send a close_notify
// the peer will send an handshake error alert. If we attempt to receive the handshake alert,
// the peer might send an handshake error alert. If we attempt to receive the handshake alert,
// the engine will throw an IllegalStateException as it is not in a proper state to receive
// handshake message. Closing inbound immediately after close_notify is the cleanest option.
needToReceiveClose = false;
Expand Down
43 changes: 34 additions & 9 deletions ...security/src/test/java/org/elasticsearch/xpack/security/transport/nio/SSLDriverTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,15 @@ public void testPingPongAndClose() throws Exception {
public void testRenegotiate() throws Exception {
SSLContext sslContext = getSSLContext();

SSLDriver clientDriver = getDriver(sslContext.createSSLEngine(), true);
SSLDriver serverDriver = getDriver(sslContext.createSSLEngine(), false);
SSLEngine serverEngine = sslContext.createSSLEngine();
SSLEngine clientEngine = sslContext.createSSLEngine();

String[] serverProtocols = {"TLSv1.2"};
serverEngine.setEnabledProtocols(serverProtocols);
String[] clientProtocols = {"TLSv1.2"};
clientEngine.setEnabledProtocols(clientProtocols);
SSLDriver clientDriver = getDriver(clientEngine, true);
SSLDriver serverDriver = getDriver(serverEngine, false);

handshake(clientDriver, serverDriver);

Expand Down Expand Up @@ -119,16 +126,27 @@ public void testHandshakeFailureBecauseProtocolMismatch() throws Exception {
SSLContext sslContext = getSSLContext();
SSLEngine clientEngine = sslContext.createSSLEngine();
SSLEngine serverEngine = sslContext.createSSLEngine();
String[] serverProtocols = {"TLSv1.1", "TLSv1.2"};
String[] serverProtocols = {"TLSv1.2"};
serverEngine.setEnabledProtocols(serverProtocols);
String[] clientProtocols = {"TLSv1"};
String[] clientProtocols = {"TLSv1.1"};
clientEngine.setEnabledProtocols(clientProtocols);
SSLDriver clientDriver = getDriver(clientEngine, true);
SSLDriver serverDriver = getDriver(serverEngine, false);

SSLException sslException = expectThrows(SSLException.class, () -> handshake(clientDriver, serverDriver));
assertEquals("Client requested protocol TLSv1 not enabled or not supported", sslException.getMessage());
failedCloseAlert(serverDriver, clientDriver);
String oldExpected = "Client requested protocol TLSv1.1 not enabled or not supported";
String jdk11Expected = "Received fatal alert: protocol_version";
boolean expectedMessage = oldExpected.equals(sslException.getMessage()) || jdk11Expected.equals(sslException.getMessage());
assertTrue("Unexpected exception message: " + sslException.getMessage(), expectedMessage);

// In JDK11 we need an non-application write
if (serverDriver.needsNonApplicationWrite()) {
serverDriver.nonApplicationWrite();
}
// Prior to JDK11 we still need to send a close alert
if (serverDriver.isClosed() == false) {
failedCloseAlert(serverDriver, clientDriver);
}
}

public void testHandshakeFailureBecauseNoCiphers() throws Exception {
Expand All @@ -144,11 +162,18 @@ public void testHandshakeFailureBecauseNoCiphers() throws Exception {
SSLDriver clientDriver = getDriver(clientEngine, true);
SSLDriver serverDriver = getDriver(serverEngine, false);

SSLException sslException = expectThrows(SSLException.class, () -> handshake(clientDriver, serverDriver));
assertEquals("no cipher suites in common", sslException.getMessage());
failedCloseAlert(serverDriver, clientDriver);
expectThrows(SSLException.class, () -> handshake(clientDriver, serverDriver));
// In JDK11 we need an non-application write
if (serverDriver.needsNonApplicationWrite()) {
serverDriver.nonApplicationWrite();
}
// Prior to JDK11 we still need to send a close alert
if (serverDriver.isClosed() == false) {
failedCloseAlert(serverDriver, clientDriver);
}
}

@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/32144")
public void testCloseDuringHandshake() throws Exception {
SSLContext sslContext = getSSLContext();
SSLDriver clientDriver = getDriver(sslContext.createSSLEngine(), true);
Expand Down