diff options
Diffstat (limited to 'jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server')
3 files changed, 170 insertions, 54 deletions
diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java index 31ecf3ca2fc..7ecf1b00fc1 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/HttpServerTest.java @@ -5,10 +5,12 @@ import com.google.inject.AbstractModule; import com.google.inject.Module; import com.yahoo.container.logging.AccessLog; import com.yahoo.container.logging.AccessLogEntry; +import com.yahoo.jdisc.Metric; import com.yahoo.jdisc.References; import com.yahoo.jdisc.Request; import com.yahoo.jdisc.Response; import com.yahoo.jdisc.application.BindingSetSelector; +import com.yahoo.jdisc.application.MetricConsumer; import com.yahoo.jdisc.handler.AbstractRequestHandler; import com.yahoo.jdisc.handler.CompletionHandler; import com.yahoo.jdisc.handler.ContentChannel; @@ -21,6 +23,8 @@ import com.yahoo.jdisc.http.Cookie; import com.yahoo.jdisc.http.HttpRequest; import com.yahoo.jdisc.http.HttpResponse; import com.yahoo.jdisc.http.ServerConfig; +import com.yahoo.jdisc.http.server.jetty.JettyHttpServer.Metrics; +import com.yahoo.jdisc.http.server.jetty.TestDrivers.TlsClientAuth; import com.yahoo.jdisc.service.BindingSetNotFoundException; import com.yahoo.security.KeyUtils; import com.yahoo.security.SslContextBuilder; @@ -34,6 +38,8 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import javax.net.ssl.SSLContext; +import javax.net.ssl.SSLException; +import javax.net.ssl.SSLHandshakeException; import javax.security.auth.x500.X500Principal; import java.io.IOException; import java.math.BigInteger; @@ -54,6 +60,8 @@ import java.util.List; import java.util.Map; import java.util.TreeMap; import java.util.concurrent.TimeUnit; +import java.util.logging.Level; +import java.util.logging.Logger; import java.util.regex.Pattern; import static com.yahoo.jdisc.Response.Status.GATEWAY_TIMEOUT; @@ -81,15 +89,21 @@ import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.startsWith; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.equalTo; +import static org.junit.Assert.fail; +import static org.mockito.ArgumentMatchers.anyMap; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; /** * @author Oyvind Bakksjo * @author Simon Thoresen Hult + * @author bjorncs */ public class HttpServerTest { + private static final Logger log = Logger.getLogger(HttpServerTest.class.getName()); + @Rule public TemporaryFolder tmpFolder = new TemporaryFolder(); @@ -478,7 +492,7 @@ public class HttpServerTest { Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); + final TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); driver.client().get("/status.html") .expectStatusCode(is(OK)); assertThat(driver.close(), is(true)); @@ -489,7 +503,7 @@ public class HttpServerTest { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); + TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); SSLContext trustStoreOnlyCtx = new SslContextBuilder() .withTrustStore(certificateFile) @@ -507,7 +521,7 @@ public class HttpServerTest { Path privateKeyFile = tmpFolder.newFile().toPath(); Path certificateFile = tmpFolder.newFile().toPath(); generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); - TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile); + TestDriver driver = TestDrivers.newInstanceWithSsl(new EchoRequestHandler(), certificateFile, privateKeyFile, TlsClientAuth.WANT); SSLContext trustStoreOnlyCtx = new SslContextBuilder() .withTrustStore(certificateFile) @@ -559,6 +573,114 @@ public class HttpServerTest { assertThat(driver.close(), is(true)); } + @Test + public void requireThatMetricIsIncrementedWhenClientIsMissingCertificateOnHandshake() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + var metricConsumer = new MetricConsumerMock(); + TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer); + + SSLContext clientCtx = new SslContextBuilder() + .withTrustStore(certificateFile) + .build(); + assertHttpsRequestTriggersSslHandshakeException( + driver, clientCtx, null, null, "Received fatal alert: bad_certificate"); + verify(metricConsumer.mockitoMock()) + .add(Metrics.SSL_HANDSHAKE_FAILURE_MISSING_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(driver.close(), is(true)); + } + + @Test + public void requireThatMetricIsIncrementedWhenClientUsesIncompatibleTlsVersion() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + var metricConsumer = new MetricConsumerMock(); + TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer); + + SSLContext clientCtx = new SslContextBuilder() + .withTrustStore(certificateFile) + .withKeyStore(privateKeyFile, certificateFile) + .build(); + + assertHttpsRequestTriggersSslHandshakeException( + driver, clientCtx, "TLSv1.1", null, "Received fatal alert: protocol_version"); + verify(metricConsumer.mockitoMock()) + .add(Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_PROTOCOLS, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(driver.close(), is(true)); + } + + @Test + public void requireThatMetricIsIncrementedWhenClientUsesIncompatibleCiphers() throws IOException { + Path privateKeyFile = tmpFolder.newFile().toPath(); + Path certificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(privateKeyFile, certificateFile); + var metricConsumer = new MetricConsumerMock(); + TestDriver driver = createSslTestDriver(certificateFile, privateKeyFile, metricConsumer); + + SSLContext clientCtx = new SslContextBuilder() + .withTrustStore(certificateFile) + .withKeyStore(privateKeyFile, certificateFile) + .build(); + + assertHttpsRequestTriggersSslHandshakeException( + driver, clientCtx, null, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", "Received fatal alert: handshake_failure"); + verify(metricConsumer.mockitoMock()) + .add(Metrics.SSL_HANDSHAKE_FAILURE_INCOMPATIBLE_CIPHERS, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(driver.close(), is(true)); + } + + @Test + public void requireThatMetricIsIncrementedWhenClientUsesInvalidCertificateInHandshake() throws IOException { + Path serverPrivateKeyFile = tmpFolder.newFile().toPath(); + Path serverCertificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(serverPrivateKeyFile, serverCertificateFile); + var metricConsumer = new MetricConsumerMock(); + TestDriver driver = createSslTestDriver(serverCertificateFile, serverPrivateKeyFile, metricConsumer); + + Path clientPrivateKeyFile = tmpFolder.newFile().toPath(); + Path clientCertificateFile = tmpFolder.newFile().toPath(); + generatePrivateKeyAndCertificate(clientPrivateKeyFile, clientCertificateFile); + + SSLContext clientCtx = new SslContextBuilder() + .withKeyStore(clientPrivateKeyFile, clientCertificateFile) + .withTrustStore(serverCertificateFile) + .build(); + + assertHttpsRequestTriggersSslHandshakeException( + driver, clientCtx, null, null, "Received fatal alert: certificate_unknown"); + verify(metricConsumer.mockitoMock()) + .add(Metrics.SSL_HANDSHAKE_FAILURE_INVALID_CLIENT_CERT, 1L, MetricConsumerMock.STATIC_CONTEXT); + assertThat(driver.close(), is(true)); + } + + private static TestDriver createSslTestDriver( + Path serverCertificateFile, Path serverPrivateKeyFile, MetricConsumerMock metricConsumer) throws IOException { + return TestDrivers.newInstanceWithSsl( + new EchoRequestHandler(), serverCertificateFile, serverPrivateKeyFile, TlsClientAuth.NEED, metricConsumer.asGuiceModule()); + } + + private static void assertHttpsRequestTriggersSslHandshakeException( + TestDriver testDriver, + SSLContext sslContext, + String protocolOverride, + String cipherOverride, + String expectedExceptionSubstring) throws IOException { + List<String> protocols = protocolOverride != null ? List.of(protocolOverride) : null; + List<String> ciphers = cipherOverride != null ? List.of(cipherOverride) : null; + try (var client = new SimpleHttpClient(sslContext, protocols, ciphers, testDriver.server().getListenPort(), false)) { + client.get("/status.html"); + fail("SSLHandshakeException expected"); + } catch (SSLHandshakeException e) { + assertThat(e.getMessage(), containsString(expectedExceptionSubstring)); + } catch (SSLException e) { + // Jetty may sometime close the connection before the apache client has fully consumed the TLS handshake frame + log.log(Level.WARNING, "Client failed to get a proper TLS handshake response: " + e.getMessage(), e); + assertThat(e.getMessage(), containsString("readHandshakeRecord")); // Only ignore this specific ssl exception + } + } + private static void generatePrivateKeyAndCertificate(Path privateKeyFile, Path certificateFile) throws IOException { KeyPair keyPair = KeyUtils.generateKeypair(RSA, 2048); Files.writeString(privateKeyFile, KeyUtils.toPem(keyPair.getPrivate())); @@ -749,4 +871,18 @@ public class HttpServerTest { return lhs.getName().compareTo(rhs.getName()); } } + + private static class MetricConsumerMock { + static final Metric.Context STATIC_CONTEXT = new Metric.Context() {}; + + private final MetricConsumer mockitoMock = mock(MetricConsumer.class); + + MetricConsumerMock() { + when(mockitoMock.createContext(anyMap())).thenReturn(STATIC_CONTEXT); + } + + MetricConsumer mockitoMock() { return mockitoMock; } + Module asGuiceModule() { return binder -> binder.bind(MetricConsumer.class).toInstance(mockitoMock); } + } + } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java index b0f570317d6..8035734a76c 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/SimpleHttpClient.java @@ -1,12 +1,11 @@ // Copyright 2017 Yahoo Holdings. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root. package com.yahoo.jdisc.http.server.jetty; -import com.yahoo.jdisc.http.HttpHeaders; import org.apache.http.Header; import org.apache.http.HttpEntity; import org.apache.http.HttpResponse; -import org.apache.http.client.HttpClient; import org.apache.http.client.entity.GzipCompressingEntity; +import org.apache.http.client.methods.CloseableHttpResponse; import org.apache.http.client.methods.HttpGet; import org.apache.http.client.methods.HttpPost; import org.apache.http.client.methods.HttpUriRequest; @@ -19,6 +18,7 @@ import org.apache.http.entity.ByteArrayEntity; import org.apache.http.entity.StringEntity; import org.apache.http.entity.mime.FormBodyPart; import org.apache.http.entity.mime.MultipartEntityBuilder; +import org.apache.http.impl.client.CloseableHttpClient; import org.apache.http.impl.client.HttpClientBuilder; import org.apache.http.impl.conn.BasicHttpClientConnectionManager; import org.apache.http.util.EntityUtils; @@ -26,16 +26,11 @@ import org.hamcrest.Matcher; import org.hamcrest.MatcherAssert; import javax.net.ssl.SSLContext; -import java.io.ByteArrayOutputStream; -import java.io.EOFException; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStreamWriter; -import java.net.Socket; import java.net.URI; import java.nio.charset.StandardCharsets; import java.util.Arrays; -import java.util.regex.Pattern; +import java.util.List; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; @@ -46,14 +41,20 @@ import static org.hamcrest.MatcherAssert.assertThat; * A simple http client for testing * * @author Simon Thoresen Hult + * @author bjorncs */ -public class SimpleHttpClient { +public class SimpleHttpClient implements AutoCloseable { - private final HttpClient delegate; + private final CloseableHttpClient delegate; private final String scheme; private final int listenPort; - public SimpleHttpClient(final SSLContext sslContext, final int listenPort, final boolean useCompression) { + public SimpleHttpClient(SSLContext sslContext, int listenPort, boolean useCompression) { + this(sslContext, null, null, listenPort, useCompression); + } + + public SimpleHttpClient(SSLContext sslContext, List<String> enabledProtocols, List<String> enabledCiphers, + int listenPort, boolean useCompression) { HttpClientBuilder builder = HttpClientBuilder.create(); if (!useCompression) { builder.disableContentCompression(); @@ -61,6 +62,8 @@ public class SimpleHttpClient { if (sslContext != null) { SSLConnectionSocketFactory sslConnectionFactory = new SSLConnectionSocketFactory( sslContext, + toArray(enabledProtocols), + toArray(enabledCiphers), new DefaultHostnameVerifier()); builder.setSSLSocketFactory(sslConnectionFactory); @@ -76,6 +79,10 @@ public class SimpleHttpClient { this.listenPort = listenPort; } + private static String[] toArray(List<String> list) { + return list != null ? list.toArray(new String[0]) : null; + } + public URI newUri(final String path) { return URI.create(scheme + "://localhost:" + listenPort + path); } @@ -100,40 +107,9 @@ public class SimpleHttpClient { return newGet(path).execute(); } - public String raw(final String request) throws IOException { - final Socket socket = new Socket("localhost", listenPort); - final OutputStreamWriter out = new OutputStreamWriter(socket.getOutputStream(), StandardCharsets.UTF_8); - out.write(request); - out.flush(); - - final ByteArrayOutputStream buf = new ByteArrayOutputStream(); - final InputStream in = socket.getInputStream(); - final int[] TERMINATOR = { '\r', '\n', '\r', '\n' }; - for (int pos = 0; pos < TERMINATOR.length; ++pos) { - final int b = in.read(); - if (b < 0) { - throw new EOFException(); - } - if (b != TERMINATOR[pos]) { - pos = -1; - } - buf.write(b); - } - final String response = buf.toString(StandardCharsets.UTF_8.name()); - final java.util.regex.Matcher matcher = Pattern.compile(HttpHeaders.Names.CONTENT_LENGTH + ": (.+)\r\n").matcher(response); - if (matcher.find()) { - final int len = Integer.valueOf(matcher.group(1)); - for (int i = 0; i < len; ++i) { - final int b = in.read(); - if (b < 0) { - throw new EOFException(); - } - buf.write(b); - } - } - - socket.close(); - return buf.toString(StandardCharsets.UTF_8.name()); + @Override + public void close() throws IOException { + delegate.close(); } public class RequestExecutor { @@ -177,7 +153,9 @@ public class SimpleHttpClient { if (entity != null) { ((HttpPost)request).setEntity(entity); } - return new ResponseValidator(delegate.execute(request)); + try (CloseableHttpResponse response = delegate.execute(request)){ + return new ResponseValidator(response); + } } } @@ -218,9 +196,5 @@ public class SimpleHttpClient { return this; } - public ResponseValidator expectTrailer(final String trailerName, final Matcher<String> matcher) { - // TODO: check trailer, not header - return expectHeader(trailerName, matcher); - } } } diff --git a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java index e0933ac485e..4908da2ba75 100644 --- a/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java +++ b/jdisc_http_service/src/test/java/com/yahoo/jdisc/http/server/jetty/TestDrivers.java @@ -20,6 +20,7 @@ import java.nio.file.Path; /** * @author Simon Thoresen Hult + * @author bjorncs */ public class TestDrivers { @@ -45,9 +46,12 @@ public class TestDrivers { )); } + public enum TlsClientAuth { NEED, WANT } + public static TestDriver newInstanceWithSsl(final RequestHandler requestHandler, Path certificateFile, Path privateKeyFile, + TlsClientAuth tlsClientAuth, final Module... guiceModules) throws IOException { return TestDriver.newInstance( JettyHttpServer.class, @@ -61,7 +65,9 @@ public class TestDrivers { .pathWhitelist("/status.html")) .ssl(new ConnectorConfig.Ssl.Builder() .enabled(true) - .clientAuth(ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH) + .clientAuth(tlsClientAuth == TlsClientAuth.NEED + ? ConnectorConfig.Ssl.ClientAuth.Enum.NEED_AUTH + : ConnectorConfig.Ssl.ClientAuth.Enum.WANT_AUTH) .privateKeyFile(privateKeyFile.toString()) .certificateFile(certificateFile.toString()) .caCertificateFile(certificateFile.toString())), |