CalvinKirs commented on code in PR #68129:
URL: https://github.com/apache/doris/pull/68129#discussion_r4035131243
##########
fe/fe-connector/fe-connector-metastore-iceberg/src/main/java/org/apache/doris/connector/metastore/iceberg/jdbc/IcebergJdbcMetaStoreProperties.java:
##########
@@ -141,5 +142,10 @@ public void validate() {
throw new IllegalArgumentException("Property
iceberg.jdbc.catalog_name is required.");
}
requireWarehouse();
+ // Mandatory, non-configurable security rule for the jar this flavor
loads into the FE JVM,
+ // shared with the jdbc / paimon-jdbc catalogs. validate() is reached
only from the CREATE /
+ // ALTER statement paths (checkCreateTimeOnlyRules -> bindForType),
never from a catalog
+ // rebuild, which is what keeps pre-rule catalogs loadable after an FE
restart.
+ JdbcDriverUrlSecurity.check(driverUrl);
Review Comment:
Load-bearing placement: `validate()` is reachable only through
`IcebergCatalogProperties.checkCreateTimeOnlyRules` (`bindForType(flavor,
...).validate()`), which only the CREATE/ALTER statement paths call - verified
by auditing every `bindForType`/`of()` call site in the tree; the connector
build, scan and factory paths bind without validating. That is what makes this
one line cover both DDL statements while a pre-rule catalog still rebuilds
after an FE restart. The flavor gating is inherited: `bindForType` selects this
holder only for `iceberg.catalog.type=jdbc` (lowercased at bind), so a stray
driver_url on a REST/HMS catalog is never checked.
##########
fe/fe-foundation/src/main/java/org/apache/doris/foundation/security/JdbcDriverUrlSecurity.java:
##########
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.doris.foundation.security;
+
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.regex.Pattern;
+
+/**
+ * The mandatory, non-configurable {@code driver_url} security rule, shared by
every connector that
+ * loads a JDBC driver jar into the FE JVM.
+ *
+ * <p>Three catalog types reach the same {@code URLClassLoader} + {@code
Class.forName(name, true, loader)}
+ * sink from a user-supplied catalog property, so they must share one rule
rather than each re-deriving it:
+ * the {@code jdbc} catalog ({@code driver_url}), the Iceberg JDBC catalog
+ * ({@code iceberg.jdbc.driver_url}) and the Paimon JDBC catalog
+ * ({@code paimon.jdbc.driver_url} / {@code jdbc.driver_url}). This class is
that single source of truth;
+ * it lives in fe-foundation because that is the one module every properties
holder already depends on.
+ *
+ * <p>The rule cannot be turned off:
+ * <ul>
+ * <li>any {@code ..} path-traversal segment is rejected, for {@code
file://} and {@code http(s)} alike,
+ * checked on the percent-decoded path so {@code %2e%2e} cannot slip
past;</li>
+ * <li>a scheme-less driver_url must be a bare jar file name matching {@code
[A-Za-z0-9._-]+.jar}
+ * (no directories, no special characters), which is then resolved under
the connector's drivers
+ * directory.</li>
+ * </ul>
+ * Whether a remote/absolute URL is allowed <em>at all</em> remains governed
by the fe.conf-only
+ * {@code jdbc_driver_secure_path} / {@code jdbc_driver_url_white_list}
configs, which the engine applies
+ * separately; this rule only forbids traversal and enforces the bare-name
charset.
+ *
+ * <p><b>Where callers invoke it: statement-time validation only.</b> The call
sites are the property
+ * holders' create-time-only hooks ({@code
JdbcCatalogProperties.checkCreateTimeOnlyRules} and the
+ * iceberg/paimon JDBC metastore holders' {@code validate()}), which the
engine reaches from the
+ * user-facing CREATE and ALTER CATALOG paths and never from edit-log replay
or a catalog rebuild.
+ * That placement is load-bearing: a catalog created before this rule existed
must keep coming back
+ * after an FE restart, so the rule must never run from a holder's {@code
of()}.
+ *
+ * <p>Throws {@link IllegalArgumentException} so the engine wraps it into a
{@code DdlException}
+ * (and, on ALTER, triggers the property rollback).
+ */
+public final class JdbcDriverUrlSecurity {
+
+ // A scheme-less driver_url must be a plain jar file name: letters,
digits, dot, underscore, hyphen.
+ // This intentionally forbids any path separator, so it can never escape
the drivers directory.
+ private static final Pattern SAFE_DRIVER_FILE_NAME =
Pattern.compile("^[A-Za-z0-9._-]+\\.jar$");
+
+ private JdbcDriverUrlSecurity() {
+ }
+
+ /**
+ * Applies the rule to a raw, alias-resolved {@code driver_url}. A
null/empty value means "use the
+ * engine-provided driver" and is accepted; every other value must satisfy
the rule above.
+ */
+ public static void check(String driverUrl) {
Review Comment:
Byte-for-byte the rule moved out of
`JdbcDorisConnector.checkDriverUrlSecurityRule` (diffed line by line, including
the `%2e%2e` decode and the backslash normalization) - no semantic change hides
in the move. It lives in fe-foundation because that is the only module all
three property holders already depend on (`fe-connector-jdbc` and the two
metastore modules do not share any fe-connector module), so no pom changes and
no plugin-API surface change were needed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
