Skip to content

Commit

Permalink
Issue #5684 - more disabled test cleanup
Browse files Browse the repository at this point in the history
+ Assumption based on existence of
  possible DNS Hijacking
+ Alternate logic for client side
  protocol and cipher suite mismatch
  behavior on server side based
  on client side protocol existence
  of TLSv1.3

Signed-off-by: Joakim Erdfelt <joakim.erdfelt@gmail.com>
  • Loading branch information
joakime committed Jun 18, 2021
1 parent 738d3a9 commit 704abc6
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 459 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,13 @@
package org.eclipse.jetty.client;

import java.io.IOException;
import java.net.InetAddress;
import java.net.URLDecoder;
import java.net.UnknownHostException;
import java.nio.ByteBuffer;
import java.nio.channels.UnresolvedAddressException;
import java.nio.charset.StandardCharsets;
import java.util.Random;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
Expand All @@ -42,8 +45,10 @@
import org.eclipse.jetty.http.HttpStatus;
import org.eclipse.jetty.server.Request;
import org.eclipse.jetty.toolchain.test.IO;
import org.eclipse.jetty.util.log.Log;
import org.eclipse.jetty.util.log.Logger;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Assumptions;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ArgumentsSource;

Expand All @@ -57,6 +62,8 @@

public class HttpClientRedirectTest extends AbstractHttpClientServerTest
{
private static final Logger LOG = Log.getLogger(HttpClientRedirectTest.class);

@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
public void test303(Scenario scenario) throws Exception
Expand Down Expand Up @@ -292,24 +299,26 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r

@ParameterizedTest
@ArgumentsSource(ScenarioProvider.class)
@Disabled
public void testRedirectFailed(Scenario scenario) throws Exception
{
// TODO this test is failing with timout after an ISP upgrade?? DNS dependent?
// Skip this test if DNS Hijacking is detected
Assumptions.assumeFalse(detectDnsHijacking());

start(scenario, new RedirectHandler());

try
{
client.newRequest("localhost", connector.getLocalPort())
ExecutionException e = assertThrows(ExecutionException.class,
() -> client.newRequest("localhost", connector.getLocalPort())
.scheme(scenario.getScheme())
.path("/303/doesNotExist/done")
.timeout(5, TimeUnit.SECONDS)
.send();
}
catch (ExecutionException x)
{
assertThat(x.getCause(), Matchers.instanceOf(UnresolvedAddressException.class));
}
.send());

assertThat("Cause", e.getCause(), Matchers.anyOf(
// Exception seen on some updates of OpenJDK 8
Matchers.instanceOf(UnresolvedAddressException.class),
// Exception seen on OpenJDK 11+
Matchers.instanceOf(UnknownHostException.class))
);
}

@ParameterizedTest
Expand Down Expand Up @@ -714,4 +723,46 @@ protected void service(String target, Request jettyRequest, HttpServletRequest r
}
}
}

public static boolean detectDnsHijacking()
{
String host1 = randomHostname();
String host2 = randomHostname();
String addr1 = getInetHostAddress(host1);
String addr2 = getInetHostAddress(host2);

if (LOG.isDebugEnabled())
{
LOG.debug("DNS Hijacking Lookup: host1={} ({}), host2={} ({})",
host1, addr1,
host2, addr2);
}

return (addr1.equals(addr2));
}

private static String getInetHostAddress(String hostname)
{
try
{
InetAddress addr = InetAddress.getByName(hostname);
return addr.getHostAddress();
}
catch (Throwable t)
{
return "<unknown:" + hostname + ">";
}
}

private static String randomHostname()
{
String digits = "0123456789abcdefghijklmnopqrstuvwxyz";
Random random = new Random();
char[] host = new char[7 + random.nextInt(8)];
for (int i = 0; i < host.length; ++i)
{
host[i] = digits.charAt(random.nextInt(digits.length()));
}
return new String(host) + ".tld.";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.Executor;
Expand Down Expand Up @@ -69,7 +70,6 @@
import org.eclipse.jetty.util.thread.QueuedThreadPool;
import org.hamcrest.Matchers;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Disabled;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.EnabledOnJre;
import org.junit.jupiter.api.condition.JRE;
Expand Down Expand Up @@ -265,10 +265,20 @@ public void handshakeFailed(Event event, Throwable failure)
assertTrue(clientLatch.await(1, TimeUnit.SECONDS));
}

// In JDK 11+, a mismatch on the client does not generate any bytes towards
// the server, while in previous JDKs the client sends to the server the close_notify.
// @EnabledOnJre({JRE.JAVA_8, JRE.JAVA_9, JRE.JAVA_10})
@Disabled("No longer viable, TLS protocol behavior changed in 8u272")
/**
* This tests the behavior of the Client side when you have an intentional
* mismatch of the TLS Protocol and TLS Ciphers on the client and attempt to
* initiate a connection.
* <p>
* In older versions of Java 8 (pre 8u272) the JDK client side logic
* would generate bytes and send it to the server along with a close_notify
* </p>
* <p>
* Starting in Java 8 (8u272) and Java 11.0.0, the client logic will not
* send any bytes to the server.
* </p>
*/
@Test
public void testMismatchBetweenTLSProtocolAndTLSCiphersOnClient() throws Exception
{
SslContextFactory serverTLSFactory = createServerSslContextFactory();
Expand All @@ -285,11 +295,18 @@ public void handshakeFailed(Event event, Throwable failure)
});

SslContextFactory clientTLSFactory = createClientSslContextFactory();
// TLS 1.1 protocol, but only TLS 1.2 ciphers.
clientTLSFactory.setIncludeProtocols("TLSv1.1");
clientTLSFactory.setIncludeCipherSuites("TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256");
// Use TLSv1.2 (as TLSv1.1 and older are now disabled in Java 8u292)
clientTLSFactory.setIncludeProtocols("TLSv1.2");
// Use only a cipher suite that exists for TLSv1.3 (this is the mismatch)
clientTLSFactory.setIncludeCipherSuites("TLS_AES_256_GCM_SHA384");
startClient(clientTLSFactory);

// If this JVM has "TLSv1.3" present, then it's assumed that it also has the new logic
// to not send bytes to the server when a protocol and cipher suite mismatch occurs
// on the client side configuration.
List<String> supportedClientProtocols = Arrays.asList(clientTLSFactory.getSslContext().getSupportedSSLParameters().getProtocols());
boolean expectServerFailure = !(supportedClientProtocols.contains("TLSv1.3"));

CountDownLatch clientLatch = new CountDownLatch(1);
client.addBean(new SslHandshakeListener()
{
Expand All @@ -306,7 +323,8 @@ public void handshakeFailed(Event event, Throwable failure)
.timeout(5, TimeUnit.SECONDS)
.send());

assertTrue(serverLatch.await(1, TimeUnit.SECONDS));
if (expectServerFailure)
assertTrue(serverLatch.await(1, TimeUnit.SECONDS));
assertTrue(clientLatch.await(1, TimeUnit.SECONDS));
}

Expand Down

0 comments on commit 704abc6

Please sign in to comment.