honeymoose/OpenSearch
1
0
Fork 0
mirror of https://github.com/honeymoose/OpenSearch.git synced 2025-02-17 02:14:54 +00:00
Code Issues Packages Projects Releases Wiki Activity

Do not load SSLService in plugin contructor (#50519)

Browse Source 
XPackPlugin created an SSLService within the plugin contructor.
This has 2 negative consequences:

1. The service may be constructed based on a partial view of settings.
   Other plugins are free to add setting values via the
   additionalSettings() method, but this (necessarily) happens after
   plugins have been constructed.

2. Any exceptions thrown during the plugin construction are handled
   differently than exceptions thrown during "createComponents".
   Since SSL configurations exceptions are relatively common, it is
   far preferable for them to be thrown and handled as part of the
   createComponents flow.

This commit moves the creation of the SSLService to
XPackPlugin.createComponents, and alters the sequence of some other
steps to accommodate this change.

Backport of: #49667
This commit is contained in:
Tim Vernum 2019-12-30 14:42:32 +11:00 committed by GitHub
parent 72840c0cb2
commit cad0f6bf28
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java
View File

@ -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<Object> 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());

8
x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ssl/SSLService.java
View File

@ -128,6 +128,14 @@ public class SSLService {
private final SetOnce<SSLConfiguration> 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

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

@ -294,7 +294,7 @@ public class Security extends Plugin implements ActionPlugin, IngestPlugin, Netw
private final SetOnce<SecurityIndexManager> securityIndex = new SetOnce<>();
private final SetOnce<NioGroupFactory> groupFactory = new SetOnce<>();
private final SetOnce<DocumentSubsetBitsetCache> dlsBitsetCache = new SetOnce<>();
private final List<BootstrapCheck> bootstrapChecks;
private final SetOnce<List<BootstrapCheck>> bootstrapChecks = new SetOnce<>();
private final List<SecurityExtension> 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<SecurityExtension> 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<BootstrapCheck> 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<BootstrapCheck> 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<Object> 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<BootstrapCheck> getBootstrapChecks() {
return bootstrapChecks;
return bootstrapChecks.get();
}
@Override