From 7cf643cf6dc0e1446b7317ca2bc940f864ac9026 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Thu, 3 Jul 2025 13:34:57 +0000 Subject: [PATCH 1/3] Adds root permissions test Adds a test to verify that the root user is unable to scan a user created table in a namespace where the root user has SYSTEM level permissions. --- .../test/functional/PermissionsIT.java | 53 +++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java index c36becd6351..d2badbba6f5 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java @@ -22,6 +22,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -33,6 +34,7 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; +import java.util.stream.Collectors; import org.apache.accumulo.cluster.ClusterUser; import org.apache.accumulo.core.client.Accumulo; @@ -47,12 +49,14 @@ import org.apache.accumulo.core.client.security.tokens.AuthenticationToken; import org.apache.accumulo.core.client.security.tokens.PasswordToken; import org.apache.accumulo.core.client.summary.Summary; +import org.apache.accumulo.core.clientImpl.Namespace; import org.apache.accumulo.core.conf.Property; import org.apache.accumulo.core.data.Key; import org.apache.accumulo.core.data.Mutation; import org.apache.accumulo.core.data.Value; import org.apache.accumulo.core.metadata.MetadataTable; import org.apache.accumulo.core.security.Authorizations; +import org.apache.accumulo.core.security.NamespacePermission; import org.apache.accumulo.core.security.SystemPermission; import org.apache.accumulo.core.security.TablePermission; import org.apache.accumulo.harness.AccumuloClusterHarness; @@ -725,6 +729,55 @@ public void tablePermissionTest() throws Exception { } } + @Test + public void rootUserTablePermissionTest() throws Exception { + // create the test user + ClusterUser testUser = getUser(0); + ClusterUser rootUser = getAdminUser(); + + String principal = testUser.getPrincipal(); + AuthenticationToken token = testUser.getToken(); + PasswordToken passwordToken = null; + if (token instanceof PasswordToken) { + passwordToken = (PasswordToken) token; + } + loginAs(rootUser); + try (AccumuloClient c = Accumulo.newClient().from(getClientProps()).build()) { + c.securityOperations().createLocalUser(principal, passwordToken); + loginAs(testUser); + try (AccumuloClient test_user_client = + Accumulo.newClient().from(c.properties()).as(principal, token).build()) { + + String tableName = getUniqueNames(1)[0] + "__TABLE_READ_PERMISSION_TEST__"; + // Allow test user to create a table in default namespace + loginAs(rootUser); + c.securityOperations().grantNamespacePermission(testUser.getPrincipal(), + Namespace.DEFAULT.name(), NamespacePermission.CREATE_TABLE); + + // create the test table + test_user_client.tableOperations().create(tableName); + // put in some initial data + try (BatchWriter writer = test_user_client.createBatchWriter(tableName)) { + Mutation m = new Mutation(new Text("row")); + m.put("cf", "cq", "val"); + writer.addMutation(m); + } + + // Attempt to scan table as test user + try (Scanner scanner = test_user_client.createScanner(tableName, Authorizations.EMPTY)) { + for (Entry keyValueEntry : scanner) { + assertNotNull(keyValueEntry); + } + } + + Scanner scanner = c.createScanner(tableName, Authorizations.EMPTY); + // Attempt to scan table as root user + assertThrows(RuntimeException.class, () -> scanner.stream().collect(Collectors.toList()), + "Error PERMISSION_DENIED"); + } + } + } + private void createTestTable(AccumuloClient c, String testUser, String tableName) throws Exception { if (!c.tableOperations().exists(tableName)) { From a1f42e723db92eab99e3652f74ec706b6d1d7b78 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Tue, 1 Jul 2025 17:01:36 +0000 Subject: [PATCH 2/3] Reduce unnecessary permissions modification When a table is created by a user that has the SYSTEM permission there is no need to add additional per-table permissions to that user Likewise, if the user has the SYSTEM permission, then table actions should be allowed. --- .../accumulo/server/security/SecurityOperation.java | 3 ++- .../manager/tableOps/create/SetupPermissions.java | 8 ++++++-- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java index 45e665c8823..bb9ae24601d 100644 --- a/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java +++ b/server/base/src/main/java/org/apache/accumulo/server/security/SecurityOperation.java @@ -342,7 +342,8 @@ private boolean _hasSystemPermission(String user, SystemPermission permission, b private boolean hasTablePermission(TCredentials credentials, TableId tableId, NamespaceId namespaceId, TablePermission permission, boolean useCached) throws ThriftSecurityException { - if (isSystemUser(credentials)) { + if (isSystemUser(credentials) + || hasSystemPermission(credentials, SystemPermission.SYSTEM, false)) { return true; } return _hasTablePermission(credentials.getPrincipal(), tableId, permission, useCached) diff --git a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java index 49ff9f2914f..97cab4a59d2 100644 --- a/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java +++ b/server/manager/src/main/java/org/apache/accumulo/manager/tableOps/create/SetupPermissions.java @@ -20,6 +20,7 @@ import org.apache.accumulo.core.clientImpl.thrift.ThriftSecurityException; import org.apache.accumulo.core.fate.Repo; +import org.apache.accumulo.core.security.SystemPermission; import org.apache.accumulo.core.security.TablePermission; import org.apache.accumulo.manager.Manager; import org.apache.accumulo.manager.tableOps.ManagerRepo; @@ -38,9 +39,12 @@ class SetupPermissions extends ManagerRepo { @Override public Repo call(long tid, Manager env) throws Exception { - // give all table permissions to the creator + // give all table permissions to the creator if that creator is not the system user or has + // SYSTEM level permissions var security = env.getContext().getSecurityOperation(); - if (!tableInfo.getUser().equals(env.getContext().getCredentials().getPrincipal())) { + if (!tableInfo.getUser().equals(env.getContext().getCredentials().getPrincipal()) + && !security.hasSystemPermission(env.getContext().rpcCreds(), tableInfo.getUser(), + SystemPermission.SYSTEM)) { for (TablePermission permission : TablePermission.values()) { try { security.grantTablePermission(env.getContext().rpcCreds(), tableInfo.getUser(), From bcc0f7eee2909cce37497c79bd859048f988eac6 Mon Sep 17 00:00:00 2001 From: Daniel Roberts ddanielr Date: Thu, 3 Jul 2025 13:44:28 +0000 Subject: [PATCH 3/3] update test to pass --- .../apache/accumulo/test/functional/PermissionsIT.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java index d2badbba6f5..0cb70bdb4ba 100644 --- a/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java +++ b/test/src/main/java/org/apache/accumulo/test/functional/PermissionsIT.java @@ -22,7 +22,6 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assumptions.assumeTrue; @@ -34,7 +33,6 @@ import java.util.Map; import java.util.Map.Entry; import java.util.Set; -import java.util.stream.Collectors; import org.apache.accumulo.cluster.ClusterUser; import org.apache.accumulo.core.client.Accumulo; @@ -770,10 +768,12 @@ public void rootUserTablePermissionTest() throws Exception { } } - Scanner scanner = c.createScanner(tableName, Authorizations.EMPTY); // Attempt to scan table as root user - assertThrows(RuntimeException.class, () -> scanner.stream().collect(Collectors.toList()), - "Error PERMISSION_DENIED"); + try (Scanner scanner = c.createScanner(tableName, Authorizations.EMPTY)) { + for (Entry keyValueEntry : scanner) { + assertNotNull(keyValueEntry); + } + } } } }