This is an automated email from the ASF dual-hosted git repository. heneveld pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/brooklyn-server.git
commit 3d9b3324ab5724962d800ca58bb51a60d34a9f2d Author: Alex Heneveld <[email protected]> AuthorDate: Fri Oct 1 21:44:04 2021 +0100 add support for blocking selected DSL expressions --- .../brooklyn/camp/brooklyn/ConfigYamlTest.java | 33 ++++++++++++++++++++++ .../org/apache/brooklyn/core/config/Sanitizer.java | 26 +++++++++++++++++ .../brooklyn/core/server/BrooklynServerConfig.java | 2 ++ .../core/typereg/AbstractTypePlanTransformer.java | 31 ++++++++++++++++---- 4 files changed, 87 insertions(+), 5 deletions(-) diff --git a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java index 90d2feb..9ede4a1 100644 --- a/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java +++ b/camp/camp-brooklyn/src/test/java/org/apache/brooklyn/camp/brooklyn/ConfigYamlTest.java @@ -576,6 +576,39 @@ public class ConfigYamlTest extends AbstractYamlTest { }); } + @Test + public void testSensitiveConfigDslWorksOrFailsDependingHowConfigured() throws Exception { + String yaml = Joiner.on("\n").join( + "services:", + "- type: org.apache.brooklyn.core.test.entity.TestEntity", + " brooklyn.config:", + " secret1: $brooklyn:literal(\"myval\")"); + + // allowed + withSensitiveFieldsBlocked(() -> { + return createStartWaitAndLogApplication(yaml); + }); + + Asserts.assertFailsWith(() -> { + return withSensitiveFieldsBlocked(() -> { + String oldValue = + //((BrooklynProperties) mgmt().getConfig()).put(BrooklynServerConfig.SENSITIVE_FIELDS_PLAINTEXT_BLOCKED, true); + System.setProperty(BrooklynServerConfig.SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES.getName(), "[ \"$brooklyn:literal\" ]"); + Sanitizer.getSensitiveFieldsTokens(true); + try { + return createStartWaitAndLogApplication(yaml); + } finally { + System.setProperty(BrooklynServerConfig.SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES.getName(), oldValue!=null ? oldValue : ""); + Sanitizer.getSensitiveFieldsTokens(true); + } + }); + }, e -> { + Asserts.expectedFailureContainsIgnoreCase(e, "literal"); + Asserts.expectedFailureDoesNotContain(e, "myval"); + return true; + }); + } + @Beta public static <T> T withSensitiveFieldsBlocked(Callable<T> r) throws Exception { String oldValue = diff --git a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java index 418fc5b..a4b4669 100644 --- a/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java +++ b/core/src/main/java/org/apache/brooklyn/core/config/Sanitizer.java @@ -32,6 +32,7 @@ import java.util.stream.Collectors; import org.apache.brooklyn.api.mgmt.ManagementContext; import org.apache.brooklyn.config.ConfigKey; import org.apache.brooklyn.core.server.BrooklynServerConfig; +import org.apache.brooklyn.util.collections.MutableList; import org.apache.brooklyn.util.collections.MutableSet; import org.apache.brooklyn.util.core.config.ConfigBag; import org.apache.brooklyn.util.core.flags.TypeCoercions; @@ -45,6 +46,7 @@ public final class Sanitizer { public static final ConfigKey<List<String>> SENSITIVE_FIELDS_TOKENS = BrooklynServerConfig.SENSITIVE_FIELDS_TOKENS; public static final ConfigKey<Boolean> SENSITIVE_FIELDS_PLAINTEXT_BLOCKED = BrooklynServerConfig.SENSITIVE_FIELDS_PLAINTEXT_BLOCKED; + public static final ConfigKey<List<String>> SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = BrooklynServerConfig.SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES; /** * Names that, if they appear anywhere in an attribute/config/field @@ -64,6 +66,8 @@ public final class Sanitizer { private static List<String> LAST_SENSITIVE_FIELDS_TOKENS = null; private static Boolean LAST_SENSITIVE_FIELDS_PLAINTEXT_BLOCKED = null; + private static List<String> LAST_SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = null; + private static long LAST_SENSITIVE_FIELDS_LOAD_TIME = -1; private static long LAST_SENSITIVE_FIELDS_CACHE_MILLIS = 60*1000; @@ -83,9 +87,11 @@ public final class Sanitizer { ManagementContext mgmt = Osgis.getManagementContext(); List<String> tokens = null; Boolean plaintextBlocked = null; + List<String> extPhrases = null; if (mgmt != null) { tokens = mgmt.getConfig().getConfig(SENSITIVE_FIELDS_TOKENS); plaintextBlocked = mgmt.getConfig().getConfig(SENSITIVE_FIELDS_PLAINTEXT_BLOCKED); + extPhrases = mgmt.getConfig().getConfig(SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES); } if (tokens==null) { @@ -108,8 +114,19 @@ public final class Sanitizer { plaintextBlocked = Boolean.FALSE; } + if (extPhrases==null) { + StringSystemProperty extPhrasesSP = new StringSystemProperty(SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES.getName()); + if (extPhrasesSP.isNonEmpty()) { + extPhrases = TypeCoercions.coerce(extPhrasesSP.getValue(), SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES.getTypeToken()); + } + } + if (extPhrases==null) { + extPhrases = MutableList.of(); + } + LAST_SENSITIVE_FIELDS_TOKENS = tokens.stream().map(String::toLowerCase).collect(Collectors.toList()); LAST_SENSITIVE_FIELDS_PLAINTEXT_BLOCKED = plaintextBlocked; + LAST_SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = extPhrases; LAST_SENSITIVE_FIELDS_LOAD_TIME = System.currentTimeMillis(); } } @@ -131,6 +148,15 @@ public final class Sanitizer { return LAST_SENSITIVE_FIELDS_PLAINTEXT_BLOCKED; } + public static List<String> getSensitiveFieldsExtBlockedPhrases() { + return getSensitiveFieldsExtBlockedPhrases(null); + } + public static List<String> getSensitiveFieldsExtBlockedPhrases(Boolean refresh) { + refreshProperties(refresh); + return LAST_SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES; + } + + public static final Predicate<Object> IS_SECRET_PREDICATE = new IsSecretPredicate(); public static Object suppressIfSecret(Object key, Object value) { diff --git a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java index 058ab78..fc6979e 100644 --- a/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java +++ b/core/src/main/java/org/apache/brooklyn/core/server/BrooklynServerConfig.java @@ -175,5 +175,7 @@ public class BrooklynServerConfig { "List of tokens which get treated as sensitive-named fields and suppressed in many places"); public static final ConfigKey<Boolean> SENSITIVE_FIELDS_PLAINTEXT_BLOCKED = ConfigKeys.newBooleanConfigKey("brooklyn.security.sensitive.fields.plaintext.blocked", "Whether plaintext values for sensitive-named fields are blocked"); + public static final ConfigKey<List<String>> SENSITIVE_FIELDS_EXT_BLOCKED_PHRASES = ConfigKeys.newConfigKey(new TypeToken<List<String>>() {}, "brooklyn.security.sensitive.fields.ext.blocked.phrases", + "Extended blocking settings when plaintext values are blocked, allowing also to block DSL and complex values which contain any of the phrases supplied in this config key (comma-separated list)"); } diff --git a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java index 6f14c2f..92dce59 100644 --- a/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java +++ b/core/src/main/java/org/apache/brooklyn/core/typereg/AbstractTypePlanTransformer.java @@ -249,29 +249,50 @@ public abstract class AbstractTypePlanTransformer implements BrooklynTypePlanTra } public static void failOnInsecureValueForSensitiveNamedField(Predicate<Object> tokens, String key, Object val) { - if (val instanceof DeferredSupplier || val==null) { - // value allowed; key is irrelevant + if (val==null) { + // not set; key is irrelevant return; } + if (!tokens.apply(key)) { // not a sensitive named key return; } + if (val instanceof DeferredSupplier || val==null) { + // allows unless phrase blocked + failOnExtBlockedPhraseInValueForSensitiveNamedField(key, val); + return; + } + // sensitive named key if (val instanceof String) { if (((String) val).startsWith("$brooklyn:")) { - // DSL expression, allow + // DSL expression, allow unless phrase blocked + failOnExtBlockedPhraseInValueForSensitiveNamedField(key, val); return; } } if (val instanceof String || Boxing.isPrimitiveOrBoxedClass(val.getClass()) || val instanceof Number) { // non-DSL plaintext value - throw new IllegalStateException("Insecure value supplied for '"+key+"'; external suppliers must be used here"); + throw new IllegalStateException("Insecure value supplied for '"+key+"'; external suppliers should be used here"); + } + + // complex values allowed unless phrase blocked + failOnExtBlockedPhraseInValueForSensitiveNamedField(key, val); + } + + private static void failOnExtBlockedPhraseInValueForSensitiveNamedField(String key, Object value) { + List<String> blockedPhrases = Sanitizer.getSensitiveFieldsExtBlockedPhrases(); + if (blockedPhrases==null || blockedPhrases.isEmpty()) return; + String valS = ""+value; + for (String blockedPhrase: blockedPhrases) { + if (valS.contains(blockedPhrase)) { + throw new IllegalStateException("Improperly secured value supplied for '"+key+"'; value must not contain '"+blockedPhrase+"'"); + } } - // complex values allowed } }
