You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@storm.apache.org by rz...@apache.org on 2026/04/14 11:37:37 UTC
(storm) branch master updated (d0ff7c151 -> 81b5b80ec)
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
from d0ff7c151 Bump actions/cache from 5.0.4 to 5.0.5
new 5aa8698ea Hardening: validate JSONP callback and add nosniff on UI JSON responses
new 81b5b80ec The root cause of the CI failure was that the old code's SSLContext.setDefault() silently weakened TLS for the entire JVM, and the test's readContent was unknowingly piggy-backing on that global relaxation. After the hardening fix, readContent now does its own per-connection trust-all + hostname-verifier override (same pattern as the production fix), so the test no longer depends on JVM-wide state. This also makes the test an implicit regression guard: if someone [...]
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../PrometheusPreparableReporterTest.java | 31 +++++++++
.../java/org/apache/storm/daemon/ui/UIHelpers.java | 23 ++++++-
.../org/apache/storm/daemon/ui/UIHelpersTest.java | 73 ++++++++++++++++++++++
3 files changed, 124 insertions(+), 3 deletions(-)
(storm) 01/02: Hardening: validate JSONP callback and add nosniff on UI JSON responses
Posted by rz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
commit 5aa8698eae70f449a7eb1035145750ba43a9ff5e
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Tue Apr 14 13:35:33 2026 +0200
Hardening: validate JSONP callback and add nosniff on UI JSON responses
---
.../java/org/apache/storm/daemon/ui/UIHelpers.java | 23 ++++++-
.../org/apache/storm/daemon/ui/UIHelpersTest.java | 73 ++++++++++++++++++++++
2 files changed, 93 insertions(+), 3 deletions(-)
diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java
index 274d5a0bd..7ebb39cc3 100644
--- a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java
+++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/UIHelpers.java
@@ -444,9 +444,23 @@ public class UIHelpers {
stormRunJetty(port, null, null, headerBufferSize, configurator);
}
+ private static final Pattern JSONP_CALLBACK_PATTERN =
+ Pattern.compile("^[A-Za-z_$][A-Za-z0-9_$]*(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)*$");
+
+ private static String sanitizeJsonpCallback(String callback) {
+ if (callback == null) {
+ return null;
+ }
+ if (callback.length() > 128 || !JSONP_CALLBACK_PATTERN.matcher(callback).matches()) {
+ LOG.warn("Ignoring invalid JSONP callback parameter");
+ return null;
+ }
+ return callback;
+ }
+
/**
* wrapJsonInCallback.
- * @param callback callbackParameterName
+ * @param callback callbackParameterName (must already be validated)
* @param response response
* @return wrapJsonInCallback
*/
@@ -461,6 +475,7 @@ public class UIHelpers {
* @return getJsonResponseHeaders
*/
public static Map getJsonResponseHeaders(String callback, Map headers) {
+ final String safeCallback = sanitizeJsonpCallback(callback);
Map<String, String> headersResult = new HashMap<>();
headersResult.put("Cache-Control", "no-cache, no-store");
headersResult.put("Access-Control-Allow-Origin", "*");
@@ -469,7 +484,8 @@ public class UIHelpers {
+ "Access-Controler-Allow-Origin, "
+ "X-Requested-By, X-Csrf-Token, "
+ "Authorization, X-Requested-With");
- if (callback != null) {
+ headersResult.put("X-Content-Type-Options", "nosniff");
+ if (safeCallback != null) {
headersResult.put("Content-Type", "application/javascript;charset=utf-8");
} else {
headersResult.put("Content-Type", "application/json;charset=utf-8");
@@ -481,8 +497,9 @@ public class UIHelpers {
}
public static String getJsonResponseBody(Object data, String callback, boolean needSerialize) {
+ String safeCallback = sanitizeJsonpCallback(callback);
String serializedData = needSerialize ? JSONValue.toJSONString(data) : (String) data;
- return callback != null ? wrapJsonInCallback(callback, serializedData) : serializedData;
+ return safeCallback != null ? wrapJsonInCallback(safeCallback, serializedData) : serializedData;
}
/**
diff --git a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java
index 30e8e0e53..852af5cb9 100644
--- a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java
+++ b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/UIHelpersTest.java
@@ -592,4 +592,77 @@ class UIHelpersTest {
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("Unable to find entry for spoutId '" + spoutId + "'"));
}
+
+ @Test
+ public void testGetJsonResponseBodyNoCallbackReturnsJson() {
+ Map<String, Object> data = new HashMap<>();
+ data.put("a", 1);
+ String body = UIHelpers.getJsonResponseBody(data, null, true);
+ assertEquals("{\"a\":1}", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyValidCallbackIsWrapped() {
+ Map<String, Object> data = new HashMap<>();
+ data.put("a", 1);
+ String body = UIHelpers.getJsonResponseBody(data, "myCb", true);
+ assertEquals("myCb({\"a\":1});", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyValidDottedCallbackIsWrapped() {
+ String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "foo.bar.$baz_0", false);
+ assertEquals("foo.bar.$baz_0({\"x\":1});", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyInvalidCallbackFallsBackToJson() {
+ Map<String, Object> data = new HashMap<>();
+ data.put("a", 1);
+ String body = UIHelpers.getJsonResponseBody(data, "alert(document.cookie)//", true);
+ assertEquals("{\"a\":1}", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyEmptyCallbackFallsBackToJson() {
+ String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "", false);
+ assertEquals("{\"x\":1}", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyTooLongCallbackFallsBackToJson() {
+ StringBuilder sb = new StringBuilder("cb");
+ for (int i = 0; i < 200; i++) {
+ sb.append('x');
+ }
+ String body = UIHelpers.getJsonResponseBody("{\"x\":1}", sb.toString(), false);
+ assertEquals("{\"x\":1}", body);
+ }
+
+ @Test
+ public void testGetJsonResponseBodyRejectsCallbacksStartingWithDigit() {
+ String body = UIHelpers.getJsonResponseBody("{\"x\":1}", "1cb", false);
+ assertEquals("{\"x\":1}", body);
+ }
+
+ @Test
+ public void testGetJsonResponseHeadersNoCallbackUsesJsonContentType() {
+ Map headers = UIHelpers.getJsonResponseHeaders(null, null);
+ assertEquals("application/json;charset=utf-8", headers.get("Content-Type"));
+ assertEquals("nosniff", headers.get("X-Content-Type-Options"));
+ }
+
+ @Test
+ public void testGetJsonResponseHeadersValidCallbackUsesJavaScriptContentType() {
+ Map headers = UIHelpers.getJsonResponseHeaders("myCb", null);
+ assertEquals("application/javascript;charset=utf-8", headers.get("Content-Type"));
+ assertEquals("nosniff", headers.get("X-Content-Type-Options"));
+ }
+
+ @Test
+ public void testGetJsonResponseHeadersInvalidCallbackFallsBackToJsonContentType() {
+ Map headers = UIHelpers.getJsonResponseHeaders("alert(1)//", null);
+ assertEquals("application/json;charset=utf-8", headers.get("Content-Type"));
+ assertEquals("nosniff", headers.get("X-Content-Type-Options"));
+ }
}
\ No newline at end of file
(storm) 02/02: The root cause of the CI failure was that the old code's SSLContext.setDefault() silently weakened TLS for the entire JVM, and the test's readContent was unknowingly piggy-backing on that global relaxation. After the hardening fix, readContent now does its own per-connection trust-all + hostname-verifier override (same pattern as the production fix), so the test no longer depends on JVM-wide state. This also makes the test an implicit regression guard: if someone reintroduces SSLCon [...]
Posted by rz...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
rzo1 pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/storm.git
commit 81b5b80ec652c931bec906adf37ad8ad1db62070
Author: Richard Zowalla <rz...@apache.org>
AuthorDate: Tue Apr 14 13:37:18 2026 +0200
The root cause of the CI failure was that the old code's SSLContext.setDefault() silently weakened TLS for the entire JVM, and the
test's readContent was unknowingly piggy-backing on that global relaxation. After the hardening fix, readContent now does its own per-connection trust-all + hostname-verifier override (same pattern as the production fix), so the test no longer depends on JVM-wide
state. This also makes the test an implicit regression guard: if someone reintroduces SSLContext.setDefault() in the reporter, nothing
in the test relies on it.
---
.../PrometheusPreparableReporterTest.java | 31 ++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/external/storm-metrics-prometheus/src/test/java/org/apache/storm/metrics/prometheus/PrometheusPreparableReporterTest.java b/external/storm-metrics-prometheus/src/test/java/org/apache/storm/metrics/prometheus/PrometheusPreparableReporterTest.java
index a49a808d7..9e33be80f 100644
--- a/external/storm-metrics-prometheus/src/test/java/org/apache/storm/metrics/prometheus/PrometheusPreparableReporterTest.java
+++ b/external/storm-metrics-prometheus/src/test/java/org/apache/storm/metrics/prometheus/PrometheusPreparableReporterTest.java
@@ -22,11 +22,17 @@ import org.testcontainers.containers.wait.strategy.Wait;
import org.testcontainers.junit.jupiter.Testcontainers;
import org.testcontainers.utility.MountableFile;
+import javax.net.ssl.HttpsURLConnection;
+import javax.net.ssl.SSLContext;
+import javax.net.ssl.TrustManager;
+import javax.net.ssl.X509TrustManager;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStreamReader;
import java.net.HttpURLConnection;
import java.net.URL;
+import java.security.GeneralSecurityException;
+import java.security.cert.X509Certificate;
import java.util.Arrays;
import java.util.Base64;
import java.util.HashSet;
@@ -163,6 +169,31 @@ public class PrometheusPreparableReporterTest {
private String readContent(String url, Map<String, Object> conf) throws IOException {
final URL obj = new URL(url);
final HttpURLConnection con = (HttpURLConnection) obj.openConnection();
+ if (con instanceof HttpsURLConnection) {
+ // The test PushGateway uses a self-signed certificate. Scope the trust-all
+ // SSLContext to this connection only, so we do not mutate JVM-wide TLS state.
+ try {
+ final SSLContext sslContext = SSLContext.getInstance("TLS");
+ sslContext.init(null, new TrustManager[]{new X509TrustManager() {
+ @Override
+ public X509Certificate[] getAcceptedIssuers() {
+ return new X509Certificate[0];
+ }
+
+ @Override
+ public void checkClientTrusted(X509Certificate[] chain, String authType) {
+ }
+
+ @Override
+ public void checkServerTrusted(X509Certificate[] chain, String authType) {
+ }
+ }}, null);
+ ((HttpsURLConnection) con).setSSLSocketFactory(sslContext.getSocketFactory());
+ ((HttpsURLConnection) con).setHostnameVerifier((h, s) -> true);
+ } catch (GeneralSecurityException e) {
+ throw new IOException(e);
+ }
+ }
con.setRequestMethod("GET");
if (conf.containsKey("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_user")) {