[WFCORE-6544] CVE-2023-4061 Management RBAC permission allows unexpected reading of system-properties via resolve-expression

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

@@ -38,7 +38,7 @@ public class SensitivityClassification extends AbstractSensitivity {
     public static final SensitivityClassification SOCKET_CONFIG = new SensitivityClassification("socket-config", false, false, true);
     public static final SensitivityClassification SNAPSHOTS = new SensitivityClassification("snapshots", false, false, false);
     public static final SensitivityClassification SSL_REF = new SensitivityClassification("ssl-ref", true, true, true);
-    public static final SensitivityClassification SYSTEM_PROPERTY = new SensitivityClassification("system-property", false, false, true);
+    public static final SensitivityClassification SYSTEM_PROPERTY = new SensitivityClassification("system-property", false, true, true);
 
     private final boolean core;
     private final String subsystem;

controller/src/main/java/org/jboss/as/controller/operations/common/ResolveExpressionHandler.java

@@ -7,9 +7,11 @@
 
 import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OP;
 
+import java.util.EnumSet;
 import java.util.logging.Level;
 
 import org.jboss.as.controller.ExpressionResolver;
+import org.jboss.as.controller.access.Action;
 import org.jboss.as.controller.logging.ControllerLogger;
 import org.jboss.as.controller.OperationContext;
 import org.jboss.as.controller.OperationDefinition;
@@ -45,6 +47,7 @@ public class ResolveExpressionHandler implements OperationStepHandler {
         .setReadOnly()
         .setRuntimeOnly()
         .addAccessConstraint(SensitiveTargetAccessConstraintDefinition.SYSTEM_PROPERTY)
+        .addAccessConstraint(SensitiveTargetAccessConstraintDefinition.JVM) // use the JVM constraint as a guard against unauthorized reads of env vars
         .build();
 
 
@@ -54,6 +57,10 @@ private ResolveExpressionHandler() {
     @Override
     public void execute(OperationContext context, ModelNode operation) throws OperationFailedException {
 
+        // Resolving can involve reading a system property or env vars, so ensure the caller
+        // is authorized to do that.
+        context.authorize(operation, EnumSet.of(Action.ActionEffect.ADDRESS, Action.ActionEffect.READ_RUNTIME)).failIfDenied(operation);
+
         // Run at Stage.RUNTIME so we get the current values of system properties set by earlier steps in a composite
         context.addStep(new OperationStepHandler() {
             @Override

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

@@ -0,0 +1,109 @@
+package org.jboss.as.test.integration.mgmt.access;
+
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.FAILED;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.FAILURE_DESCRIPTION;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.NAME;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.OUTCOME;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.RESULT;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.SUCCESS;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.VALUE;
+import static org.jboss.as.controller.descriptions.ModelDescriptionConstants.WRITE_ATTRIBUTE_OPERATION;
+import static org.jboss.as.test.integration.management.util.ModelUtil.createOpNode;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+
+import java.io.IOException;
+
+import org.jboss.as.controller.client.ModelControllerClient;
+import org.jboss.as.test.integration.management.rbac.RbacUtil;
+import org.jboss.dmr.ModelNode;
+import org.junit.After;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.wildfly.core.testrunner.ServerSetup;
+import org.wildfly.core.testrunner.UnsuccessfulOperationException;
+import org.wildfly.core.testrunner.WildflyTestRunner;
+
+@RunWith(WildflyTestRunner.class)
+@ServerSetup(StandardUsersSetupTask.class)
+public class ResolveExpressionTestCase extends AbstractRbacTestCase {
+
+    private static final ModelNode SYSTEM_PROP_READ = createReadNode("system-property");
+    private static final ModelNode JVM_READ = createReadNode( "jvm");
+
+    private static ModelNode createReadNode(String classification) {
+        final String address = "core-service=management/access=authorization/" +
+                "constraint=sensitivity-classification/type=core/classification=" + classification;
+        ModelNode result = createOpNode(address, WRITE_ATTRIBUTE_OPERATION);
+        result.get(NAME).set("configured-requires-read");
+        result.get(VALUE); // leave it undefined
+        result.protect();
+        return result;
+    }
+
+    @After
+    public void restoreDefaultConfig() throws UnsuccessfulOperationException {
+        // Go back to default settings by undefining the configured-requires-read settings
+        try {
+            managementClient.executeForResult(SYSTEM_PROP_READ);
+        } finally {
+            managementClient.executeForResult(JVM_READ);
+        }
+    }
+
+    @Test
+    public void testDefaultSettings() throws IOException {
+        testUserSet(false);
+    }
+
+    /** Tests that resolution works if sys-prop reads are configured to be allowed. */
+    @Test
+    public void testNonSensitiveSystemPropertyRead() throws IOException, UnsuccessfulOperationException {
+        ModelNode allowRead = SYSTEM_PROP_READ.clone();
+        allowRead.get(VALUE).set(false);
+        managementClient.executeForResult(allowRead);
+
+        testUserSet(true);
+    }
+
+    /** Tests that resolution fails even though sys-prop reads are allowed if jvm reads are not */
+    @Test
+    public void testSensitiveJvmRead() throws IOException, UnsuccessfulOperationException {
+        ModelNode allowRead = SYSTEM_PROP_READ.clone();
+        allowRead.get(VALUE).set(false);
+        managementClient.executeForResult(allowRead);
+
+        ModelNode disallowRead = JVM_READ.clone();
+        disallowRead.get(VALUE).set(true);
+        managementClient.executeForResult(disallowRead);
+
+        testUserSet(false);
+    }
+
+    private void testUserSet(boolean allCanRead) throws IOException {
+        testUser(RbacUtil.MONITOR_USER, allCanRead);
+        testUser(RbacUtil.OPERATOR_USER, allCanRead);
+        testUser(RbacUtil.MAINTAINER_USER, allCanRead);
+        testUser(RbacUtil.DEPLOYER_USER, allCanRead);
+        testUser(RbacUtil.ADMINISTRATOR_USER, true);
+        testUser(RbacUtil.AUDITOR_USER, true);
+        testUser(RbacUtil.SUPERUSER_USER, true);
+    }
+
+    private void testUser(String userName, boolean canRead) throws IOException {
+        ModelControllerClient client = getClientForUser(userName);
+
+        ModelNode operation = createOpNode(null, "resolve-expression");
+        operation.get("expression").set("${foo:bar}");
+        ModelNode response = client.execute(operation);
+        if (canRead) {
+            assertEquals(response.toString(), SUCCESS, response.get(OUTCOME).asString());
+            assertEquals(response.toString(), "bar", response.get(RESULT).asString());
+        } else {
+            assertEquals(response.toString(), FAILED, response.get(OUTCOME).asString());
+            assertFalse(response.toString(), response.hasDefined(RESULT));
+            assertTrue(response.toString(), response.get(FAILURE_DESCRIPTION).asString().contains("WFLYCTL0313"));
+        }
+    }
+}