morningman opened a new pull request, #70364: URL: https://github.com/apache/doris/pull/70364
### Background When an operator configures `security_checker_class_name`, CREATE CATALOG and ALTER CATALOG ask that checker whether the FE may connect to each endpoint the catalog's connector declares through `ConnectorSecuritySurface.ofSsrfEndpoints`, and refuse an intranet target (#69588). `PluginDrivenExternalCatalog.parseEndpoint` turns each declared value into the host and port to probe: `scheme://host:port`, or a bare `host:port`. A value it cannot turn into a concrete host and port is not probed. ### What problem does this PR solve? Issue Number: None Related PR: #69588 (added the gate) PR Type: Bugfix Problem Summary: An http or https endpoint written without a port, such as `http://10.0.0.1/ns` or `https://10.0.0.1`, passes the gate unprobed, although an HTTP client connects to port 80 or 443 for such a URI. Today only the Hive connector declares endpoints, and its thrift URIs carry their port, so no connector on master reaches this yet; the first http endpoint will be the Lance REST catalog's `lance.rest.uri`, and an https service URL usually names no port. The gate now probes such an endpoint at its scheme's default port. ### Details - **Frequency:** Not observed; found by code review while declaring the Lance REST catalog's endpoint. Triggers when an SSRF checker is configured and a declared endpoint is an http or https URL without a port. - **Root cause:** `parseEndpoint` returned null for a `scheme://` value whose `URI.getPort()` is -1, and `checkEndpointsAgainstSsrf` skips a null as nothing to probe. - **Why existing tests missed it:** Unit tests: `PluginDrivenExternalCatalogSsrfGateTest` covers thrift URIs, bare `host:port`, IPv6 and URI suffixes, and asserts that a thrift URI without a port is not probed; no case used an http or https URL. Regression: no suite reaches the gate, since it needs the operator's security checker SDK, which CI does not have; the unit tests use a fake. - **Fix:** For an http or https URI that names no port, `parseEndpoint` takes 80 or 443, the ports RFC 9110 assigns to the scheme; an explicit port still wins. A URI of another scheme without a port is still not probed: its default port, if it has one, belongs to the client, and the engine knows no connector's client. The fix is in the engine because the port follows from the scheme alone. - **Other paths and branches:** CREATE (`checkWhenCreating`) and both ALTER paths go through `parseEndpoint`, so all are fixed. branch-4.1 and branch-4.2 have the same rule in `HMSExternalCatalog.parseEndpoint`, but gate only Hive metastore thrift URIs, so they are not affected. Introduced by #69588. ### Testing Local (macOS), on master `f1bc8b3690c` plus this change: - `PluginDrivenExternalCatalogSsrfGateTest`: the new `parseEndpointUsesTheDefaultPortOfAWebScheme` (seven URIs: with and without a path, an upper-case scheme, user-info, query and fragment, IPv6, an explicit port) and `alterRejectsAnIntranetWebEndpointWithoutAPort` (an http and an https endpoint through the real ALTER validation, with `10.0.0.1` flagged as intranet). On the code before this change 8 of them fail: the six URIs without a port parse to null, and both ALTER cases accept the endpoint ("Expected DdlException to be thrown, but nothing was thrown"). With it, all 44 cases of the class pass, as do the 18 of `PluginDrivenExternalCatalogDriverUrlGateTest`; fe-core checkstyle passes. ### Notes for Reviewers - The Javadoc of `ConnectorSecuritySurface.ofSsrfEndpoints` still describes the value as a `host:port`, optionally scheme-prefixed. It is left alone here because #70338, still open, edits that file. ### Release note None ### Check List (For Author) - Behavior changed: No - Does this need documentation: No - Pre-submission status: Ready ### AI Disclosure - Tool: Claude Code (Claude Opus 5.5) - Generated: the fix, the tests, and this description - Human review: The author reviewed part of the changes. - Prompts: While porting the Lance REST catalog, which declares `lance.rest.uri` as an endpoint, the author learned that the gate skips a URL without a port, and asked for a separate engine PR that probes http and https at their default ports. -- 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]
