Handle url contains # and unsafe parameters (#4105)

Author: kinsanities

.gitignore

@@ -15,6 +15,7 @@
 .scala_dependencies
 .settings
 .classpath
+.flattened-pom.xml
 
 # For SBT
 .jvmopts

docs/info-1.3.1.md

@@ -1,5 +1,6 @@
 ## 参数变化
 
-| 模块名(服务名)         | 类型  | 参数名                                                                  | 默认值  | 描述                                                    |
-|------------------| ----- |----------------------------------------------------------------------|------| ------------------------------------------------------- |
-| ps-linkismanager | 修改  | pipeline.output.isoverwtite <br/>-><br/> pipeline.output.isoverwrite | true |取值范围：true或false|
+| 模块名(服务名)                                          | 类型  | 参数名                                                                  | 默认值   | 描述                                                    |
+|---------------------------------------------------|-----|----------------------------------------------------------------------|-------| ------------------------------------------------------- |
+| ps-linkismanager                                  | 修改  | pipeline.output.isoverwtite <br/>-><br/> pipeline.output.isoverwrite | true  |取值范围：true或false|
+| linkis-engineconn-plugins <br/> linkis-datasource | 新增  | linkis.mysql.strong.security.enable | false |取值范围：true或false|

linkis-dist/package/conf/linkis.properties

@@ -27,6 +27,7 @@ wds.linkis.server.mybatis.datasource.username=
 wds.linkis.server.mybatis.datasource.password=
 # mysql
 wds.linkis.mysql.is.encrypt=false
+linkis.mysql.strong.security.enable=false
 
 #hadoop/hive/spark config
 #hadoop.config.dir=/appcom/config/hadoop-config

linkis-engineconn-plugins/jdbc/src/main/java/org/apache/linkis/manager/engineplugin/jdbc/utils/JdbcParamUtils.java

@@ -17,13 +17,17 @@
 
 package org.apache.linkis.manager.engineplugin.jdbc.utils;
 
+import org.apache.linkis.common.conf.CommonVars;
+import org.apache.linkis.common.conf.CommonVars$;
 import org.apache.linkis.manager.engineplugin.jdbc.JDBCPropertiesParser;
 import org.apache.linkis.manager.engineplugin.jdbc.constant.JDBCEngineConnConstant;
 import org.apache.linkis.manager.engineplugin.jdbc.exception.JDBCParamsIllegalException;
 
 import org.apache.commons.lang3.StringUtils;
 
+import java.util.HashMap;
 import java.util.Map;
