Skip to content

Commit 1c03908

Browse files
[#7614] improvement(core, jdbc-catalog): Validate URLs for JDBC entity store. (#7684)
### What changes were proposed in this pull request? Add validation logic for URLs of JDBC entity store. ### Why are the changes needed? To fix possible problem caused by improper configurations in the URL URLs. Fix: #7614 ### Does this PR introduce _any_ user-facing change? N/A. ### How was this patch tested? Existing tests. Co-authored-by: Mini Yu <[email protected]>
1 parent 620fe72 commit 1c03908

File tree

4 files changed

+133
-63
lines changed

4 files changed

+133
-63
lines changed

catalogs/catalog-jdbc-common/src/main/java/org/apache/gravitino/catalog/jdbc/utils/DataSourceUtils.java

Lines changed: 2 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -19,15 +19,14 @@
1919
package org.apache.gravitino.catalog.jdbc.utils;
2020

2121
import java.sql.SQLException;
22-
import java.util.Arrays;
23-
import java.util.List;
2422
import java.util.Map;
2523
import java.util.Properties;
2624
import javax.sql.DataSource;
2725
import org.apache.commons.dbcp2.BasicDataSource;
2826
import org.apache.commons.dbcp2.BasicDataSourceFactory;
2927
import org.apache.gravitino.catalog.jdbc.config.JdbcConfig;
3028
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
29+
import org.apache.gravitino.utils.JdbcUrlUtils;
3130

3231
/**
3332
* Utility class for creating a {@link DataSource} from a {@link JdbcConfig}. It is mainly
@@ -39,26 +38,6 @@ public class DataSourceUtils {
3938
/** SQL statements for database connection pool testing. */
4039
private static final String POOL_TEST_QUERY = "SELECT 1";
4140

42-
private static final List<String> unsafeMySQLParameters =
43-
Arrays.asList(
44-
"maxAllowedPacket",
45-
"autoDeserialize",
46-
"queryInterceptors",
47-
"statementInterceptors",
48-
"detectCustomCollations",
49-
"allowloadlocalinfile",
50-
"allowUrlInLocalInfile",
51-
"allowLoadLocalInfileInPath");
52-
53-
private static final List<String> unsafePostgresParameters =
54-
Arrays.asList(
55-
"socketFactory",
56-
"socketFactoryArg",
57-
"sslfactory",
58-
"sslhostnameverifier",
59-
"sslpasswordcallback",
60-
"authenticationPluginClassName");
61-
6241
public static DataSource createDataSource(Map<String, String> properties) {
6342
return createDataSource(new JdbcConfig(properties));
6443
}
@@ -73,7 +52,7 @@ public static DataSource createDataSource(JdbcConfig jdbcConfig)
7352
}
7453

