diff --git a/presto-main-base/src/main/java/com/facebook/presto/execution/DDLDefinitionExecution.java b/presto-main-base/src/main/java/com/facebook/presto/execution/DDLDefinitionExecution.java index e81322959b865..509f7b5b7c668 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/execution/DDLDefinitionExecution.java +++ b/presto-main-base/src/main/java/com/facebook/presto/execution/DDLDefinitionExecution.java @@ -24,6 +24,7 @@ import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.Statement; import com.facebook.presto.transaction.TransactionManager; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ListenableFuture; import jakarta.inject.Inject; @@ -61,6 +62,8 @@ private DDLDefinitionExecution( @Override protected ListenableFuture executeTask() { + accessControl.checkQueryIntegrity(stateMachine.getSession().getIdentity(), stateMachine.getSession().getAccessControlContext(), query, ImmutableMap.of(), ImmutableMap.of()); + return task.execute(statement, transactionManager, metadata, accessControl, stateMachine.getSession(), parameters, stateMachine.getWarningCollector(), query); } diff --git a/presto-main-base/src/main/java/com/facebook/presto/execution/SessionDefinitionExecution.java b/presto-main-base/src/main/java/com/facebook/presto/execution/SessionDefinitionExecution.java index c84b6d4c2718b..faa589d46b42e 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/execution/SessionDefinitionExecution.java +++ b/presto-main-base/src/main/java/com/facebook/presto/execution/SessionDefinitionExecution.java @@ -24,6 +24,7 @@ import com.facebook.presto.sql.tree.Expression; import com.facebook.presto.sql.tree.Statement; import com.facebook.presto.transaction.TransactionManager; +import com.google.common.collect.ImmutableMap; import com.google.common.util.concurrent.ListenableFuture; import jakarta.inject.Inject; @@ -61,6 +62,8 @@ private SessionDefinitionExecution( @Override protected ListenableFuture executeTask() { + accessControl.checkQueryIntegrity(stateMachine.getSession().getIdentity(), stateMachine.getSession().getAccessControlContext(), query, ImmutableMap.of(), ImmutableMap.of()); + return task.execute(statement, transactionManager, metadata, accessControl, stateMachine, parameters, query); } diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java b/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java index f0fde936968db..16b7ba6267406 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java +++ b/presto-main-base/src/main/java/com/facebook/presto/security/AccessControlManager.java @@ -95,6 +95,7 @@ public AccessControlManager(TransactionManager transactionManager) addSystemAccessControlFactory(new AllowAllSystemAccessControl.Factory()); addSystemAccessControlFactory(new ReadOnlySystemAccessControl.Factory()); addSystemAccessControlFactory(new FileBasedSystemAccessControl.Factory()); + addSystemAccessControlFactory(new DenyQueryIntegrityCheckSystemAccessControl.Factory()); } public void addSystemAccessControlFactory(SystemAccessControlFactory accessControlFactory) diff --git a/presto-main-base/src/main/java/com/facebook/presto/security/DenyQueryIntegrityCheckSystemAccessControl.java b/presto-main-base/src/main/java/com/facebook/presto/security/DenyQueryIntegrityCheckSystemAccessControl.java new file mode 100644 index 0000000000000..2c912ce326c37 --- /dev/null +++ b/presto-main-base/src/main/java/com/facebook/presto/security/DenyQueryIntegrityCheckSystemAccessControl.java @@ -0,0 +1,106 @@ +/* + * Licensed 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 com.facebook.presto.security; + +import com.facebook.presto.common.QualifiedObjectName; +import com.facebook.presto.spi.CatalogSchemaTableName; +import com.facebook.presto.spi.MaterializedViewDefinition; +import com.facebook.presto.spi.analyzer.ViewDefinition; +import com.facebook.presto.spi.security.AccessControlContext; +import com.facebook.presto.spi.security.Identity; +import com.facebook.presto.spi.security.SystemAccessControl; +import com.facebook.presto.spi.security.SystemAccessControlFactory; + +import java.security.Principal; +import java.util.Map; +import java.util.Optional; +import java.util.Set; + +import static com.facebook.presto.spi.security.AccessDeniedException.denyQueryIntegrityCheck; +import static com.google.common.base.Preconditions.checkArgument; +import static java.util.Objects.requireNonNull; + +public class DenyQueryIntegrityCheckSystemAccessControl + implements SystemAccessControl +{ + public static final String NAME = "deny-query-integrity-check"; + + private static final DenyQueryIntegrityCheckSystemAccessControl INSTANCE = new DenyQueryIntegrityCheckSystemAccessControl(); + + public static class Factory + implements SystemAccessControlFactory + { + @Override + public String getName() + { + return NAME; + } + + @Override + public SystemAccessControl create(Map config) + { + requireNonNull(config, "config is null"); + checkArgument(config.isEmpty(), "This access controller does not support any configuration properties"); + return INSTANCE; + } + } + + @Override + public void checkQueryIntegrity(Identity identity, AccessControlContext context, String query, Map viewDefinitions, Map materializedViewDefinitions) + { + denyQueryIntegrityCheck(); + } + + @Override + public void checkCanSetUser(Identity identity, AccessControlContext context, Optional principal, String userName) + { + } + + @Override + public void checkCanSetSystemSessionProperty(Identity identity, AccessControlContext context, String propertyName) + { + } + + @Override + public void checkCanAccessCatalog(Identity identity, AccessControlContext context, String catalogName) + { + } + + @Override + public Set filterCatalogs(Identity identity, AccessControlContext context, Set catalogs) + { + return catalogs; + } + + @Override + public Set filterSchemas(Identity identity, AccessControlContext context, String catalogName, Set schemaNames) + { + return schemaNames; + } + + @Override + public void checkCanCreateTable(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + } + + @Override + public void checkCanShowCreateTable(Identity identity, AccessControlContext context, CatalogSchemaTableName table) + { + } + + @Override + public void checkCanSelectFromColumns(Identity identity, AccessControlContext context, CatalogSchemaTableName table, Set columns) + { + } +} diff --git a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/Analyzer.java b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/Analyzer.java index d4cb14ac2ac68..3ecc50bfa7fb1 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/Analyzer.java +++ b/presto-main-base/src/main/java/com/facebook/presto/sql/analyzer/Analyzer.java @@ -16,6 +16,7 @@ import com.facebook.presto.Session; import com.facebook.presto.metadata.Metadata; import com.facebook.presto.spi.WarningCollector; +import com.facebook.presto.spi.analyzer.AccessControlReferences; import com.facebook.presto.spi.function.FunctionHandle; import com.facebook.presto.spi.security.AccessControl; import com.facebook.presto.sql.parser.SqlParser; @@ -43,7 +44,8 @@ import static com.facebook.presto.sql.analyzer.SemanticErrorCode.CANNOT_HAVE_AGGREGATIONS_WINDOWS_OR_GROUPING; import static com.facebook.presto.sql.analyzer.SemanticErrorCode.NOT_SUPPORTED; import static com.facebook.presto.sql.analyzer.UtilizedColumnsAnalyzer.analyzeForUtilizedColumns; -import static com.facebook.presto.util.AnalyzerUtil.checkAccessPermissions; +import static com.facebook.presto.util.AnalyzerUtil.checkAccessPermissionsForColumns; +import static com.facebook.presto.util.AnalyzerUtil.checkAccessPermissionsForTable; import static java.util.Objects.requireNonNull; public class Analyzer @@ -107,7 +109,9 @@ public Analysis analyze(Statement statement) public Analysis analyze(Statement statement, boolean isDescribe) { Analysis analysis = analyzeSemantic(statement, isDescribe); - checkAccessPermissions(analysis.getAccessControlReferences(), query); + AccessControlReferences accessControlReferences = analysis.getAccessControlReferences(); + checkAccessPermissionsForTable(accessControlReferences); + checkAccessPermissionsForColumns(accessControlReferences); return analysis; } diff --git a/presto-main-base/src/main/java/com/facebook/presto/util/AnalyzerUtil.java b/presto-main-base/src/main/java/com/facebook/presto/util/AnalyzerUtil.java index f43ce90779cb7..ba91087dcc245 100644 --- a/presto-main-base/src/main/java/com/facebook/presto/util/AnalyzerUtil.java +++ b/presto-main-base/src/main/java/com/facebook/presto/util/AnalyzerUtil.java @@ -102,14 +102,14 @@ public static AnalyzerContext getAnalyzerContext( public static void checkAccessPermissions(AccessControlReferences accessControlReferences, String query) { // Query check - checkAccessPermissionsForQuery(accessControlReferences, query); + checkQueryIntegrity(accessControlReferences, query); // Table checks checkAccessPermissionsForTable(accessControlReferences); // Table Column checks checkAccessPermissionsForColumns(accessControlReferences); } - private static void checkAccessPermissionsForQuery(AccessControlReferences accessControlReferences, String query) + public static void checkQueryIntegrity(AccessControlReferences accessControlReferences, String query) { AccessControlInfo queryAccessControlInfo = accessControlReferences.getQueryAccessControlInfo(); // Only check access if query gets analyzed @@ -124,7 +124,7 @@ private static void checkAccessPermissionsForQuery(AccessControlReferences acces } } - private static void checkAccessPermissionsForColumns(AccessControlReferences accessControlReferences) + public static void checkAccessPermissionsForColumns(AccessControlReferences accessControlReferences) { accessControlReferences.getTableColumnAndSubfieldReferencesForAccessControl() .forEach((accessControlInfo, tableColumnReferences) -> @@ -140,7 +140,7 @@ private static void checkAccessPermissionsForColumns(AccessControlReferences acc })); } - private static void checkAccessPermissionsForTable(AccessControlReferences accessControlReferences) + public static void checkAccessPermissionsForTable(AccessControlReferences accessControlReferences) { accessControlReferences.getTableReferences().forEach((accessControlRole, accessControlInfoForTables) -> accessControlInfoForTables.forEach(accessControlInfoForTable -> { AccessControlInfo accessControlInfo = accessControlInfoForTable.getAccessControlInfo(); diff --git a/presto-tests/src/test/java/com/facebook/presto/tests/TestCheckAccessPermissionsForQueryTypes.java b/presto-tests/src/test/java/com/facebook/presto/tests/TestCheckAccessPermissionsForQueryTypes.java new file mode 100644 index 0000000000000..a1395ae886ca5 --- /dev/null +++ b/presto-tests/src/test/java/com/facebook/presto/tests/TestCheckAccessPermissionsForQueryTypes.java @@ -0,0 +1,57 @@ +/* + * Licensed 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 com.facebook.presto.tests; + +import com.facebook.presto.security.DenyQueryIntegrityCheckSystemAccessControl; +import com.facebook.presto.testing.QueryRunner; +import com.facebook.presto.tests.tpch.TpchQueryRunnerBuilder; +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +public class TestCheckAccessPermissionsForQueryTypes + extends AbstractTestQueryFramework +{ + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + DistributedQueryRunner queryRunner = TpchQueryRunnerBuilder.builder() + .setAccessControlProperties(ImmutableMap.of("access-control.name", DenyQueryIntegrityCheckSystemAccessControl.NAME)).build(); + + queryRunner.loadSystemAccessControl(); + + return queryRunner; + } + + @Override + protected QueryRunner createExpectedQueryRunner() + throws Exception + { + QueryRunner queryRunner = TpchQueryRunnerBuilder.builder().build(); + return queryRunner; + } + + @Test + public void testCheckQueryIntegrityCalls() + { + assertAccessDenied("select * from orders", ".*Query integrity check failed.*"); + assertAccessDenied("analyze orders", ".*Query integrity check failed.*"); + assertAccessDenied("explain analyze select * from orders", ".*Query integrity check failed.*"); + assertAccessDenied("explain select * from orders", ".*Query integrity check failed.*"); + assertAccessDenied("explain (type validate) select * from orders", ".*Query integrity check failed.*"); + assertAccessDenied("CREATE TABLE test_empty (a BIGINT)", ".*Query integrity check failed.*"); + assertAccessDenied("use tpch.tiny", ".*Query integrity check failed.*"); + } +}