You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by kr...@apache.org on 2022/05/03 13:35:39 UTC
[solr] branch branch_9_0 updated: SOLR-16164: ConfigSet API returns error if untrusted user creates from _default configset (#825)
This is an automated email from the ASF dual-hosted git repository.
krisden pushed a commit to branch branch_9_0
in repository https://gitbox.apache.org/repos/asf/solr.git
The following commit(s) were added to refs/heads/branch_9_0 by this push:
new d2fddf1c72e SOLR-16164: ConfigSet API returns error if untrusted user creates from _default configset (#825)
d2fddf1c72e is described below
commit d2fddf1c72e892b4ed241c1a0e7b795bb211d678
Author: Eric Pugh <ep...@opensourceconnections.com>
AuthorDate: Fri Apr 29 22:28:24 2022 -0400
SOLR-16164: ConfigSet API returns error if untrusted user creates from _default configset (#825)
Co-authored-by: epugh@opensourceconnections.com <>
Co-authored-by: Kevin Risden <kr...@apache.org>
---
solr/CHANGES.txt | 2 +
.../org/apache/solr/cloud/ZkConfigSetService.java | 2 -
.../org/apache/solr/core/ConfigSetProperties.java | 5 +-
.../java/org/apache/solr/core/RequestParams.java | 18 ++---
.../src/java/org/apache/solr/core/SolrConfig.java | 3 +-
.../src/java/org/apache/solr/rest/RestManager.java | 5 +-
.../org/apache/solr/cloud/TestConfigSetsAPI.java | 88 ++++++++++++++--------
.../java/org/apache/solr/common/cloud/Aliases.java | 7 +-
.../apache/solr/common/cloud/ZkStateReader.java | 3 +-
.../java/org/apache/solr/common/util/Utils.java | 8 ++
10 files changed, 79 insertions(+), 62 deletions(-)
diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 883e438c55d..996e2ab4d1f 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -688,6 +688,8 @@ Bug Fixes
* SOLR-16143: SolrConfig ResourceProvider can miss updates from ZooKeeper (Mike Drob)
+* SOLR-16164: ConfigSet API returns error if untrusted user creates from _default configset (Eric Pugh, Kevin Risden)
+
================== 8.11.1 ==================
Bug Fixes
diff --git a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
index 4f2d88fec7f..f6e4625da45 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
@@ -21,7 +21,6 @@ import java.lang.invoke.MethodHandles;
import java.nio.file.Path;
import java.util.ArrayList;
import java.util.Collections;
-import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Objects;
@@ -237,7 +236,6 @@ public class ZkConfigSetService extends ConfigSetService {
Map<String, Object> data =
(Map<String, Object>)
Utils.fromJSON(zkClient.getData(CONFIGS_ZKNODE + "/" + configName, null, null, true));
- if (data == null) return new HashMap<>();
return data;
} catch (KeeperException | InterruptedException e) {
throw new IOException("Error getting config metadata", SolrZkClient.checkInterrupted(e));
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetProperties.java b/solr/core/src/java/org/apache/solr/core/ConfigSetProperties.java
index 35687d32816..0ebfff643e3 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigSetProperties.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigSetProperties.java
@@ -16,8 +16,6 @@
*/
package org.apache.solr.core;
-import static org.apache.solr.common.util.Utils.fromJSON;
-
import java.io.InputStreamReader;
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
@@ -26,6 +24,7 @@ import org.apache.commons.io.IOUtils;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -73,7 +72,7 @@ public class ConfigSetProperties {
public static NamedList<Object> readFromInputStream(InputStreamReader reader) {
try {
- Object object = fromJSON(reader);
+ Object object = Utils.fromJSON(reader);
if (!(object instanceof Map)) {
final String objectClass = object == null ? "null" : object.getClass().getName();
throw new SolrException(
diff --git a/solr/core/src/java/org/apache/solr/core/RequestParams.java b/solr/core/src/java/org/apache/solr/core/RequestParams.java
index 1c5f19ae825..2592c485cb1 100644
--- a/solr/core/src/java/org/apache/solr/core/RequestParams.java
+++ b/solr/core/src/java/org/apache/solr/core/RequestParams.java
@@ -16,10 +16,6 @@
*/
package org.apache.solr.core;
-import static java.util.Collections.singletonMap;
-import static org.apache.solr.common.util.Utils.fromJSON;
-import static org.apache.solr.common.util.Utils.getDeepCopy;
-
import com.google.common.collect.ImmutableMap;
import java.io.IOException;
import java.io.InputStream;
@@ -67,7 +63,7 @@ public class RequestParams implements MapSerializable {
@SuppressWarnings({"rawtypes"})
public static ParamSet createParamSet(Map map, Long version) {
- Map copy = getDeepCopy(map, 3);
+ Map copy = Utils.getDeepCopy(map, 3);
@SuppressWarnings("unchecked")
Map<String, Long> meta = (Map<String, Long>) copy.remove("");
if (meta == null && version != null) {
@@ -132,7 +128,7 @@ public class RequestParams implements MapSerializable {
@SuppressWarnings({"unchecked", "rawtypes"})
public RequestParams setParams(String name, ParamSet paramSet) {
- Map deepCopy = getDeepCopy(data, 3);
+ Map deepCopy = Utils.getDeepCopy(data, 3);
Map p = (Map) deepCopy.get(NAME);
if (p == null) deepCopy.put(NAME, p = new LinkedHashMap<>());
if (paramSet == null) p.remove(name);
@@ -192,7 +188,7 @@ public class RequestParams implements MapSerializable {
log.info("conf resource {} loaded . version : {} ", name, version);
}
try {
- Map<?, ?> m = (Map<?, ?>) fromJSON(in);
+ Map<?, ?> m = (Map<?, ?>) Utils.fromJSON(in);
return new Object[] {m, version};
} catch (Exception e) {
throw new SolrException(
@@ -253,10 +249,10 @@ public class RequestParams implements MapSerializable {
public ParamSet update(@SuppressWarnings({"rawtypes"}) Map map) {
ParamSet p = createParamSet(map, null);
return new ParamSet(
- mergeMaps(getDeepCopy(defaults, 2), p.defaults),
- mergeMaps(getDeepCopy(invariants, 2), p.invariants),
- mergeMaps(getDeepCopy(appends, 2), p.appends),
- mergeMaps(getDeepCopy(meta, 2), singletonMap("v", getVersion() + 1)));
+ mergeMaps(Utils.getDeepCopy(defaults, 2), p.defaults),
+ mergeMaps(Utils.getDeepCopy(invariants, 2), p.invariants),
+ mergeMaps(Utils.getDeepCopy(appends, 2), p.appends),
+ mergeMaps(Utils.getDeepCopy(meta, 2), Collections.singletonMap("v", getVersion() + 1)));
}
private static <K, V> Map<K, V> mergeMaps(Map<K, V> m1, Map<K, V> m2) {
diff --git a/solr/core/src/java/org/apache/solr/core/SolrConfig.java b/solr/core/src/java/org/apache/solr/core/SolrConfig.java
index d77a8c55b3b..2aa26097bbe 100644
--- a/solr/core/src/java/org/apache/solr/core/SolrConfig.java
+++ b/solr/core/src/java/org/apache/solr/core/SolrConfig.java
@@ -18,7 +18,6 @@ package org.apache.solr.core;
import static org.apache.solr.common.params.CommonParams.NAME;
import static org.apache.solr.common.params.CommonParams.PATH;
-import static org.apache.solr.common.util.Utils.fromJSON;
import static org.apache.solr.core.ConfigOverlay.ZNODEVER;
import static org.apache.solr.core.SolrConfig.PluginOpts.LAZY;
import static org.apache.solr.core.SolrConfig.PluginOpts.MULTI_OK;
@@ -601,7 +600,7 @@ public class SolrConfig implements MapSerializable {
log.debug("Config overlay loaded. version : {} ", version);
}
@SuppressWarnings("unchecked")
- Map<String, Object> m = (Map<String, Object>) fromJSON(in);
+ Map<String, Object> m = (Map<String, Object>) Utils.fromJSON(in);
return new ConfigOverlay(m, version);
} catch (Exception e) {
throw new SolrException(ErrorCode.SERVER_ERROR, "Error reading config overlay", e);
diff --git a/solr/core/src/java/org/apache/solr/rest/RestManager.java b/solr/core/src/java/org/apache/solr/rest/RestManager.java
index aa55678f7c2..e2860b762d7 100644
--- a/solr/core/src/java/org/apache/solr/rest/RestManager.java
+++ b/solr/core/src/java/org/apache/solr/rest/RestManager.java
@@ -16,8 +16,6 @@
*/
package org.apache.solr.rest;
-import static org.apache.solr.common.util.Utils.fromJSON;
-
import java.io.FileNotFoundException;
import java.io.IOException;
import java.io.Reader;
@@ -43,6 +41,7 @@ import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.util.ContentStream;
import org.apache.solr.common.util.NamedList;
+import org.apache.solr.common.util.Utils;
import org.apache.solr.core.SolrResourceLoader;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.response.SolrQueryResponse;
@@ -374,7 +373,7 @@ public class RestManager {
Iterator<ContentStream> iter = req.getContentStreams().iterator();
if (iter.hasNext()) {
try (Reader reader = iter.next().getReader()) {
- return fromJSON(reader);
+ return Utils.fromJSON(reader);
} catch (IOException ioExc) {
throw new SolrException(ErrorCode.SERVER_ERROR, ioExc);
}
diff --git a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
index b30ec3d7442..6ab945fbdcc 100644
--- a/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
+++ b/solr/core/src/test/org/apache/solr/cloud/TestConfigSetsAPI.java
@@ -198,54 +198,79 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
@Test
public void testCreateWithTrust() throws Exception {
String configsetName = "regular";
- String configsetSuffix = "testCreateWithTrust";
- String configsetSuffix2 = "testCreateWithTrust2";
- uploadConfigSetWithAssertions(configsetName, configsetSuffix, "solr");
- uploadConfigSetWithAssertions(configsetName, configsetSuffix2, null);
+ String trustedConfigsetSuffix = "testCreateWithTrustTrusted";
+ String untrustedConfigsetSuffix = "testCreateWithTrustUntrusted";
+ uploadConfigSetWithAssertions(configsetName, trustedConfigsetSuffix, "solr");
+ uploadConfigSetWithAssertions(configsetName, untrustedConfigsetSuffix, null);
try (SolrZkClient zkClient =
new SolrZkClient(
cluster.getZkServer().getZkAddress(), AbstractZkTestCase.TIMEOUT, 45000, null)) {
- assertTrue(isTrusted(zkClient, configsetName, configsetSuffix));
- assertFalse(isTrusted(zkClient, configsetName, configsetSuffix2));
- try {
- ignoreException("unauthenticated request");
- // trusted -> unstrusted
- createConfigSet(
- configsetName + configsetSuffix,
- "foo",
- Collections.emptyMap(),
- cluster.getSolrClient(),
- null);
- fail("Expecting exception");
- } catch (SolrException e) {
- assertEquals(SolrException.ErrorCode.UNAUTHORIZED.code, e.code());
- unIgnoreException("unauthenticated request");
- }
+ assertTrue(isTrusted(zkClient, configsetName, trustedConfigsetSuffix));
+ assertFalse(isTrusted(zkClient, configsetName, untrustedConfigsetSuffix));
+ // base trusted -> untrusted
+ SolrException e =
+ assertThrows(
+ SolrException.class,
+ () ->
+ createConfigSet(
+ null, // base is default trusted
+ "abc",
+ Collections.emptyMap(),
+ cluster.getSolrClient(),
+ null) // without username is untrusted
+ );
+ assertEquals(SolrException.ErrorCode.UNAUTHORIZED.code, e.code());
+ // base trusted -> untrusted
+ SolrException e2 =
+ assertThrows(
+ SolrException.class,
+ () ->
+ createConfigSet(
+ "_default", // base is default trusted
+ "def",
+ Collections.emptyMap(),
+ cluster.getSolrClient(),
+ null) // without username is untrusted
+ );
+ assertEquals(SolrException.ErrorCode.UNAUTHORIZED.code, e2.code());
+ // trusted -> untrusted
+ SolrException e3 =
+ assertThrows(
+ SolrException.class,
+ () ->
+ createConfigSet(
+ configsetName + trustedConfigsetSuffix,
+ "foo",
+ Collections.emptyMap(),
+ cluster.getSolrClient(),
+ null) // without username is untrusted
+ );
+ assertEquals(SolrException.ErrorCode.UNAUTHORIZED.code, e3.code());
// trusted -> trusted
verifyCreate(
- configsetName + configsetSuffix,
+ configsetName + trustedConfigsetSuffix,
"foo2",
Collections.emptyMap(),
Collections.emptyMap(),
- "solr");
+ "solr"); // with username is trusted
assertTrue(isTrusted(zkClient, "foo2", ""));
- // unstrusted -> unstrusted
+ // untrusted -> untrusted
verifyCreate(
- configsetName + configsetSuffix2,
+ configsetName + untrustedConfigsetSuffix,
"bar",
Collections.emptyMap(),
Collections.emptyMap(),
- null);
+ null); // without username is untrusted
assertFalse(isTrusted(zkClient, "bar", ""));
- // unstrusted -> trusted
+ // untrusted -> trusted
verifyCreate(
- configsetName + configsetSuffix2,
+ configsetName + untrustedConfigsetSuffix,
"bar2",
Collections.emptyMap(),
Collections.emptyMap(),
- "solr");
+ "solr"); // with username is trusted
assertFalse(isTrusted(zkClient, "bar2", ""));
}
}
@@ -276,13 +301,12 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
try (final SolrClient solrClient = getHttpSolrClient(baseUrl)) {
setupBaseConfigSet(baseConfigSetName, oldProps);
- SolrZkClient zkClient =
+ try (SolrZkClient zkClient =
new SolrZkClient(
cluster.getZkServer().getZkAddress(),
AbstractZkTestCase.TIMEOUT,
AbstractZkTestCase.TIMEOUT,
- null);
- try {
+ null)) {
assertFalse(getConfigSetService().checkConfigExists(configSetName));
ConfigSetAdminResponse response =
@@ -291,8 +315,6 @@ public class TestConfigSetsAPI extends SolrCloudTestCase {
assertTrue(getConfigSetService().checkConfigExists(configSetName));
verifyProperties(configSetName, oldProps, newProps, zkClient);
- } finally {
- zkClient.close();
}
}
}
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
index 11480909a55..074d93c642f 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/Aliases.java
@@ -95,12 +95,7 @@ public class Aliases {
*/
@SuppressWarnings({"unchecked", "rawtypes"})
public static Aliases fromJSON(byte[] bytes, int zNodeVersion) {
- Map<String, Map> aliasMap;
- if (bytes == null || bytes.length == 0) {
- aliasMap = Collections.emptyMap();
- } else {
- aliasMap = (Map<String, Map>) Utils.fromJSON(bytes);
- }
+ Map<String, Map> aliasMap = (Map<String, Map>) Utils.fromJSON(bytes);
@SuppressWarnings({"rawtypes"})
Map colAliases = aliasMap.getOrDefault(COLLECTION, Collections.emptyMap());
diff --git a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
index 21a201de169..357324e148d 100644
--- a/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
+++ b/solr/solrj/src/java/org/apache/solr/common/cloud/ZkStateReader.java
@@ -18,7 +18,6 @@ package org.apache.solr.common.cloud;
import static java.util.Collections.emptyMap;
import static java.util.Collections.emptySortedSet;
-import static org.apache.solr.common.util.Utils.fromJSON;
import java.lang.invoke.MethodHandles;
import java.nio.charset.StandardCharsets;
@@ -441,7 +440,7 @@ public class ZkStateReader implements SolrCloseable {
cd.data =
pair.first() == null || pair.first().length == 0
? emptyMap()
- : Utils.getDeepCopy((Map) fromJSON(pair.first()), 4, false);
+ : Utils.getDeepCopy((Map) Utils.fromJSON(pair.first()), 4, false);
cd.version = pair.second() == null ? -1 : pair.second().getVersion();
securityData = cd;
securityNodeListener.run();
diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
index 53b967ad19c..4fe9bfca4e4 100644
--- a/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
+++ b/solr/solrj/src/java/org/apache/solr/common/util/Utils.java
@@ -302,10 +302,18 @@ public class Utils {
}
public static Object fromJSON(byte[] utf8) {
+ // Need below check in both fromJSON methods since
+ // utf8.length returns a NPE without this check.
+ if (utf8 == null || utf8.length == 0) {
+ return Collections.emptyMap();
+ }
return fromJSON(utf8, 0, utf8.length);
}
public static Object fromJSON(byte[] utf8, int offset, int length) {
+ if (utf8 == null || utf8.length == 0 || length == 0) {
+ return Collections.emptyMap();
+ }
// convert directly from bytes to chars
// and parse directly from that instead of going through
// intermediate strings or readers