Update SSLDriver for JDK 11 changes (#34398)

JDK11 introduced some changes with the SSLEngine. A number of error
messages were changed. Additionally, there were some behavior changes
in regard to how the SSLEngine handles closes during the handshake
process. This commit updates our tests and SSLDriver to support these
changes.
This commit is contained in:
Tim Brooks 2018-10-22 19:01:28 -04:00 committed by GitHub
parent ae1e46b852
commit d4bb3d1ce5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 72 additions and 28 deletions

View File

@ -533,17 +533,24 @@ public class SSLDriver implements AutoCloseable {
} else {
engine.closeOutbound();
}
}
@Override
public void read(InboundChannelBuffer buffer) throws SSLException {
if (needToReceiveClose == false) {
// There is an issue where receiving handshake messages after initiating the close process
// can place the SSLEngine back into handshaking mode. In order to handle this, if we
// initiate close during a handshake we do not wait to receive close. As we do not need to
// receive close, we will not handle reads.
return;
}
ensureApplicationBufferSize(buffer);
boolean continueUnwrap = true;
while (continueUnwrap && networkReadBuffer.position() > 0) {
networkReadBuffer.flip();
SSLEngineResult result = unwrap(buffer);
continueUnwrap = result.bytesProduced() > 0;
continueUnwrap = result.bytesProduced() > 0 || result.bytesConsumed() > 0;
}
if (engine.isInboundDone()) {
needToReceiveClose = false;
@ -598,7 +605,7 @@ public class SSLDriver implements AutoCloseable {
try {
engine.closeInbound();
} catch (SSLException e) {
if (e.getMessage().startsWith("Inbound closed before receiving peer's close_notify") == false) {
if (e.getMessage().contains("before receiving peer's close_notify") == false) {
throw e;
}
}

View File

@ -23,6 +23,7 @@ import java.nio.charset.StandardCharsets;
import java.security.SecureRandom;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.function.Supplier;
public class SSLDriverTests extends ESTestCase {
@ -96,7 +97,7 @@ public class SSLDriverTests extends ESTestCase {
normalClose(clientDriver, serverDriver);
}
public void testBigAppData() throws Exception {
public void testBigApplicationData() throws Exception {
SSLContext sslContext = getSSLContext();
SSLDriver clientDriver = getDriver(sslContext.createSSLEngine(), true);
@ -124,8 +125,6 @@ public class SSLDriverTests extends ESTestCase {
}
public void testHandshakeFailureBecauseProtocolMismatch() throws Exception {
// See https://github.com/elastic/elasticsearch/issues/33751
assumeTrue("test fails on JDK 11 >= ea28 currently", JavaVersion.current().compareTo(JavaVersion.parse("11")) < 0);
SSLContext sslContext = getSSLContext();
SSLEngine clientEngine = sslContext.createSSLEngine();
SSLEngine serverEngine = sslContext.createSSLEngine();
@ -138,7 +137,7 @@ public class SSLDriverTests extends ESTestCase {
SSLException sslException = expectThrows(SSLException.class, () -> handshake(clientDriver, serverDriver));
String oldExpected = "Client requested protocol TLSv1.1 not enabled or not supported";
String jdk11Expected = "Received fatal alert: protocol_version";
String jdk11Expected = "The client supported protocol versions [TLSv1.1] are not accepted by server preferences [TLS12]";
boolean expectedMessage = oldExpected.equals(sslException.getMessage()) || jdk11Expected.equals(sslException.getMessage());
assertTrue("Unexpected exception message: " + sslException.getMessage(), expectedMessage);
@ -148,7 +147,8 @@ public class SSLDriverTests extends ESTestCase {
}
// Prior to JDK11 we still need to send a close alert
if (serverDriver.isClosed() == false) {
failedCloseAlert(serverDriver, clientDriver);
failedCloseAlert(serverDriver, clientDriver, Arrays.asList("Received fatal alert: protocol_version",
"Received fatal alert: handshake_failure"));
}
}
@ -172,12 +172,14 @@ public class SSLDriverTests extends ESTestCase {
}
// Prior to JDK11 we still need to send a close alert
if (serverDriver.isClosed() == false) {
failedCloseAlert(serverDriver, clientDriver);
List<String> messages = Arrays.asList("Received fatal alert: handshake_failure",
"Received close_notify during handshake");
failedCloseAlert(serverDriver, clientDriver, messages);
}
}
@AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/32144")
public void testCloseDuringHandshake() throws Exception {
public void testCloseDuringHandshakeJDK11() throws Exception {
assumeTrue("this tests ssl engine for JDK11", JavaVersion.current().compareTo(JavaVersion.parse("11")) >= 0);
SSLContext sslContext = getSSLContext();
SSLDriver clientDriver = getDriver(sslContext.createSSLEngine(), true);
SSLDriver serverDriver = getDriver(sslContext.createSSLEngine(), false);
@ -199,30 +201,66 @@ public class SSLDriverTests extends ESTestCase {
serverDriver.initiateClose();
assertTrue(serverDriver.needsNonApplicationWrite());
assertFalse(serverDriver.isClosed());
sendNeededWrites(serverDriver, clientDriver);
sendNonApplicationWrites(serverDriver, clientDriver);
// We are immediately fully closed due to SSLEngine inconsistency
assertTrue(serverDriver.isClosed());
// This should not throw exception yet as the SSLEngine will not UNWRAP data while attempting to WRAP
clientDriver.read(clientBuffer);
sendNeededWrites(clientDriver, serverDriver);
SSLException sslException = expectThrows(SSLException.class, () -> clientDriver.read(clientBuffer));
assertEquals("Received close_notify during handshake", sslException.getMessage());
assertTrue(clientDriver.needsNonApplicationWrite());
sendNeededWrites(clientDriver, serverDriver);
sendNonApplicationWrites(clientDriver, serverDriver);
clientDriver.read(clientBuffer);
sendNonApplicationWrites(clientDriver, serverDriver);
serverDriver.read(serverBuffer);
assertTrue(clientDriver.isClosed());
}
private void failedCloseAlert(SSLDriver sendDriver, SSLDriver receiveDriver) throws SSLException {
public void testCloseDuringHandshakePreJDK11() throws Exception {
assumeTrue("this tests ssl engine for pre-JDK11", JavaVersion.current().compareTo(JavaVersion.parse("11")) < 0);
SSLContext sslContext = getSSLContext();
SSLDriver clientDriver = getDriver(sslContext.createSSLEngine(), true);
SSLDriver serverDriver = getDriver(sslContext.createSSLEngine(), false);
clientDriver.init();
serverDriver.init();
assertTrue(clientDriver.needsNonApplicationWrite());
assertFalse(serverDriver.needsNonApplicationWrite());
sendHandshakeMessages(clientDriver, serverDriver);
sendHandshakeMessages(serverDriver, clientDriver);
sendData(clientDriver, serverDriver);
assertTrue(clientDriver.isHandshaking());
assertTrue(serverDriver.isHandshaking());
assertFalse(serverDriver.needsNonApplicationWrite());
serverDriver.initiateClose();
assertTrue(serverDriver.needsNonApplicationWrite());
assertFalse(serverDriver.isClosed());
sendNonApplicationWrites(serverDriver, clientDriver);
// We are immediately fully closed due to SSLEngine inconsistency
assertTrue(serverDriver.isClosed());
// This should not throw exception yet as the SSLEngine will not UNWRAP data while attempting to WRAP
clientDriver.read(clientBuffer);
sendNonApplicationWrites(clientDriver, serverDriver);
SSLException sslException = expectThrows(SSLException.class, () -> clientDriver.read(clientBuffer));
assertEquals("Received close_notify during handshake", sslException.getMessage());
assertTrue(clientDriver.needsNonApplicationWrite());
sendNonApplicationWrites(clientDriver, serverDriver);
serverDriver.read(serverBuffer);
assertTrue(clientDriver.isClosed());
}
private void failedCloseAlert(SSLDriver sendDriver, SSLDriver receiveDriver, List<String> messages) throws SSLException {
assertTrue(sendDriver.needsNonApplicationWrite());
assertFalse(sendDriver.isClosed());
sendNeededWrites(sendDriver, receiveDriver);
sendNonApplicationWrites(sendDriver, receiveDriver);
assertTrue(sendDriver.isClosed());
sendDriver.close();
SSLException sslException = expectThrows(SSLException.class, () -> receiveDriver.read(genericBuffer));
assertEquals("Received fatal alert: handshake_failure", sslException.getMessage());
assertTrue("Expected one of the following exception messages: " + messages + ". Found: " + sslException.getMessage(),
messages.stream().anyMatch(m -> sslException.getMessage().equals(m)));
if (receiveDriver.needsNonApplicationWrite() == false) {
assertTrue(receiveDriver.isClosed());
receiveDriver.close();
@ -249,7 +287,7 @@ public class SSLDriverTests extends ESTestCase {
sendDriver.initiateClose();
assertFalse(sendDriver.readyForApplicationWrites());
assertTrue(sendDriver.needsNonApplicationWrite());
sendNeededWrites(sendDriver, receiveDriver);
sendNonApplicationWrites(sendDriver, receiveDriver);
assertFalse(sendDriver.isClosed());
receiveDriver.read(genericBuffer);
@ -257,7 +295,7 @@ public class SSLDriverTests extends ESTestCase {
assertFalse(receiveDriver.readyForApplicationWrites());
assertTrue(receiveDriver.needsNonApplicationWrite());
sendNeededWrites(receiveDriver, sendDriver);
sendNonApplicationWrites(receiveDriver, sendDriver);
assertTrue(receiveDriver.isClosed());
sendDriver.read(genericBuffer);
@ -267,7 +305,7 @@ public class SSLDriverTests extends ESTestCase {
receiveDriver.close();
}
private void sendNeededWrites(SSLDriver sendDriver, SSLDriver receiveDriver) throws SSLException {
private void sendNonApplicationWrites(SSLDriver sendDriver, SSLDriver receiveDriver) throws SSLException {
while (sendDriver.needsNonApplicationWrite() || sendDriver.hasFlushPending()) {
if (sendDriver.hasFlushPending() == false) {
sendDriver.nonApplicationWrite();
@ -315,7 +353,6 @@ public class SSLDriverTests extends ESTestCase {
assertTrue(sendDriver.needsNonApplicationWrite() || sendDriver.hasFlushPending());
while (sendDriver.needsNonApplicationWrite() || sendDriver.hasFlushPending()) {
assertFalse(receiveDriver.needsNonApplicationWrite());
if (sendDriver.hasFlushPending() == false) {
sendDriver.nonApplicationWrite();
}