Skip to content

Commit

Permalink
Add support for certificates hot reload
Browse files Browse the repository at this point in the history
Added support for certificates hot reload
issue: 4427

Signed-off-by: Andrey Pleskach <[email protected]>
  • Loading branch information
willyborankin committed Nov 12, 2024
1 parent 9b67d54 commit b1f826c
Show file tree
Hide file tree
Showing 8 changed files with 362 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,10 @@
import static org.opensearch.security.setting.DeprecatedSettings.checkForDeprecatedSetting;
import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX;
import static org.opensearch.security.support.ConfigConstants.SECURITY_ALLOW_DEFAULT_INIT_USE_CLUSTER_STATE;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_SSL_CERT_RELOAD_ENABLED;
import static org.opensearch.security.support.ConfigConstants.SECURITY_UNSUPPORTED_RESTAPI_ALLOW_SECURITYCONFIG_MODIFICATION;

// CS-ENFORCE-SINGLE

public final class OpenSearchSecurityPlugin extends OpenSearchSecuritySSLPlugin
Expand Down Expand Up @@ -313,7 +316,11 @@ private static boolean useClusterStateToInitSecurityConfig(final Settings settin
* @return true if ssl cert reload is enabled else false
*/
private static boolean isSslCertReloadEnabled(final Settings settings) {
return settings.getAsBoolean(ConfigConstants.SECURITY_SSL_CERT_RELOAD_ENABLED, false);
return settings.getAsBoolean(SECURITY_SSL_CERT_RELOAD_ENABLED, false);
}

private boolean sslCertificatesHotReloadEnabled(final Settings settings) {
return settings.getAsBoolean(SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED, false);
}

@SuppressWarnings("removal")
Expand Down Expand Up @@ -1203,6 +1210,19 @@ public Collection<Object> createComponents(
components.add(passwordHasher);

components.add(sslSettingsManager);
if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) {
throw new OpenSearchException(

Check warning on line 1214 in src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L1214

Added line #L1214 was not covered by tests
"Either "
+ SECURITY_SSL_CERT_RELOAD_ENABLED
+ " or "
+ SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED
+ " can be set to true, but not both."
);
}

if (sslCertificatesHotReloadEnabled(settings) && !isSslCertReloadEnabled(settings)) {
sslSettingsManager.addSslConfigurationsChangeListener(resourceWatcherService);

Check warning on line 1224 in src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java#L1224

Added line #L1224 was not covered by tests
}

final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
final var useClusterState = useClusterStateToInitSecurityConfig(settings);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@
import java.util.stream.Stream;
import javax.net.ssl.SSLEngine;

import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;

import org.opensearch.security.ssl.config.Certificate;
import org.opensearch.transport.NettyAllocator;

Expand All @@ -30,6 +33,8 @@

public class SslContextHandler {

private final static Logger LOGGER = LogManager.getLogger(SslContextHandler.class);

private SslContext sslContext;

private final SslConfiguration sslConfiguration;
Expand Down Expand Up @@ -78,7 +83,7 @@ Stream<Certificate> keyMaterialCertificates(final List<Certificate> certificates
return certificates.stream().filter(Certificate::hasKey);
}

void reloadSslContext() throws CertificateException {
boolean reloadSslContext() throws CertificateException {
final var newCertificates = sslConfiguration.certificates();

boolean hasChanges = false;
Expand All @@ -89,11 +94,13 @@ void reloadSslContext() throws CertificateException {
final var newKeyMaterialCertificates = keyMaterialCertificates(newCertificates).collect(Collectors.toList());

if (notSameCertificates(loadedAuthorityCertificates, newAuthorityCertificates)) {
LOGGER.debug("Certification authority has changed");
hasChanges = true;
validateDates(newAuthorityCertificates);
}

if (notSameCertificates(loadedKeyMaterialCertificates, newKeyMaterialCertificates)) {
LOGGER.debug("Key material and access certificate has changed");
hasChanges = true;
validateNewKeyMaterialCertificates(
loadedKeyMaterialCertificates,
Expand All @@ -111,6 +118,7 @@ void reloadSslContext() throws CertificateException {
loadedCertificates.clear();
loadedCertificates.addAll(newCertificates);
}
return hasChanges;
}

private boolean notSameCertificates(final List<Certificate> loadedCertificates, final List<Certificate> newCertificates) {
Expand Down
57 changes: 54 additions & 3 deletions src/main/java/org/opensearch/security/ssl/SslSettingsManager.java
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@

package org.opensearch.security.ssl;

import java.io.IOException;
import java.nio.file.Path;
import java.security.NoSuchAlgorithmException;
import java.security.cert.CertificateException;
import java.util.Locale;
import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.stream.Collectors;
import javax.crypto.Cipher;

import com.google.common.collect.ImmutableMap;
Expand All @@ -28,6 +32,9 @@
import org.opensearch.security.ssl.config.CertType;
import org.opensearch.security.ssl.config.SslCertificatesLoader;
import org.opensearch.security.ssl.config.SslParameters;
import org.opensearch.watcher.FileChangesListener;
import org.opensearch.watcher.FileWatcher;
import org.opensearch.watcher.ResourceWatcherService;

import io.netty.handler.ssl.ClientAuth;
import io.netty.handler.ssl.OpenSsl;
Expand Down Expand Up @@ -107,13 +114,13 @@ private Map<CertType, SslContextHandler> buildSslContexts(final Environment envi

public synchronized void reloadSslContext(final CertType certType) {
sslContextHandler(certType).ifPresentOrElse(sscContextHandler -> {
LOGGER.info("Reloading {} SSL context", certType.name());
try {
sscContextHandler.reloadSslContext();
if (sscContextHandler.reloadSslContext()) {
LOGGER.info("{} SSL context reloaded", certType.name());
}
} catch (CertificateException e) {
throw new OpenSearchException(e);
}
LOGGER.info("{} SSL context reloaded", certType.name());
}, () -> LOGGER.error("Missing SSL Context for {}", certType.name()));
}

Expand Down Expand Up @@ -179,6 +186,50 @@ private Map<CertType, SslConfiguration> loadConfigurations(final Environment env
return configurationBuilder.build();
}

public void addSslConfigurationsChangeListener(final ResourceWatcherService resourceWatcherService) {
for (final var directoryToMonitor : directoriesToMonitor()) {
final var fileWatcher = new FileWatcher(directoryToMonitor);
fileWatcher.addListener(new FileChangesListener() {
@Override
public void onFileCreated(final Path file) {
onFileChanged(file);
}

Check warning on line 196 in src/main/java/org/opensearch/security/ssl/SslSettingsManager.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/SslSettingsManager.java#L195-L196

Added lines #L195 - L196 were not covered by tests

@Override
public void onFileDeleted(final Path file) {
onFileChanged(file);
}

Check warning on line 201 in src/main/java/org/opensearch/security/ssl/SslSettingsManager.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/SslSettingsManager.java#L200-L201

Added lines #L200 - L201 were not covered by tests

@Override
public void onFileChanged(final Path file) {
for (final var e : sslSettingsContexts.entrySet()) {
final var certType = e.getKey();
final var sslConfiguration = e.getValue().sslConfiguration();
if (sslConfiguration.dependentFiles().contains(file)) {
SslSettingsManager.this.reloadSslContext(certType);
}
}
}
});
try {
resourceWatcherService.add(fileWatcher, ResourceWatcherService.Frequency.HIGH);
LOGGER.info("Added SSL configuration change listener for: {}", directoryToMonitor);
} catch (IOException e) {

Check warning on line 217 in src/main/java/org/opensearch/security/ssl/SslSettingsManager.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/SslSettingsManager.java#L217

Added line #L217 was not covered by tests
// TODO: should we fail here, or are error logs sufficient?
throw new OpenSearchException("Couldn't add SSL configurations change listener", e);

Check warning on line 219 in src/main/java/org/opensearch/security/ssl/SslSettingsManager.java

View check run for this annotation

Codecov / codecov/patch

src/main/java/org/opensearch/security/ssl/SslSettingsManager.java#L219

Added line #L219 was not covered by tests
}
}
}

private Set<Path> directoriesToMonitor() {
return sslSettingsContexts.values()
.stream()
.map(SslContextHandler::sslConfiguration)
.flatMap(c -> c.dependentFiles().stream())
.map(Path::getParent)
.collect(Collectors.toSet());
}

private boolean clientNode(final Settings settings) {
return !"node".equals(settings.get(OpenSearchSecuritySSLPlugin.CLIENT_TYPE));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -287,6 +287,7 @@ public class ConfigConstants {
public static final String SECURITY_SSL_DUAL_MODE_SKIP_SECURITY = OPENDISTRO_SECURITY_CONFIG_PREFIX + "passive_security";
public static final String LEGACY_OPENDISTRO_SECURITY_CONFIG_SSL_DUAL_MODE_ENABLED = "opendistro_security_config.ssl_dual_mode_enabled";
public static final String SECURITY_SSL_CERT_RELOAD_ENABLED = "plugins.security.ssl_cert_reload_enabled";
public static final String SECURITY_SSL_CERTIFICATES_HOT_RELOAD_ENABLED = "plugins.security.ssl.certificates_hot_reload.enabled";
public static final String SECURITY_DISABLE_ENVVAR_REPLACEMENT = "plugins.security.disable_envvar_replacement";
public static final String SECURITY_DFM_EMPTY_OVERRIDES_ALL = "plugins.security.dfm_empty_overrides_all";

Expand Down
22 changes: 17 additions & 5 deletions src/test/java/org/opensearch/security/ssl/CertificatesRule.java
Original file line number Diff line number Diff line change
Expand Up @@ -69,16 +69,28 @@ public class CertificatesRule extends ExternalResource {

private PrivateKey accessCertificatePrivateKey;

private final boolean generateDefaultCertificates;

public CertificatesRule() {
this(true);
}

public CertificatesRule(final boolean generateDefaultCertificates) {
this.generateDefaultCertificates = generateDefaultCertificates;
}

@Override
protected void before() throws Throwable {
super.before();
temporaryFolder.create();
configRootFolder = temporaryFolder.newFolder("esHome").toPath();
final var keyPair = generateKeyPair();
caCertificateHolder = generateCaCertificate(keyPair);
final var keyAndCertificate = generateAccessCertificate(keyPair);
accessCertificatePrivateKey = keyAndCertificate.v1();
accessCertificateHolder = keyAndCertificate.v2();
if (generateDefaultCertificates) {
final var keyPair = generateKeyPair();
caCertificateHolder = generateCaCertificate(keyPair);
final var keyAndCertificate = generateAccessCertificate(keyPair);
accessCertificatePrivateKey = keyAndCertificate.v1();
accessCertificateHolder = keyAndCertificate.v2();
}
}

@Override
Expand Down
Loading

0 comments on commit b1f826c

Please sign in to comment.