You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@solr.apache.org by ho...@apache.org on 2023/07/13 16:52:18 UTC

[solr] branch main updated: SOLR-16777: Make SchemaDesigner use ConfigSet trust

This is an automated email from the ASF dual-hosted git repository.

houston pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/solr.git


The following commit(s) were added to refs/heads/main by this push:
     new 35d352522df SOLR-16777: Make SchemaDesigner use ConfigSet trust
35d352522df is described below

commit 35d352522df046aab9035d60806ababffa0f00e8
Author: Houston Putman <ho...@apache.org>
AuthorDate: Mon Jul 10 16:12:09 2023 -0400

    SOLR-16777: Make SchemaDesigner use ConfigSet trust
---
 solr/CHANGES.txt                                   |  2 +-
 .../org/apache/solr/cloud/ZkConfigSetService.java  |  4 +--
 .../org/apache/solr/core/ConfigSetService.java     | 42 ++++++++++++++--------
 .../solr/handler/configsets/ConfigSetAPIBase.java  |  9 ++---
 .../handler/configsets/CreateConfigSetAPI.java     |  2 +-
 .../handler/configsets/UploadConfigSetAPI.java     |  5 ++-
 .../solr/handler/designer/SchemaDesignerAPI.java   | 12 +++++--
 .../designer/SchemaDesignerConfigSetHelper.java    | 41 ++++++++++++++++-----
 .../designer/SchemaDesignerSettingsDAO.java        | 29 +++++++--------
 .../designer/TestSchemaDesignerSettingsDAO.java    |  3 +-
 .../indexing-guide/pages/schema-designer.adoc      | 15 ++++++++
 .../pages/major-changes-in-solr-9.adoc             |  1 +
 12 files changed, 111 insertions(+), 54 deletions(-)

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 7e0562afedf..45cec2b1cfd 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -288,7 +288,7 @@ Bug Fixes
 
 * SOLR-13605: Fix setting client scoped socket and connect timeouts when using HttpSolrClient.Builder.withHttpClient() method.  (Eric Pugh, Alex Deparvu)
 
-* SOLR-16777: Fix for Schema Designer blindly trusting potentially malicious configsets (Ishan Chattopadhyaya, Skay)
+* SOLR-16777: Schema Designer now correctly manages trust of the ConfigSets it is managing. (Ishan Chattopadhyaya, Skay, Houston Putman)
 
 * SOLR-16771: Fixed behavior and handling of 'unset' logging levels in /admin/info/logging API and related Admin UI (hossman)
 
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 88f6a7f9f91..5f83f88c8eb 100644
--- a/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/cloud/ZkConfigSetService.java
@@ -101,9 +101,9 @@ public class ZkConfigSetService extends ConfigSetService {
   }
 
   @Override
