diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java index 60584de13f6..23424a4ed39 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java @@ -135,11 +135,11 @@ public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, final Settings settings, final Path configPath) { super(settings); + // FIXME: The settings might be changed after this (e.g. from "additionalSettings" method in other plugins) + // We should only depend on the settings from the Environment object passed to createComponents this.settings = settings; this.transportClientMode = transportClientMode(settings); - Environment env = transportClientMode ? null : new Environment(settings, configPath); - setSslService(new SSLService(settings, env)); setLicenseState(new XPackLicenseState(settings)); this.licensing = new Licensing(settings); @@ -156,7 +156,14 @@ public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, protected void setSslService(SSLService sslService) { XPackPlugin.sslService.set(sslService); } protected void setLicenseService(LicenseService licenseService) { XPackPlugin.licenseService.set(licenseService); } protected void setLicenseState(XPackLicenseState licenseState) { XPackPlugin.licenseState.set(licenseState); } - public static SSLService getSharedSslService() { return sslService.get(); } + + public static SSLService getSharedSslService() { + final SSLService ssl = XPackPlugin.sslService.get(); + if (ssl == null) { + throw new IllegalStateException("SSL Service is not constructed yet"); + } + return ssl; + } public static LicenseService getSharedLicenseService() { return licenseService.get(); } public static XPackLicenseState getSharedLicenseState() { return licenseState.get(); } @@ -249,14 +256,16 @@ public class XPackPlugin extends XPackClientPlugin implements ExtensiblePlugin, NodeEnvironment nodeEnvironment, NamedWriteableRegistry namedWriteableRegistry) { List components = new ArrayList<>(); + final SSLService sslService = new SSLService(environment); + setSslService(sslService); // just create the reloader as it will pull all of the loaded ssl configurations and start watching them - new SSLConfigurationReloader(environment, getSslService(), resourceWatcherService); + new SSLConfigurationReloader(environment, sslService, resourceWatcherService); setLicenseService(new LicenseService(settings, clusterService, getClock(), environment, resourceWatcherService, getLicenseState())); // It is useful to override these as they are what guice is injecting into actions - components.add(getSslService()); + components.add(sslService); components.add(getLicenseService()); components.add(getLicenseState()); 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 e79583589d1..ce61422d0a0 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 @@ -128,6 +128,14 @@ public class SSLService { private final SetOnce transportSSLConfiguration = new SetOnce<>(); private final Environment env; + /** + * Create a new SSLService using the {@code Settings} from {@link Environment#settings()}. + * @see #SSLService(Settings, Environment) + */ + public SSLService(Environment environment) { + this(environment.settings(), environment); + } + /** * Create a new SSLService that parses the settings for the ssl contexts that need to be created, creates them, and then caches them * for use later 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 d9d03dc670f..7b22659c407 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 @@ -294,7 +294,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw private final SetOnce securityIndex = new SetOnce<>(); private final SetOnce groupFactory = new SetOnce<>(); private final SetOnce dlsBitsetCache = new SetOnce<>(); - private final List bootstrapChecks; + private final SetOnce> bootstrapChecks = new SetOnce<>(); private final List securityExtensions = new ArrayList<>(); public Security(Settings settings, final Path configPath) { @@ -302,25 +302,20 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw } Security(Settings settings, final Path configPath, List extensions) { + // TODO This is wrong. Settings can change after this. We should use the settings from createComponents this.settings = settings; this.transportClientMode = XPackPlugin.transportClientMode(settings); + // TODO this is wrong, we should only use the environment that is provided to createComponents this.env = transportClientMode ? null : new Environment(settings, configPath); this.enabled = XPackSettings.SECURITY_ENABLED.get(settings); if (enabled && transportClientMode == false) { 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<>(); - checks.addAll(Arrays.asList( - new ApiKeySSLBootstrapCheck(), - new TokenSSLBootstrapCheck(), - new PkiRealmBootstrapCheck(getSslService()), - new TLSLicenseBootstrapCheck())); - checks.addAll(InternalRealms.getBootstrapChecks(settings, env)); - this.bootstrapChecks = Collections.unmodifiableList(checks); + Automatons.updateConfiguration(settings); } else { - this.bootstrapChecks = Collections.emptyList(); + this.bootstrapChecks.set(Collections.emptyList()); } this.securityExtensions.addAll(extensions); @@ -346,7 +341,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw } modules.add(b -> { // for transport client we still must inject these ssl classes with guice - b.bind(SSLService.class).toInstance(getSslService()); + b.bind(SSLService.class).toProvider(this::getSslService); }); return modules; @@ -403,6 +398,17 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw return Collections.emptyList(); } + // We need to construct the checks here while the secure settings are still available. + // If we wait until #getBoostrapChecks the secure settings will have been cleared/closed. + final List checks = new ArrayList<>(); + checks.addAll(Arrays.asList( + new ApiKeySSLBootstrapCheck(), + new TokenSSLBootstrapCheck(), + new PkiRealmBootstrapCheck(getSslService()), + new TLSLicenseBootstrapCheck())); + checks.addAll(InternalRealms.getBootstrapChecks(settings, env)); + this.bootstrapChecks.set(Collections.unmodifiableList(checks)); + threadContext.set(threadPool.getThreadContext()); List components = new ArrayList<>(); securityContext.set(new SecurityContext(settings, threadPool.getThreadContext())); @@ -698,7 +704,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw @Override public List getBootstrapChecks() { - return bootstrapChecks; + return bootstrapChecks.get(); } @Override