diff --git a/presto-main/src/main/java/com/facebook/presto/execution/SqlQueryManager.java b/presto-main/src/main/java/com/facebook/presto/execution/SqlQueryManager.java index 26f206141a292..33e311dee44cd 100644 --- a/presto-main/src/main/java/com/facebook/presto/execution/SqlQueryManager.java +++ b/presto-main/src/main/java/com/facebook/presto/execution/SqlQueryManager.java @@ -335,6 +335,7 @@ private void createQueryInternal(QueryId queryId, SessionContext sessionCont // decode session session = sessionSupplier.createSession(queryId, sessionContext); + accessControl.checkQueryIntegrity(session.getIdentity(), query); WarningCollector warningCollector = warningCollectorFactory.create(); diff --git a/presto-main/src/main/java/com/facebook/presto/security/AccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/AccessControl.java index fc0aa8e9d1779..bc395e7672a52 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AccessControl.java @@ -34,6 +34,12 @@ public interface AccessControl */ void checkCanSetUser(Optional principal, String userName); + /** + * Check if the query is unexpectedly modified compared to the token passed in identity. + * @throws com.facebook.presto.spi.security.AccessDeniedException if query is modified. + */ + void checkQueryIntegrity(Identity identity, String query); + /** * Filter the list of catalogs to those visible to the identity. */ diff --git a/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java b/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java index b7612c8b41f98..ca328c9c5621e 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AccessControlManager.java @@ -150,6 +150,15 @@ public void checkCanSetUser(Optional principal, String userName) authenticationCheck(() -> systemAccessControl.get().checkCanSetUser(principal, userName)); } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + requireNonNull(identity, "extraCredentials is null"); + requireNonNull(query, "query is null"); + + authenticationCheck(() -> systemAccessControl.get().checkQueryIntegrity(identity, query)); + } + @Override public Set filterCatalogs(Identity identity, Set catalogs) { @@ -753,6 +762,12 @@ public ConnectorTransactionHandle getTransactionHandle(TransactionId transaction private static class InitializingSystemAccessControl implements SystemAccessControl { + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + throw new PrestoException(SERVER_STARTING_UP, "Presto server is still initializing"); + } + @Override public void checkCanSetUser(Optional principal, String userName) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/AllowAllAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/AllowAllAccessControl.java index a136018959736..77ce4e5fa9b63 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AllowAllAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AllowAllAccessControl.java @@ -33,6 +33,11 @@ public void checkCanSetUser(Optional principal, String userName) { } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + } + @Override public Set filterCatalogs(Identity identity, Set catalogs) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java index b3df912b44b03..9654673b3b5a7 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/AllowAllSystemAccessControl.java @@ -55,6 +55,11 @@ public SystemAccessControl create(Map config) } } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + } + @Override public void checkCanSetUser(Optional principal, String userName) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/DenyAllAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/DenyAllAccessControl.java index 2be047067d8ba..b4d7a12f9fab3 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/DenyAllAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/DenyAllAccessControl.java @@ -42,6 +42,7 @@ import static com.facebook.presto.spi.security.AccessDeniedException.denyGrantRoles; import static com.facebook.presto.spi.security.AccessDeniedException.denyGrantTablePrivilege; import static com.facebook.presto.spi.security.AccessDeniedException.denyInsertTable; +import static com.facebook.presto.spi.security.AccessDeniedException.denyQueryIntegrityCheck; import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameColumn; import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameSchema; import static com.facebook.presto.spi.security.AccessDeniedException.denyRenameTable; @@ -67,6 +68,12 @@ public void checkCanSetUser(Optional principal, String userName) denySetUser(principal, userName); } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + denyQueryIntegrityCheck(); + } + @Override public Set filterCatalogs(Identity identity, Set catalogs) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java index bf311700090c0..17659f86c697c 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/FileBasedSystemAccessControl.java @@ -157,6 +157,11 @@ public void checkCanSetUser(Optional principal, String userName) denySetUser(principal, userName); } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + } + @Override public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) { diff --git a/presto-main/src/main/java/com/facebook/presto/security/ReadOnlySystemAccessControl.java b/presto-main/src/main/java/com/facebook/presto/security/ReadOnlySystemAccessControl.java index 901d833242bce..66466c7fccb32 100644 --- a/presto-main/src/main/java/com/facebook/presto/security/ReadOnlySystemAccessControl.java +++ b/presto-main/src/main/java/com/facebook/presto/security/ReadOnlySystemAccessControl.java @@ -58,6 +58,11 @@ public void checkCanSetUser(Optional principal, String userName) { } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + } + @Override public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) { diff --git a/presto-main/src/test/java/com/facebook/presto/security/TestAccessControlManager.java b/presto-main/src/test/java/com/facebook/presto/security/TestAccessControlManager.java index 3429b8ab3f5fa..5ee1a48e78c5a 100644 --- a/presto-main/src/test/java/com/facebook/presto/security/TestAccessControlManager.java +++ b/presto-main/src/test/java/com/facebook/presto/security/TestAccessControlManager.java @@ -51,18 +51,21 @@ import static com.facebook.presto.spi.ConnectorId.createInformationSchemaConnectorId; import static com.facebook.presto.spi.ConnectorId.createSystemTablesConnectorId; +import static com.facebook.presto.spi.security.AccessDeniedException.denyQueryIntegrityCheck; import static com.facebook.presto.spi.security.AccessDeniedException.denySelectColumns; import static com.facebook.presto.spi.security.AccessDeniedException.denySelectTable; import static com.facebook.presto.transaction.InMemoryTransactionManager.createTestTransactionManager; import static com.facebook.presto.transaction.TransactionBuilder.transaction; import static java.util.Objects.requireNonNull; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertThrows; import static org.testng.Assert.fail; public class TestAccessControlManager { private static final Principal PRINCIPAL = new BasicPrincipal("principal"); private static final String USER_NAME = "user_name"; + private static final String QUERY_TOKEN_FIELD = "query_token"; @Test(expectedExceptions = PrestoException.class, expectedExceptionsMessageRegExp = "Presto server is still initializing") public void testInitializing() @@ -131,6 +134,27 @@ public void testSetAccessControl() assertEquals(accessControlFactory.getCheckedPrincipal(), Optional.of(PRINCIPAL)); } + @Test + public void testCheckQueryIntegrity() + { + AccessControlManager accessControlManager = new AccessControlManager(createTestTransactionManager()); + + TestSystemAccessControlFactory accessControlFactory = new TestSystemAccessControlFactory("test"); + accessControlManager.addSystemAccessControlFactory(accessControlFactory); + accessControlManager.setSystemAccessControl("test", ImmutableMap.of()); + String testQuery = "test_query"; + + accessControlManager.checkQueryIntegrity(new Identity(USER_NAME, Optional.of(PRINCIPAL), ImmutableMap.of(), ImmutableMap.of(QUERY_TOKEN_FIELD, testQuery)), testQuery); + assertEquals(accessControlFactory.getCheckedUserName(), USER_NAME); + assertEquals(accessControlFactory.getCheckedPrincipal(), Optional.of(PRINCIPAL)); + assertEquals(accessControlFactory.getCheckedQuery(), testQuery); + + assertThrows(AccessDeniedException.class, + () -> accessControlManager.checkQueryIntegrity( + new Identity(USER_NAME, Optional.of(PRINCIPAL), ImmutableMap.of(), ImmutableMap.of(QUERY_TOKEN_FIELD, testQuery + " modified")), + testQuery)); + } + @Test public void testNoCatalogAccessControl() { @@ -219,6 +243,7 @@ private static class TestSystemAccessControlFactory private Optional checkedPrincipal; private String checkedUserName; + private String checkedQuery; public TestSystemAccessControlFactory(String name) { @@ -240,6 +265,11 @@ public String getCheckedUserName() return checkedUserName; } + public String getCheckedQuery() + { + return checkedQuery; + } + @Override public String getName() { @@ -259,6 +289,17 @@ public void checkCanSetUser(Optional principal, String userName) checkedUserName = userName; } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + if (!query.equals(identity.getExtraCredentials().get(QUERY_TOKEN_FIELD))) { + denyQueryIntegrityCheck(); + } + checkedUserName = identity.getUser(); + checkedPrincipal = identity.getPrincipal(); + checkedQuery = query; + } + @Override public void checkCanAccessCatalog(Identity identity, String catalogName) { diff --git a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java index 1e5277f5c13dd..3467eddb1baa6 100644 --- a/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java +++ b/presto-plugin-toolkit/src/main/java/com/facebook/presto/plugin/base/security/ForwardingSystemAccessControl.java @@ -52,6 +52,12 @@ public void checkCanSetUser(Optional principal, String userName) delegate().checkCanSetUser(principal, userName); } + @Override + public void checkQueryIntegrity(Identity identity, String query) + { + delegate().checkQueryIntegrity(identity, query); + } + @Override public void checkCanSetSystemSessionProperty(Identity identity, String propertyName) { diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java index 9ac6c77eba9c3..bdf220546edc9 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/AccessDeniedException.java @@ -36,6 +36,11 @@ public static void denySetUser(Optional principal, String userName) denySetUser(principal, userName, null); } + public static void denyQueryIntegrityCheck() + { + throw new AccessDeniedException("Query integrity check failed."); + } + public static void denySetUser(Optional principal, String userName, String extraInfo) { throw new AccessDeniedException(format("Principal %s cannot become user %s%s", principal.orElse(null), userName, formatExtraInfo(extraInfo))); diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java b/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java index 812b2b9512eb1..ba022aba397d5 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/security/SystemAccessControl.java @@ -53,6 +53,12 @@ public interface SystemAccessControl */ void checkCanSetUser(Optional principal, String userName); + /** + * Check if the query is unexpectedly modified compared to token provided in Identity. + * @throws AccessDeniedException if query is modified. + */ + void checkQueryIntegrity(Identity identity, String query); + /** * Check if identity is allowed to set the specified system property. *