[Kerberos] Remove Kerberos bootstrap checks (#32451)

This commit removes Kerberos bootstrap checks as they were more
validation checks and better done in Kerberos realm constructor
than as bootstrap checks. This also moves the check
for one Kerberos realm per node to where we initialize realms.
This commit adds few validations which were missing earlier
like missing read permissions on keytab file or if it is directory
to throw exception with error message.

Author: bizybot

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java

@@ -178,7 +178,6 @@
 import org.elasticsearch.xpack.security.authc.TokenService;
 import org.elasticsearch.xpack.security.authc.esnative.NativeUsersStore;
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
-import org.elasticsearch.xpack.security.authc.kerberos.KerberosRealmBootstrapCheck;
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
 import org.elasticsearch.xpack.security.authz.AuthorizationService;
 import org.elasticsearch.xpack.security.authz.SecuritySearchOperationListener;
@@ -315,8 +314,7 @@ public Security(Settings settings, final Path configPath) {
                 new PasswordHashingAlgorithmBootstrapCheck(),
                 new FIPS140SecureSettingsBootstrapCheck(settings, env),
                 new FIPS140JKSKeystoreBootstrapCheck(settings),
-                new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings),
-                new KerberosRealmBootstrapCheck(env)));
+                new FIPS140PasswordHashingAlgorithmBootstrapCheck(settings)));
             checks.addAll(InternalRealms.getBootstrapChecks(settings, env));
             this.bootstrapChecks = Collections.unmodifiableList(checks);
             Automatons.updateMaxDeterminizedStates(settings);

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/Realms.java

