diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index 973721ebdf..efef38e884 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -159,6 +159,7 @@ import org.opensearch.security.configuration.DlsFlsRequestValve; import org.opensearch.security.configuration.DlsFlsValveImpl; import org.opensearch.security.configuration.SecurityConfigVersionHandler; +import org.opensearch.security.configuration.Salt; import org.opensearch.security.configuration.SecurityFlsDlsIndexSearcherWrapper; import org.opensearch.security.dlic.rest.api.Endpoint; import org.opensearch.security.dlic.rest.api.SecurityRestApiActions; @@ -544,6 +545,8 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) } + Salt.warnIfDefaultComplianceSalt(settings); + // TODO: Uncomment for 4.0 - enforce that the default compliance salt is not used outside of demo configuration // Salt.validateSaltSettings(settings); } diff --git a/src/main/java/org/opensearch/security/configuration/Salt.java b/src/main/java/org/opensearch/security/configuration/Salt.java index 3882e20ecd..59a5ca0d73 100644 --- a/src/main/java/org/opensearch/security/configuration/Salt.java +++ b/src/main/java/org/opensearch/security/configuration/Salt.java @@ -44,12 +44,6 @@ public Salt(final byte[] salt) { private Salt(final String saltAsString) { this.salt16 = new byte[SALT_SIZE]; - if (saltAsString.equals(ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT)) { - log.warn( - "If you plan to use field masking pls configure compliance salt {} to be a random string of 16 chars length identical on all nodes", - saltAsString - ); - } try { ByteBuffer byteBuffer = StandardCharsets.UTF_8.encode(saltAsString); byteBuffer.get(salt16); @@ -86,6 +80,26 @@ public static Salt from(final Settings settings) { return new Salt(saltAsString); } + /** + * Logs a warning when the default compliance salt is in use. + * Must be called once during plugin startup. + * @param settings fully loaded node settings + */ + public static void warnIfDefaultComplianceSalt(final Settings settings) { + final String saltAsString = settings.get( + ConfigConstants.SECURITY_COMPLIANCE_SALT, + ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT + ); + if (ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT.equals(saltAsString)) { + log.warn( + "Default compliance salt is in use. Configure {} to a random 16-character string identical on all nodes, " + + "or set {} to true for demo/test environments.", + ConfigConstants.SECURITY_COMPLIANCE_SALT, + ConfigConstants.SECURITY_ALLOW_UNSAFE_DEMOCERTIFICATES + ); + } + } + /** * Validates that the default compliance salt is not used unless allow_unsafe_democertificates is enabled. * Must be called after node settings are fully loaded (e.g. during plugin startup). diff --git a/src/test/java/org/opensearch/security/configuration/SaltTest.java b/src/test/java/org/opensearch/security/configuration/SaltTest.java index d5716718f0..d3a3a04c05 100644 --- a/src/test/java/org/opensearch/security/configuration/SaltTest.java +++ b/src/test/java/org/opensearch/security/configuration/SaltTest.java @@ -12,7 +12,15 @@ package org.opensearch.security.configuration; import java.nio.charset.StandardCharsets; - +import java.util.List; + +import org.apache.logging.log4j.Level; +import org.apache.logging.log4j.LogManager; +import org.apache.logging.log4j.core.LogEvent; +import org.apache.logging.log4j.core.Logger; +import org.apache.logging.log4j.core.appender.AbstractAppender; +import org.apache.logging.log4j.core.config.Property; +import org.apache.logging.log4j.core.layout.PatternLayout; import org.apache.lucene.tests.util.LuceneTestCase; import org.junit.Rule; import org.junit.Test; @@ -23,9 +31,11 @@ import org.opensearch.security.support.ConfigConstants; import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.empty; import static org.hamcrest.Matchers.is; import static org.opensearch.security.configuration.Salt.SALT_SIZE; import static org.junit.Assert.assertArrayEquals; +import static org.junit.Assert.assertTrue; public class SaltTest extends LuceneTestCase { @@ -42,6 +52,36 @@ public void testDefault() { assertArrayEquals(ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT.getBytes(StandardCharsets.UTF_8), salt.getSalt16()); } + @Test + public void testSaltFromDoesNotLogDefaultSaltWarning() { + final List warnings = captureWarnLogs(() -> { + Salt.from(Settings.EMPTY); + Salt.from(Settings.EMPTY); + Salt.from(Settings.EMPTY); + }); + + assertThat(warnings, empty()); + } + + @Test + public void testWarnIfDefaultComplianceSaltLogsOnce() { + final List warnings = captureWarnLogs(() -> Salt.warnIfDefaultComplianceSalt(Settings.EMPTY)); + + assertThat(warnings.size(), is(1)); + assertTrue(warnings.get(0).contains("Default compliance salt is in use")); + assertTrue(warnings.get(0).contains(ConfigConstants.SECURITY_COMPLIANCE_SALT)); + assertTrue(warnings.get(0).contains(ConfigConstants.SECURITY_ALLOW_UNSAFE_DEMOCERTIFICATES)); + } + + @Test + public void testWarnIfDefaultComplianceSaltSkipsCustomSalt() { + final Settings settings = Settings.builder().put(ConfigConstants.SECURITY_COMPLIANCE_SALT, "abcdefghijklmnop").build(); + + final List warnings = captureWarnLogs(() -> Salt.warnIfDefaultComplianceSalt(settings)); + + assertThat(warnings, empty()); + } + @Test public void testDefaultSaltRejectedInProduction() { // assert @@ -123,4 +163,40 @@ public void testSaltThrowsNoExceptionWhenCorrectBytesArrayProvided() { // act new Salt(new byte[] { 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5, 1 }); } + + private static List captureWarnLogs(Runnable action) { + final Logger logger = (Logger) LogManager.getLogger(Salt.class); + final var appender = new AbstractAppender( + "SaltWarningCapture", + null, + PatternLayout.createDefaultLayout(), + false, + Property.EMPTY_ARRAY + ) { + private final java.util.List events = new java.util.ArrayList<>(); + + @Override + public void append(LogEvent event) { + events.add(event.toImmutable()); + } + + java.util.List getEvents() { + return events; + } + }; + appender.start(); + logger.addAppender(appender); + logger.setLevel(Level.WARN); + try { + action.run(); + return appender.getEvents() + .stream() + .filter(e -> e.getLevel() == Level.WARN) + .map(e -> e.getMessage().getFormattedMessage()) + .toList(); + } finally { + logger.removeAppender(appender); + appender.stop(); + } + } }