From 67030409e82326126ec2d2da6f567d4c8203aaa3 Mon Sep 17 00:00:00 2001 From: Richard Zowalla Date: Sat, 22 Aug 2026 20:32:28 +0200 Subject: [PATCH] Apply the nimbus ACLs to listBlobs so callers only see blobs they may read --- .../auth/authorizer/SimpleACLAuthorizer.java | 1 + .../authorizer/SimpleACLAuthorizerTest.java | 5 ++++ .../apache/storm/daemon/nimbus/Nimbus.java | 13 ++++++++-- .../storm/daemon/nimbus/NimbusTest.java | 26 +++++++++++++++++++ 4 files changed, 43 insertions(+), 2 deletions(-) diff --git a/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java b/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java index 1e088752f72..aaca461bed0 100644 --- a/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java +++ b/storm-client/src/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizer.java @@ -39,6 +39,7 @@ public class SimpleACLAuthorizer implements IAuthorizer { "submitTopology", "fileUpload", "getNimbusConf", + "listBlobs", "getClusterInfo", "getLeader", "isTopologyNameAllowed", diff --git a/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java b/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java index 15f4b9bbeed..2091a781214 100644 --- a/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java +++ b/storm-client/test/jvm/org/apache/storm/security/auth/authorizer/SimpleACLAuthorizerTest.java @@ -68,6 +68,11 @@ public void SimpleACLUserAuthTest() { assertTrue(authorizer.permit(new ReqContext(userA), "getNimbusConf", new HashMap<>())); assertTrue(authorizer.permit(new ReqContext(userB), "getNimbusConf", new HashMap<>())); + assertTrue(authorizer.permit(new ReqContext(adminUser), "listBlobs", new HashMap<>())); + assertFalse(authorizer.permit(new ReqContext(supervisorUser), "listBlobs", new HashMap<>())); + assertTrue(authorizer.permit(new ReqContext(userA), "listBlobs", new HashMap<>())); + assertTrue(authorizer.permit(new ReqContext(userB), "listBlobs", new HashMap<>())); + assertTrue(authorizer.permit(new ReqContext(adminUser), "getClusterInfo", new HashMap<>())); assertFalse(authorizer.permit(new ReqContext(supervisorUser), "getClusterInfo", new HashMap<>())); assertTrue(authorizer.permit(new ReqContext(userA), "getClusterInfo", new HashMap<>())); 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 ecf7dd2a24e..db1f2e52a29 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 @@ -4067,6 +4067,7 @@ public void deleteBlob(String key) throws AuthorizationException, KeyNotFoundExc @Override public ListBlobsResult listBlobs(String session) throws TException { try { + checkAuthorization(null, null, "listBlobs"); Iterator keyIt; //Create a new session id if the user gave an empty session string. // This is the use case when the user wishes to list blobs @@ -4089,9 +4090,17 @@ public ListBlobsResult listBlobs(String session) throws TException { return new ListBlobsResult(Collections.emptyList(), session); } + Subject who = getSubject(); ArrayList listChunk = new ArrayList<>(); - for (int i = 0; i < 100 && keyIt.hasNext(); i++) { - listChunk.add(keyIt.next()); + while (listChunk.size() < 100 && keyIt.hasNext()) { + String key = keyIt.next(); + //Only list the blobs whose metadata the caller may read, the same check getBlobMeta does. + try { + blobStore.getBlobMeta(key, who); + listChunk.add(key); + } catch (AuthorizationException | KeyNotFoundException e) { + LOG.debug("Not listing blob {} for {}", key, who); + } } blobListers.put(session, keyIt); LOG.info("Downloading {} entries", listChunk.size()); diff --git a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java index 2380d49a809..3429fdd3172 100644 --- a/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java +++ b/storm-server/src/test/java/org/apache/storm/daemon/nimbus/NimbusTest.java @@ -23,6 +23,7 @@ import java.nio.file.Paths; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Set; @@ -36,6 +37,7 @@ import org.apache.storm.generated.AuthorizationException; import org.apache.storm.generated.InvalidTopologyException; import org.apache.storm.generated.KeyNotFoundException; +import org.apache.storm.generated.ListBlobsResult; import org.apache.storm.generated.StormTopology; import org.apache.storm.metric.StormMetricsRegistry; import org.apache.storm.nimbus.ILeaderElector; @@ -46,11 +48,13 @@ import org.apache.storm.scheduler.resource.strategies.scheduling.GenericResourceAwareStrategyOld; import org.apache.storm.scheduler.resource.strategies.scheduling.RoundRobinResourceAwareStrategy; import org.apache.storm.security.auth.IGroupMappingServiceProvider; +import org.apache.storm.security.auth.authorizer.DenyAuthorizer; import org.apache.storm.testing.TestWordSpout; import org.apache.storm.thrift.TException; import org.apache.storm.topology.TopologyBuilder; import org.apache.storm.utils.ServerUtils; import org.apache.storm.utils.Time; +import org.apache.storm.utils.WrappedAuthorizationException; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.mockito.Mock; @@ -206,6 +210,28 @@ void testCreateStateInZookeeperWhenKeyNotFoundHandlesException() throws Exceptio } } + @Test + void testListBlobsOnlyReturnsKeysTheCallerMayReadTheMetadataOf() throws Exception { + when(localBlobStore.listKeys()).thenReturn(List.of("readable-key", "other-users-key").iterator()); + when(localBlobStore.getBlobMeta(eq("other-users-key"), any())) + .thenThrow(new WrappedAuthorizationException("not allowed")); + + ListBlobsResult result = nimbus.listBlobs(""); + + assertEquals(List.of("readable-key"), result.get_keys()); + } + + @Test + void testListBlobsIsAuthorized() throws Exception { + Map conf = Map.of(DaemonConfig.NIMBUS_MONITOR_FREQ_SECS, 10, + DaemonConfig.NIMBUS_AUTHORIZER, DenyAuthorizer.class.getName()); + nimbus = new Nimbus(conf, iNimbus, stormClusterState, nimbusInfo, localBlobStore, leaderElector, groupMapper, metricRegistry); + when(localBlobStore.listKeys()).thenReturn(List.of("readable-key").iterator()); + + assertThrows(AuthorizationException.class, () -> nimbus.listBlobs("")); + verify(localBlobStore, never()).listKeys(); + } + @Test void testValidateUploadedJarLocationRejectsLocationsOutsideTheInbox() throws Exception { Path inbox = Files.createTempDirectory("nimbus-inbox");