This is an automated email from the ASF dual-hosted git repository.
yiguolei pushed a commit to branch branch-4.1
in repository https://gitbox.apache.org/repos/asf/doris.git
The following commit(s) were added to refs/heads/branch-4.1 by this push:
new fe4248db8ba branch-4.1: [feature](fe) Gate HMS catalog SSRF endpoints
at CREATE / ALTER CATALOG #69588 (#70092)
fe4248db8ba is described below
commit fe4248db8ba6804b331ac6f935b87aed0913632b
Author: Mingyu Chen (Rayner) <[email protected]>
AuthorDate: Wed Nov 4 00:32:24 2026 +0000
branch-4.1: [feature](fe) Gate HMS catalog SSRF endpoints at CREATE / ALTER
CATALOG #69588 (#70092)
### What problem does this PR solve?
Issue Number: None
Related PR: #69588 (Same backport for branch-4.2: #70091)
Problem Summary:
Backport of #69588 to branch-4.1. The master change cannot be
cherry-picked: it is built on the fe-connector plugin SPI
(`ConnectorProvider`, `ConnectorSecuritySurface`,
`PluginDrivenExternalCatalog`), and none of those exist on 4.1. This PR
ports the security fix itself onto the 4.1 catalog classes, with the
same rules and the same error messages.
**In short**
With the SSRF security SDK configured (`security_checker_class_name`),
CREATE / ALTER CATALOG on 4.1 never checked an HMS catalog's metastore
endpoint, so a user with CREATE_PRIV on a catalog (no ADMIN) could make
the FE's HMS client connect to an intranet address. `HMSExternalCatalog`
now runs the same CREATE / ALTER gate master got in #69588 (on ALTER,
before `CatalogMgr` takes its write lock), and the SDK binding no longer
leaks its net-hook onto pooled FE threads. Without a checker configured
(the open-source default), the new gate does nothing.
**Background**
- `security_checker_class_name` names a third-party SDK (Aliyun
`com.aliyun.securitysdk.SecurityUtil`), loaded by reflection through
`SecurityChecker` / `UrlSecurityChecker` in fe-common. Its net-hook sits
at the JDK socket layer: install it on the current thread, connect, and
the connect throws `SSRFUnsafeConnectionException` (a
`SecurityException`) if the resolved address is intranet. The SDK has no
"is this host intranet" string API, so it can only judge a real connect.
Today only HTTP callers use it (HTTP TVF, S3, ES, small-file download,
stage creation).
- An HMS catalog takes its endpoint from `hive.metastore.uris` or its
alias `uri`. `HMSBaseProperties` sets that value, plus every other
`hive.`-prefixed catalog property, into the `HiveConf` of the FE's HMS
client. `HiveMetaStoreClient` reads the endpoint through
`MetastoreConf.getVar`, which looks at `metastore.thrift.uris` first,
and through Hadoop's `Configuration.get`, which expands `${...}`
placeholders (other properties, `${env.X}`, JVM system properties).
**1. The problem, and what it cost**
- CREATE CATALOG (CREATE_PRIV) and ALTER CATALOG (ALTER privilege on the
catalog) accepted any metastore endpoint, so the FE opened a thrift
connection to a host inside its own network on behalf of a non-admin
user.
- `UrlSecurityChecker` bound the SDK's stop method as
`stopSSRFNetHookChecking(String)` but invoked it with no argument, so
stop never ran and the net-hook stayed on the pooled FE thread after
every bracketed HTTP request.
- Only the http form of the SDK's start was bound, so nothing could hook
a non-http connection such as HMS thrift.
**2. How this PR fixes it**
- fe-common, `SecurityChecker`: adds the no-arg `startSSRFChecking()`
(the SDK's TCP form), `isEnabled()`, and `checkEndpointForSsrf(host,
port)`, which brackets a single 5-second TCP connect with the net-hook.
A `SecurityException` from the connect is the rejection; a refused /
timed-out / unknown-host connect is ignored, so a public endpoint that
is down right now can still be used. `UrlSecurityChecker` binds the
SDK's no-arg start and no-arg stop; `DummySecurityChecker.isEnabled()`
returns false. The singleton becomes lazily initialized with a test
setter.
- fe-core, `HMSExternalCatalog`: `checkWhenCreating()` (CREATE, before
`test_connection`) and the new
`validatePropertiesBeforeUpdateUnlocked()` (ALTER) call the new gate on
`hive.metastore.uris`, `uri` and `metastore.thrift.uris`. The rules
match master: no-op without a checker; a value containing `${` is
rejected before anything else (so an ALTER of only the property a stored
placeholder refers to is rejected as well); an ALTER probes only when it
touches one of these keys; every entry of a comma-separated HA list is
probed; schemed values are parsed with `java.net.URI` host/port
semantics, so a query, fragment or userinfo part cannot hide the real
destination.
- fe-core, `CatalogMgr.alterCatalogProps`: on 4.1 ALTER CATALOG is
validated under `CatalogMgr`'s write lock, and a probe to an unreachable
endpoint can wait up to 5 seconds, so the probe must not run there.
ALTER now takes a snapshot of the catalog's properties under the read
lock, runs `ExternalCatalog.validatePropertiesBeforeUpdateUnlocked`
(new, a no-op by default) on it without holding the lock, and then
applies the change under the write lock only if the same catalog still
has the snapshot's properties; if another DDL changed or dropped the
catalog in between, it starts over from a new snapshot. The rest of the
ALTER validation still runs under the lock, unchanged. CREATE already
probes before `CatalogMgr` takes its lock.
Where the backport differs from master:
| Master (#69588) | This PR | Why |
| --- | --- | --- |
| Endpoint keys declared by the hive connector through
`ConnectorSecuritySurface` | Keys are a constant in `HMSExternalCatalog`
| 4.1 has no connector SPI; `HMSExternalCatalog` is already the
HMS-specific class |
| Gate in `PluginDrivenExternalCatalog` | Gate in `HMSExternalCatalog`,
through `checkWhenCreating` (CREATE) and the new
`validatePropertiesBeforeUpdateUnlocked` (ALTER) | Neither runs on
replay: `CatalogFactory` calls `checkWhenCreating` only for a new
catalog, and replay goes through `replayAlterCatalogProps`, not
`alterCatalogProps` |
| `driver_url` gate moved onto the security surface, `driverUrlKeysOf`
deleted | Not ported | 4.1 checks `driver_url` in `JdbcExternalCatalog`
/ `IcebergExternalCatalog` / `PaimonExternalCatalog.checkProperties`;
there is no key table to replace |
| ALTER rejected while the connector provider is missing | Not ported |
4.1 has no connector providers |
| `checkEndpointForSsrf` throws `DdlException` | Throws `Exception` with
the same messages; the HMS gate rethrows it as `DdlException` | On 4.1
`DdlException` is in fe-core, which fe-common cannot reference |
| Connector plugin API 22.0 -> 23.0 | No version change | No connector
plugin API on 4.1 |
| All ALTER validation runs before `CatalogMgr` takes its write lock
(#68585) | Only the lock-free checks (the probe) run before the write
lock, through `validatePropertiesBeforeUpdateUnlocked`, on a snapshot
that is checked again under the lock; the rest of ALTER validation stays
under the lock | #68585 is not on 4.1 and also changes HDFS HA
validation; this keeps the change to the probe |
| `metastore.thrift.uris` gated because the hive connector passes every
property to `HiveConf` | Gated as well | `HMSBaseProperties` copies only
`hive.`-prefixed properties today, so the key does not reach the client
yet; gating it keeps the gate correct if that copy widens, and keeps one
rule on both branches |
**3. Results**
With an SSRF checker configured:
| Statement | Before | After |
| --- | --- | --- |
| `CREATE CATALOG ... ("type"="hms",
"hive.metastore.uris"="thrift://10.0.0.1:9083")` | Accepted | Rejected:
`Endpoint 10.0.0.1:9083 is rejected by the SSRF policy (intranet address
not allowed): ...` |
| Same, with `test_connection` = false | Accepted | Rejected (the gate
does not depend on `test_connection`) |
| `ALTER CATALOG c SET PROPERTIES
("hive.metastore.uris"="thrift://10.0.0.1:9083")` | Accepted | Rejected
|
| `thrift://[email protected]:9083?x#y`, or an intranet host second in an HA
list | Accepted | Rejected |
| `"hive.metastore.uris"="thrift://${hive.x}:9083"` with
`"hive.x"="127.0.0.1"` | Accepted; the client connects to 127.0.0.1 |
Rejected: `... must give its endpoint literally while an SSRF checker is
configured ...` |
| Public endpoint that is currently unreachable | Accepted | Accepted (a
failed connect is not an SSRF signal) |
| ALTER of an unrelated property | Accepted | Accepted, no probe |
| ALTER that probes an unreachable public endpoint | No probe | Probes
for up to 5 seconds without holding `CatalogMgr`'s lock |
| Replay of CREATE / ALTER from the edit log | No probe | No probe |
Without a checker configured, nothing changes. The only change outside
the gate is that the SDK's net-hook is now removed after each bracketed
HTTP request, as intended.
**4. The classes, and how they call each other**
- `SecurityChecker` (fe-common): the SSRF primitive
`checkEndpointForSsrf`, plus `isEnabled()` and the TCP start.
- `UrlSecurityChecker` / `DummySecurityChecker` (fe-common): SDK binding
/ no-op.
- `HMSExternalCatalog` (fe-core): the endpoint keys,
`checkEndpointsAgainstSsrf`, `parseEndpoint`, and the two hooks.
- `ExternalCatalog` (fe-core): the new
`validatePropertiesBeforeUpdateUnlocked` hook, a no-op by default.
- `CatalogMgr` (fe-core): `alterCatalogProps` runs that hook on a
snapshot before taking its write lock and checks the snapshot again
under it.
```
CREATE CATALOG
CatalogFactory.createFromCommand
└ HMSExternalCatalog.checkWhenCreating
├ checkEndpointsAgainstSsrf (new) ->
SecurityChecker.checkEndpointForSsrf
└ super.checkWhenCreating (test_connection)
ALTER CATALOG ... SET PROPERTIES
CatalogMgr.alterCatalogProps
├ read lock: snapshot the catalog's properties
├ no lock: HMSExternalCatalog.validatePropertiesBeforeUpdateUnlocked
(new)
│ └ checkEndpointsAgainstSsrf (new) ->
SecurityChecker.checkEndpointForSsrf
└ write lock: same catalog, same properties? if not, start over
└ alterExternalCatalogPropsFenced ->
validatePropertiesBeforeUpdate
-> checkProperties (unchanged), publish, journal
SecurityChecker.checkEndpointForSsrf(host, port)
├ startSSRFChecking() -> SDK startSSRFNetHookChecking()
├ doConnect(host, port, 5s) -> SDK throws SecurityException for an
intranet target
└ stopSSRFChecking() -> SDK stopSSRFNetHookChecking()
Replay: CatalogFactory.createFromLog and
replayAlterCatalogProps(isReplay=true) call neither hook.
```
SSRF surface note (per repo policy): the probe is a new FE ->
user-named-host outbound TCP connect, made only when the operator
installed the SSRF SDK and only at CREATE / ALTER CATALOG (CREATE_PRIV
or ADMIN for CREATE, ALTER privilege for ALTER). It exists only so the
SDK can reject an intranet target. Known limits, same as master: it is a
point-in-time check, so it does not guard a later runtime connection
(DNS rebinding) or a client that bypasses the JDK socket layer.
### Release note
When an SSRF checker is configured, CREATE / ALTER CATALOG checks HMS
endpoint destinations (`hive.metastore.uris`, `uri`, and
`metastore.thrift.uris`), including URIs containing query, fragment, or
userinfo components, and rejects an HMS endpoint value that contains a
`${...}` placeholder.
### Check List (For Author)
- Test <!-- At least one of them must be included. -->
- [ ] Regression test
- [x] Unit Test
- [ ] Manual test (add detailed scripts or steps below)
- [ ] No need to test or manual test. Explain why:
- [ ] This is a refactor/code format and no logic has been changed.
- [ ] Previous test can cover this change.
- [ ] No code files have been changed.
- [ ] Other reason <!-- Add your reason? -->
- Unit Test: the new `SecurityCheckerTest` (6 tests) and
`HMSExternalCatalogSsrfGateTest` (48), plus the existing
`HMSExternalCatalogParquetTimeZoneTest`, `HttpTableValuedFunctionTest`,
`CatalogMgrTest`, `CatalogMgrLanceGuardTest`, `ExternalCatalogTest`,
`ExternalCatalogRuntimeStateTest`, `CatalogVarbinaryMigrationTest`,
`HMSPropertiesTest`, `HiveTableSinkTest`, `JdbcExternalCatalogTest` and
`PaimonExternalCatalogDriverUrlSecurityTest`: 134 tests in 13 classes
passed with `TZ=Asia/Shanghai mvn test -pl fe-common,fe-core -am`;
Checkstyle reports 0 violations in fe-common and fe-core.
- `HMSExternalCatalogSsrfGateTest` also drives ALTER through a real
`CatalogMgr`. It checks that the probe runs while the thread holds
neither the read nor the write lock, that an intranet target leaves the
catalog and the journal untouched, and that an ALTER whose catalog is
altered or dropped during the probe checks again, or fails with `No
catalog found`, instead of applying a stale check.
- Mutation check: each of 21 mutants is killed by at least one new test.
The mutants drop either hook call, the `isEnabled()` guard of the gate
or of `checkEndpointForSsrf`, the touched-key guard, a declared key, the
comma split or the URI parsing; run the placeholder rule only on touched
keys or ahead of the `isEnabled()` guard; swallow the SDK's rejection or
a failed hook install; treat a failed connect as a rejection; skip
`stopSSRFChecking()` in `finally`; probe under `CatalogMgr`'s write lock
(from `validatePropertiesBeforeUpdate`, or by calling the lock-free hook
under the lock); or drop the snapshot comparison or the catalog identity
check under the lock.
- Checked on this branch that `HMSBaseProperties` hands
`hive.metastore.uris` = `thrift://${hive.x}:9083` with `hive.x` =
`127.0.0.1` to the HMS client as `thrift://127.0.0.1:9083`, and does not
hand it a `metastore.thrift.uris` catalog property.
- Behavior changed:
- [ ] No.
- [x] Yes. With an SSRF checker configured, CREATE / ALTER CATALOG
probes HMS metastore endpoints and rejects intranet targets and `${...}`
placeholders; the SDK net-hook is now removed after each bracketed
request. Without a checker, nothing changes.
- Does this need documentation?
- [x] No.
- [ ] Yes. <!-- Add document PR link here. eg:
https://github.com/apache/doris-website/pull/1214 -->
### Check List (For Reviewer who merge this PR)
- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
.../doris/cloud/security/DummySecurityChecker.java | 10 +
.../doris/cloud/security/SecurityChecker.java | 105 +++-
.../doris/cloud/security/UrlSecurityChecker.java | 21 +-
.../doris/cloud/security/SecurityCheckerTest.java | 144 +++++
.../org/apache/doris/datasource/CatalogMgr.java | 91 +--
.../apache/doris/datasource/ExternalCatalog.java | 11 +
.../doris/datasource/hive/HMSExternalCatalog.java | 131 +++++
.../hive/HMSExternalCatalogSsrfGateTest.java | 611 +++++++++++++++++++++
8 files changed, 1085 insertions(+), 39 deletions(-)
diff --git
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/DummySecurityChecker.java
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/DummySecurityChecker.java
index c270eb75e3b..0c2cc6ecf05 100644
---
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/DummySecurityChecker.java
+++
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/DummySecurityChecker.java
@@ -30,8 +30,18 @@ public class DummySecurityChecker extends SecurityChecker {
return;
}
+ @Override
+ public void startSSRFChecking() throws Exception {
+ return;
+ }
+
@Override
public void stopSSRFChecking() {
return;
}
+
+ @Override
+ public boolean isEnabled() {
+ return false;
+ }
}
diff --git
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/SecurityChecker.java
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/SecurityChecker.java
index 10b0e87c888..f7e9bba35e8 100644
---
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/SecurityChecker.java
+++
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/SecurityChecker.java
@@ -19,21 +19,45 @@ package org.apache.doris.cloud.security;
import org.apache.doris.common.Config;
+import com.google.common.annotations.VisibleForTesting;
import com.google.common.base.Strings;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
+
+import java.io.IOException;
+import java.net.InetSocketAddress;
+import java.net.Socket;
/**
* SecurityChecker is used to check if the url is safe
*/
public abstract class SecurityChecker {
- private static class SingletonHolder {
- private static final SecurityChecker INSTANCE =
Strings.isNullOrEmpty(Config.security_checker_class_name)
- ? new DummySecurityChecker() : new UrlSecurityChecker();
- }
+ private static final Logger LOG =
LogManager.getLogger(SecurityChecker.class);
+
+ // Upper bound for one SSRF connectivity probe. The probe only needs the
net-hook to observe the
+ // resolved IP before the TCP handshake completes; a short timeout keeps
CREATE/ALTER responsive when
+ // the endpoint is a public-but-unreachable host (that outcome is ignored,
see checkEndpointForSsrf).
+ private static final int SSRF_PROBE_CONNECT_TIMEOUT_MS = 5000;
+
+ private static volatile SecurityChecker instance;
protected SecurityChecker() {}
public static SecurityChecker getInstance() {
- return SingletonHolder.INSTANCE;
+ if (instance == null) {
+ synchronized (SecurityChecker.class) {
+ if (instance == null) {
+ instance =
Strings.isNullOrEmpty(Config.security_checker_class_name)
+ ? new DummySecurityChecker() : new
UrlSecurityChecker();
+ }
+ }
+ }
+ return instance;
+ }
+
+ @VisibleForTesting
+ public static void setInstanceForTest(SecurityChecker checker) {
+ instance = checker;
}
/**
@@ -45,12 +69,79 @@ public abstract class SecurityChecker {
public abstract String getSafeJdbcUrl(String originJdbcUrl) throws
Exception;
/**
- * Check if the uri is safe, avoid SSRF attack
- * Only handle http:// https://
+ * Install the net-hook for an http(s) request on the current thread,
avoid SSRF attack.
+ * Only handle http:// https://. Must bracket a real request:
startSSRFChecking before issuing it,
+ * {@link #stopSSRFChecking()} after it, and no further request may be
issued once stopped.
* @param originUri
* @throws Exception
*/
public abstract void startSSRFChecking(String originUri) throws Exception;
+ /**
+ * Install the net-hook for a raw TCP connection on the current thread,
avoid SSRF attack.
+ * Handles any protocol that connects through a JDK socket (thrift, jdbc,
...), not only http.
+ * Same bracketing contract as {@link #startSSRFChecking(String)}.
+ * @throws Exception
+ */
+ public abstract void startSSRFChecking() throws Exception;
+
public abstract void stopSSRFChecking();
+
+ /**
+ * Whether a real SSRF checker (the third-party SDK) is installed. When
false every
+ * {@link #checkEndpointForSsrf(String, int)} is a no-op, so the default
open-source build behaves
+ * exactly as before.
+ */
+ public abstract boolean isEnabled();
+
+ /**
+ * Point-in-time SSRF gate for a single user-supplied endpoint: brackets a
lightweight TCP connect
+ * with the no-arg net-hook so the SDK resolves the host and rejects an
intranet target before the
+ * handshake completes.
+ *
+ * <p>The SDK signals an intranet target by throwing {@code
SSRFUnsafeConnectionException} (a
+ * {@link SecurityException}) from inside the connect; that — and only
that — is the rejection. A plain
+ * connectivity failure (refused / timeout / unknown host) is NOT an SSRF
signal and is ignored, so a
+ * catalog that points at a currently-unreachable but public endpoint can
still be created.
+ *
+ * @throws Exception with a user-facing message when the SDK rejects the
endpoint or its net-hook cannot
+ * be installed; callers rethrow it as their own DDL error
+ */
+ public final void checkEndpointForSsrf(String host, int port) throws
Exception {
+ if (!isEnabled()) {
+ return;
+ }
+ try {
+ startSSRFChecking();
+ } catch (Exception e) {
+ // isEnabled() is true but the SDK net-hook could not be
installed: surface the misconfiguration
+ // rather than silently skipping the security gate.
+ throw new Exception("Failed to initialize SSRF checking for
endpoint " + host + ":" + port
+ + ": " + e.getMessage(), e);
+ }
+ try {
+ doConnect(host, port, SSRF_PROBE_CONNECT_TIMEOUT_MS);
+ } catch (SecurityException e) {
+ throw new Exception("Endpoint " + host + ":" + port
+ + " is rejected by the SSRF policy (intranet address not
allowed): " + e.getMessage());
+ } catch (IOException e) {
+ if (LOG.isDebugEnabled()) {
+ LOG.debug("SSRF probe to {}:{} did not complete, ignored (not
an SSRF rejection): {}",
+ host, port, e.toString());
+ }
+ } finally {
+ stopSSRFChecking();
+ }
+ }
+
+ /**
+ * The connection seam of {@link #checkEndpointForSsrf(String, int)}: a
single real TCP connect the
+ * net-hook can observe. Overridable so unit tests can drive the SSRF /
unreachable / success branches
+ * without the third-party SDK or a live server.
+ */
+ protected void doConnect(String host, int port, int connectTimeoutMs)
throws IOException {
+ try (Socket socket = new Socket()) {
+ socket.connect(new InetSocketAddress(host, port),
connectTimeoutMs);
+ }
+ }
}
diff --git
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/UrlSecurityChecker.java
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/UrlSecurityChecker.java
index 5f79343d0f9..8a858ad955e 100644
---
a/fe/fe-common/src/main/java/org/apache/doris/cloud/security/UrlSecurityChecker.java
+++
b/fe/fe-common/src/main/java/org/apache/doris/cloud/security/UrlSecurityChecker.java
@@ -29,6 +29,7 @@ public class UrlSecurityChecker extends SecurityChecker {
private static final Logger LOG =
LogManager.getLogger(UrlSecurityChecker.class);
private static Method jdbcUrlCheckMethod = null;
private static Method urlSecurityCheckMethod = null;
+ private static Method tcpSecurityCheckMethod = null;
private static Method urlSecurityStopCheckMethod = null;
static {
@@ -36,7 +37,11 @@ public class UrlSecurityChecker extends SecurityChecker {
Class clazz = Class.forName(Config.security_checker_class_name);
jdbcUrlCheckMethod = clazz.getMethod("filterJdbcConnectionSource",
String.class);
urlSecurityCheckMethod =
clazz.getMethod("startSSRFNetHookChecking", String.class);
- urlSecurityStopCheckMethod =
clazz.getMethod("stopSSRFNetHookChecking", String.class);
+ // No-arg overload: hooks a raw TCP connection (thrift / jdbc /
...), not only http.
+ tcpSecurityCheckMethod =
clazz.getMethod("startSSRFNetHookChecking");
+ // The SDK's stop is no-arg. Binding it with String.class (then
invoking with no argument)
+ // meant stop never fired and the net-hook leaked onto the pooled
thread.
+ urlSecurityStopCheckMethod =
clazz.getMethod("stopSSRFNetHookChecking");
} catch (Exception e) {
LOG.warn("security_checker_class_name:{} exception:",
Config.security_checker_class_name, e);
LogUtils.stderr("Failed to find
com.aliyun.securitysdk.SecurityUtil's method");
@@ -63,6 +68,20 @@ public class UrlSecurityChecker extends SecurityChecker {
throw new Exception("SecurityUtil.startSSRFNetHookChecking not found");
}
+ @Override
+ public void startSSRFChecking() throws Exception {
+ if (tcpSecurityCheckMethod != null) {
+ tcpSecurityCheckMethod.invoke(null);
+ return;
+ }
+ throw new Exception("SecurityUtil.startSSRFNetHookChecking (no-arg)
not found");
+ }
+
+ @Override
+ public boolean isEnabled() {
+ return true;
+ }
+
@Override
public void stopSSRFChecking() {
if (urlSecurityStopCheckMethod != null) {
diff --git
a/fe/fe-common/src/test/java/org/apache/doris/cloud/security/SecurityCheckerTest.java
b/fe/fe-common/src/test/java/org/apache/doris/cloud/security/SecurityCheckerTest.java
new file mode 100644
index 00000000000..79c0325406e
--- /dev/null
+++
b/fe/fe-common/src/test/java/org/apache/doris/cloud/security/SecurityCheckerTest.java
@@ -0,0 +1,144 @@
+// 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.cloud.security;
+
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.Test;
+
+import java.io.IOException;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
+/**
+ * {@link SecurityChecker#checkEndpointForSsrf} is the single primitive the
catalog SSRF gate delegates to.
+ * The real third-party SDK is operator-supplied and absent in CI, so these
tests drive its branches through
+ * the {@code doConnect} seam: the SDK signals an intranet target by throwing
{@link SecurityException} from
+ * inside the connect, a plain connectivity failure is {@link IOException},
and when no checker is installed
+ * the whole probe is a no-op.
+ */
+public class SecurityCheckerTest {
+
+ private enum ConnectBehavior { SUCCESS, SSRF, IO }
+
+ /** A SecurityChecker whose net-hook bracket and connect are recorded, not
real. */
+ private static final class RecordingChecker extends SecurityChecker {
+ private boolean enabled = true;
+ private boolean startThrows = false;
+ private ConnectBehavior connectBehavior = ConnectBehavior.SUCCESS;
+ private final List<String> events = new ArrayList<>();
+
+ @Override
+ public String getSafeJdbcUrl(String originJdbcUrl) {
+ return originJdbcUrl;
+ }
+
+ @Override
+ public void startSSRFChecking(String originUri) throws Exception {
+ start();
+ }
+
+ @Override
+ public void startSSRFChecking() throws Exception {
+ start();
+ }
+
+ private void start() throws Exception {
+ events.add("start");
+ if (startThrows) {
+ throw new Exception("cannot install net-hook");
+ }
+ }
+
+ @Override
+ public void stopSSRFChecking() {
+ events.add("stop");
+ }
+
+ @Override
+ public boolean isEnabled() {
+ return enabled;
+ }
+
+ @Override
+ protected void doConnect(String host, int port, int connectTimeoutMs)
throws IOException {
+ events.add("connect");
+ switch (connectBehavior) {
+ case SSRF:
+ throw new SecurityException("simulated
SSRFUnsafeConnectionException for " + host);
+ case IO:
+ throw new IOException("connection refused");
+ default:
+ break;
+ }
+ }
+ }
+
+ @Test
+ public void disabledCheckerSkipsProbeEntirely() throws Exception {
+ RecordingChecker checker = new RecordingChecker();
+ checker.enabled = false;
+ // Intranet-looking host, but a disabled checker must not even connect.
+ checker.checkEndpointForSsrf("10.0.0.1", 9083);
+ Assertions.assertTrue(checker.events.isEmpty(), "disabled checker must
not touch the network");
+ }
+
+ @Test
+ public void successfulProbeBracketsConnectAndReturns() throws Exception {
+ RecordingChecker checker = new RecordingChecker();
+ checker.connectBehavior = ConnectBehavior.SUCCESS;
+ checker.checkEndpointForSsrf("public.example.com", 9083);
+ Assertions.assertEquals(Arrays.asList("start", "connect", "stop"),
checker.events);
+ }
+
+ @Test
+ public void intranetTargetIsRejectedWithPolicyMessage() {
+ RecordingChecker checker = new RecordingChecker();
+ checker.connectBehavior = ConnectBehavior.SSRF;
+ Exception e = Assertions.assertThrows(Exception.class,
+ () -> checker.checkEndpointForSsrf("10.0.0.1", 9083));
+ Assertions.assertTrue(e.getMessage().contains("SSRF policy"),
e.getMessage());
+ // stop still runs in the finally block even when the connect throws.
+ Assertions.assertEquals(Arrays.asList("start", "connect", "stop"),
checker.events);
+ }
+
+ @Test
+ public void unreachableEndpointIsNotAnSsrfRejection() throws Exception {
+ RecordingChecker checker = new RecordingChecker();
+ checker.connectBehavior = ConnectBehavior.IO;
+ // A refused/timed-out connect is a reachability fact, not an SSRF
signal: must not throw.
+ checker.checkEndpointForSsrf("public-but-down.example.com", 9083);
+ Assertions.assertEquals(Arrays.asList("start", "connect", "stop"),
checker.events);
+ }
+
+ @Test
+ public void failedHookInstallSurfacesAsErrorWithoutConnecting() {
+ RecordingChecker checker = new RecordingChecker();
+ checker.startThrows = true;
+ Exception e = Assertions.assertThrows(Exception.class,
+ () -> checker.checkEndpointForSsrf("some.host", 9083));
+ Assertions.assertTrue(e.getMessage().contains("Failed to initialize
SSRF checking"), e.getMessage());
+ // No connect attempt, and nothing to stop since the hook was never
installed.
+ Assertions.assertEquals(Arrays.asList("start"), checker.events);
+ }
+
+ @Test
+ public void dummyCheckerIsDisabled() {
+ Assertions.assertFalse(new DummySecurityChecker().isEnabled());
+ }
+}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
index bede3d4d6a0..4481d0f7259 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/CatalogMgr.java
@@ -441,40 +441,69 @@ public class CatalogMgr implements Writable,
GsonPostProcessable {
* Modify the catalog property and write the meta log.
*/
public void alterCatalogProps(String catalogName, Map<String, String>
newProperties) throws UserException {
- writeLock();
- try {
- CatalogIf catalog = nameToCatalog.get(catalogName);
- if (catalog == null) {
- throw new DdlException("No catalog found with name: " +
catalogName);
- }
- Map<String, String> oldProperties = catalog.getProperties();
- if (newProperties.containsKey("type") && !catalog.getType()
- .equalsIgnoreCase(newProperties.get("type"))) {
- throw new DdlException("Can't modify the type of catalog
property with name: " + catalogName);
- }
- CatalogLog log = new CatalogLog();
- log.setCatalogId(catalog.getId());
- Map<String, String> loggedProperties =
Maps.newHashMap(newProperties);
- if (catalog instanceof ExternalCatalog
- &&
loggedProperties.containsKey(CatalogProperty.ENABLE_MAPPING_VARBINARY)) {
- // Older FEs replay this map verbatim, so normalize the
durable record, not just
- // the live CatalogProperty. Do not mutate the caller's
possibly immutable map.
- loggedProperties.put(CatalogProperty.ENABLE_MAPPING_VARBINARY,
"true");
+ while (true) {
+ CatalogIf catalog;
+ Map<String, String> oldProperties;
+ readLock();
+ try {
+ catalog = nameToCatalog.get(catalogName);
+ if (catalog == null) {
+ throw new DdlException("No catalog found with name: " +
catalogName);
+ }
+ oldProperties = Maps.newHashMap(catalog.getProperties());
+ if (newProperties.containsKey("type") && !catalog.getType()
+ .equalsIgnoreCase(newProperties.get("type"))) {
+ throw new DdlException("Can't modify the type of catalog
property with name: " + catalogName);
+ }
+ } finally {
+ readUnlock();
}
- if (catalog instanceof ExternalCatalog
- &&
loggedProperties.containsKey(CatalogProperty.ENABLE_MAPPING_TIMESTAMP_TZ)) {
- // Replicate the effective type policy so older followers
infer the same schema.
-
loggedProperties.put(CatalogProperty.ENABLE_MAPPING_TIMESTAMP_TZ, "true");
+ if (catalog instanceof ExternalCatalog) {
+ // Checks that may block on the network (the SSRF endpoint
probe) must not hold the global catalog
+ // write lock: run them on the snapshot, and apply below only
if no other DDL changed it meanwhile.
+ ((ExternalCatalog)
catalog).validatePropertiesBeforeUpdateUnlocked(oldProperties, newProperties);
}
- log.setNewProps(loggedProperties);
- replayAlterCatalogProps(log, oldProperties, false);
- if (!(catalog instanceof ExternalCatalog)) {
- // External catalogs journal the committed change from the
fenced helper once publication
- // has happened; non-external catalogs only reach this point
on success.
-
Env.getCurrentEnv().getEditLog().logCatalogLog(OperationType.OP_ALTER_CATALOG_PROPS,
log);
+ writeLock();
+ try {
+ if (nameToCatalog.get(catalogName) != catalog
+ || !oldProperties.equals(catalog.getProperties())) {
+ // The catalog was replaced or altered after the snapshot:
check the current one again.
+ continue;
+ }
+ alterCatalogPropsLocked(catalog, newProperties, oldProperties);
+ return;
+ } finally {
+ writeUnlock();
}
- } finally {
- writeUnlock();
+ }
+ }
+
+ /**
+ * Journals and applies one ALTER CATALOG ... SET PROPERTIES. The caller
holds the write lock and has already
+ * run {@link ExternalCatalog#validatePropertiesBeforeUpdateUnlocked} on
these same old properties.
+ */
+ private void alterCatalogPropsLocked(CatalogIf catalog, Map<String,
String> newProperties,
+ Map<String, String> oldProperties) throws DdlException {
+ CatalogLog log = new CatalogLog();
+ log.setCatalogId(catalog.getId());
+ Map<String, String> loggedProperties = Maps.newHashMap(newProperties);
+ if (catalog instanceof ExternalCatalog
+ &&
loggedProperties.containsKey(CatalogProperty.ENABLE_MAPPING_VARBINARY)) {
+ // Older FEs replay this map verbatim, so normalize the durable
record, not just
+ // the live CatalogProperty. Do not mutate the caller's possibly
immutable map.
+ loggedProperties.put(CatalogProperty.ENABLE_MAPPING_VARBINARY,
"true");
+ }
+ if (catalog instanceof ExternalCatalog
+ &&
loggedProperties.containsKey(CatalogProperty.ENABLE_MAPPING_TIMESTAMP_TZ)) {
+ // Replicate the effective type policy so older followers infer
the same schema.
+ loggedProperties.put(CatalogProperty.ENABLE_MAPPING_TIMESTAMP_TZ,
"true");
+ }
+ log.setNewProps(loggedProperties);
+ replayAlterCatalogProps(log, oldProperties, false);
+ if (!(catalog instanceof ExternalCatalog)) {
+ // External catalogs journal the committed change from the fenced
helper once publication
+ // has happened; non-external catalogs only reach this point on
success.
+
Env.getCurrentEnv().getEditLog().logCatalogLog(OperationType.OP_ALTER_CATALOG_PROPS,
log);
}
}
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
index 68685cdfa99..1aed448a2c0 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/datasource/ExternalCatalog.java
@@ -513,6 +513,17 @@ public abstract class ExternalCatalog
return false;
}
+ /**
+ * Validate an ALTER candidate with checks that may block on the network,
such as the SSRF probe of a
+ * user-supplied endpoint. {@link CatalogMgr#alterCatalogProps} calls this
on a snapshot of the catalog's
+ * properties before it takes its write lock, and applies the ALTER only
if the properties still equal that
+ * snapshot once it holds the lock. Replay never calls it. Checks that do
not block belong in
+ * {@link #validatePropertiesBeforeUpdate}, which runs under the lock.
+ */
+ public void validatePropertiesBeforeUpdateUnlocked(
+ Map<String, String> currentProperties, Map<String, String>
updatedProperties) throws DdlException {
+ }
+
/**
* eg:
* (
diff --git
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
index 0537996859b..1264f7d29df 100644
---
a/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
+++
b/fe/fe-core/src/main/java/org/apache/doris/datasource/hive/HMSExternalCatalog.java
@@ -18,6 +18,7 @@
package org.apache.doris.datasource.hive;
import org.apache.doris.catalog.Env;
+import org.apache.doris.cloud.security.SecurityChecker;
import org.apache.doris.cluster.ClusterNamespace;
import org.apache.doris.common.DdlException;
import org.apache.doris.common.ThreadPoolManager;
@@ -40,6 +41,7 @@ import org.apache.doris.datasource.iceberg.IcebergUtils;
import org.apache.doris.datasource.metacache.CacheSpec;
import org.apache.doris.datasource.operations.ExternalMetadataOperations;
import org.apache.doris.datasource.property.metastore.AbstractHiveProperties;
+import org.apache.doris.datasource.property.metastore.HMSBaseProperties;
import org.apache.doris.datasource.property.metastore.MetastoreProperties;
import org.apache.doris.datasource.property.storage.StorageProperties;
import org.apache.doris.fs.FileSystemProvider;
@@ -48,6 +50,8 @@ import org.apache.doris.fs.remote.dfs.DFSFileSystem;
import org.apache.doris.transaction.TransactionManagerFactory;
import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableList;
+import com.google.common.net.HostAndPort;
import org.apache.commons.lang3.math.NumberUtils;
import org.apache.iceberg.Table;
import org.apache.iceberg.catalog.Catalog;
@@ -56,6 +60,8 @@ import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
@@ -83,6 +89,16 @@ public class HMSExternalCatalog extends ExternalCatalog {
// from remoteTable object.
public static final String GET_SCHEMA_FROM_TABLE = "get_schema_from_table";
+ /**
+ * Every catalog property the HMS client may take its thrift endpoint
from: hive.metastore.uris, its
+ * documented alias uri, and Hive 3's metastore.thrift.uris, which the
client's MetastoreConf reads before
+ * hive.metastore.uris. {@link HMSBaseProperties} copies only
hive.-prefixed catalog properties into the
+ * client's HiveConf, so the last key does not reach the client today; it
is gated anyway so the gate stays
+ * correct if that copy widens. The CREATE / ALTER CATALOG SSRF gate
probes every endpoint under these keys.
+ */
+ private static final List<String> SSRF_ENDPOINT_KEYS = ImmutableList.of(
+ HMSBaseProperties.HIVE_METASTORE_URIS, "uri",
"metastore.thrift.uris");
+
private static final int FILE_SYSTEM_EXECUTOR_THREAD_NUM = 16;
private ThreadPoolExecutor fileSystemExecutor;
@@ -167,6 +183,16 @@ public class HMSExternalCatalog extends ExternalCatalog {
checkProperties(catalogProperty);
}
+ @Override
+ public void checkWhenCreating() throws DdlException {
+ // Reject an intranet endpoint before any real connect in super
(test_connection), regardless of that
+ // flag (CREATE path; never reached on replay). On CREATE every
property is newly supplied, so the
+ // full map is both the candidate and the touched set.
+ Map<String, String> properties = catalogProperty.getProperties();
+ checkEndpointsAgainstSsrf(properties, properties);
+ super.checkWhenCreating();
+ }
+
@Override
public boolean validatePropertiesBeforeUpdate(
Map<String, String> currentProperties, Map<String, String>
updatedProperties) throws DdlException {
@@ -177,6 +203,111 @@ public class HMSExternalCatalog extends ExternalCatalog {
return true;
}
+ @Override
+ public void validatePropertiesBeforeUpdateUnlocked(
+ Map<String, String> currentProperties, Map<String, String>
updatedProperties) throws DdlException {
+ Map<String, String> candidateProperties = currentProperties == null
+ ? new HashMap<>() : new HashMap<>(currentProperties);
+ candidateProperties.putAll(updatedProperties);
+ // The probe connects to the endpoint, so it runs here, before
CatalogMgr takes its write lock, rather
+ // than in validatePropertiesBeforeUpdate.
+ checkEndpointsAgainstSsrf(candidateProperties, updatedProperties);
+ }
+
+ /**
+ * Applies the operator's SSRF policy (the third-party {@link
SecurityChecker} SDK) to every metastore
+ * endpoint these properties would make the FE connect to, rejecting an
intranet target. It runs on CREATE
+ * (through {@link #checkWhenCreating()}, before the catalog is
registered) and on the interactive ALTER path
+ * (through {@link #validatePropertiesBeforeUpdateUnlocked}, before
CatalogMgr takes its write lock) but never
+ * on replay, and it is a no-op unless the operator installed an SSRF
checker
+ * ({@code security_checker_class_name}).
+ *
+ * <p>The judgment itself is delegated to the SDK — see {@link
SecurityChecker#checkEndpointForSsrf}.
+ * This is a point-in-time check of the endpoint supplied at CREATE/ALTER;
it does not guard a later
+ * runtime connection (DNS rebinding, lazy metadata access) — that is a
deliberate scope limit.
+ *
+ * <p>An endpoint must be given literally. The HMS client reads it through
Hadoop's {@code Configuration},
+ * which expands a {@code ${...}} placeholder (another property, {@code
${env.X}}, a JVM system property)
+ * before it parses the endpoint, so the probe would judge a different
destination than the one the client
+ * connects to.
+ */
+ private void checkEndpointsAgainstSsrf(Map<String, String> candidate,
Map<String, String> updatedProperties)
+ throws DdlException {
+ SecurityChecker checker = SecurityChecker.getInstance();
+ if (!checker.isEnabled()) {
+ return;
+ }
+ // Checked before the touched guard below: a placeholder value was
never probed, and an ALTER that
+ // changes only the property it refers to repoints the catalog without
touching an endpoint key.
+ for (String key : SSRF_ENDPOINT_KEYS) {
+ String rawValue = candidate.get(key);
+ if (rawValue != null && rawValue.contains("${")) {
+ throw new DdlException("Property '" + key + "' of catalog '" +
name + "' must give its endpoint"
+ + " literally while an SSRF checker is configured: a
${...} placeholder is expanded"
+ + " after this check, so its destination cannot be
validated");
+ }
+ }
+ // Only an ALTER that touches an endpoint key can repoint the catalog
at a new host; a stored literal
+ // value was gated at its own CREATE/ALTER time. On CREATE the full
property map is passed as both
+ // arguments, so every supplied endpoint counts as touched and is
checked.
+ boolean touched =
SSRF_ENDPOINT_KEYS.stream().anyMatch(updatedProperties::containsKey);
+ if (!touched) {
+ return;
+ }
+ for (String key : SSRF_ENDPOINT_KEYS) {
+ String rawValue = candidate.get(key);
+ if (rawValue == null || rawValue.trim().isEmpty()) {
+ continue;
+ }
+ // A value may list several comma-separated endpoints (e.g. HA
hive.metastore.uris).
+ for (String endpoint : rawValue.split(",")) {
+ HostAndPort hostAndPort = parseEndpoint(endpoint.trim());
+ if (hostAndPort == null) {
+ continue;
+ }
+ try {
+ checker.checkEndpointForSsrf(hostAndPort.getHost(),
hostAndPort.getPort());
+ } catch (Exception e) {
+ throw new DdlException(e.getMessage(), e);
+ }
+ }
+ }
+ }
+
+ /**
+ * Parse one endpoint — {@code scheme://host:port} (e.g. {@code
thrift://host:9083}) or a bare
+ * {@code host:port} — into host and port for the SSRF probe. Returns null
when no concrete host:port
+ * can be extracted (nothing to probe); {@link HostAndPort} handles IPv6
literals in brackets natively.
+ */
+ static HostAndPort parseEndpoint(String value) {
+ if (value == null || value.isEmpty()) {
+ return null;
+ }
+ try {
+ if (value.contains("://")) {
+ // Match the HMS client's URI semantics: query, fragment and
userinfo do not name
+ // the socket destination. HostAndPort.fromParts also strips
IPv6 brackets.
+ URI uri = new URI(value);
+ if (uri.getHost() == null || uri.getPort() < 0) {
+ return null;
+ }
+ return HostAndPort.fromParts(uri.getHost(), uri.getPort());
+ }
+ String authority = value;
+ int slashIdx = authority.indexOf('/');
+ if (slashIdx >= 0) {
+ authority = authority.substring(0, slashIdx);
+ }
+ HostAndPort hostAndPort = HostAndPort.fromString(authority);
+ if (!hostAndPort.hasPort() || hostAndPort.getHost().isEmpty()) {
+ return null;
+ }
+ return hostAndPort;
+ } catch (URISyntaxException | IllegalArgumentException e) {
+ return null;
+ }
+ }
+
@Override
protected void checkProperties(CatalogProperty property) throws
DdlException {
super.checkProperties(property);
diff --git
a/fe/fe-core/src/test/java/org/apache/doris/datasource/hive/HMSExternalCatalogSsrfGateTest.java
b/fe/fe-core/src/test/java/org/apache/doris/datasource/hive/HMSExternalCatalogSsrfGateTest.java
new file mode 100644
index 00000000000..079c57ade4c
--- /dev/null
+++
b/fe/fe-core/src/test/java/org/apache/doris/datasource/hive/HMSExternalCatalogSsrfGateTest.java
@@ -0,0 +1,611 @@
+// 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.datasource.hive;
+
+import org.apache.doris.catalog.Env;
+import org.apache.doris.cloud.security.SecurityChecker;
+import org.apache.doris.common.DdlException;
+import org.apache.doris.datasource.CatalogFactory;
+import org.apache.doris.datasource.CatalogIf;
+import org.apache.doris.datasource.CatalogLog;
+import org.apache.doris.datasource.CatalogMgr;
+import org.apache.doris.datasource.ExternalMetaCacheMgr;
+import org.apache.doris.nereids.trees.plans.commands.CreateCatalogCommand;
+import org.apache.doris.persist.EditLog;
+import org.apache.doris.persist.OperationType;
+
+import com.google.common.net.HostAndPort;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
+import org.mockito.MockedStatic;
+import org.mockito.Mockito;
+
+import java.io.IOException;
+import java.lang.reflect.Field;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.concurrent.locks.ReentrantReadWriteLock;
+import java.util.function.Supplier;
+
+/**
+ * CREATE / ALTER CATALOG must apply the operator's SSRF policy to every
metastore endpoint an HMS catalog
+ * would make the FE connect to, rejecting an intranet target. The judgment is
delegated to the third-party
+ * {@code SecurityChecker} SDK (absent in CI, so a recording fake stands in);
the gate runs on CREATE
+ * ({@code checkWhenCreating}) and on ALTER ({@code
validatePropertiesBeforeUpdateUnlocked}, which CatalogMgr runs
+ * before it takes its write lock) but never on replay, and it is a no-op when
no checker is installed.
+ */
+public class HMSExternalCatalogSsrfGateTest {
+
+ private RecordingChecker checker;
+
+ @BeforeEach
+ public void setUp() {
+ checker = new RecordingChecker();
+ SecurityChecker.setInstanceForTest(checker);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ // Revert to the Config-driven singleton so a reused surefire fork is
not left with the fake.
+ SecurityChecker.setInstanceForTest(null);
+ }
+
+ // ---- static helper: endpoint parsing ----
+
+ @Test
+ public void parseEndpointExtractsHostAndPort() {
+ HostAndPort schemed =
HMSExternalCatalog.parseEndpoint("thrift://host1:9083");
+ Assertions.assertNotNull(schemed);
+ Assertions.assertEquals("host1", schemed.getHost());
+ Assertions.assertEquals(9083, schemed.getPort());
+
+ HostAndPort bare = HMSExternalCatalog.parseEndpoint("host2:9083");
+ Assertions.assertNotNull(bare);
+ Assertions.assertEquals("host2", bare.getHost());
+ Assertions.assertEquals(9083, bare.getPort());
+ }
+
+ @Test
+ public void parseEndpointReturnsNullWhenNoHostPort() {
+ Assertions.assertNull(HMSExternalCatalog.parseEndpoint(""));
+
Assertions.assertNull(HMSExternalCatalog.parseEndpoint("thrift://host-without-port"));
+ Assertions.assertNull(HMSExternalCatalog.parseEndpoint("thrift://"));
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"thrift://host1:9083?ignored",
"thrift://host1:9083#ignored",
+ "thrift://user@host1:9083",
"thrift://user:password@host1:9083/path?ignored#ignored",
+ "thrift://host1:9083/path", "host1:9083/path"})
+ public void parseEndpointUsesUriDestination(String endpoint) {
+ HostAndPort parsed = HMSExternalCatalog.parseEndpoint(endpoint);
+ Assertions.assertNotNull(parsed);
+ Assertions.assertEquals("host1", parsed.getHost());
+ Assertions.assertEquals(9083, parsed.getPort());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"thrift://[::1]:9083",
"thrift://user@[::1]:9083?ignored#ignored", "[::1]:9083"})
+ public void parseEndpointPreservesIpv6Destination(String endpoint) {
+ HostAndPort parsed = HMSExternalCatalog.parseEndpoint(endpoint);
+ Assertions.assertNotNull(parsed);
+ Assertions.assertEquals("::1", parsed.getHost());
+ Assertions.assertEquals(9083, parsed.getPort());
+ }
+
+ // ---- ALTER gate ----
+
+ @Test
+ public void alterRejectsRepointingMetastoreToIntranet() {
+ checker.intranetHost = "10.0.0.1";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+ // MUTATION: drop checkEndpointsAgainstSsrf from
validatePropertiesBeforeUpdateUnlocked -> the repoint to
+ // an intranet metastore is accepted -> red.
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(hms("thrift://hms.public.example.com:9083"),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://10.0.0.1:9083")));
+ Assertions.assertTrue(e.getMessage().contains("SSRF policy"),
e.getMessage());
+ }
+
+ @Test
+ public void alterAcceptsPublicMetastoreAndProbesIt() throws Exception {
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+
catalog.validatePropertiesBeforeUpdateUnlocked(hms("thrift://hms.public.example.com:9083"),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://hms2.public.example.com:9083"));
+
Assertions.assertTrue(checker.probed.contains("hms2.public.example.com:9083"),
checker.probed.toString());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"thrift://10.0.0.1:9083?ignored",
"thrift://10.0.0.1:9083#ignored",
+ "thrift://[email protected]:9083",
"thrift://user:[email protected]:9083/path?ignored#ignored"})
+ public void alterRejectsUriDestinationWithSuffixes(String endpoint) {
+ checker.intranetHost = "10.0.0.1";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+ Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
+ Collections.singletonMap("hive.metastore.uris",
endpoint)));
+ Assertions.assertEquals(Collections.singletonList("10.0.0.1:9083"),
checker.probed);
+ }
+
+ @Test
+ public void alterChecksEveryEndpointInAnHaUriList() {
+ checker.intranetHost = "192.168.0.7";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+ // Second URI in the HA list resolves to an intranet host and must
still be rejected.
+ Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(hms("thrift://hms.public.example.com:9083"),
+ Collections.singletonMap("hive.metastore.uris",
+
"thrift://ok.public.example.com:9083,thrift://192.168.0.7:9083?ignored#ignored")));
+ }
+
+ @Test
+ public void alterTouchingOnlyTheUriAliasIsProbed() {
+ // uri is the documented alias HMSBaseProperties binds to
hive.metastore.uris.
+ checker.intranetHost = "127.0.0.1";
+ Map<String, String> stored = new HashMap<>();
+ stored.put("type", "hms");
+ stored.put("uri", "thrift://hms.public.example.com:9083");
+ HMSExternalCatalog catalog = catalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
+ Collections.singletonMap("uri",
"thrift://127.0.0.1:9083")));
+ Assertions.assertEquals(Collections.singletonList("127.0.0.1:9083"),
checker.probed);
+ }
+
+ @Test
+ public void alterTouchingOnlyModernMetastoreKeyIsProbed() {
+ // The client's MetastoreConf reads metastore.thrift.uris before
hive.metastore.uris. The key is declared
+ // even though HMSBaseProperties does not hand it to the client today,
so this ALTER alone is probed.
+ checker.intranetHost = "127.0.0.1";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+ Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
+ Collections.singletonMap("metastore.thrift.uris",
"thrift://127.0.0.1:9083")));
+ // A touched endpoint key probes every declared endpoint of the
candidate, the stored legacy uri included.
+ Assertions.assertTrue(checker.probed.contains("127.0.0.1:9083"),
checker.probed.toString());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"hive.metastore.uris", "uri",
"metastore.thrift.uris"})
+ public void alterRejectsPlaceholderEndpointWithoutProbing(String key) {
+ // MUTATION: drop the placeholder check -> URI parsing fails, the
endpoint is skipped and the ALTER is
+ // accepted, although the client's HiveConf would expand
hive.metastore.uris to 127.0.0.1 -> red.
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+ Map<String, String> update = new HashMap<>();
+ update.put(key, "thrift://${hive.ssrf.target}:9083");
+ // hive.-prefixed, so HMSBaseProperties copies it into the client's
HiveConf, where it is expanded.
+ update.put("hive.ssrf.target", "127.0.0.1");
+
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
update));
+ Assertions.assertTrue(e.getMessage().contains("placeholder"),
e.getMessage());
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ @Test
+ public void alterOfThePropertyAStoredPlaceholderRefersToIsRejected() {
+ // A catalog stored before a checker was configured can carry a
placeholder; changing only the property
+ // it refers to repoints the client without touching an endpoint key.
+ // MUTATION: move the placeholder check after the touched guard -> the
ALTER is accepted -> red.
+ Map<String, String> stored = hms("thrift://${hive.ssrf.target}:9083");
+ stored.put("hive.ssrf.target", "hms.public.example.com");
+ HMSExternalCatalog catalog = catalog(stored);
+
+ Assertions.assertThrows(DdlException.class,
+ () ->
catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
+ Collections.singletonMap("hive.ssrf.target",
"127.0.0.1")));
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ @Test
+ public void alterAcceptsPlaceholderWhenNoCheckerInstalled() throws
Exception {
+ // MUTATION: run the placeholder check before the isEnabled() guard ->
rejected -> red.
+ checker.enabled = false;
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+ catalog.validatePropertiesBeforeUpdateUnlocked(catalog.getProperties(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://${hive.ssrf.target}:9083"));
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ @Test
+ public void alterNotTouchingMetastoreUriSkipsTheGate() throws Exception {
+ // A stored uri was gated at its own CREATE/ALTER time; an unrelated
ALTER must not re-probe it.
+ // MUTATION: drop the touched-keys guard -> the stored intranet uri is
probed and rejected -> red.
+ checker.intranetHost = "10.0.0.1";
+ HMSExternalCatalog catalog = catalog(hms("thrift://10.0.0.1:9083"));
+
+
catalog.validatePropertiesBeforeUpdateUnlocked(hms("thrift://10.0.0.1:9083"),
+ Collections.singletonMap("only_specified_database", "true"));
+ Assertions.assertTrue(checker.probed.isEmpty(), "untouched endpoint
must not be probed");
+ }
+
+ @Test
+ public void alterSkipsTheGateWhenNoCheckerInstalled() throws Exception {
+ // MUTATION: drop the isEnabled() guard -> the probe runs and the
intranet repoint is rejected -> red.
+ checker.enabled = false;
+ checker.intranetHost = "10.0.0.1";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+
catalog.validatePropertiesBeforeUpdateUnlocked(hms("thrift://hms.public.example.com:9083"),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://10.0.0.1:9083"));
+ Assertions.assertTrue(checker.probed.isEmpty(), "disabled checker must
not probe");
+ }
+
+ @Test
+ public void lockedAlterValidationNeverProbes() throws Exception {
+ // validatePropertiesBeforeUpdate runs under CatalogMgr's write lock,
so it must not reach the network.
+ // MUTATION: call checkEndpointsAgainstSsrf from
validatePropertiesBeforeUpdate -> probed -> red.
+ checker.intranetHost = "10.0.0.1";
+ HMSExternalCatalog catalog =
catalog(hms("thrift://hms.public.example.com:9083"));
+
+
Assertions.assertTrue(catalog.validatePropertiesBeforeUpdate(catalog.getProperties(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://10.0.0.1:9083")));
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ // ---- CREATE gate ----
+
+ @Test
+ public void createRejectsIntranetMetastore() {
+ checker.intranetHost = "127.0.0.1";
+ HMSExternalCatalog catalog = catalog(hms("thrift://127.0.0.1:9083"));
+
+ // MUTATION: drop checkEndpointsAgainstSsrf from checkWhenCreating ->
CREATE accepts the intranet
+ // metastore -> red.
+ Assertions.assertThrows(DdlException.class,
catalog::checkWhenCreating);
+ }
+
+ @Test
+ public void createAcceptsPublicEndpointsAndProbesEveryDeclaredKey() throws
Exception {
+ Map<String, String> properties =
hms("thrift://hms1.public.example.com:9083");
+ properties.put("uri", "thrift://hms2.public.example.com:9083");
+ properties.put("metastore.thrift.uris",
"thrift://hms3.public.example.com:9083");
+ properties.put("test_connection", "false");
+ HMSExternalCatalog catalog = catalog(properties);
+
+ catalog.checkWhenCreating();
+ Assertions.assertEquals(Arrays.asList("hms1.public.example.com:9083",
"hms2.public.example.com:9083",
+ "hms3.public.example.com:9083"), checker.probed);
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"thrift://127.0.0.1:9083?ignored",
"thrift://127.0.0.1:9083#ignored",
+ "thrift://[email protected]:9083",
"thrift://user:[email protected]:9083/path?ignored#ignored"})
+ public void createRejectsUriDestinationWithConnectionTestDisabled(String
endpoint) {
+ checker.intranetHost = "127.0.0.1";
+ Map<String, String> properties = hms(endpoint);
+ properties.put("test_connection", "false");
+ HMSExternalCatalog catalog = catalog(properties);
+
+ Assertions.assertThrows(DdlException.class,
catalog::checkWhenCreating);
+ Assertions.assertEquals(Collections.singletonList("127.0.0.1:9083"),
checker.probed);
+ }
+
+ @Test
+ public void createProbesModernMetastoreKeyBesideLegacyKey() {
+ // MetastoreConf prefers metastore.thrift.uris when both keys reach
the client, so a public legacy uri must
+ // not be the only value probed.
+ checker.intranetHost = "127.0.0.1";
+ Map<String, String> properties =
hms("thrift://hms.public.example.com:9083");
+ properties.put("metastore.thrift.uris", "thrift://127.0.0.1:9083");
+ properties.put("test_connection", "false");
+ HMSExternalCatalog catalog = catalog(properties);
+
+ Assertions.assertThrows(DdlException.class,
catalog::checkWhenCreating);
+ Assertions.assertTrue(checker.probed.contains("127.0.0.1:9083"),
checker.probed.toString());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"thrift://${hive.ssrf.target}:9083",
"${hive.ssrf.target}:9083", "${hive.ssrf.target}",
+ "thrift://${env.HOSTNAME}:9083"})
+ public void
createRejectsPlaceholderEndpointWithConnectionTestDisabled(String endpoint) {
+ // Without the placeholder check the schemed forms fail URI parsing
and are skipped, and
+ // "${hive.ssrf.target}:9083" probes a host literally named
"${hive.ssrf.target}", whose DNS failure is
+ // ignored.
+ Map<String, String> properties = hms(endpoint);
+ properties.put("hive.ssrf.target", "127.0.0.1");
+ properties.put("test_connection", "false");
+ HMSExternalCatalog catalog = catalog(properties);
+
+ Assertions.assertThrows(DdlException.class,
catalog::checkWhenCreating);
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ @ParameterizedTest
+ @ValueSource(strings = {"hive.metastore.uris", "uri",
"metastore.thrift.uris"})
+ public void createRejectsPlaceholderUnderEveryDeclaredKey(String key) {
+ // On CREATE the gate runs before the property validation, so every
declared key reaches the
+ // placeholder rule, the uri alias included.
+ Map<String, String> properties =
hms("thrift://hms.public.example.com:9083");
+ properties.put(key, "thrift://${hive.ssrf.target}:9083");
+ properties.put("hive.ssrf.target", "127.0.0.1");
+ properties.put("test_connection", "false");
+ HMSExternalCatalog catalog = catalog(properties);
+
+ DdlException e = Assertions.assertThrows(DdlException.class,
catalog::checkWhenCreating);
+ Assertions.assertTrue(e.getMessage().contains("placeholder")
+ && e.getMessage().contains("Property '" + key + "'"),
e.getMessage());
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ // ---- wiring through CatalogFactory: CREATE runs the gate, replay never
does ----
+
+ @Test
+ public void createThroughCatalogFactoryRunsTheGate() {
+ checker.intranetHost = "127.0.0.1";
+ CreateCatalogCommand cmd = new
CreateCatalogCommand("ssrf_gate_catalog", false, null, "",
+ hms("thrift://127.0.0.1:9083"));
+
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> CatalogFactory.createFromCommand(1L, cmd));
+ Assertions.assertTrue(e.getMessage().contains("SSRF policy"),
e.getMessage());
+ }
+
+ @Test
+ public void replayThroughCatalogFactoryNeverProbes() throws Exception {
+ // A catalog in the edit log was gated when it was created; replay
must not reach the network.
+ checker.intranetHost = "127.0.0.1";
+ CatalogLog log = new CatalogLog();
+ log.setCatalogId(1L);
+ log.setCatalogName("ssrf_gate_catalog");
+ log.setComment("");
+ log.setProps(hms("thrift://127.0.0.1:9083"));
+
+ CatalogIf catalog = CatalogFactory.createFromLog(log);
+ Assertions.assertTrue(catalog instanceof HMSExternalCatalog,
String.valueOf(catalog));
+ Assertions.assertTrue(checker.probed.isEmpty(),
checker.probed.toString());
+ }
+
+ // ---- ALTER through CatalogMgr: the probe runs before the write lock is
taken ----
+
+ @Test
+ public void alterThroughCatalogMgrProbesOutsideTheWriteLock() throws
Exception {
+ // MUTATION: run the gate from validatePropertiesBeforeUpdate, under
CatalogMgr's write lock -> the probe
+ // sees the lock held -> red.
+ CatalogMgr manager = new CatalogMgr();
+ AlterableHmsCatalog catalog = new
AlterableHmsCatalog(hms("thrift://hms.public.example.com:9083"));
+ addCatalog(manager, catalog);
+ checker.catalogMgrLock = catalogMgrLock(manager);
+ List<Map<String, String>> journaled = new ArrayList<>();
+
+ try (MockedStatic<Env> mockedEnv = mockEnv(journaled)) {
+ manager.alterCatalogProps(catalog.getName(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://hms2.public.example.com:9083"));
+ }
+
+
Assertions.assertEquals(Collections.singletonList("hms2.public.example.com:9083"),
checker.probed);
+ Assertions.assertEquals(Collections.singletonList(false),
checker.catalogMgrLockHeld);
+ Assertions.assertEquals("thrift://hms2.public.example.com:9083",
+ catalog.getProperties().get("hive.metastore.uris"));
+ Assertions.assertEquals(1, journaled.size(), journaled.toString());
+ }
+
+ @Test
+ public void
alterThroughCatalogMgrRejectsIntranetWithoutChangingTheCatalog() throws
Exception {
+ checker.intranetHost = "10.0.0.1";
+ CatalogMgr manager = new CatalogMgr();
+ AlterableHmsCatalog catalog = new
AlterableHmsCatalog(hms("thrift://hms.public.example.com:9083"));
+ addCatalog(manager, catalog);
+ checker.catalogMgrLock = catalogMgrLock(manager);
+ List<Map<String, String>> journaled = new ArrayList<>();
+
+ try (MockedStatic<Env> mockedEnv = mockEnv(journaled)) {
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> manager.alterCatalogProps(catalog.getName(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://10.0.0.1:9083")));
+ Assertions.assertTrue(e.getMessage().contains("SSRF policy"),
e.getMessage());
+ }
+
+ Assertions.assertEquals(Collections.singletonList(false),
checker.catalogMgrLockHeld);
+ Assertions.assertEquals("thrift://hms.public.example.com:9083",
+ catalog.getProperties().get("hive.metastore.uris"));
+ Assertions.assertTrue(journaled.isEmpty(), journaled.toString());
+ }
+
+ @Test
+ public void
alterThroughCatalogMgrChecksAgainWhenTheCatalogChangesDuringTheProbe() throws
Exception {
+ // Another ALTER commits while this one probes outside the lock, so
the probed snapshot is stale: the ALTER
+ // must check the current properties again before it applies.
+ // MUTATION: drop the snapshot comparison under the write lock ->
applied after a single probe -> red.
+ CatalogMgr manager = new CatalogMgr();
+ AlterableHmsCatalog catalog = new
AlterableHmsCatalog(hms("thrift://hms.public.example.com:9083"));
+ addCatalog(manager, catalog);
+ checker.catalogMgrLock = catalogMgrLock(manager);
+ checker.duringFirstProbe = () -> {
+ try {
+ manager.alterCatalogProps(catalog.getName(),
+ Collections.singletonMap("only_specified_database",
"true"));
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ };
+ List<Map<String, String>> journaled = new ArrayList<>();
+
+ try (MockedStatic<Env> mockedEnv = mockEnv(journaled)) {
+ manager.alterCatalogProps(catalog.getName(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://hms2.public.example.com:9083"));
+ }
+
+ Assertions.assertEquals(Arrays.asList("hms2.public.example.com:9083",
"hms2.public.example.com:9083"),
+ checker.probed);
+ Assertions.assertEquals(Arrays.asList(false, false),
checker.catalogMgrLockHeld);
+ Map<String, String> properties = catalog.getProperties();
+ Assertions.assertEquals("true",
properties.get("only_specified_database"));
+ Assertions.assertEquals("thrift://hms2.public.example.com:9083",
properties.get("hive.metastore.uris"));
+ Assertions.assertEquals(2, journaled.size(), journaled.toString());
+ }
+
+ @Test
+ public void
alterThroughCatalogMgrFailsCleanlyWhenTheCatalogIsDroppedDuringTheProbe()
throws Exception {
+ // A DROP CATALOG that commits while the ALTER probes outside the lock
leaves the probed object detached.
+ // MUTATION: drop the catalog identity check under the write lock ->
the ALTER is applied to the dropped
+ // object and fails with a NullPointerException instead of "No catalog
found" -> red.
+ CatalogMgr manager = new CatalogMgr();
+ AlterableHmsCatalog catalog = new
AlterableHmsCatalog(hms("thrift://hms.public.example.com:9083"));
+ addCatalog(manager, catalog);
+ checker.duringFirstProbe = () -> removeCatalog(manager, catalog);
+ List<Map<String, String>> journaled = new ArrayList<>();
+
+ try (MockedStatic<Env> mockedEnv = mockEnv(journaled)) {
+ DdlException e = Assertions.assertThrows(DdlException.class,
+ () -> manager.alterCatalogProps(catalog.getName(),
+ Collections.singletonMap("hive.metastore.uris",
"thrift://hms2.public.example.com:9083")));
+ Assertions.assertTrue(e.getMessage().contains("No catalog found"),
e.getMessage());
+ }
+ Assertions.assertTrue(journaled.isEmpty(), journaled.toString());
+ }
+
+ private static HMSExternalCatalog catalog(Map<String, String> props) {
+ return new HMSExternalCatalog(1L, "ssrf-gate-catalog", null, props,
"");
+ }
+
+ private static Map<String, String> hms(String metastoreUris) {
+ Map<String, String> props = new HashMap<>();
+ props.put("type", "hms");
+ props.put("hive.metastore.uris", metastoreUris);
+ return props;
+ }
+
+ private static void addCatalog(CatalogMgr manager, HMSExternalCatalog
catalog) throws Exception {
+ Field ids = CatalogMgr.class.getDeclaredField("idToCatalog");
+ ids.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<Long, CatalogIf> idToCatalog = (Map<Long, CatalogIf>)
ids.get(manager);
+ idToCatalog.put(catalog.getId(), catalog);
+ Field names = CatalogMgr.class.getDeclaredField("nameToCatalog");
+ names.setAccessible(true);
+ @SuppressWarnings("unchecked")
+ Map<String, CatalogIf> nameToCatalog = (Map<String, CatalogIf>)
names.get(manager);
+ nameToCatalog.put(catalog.getName(), catalog);
+ }
+
+ /** Unregisters the catalog the way a committed DROP CATALOG leaves
CatalogMgr's maps. */
+ private static void removeCatalog(CatalogMgr manager, HMSExternalCatalog
catalog) {
+ try {
+ Field ids = CatalogMgr.class.getDeclaredField("idToCatalog");
+ ids.setAccessible(true);
+ ((Map<?, ?>) ids.get(manager)).remove(catalog.getId());
+ Field names = CatalogMgr.class.getDeclaredField("nameToCatalog");
+ names.setAccessible(true);
+ ((Map<?, ?>) names.get(manager)).remove(catalog.getName());
+ } catch (ReflectiveOperationException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
+ private static ReentrantReadWriteLock catalogMgrLock(CatalogMgr manager)
throws Exception {
+ Field lock = CatalogMgr.class.getDeclaredField("lock");
+ lock.setAccessible(true);
+ return (ReentrantReadWriteLock) lock.get(manager);
+ }
+
+ /** An Env whose edit log records each journaled ALTER and whose cache
manager runs fenced actions inline. */
+ private static MockedStatic<Env> mockEnv(List<Map<String, String>>
journaled) {
+ Env env = Mockito.mock(Env.class);
+ EditLog editLog = Mockito.mock(EditLog.class);
+ ExternalMetaCacheMgr cacheMgr =
Mockito.mock(ExternalMetaCacheMgr.class);
+ Mockito.when(env.getEditLog()).thenReturn(editLog);
+ Mockito.when(env.getExtMetaCacheMgr()).thenReturn(cacheMgr);
+ Mockito.when(cacheMgr.withCatalogLifecycleLock(Mockito.anyLong(),
Mockito.any()))
+ .thenAnswer(invocation -> ((Supplier<?>)
invocation.getArgument(1)).get());
+ Mockito.doAnswer(invocation -> {
+ CatalogLog log = invocation.getArgument(1);
+ journaled.add(new HashMap<>(log.getNewProps()));
+ return null;
+
}).when(editLog).logCatalogLog(Mockito.eq(OperationType.OP_ALTER_CATALOG_PROPS),
Mockito.any());
+ MockedStatic<Env> mockedEnv = Mockito.mockStatic(Env.class);
+ mockedEnv.when(Env::getCurrentEnv).thenReturn(env);
+ return mockedEnv;
+ }
+
+ /** Records the net-hook bracket and the probed endpoints; throws
SecurityException for an intranet host. */
+ private static final class RecordingChecker extends SecurityChecker {
+ private boolean enabled = true;
+ private String intranetHost;
+ private final List<String> probed = new ArrayList<>();
+ // Set by the CatalogMgr tests: whether this thread held CatalogMgr's
lock at each probe.
+ private ReentrantReadWriteLock catalogMgrLock;
+ private final List<Boolean> catalogMgrLockHeld = new ArrayList<>();
+ // Runs once, inside the first probe, to simulate a DDL that commits
while the probe is in flight.
+ private Runnable duringFirstProbe;
+
+ @Override
+ public String getSafeJdbcUrl(String originJdbcUrl) {
+ return originJdbcUrl;
+ }
+
+ @Override
+ public void startSSRFChecking(String originUri) {
+ }
+
+ @Override
+ public void startSSRFChecking() {
+ }
+
+ @Override
+ public void stopSSRFChecking() {
+ }
+
+ @Override
+ public boolean isEnabled() {
+ return enabled;
+ }
+
+ @Override
+ protected void doConnect(String host, int port, int connectTimeoutMs)
throws IOException {
+ probed.add(host + ":" + port);
+ if (catalogMgrLock != null) {
+
catalogMgrLockHeld.add(catalogMgrLock.isWriteLockedByCurrentThread()
+ || catalogMgrLock.getReadHoldCount() > 0);
+ }
+ if (duringFirstProbe != null) {
+ Runnable action = duringFirstProbe;
+ duringFirstProbe = null;
+ action.run();
+ }
+ if (host.equals(intranetHost)) {
+ throw new SecurityException("simulated
SSRFUnsafeConnectionException for " + host);
+ }
+ }
+ }
+
+ /** An HMS catalog whose committed ALTER skips the cache and client reset,
which needs a running FE. */
+ private static final class AlterableHmsCatalog extends HMSExternalCatalog {
+ AlterableHmsCatalog(Map<String, String> props) {
+ super(2L, "ssrf_gate_alter_catalog", null, props, "");
+ }
+
+ @Override
+ public void notifyPropertiesUpdated(Map<String, String> updatedProps) {
+ }
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
