Revert "Revert "Flag to allow disabling mtls""

6 files changed, 77 insertions, 17 deletions

diff --git a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
index 3ac8d83f401..3a2f71e2b8c 100644
--- a/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
+++ b/config-model-api/src/main/java/com/yahoo/config/model/api/ModelContext.java
@@ -132,6 +132,8 @@ public interface ModelContext {
         // TODO: Remove after May 2021
         default boolean dedicatedClusterControllerCluster() { return hostedVespa(); }
 
+        // Allow disabling mTLS for now, harden later
+        default boolean allowDisableMtls() { return true; }
     }
 
     @Retention(RetentionPolicy.RUNTIME)
diff --git a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
index 59930a77a21..75a1a167446 100644
--- a/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
+++ b/config-model/src/main/java/com/yahoo/config/model/deploy/TestProperties.java
@@ -61,6 +61,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     private List<TenantSecretStore> tenantSecretStores = Collections.emptyList();
     private String jvmOmitStackTraceInFastThrowOption;
     private int numDistributorStripes = 0;
+    private boolean allowDisableMtls = true;
 
     @Override public ModelContext.FeatureFlags featureFlags() { return this; }
     @Override public boolean multitenant() { return multitenant; }
@@ -102,6 +103,7 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
     @Override public List<TenantSecretStore> tenantSecretStores() { return tenantSecretStores; }
     @Override public String jvmOmitStackTraceInFastThrowOption(ClusterSpec.Type type) { return jvmOmitStackTraceInFastThrowOption; }
     @Override public int numDistributorStripes() { return numDistributorStripes; }
+    @Override public boolean allowDisableMtls() { return allowDisableMtls; }
 
     public TestProperties setFeedConcurrency(double feedConcurrency) {
         this.feedConcurrency = feedConcurrency;
@@ -248,6 +250,11 @@ public class TestProperties implements ModelContext.Properties, ModelContext.Fea
         return this;
     }
 
+    public TestProperties allowDisableMtls(boolean value) {
+        this.allowDisableMtls = value;
+        return this;
+    }
+
     public static class Spec implements ConfigServerSpec {
 
         private final String hostName;
diff --git a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
index 5417a522d6a..5c8028575aa 100644
--- a/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
+++ b/config-model/src/main/java/com/yahoo/vespa/model/container/http/xml/HttpBuilder.java
@@ -78,13 +78,16 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
                 readAttr -> builder.readEnabled(Boolean.valueOf(readAttr)));
         XmlHelper.getOptionalAttribute(accessControlElem, "write").ifPresent(
                 writeAttr -> builder.writeEnabled(Boolean.valueOf(writeAttr)));
