Skip to content
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 @@ -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;
Expand Down Expand Up @@ -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);
}
Expand Down
26 changes: 20 additions & 6 deletions src/main/java/org/opensearch/security/configuration/Salt.java
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {

Expand All @@ -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<String> warnings = captureWarnLogs(() -> {
Salt.from(Settings.EMPTY);
Salt.from(Settings.EMPTY);
Salt.from(Settings.EMPTY);
});

assertThat(warnings, empty());
}

@Test
public void testWarnIfDefaultComplianceSaltLogsOnce() {
final List<String> 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<String> warnings = captureWarnLogs(() -> Salt.warnIfDefaultComplianceSalt(settings));

assertThat(warnings, empty());
}

@Test
public void testDefaultSaltRejectedInProduction() {
// assert
Expand Down Expand Up @@ -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<String> 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<LogEvent> events = new java.util.ArrayList<>();

@Override
public void append(LogEvent event) {
events.add(event.toImmutable());
}

java.util.List<LogEvent> 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();
}
}
}
Loading