@@ -35,6 +35,7 @@
 import org.elasticsearch.xpack.core.security.authc.esnative.NativeRealmSettings;
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings;
+import org.elasticsearch.xpack.core.security.authc.kerberos.KerberosRealmSettings;
 
 
 /**
@@ -152,6 +153,7 @@ protected List<Realm> initRealms() throws Exception {
         Settings realmsSettings = RealmSettings.get(settings);
         Set<String> internalTypes = new HashSet<>();
         List<Realm> realms = new ArrayList<>();
+        List<String> kerberosRealmNames = new ArrayList<>();
         for (String name : realmsSettings.names()) {
             Settings realmSettings = realmsSettings.getAsSettings(name);
             String type = realmSettings.get("type");
@@ -178,6 +180,13 @@ protected List<Realm> initRealms() throws Exception {
                 }
                 internalTypes.add(type);
             }
+            if (KerberosRealmSettings.TYPE.equals(type)) {
+                kerberosRealmNames.add(name);
+                if (kerberosRealmNames.size() > 1) {
+                    throw new IllegalArgumentException("multiple realms " + kerberosRealmNames.toString() + " configured of type [" + type
+                            + "], [" + type + "] can only have one such realm configured");
+                }
+            }
             realms.add(factory.create(config));
         }
 

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealm.java

@@ -25,6 +25,7 @@
 import org.elasticsearch.xpack.security.authc.support.mapper.NativeRoleMappingStore;
 import org.ietf.jgss.GSSException;
 
+import java.nio.file.Files;
 import java.nio.file.Path;
 import java.util.Collections;
 import java.util.List;
@@ -87,6 +88,16 @@ public KerberosRealm(final RealmConfig config, final NativeRoleMappingStore nati
         this.kerberosTicketValidator = kerberosTicketValidator;
         this.threadPool = threadPool;
         this.keytabPath = config.env().configFile().resolve(KerberosRealmSettings.HTTP_SERVICE_KEYTAB_PATH.get(config.settings()));
+
+        if (Files.exists(keytabPath) == false) {
+            throw new IllegalArgumentException("configured service key tab file [" + keytabPath + "] does not exist");
+        }
+        if (Files.isDirectory(keytabPath)) {
+            throw new IllegalArgumentException("configured service key tab file [" + keytabPath + "] is a directory");
+        }
+        if (Files.isReadable(keytabPath) == false) {
+            throw new IllegalArgumentException("configured service key tab file [" + keytabPath + "] must have read permission");
+        }
         this.enableKerberosDebug = KerberosRealmSettings.SETTING_KRB_DEBUG_ENABLE.get(config.settings());
         this.removeRealmName = KerberosRealmSettings.SETTING_REMOVE_REALM_NAME.get(config.settings());
     }

x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/authc/kerberos/KerberosRealmBootstrapCheck.java

@@ -27,6 +27,7 @@
 import org.elasticsearch.xpack.security.authc.esnative.ReservedRealm;
 import org.junit.Before;
 
+import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
@@ -51,13 +52,16 @@ public class RealmsTests extends ESTestCase {
     private XPackLicenseState licenseState;
     private ThreadContext threadContext;
     private ReservedRealm reservedRealm;
+    private int randomRealmTypesCount;
 
     @Before
     public void init() throws Exception {
         factories = new HashMap<>();
         factories.put(FileRealmSettings.TYPE, config -> new DummyRealm(FileRealmSettings.TYPE, config));
         factories.put(NativeRealmSettings.TYPE, config -> new DummyRealm(NativeRealmSettings.TYPE, config));
-        for (int i = 0; i < randomIntBetween(1, 5); i++) {
+        factories.put(KerberosRealmSettings.TYPE, config -> new DummyRealm(KerberosRealmSettings.TYPE, config));
+        randomRealmTypesCount = randomIntBetween(1, 5);
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             String name = "type_" + i;
             factories.put(name, config -> new DummyRealm(name, config));
         }
@@ -73,13 +77,13 @@ public void init() throws Exception {
     public void testWithSettings() throws Exception {
         Settings.Builder builder = Settings.builder()
                 .put("path.home", createTempDir());
-        List<Integer> orders = new ArrayList<>(factories.size() - 2);
-        for (int i = 0; i < factories.size() - 2; i++) {
+        List<Integer> orders = new ArrayList<>(randomRealmTypesCount);
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             orders.add(i);
         }
         Collections.shuffle(orders, random());
         Map<Integer, Integer> orderToIndex = new HashMap<>();
-        for (int i = 0; i < factories.size() - 2; i++) {
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             builder.put("xpack.security.authc.realms.realm_" + i + ".type", "type_" + i);
             builder.put("xpack.security.authc.realms.realm_" + i + ".order", orders.get(i));
             orderToIndex.put(orders.get(i), i);
@@ -107,14 +111,14 @@ public void testWithSettings() throws Exception {
     public void testWithSettingsWhereDifferentRealmsHaveSameOrder() throws Exception {
         Settings.Builder builder = Settings.builder()
                 .put("path.home", createTempDir());
-        List<Integer> randomSeq = new ArrayList<>(factories.size() - 2);
-        for (int i = 0; i < factories.size() - 2; i++) {
+        List<Integer> randomSeq = new ArrayList<>(randomRealmTypesCount);
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             randomSeq.add(i);
         }
         Collections.shuffle(randomSeq, random());
 
         TreeMap<String, Integer> nameToRealmId = new TreeMap<>();
-        for (int i = 0; i < factories.size() - 2; i++) {
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             int randomizedRealmId = randomSeq.get(i);
             String randomizedRealmName = randomAlphaOfLengthBetween(12,32);
             nameToRealmId.put("realm_" + randomizedRealmName, randomizedRealmId);
@@ -181,13 +185,13 @@ public void testWithEmptySettings() throws Exception {
     public void testUnlicensedWithOnlyCustomRealms() throws Exception {
         Settings.Builder builder = Settings.builder()
                 .put("path.home", createTempDir());
-        List<Integer> orders = new ArrayList<>(factories.size() - 2);
-        for (int i = 0; i < factories.size() - 2; i++) {
+        List<Integer> orders = new ArrayList<>(randomRealmTypesCount);
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             orders.add(i);
         }
         Collections.shuffle(orders, random());
         Map<Integer, Integer> orderToIndex = new HashMap<>();
-        for (int i = 0; i < factories.size() - 2; i++) {
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             builder.put("xpack.security.authc.realms.realm_" + i + ".type", "type_" + i);
             builder.put("xpack.security.authc.realms.realm_" + i + ".order", orders.get(i));
             orderToIndex.put(orders.get(i), i);
@@ -384,13 +388,13 @@ public void testUnlicensedWithNonStandardRealms() throws Exception {
     public void testDisabledRealmsAreNotAdded() throws Exception {
         Settings.Builder builder = Settings.builder()
                 .put("path.home", createTempDir());
-        List<Integer> orders = new ArrayList<>(factories.size() - 2);
-        for (int i = 0; i < factories.size() - 2; i++) {
+        List<Integer> orders = new ArrayList<>(randomRealmTypesCount);
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             orders.add(i);
         }
         Collections.shuffle(orders, random());
         Map<Integer, Integer> orderToIndex = new HashMap<>();
-        for (int i = 0; i < factories.size() - 2; i++) {
+        for (int i = 0; i < randomRealmTypesCount; i++) {
             builder.put("xpack.security.authc.realms.realm_" + i + ".type", "type_" + i);
             builder.put("xpack.security.authc.realms.realm_" + i + ".order", orders.get(i));
             boolean enabled = randomBoolean();
@@ -520,6 +524,20 @@ public void testUsageStats() throws Exception {
         }
     }
 
+    public void testInitRealmsFailsForMultipleKerberosRealms() throws IOException {
+        final Settings.Builder builder = Settings.builder().put("path.home", createTempDir());
+        builder.put("xpack.security.authc.realms.realm_1.type", "kerberos");
+        builder.put("xpack.security.authc.realms.realm_1.order", 1);
+        builder.put("xpack.security.authc.realms.realm_2.type", "kerberos");
+        builder.put("xpack.security.authc.realms.realm_2.order", 2);
+        final Settings settings = builder.build();
+        Environment env = TestEnvironment.newEnvironment(settings);
+        final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class,
+                () -> new Realms(settings, env, factories, licenseState, threadContext, reservedRealm));
+        assertThat(iae.getMessage(), is(equalTo(
+                "multiple realms [realm_1, realm_2] configured of type [kerberos], [kerberos] can only have one such realm configured")));
+    }
+
     static class DummyRealm extends Realm {
 
         DummyRealm(String type, RealmConfig config) {