DanielZhu58 commented on code in PR #6884:
URL: https://github.com/apache/hive/pull/6884#discussion_r4406860307
##########
standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/handler/TransactionHandler.java:
##########
@@ -505,9 +522,108 @@ public ShowCompactResponse
show_compact(ShowCompactRequest rqst) throws TExcepti
@Override
public boolean submit_for_cleanup(CompactionRequest rqst, long
highestWriteId, long txnId)
throws TException {
+ try {
+ authorizeAndPrepareCompactionRequest(rqst, true);
+ } catch (Exception e) {
+ rethrowException(e);
+ }
return getTxnHandler().submitForCleanup(rqst, highestWriteId, txnId);
}
+ /**
+ * Authorize client-initiated compaction/cleanup requests and derive
server-side run-as identity.
+ * Internal callers (e.g. {@link
org.apache.hadoop.hive.metastore.AcidEventListener}) invoke
+ * {@code TxnHandler} directly and are not subject to this path.
+ *
+ * @param allowMissingTable only for cleanup requests: a rolled-back CTAS
(see {@code HiveQueryLifeTimeHook})
+ * asks for cleanup of a directory whose table was never created or has
already been dropped. In that case
+ * the request is authorized against the database and the requested
location must be strictly inside the
+ * database's managed/external location.
+ */
+ private void authorizeAndPrepareCompactionRequest(CompactionRequest rqst,
boolean allowMissingTable)
+ throws TException, IOException, InterruptedException {
+ if (rqst.getDbname() == null || rqst.getTablename() == null) {
+ throw new MetaException("The DB and table name cannot be null.");
+ }
+ String[] parsedDbName = parseDbName(rqst.getDbname(), conf);
+ String catName = parsedDbName[CAT_NAME];
+ String dbName = parsedDbName[DB_NAME];
+ String tblName = rqst.getTablename();
+ rqst.setDbname(dbName);
+
+ FilterUtils.checkDbAndTableFilters(isServerFilterEnabled, filterHook,
catName, dbName, tblName);
+
+ GetTableRequest getTableRequest = new GetTableRequest(dbName, tblName);
+ getTableRequest.setCatName(catName);
+ Table table;
+ try {
+ table = get_table_core(getTableRequest);
+ } catch (NoSuchObjectException e) {
+ if (!allowMissingTable || rqst.isSetPartitionname()) {
+ throw e;
+ }
+ authorizeCleanupOfMissingTable(rqst, catName, dbName);
+ return;
+ }
+ firePreEvent(new PreAlterTableEvent(table, table, this));
+
+ String location = table.getSd() == null ? null :
table.getSd().getLocation();
+ if (rqst.isSetPartitionname() && rqst.getPartitionname() != null) {
+ List<String> partVals = getPartValsFromName(table,
rqst.getPartitionname());
+ Partition partition = getMS().getPartition(catName, dbName, tblName,
partVals);
+ location = partition.getSd() == null ? null :
partition.getSd().getLocation();
+ }
+ if (location == null) {
+ throw new MetaException("Cannot determine storage location of " + dbName
+ "." + tblName
+ + "; compaction is not supported for this table or partition");
+ }
+ rqst.setRunas(TxnUtils.findUserToRunAs(location, table, conf));
+ }
+
+ /**
+ * Cleanup of a directory that has no backing table: authorize against the
database, require that the
+ * requested location is strictly inside the database's locations (so that a
client cannot get an arbitrary
+ * path purged by the Cleaner) and derive the run-as user from the directory
owner.
+ */
+ private void authorizeCleanupOfMissingTable(CompactionRequest rqst, String
catName, String dbName)
+ throws TException, IOException, InterruptedException {
+ Database db = get_database_core(catName, dbName);
+ firePreEvent(new PreAlterDatabaseEvent(db, db, this));
+
+ String requested = rqst.getProperties() == null ? null :
rqst.getProperties().get(META_TABLE_LOCATION);
+ if (requested == null || requested.isEmpty()) {
+ throw new MetaException("Cannot determine storage location of " + dbName
+ "." + rqst.getTablename()
+ + "; the table does not exist and no location was supplied for the
cleanup");
+ }
+ Warehouse warehouse = getWh();
+ Path location = normalizePath(warehouse, new Path(requested));
+ List<Path> allowedRoots = new ArrayList<>(3);
+ if (db.getLocationUri() != null) {
+ allowedRoots.add(normalizePath(warehouse, new
Path(db.getLocationUri())));
+ }
+ if (db.getManagedLocationUri() != null) {
+ allowedRoots.add(normalizePath(warehouse, new
Path(db.getManagedLocationUri())));
+ }
+ allowedRoots.add(normalizePath(warehouse,
warehouse.getDatabaseManagedPath(db)));
+ boolean allowed = allowedRoots.stream()
Review Comment:
Acknowledged.
Fixed by removing the database-level fallback.
Non-privileged callers can only clean up for an existing table they are
authorized on, and the location is constrained to it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