7554
private static DataSource createDBCPDataSource(JdbcConfig jdbcConfig) throws Exception {
76-
validateJdbcConfig(
55+
JdbcUrlUtils.validateJdbcConfig(
7756
jdbcConfig.getJdbcDriver(), jdbcConfig.getJdbcUrl(), jdbcConfig.getAllConfig());
7857
BasicDataSource basicDataSource =
7958
BasicDataSourceFactory.createDataSource(getProperties(jdbcConfig));
@@ -117,44 +96,6 @@ private static String recursiveDecode(String url) {
11796
return decoded;
11897
}
11998

120-
private static void checkUnsafeParameters(
121-
String url, Map<String, String> config, List<String> unsafeParams, String dbType) {
122-
123-
String lowerUrl = url.toLowerCase();
124-
125-
for (String param : unsafeParams) {
126-
String lowerParam = param.toLowerCase();
127-
if (lowerUrl.contains(lowerParam) || containsValueIgnoreCase(config, param)) {
128-
throw new GravitinoRuntimeException(
129-
"Unsafe %s parameter '%s' detected in JDBC URL", dbType, param);
130-
}
131-
}
132-
}
133-
134-
public static void validateJdbcConfig(String driver, String url, Map<String, String> all) {
135-
String lowerUrl = url.toLowerCase();
136-
String decodedUrl = recursiveDecode(lowerUrl);
137-
138-
if (driver != null) {
139-
if (decodedUrl.startsWith("jdbc:mysql")) {
140-
checkUnsafeParameters(decodedUrl, all, unsafeMySQLParameters, "MySQL");
141-
} else if (decodedUrl.startsWith("jdbc:mariadb")) {
142-
checkUnsafeParameters(decodedUrl, all, unsafeMySQLParameters, "MariaDB");
143-
} else if (decodedUrl.startsWith("jdbc:postgresql")) {
144-
checkUnsafeParameters(decodedUrl, all, unsafePostgresParameters, "PostgreSQL");
145-
}
146-
}
147-
}
148-
149-
private static boolean containsValueIgnoreCase(Map<String, String> map, String value) {
150-
for (String keyValue : map.values()) {
151-
if (keyValue != null && keyValue.equalsIgnoreCase(value)) {
152-
return true;
153-
}
154-
}
155-
return false;
156-
}
157-
15899
public static void closeDataSource(DataSource dataSource) {
159100
if (null != dataSource) {
160101
try {
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
import org.junit.jupiter.api.Assertions;
2929
import org.junit.jupiter.api.Test;
3030

31-
public class TestDataSourceUtils {
31+
public class TestDataSourceUrlValidation {
3232

3333
@Test
3434
public void testCreateDataSource() throws SQLException {
Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.gravitino.utils;
21+
22+
import java.net.URLDecoder;
23+
import java.util.Arrays;
24+
import java.util.List;
25+
import java.util.Map;
26+
import org.apache.gravitino.exceptions.GravitinoRuntimeException;
27+
28+
/**
29+
* Utility class for validating JDBC URLs and configurations. It checks for unsafe parameters
30+
* specific to MySQL, MariaDB, and PostgreSQL JDBC URLs. Unsafe parameters can lead to security
31+
* vulnerabilities or unexpected behavior, so this utility helps ensure that JDBC configurations are
32+
* safe to use.
33+
*/
34+
public class JdbcUrlUtils {
35+
36+
// Unsafe parameters for MySQL and MariaDB, other parameters like
37+
// trustCertificateKeyStoreUrl, serverTimezone, characterEncoding,
38+
// useSSL are also risky, please use it with caution.
39+
private static final List<String> UNSAFE_MYSQL_PARAMETERS =
40+
Arrays.asList(
41+
"maxAllowedPacket",
42+
"autoDeserialize",
43+
"queryInterceptors",
44+
"statementInterceptors",
45+
"detectCustomCollations",
46+
"allowloadlocalinfile",
47+
"allowUrlInLocalInfile",
48+
"allowLoadLocalInfileInPath");
49+
50+
private static final List<String> UNSAFE_POSTGRES_PARAMETERS =
51+
Arrays.asList(
52+
"socketFactory",
53+
"socketFactoryArg",
54+
"sslfactory",
55+
"sslhostnameverifier",
56+
"sslpasswordcallback",
57+
"authenticationPluginClassName");
58+
59+
private JdbcUrlUtils() {
60+
// Utility class, no instantiation allowed
61+
}
62+
63+
/**
64+
* Validates the JDBC configuration by checking the driver and URL for unsafe parameters.
65+
*
66+
* @param driver the JDBC driver class name
67+
* @param url the JDBC URL
68+
* @param all the JDBC configuration properties
69+
*/
70+
public static void validateJdbcConfig(String driver, String url, Map<String, String> all) {
71+
String lowerUrl = url.toLowerCase();
72+
String decodedUrl = recursiveDecode(lowerUrl);
73+
74+
// As H2 is only used for testing, we do not check unsafe parameters for H2.
75+
if (driver != null) {
76+
if (decodedUrl.startsWith("jdbc:mysql")) {
77+
checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS, "MySQL");
78+
} else if (decodedUrl.startsWith("jdbc:mariadb")) {
79+
checkUnsafeParameters(decodedUrl, all, UNSAFE_MYSQL_PARAMETERS, "MariaDB");
80+
} else if (decodedUrl.startsWith("jdbc:postgresql")) {
81+
checkUnsafeParameters(decodedUrl, all, UNSAFE_POSTGRES_PARAMETERS, "PostgreSQL");
82+
}
83+
}
84+
}
85+
86+
private static void checkUnsafeParameters(
87+
String url, Map<String, String> config, List<String> unsafeParams, String dbType) {
88+
89+
String lowerUrl = url.toLowerCase();
90+
91+
for (String param : unsafeParams) {
92+
String lowerParam = param.toLowerCase();
93+
if (lowerUrl.contains(lowerParam) || containsValueIgnoreCase(config, param)) {
94+
throw new GravitinoRuntimeException(
95+
"Unsafe %s parameter '%s' detected in JDBC URL", dbType, param);
96+
}
97+
}
98+
}
99+
100+
private static String recursiveDecode(String url) {
101+
String prev;
102+
String decoded = url;
103+
int max = 5;
104+
105+
do {
106+
prev = decoded;
107+
try {
108+
decoded = URLDecoder.decode(prev, "UTF-8");
109+
} catch (Exception e) {
110+
throw new GravitinoRuntimeException("Unable to decode JDBC URL");
111+
}
112+
} while (!prev.equals(decoded) && --max > 0);
113+
114+
return decoded;
115+
}
116+
117+
private static boolean containsValueIgnoreCase(Map<String, String> map, String value) {
118+
for (String keyValue : map.values()) {
119+
if (keyValue != null && keyValue.equalsIgnoreCase(value)) {
120+
return true;
121+
}
122+
}
123+
return false;
124+
}
125+
}

core/src/main/java/org/apache/gravitino/storage/relational/session/SqlSessionFactoryHelper.java

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@
3232
import org.apache.gravitino.metrics.source.RelationDatasourceMetricsSource;
3333
import org.apache.gravitino.storage.relational.JDBCBackend.JDBCBackendType;
3434
import org.apache.gravitino.storage.relational.mapper.provider.MapperPackageProvider;
35+
import org.apache.gravitino.utils.JdbcUrlUtils;
3536
import org.apache.ibatis.mapping.Environment;
3637
import org.apache.ibatis.session.Configuration;
3738
import org.apache.ibatis.session.SqlSessionFactory;
@@ -64,9 +65,12 @@ public void init(Config config) {
6465
// Initialize the data source
6566
BasicDataSource dataSource = new BasicDataSource();
6667
String jdbcUrl = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_URL);
68+
String driverClass = config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER);
69+
JdbcUrlUtils.validateJdbcConfig(jdbcUrl, driverClass, config.getAllConfig());
70+
6771
JDBCBackendType jdbcType = JDBCBackendType.fromURI(jdbcUrl);
6872
dataSource.setUrl(jdbcUrl);
69-
dataSource.setDriverClassName(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_DRIVER));
73+
dataSource.setDriverClassName(driverClass);
7074
dataSource.setUsername(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_USER));
7175
dataSource.setPassword(config.get(Configs.ENTITY_RELATIONAL_JDBC_BACKEND_PASSWORD));
7276
// Close the auto commit, so that we can control the transaction manual commit

0 commit comments

Comments
 (0)