Merge pull request #4669 from darranl/WFCORE-5511/16.x

[16.x] [WFCORE-5511] wildfly-core: Invalid Sensitivity Classification of Vault Expression

Author: jmesnil

controller/src/main/java/org/jboss/as/controller/access/constraint/SensitiveVaultExpressionConstraint.java

@@ -22,8 +22,8 @@
 
 package org.jboss.as.controller.access.constraint;
 
-import org.jboss.as.controller.ExpressionResolver;
-import org.jboss.as.controller.VaultReader;
+import java.util.regex.Pattern;
+
 import org.jboss.as.controller.access.Action;
 import org.jboss.as.controller.access.JmxAction;
 import org.jboss.as.controller.access.JmxTarget;
@@ -45,6 +45,8 @@ public class SensitiveVaultExpressionConstraint extends AllowAllowNotConstraint
 
     public static final ConstraintFactory FACTORY = new Factory();
 
+    private static final Pattern VAULT_EXPRESSION_PATTERN = Pattern.compile(".*\\$\\{VAULT::.*::.*::.*}.*");
+
     private static final SensitiveVaultExpressionConstraint SENSITIVE = new SensitiveVaultExpressionConstraint(true);
     private static final SensitiveVaultExpressionConstraint NOT_SENSITIVE = new SensitiveVaultExpressionConstraint(false);
     private static final SensitiveVaultExpressionConstraint ALLOWS = new SensitiveVaultExpressionConstraint(true, true);
@@ -123,12 +125,7 @@ private boolean isSensitiveValue(ModelNode value) {
             if (value.getType() == ModelType.EXPRESSION
                     || value.getType() == ModelType.STRING) {
                 String valueString = value.asString();
-                if (ExpressionResolver.EXPRESSION_PATTERN.matcher(valueString).matches()) {
-                    int start = valueString.indexOf("${") + 2;
-                    int end = valueString.indexOf("}", start);
-                    valueString = valueString.substring(start, end);
-                    return VaultReader.STANDARD_VAULT_PATTERN.matcher(valueString).matches();
-                }
+                return VAULT_EXPRESSION_PATTERN.matcher(valueString).matches();
             }
             return false;
         }

testsuite/rbac/src/test/java/org/jboss/as/test/integration/mgmt/access/VaultExpressionSensitivityTestCase.java

@@ -226,6 +226,18 @@ public void testReadNonSensitiveWriteNonSensitive() throws Exception {
         }
     }
 
+    /*
+     * Test if the Monitor role can smuggle a vault expression to retrieve the value from the vault.
+     */
+    @Test
+    public void testHiddenExpression() throws Exception {
+        ModelControllerClient client = getClientForUser(RbacUtil.MAINTAINER_USER);
+
+        ModelNode operation = createOpNode("subsystem=logging/logger=vault-test", ADD);
+        operation.get("level").set("${someproperty:XXX} " + vaultPassword);
+        RbacUtil.executeOperation(client, operation, Outcome.UNAUTHORIZED);
+    }
+
     private void test(String userName, boolean canRead, boolean canWrite) throws Exception {
         ModelControllerClient client = getClientForUser(userName);