[
https://issues.apache.org/jira/browse/DRILL-8548?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18107193#comment-18107193
]
ASF GitHub Bot commented on DRILL-8548:
---------------------------------------
cgivre commented on code in PR #3056:
URL: https://github.com/apache/drill/pull/3056#discussion_r3839544015
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/SqlConverter.java:
##########
@@ -245,6 +245,12 @@ public RelRoot toRel(final SqlNode validatedNode) {
RelNode project = LogicalProject.create(rel.rel,
Collections.emptyList(), expressions, rel.validatedRowType);
rel = RelRoot.of(project, rel.validatedRowType, rel.kind);
}
+
+ // Column-level SELECT authorization check. Done after SqlToRelConverter
has
+ // resolved all column references (so we can trace each to its TableScan)
+ // and before flattenTypes/optimization (so column references are intact).
+ new ColumnAccessChecker(session, drillConfig,
cluster.getMetadataQuery()).check(rel.rel);
Review Comment:
**Planning-time cost is paid on every query, even when Ranger is disabled.**
`check()` walks the whole tree and calls
`RelMetadataQuery.getColumnOrigins()` once per output column of every node
*before* anything consults `authorizer.isEnabled()` — that test lives down in
`ColumnAccessChecker.enforceColumnAccess()`, which only runs after the
traversal has already happened. `getColumnOrigins` is not a cheap metadata
query on wide row types or deep plans.
Since `drill.exec.security.ranger.enabled` defaults to `false`, every
existing Drill deployment pays this on every `toRel` for a feature it isn't
using.
Suggest hoisting the check here:
```java
AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
if (authorizer.isEnabled()) {
new ColumnAccessChecker(session, drillConfig, cluster.getMetadataQuery(),
authorizer).check(rel.rel);
}
```
and passing the resolved `authorizer` into the checker so
`enforceColumnAccess` stops re-resolving it per table.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/DrillCalciteCatalogReader.java:
##########
@@ -103,14 +106,99 @@ void disallowTemporaryTables() {
public Prepare.PreparingTable getTable(List<String> names) {
checkTemporaryTable(names);
Prepare.PreparingTable table = super.getTable(names);
- DrillTable drillTable;
- if (table != null && (drillTable = table.unwrap(DrillTable.class)) !=
null) {
- drillTable.setOptions(session.getOptions());
-
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ if (table != null) {
+ // Ranger SELECT authorization check for ALL table types, including
+ // JDBC storage plugin tables (JdbcTable) that are not DrillTable.
+ checkTableAccess(table, names);
+
+ DrillTable drillTable = table.unwrap(DrillTable.class);
+ if (drillTable != null) {
+ drillTable.setOptions(session.getOptions());
+
drillTable.setTableMetadataProviderManager(tableCache.getUnchecked(DrillTableKey.of(names,
drillTable)));
+ }
}
return table;
}
+ /**
+ * Checks SELECT permission on the resolved table via the configured {@link
AccessAuthorizer}
+ * (Ranger by default). No-op when authorization is disabled (fail-open).
System schemas
+ * (INFORMATION_SCHEMA, sys) are bypassed inside the authorizer
implementation.
+ *
+ * <p>Extracts datasource/schema/table from the resolved qualified name so
it works
+ * for both DrillTable (native storage plugins) and non-DrillTable (JDBC
storage plugin).
+ * The {@code names} argument is used as a fallback to determine the
datasource when
+ * the qualified name does not expose it.
+ */
+ private void checkTableAccess(Prepare.PreparingTable table, List<String>
names) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+ // Use the resolved qualified name (includes default schema resolution)
rather than
+ // the raw input names, which may be incomplete when the user omits the
schema.
+ //
+ // Ranger four-level resource model: datasource / schema / table / column.
+ // The first segment of qualifiedName is the datasource (storage plugin
name);
+ // the LAST segment is the table; any segments in between form the schema
path.
+ // For example "mysql.shf.users" -> datasource=mysql, schema=shf,
table=users.
+ // The schema MUST NOT include the datasource prefix, otherwise Ranger
policy
+ // matching fails (policy has schema=shf but request sends
schema=mysql.shf).
+ //
+ // Some backends have no schema concept (e.g. a flat file store). To keep
the
+ // four-level model uniform, we synthesize a default schema per datasource
via
+ // getDefaultSchemaByDataSource(). The default branch returns the
datasource
+ // name itself so each storage plugin gets its own default schema namespace
+ // until an explicit mapping is added.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ // datasource.schema.table OR datasource.subschema.table
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ // datasource.table — backend has no schema; synthesize a default so the
+ // four-level resource stays complete (Ranger policy matching requires a
+ // non-null schema key when schema is a mandatory resource).
+ dataSource = qualifiedName.get(0);
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ } else {
+ // Single-element qualified name: fall back to the input names list to
find the datasource.
+ dataSource = !names.isEmpty() ? names.get(0) : tableName;
+ schemaPath = getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
+
+ if (!authorizer.checkTableAccess(userName, dataSource, schemaPath,
tableName, AccessTypes.SELECT)) {
+ throw UserException.permissionError()
+ .message("Access denied: user '%s' lacks SELECT privilege on
%s.%s.%s",
+ userName, dataSource, schemaPath, tableName)
+ .build(logger);
+ }
+ }
+
+ /**
+ * Returns the default schema name to use when a table's qualified name does
not
+ * contain an explicit schema segment (i.e. two-segment {@code
datasource.table}
+ * or a single-segment fallback). This keeps the Ranger four-level resource
+ * model complete even for backends that have no native schema concept.
+ *
+ * <p>Add explicit cases below as new storage plugins are integrated. The
+ * {@code default} branch returns the datasource name itself so each plugin
+ * gets a distinct default schema namespace without further
configuration.</p>
+ *
+ * @param dataSource the storage plugin / datasource name
+ * @return a non-null default schema name
+ */
+ static String getDefaultSchemaByDataSource(String dataSource) {
+ return switch (dataSource.toLowerCase()) {
+ case "dfs", "cp" -> "default";
+ default -> dataSource;
+ };
Review Comment:
Hardcoding `dfs` and `cp` here doesn't hold: storage plugin names are chosen
by the operator. `dfs` and `cp` are conventions from the bundled
`bootstrap-storage-plugins.json`, not reserved names — a site can rename them,
delete them, or add a second filesystem plugin under any name, and then the
synthesized schema silently changes from `"default"` to the plugin name and
stops matching the Ranger policy the admin wrote.
Since this value participates in policy matching, it should be configurable
rather than compiled in — e.g. a `drill.exec.security.ranger.default_schema`
map in `drill-module.conf`, defaulting to the plugin name for anything not
listed. That also keeps `DrillCalciteCatalogReader` from having to know
anything about specific plugins.
Minor: the javadoc says "Add explicit cases below as new storage plugins are
integrated" — that's the part I'd like to avoid, since it makes the security
mapping a code change rather than a config change.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/security/ranger/AccessAuthorizerFactory.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.security.ranger;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Singleton factory for {@link AccessAuthorizer}. Follows the same
configuration-driven
+ * reflective loading pattern as
+ * {@link org.apache.drill.exec.rpc.user.security.UserAuthenticatorFactory}.
+ *
+ * <p>When {@code drill.exec.security.ranger.enabled} is {@code false} (the
default),
+ * a {@link NoOpAccessAuthorizer} is returned. When enabled, the
implementation class
+ * named by {@code drill.exec.security.ranger.impl} is loaded reflectively and
+ * initialized with the configured service name.</p>
+ */
+public class AccessAuthorizerFactory {
+ private static final Logger logger =
LoggerFactory.getLogger(AccessAuthorizerFactory.class);
+
+ // Default values used when the corresponding config keys are absent.
+ // Package-private so tests can verify the default-impl selection logic
+ // without triggering RangerAccessAuthorizer.init() (which calls
+ // RangerPluginClassLoader.getInstance() and crashes the JVM in the test
+ // environment).
+ static final String DEFAULT_AUTHORIZER_IMPL =
+ "org.apache.drill.exec.security.ranger.RangerAccessAuthorizer";
+ static final String DEFAULT_SERVICE_NAME = "drill";
+
+ private static volatile AccessAuthorizer instance;
+
+ private AccessAuthorizerFactory() {
+ }
+
+ /**
+ * Returns the singleton {@link AccessAuthorizer} instance, initializing it
from
+ * the given configuration on first call.
+ *
+ * @param config the Drill configuration
+ * @return the authorizer (never {@code null})
+ */
+ public static AccessAuthorizer getAuthorizer(DrillConfig config) {
Review Comment:
**JVM-wide static singleton pinned by whichever `DrillConfig` calls first.**
`instance` is `static`, has no reset, and ignores the `config` argument on
every call after the first. Drill routinely runs more than one Drillbit in a
single JVM — `ClusterFixture` / `BaseTestQuery` spin up multi-node clusters,
and embedded mode shares the process with the client. The first
`getAuthorizer(config)` call wins for the lifetime of the process, so a second
Drillbit configured with Ranger enabled silently gets the
`NoOpAccessAuthorizer` created for the first one (or vice versa).
That also makes this effectively untestable in-process: once any test
touches it, every later test in the same fork sees the cached instance.
`AccessAuthorizerFactoryTest` works around this today, which is a signal.
Suggest scoping the authorizer to `DrillbitContext` instead — construct it
once in `Drillbit.run()` and hand it to `SqlConverter` /
`DrillCalciteCatalogReader` through the context, the same way Drill scopes
`StoragePluginRegistry` and `OptionManager`. That removes the static entirely
and makes lifecycle (including `cleanUp()` on the Ranger plugin, which
currently never gets called) explicit.
##########
exec/java-exec/src/main/java/org/apache/drill/exec/security/ranger/AccessAuthorizerFactory.java:
##########
@@ -0,0 +1,114 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.security.ranger;
+
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.exec.ExecConstants;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * Singleton factory for {@link AccessAuthorizer}. Follows the same
configuration-driven
+ * reflective loading pattern as
+ * {@link org.apache.drill.exec.rpc.user.security.UserAuthenticatorFactory}.
+ *
+ * <p>When {@code drill.exec.security.ranger.enabled} is {@code false} (the
default),
+ * a {@link NoOpAccessAuthorizer} is returned. When enabled, the
implementation class
+ * named by {@code drill.exec.security.ranger.impl} is loaded reflectively and
+ * initialized with the configured service name.</p>
+ */
+public class AccessAuthorizerFactory {
+ private static final Logger logger =
LoggerFactory.getLogger(AccessAuthorizerFactory.class);
+
+ // Default values used when the corresponding config keys are absent.
+ // Package-private so tests can verify the default-impl selection logic
+ // without triggering RangerAccessAuthorizer.init() (which calls
+ // RangerPluginClassLoader.getInstance() and crashes the JVM in the test
+ // environment).
+ static final String DEFAULT_AUTHORIZER_IMPL =
+ "org.apache.drill.exec.security.ranger.RangerAccessAuthorizer";
+ static final String DEFAULT_SERVICE_NAME = "drill";
+
+ private static volatile AccessAuthorizer instance;
+
+ private AccessAuthorizerFactory() {
+ }
+
+ /**
+ * Returns the singleton {@link AccessAuthorizer} instance, initializing it
from
+ * the given configuration on first call.
+ *
+ * @param config the Drill configuration
+ * @return the authorizer (never {@code null})
+ */
+ public static AccessAuthorizer getAuthorizer(DrillConfig config) {
+ if (instance != null) {
+ return instance;
+ }
+ synchronized (AccessAuthorizerFactory.class) {
+ if (instance != null) {
+ return instance;
+ }
+ instance = createAuthorizer(config);
+ return instance;
+ }
+ }
+
+ /**
+ * Resolves the authorizer implementation class name from configuration.
+ * Package-private for unit testing the selection logic without triggering
+ * {@code RangerAccessAuthorizer.init()}.
+ */
+ static String getImplClassName(DrillConfig config) {
+ return config.hasPath(ExecConstants.RANGER_AUTHORIZER_IMPL)
+ ? config.getString(ExecConstants.RANGER_AUTHORIZER_IMPL)
+ : DEFAULT_AUTHORIZER_IMPL;
+ }
+
+ /**
+ * Resolves the Ranger service name from configuration.
+ * Package-private for unit testing the selection logic.
+ */
+ static String getServiceName(DrillConfig config) {
+ return config.hasPath(ExecConstants.RANGER_SERVICE_NAME)
+ ? config.getString(ExecConstants.RANGER_SERVICE_NAME)
+ : DEFAULT_SERVICE_NAME;
+ }
+
+ private static AccessAuthorizer createAuthorizer(DrillConfig config) {
+ boolean enabled = config.hasPath(ExecConstants.RANGER_AUTH_ENABLED)
+ && config.getBoolean(ExecConstants.RANGER_AUTH_ENABLED);
+ if (!enabled) {
+ logger.info("Ranger authorization is disabled
(drill.exec.security.ranger.enabled=false)");
+ return new NoOpAccessAuthorizer();
+ }
+ String impl = getImplClassName(config);
+ String serviceName = getServiceName(config);
+ logger.info("Initializing Ranger authorizer: impl={}, service={}", impl,
serviceName);
+ try {
+ Class<?> clazz = Class.forName(impl);
+ AccessAuthorizer authorizer = (AccessAuthorizer)
clazz.getDeclaredConstructor().newInstance();
+ authorizer.init(serviceName);
+ logger.info("Ranger authorizer initialized, enabled={}",
authorizer.isEnabled());
+ return authorizer;
+ } catch (Exception e) {
+ logger.error("Failed to initialize Ranger authorizer {}, falling back to
fail-closed", impl, e);
+ throw new RuntimeException("Failed to initialize Ranger authorizer
impl:" + impl
+ + ", serviceName:" + serviceName, e);
+ }
Review Comment:
The log message says "falling back to fail-closed", but nothing falls back —
the next statement rethrows. Propagating is the right behaviour (a Drillbit
that can't initialise its authorizer should refuse to start, and
`Drillbit.run()` will surface it), the message just contradicts the code and
will send an operator looking for a fallback authorizer that doesn't exist.
```java
logger.error("Failed to initialize Ranger authorizer {}; Drillbit startup
will abort", impl, e);
throw new DrillRuntimeException(...);
```
Also worth using `DrillRuntimeException` rather than raw `RuntimeException`
for consistency with the rest of the codebase, and dropping `e.getMessage()`
string-concatenation in favour of the cause (which is already passed).
##########
exec/java-exec/src/main/java/org/apache/drill/exec/planner/sql/conversion/ColumnAccessChecker.java:
##########
@@ -0,0 +1,350 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.planner.sql.conversion;
+
+import org.apache.calcite.plan.RelOptTable;
+import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.RelShuttleImpl;
+import org.apache.calcite.rel.core.TableScan;
+import org.apache.calcite.rel.logical.LogicalAggregate;
+import org.apache.calcite.rel.logical.LogicalFilter;
+import org.apache.calcite.rel.logical.LogicalJoin;
+import org.apache.calcite.rel.logical.LogicalProject;
+import org.apache.calcite.rel.logical.LogicalSort;
+import org.apache.calcite.rel.metadata.RelColumnOrigin;
+import org.apache.calcite.rel.metadata.RelMetadataQuery;
+import org.apache.calcite.rex.RexInputRef;
+import org.apache.calcite.rex.RexNode;
+import org.apache.calcite.rex.RexSubQuery;
+import org.apache.calcite.rex.RexVisitorImpl;
+import org.apache.drill.common.config.DrillConfig;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.planner.sql.SchemaUtilities;
+import org.apache.drill.exec.rpc.user.UserSession;
+import org.apache.drill.exec.security.ranger.AccessAuthorizer;
+import org.apache.drill.exec.security.ranger.AccessAuthorizerFactory;
+import org.apache.drill.exec.security.ranger.AccessTypes;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.ArrayList;
+import java.util.HashSet;
+import java.util.IdentityHashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+/**
+ * Visitor that traverses a RelNode tree and enforces column-level SELECT
authorization
+ * via the configured {@link AccessAuthorizer} (Ranger by default).
+ *
+ * <p><b>Design:</b> For every RelNode that carries {@link RexNode} expressions
+ * (Project, Filter, Join, Aggregate, Sort), the visitor collects all {@link
RexInputRef}s
+ * and uses Calcite's {@link RelMetadataQuery#getColumnOrigins(RelNode, int)}
to trace each
+ * referenced column back to its originating {@link TableScan} column index.
This correctly
+ * handles multi-hop projections, filters, joins, and aggregations.</p>
+ *
+ * <p>For a {@link TableScan} that has NO traced column references (e.g.
+ * {@code SELECT * FROM t} with no intervening Project), ALL columns of that
table
+ * are checked.</p>
+ *
+ * <p>System schemas (INFORMATION_SCHEMA, sys) are bypassed inside the
authorizer
+ * implementation. When authorization is disabled, the visitor is a no-op
(fail-open).</p>
+ */
+class ColumnAccessChecker extends RelShuttleImpl {
+
+ private static final Logger logger =
LoggerFactory.getLogger(ColumnAccessChecker.class);
+
+ private final UserSession session;
+ private final DrillConfig drillConfig;
+ private final RelMetadataQuery mq;
+
+ // Records each table's referenced column indices. Uses IdentityHashMap
because
+ // RelOptTable equals/hashCode may be expensive or not identity-based.
+ private final Map<RelOptTable, Set<Integer>> tableToReferencedCols = new
IdentityHashMap<>();
+
+ ColumnAccessChecker(UserSession session, DrillConfig drillConfig,
RelMetadataQuery mq) {
+ this.session = session;
+ this.drillConfig = drillConfig;
+ this.mq = mq;
+ }
+
+ /**
+ * Entry point: traverse the tree and enforce column-level access.
+ */
+ void check(RelNode root) {
+ // Also trace the root node's output columns (covers bare TableScan root or
+ // top-level Project output).
+ traceOutputColumns(root);
+
+ // Walk the tree to collect RexInputRef origins from all
expression-bearing nodes.
+ root.accept(this);
+ }
+
+ // ------------------------------------------------------------------
+ // RelShuttle overrides — collect RexInputRefs from expression-bearing nodes
+ // ------------------------------------------------------------------
+
+ @Override
+ public RelNode visit(LogicalProject project) {
+ collectRefs(project.getProjects(), project.getInput());
+ return super.visit(project);
+ }
+
+ @Override
+ public RelNode visit(LogicalFilter filter) {
+ if (filter.getCondition() != null) {
+ analyzeRex(filter.getCondition(), filter.getInput(), -1, null);
+ }
+ return super.visit(filter);
+ }
+
+ @Override
+ public RelNode visit(LogicalJoin join) {
+ if (join.getCondition() != null) {
+ int leftCount = join.getLeft().getRowType().getFieldCount();
+ analyzeRex(join.getCondition(), join.getRight(), leftCount,
join.getLeft());
+ }
+ return super.visit(join);
+ }
+
+ @Override
+ public RelNode visit(LogicalAggregate aggregate) {
+ for (int i : aggregate.getGroupSet()) {
+ traceColumnOrigin(aggregate.getInput(), i);
+ }
+ return super.visit(aggregate);
+ }
+
+ @Override
+ public RelNode visit(LogicalSort sort) {
+ if (sort.getCollation() != null) {
+ sort.getCollation().getFieldCollations().forEach(fc ->
+ traceColumnOrigin(sort.getInput(), fc.getFieldIndex()));
+ }
+ return super.visit(sort);
+ }
+
+ @Override
+ public RelNode visit(TableScan scan) {
+ RelOptTable table = scan.getTable();
+
+ // Determine which columns to check: traced columns, or ALL if none were
traced
+ // (SELECT * FROM t case).
+ Set<Integer> referencedColIndices = tableToReferencedCols.get(table);
+ List<String> allColumnNames = scan.getRowType().getFieldNames();
+
+ Set<String> columnsToCheck;
+ if (referencedColIndices == null || referencedColIndices.isEmpty()) {
+ // SELECT * — check all columns
+ columnsToCheck = new HashSet<>(allColumnNames);
+ } else {
+ columnsToCheck = new HashSet<>();
+ for (int idx : referencedColIndices) {
+ if (idx >= 0 && idx < allColumnNames.size()) {
+ columnsToCheck.add(allColumnNames.get(idx));
+ }
+ }
+ }
+
+ if (columnsToCheck.isEmpty()) {
+ return scan;
+ }
+
+ enforceColumnAccess(table, columnsToCheck);
+ return scan;
+ }
+
+ /**
+ * Traces the output columns of a RelNode back to their table-scan origins.
+ */
+ private void traceOutputColumns(RelNode node) {
+ if (node == null) {
+ return;
+ }
+ int fieldCount = node.getRowType().getFieldCount();
+ for (int i = 0; i < fieldCount; i++) {
+ traceColumnOrigin(node, i);
+ }
+ }
+
+ /**
+ * Traces a single output column of {@code node} at index {@code
columnIndex} back to
+ * table-scan origins, recording them in {@link #tableToReferencedCols}.
+ */
+ private void traceColumnOrigin(RelNode node, int columnIndex) {
+ if (node == null || mq == null) {
+ return;
+ }
+ Set<RelColumnOrigin> origins;
+ try {
+ origins = mq.getColumnOrigins(node, columnIndex);
+ } catch (Exception e) {
+ logger.debug("getColumnOrigins failed for {} column {}", node,
columnIndex, e);
+ return;
+ }
+ if (origins == null) {
+ return;
+ }
+ for (RelColumnOrigin origin : origins) {
+ RelOptTable originTable = origin.getOriginTable();
+ if (originTable != null) {
+ // Record origins for ALL table types (DrillTable, JdbcTable, etc.).
+ // Previously this only recorded DrillTable origins, which caused
+ // JDBC storage plugin tables (JdbcTable) to be skipped entirely.
+ tableToReferencedCols
+ .computeIfAbsent(originTable, k -> new HashSet<>())
+ .add(origin.getOriginColumnOrdinal());
+ }
+ }
+ }
+
+ /**
+ * Collects RexInputRefs from a list of RexNodes and traces each to its
+ * table-scan origin via the input node's metadata. Also processes any
+ * {@link RexSubQuery} found in the expressions (scalar/IN/EXISTS subqueries)
+ * so that column references inside subqueries are authorized.
+ */
+ private void collectRefs(List<RexNode> rexNodes, RelNode inputNode) {
+ if (rexNodes == null || inputNode == null) {
+ return;
+ }
+ for (RexNode rex : rexNodes) {
+ analyzeRex(rex, inputNode, -1, null);
+ }
+ }
+
+ /**
+ * Analyzes a {@link RexNode} expression, collecting {@link RexInputRef}s and
+ * {@link RexSubQuery}s, tracing each input ref to its table-scan origin and
+ * recursively visiting each subquery's {@link RelNode} tree.
+ *
+ * @param rex the expression to analyze
+ * @param inputNode the input RelNode that RexInputRefs resolve against
+ * @param leftCount if {@code >= 0}, indicates a join condition: refs with
+ * index {@code < leftCount} resolve against {@code
leftInput},
+ * others resolve against {@code inputNode} (the right
input)
+ * with offset {@code leftCount}. If {@code < 0}, all refs
+ * resolve against {@code inputNode}.
+ * @param leftInput the left input of a join, or {@code null} when
+ * {@code leftCount < 0}.
+ */
+ private void analyzeRex(RexNode rex, RelNode inputNode, int leftCount,
RelNode leftInput) {
+ if (rex == null) {
+ return;
+ }
+ Set<Integer> refs = new HashSet<>();
+ List<RexSubQuery> subQueries = new ArrayList<>();
+ rex.accept(new RexRefCollector(refs, subQueries));
+ for (int refIndex : refs) {
+ if (leftCount >= 0 && refIndex < leftCount) {
+ traceColumnOrigin(leftInput, refIndex);
+ } else if (leftCount >= 0) {
+ traceColumnOrigin(inputNode, refIndex - leftCount);
+ } else {
+ traceColumnOrigin(inputNode, refIndex);
+ }
+ }
+ for (RexSubQuery sq : subQueries) {
+ // Trace the subquery's output columns to their table-scan origins.
+ // For scalar subqueries (e.g. SELECT sum(user_id) FROM t), this traces
+ // the aggregate output back to the underlying table column.
+ traceOutputColumns(sq.rel);
+ // Recursively visit the subquery's RelNode tree so that RexInputRefs
+ // inside the subquery (e.g. columns in WHERE/SELECT of the subquery)
+ // are also collected and traced.
+ sq.rel.accept(this);
+ }
+ }
+
+ /**
+ * Enforces column-level access for the given table and column set.
+ */
+ private void enforceColumnAccess(RelOptTable table, Set<String> columns) {
+ AccessAuthorizer authorizer =
AccessAuthorizerFactory.getAuthorizer(drillConfig);
+ if (!authorizer.isEnabled()) {
+ return; // fail-open when disabled
+ }
+
+ // Resolve datasource / schema / table from the qualified name, consistent
+ // with DrillCalciteCatalogReader.checkTableAccess(). This works for ALL
+ // table types (DrillTable, JdbcTable, etc.) — previously this method
+ // required a DrillTable and skipped JdbcTable, leaving JDBC storage
+ // plugin tables without column-level authorization.
+ List<String> qualifiedName = table.getQualifiedName();
+ String tableName = qualifiedName.get(qualifiedName.size() - 1);
+ String dataSource;
+ String schemaPath;
+ if (qualifiedName.size() > 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath = SchemaUtilities.getSchemaPath(qualifiedName.subList(1,
qualifiedName.size() - 1));
+ } else if (qualifiedName.size() == 2) {
+ dataSource = qualifiedName.get(0);
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ } else {
+ dataSource = tableName;
+ schemaPath =
DrillCalciteCatalogReader.getDefaultSchemaByDataSource(dataSource);
+ }
+ String userName = session.getCredentials().getUserName();
+
+ if (!authorizer.checkColumnAccess(userName, dataSource, schemaPath,
tableName, columns, AccessTypes.SELECT)) {
+ throw UserException.permissionError()
+ .message("Access denied: user '%s' lacks SELECT privilege on one or
more columns " +
+ "(%s) of table %s.%s.%s", userName, columns, dataSource,
schemaPath, tableName)
+ .build(logger);
+ }
+ }
+
+ /**
+ * RexVisitor that collects all {@link RexInputRef} indices and
+ * {@link RexSubQuery} instances encountered in a {@link RexNode} tree.
+ * <p> dispatches to
+ * {@link #visitSubQuery(RexSubQuery)} (not {@code visitCall}), so a plain
+ * {@code RexInputRef}-only visitor silently skips over subqueries. This
+ * collector overrides {@code visitSubQuery} to capture the subquery and then
+ * continues traversing its operands so that nested {@link RexInputRef}s
+ * (e.g. the left side of {@code x IN (SELECT ...)}) and nested subqueries
+ * are also collected.</p>
+ */
+ private static final class RexRefCollector extends RexVisitorImpl<Void> {
+ private final Set<Integer> refs;
+ private final List<RexSubQuery> subQueries;
+
+ RexRefCollector(Set<Integer> refs, List<RexSubQuery> subQueries) {
+ super(true);
+ this.refs = refs;
+ this.subQueries = subQueries;
+ }
+
+ @Override
+ public Void visitInputRef(RexInputRef ref) {
Review Comment:
**Column-level bypass: correlated references are never authorized.**
`RexRefCollector` recognises `RexInputRef` and `RexSubQuery`. An outer-query
column referenced *from inside* a correlated subquery is neither — Calcite
represents it as a `RexFieldAccess` whose referenceExpr is a
`RexCorrelVariable`. So it is never traced and never checked:
```sql
> Integrate Apache Ranger authorization for Drill
> -----------------------------------------------
>
> Key: DRILL-8548
> URL: https://issues.apache.org/jira/browse/DRILL-8548
> Project: Apache Drill
> Issue Type: New Feature
> Components: Server
> Affects Versions: 1.23.0
> Reporter: shihuafeng
> Priority: Major
> Fix For: 1.23.0
>
>
> This issue introduces Apache Ranger as a pluggable authorization framework
> for Drill, enabling centralized table-level and column-level access control
> for Drill queries. It is a substantial feature spanning three layers: a new
> drill-ranger module , integration hooks in exec/java-exec, and distribution
> packaging. The design follows Drill's existing AccessAuthorizer SPI and
> Calcite's RelShuttle mechanism so that column-level checks happen in the
> toRel phase before physical planning
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
