This is an automated email from the ASF dual-hosted git repository. maheshbandal15 pushed a commit to branch mb-backport-RANGER-5825 in repository https://gitbox.apache.org/repos/asf/ranger.git
commit bf1c9c4938e4a670067749f06af1a2637fad8a51 Author: Mahesh Bandal <[email protected]> AuthorDate: Thu Oct 15 17:56:54 2026 +0530 RANGER-5825: Fix HDFS authz optimization on directory rename --- .../authorization/hadoop/RangerHdfsAuthorizer.java | 35 ++++-- .../services/hdfs/RangerHdfsAuthorizerTest.java | 133 +++++++++++++++++++++ 2 files changed, 155 insertions(+), 13 deletions(-) diff --git a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java index 3cb0245ef..3a2123045 100644 --- a/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java +++ b/hdfs-agent/src/main/java/org/apache/ranger/authorization/hadoop/RangerHdfsAuthorizer.java @@ -1019,6 +1019,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 @@ -1103,22 +1104,14 @@ private OptimizedAuthzContext optimizeCreateOp() { } private OptimizedAuthzContext optimizeDeleteOp() { - int numOfRequestedAccesses = 0; - - if (ancestorAccess != null) numOfRequestedAccesses++; - if (parentAccess != null) numOfRequestedAccesses++; - if (access != null) numOfRequestedAccesses++; - if (subAccess != null) numOfRequestedAccesses++; - - if (numOfRequestedAccesses == 0) { - return OPT_BYPASS_AUTHZ; - } else { - parentAccess = FsAction.WRITE_EXECUTE; - return getOrCreateOptimizedAuthzContext(); - } + return optimizeDirectoryTreeMutationOp(); } private OptimizedAuthzContext optimizeRenameOp() { + if (inode != null && inode.isDirectory()) { + return optimizeDirectoryTreeMutationOp(); + } + INode nodeToAuthorize = getINodeToAuthorize(); if (nodeToAuthorize == null) { @@ -1135,6 +1128,22 @@ private OptimizedAuthzContext optimizeRenameOp() { return getOrCreateOptimizedAuthzContext(); } + private OptimizedAuthzContext optimizeDirectoryTreeMutationOp() { + int numOfRequestedAccesses = 0; + + if (ancestorAccess != null) numOfRequestedAccesses++; + if (parentAccess != null) numOfRequestedAccesses++; + if (access != null) numOfRequestedAccesses++; + if (subAccess != null) numOfRequestedAccesses++; + + if (numOfRequestedAccesses == 0) { + return OPT_BYPASS_AUTHZ; + } else { + parentAccess = FsAction.WRITE_EXECUTE; + return getOrCreateOptimizedAuthzContext(); + } + } + private OptimizedAuthzContext optimizeMkdirsOp() { INode nodeToAuthorize = getINodeToAuthorize(); 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 dcf927dfc..6ef968494 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 @@ -23,6 +23,7 @@ import java.io.FileOutputStream; import java.io.OutputStreamWriter; import java.io.UnsupportedEncodingException; +import java.lang.reflect.Field; import java.nio.charset.StandardCharsets; import java.util.Arrays; @@ -302,6 +303,61 @@ public void HDFSBaseFileNameTokenReadTest() throws AccessControlException { readFailWithPath("/tmp/tmpdir5/t/data-file.txt"); } + @Test + public void testOperationOptimizerRenameDirectorySourceEvaluated() 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, RangerHdfsAuthorizer.OPERATION_NAME_RENAME); + + expectAccessDenied(() -> + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, RangerHdfsAuthorizer.OPERATION_NAME_RENAME)); + + // mv of a directory: no subAccess + expectAccessDenied(() -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, null, false, RangerHdfsAuthorizer.OPERATION_NAME_RENAME)); + + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/tmpdir2/srcdir", false, true), "/tmp/tmpdir2/srcdir", null, FsAction.WRITE, null, null, false, RangerHdfsAuthorizer.OPERATION_NAME_RENAME); + }); + + // run without optimization enabled + runWithAuthzOptimizationEnabled(false, () -> + expectAccessDenied(() -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/srcdir", false, true), "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, RangerHdfsAuthorizer.OPERATION_NAME_RENAME))); + } + + @Test + public void testOperationOptimizerRenameFileUnchanged() 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, RangerHdfsAuthorizer.OPERATION_NAME_RENAME); + + expectAccessDenied(() -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, null, null, null, false, RangerHdfsAuthorizer.OPERATION_NAME_RENAME)); + expectAccessDenied(() -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, FsAction.WRITE, null, null, true, RangerHdfsAuthorizer.OPERATION_NAME_RENAME)); + }); + + runWithAuthzOptimizationEnabled(false, () -> + expectAccessDenied(() -> + checkPermission(authorizer.getExternalAccessControlEnforcer(null), "bob", createPath("/tmp/data-file3", true, true), "/tmp/data-file3", null, FsAction.WRITE, null, null, true, RangerHdfsAuthorizer.OPERATION_NAME_RENAME))); + } + + @Test + public void testOperationOptimizerDeleteRegression() 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, RangerHdfsAuthorizer.OPERATION_NAME_DELETE); + + expectAccessDenied(() -> + checkPermission(enforcer, "bob", inodes, "/tmp/srcdir", null, FsAction.WRITE, null, FsAction.ALL, true, RangerHdfsAuthorizer.OPERATION_NAME_DELETE)); + }); + } + private void readFailWithPath(String path) throws AccessControlException { final TestFileSystem fs = new TestFileSystem(path); fs.setFilePermission(READ_ONLY); @@ -350,4 +406,81 @@ private static INode createNode(String[] pathSegments, int i, String owner, Stri when(mock.toString()).thenReturn(name); 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 field = RangerHdfsAuthorizer.class.getDeclaredField("AUTHZ_OPTIMIZATION_ENABLED"); + boolean previous; + + field.setAccessible(true); + previous = field.getBoolean(authorizer); + field.setBoolean(authorizer, enabled); + + return previous; + } + + private static void expectAccessDenied(ThrowingRunnable runnable) throws Exception { + try { + runnable.run(); + Assert.fail("Access should be denied"); + } catch (AccessControlException exception) { + Assert.assertNotNull(exception); + } + } + + 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; + } }
