This is an automated email from the ASF dual-hosted git repository. maheshbandal15 pushed a commit to branch mb-RANGER-5825 in repository https://gitbox.apache.org/repos/asf/ranger.git
commit 6f795dd89ad59e686f06ffb12c303ab37177ab15 Author: Mahesh Bandal <[email protected]> AuthorDate: Wed Oct 14 17:19:11 2026 +0530 RANGER-5825: Fix HDFS authz optimization on directory rename --- .../authorization/hadoop/OperationOptimizer.java | 40 +++--- .../hadoop/RangerAccessControlEnforcer.java | 1 + .../services/hdfs/RangerHdfsAuthorizerTest.java | 142 ++++++++++++++++++++- 3 files changed, 162 insertions(+), 21 deletions(-) diff --git a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/OperationOptimizer.java b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/OperationOptimizer.java index 7f1522717..eb5915e6d 100644 --- a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/OperationOptimizer.java +++ b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/OperationOptimizer.java @@ -121,6 +121,30 @@ private OptimizedAuthzContext optimizeCreateOp() { } private OptimizedAuthzContext optimizeDeleteOp() { + return optimizeDirectoryTreeMutationOp(); + } + + private OptimizedAuthzContext optimizeRenameOp() { + if (inode != null && inode.isDirectory()) { + return optimizeDirectoryTreeMutationOp(); + } + + INode nodeToAuthorize = getINodeToAuthorize(); + + if (nodeToAuthorize == null) { + return OPT_BYPASS_AUTHZ; + } + + if (!nodeToAuthorize.isDirectory()) { + LOG.debug("nodeToCheck is not a directory for a rename operation! Optimization skipped"); + + return null; + } + + return getOrCreateOptimizedAuthzContext(); + } + + private OptimizedAuthzContext optimizeDirectoryTreeMutationOp() { int numOfRequestedAccesses = 0; if (ancestorAccess != null) { @@ -148,22 +172,6 @@ private OptimizedAuthzContext optimizeDeleteOp() { } } - private OptimizedAuthzContext optimizeRenameOp() { - INode nodeToAuthorize = getINodeToAuthorize(); - - if (nodeToAuthorize == null) { - return OPT_BYPASS_AUTHZ; - } - - if (!nodeToAuthorize.isDirectory()) { - LOG.debug("nodeToCheck is not a directory for a rename operation! Optimization skipped"); - - return null; - } - - return getOrCreateOptimizedAuthzContext(); - } - private OptimizedAuthzContext optimizeMkdirsOp() { INode nodeToAuthorize = getINodeToAuthorize(); diff --git a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerAccessControlEnforcer.java b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerAccessControlEnforcer.java index 924c4fc86..8efe1c978 100644 --- a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerAccessControlEnforcer.java +++ b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerAccessControlEnforcer.java @@ -787,6 +787,7 @@ Skipping this authorization check may break semantic equivalence (according to H Namenode calls this twice when renaming a file for source as well as target directories. For each directory, first invocation checks if the user has a EXECUTE access on the parent directory, and second invocation checks if the user has a WRITE access to the parent (or ancestor when checking target directory) . The optimized code combines these checks into a WRITE_EXECUTE access for the first invocation, and optimizes away the second call. + When the source is an existing directory, the first traverse-only call is optimized away and the subsequent substantive check is evaluated as WRITE_EXECUTE. 5. OPERATION_NAME_LISTSTATUS, OPERATION_NAME_GETEZFORPATH: Namenode calls this twice when listing a directory or getting the encryption zone for the directory. First invocation checks if the user has a EXECUTE access on the directory, and second checks if the user has a READ_EXECUTE access on the directory. The optimized code combines these checks into a READ_EXECUTE access diff --git a/hdfs-agent/src/test/java/org/apache/ranger/services/hdfs/RangerHdfsAuthorizerTest.java b/hdfs-agent/src/test/java/org/apache/ranger/services/hdfs/RangerHdfsAuthorizerTest.java index 2310988e7..0efe3ae2c 100644 --- a/hdfs-agent/src/test/java/org/apache/ranger/services/hdfs/RangerHdfsAuthorizerTest.java +++ b/hdfs-agent/src/test/java/org/apache/ranger/services/hdfs/RangerHdfsAuthorizerTest.java @@ -37,6 +37,7 @@ import java.io.File; import java.io.FileOutputStream; import java.io.OutputStreamWriter; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -46,11 +47,13 @@ * Direct tests for the {@link RangerHdfsAuthorizer} without going through the HDFS layer. */ public class RangerHdfsAuthorizerTest { - private static final int SNAPSHOT_ID = Snapshot.CURRENT_STATE_ID; - private static final String FILE_OWNER = "fileOwner"; - private static final String FILE_GROUP = "superGroup"; - private static final FsPermission READ_ONLY = new FsPermission(FsAction.READ, FsAction.NONE, FsAction.NONE); - private static final FsPermission EXEC_BY_OWNER = new FsPermission(FsAction.EXECUTE, FsAction.NONE, FsAction.NONE); + private static final int SNAPSHOT_ID = Snapshot.CURRENT_STATE_ID; + private static final String FILE_OWNER = "fileOwner"; + private static final String FILE_GROUP = "superGroup"; + private static final String OPERATION_NAME_DELETE = "delete"; + private static final String OPERATION_NAME_RENAME = "rename"; + private static final FsPermission READ_ONLY = new FsPermission(FsAction.READ, FsAction.NONE, FsAction.NONE); + private static final FsPermission EXEC_BY_OWNER = new FsPermission(FsAction.EXECUTE, FsAction.NONE, FsAction.NONE); private static RangerHdfsAuthorizer authorizer; private static AccessControlEnforcer rangerControlEnforcer; @@ -192,6 +195,62 @@ public void hdfsBaseFileNameTokenReadTest() throws AccessControlException { readFailWithPath("/tmp/tmpdir5/t/data-file.txt"); } + @Test + public void testOperationOptimizer_Rename_DirectorySourceEvaluated() throws Exception { + runWithAuthzOptimizationEnabled(true, () -> { + AccessControlEnforcer enforcer = authorizer.getExternalAccessControlEnforcer(null); + INode[] inodes = createPath("/tmp/srcdir", false, true); + + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, null, null, null, false, OPERATION_NAME_RENAME); + + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, OPERATION_NAME_RENAME)); + + // mv of a directory: no subAccess + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, null, false, OPERATION_NAME_RENAME)); + + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/tmpdir2/srcdir", false, true), "/tmp/tmpdir2/srcdir", null, FsAction.WRITE, null, null, false, OPERATION_NAME_RENAME); + }); + + runWithAuthzOptimizationEnabled(false, () -> { + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/srcdir", false, true), "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, OPERATION_NAME_RENAME)); + }); + } + + @Test + public void testOperationOptimizer_Rename_FileUnchanged() throws Exception { + runWithAuthzOptimizationEnabled(true, () -> { + AccessControlEnforcer enforcer = authorizer.getExternalAccessControlEnforcer(null); + + checkPermission(enforcer, "bob", createPath("/tmp/tmpdir2/data-file3", true, true), "/tmp/tmpdir2/data-file3", null, null, null, null, false, OPERATION_NAME_RENAME); + + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, null, null, null, false, OPERATION_NAME_RENAME)); + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, FsAction.WRITE, null, null, true, OPERATION_NAME_RENAME)); + }); + + runWithAuthzOptimizationEnabled(false, () -> { + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, FsAction.WRITE, null, null, true, OPERATION_NAME_RENAME)); + }); + } + + @Test + public void testOperationOptimizer_DeleteRegression() throws Exception { + runWithAuthzOptimizationEnabled(true, () -> { + AccessControlEnforcer enforcer = authorizer.getExternalAccessControlEnforcer(null); + INode[] inodes = createPath("/tmp/srcdir", false, true); + + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, null, null, null, false, OPERATION_NAME_DELETE); + + Assertions.assertThrows(AccessControlException.class, () -> + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, OPERATION_NAME_DELETE)); + }); + } + private void readFailWithPath(String path) { final TestFileSystem fs = new TestFileSystem(path); fs.setFilePermission(READ_ONLY); @@ -240,6 +299,79 @@ private static INode createNode(String[] pathSegments, int i, String owner, Stri return mock; } + private static INode[] createPath(String path, boolean file, boolean exists) { + String[] segments = path.split("/"); + INode[] ret = new INode[segments.length]; + + for (int i = 0; i < segments.length; i++) { + boolean isLast = i == segments.length - 1; + + ret[i] = isLast && !exists ? null : createNode(segments, i, FILE_OWNER, FILE_GROUP, isLast && file); + } + + return ret; + } + + private static void runWithAuthzOptimizationEnabled(boolean enabled, ThrowingRunnable runnable) throws Exception { + boolean previous = setAuthzOptimizationEnabled(enabled); + + try { + runnable.run(); + } finally { + setAuthzOptimizationEnabled(previous); + } + } + + private static boolean setAuthzOptimizationEnabled(boolean enabled) throws ReflectiveOperationException { + Field pluginField = RangerHdfsAuthorizer.class.getDeclaredField("rangerPlugin"); + Object plugin; + Field flagField; + boolean previous; + + pluginField.setAccessible(true); + plugin = pluginField.get(authorizer); + flagField = plugin.getClass().getDeclaredField("authzOptimizationEnabled"); + flagField.setAccessible(true); + previous = flagField.getBoolean(plugin); + flagField.setBoolean(plugin, enabled); + + return previous; + } + + private static void checkPermission(AccessControlEnforcer enforcer, String userName, INode[] inodes, String path, FsAction ancestorAccess, FsAction parentAccess, FsAction access, FsAction subAccess, boolean ignoreEmptyDir, String operationName) throws AccessControlException { + INodeAttributes[] attrs = new INodeAttributes[inodes.length]; + + for (int i = 0; i < inodes.length; i++) { + attrs[i] = inodes[i]; + } + + INodeAttributeProvider.AuthorizationContext.Builder builder = + new INodeAttributeProvider.AuthorizationContext.Builder() + .fsOwner(FILE_OWNER) + .supergroup(FILE_GROUP) + .callerUgi(UserGroupInformation.createUserForTesting(userName, new String[0])) + .inodeAttrs(attrs) + .inodes(inodes) + .pathByNameArr(new byte[0][0]) + .snapshotId(SNAPSHOT_ID) + .path(path) + .ancestorIndex(inodes.length - 2) + .doCheckOwner(false) + .ancestorAccess(ancestorAccess) + .parentAccess(parentAccess) + .access(access) + .subAccess(subAccess) + .ignoreEmptyDir(ignoreEmptyDir) + .operationName(operationName) + .callerContext(null); + + enforcer.checkPermissionWithContext(new INodeAttributeProvider.AuthorizationContext(builder)); + } + + private interface ThrowingRunnable { + void run() throws Exception; + } + static class TestFileSystem { final String path; final String[] pathSegments;
