Ensure proxy-protocol is mandatory when mixed-mode is disabled

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();