Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for certificates hot reload #4880

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
* @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 @@
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
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 @@

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 @@
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
Loading