-        builder.clientAuthentication(
+
+        AccessControl.ClientAuthentication clientAuth =
                 XmlHelper.getOptionalAttribute(accessControlElem, "tls-handshake-client-auth")
-                        .map(value -> "want".equals(value)
-                                ? AccessControl.ClientAuthentication.want
-                                : AccessControl.ClientAuthentication.need)
-                        .orElse(AccessControl.ClientAuthentication.need)
-        );
+                        .filter("want"::equals)
+                        .map(value -> AccessControl.ClientAuthentication.want)
+                        .orElse(AccessControl.ClientAuthentication.need);
+        if (! deployState.getProperties().allowDisableMtls() && clientAuth == AccessControl.ClientAuthentication.want) {
+            throw new IllegalArgumentException("Overriding 'tls-handshake-client-auth' for application is not allowed.");
+        }
+        builder.clientAuthentication(clientAuth);
 
         Element excludeElem = XML.getChild(accessControlElem, "exclude");
         if (excludeElem != null) {
diff --git a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
index 4993a51ab74..39b5cc139d9 100644
--- a/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
+++ b/config-model/src/test/java/com/yahoo/vespa/model/container/xml/AccessControlTest.java
@@ -33,6 +33,7 @@ import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author gjoranv
@@ -290,17 +291,47 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
 
     @Test
     public void access_control_client_auth_can_be_overridden() {
-        Http http = createModelAndGetHttp(
-                "  <http>",
-                "    <filtering>",
-                "      <access-control tls-handshake-client-auth=\"want\"/>",
-                "    </filtering>",
-                "  </http>");
+        AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
+        DeployState state = new DeployState.Builder().properties(
+                new TestProperties()
+                        .setAthenzDomain(tenantDomain)
+                        .setHostedVespa(true)
+                        .allowDisableMtls(true))
+                .build();
+        Http http = createModelAndGetHttp(state,
+                                          "  <http>",
+                                          "    <filtering>",
+                                          "      <access-control tls-handshake-client-auth=\"want\"/>",
+                                          "    </filtering>",
+                                          "  </http>");
         assertTrue(http.getAccessControl().isPresent());
         assertEquals(AccessControl.ClientAuthentication.want, http.getAccessControl().get().clientAuthentication);
     }
 
     @Test
+    public void access_control_client_auth_cannot_be_overridden_when_disabled() {
+        AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
+        DeployState state = new DeployState.Builder().properties(
+                new TestProperties()
+                        .setAthenzDomain(tenantDomain)
+                        .setHostedVespa(true)
+                        .allowDisableMtls(false))
+                .build();
+
+        try {
+            Http http = createModelAndGetHttp(state,
+                                              "  <http>",
+                                              "    <filtering>",
+                                              "      <access-control tls-handshake-client-auth=\"want\"/>",
+                                              "    </filtering>",
+                                              "  </http>");
+            fail("Overriding tls-handshake-client-auth allowed, but should have failed");
+        } catch (IllegalArgumentException e) {
+            assertEquals("Overriding 'tls-handshake-client-auth' for application is not allowed.", e.getMessage());
+        }
+    }
+
+    @Test
     public void local_connector_has_default_chain() {
         Http http = createModelAndGetHttp(
                 "  <http>",
@@ -323,17 +354,20 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
     }
 
     private Http createModelAndGetHttp(String... httpElement) {
-        List<String> servicesXml = new ArrayList<>();
-        servicesXml.add("<container version='1.0'>");
-        servicesXml.addAll(List.of(httpElement));
-        servicesXml.add("</container>");
-
         AthenzDomain tenantDomain = AthenzDomain.from("my-tenant-domain");
         DeployState state = new DeployState.Builder().properties(
                 new TestProperties()
                         .setAthenzDomain(tenantDomain)
                         .setHostedVespa(true))
                 .build();
+        return createModelAndGetHttp(state, httpElement);
+    }
+    private Http createModelAndGetHttp(DeployState state, String... httpElement) {
+        List<String> servicesXml = new ArrayList<>();
+        servicesXml.add("<container version='1.0'>");
+        servicesXml.addAll(List.of(httpElement));
+        servicesXml.add("</container>");
+
         createModel(root, state, null, DomBuilderTest.parse(servicesXml.toArray(String[]::new)));
         return  ((ApplicationContainer) root.getProducer("container/container.0")).getHttp();
     }
diff --git a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
index 34220b64808..8c2be6a5b07 100644
--- a/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
+++ b/configserver/src/main/java/com/yahoo/vespa/config/server/deploy/ModelContextImpl.java
@@ -28,6 +28,7 @@ import com.yahoo.config.provision.TenantName;
 import com.yahoo.config.provision.Zone;
 import com.yahoo.container.jdisc.secretstore.SecretStore;
 import com.yahoo.vespa.config.server.tenant.SecretStoreExternalIdRetriever;
+import com.yahoo.vespa.flags.BooleanFlag;
 import com.yahoo.vespa.flags.FetchVector;
 import com.yahoo.vespa.flags.FlagSource;
 import com.yahoo.vespa.flags.Flags;
@@ -295,6 +296,7 @@ public class ModelContextImpl implements ModelContext {
         private final List<TenantSecretStore> tenantSecretStores;
         private final SecretStore secretStore;
         private final StringFlag jvmGCOptionsFlag;
+        private final boolean allowDisableMtls;
 
         public Properties(ApplicationId applicationId,
                           ConfigserverConfig configserverConfig,
@@ -329,6 +331,8 @@ public class ModelContextImpl implements ModelContext {
             this.secretStore = secretStore;
             this.jvmGCOptionsFlag = PermanentFlags.JVM_GC_OPTIONS.bindTo(flagSource)
                                                                  .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm());
+            this.allowDisableMtls = PermanentFlags.ALLOW_DISABLE_MTLS.bindTo(flagSource)
+                    .with(FetchVector.Dimension.APPLICATION_ID, applicationId.serializedForm()).value();
         }
 
         @Override public ModelContext.FeatureFlags featureFlags() { return featureFlags; }
@@ -392,6 +396,10 @@ public class ModelContextImpl implements ModelContext {
             return flagValueForClusterType(jvmGCOptionsFlag, clusterType);
         }
 
+        @Override
+        public boolean allowDisableMtls() {
+            return allowDisableMtls;
+        }
 
         public String flagValueForClusterType(StringFlag flag, Optional<ClusterSpec.Type> clusterType) {
             return clusterType.map(type -> flag.with(CLUSTER_TYPE, type.name()))
diff --git a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
index b7b5398b6b7..37f688ff8d4 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
@@ -151,6 +151,12 @@ public class PermanentFlags {
             "Takes effect immediately"
     );
 
+    public static final UnboundBooleanFlag ALLOW_DISABLE_MTLS = defineFeatureFlag(
+            "allow-disable-mtls", false,
+            "Allow application to disable client authentication",
+            "Takes effect on redeployment",
+            APPLICATION_ID);
+
     private PermanentFlags() {}
 
     private static UnboundBooleanFlag defineFeatureFlag(