diff --git a/CHANGELOG.md b/CHANGELOG.md index 40d5c99b13..21679e30fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added ### Changed +- Add salt generation to demo security configuration ([#6022](https://github.com/opensearch-project/security/pull/6022) ### Features diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DlsFlsLegacyHeadersTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DlsFlsLegacyHeadersTest.java index 5fe2837d19..c1a0d67702 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DlsFlsLegacyHeadersTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/DlsFlsLegacyHeadersTest.java @@ -377,7 +377,7 @@ static DlsFlsProcessedConfig dlsFlsProcessedConfig(SecurityDynamicConfiguration< metadata.getIndicesLookup(), xContentRegistry, Settings.EMPTY, - FieldMasking.Config.DEFAULT + FieldMaskingTestHelper.DEFAULT ); } diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java index 75d05e1fae..72dc99b2e8 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTest.java @@ -111,7 +111,7 @@ static FieldMasking createSubject(SecurityDynamicConfiguration roleConfi return new FieldMasking( roleConfig, INDEX_METADATA.getIndicesLookup(), - FieldMasking.Config.DEFAULT, + FieldMaskingTestHelper.DEFAULT, Settings.builder().put("plugins.security.dfm_empty_overrides_all", true).build() ); } @@ -143,7 +143,7 @@ public void simple() throws Exception { assertNull(expression.getAlgoName()); assertNull(expression.getRegexReplacements()); - FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMasking.Config.DEFAULT); + FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMaskingTestHelper.DEFAULT); assertEquals("c042e214a8b49561577445be44c188a8e6274006b36cd0c6fba5312253cf9293", field.apply("foobar")); } @@ -179,7 +179,7 @@ public void explicitAlgorithm() throws Exception { assertEquals("field_*::SHA-256", expression.getSource()); assertNull(expression.getRegexReplacements()); - FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMasking.Config.DEFAULT); + FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMaskingTestHelper.DEFAULT); assertEquals("c3ab8ff13720e8ad9047dd39466b3c8974e592c2fa383d4a3960714caef0c4f2", field.apply("foobar")); } @@ -202,7 +202,7 @@ public void regex_single() throws Exception { expression.getRegexReplacements() ); - FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMasking.Config.DEFAULT); + FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMaskingTestHelper.DEFAULT); assertEquals("foobar", field.apply("foobar")); assertEquals("foo+masked+bar", field.apply("foobar")); } @@ -221,7 +221,7 @@ public void regex_multi() throws Exception { assertEquals("*", expression.getRegexReplacements().get(1).getReplacement()); assertEquals("field_*:://::+masked+::/\\d/::*", expression.getSource()); - FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMasking.Config.DEFAULT); + FieldMasking.FieldMaskingRule.Field field = new FieldMasking.FieldMaskingRule.Field(expression, FieldMaskingTestHelper.DEFAULT); assertEquals("foobar", field.apply("foobar")); assertEquals("foo**bar", field.apply("foo42bar")); assertEquals("foo+masked+bar**", field.apply("foobar42")); @@ -263,7 +263,7 @@ public void allowAll() { @Test public void allowAll_constructed() throws Exception { - FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMasking.Config.DEFAULT); + FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMaskingTestHelper.DEFAULT); assertTrue("FieldMasking.FieldMaskingRule without masked fields should return true for isAllowAll()", rule.isAllowAll()); assertFalse("FieldMasking.FieldMaskingRule without masked fields allows field", rule.isMasked("field")); assertEquals("FM:[]", rule.toString()); @@ -271,7 +271,7 @@ public void allowAll_constructed() throws Exception { @Test public void simple() throws Exception { - FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMasking.Config.DEFAULT, "field_masked_*"); + FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMaskingTestHelper.DEFAULT, "field_masked_*"); assertFalse("FieldMasking.FieldMaskingRule should return false for isAllowAll()", rule.isAllowAll()); assertTrue("Rule applies to field field_masked_1", rule.isMasked("field_masked_1")); assertFalse("Rule does not apply to field field_other", rule.isMasked("field_other")); @@ -285,7 +285,7 @@ public void simple() throws Exception { @Test public void keyword() throws Exception { - FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMasking.Config.DEFAULT, "field_masked"); + FieldMasking.FieldMaskingRule rule = FieldMasking.FieldMaskingRule.of(FieldMaskingTestHelper.DEFAULT, "field_masked"); assertFalse("FieldMasking.FieldMaskingRule should return false for isAllowAll()", rule.isAllowAll()); assertTrue("Rule applies to field field_masked_1", rule.isMasked("field_masked")); assertTrue("Rule applies to field field_masked_1.keyword", rule.isMasked("field_masked.keyword")); diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTestHelper.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTestHelper.java new file mode 100644 index 0000000000..f9e3a7f932 --- /dev/null +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FieldMaskingTestHelper.java @@ -0,0 +1,19 @@ +/* + * SPDX-License-Identifier: Apache-2.0 + * + * The OpenSearch Contributors require contributions made to + * this file be licensed under the Apache-2.0 license or a + * compatible open source license. + * + * Modifications Copyright OpenSearch Contributors. See + * GitHub history for details. + */ + +package org.opensearch.security.privileges.dlsfls; + +import org.opensearch.common.settings.Settings; + +public class FieldMaskingTestHelper { + + public static final FieldMasking.Config DEFAULT = FieldMasking.Config.fromSettings(Settings.EMPTY); +} diff --git a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FlsDocumentFilterTest.java b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FlsDocumentFilterTest.java index 214a940c54..e3dea6a10b 100644 --- a/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FlsDocumentFilterTest.java +++ b/src/integrationTest/java/org/opensearch/security/privileges/dlsfls/FlsDocumentFilterTest.java @@ -312,7 +312,7 @@ public void maskSimpleAttribute() throws Exception { byte[] result = FlsDocumentFilter.filter( sourceDocument.getBytes(UTF_8), FieldPrivileges.FlsRule.ALLOW_ALL, - FieldMasking.FieldMaskingRule.of(FieldMasking.Config.DEFAULT, "b"), + FieldMasking.FieldMaskingRule.of(FieldMaskingTestHelper.DEFAULT, "b"), ImmutableSet.of() ); @@ -343,7 +343,7 @@ public void maskObjectAttribute() throws Exception { byte[] result = FlsDocumentFilter.filter( sourceDocument.getBytes(UTF_8), FieldPrivileges.FlsRule.ALLOW_ALL, - FieldMasking.FieldMaskingRule.of(FieldMasking.Config.DEFAULT, "b.b1"), + FieldMasking.FieldMaskingRule.of(FieldMaskingTestHelper.DEFAULT, "b.b1"), ImmutableSet.of() ); diff --git a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java index cb28906a9e..93ed73e3d8 100644 --- a/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java +++ b/src/main/java/org/opensearch/security/OpenSearchSecurityPlugin.java @@ -517,6 +517,9 @@ public OpenSearchSecurityPlugin(final Settings settings, final Path configPath) } } + + // TODO: Uncomment for 4.0 - enforce that the default compliance salt is not used outside of demo configuration + // Salt.validateSaltSettings(settings); } private void verifyTLSVersion(final String settings, final List configuredProtocols) { diff --git a/src/main/java/org/opensearch/security/configuration/Salt.java b/src/main/java/org/opensearch/security/configuration/Salt.java index e13a430c79..056dcf730b 100644 --- a/src/main/java/org/opensearch/security/configuration/Salt.java +++ b/src/main/java/org/opensearch/security/configuration/Salt.java @@ -85,4 +85,27 @@ public static Salt from(final Settings settings) { ); return new Salt(saltAsString); } + + /** + * 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). + * @param settings fully loaded node settings + * @throws OpenSearchException if the default salt is used without the demo flag + */ + public static void validateSaltSettings(final Settings settings) { + final String saltAsString = settings.get( + ConfigConstants.SECURITY_COMPLIANCE_SALT, + ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT + ); + final boolean allowUnsafeDemoCertificates = settings.getAsBoolean(ConfigConstants.SECURITY_ALLOW_UNSAFE_DEMOCERTIFICATES, false); + if (saltAsString.equals(ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT) && !allowUnsafeDemoCertificates) { + throw new OpenSearchException( + "Default compliance salt is not allowed in production. Please configure " + + ConfigConstants.SECURITY_COMPLIANCE_SALT + + " to a random 16-character string, or set " + + ConfigConstants.SECURITY_ALLOW_UNSAFE_DEMOCERTIFICATES + + " to true for demo/test environments." + ); + } + } } diff --git a/src/main/java/org/opensearch/security/privileges/dlsfls/FieldMasking.java b/src/main/java/org/opensearch/security/privileges/dlsfls/FieldMasking.java index f89376c0ed..b6e47e6897 100644 --- a/src/main/java/org/opensearch/security/privileges/dlsfls/FieldMasking.java +++ b/src/main/java/org/opensearch/security/privileges/dlsfls/FieldMasking.java @@ -466,8 +466,6 @@ public static Config fromSettings(Settings settings) { return new Config(settings.get(ConfigConstants.SECURITY_MASKED_FIELDS_ALGORITHM_DEFAULT), Salt.from(settings)); } - public static final Config DEFAULT = fromSettings(Settings.EMPTY); - private final String defaultHashAlgorithm; private final Salt salt; private final boolean useLegacyDefaultAlgorithm; diff --git a/src/main/java/org/opensearch/security/support/ConfigHelper.java b/src/main/java/org/opensearch/security/support/ConfigHelper.java index 93b8decd78..b3bf6d653e 100644 --- a/src/main/java/org/opensearch/security/support/ConfigHelper.java +++ b/src/main/java/org/opensearch/security/support/ConfigHelper.java @@ -98,7 +98,7 @@ public static void uploadFile( if (!configType.equals(res)) { throw new Exception( - " FAIL: Configuration for '" + configType + "' failed for unknown reasons. Pls. consult logfile of opensearch" + " FAIL: Configuration for '" + configType + "' failed for unknown reasons. Please consult logfile of opensearch" ); } LOGGER.info("Doc with id '{}' and version {} is updated in {} index.", configType, configVersion, index); diff --git a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java index d02d50a14b..6d7280979f 100644 --- a/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java +++ b/src/main/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurer.java @@ -18,6 +18,7 @@ import java.io.FileWriter; import java.io.IOException; import java.nio.charset.StandardCharsets; +import java.security.SecureRandom; import java.util.LinkedHashMap; import java.util.List; import java.util.Map; @@ -324,6 +325,7 @@ Map buildSecurityConfigMap() { configMap.put("plugins.security.ssl.http.pemkey_filepath", Certificates.NODE_KEY.getFileName()); configMap.put("plugins.security.ssl.http.pemtrustedcas_filepath", Certificates.ROOT_CA.getFileName()); configMap.put("plugins.security.allow_unsafe_democertificates", true); + configMap.put(ConfigConstants.SECURITY_COMPLIANCE_SALT, generateRandomSalt()); if (installer.initsecurity) { configMap.put("plugins.security.allow_default_init_securityindex", true); @@ -353,6 +355,16 @@ Map buildSecurityConfigMap() { return configMap; } + static String generateRandomSalt() { + final String chars = "ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789!@#$%^&*"; + SecureRandom random = new SecureRandom(); + StringBuilder sb = new StringBuilder(16); + for (int i = 0; i < 16; i++) { + sb.append(chars.charAt(random.nextInt(chars.length()))); + } + return sb.toString(); + } + /** * Helper method to check if network.host config is present * @param filePath path to opensearch.yml diff --git a/src/test/java/org/opensearch/security/configuration/SaltTest.java b/src/test/java/org/opensearch/security/configuration/SaltTest.java index 1a57a04629..469a35791c 100644 --- a/src/test/java/org/opensearch/security/configuration/SaltTest.java +++ b/src/test/java/org/opensearch/security/configuration/SaltTest.java @@ -41,6 +41,22 @@ public void testDefault() { assertArrayEquals(ConfigConstants.SECURITY_COMPLIANCE_SALT_DEFAULT.getBytes(StandardCharsets.UTF_8), salt.getSalt16()); } + @Test + public void testDefaultSaltRejectedInProduction() { + // assert + thrown.expect(OpenSearchException.class); + thrown.expectMessage("Default compliance salt is not allowed in production"); + + // act - validation rejects default salt when allow_unsafe_democertificates is false + Salt.validateSaltSettings(Settings.EMPTY); + } + + @Test + public void testDefaultSaltAllowedWithDemoFlag() { + // should not throw + Salt.validateSaltSettings(Settings.builder().put(ConfigConstants.SECURITY_ALLOW_UNSAFE_DEMOCERTIFICATES, true).build()); + } + @Test public void testConfig() { // arrange diff --git a/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java b/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java index ad82514257..203056cb87 100644 --- a/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java +++ b/src/test/java/org/opensearch/security/tools/democonfig/SecuritySettingsConfigurerTests.java @@ -252,7 +252,7 @@ public void testConfigFileDoesNotExist() { public void testBuildSecurityConfigMap() { Map actual = securitySettingsConfigurer.buildSecurityConfigMap(); - assertThat(actual.size(), is(16)); + assertThat(actual.size(), is(17)); assertThat(actual.get("plugins.security.ssl.transport.pemcert_filepath"), equalTo(Certificates.NODE_CERT.getFileName())); assertThat(actual.get("plugins.security.ssl.transport.pemkey_filepath"), equalTo(Certificates.NODE_KEY.getFileName())); assertThat(actual.get("plugins.security.ssl.transport.pemtrustedcas_filepath"), equalTo(Certificates.ROOT_CA.getFileName())); @@ -262,6 +262,8 @@ public void testBuildSecurityConfigMap() { assertThat(actual.get("plugins.security.ssl.http.pemkey_filepath"), equalTo(Certificates.NODE_KEY.getFileName())); assertThat(actual.get("plugins.security.ssl.http.pemtrustedcas_filepath"), equalTo(Certificates.ROOT_CA.getFileName())); assertThat(actual.get("plugins.security.allow_unsafe_democertificates"), equalTo(true)); + assertThat(actual.containsKey(ConfigConstants.SECURITY_COMPLIANCE_SALT), equalTo(true)); + assertThat(((String) actual.get(ConfigConstants.SECURITY_COMPLIANCE_SALT)).length(), equalTo(16)); assertThat(actual.get("plugins.security.authcz.admin_dn"), equalTo(List.of("CN=kirk,OU=client,O=client,L=test,C=de"))); assertThat(actual.get("plugins.security.audit.type"), equalTo("internal_opensearch")); assertThat(actual.get("plugins.security.enable_snapshot_restore_privilege"), equalTo(true));