From cd27edc249f01e16e55ce9cf11bbaf5b1680da95 Mon Sep 17 00:00:00 2001 From: Josh Aguilar Date: Thu, 27 Jul 2023 21:07:06 +1200 Subject: [PATCH 1/6] Update guava to address CVE-2023-2976 Signed-off-by: Josh Aguilar --- build.gradle | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/build.gradle b/build.gradle index 8445f01cf6..0d8af4626e 100644 --- a/build.gradle +++ b/build.gradle @@ -84,7 +84,7 @@ configurations.all { force "org.apache.commons:commons-lang3:3.4" force "org.springframework:spring-core:5.3.28" force "org.springframework:spring-expression:5.3.28" - force "com.google.guava:guava:30.0-jre" + force "com.google.guava:guava:32.0.1-jre" force "com.fasterxml.woodstox:woodstox-core:6.4.0" force "org.scala-lang:scala-library:2.13.9" force "org.apache.bcel:bcel:6.6.0" // This line should be removed once Spotbugs is upgraded to 4.7.4 @@ -102,7 +102,7 @@ dependencies { implementation 'jakarta.annotation:jakarta.annotation-api:1.3.5' implementation "org.opensearch.plugin:transport-netty4-client:${opensearch_version}" implementation "org.opensearch.client:opensearch-rest-high-level-client:${opensearch_version}" - implementation 'com.google.guava:guava:30.0-jre' + implementation 'com.google.guava:guava:32.0.1-jre' implementation 'org.greenrobot:eventbus:3.2.0' implementation 'commons-cli:commons-cli:1.3.1' implementation 'org.bouncycastle:bcprov-jdk15to18:1.75' @@ -417,4 +417,4 @@ task updateVersion { } ant.replaceregexp(file:'build.gradle', match: '"opensearch.version", "\\d.*"', replace: '"opensearch.version", "' + newVersion.tokenize('-')[0] + '-SNAPSHOT"', flags:'g', byline:true) } -} +} From 074843fba1c789b7855554fe0b0916f27cca14c0 Mon Sep 17 00:00:00 2001 From: Josh Aguilar Date: Fri, 28 Jul 2023 09:06:35 +1200 Subject: [PATCH 2/6] Use latest version of guava Signed-off-by: Josh Aguilar --- build.gradle | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.gradle b/build.gradle index 0d8af4626e..677b878da4 100644 --- a/build.gradle +++ b/build.gradle @@ -84,7 +84,7 @@ configurations.all { force "org.apache.commons:commons-lang3:3.4" force "org.springframework:spring-core:5.3.28" force "org.springframework:spring-expression:5.3.28" - force "com.google.guava:guava:32.0.1-jre" + force "com.google.guava:guava:32.1.1-jre" force "com.fasterxml.woodstox:woodstox-core:6.4.0" force "org.scala-lang:scala-library:2.13.9" force "org.apache.bcel:bcel:6.6.0" // This line should be removed once Spotbugs is upgraded to 4.7.4 @@ -102,7 +102,7 @@ dependencies { implementation 'jakarta.annotation:jakarta.annotation-api:1.3.5' implementation "org.opensearch.plugin:transport-netty4-client:${opensearch_version}" implementation "org.opensearch.client:opensearch-rest-high-level-client:${opensearch_version}" - implementation 'com.google.guava:guava:32.0.1-jre' + implementation 'com.google.guava:guava:32.1.1-jre' implementation 'org.greenrobot:eventbus:3.2.0' implementation 'commons-cli:commons-cli:1.3.1' implementation 'org.bouncycastle:bcprov-jdk15to18:1.75' From 28fe6aaa1b01ea23cda000688b76133cf98322a4 Mon Sep 17 00:00:00 2001 From: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Date: Mon, 31 Jul 2023 14:43:51 -0400 Subject: [PATCH 3/6] Update SSLConnectionTestUtil.java Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> --- .../ssl/util/SSLConnectionTestUtil.java | 317 +++++++++--------- 1 file changed, 164 insertions(+), 153 deletions(-) diff --git a/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java b/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java index b7d0bb2031..ca0acd5864 100644 --- a/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java +++ b/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java @@ -12,178 +12,189 @@ * express or implied. See the License for the specific language governing * permissions and limitations under the License. */ - package org.opensearch.security.ssl.util; -import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.net.Socket; -import java.nio.charset.StandardCharsets; -import org.apache.logging.log4j.Logger; -import org.apache.logging.log4j.LogManager; - -/** - * Utility class to test if the server supports SSL connections. - * SSL Check will be done by sending an OpenSearch Ping to see if server is replying to pings. - * Following that a custom client hello message will be sent to the server, if the server - * side has OpenSearchPortUnificationHandler it will reply with server hello message. - */ -public class SSLConnectionTestUtil { - - private static final Logger logger = LogManager.getLogger(SSLConnectionTestUtil.class); - public static final byte[] OPENSEARCH_PING_MSG = new byte[]{(byte) 'E', (byte) 'S', (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}; - public static final String DUAL_MODE_CLIENT_HELLO_MSG = "DUALCM"; - public static final String DUAL_MODE_SERVER_HELLO_MSG = "DUALSM"; - private static final int SOCKET_TIMEOUT_MILLIS = 10 * 1000; - private boolean opensearchPingReplyReceived; - private boolean dualSSLProbeReplyReceived; - private final String host; - private final int port; - private Socket overriddenSocket = null; - private OutputStreamWriter testOutputStreamWriter = null; - private InputStreamReader testInputStreamReader = null; - - public SSLConnectionTestUtil(final String host, final int port) { - this.host = host; - this.port = port; - opensearchPingReplyReceived = false; - dualSSLProbeReplyReceived = false; +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; + +public class SSLConnectionTestUtilTests { + private Socket socket; + private OutputStream outputStream; + private InputStream inputStream; + private OutputStreamWriter outputStreamWriter; + private InputStreamReader inputStreamReader; + + @Before + public void setup() { + socket = Mockito.mock(Socket.class); + outputStream = Mockito.mock(OutputStream.class); + inputStream = Mockito.mock(InputStream.class); + outputStreamWriter = Mockito.mock(OutputStreamWriter.class); + inputStreamReader = Mockito.mock(InputStreamReader.class); } - @VisibleForTesting - protected SSLConnectionTestUtil(final String host, final int port, final Socket overriddenSocket, final OutputStreamWriter testOutputStreamWriter, - final InputStreamReader testInputStreamReader) { - this.overriddenSocket = overriddenSocket; - this.testOutputStreamWriter = testOutputStreamWriter; - this.testInputStreamReader = testInputStreamReader; - - this.host = host; - this.port = port; - opensearchPingReplyReceived = false; - dualSSLProbeReplyReceived = false; + @Test + public void testConnectionSSLAvailable() throws Exception { + Mockito.doNothing().when(outputStreamWriter).write(Mockito.anyString()); + Mockito.when(inputStreamReader.read()) + .thenReturn((int)'D') + .thenReturn((int)'U') + .thenReturn((int)'A') + .thenReturn((int)'L') + .thenReturn((int)'S') + .thenReturn((int)'M') + .thenReturn(-1); + Mockito.doNothing().when(socket).close(); + + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); + + verifyClientHelloSend(); + Mockito.verify(socket, Mockito.times(1)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_AVAILABLE, result); } - /** - * Test connection to server by performing the below steps: - * - Send Client Hello to check if the server replies with Server Hello which indicates that Server understands SSL - * - Send OpenSearch Ping to check if the server replies to the OpenSearch Ping message - * - * @return SSLConnectionTestResult i.e. OPENSEARCH_PING_FAILED or SSL_NOT_AVAILABLE or SSL_AVAILABLE - */ - public SSLConnectionTestResult testConnection() { - if (sendDualSSLClientHello()) { - return SSLConnectionTestResult.SSL_AVAILABLE; - } + @Test + public void testConnectionSSLNotAvailable() throws Exception { + setupMocksForClientHelloFailure(); + setupMocksForOpenSearchPingSuccess(); + Mockito.doNothing().when(socket).close(); - if (sendOpenSearchPing()) { - return SSLConnectionTestResult.SSL_NOT_AVAILABLE; - } + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); - return SSLConnectionTestResult.OPENSEARCH_PING_FAILED; + verifyClientHelloSend(); + verifyOpenSearchPingSend(); + Mockito.verify(socket, Mockito.times(2)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_NOT_AVAILABLE, result); } - private boolean sendDualSSLClientHello() { - boolean dualSslSupported = false; - Socket socket = null; - try { - OutputStreamWriter outputStreamWriter; - InputStreamReader inputStreamReader; - if(overriddenSocket != null) { - socket = overriddenSocket; - outputStreamWriter = testOutputStreamWriter; - inputStreamReader = testInputStreamReader; - } else { - socket = new Socket(host, port); - outputStreamWriter = new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8); - inputStreamReader = new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8); - } - - socket.setSoTimeout(SOCKET_TIMEOUT_MILLIS); - outputStreamWriter.write(DUAL_MODE_CLIENT_HELLO_MSG); - outputStreamWriter.flush(); - logger.debug("Sent DualSSL Client Hello msg to {}", host); - - StringBuilder sb = new StringBuilder(); - int currentChar; - while ((currentChar = inputStreamReader.read()) != -1) { - sb.append((char) currentChar); - } - - if (sb.toString().equals(DUAL_MODE_SERVER_HELLO_MSG)) { - logger.debug("Received DualSSL Server Hello msg from {}", host); - dualSslSupported = true; - } - } catch (IOException e) { - logger.debug("DualSSL client check failed for {}, exception {}", host, e.getMessage()); - } finally { - logger.debug("Closing DualSSL check client socket for {}", host); - if (socket != null) { - try { - socket.close(); - } catch (IOException e) { - logger.error("Exception occurred while closing DualSSL check client socket for {}. Exception: {}", host, e.getMessage()); - } - } - } - logger.debug("dualSslClient check with server {}, server supports ssl = {}", host, dualSslSupported); - return dualSslSupported; + @Test + public void testConnectionSSLNotAvailableIOException() throws Exception { + Mockito.doThrow(new IOException("Error while writing bytes to output stream")) + .when(outputStreamWriter) + .write(Mockito.anyString()); + setupMocksForOpenSearchPingSuccess(); + Mockito.doNothing().when(socket).close(); + + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); + + verifyClientHelloSend(); + Mockito.verifyNoMoreInteractions(inputStreamReader); + verifyOpenSearchPingSend(); + Mockito.verify(socket, Mockito.times(2)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_NOT_AVAILABLE, result); + } + + @Test + public void testConnectionOpenSearchPingFailed() throws Exception { + setupMocksForClientHelloFailure(); + Mockito.when(socket.getOutputStream()).thenReturn(outputStream); + Mockito.when(socket.getInputStream()).thenReturn(inputStream); + Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); + Mockito.when(inputStream.read()) + .thenReturn(-1); + Mockito.doNothing().when(socket).close(); + + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); + + verifyClientHelloSend(); + verifyOpenSearchPingSend(); + Mockito.verify(socket, Mockito.times(2)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); + } + + @Test + public void testConnectionOpenSearchPingFailedInvalidReply() throws Exception { + setupMocksForClientHelloFailure(); + Mockito.when(socket.getOutputStream()).thenReturn(outputStream); + Mockito.when(socket.getInputStream()).thenReturn(inputStream); + Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); + Mockito.when(inputStream.read()) + .thenReturn((int)'E') + .thenReturn((int)'E') + .thenReturn(0xFF) + .thenReturn(0xFF) + .thenReturn(0xFF) + .thenReturn(0xFF); + Mockito.doNothing().when(socket).close(); + + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); + + verifyClientHelloSend(); + verifyOpenSearchPingSend(); + Mockito.verify(socket, Mockito.times(2)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); + } + + @Test + public void testConnectionOpenSearchPingFailedIOException() throws Exception { + setupMocksForClientHelloFailure(); + Mockito.when(socket.getOutputStream()).thenReturn(outputStream); + Mockito.when(socket.getInputStream()).thenReturn(inputStream); + Mockito.doThrow(new IOException("Error while writing bytes to output stream")).when(outputStream).write(Mockito.any(byte[].class)); + Mockito.doNothing().when(socket).close(); + + SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); + SSLConnectionTestResult result = connectionTestUtil.testConnection(); + + verifyClientHelloSend(); + verifyOpenSearchPingSend(); + Mockito.verifyNoInteractions(inputStream); + Mockito.verify(socket, Mockito.times(2)).close(); + Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); + } + + private void verifyClientHelloSend() throws IOException { + ArgumentCaptor clientHelloMsgArgCaptor = ArgumentCaptor.forClass(String.class); + Mockito.verify(outputStreamWriter, + Mockito.times(1)) + .write(clientHelloMsgArgCaptor.capture()); + String msgWritten = clientHelloMsgArgCaptor.getValue(); + String expectedMsg = "DUALCM"; + Assert.assertEquals("Unexpected Dual SSL Client Hello message written to socket", expectedMsg, msgWritten); } - private boolean sendOpenSearchPing() { - boolean pingSucceeded = false; - Socket socket = null; - try { - if(overriddenSocket != null) { - socket = overriddenSocket; - } else { - socket = new Socket(host, port); - } - - socket.setSoTimeout(SOCKET_TIMEOUT_MILLIS); - OutputStream outputStream = socket.getOutputStream(); - InputStream inputStream = socket.getInputStream(); - - logger.debug("Sending OpenSearch Ping to {}", host); - outputStream.write(OPENSEARCH_PING_MSG); - outputStream.flush(); - - int currentByte; - int byteBufIndex = 0; - byte[] response = new byte[6]; - while ((byteBufIndex < 6) && ((currentByte = inputStream.read()) != -1)) { - response[byteBufIndex] = (byte) currentByte; - byteBufIndex++; - } - if (byteBufIndex == 6) { - logger.debug("Received reply for OpenSearch Ping. from {}", host); - pingSucceeded = true; - for(int i = 0; i < 6; i++) { - if (response[i] != OPENSEARCH_PING_MSG[i]) { - // Unexpected byte in response - logger.error("Received unexpected byte in OpenSearch Ping reply from {}", host); - pingSucceeded = false; - break; - } - } - } - } catch (IOException ex) { - logger.error("OpenSearch Ping failed for {}, exception: {}", host, ex.getMessage()); - } finally { - logger.debug("Closing OpenSearch Ping client socket for connection to {}", host); - if (socket != null) { - try { - socket.close(); - } catch (IOException e) { - logger.error("Exception occurred while closing socket for {}. Exception: {}", host, e.getMessage()); - } - } + private void verifyOpenSearchPingSend() throws IOException { + ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(byte[].class); + Mockito.verify(outputStream, + Mockito.times(1)) + .write(argumentCaptor.capture()); + byte[] bytesWritten = argumentCaptor.getValue(); + byte[] expectedBytes = new byte[]{'E','S',(byte)0xFF,(byte)0xFF,(byte)0xFF,(byte)0xFF}; + for(int i = 0; i < bytesWritten.length; i++) { + Assert.assertEquals("Unexpected OpenSearch Ping bytes written to socket", expectedBytes[i], bytesWritten[i]); } + } + + private void setupMocksForClientHelloFailure() throws IOException { + Mockito.doNothing().when(outputStreamWriter).write(Mockito.anyString()); + Mockito.when(inputStreamReader.read()) + .thenReturn(-1); + } - logger.debug("OpenSearch Ping check to server {} result = {}", host, pingSucceeded); - return pingSucceeded; + private void setupMocksForOpenSearchPingSuccess() throws IOException { + Mockito.when(socket.getOutputStream()).thenReturn(outputStream); + Mockito.when(socket.getInputStream()).thenReturn(inputStream); + Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); + Mockito.when(inputStream.read()) + .thenReturn((int)'E') + .thenReturn((int)'S') + .thenReturn(0xFF) + .thenReturn(0xFF) + .thenReturn(0xFF) + .thenReturn(0xFF); } } From a3387d5b44e4188d3c5b963fc476e41f00144f08 Mon Sep 17 00:00:00 2001 From: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Date: Mon, 31 Jul 2023 14:44:10 -0400 Subject: [PATCH 4/6] Update HTTPJwtAuthenticatorTest.java Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> --- .../dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java b/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java index e150f33d65..24ccd41aac 100644 --- a/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java +++ b/src/test/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticatorTest.java @@ -19,6 +19,7 @@ import io.jsonwebtoken.Jwts; import io.jsonwebtoken.SignatureAlgorithm; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.security.KeyPair; import java.security.KeyPairGenerator; @@ -36,7 +37,6 @@ import org.junit.Assert; import org.junit.Test; import org.mockito.Mockito; -import org.mockito.internal.util.reflection.FieldSetter; import org.opensearch.security.user.AuthCredentials; import org.opensearch.security.util.FakeRestRequest; @@ -192,14 +192,17 @@ public void testBasicAuthHeader() throws Exception { Settings settings = Settings.builder().put("signing_key", BaseEncoding.base64().encode(secretKey)).build(); HTTPJwtAuthenticator jwtAuth = new HTTPJwtAuthenticator(settings, null); JwtParser jwtParser = Mockito.spy(JwtParser.class); - FieldSetter.setField(jwtAuth, HTTPJwtAuthenticator.class.getDeclaredField("jwtParser"), jwtParser); + + Field jwtParserField = HTTPJwtAuthenticator.class.getDeclaredField("jwtParser"); + jwtParserField.setAccessible(true); + jwtParserField.set(jwtAuth, jwtParser); String basicAuth = BaseEncoding.base64().encode("user:password".getBytes(StandardCharsets.UTF_8)); Map headers = Collections.singletonMap(HttpHeaders.AUTHORIZATION, "Basic " + basicAuth); AuthCredentials creds = jwtAuth.extractCredentials(new FakeRestRequest(headers, Collections.emptyMap()), null); Assert.assertNull(creds); - Mockito.verifyZeroInteractions(jwtParser); + Mockito.verifyNoInteractions(jwtParser); } @Test From 99f0a8c5a89fd2044c0773801684a1d2489c0f1e Mon Sep 17 00:00:00 2001 From: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Date: Mon, 31 Jul 2023 14:46:35 -0400 Subject: [PATCH 5/6] Update SSLConnectionTestUtil.java Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> --- .../ssl/util/SSLConnectionTestUtil.java | 317 +++++++++--------- 1 file changed, 153 insertions(+), 164 deletions(-) diff --git a/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java b/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java index ca0acd5864..b7d0bb2031 100644 --- a/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java +++ b/src/main/java/org/opensearch/security/ssl/util/SSLConnectionTestUtil.java @@ -12,189 +12,178 @@ * express or implied. See the License for the specific language governing * permissions and limitations under the License. */ + package org.opensearch.security.ssl.util; +import com.google.common.annotations.VisibleForTesting; import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.io.OutputStream; import java.io.OutputStreamWriter; import java.net.Socket; -import org.junit.Assert; -import org.junit.Before; -import org.junit.Test; -import org.mockito.ArgumentCaptor; -import org.mockito.Mockito; - -public class SSLConnectionTestUtilTests { - private Socket socket; - private OutputStream outputStream; - private InputStream inputStream; - private OutputStreamWriter outputStreamWriter; - private InputStreamReader inputStreamReader; - - @Before - public void setup() { - socket = Mockito.mock(Socket.class); - outputStream = Mockito.mock(OutputStream.class); - inputStream = Mockito.mock(InputStream.class); - outputStreamWriter = Mockito.mock(OutputStreamWriter.class); - inputStreamReader = Mockito.mock(InputStreamReader.class); - } - - @Test - public void testConnectionSSLAvailable() throws Exception { - Mockito.doNothing().when(outputStreamWriter).write(Mockito.anyString()); - Mockito.when(inputStreamReader.read()) - .thenReturn((int)'D') - .thenReturn((int)'U') - .thenReturn((int)'A') - .thenReturn((int)'L') - .thenReturn((int)'S') - .thenReturn((int)'M') - .thenReturn(-1); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - Mockito.verify(socket, Mockito.times(1)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_AVAILABLE, result); - } - - @Test - public void testConnectionSSLNotAvailable() throws Exception { - setupMocksForClientHelloFailure(); - setupMocksForOpenSearchPingSuccess(); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - verifyOpenSearchPingSend(); - Mockito.verify(socket, Mockito.times(2)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_NOT_AVAILABLE, result); - } - - @Test - public void testConnectionSSLNotAvailableIOException() throws Exception { - Mockito.doThrow(new IOException("Error while writing bytes to output stream")) - .when(outputStreamWriter) - .write(Mockito.anyString()); - setupMocksForOpenSearchPingSuccess(); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - Mockito.verifyNoMoreInteractions(inputStreamReader); - verifyOpenSearchPingSend(); - Mockito.verify(socket, Mockito.times(2)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_NOT_AVAILABLE, result); +import java.nio.charset.StandardCharsets; +import org.apache.logging.log4j.Logger; +import org.apache.logging.log4j.LogManager; + +/** + * Utility class to test if the server supports SSL connections. + * SSL Check will be done by sending an OpenSearch Ping to see if server is replying to pings. + * Following that a custom client hello message will be sent to the server, if the server + * side has OpenSearchPortUnificationHandler it will reply with server hello message. + */ +public class SSLConnectionTestUtil { + + private static final Logger logger = LogManager.getLogger(SSLConnectionTestUtil.class); + public static final byte[] OPENSEARCH_PING_MSG = new byte[]{(byte) 'E', (byte) 'S', (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}; + public static final String DUAL_MODE_CLIENT_HELLO_MSG = "DUALCM"; + public static final String DUAL_MODE_SERVER_HELLO_MSG = "DUALSM"; + private static final int SOCKET_TIMEOUT_MILLIS = 10 * 1000; + private boolean opensearchPingReplyReceived; + private boolean dualSSLProbeReplyReceived; + private final String host; + private final int port; + private Socket overriddenSocket = null; + private OutputStreamWriter testOutputStreamWriter = null; + private InputStreamReader testInputStreamReader = null; + + public SSLConnectionTestUtil(final String host, final int port) { + this.host = host; + this.port = port; + opensearchPingReplyReceived = false; + dualSSLProbeReplyReceived = false; } - @Test - public void testConnectionOpenSearchPingFailed() throws Exception { - setupMocksForClientHelloFailure(); - Mockito.when(socket.getOutputStream()).thenReturn(outputStream); - Mockito.when(socket.getInputStream()).thenReturn(inputStream); - Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); - Mockito.when(inputStream.read()) - .thenReturn(-1); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - verifyOpenSearchPingSend(); - Mockito.verify(socket, Mockito.times(2)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); + @VisibleForTesting + protected SSLConnectionTestUtil(final String host, final int port, final Socket overriddenSocket, final OutputStreamWriter testOutputStreamWriter, + final InputStreamReader testInputStreamReader) { + this.overriddenSocket = overriddenSocket; + this.testOutputStreamWriter = testOutputStreamWriter; + this.testInputStreamReader = testInputStreamReader; + + this.host = host; + this.port = port; + opensearchPingReplyReceived = false; + dualSSLProbeReplyReceived = false; } - @Test - public void testConnectionOpenSearchPingFailedInvalidReply() throws Exception { - setupMocksForClientHelloFailure(); - Mockito.when(socket.getOutputStream()).thenReturn(outputStream); - Mockito.when(socket.getInputStream()).thenReturn(inputStream); - Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); - Mockito.when(inputStream.read()) - .thenReturn((int)'E') - .thenReturn((int)'E') - .thenReturn(0xFF) - .thenReturn(0xFF) - .thenReturn(0xFF) - .thenReturn(0xFF); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - verifyOpenSearchPingSend(); - Mockito.verify(socket, Mockito.times(2)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); - } + /** + * Test connection to server by performing the below steps: + * - Send Client Hello to check if the server replies with Server Hello which indicates that Server understands SSL + * - Send OpenSearch Ping to check if the server replies to the OpenSearch Ping message + * + * @return SSLConnectionTestResult i.e. OPENSEARCH_PING_FAILED or SSL_NOT_AVAILABLE or SSL_AVAILABLE + */ + public SSLConnectionTestResult testConnection() { + if (sendDualSSLClientHello()) { + return SSLConnectionTestResult.SSL_AVAILABLE; + } - @Test - public void testConnectionOpenSearchPingFailedIOException() throws Exception { - setupMocksForClientHelloFailure(); - Mockito.when(socket.getOutputStream()).thenReturn(outputStream); - Mockito.when(socket.getInputStream()).thenReturn(inputStream); - Mockito.doThrow(new IOException("Error while writing bytes to output stream")).when(outputStream).write(Mockito.any(byte[].class)); - Mockito.doNothing().when(socket).close(); - - SSLConnectionTestUtil connectionTestUtil = new SSLConnectionTestUtil("127.0.0.1", 443, socket, outputStreamWriter, inputStreamReader); - SSLConnectionTestResult result = connectionTestUtil.testConnection(); - - verifyClientHelloSend(); - verifyOpenSearchPingSend(); - Mockito.verifyNoInteractions(inputStream); - Mockito.verify(socket, Mockito.times(2)).close(); - Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); - } + if (sendOpenSearchPing()) { + return SSLConnectionTestResult.SSL_NOT_AVAILABLE; + } - private void verifyClientHelloSend() throws IOException { - ArgumentCaptor clientHelloMsgArgCaptor = ArgumentCaptor.forClass(String.class); - Mockito.verify(outputStreamWriter, - Mockito.times(1)) - .write(clientHelloMsgArgCaptor.capture()); - String msgWritten = clientHelloMsgArgCaptor.getValue(); - String expectedMsg = "DUALCM"; - Assert.assertEquals("Unexpected Dual SSL Client Hello message written to socket", expectedMsg, msgWritten); + return SSLConnectionTestResult.OPENSEARCH_PING_FAILED; } - private void verifyOpenSearchPingSend() throws IOException { - ArgumentCaptor argumentCaptor = ArgumentCaptor.forClass(byte[].class); - Mockito.verify(outputStream, - Mockito.times(1)) - .write(argumentCaptor.capture()); - byte[] bytesWritten = argumentCaptor.getValue(); - byte[] expectedBytes = new byte[]{'E','S',(byte)0xFF,(byte)0xFF,(byte)0xFF,(byte)0xFF}; - for(int i = 0; i < bytesWritten.length; i++) { - Assert.assertEquals("Unexpected OpenSearch Ping bytes written to socket", expectedBytes[i], bytesWritten[i]); + private boolean sendDualSSLClientHello() { + boolean dualSslSupported = false; + Socket socket = null; + try { + OutputStreamWriter outputStreamWriter; + InputStreamReader inputStreamReader; + if(overriddenSocket != null) { + socket = overriddenSocket; + outputStreamWriter = testOutputStreamWriter; + inputStreamReader = testInputStreamReader; + } else { + socket = new Socket(host, port); + outputStreamWriter = new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8); + inputStreamReader = new InputStreamReader(socket.getInputStream(), StandardCharsets.UTF_8); + } + + socket.setSoTimeout(SOCKET_TIMEOUT_MILLIS); + outputStreamWriter.write(DUAL_MODE_CLIENT_HELLO_MSG); + outputStreamWriter.flush(); + logger.debug("Sent DualSSL Client Hello msg to {}", host); + + StringBuilder sb = new StringBuilder(); + int currentChar; + while ((currentChar = inputStreamReader.read()) != -1) { + sb.append((char) currentChar); + } + + if (sb.toString().equals(DUAL_MODE_SERVER_HELLO_MSG)) { + logger.debug("Received DualSSL Server Hello msg from {}", host); + dualSslSupported = true; + } + } catch (IOException e) { + logger.debug("DualSSL client check failed for {}, exception {}", host, e.getMessage()); + } finally { + logger.debug("Closing DualSSL check client socket for {}", host); + if (socket != null) { + try { + socket.close(); + } catch (IOException e) { + logger.error("Exception occurred while closing DualSSL check client socket for {}. Exception: {}", host, e.getMessage()); + } + } } + logger.debug("dualSslClient check with server {}, server supports ssl = {}", host, dualSslSupported); + return dualSslSupported; } - private void setupMocksForClientHelloFailure() throws IOException { - Mockito.doNothing().when(outputStreamWriter).write(Mockito.anyString()); - Mockito.when(inputStreamReader.read()) - .thenReturn(-1); - } + private boolean sendOpenSearchPing() { + boolean pingSucceeded = false; + Socket socket = null; + try { + if(overriddenSocket != null) { + socket = overriddenSocket; + } else { + socket = new Socket(host, port); + } + + socket.setSoTimeout(SOCKET_TIMEOUT_MILLIS); + OutputStream outputStream = socket.getOutputStream(); + InputStream inputStream = socket.getInputStream(); + + logger.debug("Sending OpenSearch Ping to {}", host); + outputStream.write(OPENSEARCH_PING_MSG); + outputStream.flush(); + + int currentByte; + int byteBufIndex = 0; + byte[] response = new byte[6]; + while ((byteBufIndex < 6) && ((currentByte = inputStream.read()) != -1)) { + response[byteBufIndex] = (byte) currentByte; + byteBufIndex++; + } + if (byteBufIndex == 6) { + logger.debug("Received reply for OpenSearch Ping. from {}", host); + pingSucceeded = true; + for(int i = 0; i < 6; i++) { + if (response[i] != OPENSEARCH_PING_MSG[i]) { + // Unexpected byte in response + logger.error("Received unexpected byte in OpenSearch Ping reply from {}", host); + pingSucceeded = false; + break; + } + } + } + } catch (IOException ex) { + logger.error("OpenSearch Ping failed for {}, exception: {}", host, ex.getMessage()); + } finally { + logger.debug("Closing OpenSearch Ping client socket for connection to {}", host); + if (socket != null) { + try { + socket.close(); + } catch (IOException e) { + logger.error("Exception occurred while closing socket for {}. Exception: {}", host, e.getMessage()); + } + } + } - private void setupMocksForOpenSearchPingSuccess() throws IOException { - Mockito.when(socket.getOutputStream()).thenReturn(outputStream); - Mockito.when(socket.getInputStream()).thenReturn(inputStream); - Mockito.doNothing().when(outputStream).write(Mockito.any(byte[].class)); - Mockito.when(inputStream.read()) - .thenReturn((int)'E') - .thenReturn((int)'S') - .thenReturn(0xFF) - .thenReturn(0xFF) - .thenReturn(0xFF) - .thenReturn(0xFF); + logger.debug("OpenSearch Ping check to server {} result = {}", host, pingSucceeded); + return pingSucceeded; } } From 51c38d3910bcfe64a6925e46dfe71c63d919806e Mon Sep 17 00:00:00 2001 From: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> Date: Mon, 31 Jul 2023 14:47:11 -0400 Subject: [PATCH 6/6] Update SSLConnectionTestUtilTests.java Signed-off-by: Stephen Crawford <65832608+scrawfor99@users.noreply.github.com> --- .../security/ssl/util/SSLConnectionTestUtilTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/opensearch/security/ssl/util/SSLConnectionTestUtilTests.java b/src/test/java/org/opensearch/security/ssl/util/SSLConnectionTestUtilTests.java index a8efa0fc3b..ca0acd5864 100644 --- a/src/test/java/org/opensearch/security/ssl/util/SSLConnectionTestUtilTests.java +++ b/src/test/java/org/opensearch/security/ssl/util/SSLConnectionTestUtilTests.java @@ -90,7 +90,7 @@ public void testConnectionSSLNotAvailableIOException() throws Exception { SSLConnectionTestResult result = connectionTestUtil.testConnection(); verifyClientHelloSend(); - Mockito.verifyZeroInteractions(inputStreamReader); + Mockito.verifyNoMoreInteractions(inputStreamReader); verifyOpenSearchPingSend(); Mockito.verify(socket, Mockito.times(2)).close(); Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.SSL_NOT_AVAILABLE, result); @@ -152,7 +152,7 @@ public void testConnectionOpenSearchPingFailedIOException() throws Exception { verifyClientHelloSend(); verifyOpenSearchPingSend(); - Mockito.verifyZeroInteractions(inputStream); + Mockito.verifyNoInteractions(inputStream); Mockito.verify(socket, Mockito.times(2)).close(); Assert.assertEquals("Unexpected result for testConnection invocation", SSLConnectionTestResult.OPENSEARCH_PING_FAILED, result); }