mattcasters commented on PR #8287:
URL: https://github.com/apache/hop/pull/8287#issuecomment-5615259866
### Security & Integrity Review for PR #8287 / Issue #8285
Overall, moving to a Hadoop-free WebHDFS / HttpFS / Knox VFS provider is a
huge security win—it eliminates hundreds of transitive dependencies, outdated
RPC libraries, and common CVEs. Hostname verification is enabled by default,
SPNEGO credential delegation is disabled (`requestCredDeleg(false)`), and paths
are normalized by VFS.
Here is a summary of security, data integrity, and credential-handling
findings to address:
---
#### 1. [Critical / Data Integrity] Silent Data Truncation in
`GzipCompressionInputStream`
* **Location:**
`engine/src/main/java/org/apache/hop/core/compress/gzip/GzipCompressionInputStream.java:54-72`
* **Issue:** In `read()` and `read(byte[], int, int)`, catching
`EOFException` and returning `-1` masks truncated gzip streams:
```java
catch (EOFException e) {
// Network/VFS streams often end with -1 while the inflater still wants
the gzip trailer.
return -1;
}
```
* **Impact:** `GzipCompressionInputStream` is used globally by all
transforms reading `.gz` files (CSV, Text File Input, Parquet, JSON, etc.). If
a download is aborted or a file is truncated, `java.util.zip.GZIPInputStream`
throws `EOFException` because the DEFLATE stream or 8-byte trailer (CRC32 +
ISIZE) is missing. Converting this to `-1` causes pipelines to complete with
`SUCCESS` on partial data, silently corrupting data in downstream
databases/warehouses and completely skipping CRC32 validation.
* **Recommendation:** The 64KB buffering added in
`HdfsWebHdfsClient.streamEntity` and `GzipCompressionInputStream` already
resolves the issue of reading trailers across packet boundaries. Swallowing
`EOFException` globally should be removed so truncated files fail fast.
---
#### 2. [High] Unconditional SPNEGO Ticket Acquisition in
`HdfsWebHdfsClient.putStream`
* **Location:**
`plugins/tech/hadoop/src/main/java/org/apache/hop/vfs/hdfs/client/HdfsWebHdfsClient.java:236-242`
* **Issue:** While `getAbsoluteStream` guards SPNEGO via `if
(shouldSpnegoForLocation(location))`, `putStream` calls `addSpnego(put, uri)`
unconditionally:
```java
private void putStream(String uri, InputStream body) throws IOException {
HttpPut put = new HttpPut(uri);
...
addSpnego(put, uri);
executeRequest(put);
}
```
* **Impact:** During WebHDFS `CREATE`, NameNode returns a DataNode
`Location` URL. `addSpnego` extracts the host and requests a Kerberos service
ticket for `HTTP@<datanode-host>`, attaching `Authorization: Negotiate <token>`
to the PUT request.
1. If redirected to an untrusted host or external location, the user's
SPNEGO Kerberos ticket is leaked.
2. On standard Kerberized clusters, DataNodes expect delegation tokens in
the query parameters (as noted in line 288); sending a SPNEGO ticket to
DataNodes can fail if no SPN exists for individual DataNodes or if the DataNode
rejects it.
* **Fix:** Guard with `shouldSpnegoForLocation(uri)` in `putStream`:
```java
if (shouldSpnegoForLocation(uri)) {
addSpnego(put, uri);
}
```
---
#### 3. [Medium] Missing Password Decryption for `truststorePassword` in
`HdfsTls`
* **Location:**
`plugins/tech/hadoop/src/main/java/org/apache/hop/vfs/hdfs/HdfsTls.java:66`
* **Issue:** `HdfsMeta.truststorePassword` is annotated with
`@HopMetadataProperty(password = true)`. Hop encrypts these values upon saving
(e.g. `Encrypted 2be98af...`). In `HdfsTls.loadKeyStore`, the raw value is
passed to `keyStore.load()` without decrypting:
```java
loadKeyStore(bytes,
variables.resolve(Const.NVL(meta.getTruststorePassword(), "")));
```
* **Impact:** Saved connections with password-protected keystores
(JKS/PKCS12) will fail to open on reload because the ciphertext is used as the
password.
* **Fix:** Wrap with `Encr.decryptPasswordOptionallyEncrypted(...)`:
```java
String password = Encr.decryptPasswordOptionallyEncrypted(
variables.resolve(Const.NVL(meta.getTruststorePassword(), "")));
return loadKeyStore(bytes, password);
```
---
#### 4. [Medium] Potential Protocol Downgrade on Redirects (HTTPS -> HTTP)
* **Location:**
`plugins/tech/hadoop/src/main/java/org/apache/hop/vfs/hdfs/client/HdfsWebHdfsClient.java:160-167,
209-214`
* **Issue:** When the connection is configured for HTTPS (`meta.isHttps()`
or Knox), the initial request is encrypted. However, WebHDFS 307 redirects or
JSON `Location` values from NameNode contain absolute URLs.
* **Impact:** If a cluster returns an `http://` DataNode URL
(misconfiguration or proxy/MITM), `getAbsoluteStream` and `putStream` will
silently connect over plaintext HTTP, transmitting data and any delegation
tokens unencrypted.
* **Recommendation:** If `httpScheme` is `"https"`, enforce that `location`
starts with `https://` (or rewrite/reject if downgraded to `http://`).
---
#### 5. [Medium / Architectural] JVM-Wide System Property Mutation in
`HdfsKerberosSession`
* **Location:**
`plugins/tech/hadoop/src/main/java/org/apache/hop/vfs/hdfs/kerberos/HdfsKerberosSession.java:164-178`
* **Issue:** `applyJvmKerberosConfig` sets `java.security.krb5.conf`,
`realm`, and `kdc` via `System.setProperty()`.
* **Impact:** These properties are JVM-wide. In Hop Server or Hop GUI
environments where multiple HDFS connections or other Kerberized services
(Kafka, JDBC) run concurrently with different realms, modifying global system
properties can cause cross-connection collisions.
---
#### 6. [Low] Monotonic Session Growth in `HdfsKerberosRenewer`
* **Location:**
`plugins/tech/hadoop/src/main/java/org/apache/hop/vfs/hdfs/kerberos/HdfsKerberosRenewer.java:43-48`
* **Issue:** Every connection test or VFS filesystem creation instantiates a
new `HdfsKerberosSession` and registers it in `HdfsKerberosRenewer`. Because
sessions are never unregistered and don't implement `equals`/`hashCode`,
`sessions` grows monotonically.
* **Impact:** A long-running Hop instance (like Hop Server or GUI) will keep
attempting re-login every minute for dead/test sessions, keeping credentials
and `Subject` references in memory.
* **Recommendation:** Add an `unregister(HdfsKerberosSession)` method and
call it when the VFS filesystem is closed or a test probe finishes.
--
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]
