diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java index 1a7641ef64b..79ccaeaae1a 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java @@ -619,12 +619,19 @@ public class SSLService { final Map sslSettings = new HashMap<>(); final String prefix = "xpack.security.authc.realms."; final Map settingsByRealmType = settings.getGroups(prefix); - settingsByRealmType.forEach((realmType, typeSettings) -> - typeSettings.getAsGroups().forEach((realmName, realmSettings) -> { - Settings realmSSLSettings = realmSettings.getByPrefix("ssl."); - // Put this even if empty, so that the name will be mapped to the global SSL configuration - sslSettings.put(prefix + realmType + "." + realmName + ".ssl", realmSSLSettings); - }) + settingsByRealmType.forEach((realmType, typeSettings) -> { + final Optional nonDottedSetting = typeSettings.keySet().stream().filter(k -> k.indexOf('.') == -1).findAny(); + if (nonDottedSetting.isPresent()) { + logger.warn("Skipping any SSL configuration from realm [{}{}] because the key [{}] is not in the correct format", + prefix, realmType, nonDottedSetting.get()); + } else { + typeSettings.getAsGroups().forEach((realmName, realmSettings) -> { + Settings realmSSLSettings = realmSettings.getByPrefix("ssl."); + // Put this even if empty, so that the name will be mapped to the global SSL configuration + sslSettings.put(prefix + realmType + "." + realmName + ".ssl", realmSSLSettings); + }); + } + } ); return sslSettings; } diff --git a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java index 3a735332a6b..ad9f1d7aa94 100644 --- a/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java +++ b/x-pack/plugin/security/src/main/java/org/elasticsearch/xpack/security/Security.java @@ -115,6 +115,8 @@ import org.elasticsearch.xpack.core.security.authc.AuthenticationServiceField; import org.elasticsearch.xpack.core.security.authc.DefaultAuthenticationFailureHandler; import org.elasticsearch.xpack.core.security.authc.InternalRealmsSettings; import org.elasticsearch.xpack.core.security.authc.Realm; +import org.elasticsearch.xpack.core.security.authc.RealmConfig; +import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.security.authc.support.UsernamePasswordToken; import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; @@ -244,6 +246,7 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; import java.util.function.UnaryOperator; +import java.util.stream.Collectors; import static java.util.Collections.emptyList; import static java.util.Collections.singletonList; @@ -296,7 +299,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw this.env = transportClientMode ? null : new Environment(settings, configPath); this.enabled = XPackSettings.SECURITY_ENABLED.get(settings); if (enabled && transportClientMode == false) { - validateAutoCreateIndex(settings); + runStartupChecks(settings); // we load them all here otherwise we can't access secure settings since they are closed once the checks are // fetched final List checks = new ArrayList<>(); @@ -315,6 +318,12 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw this.bootstrapChecks = Collections.emptyList(); } this.securityExtensions.addAll(extensions); + + } + + private static void runStartupChecks(Settings settings) { + validateAutoCreateIndex(settings); + validateRealmSettings(settings); } @Override @@ -781,6 +790,40 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw return Collections.singletonMap(SetSecurityUserProcessor.TYPE, new SetSecurityUserProcessor.Factory(parameters.threadContext)); } + /** + * Realm settings were changed in 7.0. This method validates that the settings in use on this node match the new style of setting. + * In 6.x a realm config would be + * 
+     *   xpack.security.authc.realms.file1.type: file
+     *   xpack.security.authc.realms.file1.order: 0
+     *
+ * In 7.x this realm should be + * 
+     *   xpack.security.authc.realms.file.file1.order: 0
+     *
+ * If confronted with an old style config, the ES Settings validation would simply fail with an error such as + * unknown setting [xpack.security.authc.realms.file1.order]. This validation method provides an error that is easier to + * understand and take action on. + */ + static void validateRealmSettings(Settings settings) { + final Set badRealmSettings = settings.keySet().stream() + .filter(k -> k.startsWith(RealmSettings.PREFIX)) + .filter(key -> { + final String suffix = key.substring(RealmSettings.PREFIX.length()); + // suffix-part, only contains a single '.' + return suffix.indexOf('.') == suffix.lastIndexOf('.'); + }) + .collect(Collectors.toSet()); + if (badRealmSettings.isEmpty() == false) { + String sampleRealmSetting = RealmSettings.realmSettingPrefix(new RealmConfig.RealmIdentifier("file", "my_file")) + "order"; + throw new IllegalArgumentException("Incorrect realm settings found. " + + "Realm settings have been changed to include the type as part of the setting key.\n" + + "For example '" + sampleRealmSetting + "'\n" + + "Found invalid config: " + Strings.collectionToDelimitedString(badRealmSettings, ", ") + "\n" + + "Please see the breaking changes documentation." + ); + } + } static boolean indexAuditLoggingEnabled(Settings settings) { if (XPackSettings.AUDIT_ENABLED.get(settings)) { diff --git a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java index e9924b9d852..8674a5b2950 100644 --- a/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java +++ b/x-pack/plugin/security/src/test/java/org/elasticsearch/xpack/security/SecurityTests.java @@ -35,6 +35,7 @@ import org.elasticsearch.xpack.core.XPackSettings; import org.elasticsearch.xpack.core.security.SecurityExtension; import org.elasticsearch.xpack.core.security.SecurityField; import org.elasticsearch.xpack.core.security.authc.Realm; +import org.elasticsearch.xpack.core.security.authc.RealmSettings; import org.elasticsearch.xpack.core.security.authc.file.FileRealmSettings; import org.elasticsearch.xpack.core.security.authz.AuthorizationServiceField; import org.elasticsearch.xpack.core.security.authz.accesscontrol.IndicesAccessControl; @@ -66,8 +67,8 @@ import java.util.stream.Collectors; import static org.elasticsearch.cluster.metadata.IndexMetaData.INDEX_FORMAT_SETTING; import static org.elasticsearch.discovery.DiscoveryModule.ZEN2_DISCOVERY_TYPE; import static org.elasticsearch.discovery.DiscoveryModule.ZEN_DISCOVERY_TYPE; -import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME; import static org.elasticsearch.xpack.security.support.SecurityIndexManager.INTERNAL_INDEX_FORMAT; +import static org.elasticsearch.xpack.security.support.SecurityIndexManager.SECURITY_INDEX_NAME; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.equalTo; @@ -99,9 +100,9 @@ public class SecurityTests extends ESTestCase { throw new IllegalStateException("Security object already exists (" + security + ")"); } Settings settings = Settings.builder() - .put("xpack.security.enabled", true) - .put(testSettings) - .put("path.home", createTempDir()).build(); + .put("xpack.security.enabled", true) + .put(testSettings) + .put("path.home", createTempDir()).build(); Environment env = TestEnvironment.newEnvironment(settings); licenseState = new TestUtils.UpdatableLicenseState(settings); SSLService sslService = new SSLService(settings, env); @@ -159,7 +160,7 @@ public class SecurityTests extends ESTestCase { public void testCustomRealmExtensionConflict() throws Exception { IllegalArgumentException e = expectThrows(IllegalArgumentException.class, - () -> createComponents(Settings.EMPTY, new DummyExtension(FileRealmSettings.TYPE))); + () -> createComponents(Settings.EMPTY, new DummyExtension(FileRealmSettings.TYPE))); assertEquals("Realm type [" + FileRealmSettings.TYPE + "] is already registered", e.getMessage()); } @@ -181,8 +182,8 @@ public class SecurityTests extends ESTestCase { public void testIndexAuditTrail() throws Exception { Settings settings = Settings.builder() - .put(XPackSettings.AUDIT_ENABLED.getKey(), true) - .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "index").build(); + .put(XPackSettings.AUDIT_ENABLED.getKey(), true) + .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "index").build(); Collection components = createComponents(settings); AuditTrailService service = findComponent(AuditTrailService.class, components); assertNotNull(service); @@ -192,8 +193,8 @@ public class SecurityTests extends ESTestCase { public void testIndexAndLoggingAuditTrail() throws Exception { Settings settings = Settings.builder() - .put(XPackSettings.AUDIT_ENABLED.getKey(), true) - .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "index,logfile").build(); + .put(XPackSettings.AUDIT_ENABLED.getKey(), true) + .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "index,logfile").build(); Collection components = createComponents(settings); AuditTrailService service = findComponent(AuditTrailService.class, components); assertNotNull(service); @@ -204,8 +205,8 @@ public class SecurityTests extends ESTestCase { public void testUnknownOutput() { Settings settings = Settings.builder() - .put(XPackSettings.AUDIT_ENABLED.getKey(), true) - .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "foo").build(); + .put(XPackSettings.AUDIT_ENABLED.getKey(), true) + .put(Security.AUDIT_OUTPUTS_SETTING.getKey(), "foo").build(); IllegalArgumentException e = expectThrows(IllegalArgumentException.class, () -> createComponents(settings)); assertEquals("Unknown audit trail output [foo]", e.getMessage()); } @@ -218,9 +219,9 @@ public class SecurityTests extends ESTestCase { public void testTransportSettingNetty4Both() { Settings both4 = Security.additionalSettings(Settings.builder() - .put(NetworkModule.TRANSPORT_TYPE_KEY, SecurityField.NAME4) - .put(NetworkModule.HTTP_TYPE_KEY, SecurityField.NAME4) - .build(), true, false); + .put(NetworkModule.TRANSPORT_TYPE_KEY, SecurityField.NAME4) + .put(NetworkModule.HTTP_TYPE_KEY, SecurityField.NAME4) + .build(), true, false); assertFalse(NetworkModule.TRANSPORT_TYPE_SETTING.exists(both4)); assertFalse(NetworkModule.HTTP_TYPE_SETTING.exists(both4)); } @@ -229,13 +230,13 @@ public class SecurityTests extends ESTestCase { final String badType = randomFrom("netty4", "other", "security1"); Settings settingsTransport = Settings.builder().put(NetworkModule.TRANSPORT_TYPE_KEY, badType).build(); IllegalArgumentException badTransport = expectThrows(IllegalArgumentException.class, - () -> Security.additionalSettings(settingsTransport, true, false)); + () -> Security.additionalSettings(settingsTransport, true, false)); assertThat(badTransport.getMessage(), containsString(SecurityField.NAME4)); assertThat(badTransport.getMessage(), containsString(NetworkModule.TRANSPORT_TYPE_KEY)); Settings settingsHttp = Settings.builder().put(NetworkModule.HTTP_TYPE_KEY, badType).build(); IllegalArgumentException badHttp = expectThrows(IllegalArgumentException.class, - () -> Security.additionalSettings(settingsHttp, true, false)); + () -> Security.additionalSettings(settingsHttp, true, false)); assertThat(badHttp.getMessage(), containsString(SecurityField.NAME4)); assertThat(badHttp.getMessage(), containsString(NetworkModule.HTTP_TYPE_KEY)); } @@ -249,21 +250,21 @@ public class SecurityTests extends ESTestCase { public void testFilteredSettings() throws Exception { createComponents(Settings.EMPTY); final List> realmSettings = security.getSettings().stream() - .filter(s -> s.getKey().startsWith("xpack.security.authc.realms")) - .collect(Collectors.toList()); + .filter(s -> s.getKey().startsWith("xpack.security.authc.realms")) + .collect(Collectors.toList()); Arrays.asList( - "bind_dn", "bind_password", - "hostname_verification", - "truststore.password", "truststore.path", "truststore.algorithm", - "keystore.key_password").forEach(suffix -> { + "bind_dn", "bind_password", + "hostname_verification", + "truststore.password", "truststore.path", "truststore.algorithm", + "keystore.key_password").forEach(suffix -> { final List> matching = realmSettings.stream() - .filter(s -> s.getKey().endsWith("." + suffix)) - .collect(Collectors.toList()); + .filter(s -> s.getKey().endsWith("." + suffix)) + .collect(Collectors.toList()); assertThat("For suffix " + suffix, matching, Matchers.not(empty())); matching.forEach(setting -> assertThat("For setting " + setting, - setting.getProperties(), Matchers.hasItem(Setting.Property.Filtered))); + setting.getProperties(), Matchers.hasItem(Setting.Property.Filtered))); }); } @@ -290,7 +291,7 @@ public class SecurityTests extends ESTestCase { TestUtils.putLicense(builder, license); ClusterState state = ClusterState.builder(ClusterName.DEFAULT).metaData(builder.build()).build(); EnumSet productionModes = EnumSet.of(License.OperationMode.GOLD, License.OperationMode.PLATINUM, - License.OperationMode.STANDARD); + License.OperationMode.STANDARD); if (productionModes.contains(license.operationMode()) && tlsOn == false && "single-node".equals(discoveryType) == false) { IllegalStateException ise = expectThrows(IllegalStateException.class, () -> validator.accept(node, state)); assertEquals("TLS setup is required for license type [" + license.operationMode().name() + "]", ise.getMessage()); @@ -340,18 +341,18 @@ public class SecurityTests extends ESTestCase { assertNotNull(joinValidator); DiscoveryNode node = new DiscoveryNode("foo", buildNewFakeTransportAddress(), Version.CURRENT); IndexMetaData indexMetaData = IndexMetaData.builder(SECURITY_INDEX_NAME) - .settings(settings(Version.V_6_1_0).put(INDEX_FORMAT_SETTING.getKey(), INTERNAL_INDEX_FORMAT - 1)) - .numberOfShards(1).numberOfReplicas(0) - .build(); + .settings(settings(Version.V_6_1_0).put(INDEX_FORMAT_SETTING.getKey(), INTERNAL_INDEX_FORMAT - 1)) + .numberOfShards(1).numberOfReplicas(0) + .build(); DiscoveryNode existingOtherNode = new DiscoveryNode("bar", buildNewFakeTransportAddress(), Version.V_6_1_0); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(existingOtherNode).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) - .nodes(discoveryNodes) - .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); + .nodes(discoveryNodes) + .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); IllegalStateException e = expectThrows(IllegalStateException.class, - () -> joinValidator.accept(node, clusterState)); + () -> joinValidator.accept(node, clusterState)); assertThat(e.getMessage(), equalTo("Security index is not on the current version [6] - " + - "The Upgrade API must be run for 7.x nodes to join the cluster")); + "The Upgrade API must be run for 7.x nodes to join the cluster")); } public void testIndexJoinValidator_FullyCurrentCluster() throws Exception { @@ -361,14 +362,14 @@ public class SecurityTests extends ESTestCase { DiscoveryNode node = new DiscoveryNode("foo", buildNewFakeTransportAddress(), Version.CURRENT); int indexFormat = randomBoolean() ? INTERNAL_INDEX_FORMAT : INTERNAL_INDEX_FORMAT - 1; IndexMetaData indexMetaData = IndexMetaData.builder(SECURITY_INDEX_NAME) - .settings(settings(Version.V_6_1_0).put(INDEX_FORMAT_SETTING.getKey(), indexFormat)) - .numberOfShards(1).numberOfReplicas(0) - .build(); + .settings(settings(Version.V_6_1_0).put(INDEX_FORMAT_SETTING.getKey(), indexFormat)) + .numberOfShards(1).numberOfReplicas(0) + .build(); DiscoveryNode existingOtherNode = new DiscoveryNode("bar", buildNewFakeTransportAddress(), Version.CURRENT); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(existingOtherNode).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) - .nodes(discoveryNodes) - .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); + .nodes(discoveryNodes) + .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); joinValidator.accept(node, clusterState); } @@ -379,14 +380,14 @@ public class SecurityTests extends ESTestCase { Version version = randomBoolean() ? Version.CURRENT : Version.V_6_1_0; DiscoveryNode node = new DiscoveryNode("foo", buildNewFakeTransportAddress(), Version.CURRENT); IndexMetaData indexMetaData = IndexMetaData.builder(SECURITY_INDEX_NAME) - .settings(settings(version).put(INDEX_FORMAT_SETTING.getKey(), INTERNAL_INDEX_FORMAT)) - .numberOfShards(1).numberOfReplicas(0) - .build(); + .settings(settings(version).put(INDEX_FORMAT_SETTING.getKey(), INTERNAL_INDEX_FORMAT)) + .numberOfShards(1).numberOfReplicas(0) + .build(); DiscoveryNode existingOtherNode = new DiscoveryNode("bar", buildNewFakeTransportAddress(), version); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(existingOtherNode).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) - .nodes(discoveryNodes) - .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); + .nodes(discoveryNodes) + .metaData(MetaData.builder().put(indexMetaData, true).build()).build(); joinValidator.accept(node, clusterState); } @@ -398,7 +399,7 @@ public class SecurityTests extends ESTestCase { DiscoveryNode existingOtherNode = new DiscoveryNode("bar", buildNewFakeTransportAddress(), Version.V_6_1_0); DiscoveryNodes discoveryNodes = DiscoveryNodes.builder().add(existingOtherNode).build(); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT) - .nodes(discoveryNodes).build(); + .nodes(discoveryNodes).build(); joinValidator.accept(node, clusterState); } @@ -409,15 +410,15 @@ public class SecurityTests extends ESTestCase { Map permissionsMap = new HashMap<>(); FieldPermissions permissions = new FieldPermissions( - new FieldPermissionsDefinition(new String[]{"field_granted"}, Strings.EMPTY_ARRAY)); + new FieldPermissionsDefinition(new String[] { "field_granted" }, Strings.EMPTY_ARRAY)); IndicesAccessControl.IndexAccessControl indexGrantedAccessControl = new IndicesAccessControl.IndexAccessControl(true, permissions, - Collections.emptySet()); + Collections.emptySet()); permissionsMap.put("index_granted", indexGrantedAccessControl); IndicesAccessControl.IndexAccessControl indexAccessControl = new IndicesAccessControl.IndexAccessControl(false, - FieldPermissions.DEFAULT, Collections.emptySet()); + FieldPermissions.DEFAULT, Collections.emptySet()); permissionsMap.put("index_not_granted", indexAccessControl); IndicesAccessControl.IndexAccessControl nullFieldPermissions = - new IndicesAccessControl.IndexAccessControl(true, null, Collections.emptySet()); + new IndicesAccessControl.IndexAccessControl(true, null, Collections.emptySet()); permissionsMap.put("index_null", nullFieldPermissions); IndicesAccessControl index = new IndicesAccessControl(true, permissionsMap); threadContext.putTransient(AuthorizationServiceField.INDICES_PERMISSIONS_KEY, index); @@ -446,4 +447,25 @@ public class SecurityTests extends ESTestCase { assertNotSame(MapperPlugin.NOOP_FIELD_FILTER, fieldFilter); assertSame(MapperPlugin.NOOP_FIELD_PREDICATE, fieldFilter.apply(randomAlphaOfLengthBetween(3, 6))); } + + public void testValidateRealmsWhenSettingsAreInvalid() { + final Settings settings = Settings.builder() + .put(RealmSettings.PREFIX + "my_pki.type", "pki") + .put(RealmSettings.PREFIX + "ldap1.type", "ldap") + .build(); + final IllegalArgumentException iae = expectThrows(IllegalArgumentException.class, () -> Security.validateRealmSettings(settings)); + assertThat(iae.getMessage(), containsString("Incorrect realm settings")); + assertThat(iae.getMessage(), containsString("breaking changes doc")); + assertThat(iae.getMessage(), containsString(RealmSettings.PREFIX + "my_pki.type")); + assertThat(iae.getMessage(), containsString(RealmSettings.PREFIX + "ldap1.type")); + } + + public void testValidateRealmsWhenSettingsAreCorrect() { + final Settings settings = Settings.builder() + .put(RealmSettings.PREFIX + "pki.my_pki.order", 0) + .put(RealmSettings.PREFIX + "ldap.ldap1.order", 1) + .build(); + Security.validateRealmSettings(settings); + // no-exception + } }