-  protected NamedList<Object> loadConfigSetFlags(CoreDescriptor cd, SolrResourceLoader loader)
-      throws IOException {
+  protected NamedList<Object> loadConfigSetFlags(SolrResourceLoader loader) throws IOException {
     try {
+      // ConfigSet flags are loaded from the metadata of the ZK node of the configset.
       return ConfigSetProperties.readFromResourceLoader(loader, ".");
     } catch (Exception ex) {
       log.debug("No configSet flags", ex);
diff --git a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
index 16f2a9bbd33..32432d9f18b 100644
--- a/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
+++ b/solr/core/src/java/org/apache/solr/core/ConfigSetService.java
@@ -29,7 +29,6 @@ import java.util.Map;
 import java.util.regex.Pattern;
 import org.apache.solr.cloud.ZkConfigSetService;
 import org.apache.solr.cloud.ZkController;
-import org.apache.solr.cloud.ZkSolrResourceLoader;
 import org.apache.solr.common.ConfigNode;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
@@ -220,6 +219,31 @@ public abstract class ConfigSetService {
     }
   }
 
+  /**
+   * Return whether the given configSet is trusted.
+   *
+   * @param name name of the configSet
+   */
+  public boolean isConfigSetTrusted(String name) throws IOException {
+    Map<String, Object> contentMap = getConfigMetadata(name);
+    return (boolean) contentMap.getOrDefault("trusted", true);
+  }
+
+  /**
+   * Return whether the configSet used for the given resourceLoader is trusted.
+   *
+   * @param coreLoader resourceLoader for a core
+   */
+  public boolean isConfigSetTrusted(SolrResourceLoader coreLoader) throws IOException {
+    // ConfigSet flags are loaded from the metadata of the ZK node of the configset. (For the
+    // ZKConfigSetService)
+    NamedList<?> flags = loadConfigSetFlags(coreLoader);
+
+    // Trust if there is no trusted flag (i.e. the ConfigSetApi was not used for this configSet)
+    // or if the trusted flag is set to "true".
+    return (flags == null || flags.get("trusted") == null || flags.getBooleanArg("trusted"));
+  }
+
   /**
    * Load the ConfigSet for a core
    *
@@ -233,16 +257,7 @@ public abstract class ConfigSetService {
     try {
       // ConfigSet properties are loaded from ConfigSetProperties.DEFAULT_FILENAME file.
       NamedList<?> properties = loadConfigSetProperties(dcore, coreLoader);
-      // ConfigSet flags are loaded from the metadata of the ZK node of the configset.
-      NamedList<?> flags = loadConfigSetFlags(dcore, coreLoader);
-
-      boolean trusted =
-          (coreLoader instanceof ZkSolrResourceLoader
-                  && flags != null
-                  && flags.get("trusted") != null
-                  && !flags.getBooleanArg("trusted"))
-              ? false
-              : true;
+      boolean trusted = isConfigSetTrusted(coreLoader);
 
       SolrConfig solrConfig = createSolrConfig(dcore, coreLoader, trusted);
       return new ConfigSet(
@@ -290,7 +305,7 @@ public abstract class ConfigSetService {
    * @return a SolrConfig object
    */
   protected SolrConfig createSolrConfig(
-      CoreDescriptor cd, SolrResourceLoader loader, boolean isTrusted) {
+      CoreDescriptor cd, SolrResourceLoader loader, boolean isTrusted) throws IOException {
     return SolrConfig.readFromResourceLoader(
         loader, cd.getConfigName(), isTrusted, cd.getSubstitutableProperties());
   }
@@ -364,8 +379,7 @@ public abstract class ConfigSetService {
 
   /** Return the ConfigSet flags or null if none. */
   // TODO should fold into configSetProps -- SOLR-14059
-  protected NamedList<Object> loadConfigSetFlags(CoreDescriptor cd, SolrResourceLoader loader)
-      throws IOException {
+  protected NamedList<Object> loadConfigSetFlags(SolrResourceLoader loader) throws IOException {
     return null;
   }
 
diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/ConfigSetAPIBase.java b/solr/core/src/java/org/apache/solr/handler/configsets/ConfigSetAPIBase.java
index c4eff442496..87c60fda94a 100644
--- a/solr/core/src/java/org/apache/solr/handler/configsets/ConfigSetAPIBase.java
+++ b/solr/core/src/java/org/apache/solr/handler/configsets/ConfigSetAPIBase.java
@@ -109,7 +109,7 @@ public class ConfigSetAPIBase {
     return contentStreamsIterator.next().getStream();
   }
 
-  protected boolean isTrusted(Principal userPrincipal, AuthenticationPlugin authPlugin) {
+  public static boolean isTrusted(Principal userPrincipal, AuthenticationPlugin authPlugin) {
     if (authPlugin != null && userPrincipal != null) {
       log.debug("Trusted configset request");
       return true;
@@ -118,11 +118,6 @@ public class ConfigSetAPIBase {
     return false;
   }
 
-  protected boolean isCurrentlyTrusted(String configName) throws IOException {
-    Map<String, Object> contentMap = configSetService.getConfigMetadata(configName);
-    return (boolean) contentMap.getOrDefault("trusted", true);
-  }
-
   protected void createBaseNode(
       ConfigSetService configSetService,
       boolean overwritesExisting,
@@ -146,7 +141,7 @@ public class ConfigSetAPIBase {
    * Fail if an untrusted request tries to update a trusted ConfigSet
    */
   private void ensureOverwritingUntrustedConfigSet(String configName) throws IOException {
-    boolean isCurrentlyTrusted = isCurrentlyTrusted(configName);
+    boolean isCurrentlyTrusted = configSetService.isConfigSetTrusted(configName);
     if (isCurrentlyTrusted) {
       throw new SolrException(
           SolrException.ErrorCode.BAD_REQUEST,
diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java b/solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java
index 78144edd247..796903ff73c 100644
--- a/solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/configsets/CreateConfigSetAPI.java
@@ -64,7 +64,7 @@ public class CreateConfigSetAPI extends ConfigSetAPIBase {
 
     if (!DISABLE_CREATE_AUTH_CHECKS
         && !isTrusted(obj.getRequest().getUserPrincipal(), coreContainer.getAuthenticationPlugin())
-        && isCurrentlyTrusted(createConfigPayload.baseConfigSet)) {
+        && configSetService.isConfigSetTrusted(createConfigPayload.baseConfigSet)) {
       throw new SolrException(
           SolrException.ErrorCode.UNAUTHORIZED,
           "Can't create a configset with an unauthenticated request from a trusted "
diff --git a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java
index f7910bf7616..f9bde932cb0 100644
--- a/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/configsets/UploadConfigSetAPI.java
@@ -106,7 +106,10 @@ public class UploadConfigSetAPI extends ConfigSetAPIBase {
 
     // If the request is doing a full trusted overwrite of an untrusted configSet (overwrite=true,
     // cleanup=true), then trust the configSet.
-    if (cleanup && requestIsTrusted && overwritesExisting && !isCurrentlyTrusted(configSetName)) {
+    if (cleanup
+        && requestIsTrusted
+        && overwritesExisting
+        && !configSetService.isConfigSetTrusted(configSetName)) {
       Map<String, Object> metadata = Collections.singletonMap("trusted", true);
       configSetService.setConfigMetadata(configSetName, metadata);
     }
diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java
index 5ceeb3b9b7c..a8a4b5091d4 100644
--- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java
+++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerAPI.java
@@ -72,6 +72,7 @@ import org.apache.solr.common.util.StrUtils;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.SolrConfig;
 import org.apache.solr.core.SolrResourceLoader;
+import org.apache.solr.handler.configsets.ConfigSetAPIBase;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.RawResponseWriter;
 import org.apache.solr.response.SolrQueryResponse;
@@ -113,9 +114,9 @@ public class SchemaDesignerAPI implements SchemaDesignerConstants {
     this.coreContainer = coreContainer;
     this.schemaSuggester = schemaSuggester;
     this.sampleDocLoader = sampleDocLoader;
-    this.settingsDAO = new SchemaDesignerSettingsDAO(coreContainer);
     this.configSetHelper =
         new SchemaDesignerConfigSetHelper(this.coreContainer, this.schemaSuggester);
+    this.settingsDAO = new SchemaDesignerSettingsDAO(coreContainer, configSetHelper);
   }
 
   public static SchemaSuggester newSchemaSuggester(CoreContainer coreContainer) {
@@ -244,13 +245,15 @@ public class SchemaDesignerAPI implements SchemaDesignerConstants {
         DefaultSampleDocumentsLoader.streamAsBytes(
             extractSingleContentStream(req, true).getStream());
     Exception updateFileError = null;
+    boolean requestIsTrusted =
+        ConfigSetAPIBase.isTrusted(req.getUserPrincipal(), coreContainer.getAuthenticationPlugin());
     if (SOLR_CONFIG_XML.equals(file)) {
       // verify the updated solrconfig.xml is valid before saving to ZK (to avoid things blowing up
       // later)
       try {
         InMemoryResourceLoader loader =
             new InMemoryResourceLoader(coreContainer, mutableId, SOLR_CONFIG_XML, data);
-        SolrConfig.readFromResourceLoader(loader, SOLR_CONFIG_XML, true, null);
+        SolrConfig.readFromResourceLoader(loader, SOLR_CONFIG_XML, requestIsTrusted, null);
       } catch (Exception exc) {
         updateFileError = exc;
       }
@@ -275,6 +278,11 @@ public class SchemaDesignerAPI implements SchemaDesignerConstants {
       throw new IOException(
           "Failed to save data in ZK at path: " + zkPath, SolrZkClient.checkInterrupted(e));
     }
+    // If the request is untrusted, and the configSet is trusted, remove the trusted flag on the
+    // configSet.
+    if (configSetHelper.isConfigSetTrusted(mutableId) && !requestIsTrusted) {
+      configSetHelper.removeConfigSetTrust(mutableId);
+    }
 
     configSetHelper.reloadTempCollection(mutableId, false);
 
diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
index 2ce27039c0d..db35a805c51 100644
--- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
+++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerConfigSetHelper.java
@@ -685,14 +685,10 @@ class SchemaDesignerConfigSetHelper implements SchemaDesignerConstants {
   }
 
   SolrConfig loadSolrConfig(String configSet) {
-    SolrResourceLoader resourceLoader = cc.getResourceLoader();
-    ZkSolrResourceLoader zkLoader =
-        new ZkSolrResourceLoader(
-            resourceLoader.getInstancePath(),
-            configSet,
-            resourceLoader.getClassLoader(),
-            cc.getZkController());
-    return SolrConfig.readFromResourceLoader(zkLoader, SOLR_CONFIG_XML, false, null);
+    ZkSolrResourceLoader zkLoader = zkLoaderForConfigSet(configSet);
+    boolean trusted = isConfigSetTrusted(configSet);
+
+    return SolrConfig.readFromResourceLoader(zkLoader, SOLR_CONFIG_XML, trusted, null);
   }
 
   ManagedIndexSchema loadLatestSchema(String configSet) {
@@ -1221,4 +1217,33 @@ class SchemaDesignerConfigSetHelper implements SchemaDesignerConstants {
     }
     return baos.toByteArray();
   }
+
+  public boolean isConfigSetTrusted(String configSetName) {
+    try {
+      return cc.getConfigSetService().isConfigSetTrusted(configSetName);
+    } catch (IOException e) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR,
+          "Could not load conf " + configSetName + ": " + e.getMessage(),
+          e);
+    }
+  }
+
+  public void removeConfigSetTrust(String configSetName) {
+    try {
+      Map<String, Object> metadata = Collections.singletonMap("trusted", false);
+      cc.getConfigSetService().setConfigMetadata(configSetName, metadata);
+    } catch (IOException e) {
+      throw new SolrException(
+          SolrException.ErrorCode.SERVER_ERROR,
+          "Could not remove trusted flag for configSet " + configSetName + ": " + e.getMessage(),
+          e);
+    }
+  }
+
+  protected ZkSolrResourceLoader zkLoaderForConfigSet(final String configSet) {
+    SolrResourceLoader loader = cc.getResourceLoader();
+    return new ZkSolrResourceLoader(
+        loader.getInstancePath(), configSet, loader.getClassLoader(), cc.getZkController());
+  }
 }
diff --git a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettingsDAO.java b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettingsDAO.java
index eea151dc69d..bf8e3d26e93 100644
--- a/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettingsDAO.java
+++ b/solr/core/src/java/org/apache/solr/handler/designer/SchemaDesignerSettingsDAO.java
@@ -33,7 +33,6 @@ import org.apache.solr.core.ConfigOverlay;
 import org.apache.solr.core.CoreContainer;
 import org.apache.solr.core.PluginInfo;
 import org.apache.solr.core.SolrConfig;
-import org.apache.solr.core.SolrResourceLoader;
 import org.apache.solr.update.processor.UpdateRequestProcessorChain;
 import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.data.Stat;
@@ -47,15 +46,15 @@ class SchemaDesignerSettingsDAO implements SchemaDesignerConstants {
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
 
   private final CoreContainer cc;
+  private final SchemaDesignerConfigSetHelper configSetHelper;
 
-  SchemaDesignerSettingsDAO(CoreContainer cc) {
+  SchemaDesignerSettingsDAO(CoreContainer cc, SchemaDesignerConfigSetHelper configSetHelper) {
     this.cc = cc;
+    this.configSetHelper = configSetHelper;
   }
 
   SchemaDesignerSettings getSettings(String configSet) {
-    SolrConfig solrConfig =
-        SolrConfig.readFromResourceLoader(
-            zkLoaderForConfigSet(configSet), SOLR_CONFIG_XML, true, null);
+    SolrConfig solrConfig = configSetHelper.loadSolrConfig(configSet);
     return getSettings(solrConfig);
   }
 
@@ -97,12 +96,14 @@ class SchemaDesignerSettingsDAO implements SchemaDesignerConstants {
     }
 
     if (changed) {
-      ZkController.persistConfigResourceToZooKeeper(
-          zkLoaderForConfigSet(configSet),
-          overlay.getVersion(),
-          ConfigOverlay.RESOURCE_NAME,
-          overlay.toByteArray(),
-          true);
+      try (ZkSolrResourceLoader resourceLoader = configSetHelper.zkLoaderForConfigSet(configSet)) {
+        ZkController.persistConfigResourceToZooKeeper(
+            resourceLoader,
+            overlay.getVersion(),
+            ConfigOverlay.RESOURCE_NAME,
+            overlay.toByteArray(),
+            true);
+      }
     }
 
     return changed;
@@ -169,10 +170,4 @@ class SchemaDesignerSettingsDAO implements SchemaDesignerConstants {
     }
     return hasPlugin;
   }
-
-  private ZkSolrResourceLoader zkLoaderForConfigSet(final String configSet) {
-    SolrResourceLoader loader = cc.getResourceLoader();
-    return new ZkSolrResourceLoader(
-        loader.getInstancePath(), configSet, loader.getClassLoader(), cc.getZkController());
-  }
 }
diff --git a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java
index c27f9d49b89..85a1232af23 100644
--- a/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java
+++ b/solr/core/src/test/org/apache/solr/handler/designer/TestSchemaDesignerSettingsDAO.java
@@ -71,7 +71,8 @@ public class TestSchemaDesignerSettingsDAO extends SolrCloudTestCase
             .process(cluster.getSolrClient());
     CollectionsHandler.waitForActiveCollection(collection, cc, rsp);
 
-    SchemaDesignerSettingsDAO dao = new SchemaDesignerSettingsDAO(cc);
+    SchemaDesignerConfigSetHelper csh = new SchemaDesignerConfigSetHelper(cc, null);
+    SchemaDesignerSettingsDAO dao = new SchemaDesignerSettingsDAO(cc, csh);
     SchemaDesignerSettings settings = dao.getSettings(configSet);
     assertNotNull(settings);
 
diff --git a/solr/solr-ref-guide/modules/indexing-guide/pages/schema-designer.adoc b/solr/solr-ref-guide/modules/indexing-guide/pages/schema-designer.adoc
index c01893944ac..2ec6bb70d42 100644
--- a/solr/solr-ref-guide/modules/indexing-guide/pages/schema-designer.adoc
+++ b/solr/solr-ref-guide/modules/indexing-guide/pages/schema-designer.adoc
@@ -79,6 +79,21 @@ Previously uploaded sample documents are indexed in the temporary collection eve
 Click on the btn:[Edit Documents] button on the *Query Results* panel to load a JSON representation of indexed documents into the text area.
 ====
 
+=== Trusted Config Sets
+
+ConfigSet trust is necessary when loading extra libraries from the `solrconfig.xml` and using certain features of Solr.
+Refer to the xref:configuration-guide:configsets-api.adoc#configsets-upload[ConfigSet API] section for more information on trusted configSets.
+
+Using the Schema Designer API has the same effect on the trust of a ConfigSet as using the xref:configuration-guide:configsets-api.adoc#configsets-upload[ConfigSet Upload API].
+
+* If you are authenticated:
+** If your source configSet **is** trusted, then the temporary configSets you are modifying will remain trusted.
+** If your source configSet **is not** trusted, then the temporary configSet you are modifying will never be trusted.
+* If you are unauthenticated:
+** The temporary configSet created for you will **not** be trusted.
+
+When <<publish,publishing the temporary configSet>>, the trust of the temporary configSet is used to set the trust of the permanent configSet.
+
 === Iteratively Post Sample Documents
 
 If you have sample documents spread across multiple files, you can POST them to the Schema Designer API and then load your schema in the Designer UI to design your schema.
diff --git a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
index e2a94642d2e..dd413832d54 100644
--- a/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
+++ b/solr/solr-ref-guide/modules/upgrade-notes/pages/major-changes-in-solr-9.adoc
@@ -91,6 +91,7 @@ The sysProp `-Dsolr.redaction.system.pattern` has been deprecated, use the above
 The `<hiddenSysProps>` solr.xml element under `<metrics>` has been deprecated.
 Instead, use the xref:configuration-guide:configuring-solr-xml.adoc#hiddenSysProps[<hiddenSysProps>] tag under `<solr>`, which accepts a comma-separated string.
 
+* The xref:indexing-guide:schema-designer.adoc[] now utilizes the same trust model as the xref:configuration-guide:configsets-api.adoc#configsets-upload[ConfigSet Upload API].
 
 === Official Docker Image
 * The customization of the Official Solr Dockerfile has been changed.