Revert "Revert "Revert "Flag to allow disabling mtls"""

6 files changed, 17 insertions, 77 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 3a2f71e2b8c..3ac8d83f401 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,8 +132,6 @@ 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 75a1a167446..59930a77a21 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,7 +61,6 @@ 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; }
@@ -103,7 +102,6 @@ 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;
@@ -250,11 +248,6 @@ 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 5c8028575aa..5417a522d6a 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,16 +78,13 @@ public class HttpBuilder extends VespaDomBuilder.DomConfigProducerBuilder<Http>
                 readAttr -> builder.readEnabled(Boolean.valueOf(readAttr)));
         XmlHelper.getOptionalAttribute(accessControlElem, "write").ifPresent(
                 writeAttr -> builder.writeEnabled(Boolean.valueOf(writeAttr)));
-
-        AccessControl.ClientAuthentication clientAuth =
+        builder.clientAuthentication(
                 XmlHelper.getOptionalAttribute(accessControlElem, "tls-handshake-client-auth")
-                        .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);
+                        .map(value -> "want".equals(value)
+                                ? AccessControl.ClientAuthentication.want
+                                : AccessControl.ClientAuthentication.need)
+                        .orElse(AccessControl.ClientAuthentication.need)
+        );
 
         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 39b5cc139d9..4993a51ab74 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,7 +33,6 @@ 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
@@ -291,47 +290,17 @@ public class AccessControlTest extends ContainerModelBuilderTestBase {
 
     @Test
     public void access_control_client_auth_can_be_overridden() {
-        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>");
+        Http http = createModelAndGetHttp(
+                "  <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>",
@@ -354,20 +323,17 @@ 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 8c2be6a5b07..34220b64808 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,7 +28,6 @@ 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;
@@ -296,7 +295,6 @@ 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,
@@ -331,8 +329,6 @@ 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; }
@@ -396,10 +392,6 @@ 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 37f688ff8d4..b7b5398b6b7 100644
--- a/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
+++ b/flags/src/main/java/com/yahoo/vespa/flags/PermanentFlags.java
@@ -151,12 +151,6 @@ 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(