Merge branch 'main' into resource-sharing

.github/workflows/ci.yml

@@ -81,7 +81,7 @@ jobs:
         working-directory: downloaded-artifacts
 
       - name: Upload Coverage with retry
-        uses: Wandalen/[email protected].2
+        uses: Wandalen/[email protected].3
         with:
           attempt_limit: 5
           attempt_delay: 2000

build.gradle

@@ -495,10 +495,10 @@ configurations {
             // For integrationTest
             force "org.apache.httpcomponents:httpclient:4.5.14"
             force "org.apache.httpcomponents:httpcore:4.4.16"
-            force "com.google.errorprone:error_prone_annotations:2.35.1"
+            force "com.google.errorprone:error_prone_annotations:2.36.0"
             force "org.checkerframework:checker-qual:3.48.2"
             force "ch.qos.logback:logback-classic:1.5.12"
-            force "commons-io:commons-io:2.17.0"
+            force "commons-io:commons-io:2.18.0"
         }
     }
 
@@ -586,7 +586,7 @@ dependencies {
     implementation 'commons-cli:commons-cli:1.9.0'
     implementation "org.bouncycastle:bcprov-jdk18on:${versions.bouncycastle}"
     implementation 'org.ldaptive:ldaptive:1.2.3'
-    implementation 'com.nimbusds:nimbus-jose-jwt:9.46'
+    implementation 'com.nimbusds:nimbus-jose-jwt:9.47'
     implementation 'com.rfksystems:blake2b:2.0.0'
     implementation 'com.password4j:password4j:1.8.2'
 
@@ -612,7 +612,7 @@ dependencies {
     runtimeOnly 'com.eclipsesource.minimal-json:minimal-json:0.9.5'
     runtimeOnly 'commons-codec:commons-codec:1.17.1'
     runtimeOnly 'org.cryptacular:cryptacular:1.2.7'
-    compileOnly 'com.google.errorprone:error_prone_annotations:2.35.1'
+    compileOnly 'com.google.errorprone:error_prone_annotations:2.36.0'
     runtimeOnly 'com.sun.istack:istack-commons-runtime:4.2.0'
     runtimeOnly 'jakarta.xml.bind:jakarta.xml.bind-api:4.0.2'
     runtimeOnly 'org.ow2.asm:asm:9.7.1'
@@ -621,7 +621,7 @@ dependencies {
 
     //OpenSAML
     implementation 'net.shibboleth.utilities:java-support:8.4.2'
-    runtimeOnly "io.dropwizard.metrics:metrics-core:4.2.28"
+    runtimeOnly "io.dropwizard.metrics:metrics-core:4.2.29"
     implementation "com.onelogin:java-saml:${one_login_java_saml}"
     implementation "com.onelogin:java-saml-core:${one_login_java_saml}"
     implementation "org.opensaml:opensaml-core:${open_saml_version}"
@@ -731,7 +731,7 @@ dependencies {
     integrationTestImplementation 'junit:junit:4.13.2'
     integrationTestImplementation "org.opensearch.plugin:reindex-client:${opensearch_version}"
     integrationTestImplementation "org.opensearch.plugin:percolator-client:${opensearch_version}"
-    integrationTestImplementation 'commons-io:commons-io:2.17.0'
+    integrationTestImplementation 'commons-io:commons-io:2.18.0'
     integrationTestImplementation "org.apache.logging.log4j:log4j-core:${versions.log4j}"
     integrationTestImplementation "org.apache.logging.log4j:log4j-jul:${versions.log4j}"
     integrationTestImplementation 'org.hamcrest:hamcrest:2.2'
@@ -750,7 +750,7 @@ dependencies {
     integrationTestImplementation "org.mockito:mockito-core:5.14.2"
 
     //spotless
-    implementation('com.google.googlejavaformat:google-java-format:1.24.0') {
+    implementation('com.google.googlejavaformat:google-java-format:1.25.0') {
         exclude group: 'com.google.guava'
     }
 }

gradle/wrapper/gradle-wrapper.properties

@@ -1,7 +1,7 @@
 distributionBase=GRADLE_USER_HOME
 distributionPath=wrapper/dists
-distributionSha256Sum=31c55713e40233a8303827ceb42ca48a47267a0ad4bab9177123121e71524c26
-distributionUrl=https\://services.gradle.org/distributions/gradle-8.10.2-bin.zip
+distributionSha256Sum=f397b287023acdba1e9f6fc5ea72d22dd63669d59ed4a289a29b1a76eee151c6
+distributionUrl=https\://services.gradle.org/distributions/gradle-8.11.1-bin.zip
 networkTimeout=10000
 validateDistributionUrl=true
 zipStoreBase=GRADLE_USER_HOME

src/main/java/com/amazon/dlic/auth/http/jwt/HTTPJwtAuthenticator.java

@@ -215,7 +215,7 @@ private AuthCredentials extractCredentials0(final SecurityRequest request) {
                 }
             }
         }
-        log.error("Failed to parse JWT token using any of the available parsers");
+        log.debug("Unable to authenticate JWT Token with any configured signing key");
         return null;
     }
 

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

@@ -230,7 +230,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
@@ -325,7 +328,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")
@@ -1230,6 +1237,19 @@ public Collection<Object> createComponents(
         components.add(passwordHasher);
 
         components.add(sslSettingsManager);
+        if (isSslCertReloadEnabled(settings) && sslCertificatesHotReloadEnabled(settings)) {
+            throw new OpenSearchException(
+                "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);
+        }
 
         final var allowDefaultInit = settings.getAsBoolean(SECURITY_ALLOW_DEFAULT_INIT_SECURITYINDEX, false);
         final var useClusterState = useClusterStateToInitSecurityConfig(settings);

src/main/java/org/opensearch/security/auditlog/impl/AbstractAuditLog.java

@@ -566,30 +566,33 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
         msg.addComplianceDocVersion(result.getVersion());
         msg.addComplianceOperation(result.isCreated() ? Operation.CREATE : Operation.UPDATE);
 
-        if (complianceConfig.shouldLogDiffsForWrite()
-            && originalResult != null
-            && originalResult.isExists()
-            && originalResult.internalSourceRef() != null) {
+        if (complianceConfig.shouldLogDiffsForWrite()) {
             try {
                 String originalSource = null;
                 String currentSource = null;
+                if (!(originalResult != null && originalResult.isExists() && originalResult.internalSourceRef() != null)) {
+                    // originalSource is empty
+                    originalSource = "{}";
+                }
                 if (securityIndex.equals(shardId.getIndexName())) {
-                    try (
-                        XContentParser parser = XContentHelper.createParser(
-                            NamedXContentRegistry.EMPTY,
-                            THROW_UNSUPPORTED_OPERATION,
-                            originalResult.internalSourceRef(),
-                            XContentType.JSON
-                        )
-                    ) {
-                        Object base64 = parser.map().values().iterator().next();
-                        if (base64 instanceof String) {
-                            originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
-                        } else {
-                            originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    if (originalSource == null) {
+                        try (
+                            XContentParser parser = XContentHelper.createParser(
+                                NamedXContentRegistry.EMPTY,
+                                THROW_UNSUPPORTED_OPERATION,
+                                originalResult.internalSourceRef(),
+                                XContentType.JSON
+                            )
+                        ) {
+                            Object base64 = parser.map().values().iterator().next();
+                            if (base64 instanceof String) {
+                                originalSource = (new String(BaseEncoding.base64().decode((String) base64), StandardCharsets.UTF_8));
+                            } else {
+                                originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                            }
+                        } catch (Exception e) {
+                            log.error(e.toString());
                         }
-                    } catch (Exception e) {
-                        log.error(e.toString());
                     }
 
                     try (
@@ -615,7 +618,9 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
                     );
                     msg.addSecurityConfigWriteDiffSource(diffnode.size() == 0 ? "" : diffnode.toString(), id);
                 } else {
-                    originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    if (originalSource == null) {
+                        originalSource = XContentHelper.convertToJson(originalResult.internalSourceRef(), false, XContentType.JSON);
+                    }
                     currentSource = XContentHelper.convertToJson(currentIndex.source(), false, XContentType.JSON);
                     final JsonNode diffnode = JsonDiff.asJson(
                         DefaultObjectMapper.objectMapper.readTree(originalSource),
@@ -628,7 +633,7 @@ public void logDocumentWritten(ShardId shardId, GetResult originalResult, Index
             }
         }
 
-        if (!complianceConfig.shouldLogWriteMetadataOnly()) {
+        if (!complianceConfig.shouldLogWriteMetadataOnly() && !complianceConfig.shouldLogDiffsForWrite()) {
             if (securityIndex.equals(shardId.getIndexName())) {
                 // current source, normally not null or empty
                 try (

src/main/java/org/opensearch/security/ssl/SslContextHandler.java

@@ -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;
 
@@ -30,6 +33,8 @@
 
 public class SslContextHandler {
 
+    private final static Logger LOGGER = LogManager.getLogger(SslContextHandler.class);
+
     private SslContext sslContext;
 
     private final SslConfiguration sslConfiguration;
@@ -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;
@@ -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,
@@ -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) {

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

@@ -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;
@@ -29,6 +33,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;
@@ -108,13 +115,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()));
     }
 
@@ -180,6 +187,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);
+                }
+
+                @Override
+                public void onFileDeleted(final Path file) {
+                    onFileChanged(file);
+                }
+
+                @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) {
+                // TODO: should we fail here, or are error logs sufficient?
+                throw new OpenSearchException("Couldn't add SSL configurations change listener", e);
+            }
+        }
+    }
+
+    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));
     }

src/main/java/org/opensearch/security/support/ConfigConstants.java

@@ -288,6 +288,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";