This is an automated email from the ASF dual-hosted git repository.
maheshbandal15 pushed a commit to branch ranger-2.10
in repository https://gitbox.apache.org/repos/asf/ranger.git
The following commit(s) were added to refs/heads/ranger-2.10 by this push:
new 9f97e02d6 RANGER-5825: Fix HDFS authz optimization on directory rename
(#1315)
9f97e02d6 is described below
commit 9f97e02d63528b9cffcfa581053d06da215e1e37
Author: Mahesh Bandal <[email protected]>
AuthorDate: Sat Oct 17 00:37:40 2026 +0530
RANGER-5825: Fix HDFS authz optimization on directory rename (#1315)
---
.../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;
+ }
}
