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