LuciferYang opened a new pull request, #11785:
URL: https://github.com/apache/gravitino/pull/11785

   ### What changes were proposed in this pull request?
   
   `FileFetcher` already checked a remote URI's host against an SSRF denylist, 
but the download step then let the JDK re-resolve the hostname when it 
connected. That leaves a DNS-rebinding window: the host can look safe during 
validation and resolve to an internal address when the download actually 
connects.
   
   The fix resolves the host once in `RemoteUriValidator`, validates every 
address it resolves to, and returns one for the caller to pin. 
`RemoteFileDownloader` is a small HTTP/1.0 client that connects straight to 
that pinned `InetAddress` and never looks the hostname up again, while still 
using the original hostname for TLS/SNI verification. It follows no redirects 
and accepts only Content-Length or chunked bodies, so a truncated download 
can't be mistaken for a complete one.
   
   Along the way it also:
   
   - widens the denylist to 0.0.0.0/8, CGNAT 100.64/10 (covers the Alibaba 
metadata IP), the Oracle metadata IP, broadcast, IPv6 unique-local, and the 
IPv4-compatible/mapped/NAT64/6to4/ISATAP forms that hide an IPv4 in an IPv6 
literal;
   - rejects `ftp://` when blocking is on, because its PASV data channel 
connects to an address the server picks and can't be pinned;
   - adds `SafeUri.redact` to strip userinfo and query strings from URIs before 
they reach logs or error messages.
   
   ### Why are the changes needed?
   
   The server downloads keytabs and jars from operator-supplied URIs. Without 
pinning, someone who controls DNS for one of those hosts can pass validation 
and still steer the connection to a cloud metadata endpoint or an internal 
service. The extra denylist entries and the FTP restriction remove the other 
ways to reach those addresses.
   
   Fix: #11784
   
   ### Does this PR introduce _any_ user-facing change?
   
   No new config or API. One behavior change: with 
`gravitino.fetchFile.blockUnsafeRemoteUri` enabled (the default), `ftp://` URIs 
are now rejected. Set it to false to allow them from a trusted source.
   
   ### How was this patch tested?
   
   Added 48 unit tests across four classes covering address classification 
(including the IPv6-embedded forms), address pinning, 
redirect/unframed/oversized/truncated responses, chunked decoding, FTP 
rejection, and URI redaction. `./gradlew :common:test` and 
`:common:spotlessCheck` pass.
   


-- 
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]

Reply via email to