[morningman]

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://user@10.0.0.1: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

  • Regression test

  • 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

  • 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.
  • 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?

  • No.
  • Yes.

Check List (For Reviewer who merge this PR)

• Confirm the release note
• Confirm test cases
• Confirm document
• Add branch pick label

[hello-stephen]

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
3. What features were added. Why was this function added?
4. Which code was refactored and why was this part of the code refactored?
5. Which functions were optimized and what is the difference before and after the optimization?

[morningman]

run buildall

[hello-stephen]

FE UT Coverage Report

Increment line coverage 78.31% (65/83) 🎉
Increment coverage report
Complete coverage report