Skip to content

Commit

Permalink
SOLR-15217: Rename shardsWhitelist and extract AllowListUrlChecker.
Browse files Browse the repository at this point in the history
  • Loading branch information
broustant authored and bruno-roustant committed Apr 26, 2021
1 parent 1adedf5 commit c7abf3f
Show file tree
Hide file tree
Showing 45 changed files with 731 additions and 492 deletions.
2 changes: 2 additions & 0 deletions solr/CHANGES.txt
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ when told to. The admin UI now tells it to. (Nazerke Seidan, David Smiley)

* SOLR-15329: Improve HDFS Directory size calculation (Andras Salamon via Mike Drob)

* SOLR-15340: Rename shardsWhitelist and extract AllowListUrlChecker to use it more broadly. (Bruno Roustant)

Other Changes
----------------------
* SOLR-14656: Autoscaling framework removed (Ishan Chattopadhyaya, noble, Ilan Ginzburg)
Expand Down
8 changes: 4 additions & 4 deletions solr/bin/solr.in.cmd
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,10 @@ REM -DzkDigestReadonlyUsername=readonly-user -DzkDigestReadonlyPassword=CHANGEM
REM set SOLR_OPTS=%SOLR_OPTS% %SOLR_ZK_CREDS_AND_ACLS%

REM When running Solr in non-cloud mode and if planning to do distributed search (using the "shards" parameter), the
REM list of hosts needs to be whitelisted or Solr will forbid the request. The whitelist can be configured in solr.xml,
REM or if you are using the OOTB solr.xml, can be specified using the system property "solr.shardsWhitelist". Alternatively
REM host checking can be disabled by using the system property "solr.disable.shardsWhitelist"
REM set SOLR_OPTS=%SOLR_OPTS% -Dsolr.shardsWhitelist=http://localhost:8983,http://localhost:8984
REM list of hosts needs to be defined in an allow-list or Solr will forbid the request. The allow-list can be configured
REM in solr.xml, or if you are using the OOTB solr.xml, can be specified using the system property "solr.allowUrls".
REM Alternatively host checking can be disabled by using the system property "solr.disable.allowUrls"
REM set SOLR_OPTS=%SOLR_OPTS% -Dsolr.allowUrls=http://localhost:8983,http://localhost:8984

REM For a visual indication in the Admin UI of what type of environment this cluster is, configure
REM a -Dsolr.environment property below. Valid values are prod, stage, test, dev, with an optional
Expand Down
8 changes: 4 additions & 4 deletions solr/bin/solr.in.sh
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@
#SOLR_ULIMIT_CHECKS=

# When running Solr in non-cloud mode and if planning to do distributed search (using the "shards" parameter), the
# list of hosts needs to be whitelisted or Solr will forbid the request. The whitelist can be configured in solr.xml,
# or if you are using the OOTB solr.xml, can be specified using the system property "solr.shardsWhitelist". Alternatively
# host checking can be disabled by using the system property "solr.disable.shardsWhitelist"
#SOLR_OPTS="$SOLR_OPTS -Dsolr.shardsWhitelist=http://localhost:8983,http://localhost:8984"
# list of hosts needs to be defined in an allow-list or Solr will forbid the request. The allow-list can be configured
# in solr.xml, or if you are using the OOTB solr.xml, can be specified using the system property "solr.allowUrls".
# Alternatively host checking can be disabled by using the system property "solr.disable.allowUrls"
#SOLR_OPTS="$SOLR_OPTS -Dsolr.allowUrls=http://localhost:8983,http://localhost:8984"

# For a visual indication in the Admin UI of what type of environment this cluster is, configure
# a -Dsolr.environment property below. Valid values are prod, stage, test, dev, with an optional
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,12 @@
<str name="configSetBaseDir">${configSetBaseDir:configsets}</str>
<str name="coreRootDirectory">${coreRootDirectory:.}</str>
<str name="collectionsHandler">${collectionsHandler:solr.CollectionsHandler}</str>
<str name="allowUrls">${solr.tests.allowUrls:}</str>

<shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
<str name="urlScheme">${urlScheme:}</str>
<int name="socketTimeout">${socketTimeout:90000}</int>
<int name="connTimeout">${connTimeout:15000}</int>
<str name="shardsWhitelist">${solr.tests.shardsWhitelist:}</str>
</shardHandlerFactory>

<tracerConfig name="tracerConfig" class="org.apache.solr.jaeger.JaegerTracerConfigurator" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
<str name="shareSchema">${shareSchema:false}</str>
<str name="configSetBaseDir">${configSetBaseDir:configsets}</str>
<str name="coreRootDirectory">${coreRootDirectory:.}</str>
<str name="allowUrls">${solr.tests.allowUrls:}</str>

<shardHandlerFactory name="shardHandlerFactory" class="HttpShardHandlerFactory">
<str name="urlScheme">${urlScheme:}</str>
Expand Down
12 changes: 12 additions & 0 deletions solr/core/src/java/org/apache/solr/core/CoreContainer.java
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@
import org.apache.solr.request.SolrRequestHandler;
import org.apache.solr.request.SolrRequestInfo;
import org.apache.solr.search.SolrFieldCacheBean;
import org.apache.solr.security.AllowListUrlChecker;
import org.apache.solr.security.AuditLoggerPlugin;
import org.apache.solr.security.AuthenticationPlugin;
import org.apache.solr.security.AuthorizationPlugin;
Expand Down Expand Up @@ -280,6 +281,8 @@ public CoreLoadFailure(CoreDescriptor cd, Exception loadFailure) {

private Set<Path> allowPaths;

private AllowListUrlChecker allowListUrlChecker;

// Bits for the state variable.
public final static long LOAD_COMPLETE = 0x1L;
public final static long CORE_DISCOVERY_COMPLETE = 0x2L;
Expand Down Expand Up @@ -405,6 +408,8 @@ public CoreContainer(NodeConfig config, CoresLocator locator, boolean asyncSolrC
}
}

this.allowListUrlChecker = AllowListUrlChecker.create(config);

