diff options
author | Bjørn Christian Seime <bjorncs@verizonmedia.com> | 2022-05-11 14:08:54 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2022-05-11 14:08:54 +0200 |
commit | 45dedaa74cde1a9b29da723fe8fe7d2182a79fd1 (patch) | |
tree | 710ddfc181b1b63f63a6c719c996bc20a4bbb952 | |
parent | 7f48538151a8a295bdc1596c6c43677a7e33d89d (diff) | |
parent | 386edaaa02e13489c7a53249c1f3c13b52724766 (diff) |
Merge pull request #22545 from vespa-engine/bjorncs/jdisc-proxy-protocol
Ensure proxy-protocol is mandatory when mixed-mode is disabled
-rw-r--r-- | container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java | 22 | ||||
-rw-r--r-- | container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java | 22 |
2 files changed, 36 insertions, 8 deletions
diff --git a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java index 2535d057e0e..a7c5b83f6a6 100644 --- a/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java +++ b/container-core/src/main/java/com/yahoo/jdisc/http/server/jetty/ConnectorFactory.java @@ -13,6 +13,7 @@ import org.eclipse.jetty.http2.server.AbstractHTTP2ServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2CServerConnectionFactory; import org.eclipse.jetty.http2.server.HTTP2ServerConnectionFactory; import org.eclipse.jetty.server.ConnectionFactory; +import org.eclipse.jetty.server.Connector; import org.eclipse.jetty.server.DetectorConnectionFactory; import org.eclipse.jetty.server.HttpConfiguration; import org.eclipse.jetty.server.HttpConnectionFactory; @@ -122,10 +123,7 @@ public class ConnectorFactory { sslFactory = newSslConnectionFactory(metric, http1Factory); } if (proxyProtocolConfig.enabled()) { - if (proxyProtocolConfig.mixedMode()) { - factories.add(newDetectorConnectionFactory(sslFactory)); - } - factories.add(newProxyProtocolConnectionFactory(sslFactory)); + factories.add(newProxyProtocolConnectionFactory(sslFactory, proxyProtocolConfig.mixedMode())); } factories.add(sslFactory); if (connectorConfig.http2Enabled()) factories.add(alpnFactory); @@ -199,8 +197,10 @@ public class ConnectorFactory { return new DetectorConnectionFactory(alternatives); } - private ProxyConnectionFactory newProxyProtocolConnectionFactory(ConnectionFactory wrappedFactory) { - return new ProxyConnectionFactory(wrappedFactory.getProtocol()); + private ProxyConnectionFactory newProxyProtocolConnectionFactory(ConnectionFactory wrapped, boolean mixedMode) { + return mixedMode + ? new ProxyConnectionFactory(wrapped.getProtocol()) + : new MandatoryProxyConnectionFactory(wrapped.getProtocol()); } private static boolean isSslEffectivelyEnabled(ConnectorConfig config) { @@ -210,4 +210,14 @@ public class ConnectorFactory { private static long toMillis(double seconds) { return (long)(seconds * 1000); } + /** + * A {@link ProxyConnectionFactory} which disables the default behaviour of upgrading to + * next protocol when proxy protocol is not detected. + */ + private static class MandatoryProxyConnectionFactory extends ProxyConnectionFactory { + MandatoryProxyConnectionFactory(String next) { super(next); } + @Override protected String findNextProtocol(Connector __) { return null; } + } + + } diff --git a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java index b862624efc0..d4d6dcee957 100644 --- a/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java +++ b/container-core/src/test/java/com/yahoo/jdisc/http/server/jetty/ProxyProtocolTest.java @@ -31,8 +31,10 @@ import static com.yahoo.yolean.Exceptions.uncheckInterrupted; import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V1; import static org.eclipse.jetty.client.ProxyProtocolClientConnectionFactory.V2; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertNotEquals; import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.junit.jupiter.api.Assertions.fail; /** * @author bjorncs @@ -117,6 +119,20 @@ class ProxyProtocolTest { assertNotEquals(proxyLocalPort, connectionLog.logEntries().get(0).localPort().get().intValue()); } + @Test + void requireThatSslConnectionFailsWhenMixedModeIsDisabled() throws Exception { + JettyTestDriver driver = createSslWithProxyProtocolTestDriver( + certificateFile, privateKeyFile, requestLogMock, connectionLog, false); + try { + sendJettyClientRequest(driver, certificateFile, null); + fail("Expected exception"); + } catch (ExecutionException e) { + assertInstanceOf(IOException.class, e.getCause()); + } finally { + assertTrue(driver.close()); + } + } + private static JettyTestDriver createSslWithProxyProtocolTestDriver( Path certificateFile, Path privateKeyFile, RequestLog requestLog, ConnectionLog connectionLog, boolean mixedMode) { @@ -142,6 +158,7 @@ class ProxyProtocolTest { private ContentResponse sendJettyClientRequest(JettyTestDriver testDriver, Path certificateFile, Object tag) throws Exception { HttpClient client = createJettyHttpClient(certificateFile); + ExecutionException cause = null; try { int maxAttempts = 3; for (int attempt = 0; attempt < maxAttempts; attempt++) { @@ -152,13 +169,14 @@ class ProxyProtocolTest { assertEquals(200, response.getStatus()); return response; } catch (ExecutionException e) { - // Retry when the server closes the connection before the TLS handshake is completed. This have been observed in CI. + // Retry when the server closes the connection before the TLS handshake is completed. This has been observed in CI. // We have been unable to reproduce this locally. The cause is therefor currently unknown. log.log(Level.WARNING, String.format("Attempt %d failed: %s", attempt, e.getMessage()), e); Thread.sleep(10); + cause = e; } } - throw new AssertionError("Failed to send request, see log for details"); + throw cause; } finally { client.stop(); client.destroy(); |