Skip to content

Commit

Permalink
[Kerberos] Remove Kerberos bootstrap checks (#32451)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
bizybot committed Jul 31, 2018
1 parent d75efbc commit f0b3667
Show file tree
Hide file tree
Showing 7 changed files with 106 additions and 199 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,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;
Expand Down Expand Up @@ -306,8 +305,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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;


/**
Expand Down Expand Up @@ -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");
Expand All @@ -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));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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());
}
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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));
}
Expand All @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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) {
Expand Down

This file was deleted.

Loading

0 comments on commit f0b3667

Please sign in to comment.