Path userFilesPath = getUserFilesPath(); // TODO make configurable on cfg?
try {
Files.createDirectories(userFilesPath); // does nothing if already exists
Expand Down Expand Up @@ -1445,6 +1450,13 @@ public Set<Path> getAllowPaths() {
return allowPaths;
}

/**
* Gets the URLs checker based on the {@code allowUrls} configuration of solr.xml.
*/
public AllowListUrlChecker getAllowListUrlChecker() {
return allowListUrlChecker;
}

/**
* Creates a new core based on a CoreDescriptor.
*
Expand Down
46 changes: 34 additions & 12 deletions solr/core/src/java/org/apache/solr/core/NodeConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,18 +16,19 @@
*/
package org.apache.solr.core;

import org.apache.lucene.search.IndexSearcher;
import org.apache.solr.common.SolrException;
import org.apache.solr.logging.LogWatcherConfig;
import org.apache.solr.update.UpdateShardHandlerConfig;

import java.nio.file.Path;
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Properties;
import java.util.Set;

import org.apache.lucene.search.IndexSearcher;
import org.apache.solr.common.SolrException;
import org.apache.solr.logging.LogWatcherConfig;
import org.apache.solr.update.UpdateShardHandlerConfig;


public class NodeConfig {
// all Path fields here are absolute and normalized.
Expand All @@ -44,6 +45,8 @@ public class NodeConfig {

private final Set<Path> allowPaths;

private final List<String> allowUrls;

private final String sharedLibDirectory;

private final PluginInfo shardHandlerFactoryConfig;
Expand Down Expand Up @@ -102,7 +105,8 @@ private NodeConfig(String nodeName, Path coreRootDirectory, Path solrDataHome, I
Path solrHome, SolrResourceLoader loader,
Properties solrProperties, PluginInfo[] backupRepositoryPlugins,
MetricsConfig metricsConfig, PluginInfo transientCacheConfig, PluginInfo tracerConfig,
boolean fromZookeeper, String defaultZkHost, Set<Path> allowPaths,String configSetServiceClass) {
boolean fromZookeeper, String defaultZkHost, Set<Path> allowPaths, List<String> allowUrls,
String configSetServiceClass) {
// all Path params here are absolute and normalized.
this.nodeName = nodeName;
this.coreRootDirectory = coreRootDirectory;
Expand Down Expand Up @@ -134,6 +138,7 @@ private NodeConfig(String nodeName, Path coreRootDirectory, Path solrDataHome, I
this.fromZookeeper = fromZookeeper;
this.defaultZkHost = defaultZkHost;
this.allowPaths = allowPaths;
this.allowUrls = allowUrls;
this.configSetServiceClass = configSetServiceClass;

if (this.cloudConfig != null && this.getCoreLoadThreadCount(false) < 2) {
Expand Down Expand Up @@ -309,6 +314,13 @@ public String getDefaultZkHost() {
*/
public Set<Path> getAllowPaths() { return allowPaths; }

/**
* Allow-list of Solr nodes URLs.
*/
public List<String> getAllowUrls() {
return allowUrls;
}

public static class NodeConfigBuilder {
// all Path fields here are absolute and normalized.
private SolrResourceLoader loader;
Expand Down Expand Up @@ -342,6 +354,7 @@ public static class NodeConfigBuilder {
private boolean fromZookeeper = false;
private String defaultZkHost;
private Set<Path> allowPaths = Collections.emptySet();
private List<String> allowUrls = Collections.emptyList();

private final Path solrHome;
private final String nodeName;
Expand Down Expand Up @@ -516,6 +529,11 @@ public NodeConfigBuilder setAllowPaths(Set<Path> paths) {
return this;
}

public NodeConfigBuilder setAllowUrls(List<String> urls) {
this.allowUrls = urls;
return this;
}

public NodeConfigBuilder setConfigSetServiceClass(String configSetServiceClass){
this.configSetServiceClass = configSetServiceClass;
return this;
Expand All @@ -526,12 +544,16 @@ public NodeConfig build() {
if (loader == null) {
loader = new SolrResourceLoader(solrHome);
}
return new NodeConfig(nodeName, coreRootDirectory, solrDataHome, booleanQueryMaxClauseCount,
configSetBaseDirectory, sharedLibDirectory, shardHandlerFactoryConfig,
updateShardHandlerConfig, coreAdminHandlerClass, collectionsAdminHandlerClass, healthCheckHandlerClass, infoHandlerClass, configSetsHandlerClass,
logWatcherConfig, cloudConfig, coreLoadThreads, replayUpdatesThreads, transientCacheSize, useSchemaCache, managementPath,
solrHome, loader, solrProperties,
backupRepositoryPlugins, metricsConfig, transientCacheConfig, tracerConfig, fromZookeeper, defaultZkHost, allowPaths, configSetServiceClass);
return new NodeConfig(
nodeName, coreRootDirectory, solrDataHome, booleanQueryMaxClauseCount,
configSetBaseDirectory, sharedLibDirectory, shardHandlerFactoryConfig,
updateShardHandlerConfig, coreAdminHandlerClass, collectionsAdminHandlerClass,
healthCheckHandlerClass, infoHandlerClass, configSetsHandlerClass,
logWatcherConfig, cloudConfig, coreLoadThreads, replayUpdatesThreads,
transientCacheSize, useSchemaCache, managementPath,
solrHome, loader, solrProperties,
backupRepositoryPlugins, metricsConfig, transientCacheConfig, tracerConfig,
fromZookeeper, defaultZkHost, allowPaths, allowUrls, configSetServiceClass);
}

public NodeConfigBuilder setSolrResourceLoader(SolrResourceLoader resourceLoader) {
Expand Down
8 changes: 7 additions & 1 deletion solr/core/src/java/org/apache/solr/core/SolrPaths.java
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.lang.invoke.MethodHandles;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Collections;
import java.util.Set;

import org.apache.commons.exec.OS;
Expand All @@ -34,6 +35,11 @@
public final class SolrPaths {
private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

/**
* Special path set which accepts all paths.
*/
public static final Set<Path> ALL_PATHS = Collections.singleton(Paths.get("_ALL_"));

private SolrPaths() {} // don't create this

/**
Expand All @@ -58,6 +64,7 @@ public static String normalizeDir(String path) {
* @throws SolrException if path is outside allowed paths
*/
public static void assertPathAllowed(Path pathToAssert, Set<Path> allowPaths) throws SolrException {
if (ALL_PATHS.equals(allowPaths)) return; // Catch-all allows all paths (*/_ALL_)
if (pathToAssert == null) return;
if (OS.isFamilyWindows() && pathToAssert.toString().startsWith("\\\\")) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
Expand All @@ -70,7 +77,6 @@ public static void assertPathAllowed(Path pathToAssert, Set<Path> allowPaths) th
"Path " + pathToAssert + " disallowed due to path traversal..");
}
if (!path.isAbsolute()) return; // All relative paths are accepted
if (allowPaths.contains(Paths.get("_ALL_"))) return; // Catch-all path "*"/"_ALL_" will allow all other paths
if (allowPaths.stream().noneMatch(p -> path.startsWith(Path.of(p.toString())))) {
throw new SolrException(SolrException.ErrorCode.BAD_REQUEST,
"Path " + path + " must be relative to SOLR_HOME, SOLR_DATA_HOME coreRootDirectory. Set system property 'solr.allowPaths' to add other allowed paths.");
Expand Down
32 changes: 26 additions & 6 deletions solr/core/src/java/org/apache/solr/core/SolrXmlConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@
import java.util.Map;
import java.util.Properties;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.regex.Pattern;

import com.google.common.base.Strings;
import com.google.common.collect.Sets;
import org.apache.commons.io.IOUtils;
import org.apache.commons.lang3.StringUtils;
import org.apache.solr.client.solrj.impl.HttpClientUtil;
Expand Down Expand Up @@ -74,6 +75,8 @@ public class SolrXmlConfig {

private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());

private static final Pattern COMMA_SEPARATED_PATTERN = Pattern.compile("\\s*,\\s*");

/**
* Given some node Properties, checks if non-null and a 'zkHost' is alread included. If so, the Properties are
* returned as is. If not, then the returned value will be a new Properties, wrapping the original Properties,
Expand Down Expand Up @@ -327,7 +330,7 @@ private static NodeConfig fillSolrSection(NodeConfig.NodeConfigBuilder builder,
builder.setSharedLibDirectory(value);
break;
case "allowPaths":
builder.setAllowPaths(stringToPaths(value));
builder.setAllowPaths(separatePaths(value));
break;
case "configSetBaseDir":
builder.setConfigSetBaseDirectory(value);
Expand All @@ -344,6 +347,9 @@ private static NodeConfig fillSolrSection(NodeConfig.NodeConfigBuilder builder,
case "transientCacheSize":
builder.setTransientCacheSize(parseInt(name, value));
break;
case "allowUrls":
builder.setAllowUrls(separateStrings(value));
break;
default:
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Unknown configuration value in solr.xml: " + name);
}
Expand All @@ -352,13 +358,27 @@ private static NodeConfig fillSolrSection(NodeConfig.NodeConfigBuilder builder,
return builder.build();
}

private static Set<Path> stringToPaths(String commaSeparatedString) {
private static List<String> separateStrings(String commaSeparatedString) {
if (Strings.isNullOrEmpty(commaSeparatedString)) {
return Collections.emptyList();
}
return Arrays.asList(COMMA_SEPARATED_PATTERN.split(commaSeparatedString));
}

private static Set<Path> separatePaths(String commaSeparatedString) {
if (Strings.isNullOrEmpty(commaSeparatedString)) {
return Collections.emptySet();
}
// Parse list of paths. The special value '*' is mapped to _ALL_ to mean all paths
return Arrays.stream(commaSeparatedString.split(",\\s?"))
.map(p -> Paths.get("*".equals(p) ? "_ALL_" : p)).collect(Collectors.toSet());
// Parse the list of paths. The special values '*' and '_ALL_' mean all paths.
String[] pathStrings = COMMA_SEPARATED_PATTERN.split(commaSeparatedString);
Set<Path> paths = Sets.newHashSetWithExpectedSize(pathStrings.length);
for (String p : pathStrings) {
if ("*".equals(p) || "_ALL_".equals(p)) {
return SolrPaths.ALL_PATHS;
}
paths.add(Paths.get(p));
}
return paths;
}

private static UpdateShardHandlerConfig loadUpdateConfig(NamedList<Object> nl, boolean alwaysDefine) {
Expand Down
29 changes: 26 additions & 3 deletions solr/core/src/java/org/apache/solr/handler/IndexFetcher.java
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import java.io.OutputStreamWriter;
import java.io.Writer;
import java.lang.invoke.MethodHandles;
import java.net.MalformedURLException;
import java.nio.ByteBuffer;
import java.nio.channels.FileChannel;
import java.nio.charset.StandardCharsets;
Expand Down Expand Up @@ -79,6 +80,7 @@
import org.apache.solr.cloud.ZkController;
import org.apache.solr.common.SolrException;
import org.apache.solr.common.SolrException.ErrorCode;
import org.apache.solr.common.cloud.ClusterState;
import org.apache.solr.common.cloud.Replica;
import org.apache.solr.common.params.CommonParams;
import org.apache.solr.common.params.ModifiableSolrParams;
Expand All @@ -94,6 +96,7 @@
import org.apache.solr.request.LocalSolrQueryRequest;
import org.apache.solr.request.SolrQueryRequest;
import org.apache.solr.search.SolrIndexSearcher;
import org.apache.solr.security.AllowListUrlChecker;
import org.apache.solr.update.CommitUpdateCommand;
import org.apache.solr.common.util.SolrNamedThreadFactory;
import org.apache.solr.util.FileUtils;
Expand Down Expand Up @@ -242,7 +245,7 @@ public IndexFetcher(@SuppressWarnings({"rawtypes"})final NamedList initArgs, fin
leaderUrl = leaderUrl.substring(0, leaderUrl.length()-12);
log.warn("'leaderUrl' must be specified without the {} suffix", ReplicationHandler.PATH);
}
this.leaderUrl = leaderUrl;
setLeaderUrl(leaderUrl);

this.replicationHandler = handler;
String compress = (String) initArgs.get(COMPRESSION);
Expand All @@ -261,7 +264,27 @@ public IndexFetcher(@SuppressWarnings({"rawtypes"})final NamedList initArgs, fin
String httpBasicAuthPassword = (String) initArgs.get(HttpClientUtil.PROP_BASIC_AUTH_PASS);
myHttpClient = createHttpClient(solrCore, httpBasicAuthUser, httpBasicAuthPassword, useExternalCompression);
}


private void setLeaderUrl(String leaderUrl) {
if (leaderUrl != null) {
ClusterState clusterState = solrCore.getCoreContainer().getZkController() == null ?
null : solrCore.getCoreContainer().getZkController().getClusterState();
try {
solrCore.getCoreContainer().getAllowListUrlChecker()
.checkAllowList(Collections.singletonList(leaderUrl), clusterState);
} catch (MalformedURLException e) {
throw new SolrException(SolrException.ErrorCode.SERVER_ERROR, "Malformed 'leaderUrl' " + leaderUrl, e);
} catch (SolrException e) {
throw new SolrException(SolrException.ErrorCode.FORBIDDEN,
"The '" + LEADER_URL + "' parameter value '" + leaderUrl
+ "' is not allowed: "
+ e.getMessage() + ". "
+ AllowListUrlChecker.SET_SOLR_DISABLE_URL_ALLOW_LIST_CLUE);
}
}
this.leaderUrl = leaderUrl;
}

@SuppressWarnings({"unchecked"})
protected <T> T getParameter(@SuppressWarnings({"rawtypes"})NamedList initArgs, String configKey, T defaultValue, StringBuilder sb) {
T toReturn = defaultValue;
Expand Down Expand Up @@ -389,7 +412,7 @@ IndexFetchResult fetchLatestIndex(boolean forceReplication, boolean forceCoreRel
return IndexFetchResult.LEADER_IS_NOT_ACTIVE;
}
if (!replica.getCoreUrl().equals(leaderUrl)) {
leaderUrl = replica.getCoreUrl();
setLeaderUrl(replica.getCoreUrl());
log.info("Updated leaderUrl to {}", leaderUrl);
// TODO: Do we need to set forceReplication = true?
} else {
Expand Down
Loading

0 comments on commit c7abf3f

Please sign in to comment.