From d9329c2013fada51719a09b12ec9bec818851f4e Mon Sep 17 00:00:00 2001 From: Gianluca Graziadei Date: Sun, 23 Aug 2026 11:15:17 +0200 Subject: [PATCH 1/2] Require every UI API endpoint to declare its authorization --- .../src/jvm/org/apache/storm/Config.java | 15 ++ .../org/apache/storm/utils/ConfigUtils.java | 25 +++ .../apache/storm/utils/ConfigUtilsTest.java | 98 +++++++++ .../apache/storm/daemon/nimbus/Nimbus.java | 2 +- .../nimbus/NimbusGetNimbusConfTest.java | 72 +++++++ .../ui/filters/AuthorizedUserFilter.java | 18 +- .../daemon/ui/resources/StormApiResource.java | 2 + .../ui/resources/UnauthenticatedNimbusOp.java | 39 ++++ .../ui/filters/AuthorizedUserFilterTest.java | 188 ++++++++++++++++++ 9 files changed, 457 insertions(+), 2 deletions(-) create mode 100644 storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetNimbusConfTest.java create mode 100644 storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/UnauthenticatedNimbusOp.java create mode 100644 storm-webapp/src/test/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilterTest.java diff --git a/storm-client/src/jvm/org/apache/storm/Config.java b/storm-client/src/jvm/org/apache/storm/Config.java index 226a4b4caa9..4e3dbe061bf 100644 --- a/storm-client/src/jvm/org/apache/storm/Config.java +++ b/storm-client/src/jvm/org/apache/storm/Config.java @@ -1240,6 +1240,7 @@ public class Config extends HashMap { * The password of the keystore that the nimbus TLS server uses. */ @IsString + @Password public static final String NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD = "nimbus.thrift.tls.server.keystore.password"; /** @@ -1258,6 +1259,7 @@ public class Config extends HashMap { * The password of the truststore that the nimbus TLS server uses. */ @IsString + @Password public static final String NIMBUS_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD = "nimbus.thrift.tls.server.truststore.password"; /** @@ -1272,6 +1274,7 @@ public class Config extends HashMap { * The password of the keystore that the nimbus TLS client uses. */ @IsString + @Password public static final String NIMBUS_THRIFT_TLS_CLIENT_KEYSTORE_PASSWORD = "nimbus.thrift.tls.client.keystore.password"; /** @@ -1296,6 +1299,7 @@ public class Config extends HashMap { * The password of the truststore that the nimbus TLS client uses. */ @IsString + @Password public static final String NIMBUS_THRIFT_TLS_CLIENT_TRUSTSTORE_PASSWORD = "nimbus.thrift.tls.client.truststore.password"; /** @@ -1351,6 +1355,7 @@ public class Config extends HashMap { * The password of the keystore that the supervisor TLS server uses. */ @IsString + @Password public static final String SUPERVISOR_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD = "supervisor.thrift.tls.server.keystore.password"; /** @@ -1363,6 +1368,7 @@ public class Config extends HashMap { * The password of the truststore that the supervisor TLS server uses. */ @IsString + @Password public static final String SUPERVISOR_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD = "supervisor.thrift.tls.server.truststore.password"; /** @@ -1375,6 +1381,7 @@ public class Config extends HashMap { * The password of the keystore that the supervisor TLS client uses. */ @IsString + @Password public static final String SUPERVISOR_THRIFT_TLS_CLIENT_KEYSTORE_PASSWORD = "supervisor.thrift.tls.client.keystore.password"; /** @@ -1387,6 +1394,7 @@ public class Config extends HashMap { * The password of the truststore that the supervisor TLS client uses. */ @IsString + @Password public static final String SUPERVISOR_THRIFT_TLS_CLIENT_TRUSTSTORE_PASSWORD = "supervisor.thrift.tls.client.truststore.password"; /** @@ -1534,6 +1542,7 @@ public class Config extends HashMap { * Keystore password for ZooKeeper client connection over SSL. */ @IsString + @Password public static final String STORM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD = "storm.zookeeper.ssl.keystore.password"; /** * Truststore location for ZooKeeper client connection over SSL. @@ -1544,6 +1553,7 @@ public class Config extends HashMap { * Truststore password for ZooKeeper client connection over SSL. */ @IsString + @Password public static final String STORM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD = "storm.zookeeper.ssl.truststore.password"; /** * Enable or disable hostname verification. @@ -1735,6 +1745,7 @@ public class Config extends HashMap { * deny access from workers. */ @IsString + @Password public static final String STORM_ZOOKEEPER_AUTH_PAYLOAD = "storm.zookeeper.auth.payload"; /** * What Network Topography detection classes should we use. Given a list of supervisor hostnames (or IP addresses), this class would @@ -1925,6 +1936,7 @@ public class Config extends HashMap { * Netty based messaging: Specifies the truststore password when TLS is enabled. */ @IsString + @Password public static final String STORM_MESSAGING_NETTY_TLS_TRUSTSTORE_PASSWORD = "storm.messaging.netty.tls.truststore.password"; /** @@ -1937,6 +1949,7 @@ public class Config extends HashMap { * Netty based messaging: Specifies the keystore password when TLS is enabled. */ @IsString + @Password public static final String STORM_MESSAGING_NETTY_TLS_KEYSTORE_PASSWORD = "storm.messaging.netty.tls.keystore.password"; /** @@ -1949,6 +1962,7 @@ public class Config extends HashMap { * Netty based messaging: Specifies the client truststore password when TLS is enabled. */ @IsString + @Password public static final String STORM_MESSAGING_NETTY_TLS_CLIENT_TRUSTSTORE_PASSWORD = "storm.messaging.netty.tls.client.truststore.password"; @@ -1962,6 +1976,7 @@ public class Config extends HashMap { * Netty based messaging: Specifies the client keystore password when TLS is enabled. */ @IsString + @Password public static final String STORM_MESSAGING_NETTY_TLS_CLIENT_KEYSTORE_PASSWORD = "storm.messaging.netty.tls.client.keystore.password"; diff --git a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java index 663a7915437..03fb37fd87c 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java +++ b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java @@ -25,6 +25,7 @@ import java.util.Random; import java.util.Set; import java.util.function.BooleanSupplier; +import java.util.regex.Pattern; import java.util.stream.Collectors; import org.apache.storm.Config; @@ -42,6 +43,7 @@ public class ConfigUtils { public static final double RFC1889_ALPHA = 1.0 / 16.0; private static final Set passwordConfigKeys = new HashSet<>(); + private static final Pattern CREDENTIAL_KEY_NAME = Pattern.compile("(?i)(password|passwd|secret)"); static { for (Class clazz : ConfigValidation.getConfigClasses()) { @@ -89,6 +91,29 @@ public Object transformEntry(String key, Object value) { return Maps.transformEntries(conf, maskPasswords); } + /** + * Mask credential values before a config map is served over an API. This covers what + * {@link #maskPasswords(Map)} covers, plus string values whose key name denotes a secret: plugins read their + * own keys straight out of the config map, so those keys are declared by no annotated field and the annotation + * scan cannot see them. Only string values are considered, so timeouts and class lists whose names merely + * mention credentials keep their value. + * + * @param conf the config to mask + * @return a view of the config with credential values replaced + */ + public static Map maskCredentials(final Map conf) { + Maps.EntryTransformer maskCredentials = new Maps.EntryTransformer() { + @Override + public Object transformEntry(String key, Object value) { + if (passwordConfigKeys.contains(key)) { + return "*****"; + } + return value instanceof String && CREDENTIAL_KEY_NAME.matcher(key).find() ? "*****" : value; + } + }; + return Maps.transformEntries(conf, maskCredentials); + } + public static boolean isLocalMode(Map conf) { String mode = (String) conf.get(Config.STORM_CLUSTER_MODE); if (mode != null) { diff --git a/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java b/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java index 7a5bd14f5f6..3a2c12189f1 100644 --- a/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java +++ b/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java @@ -195,4 +195,102 @@ public void upstreamFeedbackFreqSecs_rejectsNegative() { assertThrows(IllegalArgumentException.class, () -> ConfigUtils.upstreamFeedbackFreqSecs( mockMap(Config.TOPOLOGY_UPSTREAM_FEEDBACK_FREQ_SECS, -1))); } + + @Test + public void maskPasswords_masksClusterZookeeperCredentials() { + Map conf = new HashMap<>(); + conf.put(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD, "zk-user:zk-secret"); + conf.put(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD, "topo-user:topo-secret"); + + Map masked = ConfigUtils.maskPasswords(conf); + + assertEquals("*****", masked.get(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD)); + assertEquals("*****", masked.get(Config.STORM_ZOOKEEPER_TOPOLOGY_AUTH_PAYLOAD)); + } + + @Test + public void maskPasswords_masksThriftTlsStorePasswords() { + Map conf = new HashMap<>(); + conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD, "nimbus-ks"); + conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD, "nimbus-ts"); + conf.put(Config.NIMBUS_THRIFT_TLS_CLIENT_KEYSTORE_PASSWORD, "client-ks"); + conf.put(Config.NIMBUS_THRIFT_TLS_CLIENT_TRUSTSTORE_PASSWORD, "client-ts"); + conf.put(Config.SUPERVISOR_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD, "sup-ks"); + conf.put(Config.SUPERVISOR_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD, "sup-ts"); + + Map masked = ConfigUtils.maskPasswords(conf); + + assertEquals("*****", masked.get(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.NIMBUS_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.NIMBUS_THRIFT_TLS_CLIENT_KEYSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.NIMBUS_THRIFT_TLS_CLIENT_TRUSTSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.SUPERVISOR_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.SUPERVISOR_THRIFT_TLS_SERVER_TRUSTSTORE_PASSWORD)); + } + + @Test + public void maskPasswords_masksZookeeperAndNettyTlsStorePasswords() { + Map conf = new HashMap<>(); + conf.put(Config.STORM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD, "zk-ks"); + conf.put(Config.STORM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD, "zk-ts"); + conf.put(Config.STORM_MESSAGING_NETTY_TLS_KEYSTORE_PASSWORD, "netty-ks"); + conf.put(Config.STORM_MESSAGING_NETTY_TLS_TRUSTSTORE_PASSWORD, "netty-ts"); + + Map masked = ConfigUtils.maskPasswords(conf); + + assertEquals("*****", masked.get(Config.STORM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.STORM_ZOOKEEPER_SSL_TRUSTSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.STORM_MESSAGING_NETTY_TLS_KEYSTORE_PASSWORD)); + assertEquals("*****", masked.get(Config.STORM_MESSAGING_NETTY_TLS_TRUSTSTORE_PASSWORD)); + } + + @Test + public void maskCredentials_masksKeysThatOnlyPluginsDeclare() { + Map conf = new HashMap<>(); + conf.put("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_password", "plugin-secret"); + conf.put("storm.zookeeper.auth.password", "zk-pass"); + conf.put("some.plugin.shared_secret", "shared"); + + Map masked = ConfigUtils.maskCredentials(conf); + + assertEquals("*****", masked.get("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_password")); + assertEquals("*****", masked.get("storm.zookeeper.auth.password")); + assertEquals("*****", masked.get("some.plugin.shared_secret")); + } + + @Test + public void maskCredentials_masksTheAnnotatedKeysAsWell() { + Map conf = new HashMap<>(); + conf.put(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD, "zk-user:zk-secret"); + + assertEquals("*****", ConfigUtils.maskCredentials(conf).get(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD)); + } + + @Test + public void maskCredentials_leavesNonStringValuesAlone() { + Map conf = new HashMap<>(); + conf.put("task.credentials.poll.secs", 30); + conf.put("nimbus.credential.renewers.freq.secs", 600); + conf.put("topology.auto-credentials", Collections.singletonList("org.example.AutoCreds")); + conf.put("nimbus.seeds", Collections.singletonList("nimbus1")); + + Map masked = ConfigUtils.maskCredentials(conf); + + assertEquals(30, masked.get("task.credentials.poll.secs")); + assertEquals(600, masked.get("nimbus.credential.renewers.freq.secs")); + assertEquals(Collections.singletonList("org.example.AutoCreds"), masked.get("topology.auto-credentials")); + assertEquals(Collections.singletonList("nimbus1"), masked.get("nimbus.seeds")); + } + + @Test + public void maskPasswords_keepsOrdinaryValues() { + Map conf = new HashMap<>(); + conf.put(Config.STORM_ZOOKEEPER_SERVERS, Collections.singletonList("zk1")); + conf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PATH, "/etc/storm/nimbus.jks"); + + Map masked = ConfigUtils.maskPasswords(conf); + + assertEquals(Collections.singletonList("zk1"), masked.get(Config.STORM_ZOOKEEPER_SERVERS)); + assertEquals("/etc/storm/nimbus.jks", masked.get(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PATH)); + } } diff --git a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java index 4469f8dfab1..1698a5ce855 100644 --- a/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java +++ b/storm-server/src/main/java/org/apache/storm/daemon/nimbus/Nimbus.java @@ -4468,7 +4468,7 @@ public String getNimbusConf() throws AuthorizationException, TException { try { getNimbusConfCalls.mark(); checkAuthorization(null, null, "getNimbusConf"); - return JSONValue.toJSONString(conf); + return JSONValue.toJSONString(ConfigUtils.maskCredentials(conf)); } catch (Exception e) { LOG.warn("get nimbus conf exception.", e); if (e instanceof TException) { diff --git a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetNimbusConfTest.java b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetNimbusConfTest.java new file mode 100644 index 00000000000..c348697c8de --- /dev/null +++ b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusGetNimbusConfTest.java @@ -0,0 +1,72 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.storm.daemon.nimbus; + +import java.util.HashMap; +import java.util.Map; + +import net.minidev.json.JSONValue; +import org.apache.storm.Config; +import org.apache.storm.DaemonConfig; +import org.apache.storm.LocalCluster; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +public class NimbusGetNimbusConfTest { + + private static final String MASKED = "*****"; + + @SuppressWarnings("unchecked") + private static Map parse(String json) { + return (Map) JSONValue.parse(json); + } + + @Test + public void getNimbusConfMasksCredentialsAndKeepsOtherValues() throws Exception { + Map daemonConf = new HashMap<>(); + daemonConf.put(DaemonConfig.NIMBUS_AUTHORIZER, "org.apache.storm.security.auth.authorizer.NoopAuthorizer"); + daemonConf.put(DaemonConfig.SUPERVISOR_AUTHORIZER, "org.apache.storm.security.auth.authorizer.NoopAuthorizer"); + daemonConf.put(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD, "zk-user:zk-secret"); + daemonConf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD, "nimbus-keystore-secret"); + daemonConf.put(DaemonConfig.UI_HTTPS_KEYSTORE_PASSWORD, "ui-keystore-secret"); + daemonConf.put(Config.STORM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD, "zk-ssl-keystore-secret"); + daemonConf.put("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_password", "plugin-secret"); + daemonConf.put(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PATH, "/etc/storm/nimbus.jks"); + + try (LocalCluster cluster = new LocalCluster.Builder().withDaemonConf(daemonConf).build()) { + Map served = parse(cluster.getNimbus().getNimbusConf()); + + assertEquals(MASKED, served.get(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD), + "the cluster ZooKeeper auth payload should be masked"); + assertEquals(MASKED, served.get(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PASSWORD), + "thrift TLS store passwords should be masked"); + assertEquals(MASKED, served.get(DaemonConfig.UI_HTTPS_KEYSTORE_PASSWORD), + "UI keystore passwords should be masked"); + + assertEquals(MASKED, served.get(Config.STORM_ZOOKEEPER_SSL_KEYSTORE_PASSWORD), + "ZooKeeper TLS store passwords should be masked"); + assertEquals(MASKED, served.get("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_password"), + "credential keys that only a plugin declares should be masked too"); + + assertEquals("/etc/storm/nimbus.jks", served.get(Config.NIMBUS_THRIFT_TLS_SERVER_KEYSTORE_PATH), + "non-credential values should be served unchanged"); + } + } +} diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilter.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilter.java index 6dd17755f06..f26175892af 100644 --- a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilter.java +++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilter.java @@ -27,6 +27,7 @@ import jakarta.ws.rs.ext.Provider; import java.io.IOException; import java.io.InputStream; +import java.lang.reflect.Method; import java.net.InetAddress; import java.security.Principal; import java.util.Map; @@ -39,6 +40,7 @@ import org.apache.storm.daemon.ui.UIHelpers; import org.apache.storm.daemon.ui.resources.AuthNimbusOp; import org.apache.storm.daemon.ui.resources.StormApiResource; +import org.apache.storm.daemon.ui.resources.UnauthenticatedNimbusOp; import org.apache.storm.generated.AuthorizationException; import org.apache.storm.security.auth.IAuthorizer; import org.apache.storm.security.auth.ReqContext; @@ -100,12 +102,26 @@ public static Response makeResponse(Exception ex, ContainerRequestContext reques @Override public void filter(ContainerRequestContext containerRequestContext) { - AuthNimbusOp annotation = resourceInfo.getResourceMethod().getAnnotation(AuthNimbusOp.class); + Method resourceMethod = resourceInfo.getResourceMethod(); + AuthNimbusOp annotation = resourceMethod.getAnnotation(AuthNimbusOp.class); if (annotation == null) { + if (resourceMethod.getAnnotation(UnauthenticatedNimbusOp.class) != null) { + return; + } + LOG.error("Endpoint {}.{} declares no authorization; rejecting the request.", + resourceMethod.getDeclaringClass().getName(), resourceMethod.getName()); + containerRequestContext.abortWith( + makeResponse(new AuthorizationException("UI request is not authorized"), + containerRequestContext, 403) + ); return; } String op = annotation.value(); if (op == null) { + containerRequestContext.abortWith( + makeResponse(new AuthorizationException("UI request is not authorized"), + containerRequestContext, 403) + ); return; } diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/StormApiResource.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/StormApiResource.java index 62b46199293..c84f86f8b98 100644 --- a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/StormApiResource.java +++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/StormApiResource.java @@ -107,6 +107,7 @@ public StormApiResource(StormMetricsRegistry metricsRegistry) { @GET @Path("/cluster/configuration") + @AuthNimbusOp("getNimbusConf") @Produces("application/json") public Response getClusterConfiguration(@QueryParam(callbackParameterName) String callback) throws TException { clusterConfigurationRequestMeter.mark(); @@ -198,6 +199,7 @@ public Response getOwnerResource(@PathParam("id") String id, */ @GET @Path("/history/summary") + @UnauthenticatedNimbusOp("Nimbus filters the history for the authenticated remote user") @Produces("application/json") public Response getHistorySummary(@QueryParam(callbackParameterName) String callback) throws TException { try (NimbusClient nimbusClient = NimbusClient.Builder.withConf(config).build()) { diff --git a/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/UnauthenticatedNimbusOp.java b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/UnauthenticatedNimbusOp.java new file mode 100644 index 00000000000..21024c82f8b --- /dev/null +++ b/storm-webapp/src/main/java/org/apache/storm/daemon/ui/resources/UnauthenticatedNimbusOp.java @@ -0,0 +1,39 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.storm.daemon.ui.resources; + +import java.lang.annotation.Documented; +import java.lang.annotation.Retention; +import java.lang.annotation.RetentionPolicy; + +/** + * Marks an endpoint that deliberately carries no {@link AuthNimbusOp} gate, because the data it + * returns is already restricted for the requesting user further down the call chain. Endpoints + * without either annotation are rejected. + */ +@Documented +@Retention(value = RetentionPolicy.RUNTIME) +public @interface UnauthenticatedNimbusOp { + /** + * Why this endpoint needs no operation gate. + * + * @return the justification + */ + String value(); +} diff --git a/storm-webapp/src/test/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilterTest.java b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilterTest.java new file mode 100644 index 00000000000..5e9e2c5c1de --- /dev/null +++ b/storm-webapp/src/test/java/org/apache/storm/daemon/ui/filters/AuthorizedUserFilterTest.java @@ -0,0 +1,188 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.storm.daemon.ui.filters; + +import jakarta.ws.rs.container.ContainerRequestContext; +import jakarta.ws.rs.container.ResourceInfo; +import jakarta.ws.rs.core.Response; +import java.lang.reflect.Field; +import java.lang.reflect.Method; +import java.util.Map; +import org.apache.storm.daemon.ui.resources.AuthNimbusOp; +import org.apache.storm.daemon.ui.resources.UnauthenticatedNimbusOp; +import org.apache.storm.security.auth.IAuthorizer; +import org.apache.storm.security.auth.ReqContext; +import org.junit.jupiter.api.Test; +import org.mockito.ArgumentCaptor; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class AuthorizedUserFilterTest { + + /** + * Stand-in for a resource class: one endpoint gated by an operation, one explicitly opted out, + * and one carrying no annotation at all. + */ + public static class SampleResource { + @AuthNimbusOp("getNimbusConf") + public void gated() { + } + + @UnauthenticatedNimbusOp("filtered by Nimbus using the authenticated remote user") + public void optedOut() { + } + + public void unannotated() { + } + } + + private static AuthorizedUserFilter filterFor(String methodName, IAuthorizer aclHandler) throws Exception { + Method method = SampleResource.class.getMethod(methodName); + ResourceInfo resourceInfo = mock(ResourceInfo.class); + when(resourceInfo.getResourceMethod()).thenReturn(method); + + AuthorizedUserFilter filter = new AuthorizedUserFilter(); + Field field = AuthorizedUserFilter.class.getDeclaredField("resourceInfo"); + field.setAccessible(true); + field.set(filter, resourceInfo); + + AuthorizedUserFilter.uiAclHandler = aclHandler; + AuthorizedUserFilter.uiImpersonationHandler = null; + return filter; + } + + private static int statusOfAbort(ContainerRequestContext request) { + ArgumentCaptor response = ArgumentCaptor.forClass(Response.class); + verify(request).abortWith(response.capture()); + return response.getValue().getStatus(); + } + + @Test + public void unannotatedEndpointIsDenied() throws Exception { + IAuthorizer aclHandler = mock(IAuthorizer.class); + when(aclHandler.permit(any(), any(), any())).thenReturn(true); + AuthorizedUserFilter filter = filterFor("unannotated", aclHandler); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + assertEquals(403, statusOfAbort(request)); + verify(aclHandler, never()).permit(any(), any(), any()); + } + + @Test + public void annotatedEndpointIsCheckedAgainstItsOperation() throws Exception { + IAuthorizer aclHandler = mock(IAuthorizer.class); + when(aclHandler.permit(any(ReqContext.class), eq("getNimbusConf"), any())).thenReturn(false); + AuthorizedUserFilter filter = filterFor("gated", aclHandler); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + assertEquals(403, statusOfAbort(request)); + verify(aclHandler).permit(any(ReqContext.class), eq("getNimbusConf"), any()); + } + + @Test + public void permittedEndpointIsNotAborted() throws Exception { + IAuthorizer aclHandler = mock(IAuthorizer.class); + when(aclHandler.permit(any(ReqContext.class), eq("getNimbusConf"), any())).thenReturn(true); + AuthorizedUserFilter filter = filterFor("gated", aclHandler); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + verify(request, never()).abortWith(any()); + } + + @Test + public void explicitlyOptedOutEndpointSkipsTheAclHandler() throws Exception { + IAuthorizer aclHandler = mock(IAuthorizer.class); + AuthorizedUserFilter filter = filterFor("optedOut", aclHandler); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + verify(request, never()).abortWith(any()); + verify(aclHandler, never()).permit(any(), any(), any()); + } + + @Test + public void unannotatedEndpointIsDeniedEvenWithoutAnAclHandler() throws Exception { + AuthorizedUserFilter filter = filterFor("unannotated", null); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + assertEquals(403, statusOfAbort(request)); + } + + @Test + public void everyApiEndpointDeclaresItsAuthorization() throws Exception { + Class resource = Class.forName("org.apache.storm.daemon.ui.resources.StormApiResource"); + for (Method method : resource.getDeclaredMethods()) { + if (method.getAnnotation(jakarta.ws.rs.GET.class) == null + && method.getAnnotation(jakarta.ws.rs.POST.class) == null) { + continue; + } + boolean declared = method.getAnnotation(AuthNimbusOp.class) != null + || method.getAnnotation(UnauthenticatedNimbusOp.class) != null; + assertEquals(true, declared, + method.getName() + " must declare @AuthNimbusOp or @UnauthenticatedNimbusOp"); + } + } + + @Test + public void clusterConfigurationRequiresTheNimbusConfOperation() throws Exception { + Method method = Class.forName("org.apache.storm.daemon.ui.resources.StormApiResource") + .getMethod("getClusterConfiguration", String.class); + + AuthNimbusOp annotation = method.getAnnotation(AuthNimbusOp.class); + assertEquals("getNimbusConf", annotation == null ? null : annotation.value()); + } + + @Test + public void topologyIdLookupIsSkippedForOptedOutEndpoints() throws Exception { + AuthorizedUserFilter filter = filterFor("optedOut", mock(IAuthorizer.class)); + ContainerRequestContext request = mock(ContainerRequestContext.class); + + filter.filter(request); + + verify(request, never()).getUriInfo(); + } + + @Test + public void aclHandlerReceivesNullTopologyConfForNonTopologyOperations() throws Exception { + IAuthorizer aclHandler = mock(IAuthorizer.class); + when(aclHandler.permit(any(), any(), any())).thenReturn(true); + AuthorizedUserFilter filter = filterFor("gated", aclHandler); + + filter.filter(mock(ContainerRequestContext.class)); + + ArgumentCaptor topoConf = ArgumentCaptor.forClass(Map.class); + verify(aclHandler).permit(any(ReqContext.class), eq("getNimbusConf"), topoConf.capture()); + assertEquals(null, topoConf.getValue()); + } +} From 54259ff7ea903acdc4cffe4df15420f724878b90 Mon Sep 17 00:00:00 2001 From: Gianluca Graziadei Date: Sun, 23 Aug 2026 14:17:52 +0200 Subject: [PATCH 2/2] Prevent credential mask from overriding local config --- .../jvm/org/apache/storm/utils/ConfigUtils.java | 12 ++++++++++++ .../org/apache/storm/utils/ConfigUtilsTest.java | 16 ++++++++++++++++ .../apache/storm/command/UploadCredentials.java | 4 ++++ 3 files changed, 32 insertions(+) diff --git a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java index 03fb37fd87c..c78b38ff58d 100644 --- a/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java +++ b/storm-client/src/jvm/org/apache/storm/utils/ConfigUtils.java @@ -114,6 +114,18 @@ public Object transformEntry(String key, Object value) { return Maps.transformEntries(conf, maskCredentials); } + /** + * Whether a config key holds a credential, and therefore whether {@link #maskCredentials(Map)} would replace its + * value. Callers that read a config back from a daemon use this to tell which entries carry no usable value and + * must be taken from their own configuration instead. + * + * @param key the config key + * @return true when the key denotes a credential + */ + public static boolean isCredentialKey(String key) { + return passwordConfigKeys.contains(key) || CREDENTIAL_KEY_NAME.matcher(key).find(); + } + public static boolean isLocalMode(Map conf) { String mode = (String) conf.get(Config.STORM_CLUSTER_MODE); if (mode != null) { diff --git a/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java b/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java index 3a2c12189f1..9897eb75b8b 100644 --- a/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java +++ b/storm-client/test/jvm/org/apache/storm/utils/ConfigUtilsTest.java @@ -282,6 +282,22 @@ public void maskCredentials_leavesNonStringValuesAlone() { assertEquals(Collections.singletonList("nimbus1"), masked.get("nimbus.seeds")); } + @Test + public void isCredentialKey_recognisesAnnotatedAndPluginDeclaredKeys() { + assertTrue(ConfigUtils.isCredentialKey(Config.STORM_ZOOKEEPER_AUTH_PAYLOAD)); + assertTrue(ConfigUtils.isCredentialKey(Config.NIMBUS_THRIFT_TLS_CLIENT_KEYSTORE_PASSWORD)); + assertTrue(ConfigUtils.isCredentialKey("storm.daemon.metrics.reporter.plugin.prometheus.basic_auth_password")); + assertTrue(ConfigUtils.isCredentialKey("some.plugin.shared_secret")); + } + + @Test + public void isCredentialKey_ignoresKeysThatOnlyMentionCredentials() { + assertFalse(ConfigUtils.isCredentialKey("task.credentials.poll.secs")); + assertFalse(ConfigUtils.isCredentialKey(Config.TOPOLOGY_AUTO_CREDENTIALS)); + assertFalse(ConfigUtils.isCredentialKey(Config.NIMBUS_THRIFT_TLS_CLIENT_KEYSTORE_PATH)); + assertFalse(ConfigUtils.isCredentialKey(Config.TOPOLOGY_NAME)); + } + @Test public void maskPasswords_keepsOrdinaryValues() { Map conf = new HashMap<>(); diff --git a/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java b/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java index d6891721751..12d76cefebb 100644 --- a/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java +++ b/storm-core/src/jvm/org/apache/storm/command/UploadCredentials.java @@ -24,6 +24,7 @@ import org.apache.storm.StormSubmitter; import org.apache.storm.generated.Nimbus; import org.apache.storm.generated.TopologySummary; +import org.apache.storm.utils.ConfigUtils; import org.apache.storm.utils.NimbusClient; import org.apache.storm.utils.Utils; import org.slf4j.Logger; @@ -113,6 +114,9 @@ public static void main(String[] args) throws Exception { */ topologyConf.remove("java.security.auth.login.config"); topologyConf.remove(Config.NIMBUS_THRIFT_CLIENT_USE_TLS); + // Nimbus masks credentials before serving a conf, so these entries hold no usable value here. + // Dropping them lets the client's own configuration supply them, e.g. TLS store passwords. + topologyConf.keySet().removeIf(ConfigUtils::isCredentialKey); boolean throwExceptionForEmptyCreds = (boolean) cl.get("e"); boolean hasCreds = StormSubmitter.pushCredentials(topologyName, topologyConf, credentialsMap, (String) cl.get("u"));