+import java.util.stream.Collectors;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -43,6 +47,9 @@ public class JdbcParamUtils {
   private static final String APPEND_PARAMS =
       "allowLoadLocalInfile=false&autoDeserialize=false&allowLocalInfile=false&allowUrlInLocalInfile=false";
 
+  public static final CommonVars<String> MYSQL_STRONG_SECURITY_ENABLE =
+      CommonVars$.MODULE$.apply("linkis.mysql.strong.security.enable", "false");
+
   private static final char AND_SYMBOL = '&';
 
   private static final String QUOTATION_MARKS = "\"";
@@ -64,20 +71,62 @@ public static void validateJdbcUrl(String url) {
   }
 
   public static String filterJdbcUrl(String url) {
+    if (StringUtils.isBlank(url)) {
+      return url;
+    }
     // temporarily filter only mysql jdbc url. & Handles cases that start with JDBC
-    if (!url.startsWith(JDBC_MYSQL_PROTOCOL) && !url.toLowerCase().contains(JDBC_MYSQL_PROTOCOL)) {
+    if (!url.toLowerCase().contains(JDBC_MYSQL_PROTOCOL)) {
       return url;
     }
-    if (url.contains(SENSITIVE_PARAM)) {
-      int index = url.indexOf(SENSITIVE_PARAM);
-      String tmp = SENSITIVE_PARAM;
-      if (url.charAt(index - 1) == AND_SYMBOL) {
-        tmp = AND_SYMBOL + tmp;
-      } else if (url.charAt(index + 1) == AND_SYMBOL) {
-        tmp = tmp + AND_SYMBOL;
+
+    // no params
+    if (!url.contains(String.valueOf(QUESTION_MARK))) {
+      return url + QUESTION_MARK + APPEND_PARAMS;
+    }
+
+    // enable strong security
+    if (Boolean.valueOf(MYSQL_STRONG_SECURITY_ENABLE.getValue())) {
+      LOG.info("mysql engine use strong security configuration. Remove all connection parameters.");
+      return url + QUESTION_MARK + APPEND_PARAMS;
+    }
+
+    // deal with params
+    String[] items = url.split("\\?");
+    // params error: multiple question marks
+    if (items.length != 2) {
+      LOG.warn("JDBC params error, the url is : " + url);
+      return items[0];
+    }
+
+    String[] params = items[1].split("&");
+    Map<String, String> paramsMap = new HashMap<>(params.length);
+    for (String param : params) {
+      String[] keyAndValues = param.split("=");
+      // params error: key and value error
+      if (keyAndValues.length != 2) {
+        continue;
       }
-      LOG.warn("Sensitive param: {} in jdbc url is filtered.", tmp);
-      url = url.replace(tmp, "");
+      String key = keyAndValues[0];
+      String value = keyAndValues[1];
+      // key and value is blank
+      if (StringUtils.isBlank(key) || StringUtils.isBlank(value)) {
+        continue;
+      }
+      if (isSecurity(key, value)) {
+        paramsMap.put(key, value);
+      } else {
+        LOG.warn("Sensitive param : key={} and value={}", key, value);
+      }
+    }
+    String extraParamString =
+        paramsMap.entrySet().stream()
+            .map(e -> String.join("=", e.getKey(), String.valueOf(e.getValue())))
+            .collect(Collectors.joining("&"));
+
+    if (StringUtils.isBlank(extraParamString)) {
+      url = items[0];
+    } else {
+      url = items[0] + String.valueOf(QUESTION_MARK) + extraParamString;
     }
     if (url.endsWith(String.valueOf(QUESTION_MARK))) {
       url = url + APPEND_PARAMS;
@@ -90,6 +139,18 @@ public static String filterJdbcUrl(String url) {
     return url;
   }
 
+  private static boolean isSecurity(String key, String value) {
+    return !(isNotSecurity(key) || isNotSecurity(value));
+  }
+
+  private static boolean isNotSecurity(String key) {
+    return key.toLowerCase().contains("allowLoadLocalInfile".toLowerCase())
+        || key.toLowerCase().contains("autoDeserialize".toLowerCase())
+        || key.toLowerCase().contains("allowLocalInfile".toLowerCase())
+        || key.toLowerCase().contains("allowUrlInLocalInfile".toLowerCase())
+        || key.toLowerCase().contains("#".toLowerCase());
+  }
+
   public static String getJdbcUsername(Map<String, String> properties)
       throws JDBCParamsIllegalException {
     String username =

linkis-engineconn-plugins/jdbc/src/test/java/org/apache/linkis/manager/engineplugin/jdbc/utils/JdbcParamUtilsTest.java

@@ -31,7 +31,21 @@ public class JdbcParamUtilsTest {
   @Test
   @DisplayName("testFilterJdbcUrl")
   public void testFilterJdbcUrl() {
-    String url = "jdbc:mysql://localhost:3306/db_name";
+    String url =
+        "jdbc:mysql://localhost:3306/db_name?allowLoadLocalInfile=true&autoDeserialize=true&#sk=13&p1=v1";
+    url = JdbcParamUtils.filterJdbcUrl(url);
+    Assertions.assertEquals(
+        "jdbc:mysql://localhost:3306/db_name?p1=v1&allowLoadLocalInfile=false&autoDeserialize=false&allowLocalInfile=false&allowUrlInLocalInfile=false",
+        url);
+
+    url =
+        "jdbc:mysql://localhost:3306/db_name?\\#allowLoadLocalInfile=true&autoDeserialize=true&#sk=13&p1=v1";
+    url = JdbcParamUtils.filterJdbcUrl(url);
+    Assertions.assertEquals(
+        "jdbc:mysql://localhost:3306/db_name?p1=v1&allowLoadLocalInfile=false&autoDeserialize=false&allowLocalInfile=false&allowUrlInLocalInfile=false",
+        url);
+
+    url = "jdbc:mysql://localhost:3306/db_name";
     url = JdbcParamUtils.filterJdbcUrl(url);
     Assertions.assertEquals(
         "jdbc:mysql://localhost:3306/db_name?allowLoadLocalInfile=false&autoDeserialize=false&allowLocalInfile=false&allowUrlInLocalInfile=false",

linkis-public-enhancements/linkis-datasource/linkis-metadata-query/service/jdbc/src/main/java/org/apache/linkis/metadata/query/service/mysql/SqlConnection.java

@@ -20,14 +20,16 @@
 import org.apache.linkis.common.conf.CommonVars;
 import org.apache.linkis.metadata.query.common.domain.MetaColumnInfo;
 
-import org.springframework.util.CollectionUtils;
+import org.apache.commons.lang.StringUtils;
 
 import java.io.Closeable;
 import java.io.IOException;
 import java.sql.*;
 import java.util.*;
 import java.util.stream.Collectors;
 
+import scala.annotation.meta.param;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -47,6 +49,9 @@ public class SqlConnection implements Closeable {
   private static final CommonVars<Integer> SQL_SOCKET_TIMEOUT =
       CommonVars.apply("wds.linkis.server.mdm.service.sql.socket.timeout", 6000);
 
+  private static final CommonVars<Boolean> MYSQL_STRONG_SECURITY_ENABLE =
+      CommonVars.apply("linkis.mysql.strong.security.enable", false);
+
   private Connection conn;
 
   private ConnectMessage connectMessage;
@@ -74,21 +79,36 @@ public SqlConnection(
    * @param extraParams
    */
   private void validateParams(Map<String, Object> extraParams) {
-    if (CollectionUtils.isEmpty(extraParams)) {
+    if (extraParams == null) {
       return;
     }
 
+    // enable strong security
+    if (MYSQL_STRONG_SECURITY_ENABLE.getValue()) {
+      LOG.info(
+          "mysql metadata use strong security configuration. Remove all connection parameters.");
+      extraParams.clear();
+    }
+
     // Delete suspected vulnerability parameters
     Iterator<Map.Entry<String, Object>> iterator = extraParams.entrySet().iterator();
     while (iterator.hasNext()) {
       Map.Entry<String, Object> entry = iterator.next();
       String key = entry.getKey();
-      if ("allowLoadLocalInfile".equalsIgnoreCase(key)
-          || "autoDeserialize".equalsIgnoreCase(key)
-          || "allowLocalInfile".equalsIgnoreCase(key)
-          || "allowUrlInLocalInfile".equalsIgnoreCase(key)) {
-        extraParams.remove(key);
+      if (StringUtils.isBlank(key)
+          || entry.getValue() == null
+          || StringUtils.isBlank(entry.getValue().toString())) {
         iterator.remove();
+        continue;
+      }
+      String value = entry.getValue().toString();
+      if (keyAndValueIsNotSecurity(key, value, "allowLoadLocalInfile")
+          || keyAndValueIsNotSecurity(key, value, "autoDeserialize")
+          || keyAndValueIsNotSecurity(key, value, "allowLocalInfile")
+          || keyAndValueIsNotSecurity(key, value, "allowUrlInLocalInfile")
+          || keyAndValueIsNotSecurity(key, value, "#")) {
+        iterator.remove();
+        LOG.warn("mysql metadata sensitive param : key={} and value={}", key, value);
       }
     }
 
@@ -97,6 +117,26 @@ private void validateParams(Map<String, Object> extraParams) {
     extraParams.put("autoDeserialize", "false");
     extraParams.put("allowLocalInfile", "false");
     extraParams.put("allowUrlInLocalInfile", "false");
+
+    // print extraParams
+    StringBuilder sb = new StringBuilder("mysql metadata url extraParams: [ ");
+    for (Map.Entry<String, Object> paramEntry : extraParams.entrySet()) {
+      sb.append(paramEntry.getKey()).append("=").append(paramEntry.getValue()).append(" ,");
+    }
+    sb.deleteCharAt(sb.length() - 1);
+    sb.append("]");
+    LOG.info(sb.toString());
+  }
+
+  private boolean keyAndValueIsNotSecurity(String key, String value, String param) {
+    return !(isSecurity(key, param) && isSecurity(value, param));
+  }
+
+  private boolean isSecurity(String noSecurityKey, String param) {
+    if (StringUtils.isBlank(param) || StringUtils.isBlank(noSecurityKey)) {
+      return true;
+    }
+    return !noSecurityKey.toLowerCase().contains(param.toLowerCase());
   }
 
   public List<String> getAllDatabases() throws SQLException {