From f15a34ec544e84e4942bd70aac9ad1f8e28af7df Mon Sep 17 00:00:00 2001 From: Piotr Rzysko Date: Mon, 23 Dec 2024 11:08:18 +0100 Subject: [PATCH] Introduce redaction for sensitive statements This commit introduces redacting of security-sensitive information in statements containing catalog properties, specifically: * CREATE CATALOG * EXPLAIN CREATE CATALOG * EXPLAIN ANALYZE CREATE CATALOG The current approach is as follows: * For syntactically valid statements, only properties containing sensitive information are masked. * If a query is syntactically valid but retrieving security-sensitive properties fails for any reason (e.g., the query references a nonexistent connector or catalog property evaluation fails), all properties are masked. * If a query fails before or during parsing, the entire query is masked. The redacted form is created right before initialization of the QueryStateMachine and is propagated to all places that create QueryInfo and BasicQueryInfo (e.g., REST endpoints, query events, and the system.runtime.queries table). Before this change, QueryInfo/BasicQueryInfo stored the raw query text received from the end user. From now on, the text will be altered for the cases listed above. --- .../main/java/io/trino/FeaturesConfig.java | 14 + .../io/trino/connector/CatalogFactory.java | 4 + .../connector/CatalogPropertiesProvider.java | 26 ++ .../connector/DefaultCatalogFactory.java | 36 ++- .../DynamicCatalogManagerModule.java | 2 + .../DynamicCatalogPropertiesProvider.java | 43 +++ .../trino/connector/LazyCatalogFactory.java | 7 + .../trino/connector/StaticCatalogManager.java | 15 +- .../connector/StaticCatalogManagerModule.java | 3 + .../StaticCatalogPropertiesProvider.java | 38 +++ .../io/trino/dispatcher/DispatchManager.java | 24 +- .../dispatcher/DispatchQueryFactory.java | 3 +- .../dispatcher/LocalDispatchQueryFactory.java | 5 +- .../io/trino/execution/QueryStateMachine.java | 9 +- .../io/trino/server/CoordinatorModule.java | 4 + .../trino/sql/SensitiveStatementRedactor.java | 212 +++++++++++++ .../dispatcher/TestLocalDispatchQuery.java | 2 +- .../execution/BaseDataDefinitionTaskTest.java | 2 +- .../java/io/trino/execution/TestCallTask.java | 2 +- .../io/trino/execution/TestCommitTask.java | 2 +- .../execution/TestCreateCatalogTask.java | 4 +- .../trino/execution/TestDeallocateTask.java | 2 +- .../trino/execution/TestDropCatalogTask.java | 2 +- .../io/trino/execution/TestPrepareTask.java | 2 +- .../execution/TestQueryStateMachine.java | 2 +- .../trino/execution/TestResetSessionTask.java | 2 +- .../io/trino/execution/TestRoleTasks.java | 2 +- .../io/trino/execution/TestRollbackTask.java | 2 +- .../io/trino/execution/TestSetPathTask.java | 2 +- .../TestSetSessionAuthorizationTask.java | 2 +- .../trino/execution/TestSetSessionTask.java | 2 +- .../trino/execution/TestSetTimeZoneTask.java | 2 +- ...estSqlTaskManagerRaceWithCatalogPrune.java | 6 + .../execution/TestStartTransactionTask.java | 2 +- .../sql/analyzer/TestFeaturesConfig.java | 7 +- .../java/io/trino/sql/tree/CreateCatalog.java | 12 + .../execution/TestEventListenerBasic.java | 16 +- .../TestRedactSensitiveStatements.java | 292 ++++++++++++++++++ 38 files changed, 770 insertions(+), 44 deletions(-) create mode 100644 core/trino-main/src/main/java/io/trino/connector/CatalogPropertiesProvider.java create mode 100644 core/trino-main/src/main/java/io/trino/connector/DynamicCatalogPropertiesProvider.java create mode 100644 core/trino-main/src/main/java/io/trino/connector/StaticCatalogPropertiesProvider.java create mode 100644 core/trino-main/src/main/java/io/trino/sql/SensitiveStatementRedactor.java create mode 100644 testing/trino-tests/src/test/java/io/trino/execution/TestRedactSensitiveStatements.java diff --git a/core/trino-main/src/main/java/io/trino/FeaturesConfig.java b/core/trino-main/src/main/java/io/trino/FeaturesConfig.java index 707b3300e8ca..09b4fefb6bc8 100644 --- a/core/trino-main/src/main/java/io/trino/FeaturesConfig.java +++ b/core/trino-main/src/main/java/io/trino/FeaturesConfig.java @@ -122,6 +122,8 @@ public class FeaturesConfig private boolean faultTolerantExecutionExchangeEncryptionEnabled = true; + private boolean statementRedactingEnabled = true; + public enum DataIntegrityVerification { NONE, @@ -514,6 +516,18 @@ public FeaturesConfig setFaultTolerantExecutionExchangeEncryptionEnabled(boolean return this; } + public boolean isStatementRedactingEnabled() + { + return statementRedactingEnabled; + } + + @Config("deprecated.statement-redacting-enabled") + public FeaturesConfig setStatementRedactingEnabled(boolean statementRedactingEnabled) + { + this.statementRedactingEnabled = statementRedactingEnabled; + return this; + } + public void applyFaultTolerantExecutionDefaults() { exchangeCompressionCodec = LZ4; diff --git a/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java index e91151f5d8c5..93af10c1f4a0 100644 --- a/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/CatalogFactory.java @@ -20,6 +20,8 @@ import io.trino.spi.connector.ConnectorFactory; import io.trino.spi.connector.ConnectorName; +import java.util.Set; + @ThreadSafe public interface CatalogFactory { @@ -28,4 +30,6 @@ public interface CatalogFactory CatalogConnector createCatalog(CatalogProperties catalogProperties); CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector); + + Set getSecuritySensitivePropertyNames(CatalogProperties catalogProperties); } diff --git a/core/trino-main/src/main/java/io/trino/connector/CatalogPropertiesProvider.java b/core/trino-main/src/main/java/io/trino/connector/CatalogPropertiesProvider.java new file mode 100644 index 000000000000..351889d77741 --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/connector/CatalogPropertiesProvider.java @@ -0,0 +1,26 @@ +/* + * 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 io.trino.connector; + +import io.trino.spi.catalog.CatalogName; +import io.trino.spi.catalog.CatalogProperties; +import io.trino.spi.connector.ConnectorName; + +import java.util.Map; + +public interface CatalogPropertiesProvider +{ + CatalogProperties getCatalogProperties(CatalogName catalogName, ConnectorName connectorName, Map properties); +} diff --git a/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java index b9759c83fced..8075b856a352 100644 --- a/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/DefaultCatalogFactory.java @@ -13,6 +13,7 @@ */ package io.trino.connector; +import com.google.common.collect.ImmutableSet; import com.google.errorprone.annotations.ThreadSafe; import com.google.inject.Inject; import io.airlift.configuration.secrets.SecretsResolver; @@ -45,6 +46,7 @@ import java.util.Map; import java.util.Optional; +import java.util.Set; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; @@ -144,6 +146,25 @@ public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName return createCatalog(catalogHandle, connectorName, connector, Optional.empty()); } + @Override + public Set getSecuritySensitivePropertyNames(CatalogProperties catalogProperties) + { + ConnectorFactory connectorFactory = connectorFactories.get(catalogProperties.connectorName()); + if (connectorFactory == null) { + // If someone tries to use a non-existent connector, we assume they + // misspelled the name and, for safety, we redact all the properties. + return ImmutableSet.copyOf(catalogProperties.properties().keySet()); + } + + ConnectorContext context = createConnectorContext(catalogProperties.catalogHandle()); + String catalogName = catalogProperties.catalogHandle().getCatalogName().toString(); + Map config = secretsResolver.getResolvedConfiguration(catalogProperties.properties()); + + try (ThreadContextClassLoader _ = new ThreadContextClassLoader(connectorFactory.getClass().getClassLoader())) { + return connectorFactory.getSecuritySensitivePropertyNames(catalogName, config, context); + } + } + private CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector, Optional catalogProperties) { Tracer tracer = createTracer(catalogHandle); @@ -196,7 +217,16 @@ private Connector createConnector( ConnectorFactory connectorFactory, Map properties) { - ConnectorContext context = new ConnectorContextInstance( + ConnectorContext context = createConnectorContext(catalogHandle); + + try (ThreadContextClassLoader _ = new ThreadContextClassLoader(connectorFactory.getClass().getClassLoader())) { + return connectorFactory.create(catalogName, properties, context); + } + } + + private ConnectorContext createConnectorContext(CatalogHandle catalogHandle) + { + return new ConnectorContextInstance( catalogHandle, openTelemetry, createTracer(catalogHandle), @@ -206,10 +236,6 @@ private Connector createConnector( new InternalMetadataProvider(metadata, typeManager), pageSorter, pageIndexerFactory); - - try (ThreadContextClassLoader _ = new ThreadContextClassLoader(connectorFactory.getClass().getClassLoader())) { - return connectorFactory.create(catalogName, properties, context); - } } private Tracer createTracer(CatalogHandle catalogHandle) diff --git a/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogManagerModule.java b/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogManagerModule.java index 5c6d5f8ef191..4b89f8a53b56 100644 --- a/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogManagerModule.java +++ b/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogManagerModule.java @@ -41,6 +41,8 @@ protected void setup(Binder binder) configBinder(binder).bindConfig(CatalogPruneTaskConfig.class); binder.bind(CatalogPruneTask.class).in(Scopes.SINGLETON); + + binder.bind(CatalogPropertiesProvider.class).to(DynamicCatalogPropertiesProvider.class).in(Scopes.SINGLETON); } else { binder.bind(WorkerDynamicCatalogManager.class).in(Scopes.SINGLETON); diff --git a/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogPropertiesProvider.java b/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogPropertiesProvider.java new file mode 100644 index 000000000000..59429d02c9c0 --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/connector/DynamicCatalogPropertiesProvider.java @@ -0,0 +1,43 @@ +/* + * 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 io.trino.connector; + +import com.google.inject.Inject; +import io.trino.spi.catalog.CatalogName; +import io.trino.spi.catalog.CatalogProperties; +import io.trino.spi.catalog.CatalogStore; +import io.trino.spi.connector.ConnectorName; + +import java.util.Map; + +import static java.util.Objects.requireNonNull; + +public class DynamicCatalogPropertiesProvider + implements CatalogPropertiesProvider +{ + private final CatalogStore catalogStore; + + @Inject + public DynamicCatalogPropertiesProvider(CatalogStore catalogStore) + { + this.catalogStore = requireNonNull(catalogStore, "catalogStore is null"); + } + + @Override + public CatalogProperties getCatalogProperties(CatalogName catalogName, ConnectorName connectorName, Map properties) + { + return catalogStore.createCatalogProperties(catalogName, connectorName, properties); + } +} diff --git a/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java b/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java index 09513d9669f9..aab4490a129f 100644 --- a/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java +++ b/core/trino-main/src/main/java/io/trino/connector/LazyCatalogFactory.java @@ -19,6 +19,7 @@ import io.trino.spi.connector.ConnectorFactory; import io.trino.spi.connector.ConnectorName; +import java.util.Set; import java.util.concurrent.atomic.AtomicReference; import static com.google.common.base.Preconditions.checkState; @@ -51,6 +52,12 @@ public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName return getDelegate().createCatalog(catalogHandle, connectorName, connector); } + @Override + public Set getSecuritySensitivePropertyNames(CatalogProperties catalogProperties) + { + return getDelegate().getSecuritySensitivePropertyNames(catalogProperties); + } + private CatalogFactory getDelegate() { CatalogFactory catalogFactory = delegate.get(); diff --git a/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManager.java b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManager.java index b308bf3cf676..ae017ba6f7db 100644 --- a/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManager.java +++ b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManager.java @@ -14,7 +14,6 @@ package io.trino.connector; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.io.Files; import com.google.errorprone.annotations.ThreadSafe; @@ -29,7 +28,6 @@ import io.trino.spi.catalog.CatalogName; import io.trino.spi.catalog.CatalogProperties; import io.trino.spi.connector.CatalogHandle; -import io.trino.spi.connector.CatalogHandle.CatalogVersion; import io.trino.spi.connector.ConnectorName; import jakarta.annotation.PreDestroy; @@ -56,7 +54,6 @@ import static io.trino.spi.StandardErrorCode.CATALOG_NOT_AVAILABLE; import static io.trino.spi.StandardErrorCode.CATALOG_NOT_FOUND; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; -import static io.trino.spi.connector.CatalogHandle.createRootCatalogHandle; import static io.trino.util.Executors.executeUntilFailure; import static java.util.Objects.requireNonNull; @@ -77,9 +74,14 @@ private enum State { CREATED, INITIALIZED, STOPPED } private final AtomicReference state = new AtomicReference<>(State.CREATED); @Inject - public StaticCatalogManager(CatalogFactory catalogFactory, StaticCatalogManagerConfig config, @ForStartup Executor executor) + public StaticCatalogManager( + CatalogFactory catalogFactory, + StaticCatalogManagerConfig config, + @ForStartup Executor executor, + StaticCatalogPropertiesProvider propertiesProvider) { this.catalogFactory = requireNonNull(catalogFactory, "catalogFactory is null"); + requireNonNull(propertiesProvider, "propertiesProvider is null"); List disabledCatalogs = firstNonNull(config.getDisabledCatalogs(), ImmutableList.of()); ImmutableList.Builder catalogProperties = ImmutableList.builder(); @@ -107,10 +109,7 @@ public StaticCatalogManager(CatalogFactory catalogFactory, StaticCatalogManagerC log.warn("Catalog '%s' is using the deprecated connector name '%s'. The correct connector name is '%s'", catalogName, deprecatedConnectorName, connectorName); } - catalogProperties.add(new CatalogProperties( - createRootCatalogHandle(new CatalogName(catalogName), new CatalogVersion("default")), - new ConnectorName(connectorName), - ImmutableMap.copyOf(properties))); + catalogProperties.add(propertiesProvider.getCatalogProperties(new CatalogName(catalogName), new ConnectorName(connectorName), properties)); } this.catalogProperties = catalogProperties.build(); this.executor = requireNonNull(executor, "executor is null"); diff --git a/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManagerModule.java b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManagerModule.java index 7b487b37b56e..e676b8ab8f8b 100644 --- a/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManagerModule.java +++ b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogManagerModule.java @@ -34,6 +34,9 @@ public void configure(Binder binder) binder.bind(CatalogManager.class).to(StaticCatalogManager.class).in(Scopes.SINGLETON); binder.bind(LazyRegister.class).asEagerSingleton(); + + binder.bind(StaticCatalogPropertiesProvider.class).in(Scopes.SINGLETON); + binder.bind(CatalogPropertiesProvider.class).to(StaticCatalogPropertiesProvider.class).in(Scopes.SINGLETON); } private static class LazyRegister diff --git a/core/trino-main/src/main/java/io/trino/connector/StaticCatalogPropertiesProvider.java b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogPropertiesProvider.java new file mode 100644 index 000000000000..7290bb51e564 --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/connector/StaticCatalogPropertiesProvider.java @@ -0,0 +1,38 @@ +/* + * 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 io.trino.connector; + +import com.google.common.collect.ImmutableMap; +import io.trino.spi.catalog.CatalogName; +import io.trino.spi.catalog.CatalogProperties; +import io.trino.spi.connector.CatalogHandle; +import io.trino.spi.connector.ConnectorName; + +import java.util.Map; + +import static io.trino.spi.connector.CatalogHandle.createRootCatalogHandle; + +public class StaticCatalogPropertiesProvider + implements CatalogPropertiesProvider +{ + @Override + public CatalogProperties getCatalogProperties(CatalogName catalogName, ConnectorName connectorName, Map properties) + { + return new CatalogProperties( + createRootCatalogHandle(catalogName, new CatalogHandle.CatalogVersion("default")), + connectorName, + ImmutableMap.copyOf(properties)); + } +} diff --git a/core/trino-main/src/main/java/io/trino/dispatcher/DispatchManager.java b/core/trino-main/src/main/java/io/trino/dispatcher/DispatchManager.java index f49114735147..2b7c11840e3a 100644 --- a/core/trino-main/src/main/java/io/trino/dispatcher/DispatchManager.java +++ b/core/trino-main/src/main/java/io/trino/dispatcher/DispatchManager.java @@ -13,6 +13,7 @@ */ package io.trino.dispatcher; +import com.google.common.base.Function; import com.google.common.util.concurrent.AbstractFuture; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -44,6 +45,9 @@ import io.trino.spi.TrinoException; import io.trino.spi.resourcegroups.SelectionContext; import io.trino.spi.resourcegroups.SelectionCriteria; +import io.trino.sql.SensitiveStatementRedactor; +import io.trino.sql.tree.Expression; +import io.trino.sql.tree.Statement; import jakarta.annotation.PostConstruct; import jakarta.annotation.PreDestroy; import org.weakref.jmx.Flatten; @@ -84,6 +88,7 @@ public class DispatchManager private final SessionPropertyDefaults sessionPropertyDefaults; private final SessionPropertyManager sessionPropertyManager; private final Tracer tracer; + private final SensitiveStatementRedactor sensitiveStatementRedactor; private final int maxQueryLength; @@ -107,6 +112,7 @@ public DispatchManager( SessionPropertyDefaults sessionPropertyDefaults, SessionPropertyManager sessionPropertyManager, Tracer tracer, + SensitiveStatementRedactor sensitiveStatementRedactor, QueryManagerConfig queryManagerConfig, DispatchExecutor dispatchExecutor, QueryMonitor queryMonitor) @@ -121,6 +127,7 @@ public DispatchManager( this.sessionPropertyDefaults = requireNonNull(sessionPropertyDefaults, "sessionPropertyDefaults is null"); this.sessionPropertyManager = sessionPropertyManager; this.tracer = requireNonNull(tracer, "tracer is null"); + this.sensitiveStatementRedactor = requireNonNull(sensitiveStatementRedactor, "sensitiveStatementRedactor is null"); this.maxQueryLength = queryManagerConfig.getMaxQueryLength(); @@ -240,7 +247,7 @@ private void createQueryInternal(QueryId queryId, Span querySpan, Slug slug, DispatchQuery dispatchQuery = dispatchQueryFactory.createDispatchQuery( session, sessionContext.getTransactionId(), - query, + getRedactedQueryProvider(preparedQuery, query), preparedQuery, slug, selectionContext.getResourceGroupId()); @@ -266,8 +273,14 @@ private void createQueryInternal(QueryId queryId, Span querySpan, Slug slug, .setSource(sessionContext.getSource().orElse(null)) .build(); } + String redactedQuery = sensitiveStatementRedactor.redact(query); Optional preparedSql = Optional.ofNullable(preparedQuery).flatMap(PreparedQuery::getPrepareSql); - DispatchQuery failedDispatchQuery = failedDispatchQueryFactory.createFailedDispatchQuery(session, query, preparedSql, Optional.empty(), throwable); + DispatchQuery failedDispatchQuery = failedDispatchQueryFactory.createFailedDispatchQuery( + session, + redactedQuery, + preparedSql, + Optional.empty(), + throwable); queryCreated(failedDispatchQuery); // maintain proper order of calls such that EventListener has access to QueryInfo // - add query to tracker @@ -280,6 +293,13 @@ private void createQueryInternal(QueryId queryId, Span querySpan, Slug slug, } } + private Function getRedactedQueryProvider(PreparedQuery preparedQuery, String query) + { + Statement statement = preparedQuery.getStatement(); + List parameters = preparedQuery.getParameters(); + return session -> sensitiveStatementRedactor.redact(query, statement, session, parameters); + } + private boolean queryCreated(DispatchQuery dispatchQuery) { boolean queryAdded = queryTracker.addQuery(dispatchQuery); diff --git a/core/trino-main/src/main/java/io/trino/dispatcher/DispatchQueryFactory.java b/core/trino-main/src/main/java/io/trino/dispatcher/DispatchQueryFactory.java index f74f302982f7..8be3e3d7c73e 100644 --- a/core/trino-main/src/main/java/io/trino/dispatcher/DispatchQueryFactory.java +++ b/core/trino-main/src/main/java/io/trino/dispatcher/DispatchQueryFactory.java @@ -20,13 +20,14 @@ import io.trino.transaction.TransactionId; import java.util.Optional; +import java.util.function.Function; public interface DispatchQueryFactory { DispatchQuery createDispatchQuery( Session session, Optional transactionId, - String query, + Function queryProvider, PreparedQuery preparedQuery, Slug slug, ResourceGroupId resourceGroup); diff --git a/core/trino-main/src/main/java/io/trino/dispatcher/LocalDispatchQueryFactory.java b/core/trino-main/src/main/java/io/trino/dispatcher/LocalDispatchQueryFactory.java index 5205b1268a9e..ba65c701d7c4 100644 --- a/core/trino-main/src/main/java/io/trino/dispatcher/LocalDispatchQueryFactory.java +++ b/core/trino-main/src/main/java/io/trino/dispatcher/LocalDispatchQueryFactory.java @@ -44,6 +44,7 @@ import java.util.Map; import java.util.Optional; +import java.util.function.Function; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.util.StatementUtils.getQueryType; @@ -108,7 +109,7 @@ public LocalDispatchQueryFactory( public DispatchQuery createDispatchQuery( Session session, Optional existingTransactionId, - String query, + Function queryProvider, PreparedQuery preparedQuery, Slug slug, ResourceGroupId resourceGroup) @@ -117,7 +118,7 @@ public DispatchQuery createDispatchQuery( PlanOptimizersStatsCollector planOptimizersStatsCollector = new PlanOptimizersStatsCollector(queryReportedRuleStatsLimit); QueryStateMachine stateMachine = QueryStateMachine.begin( existingTransactionId, - query, + queryProvider, preparedQuery.getPrepareSql(), session, locationFactory.createQueryLocation(session.getQueryId()), diff --git a/core/trino-main/src/main/java/io/trino/execution/QueryStateMachine.java b/core/trino-main/src/main/java/io/trino/execution/QueryStateMachine.java index 83a754e92193..67879c805e76 100644 --- a/core/trino-main/src/main/java/io/trino/execution/QueryStateMachine.java +++ b/core/trino-main/src/main/java/io/trino/execution/QueryStateMachine.java @@ -80,6 +80,7 @@ import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.AtomicReference; import java.util.function.Consumer; +import java.util.function.Function; import java.util.function.Supplier; import static com.google.common.base.Preconditions.checkArgument; @@ -231,7 +232,7 @@ private QueryStateMachine( */ public static QueryStateMachine begin( Optional existingTransactionId, - String query, + Function queryProvider, Optional preparedQuery, Session session, URI self, @@ -249,7 +250,7 @@ public static QueryStateMachine begin( { return beginWithTicker( existingTransactionId, - query, + queryProvider, preparedQuery, session, self, @@ -269,7 +270,7 @@ public static QueryStateMachine begin( static QueryStateMachine beginWithTicker( Optional existingTransactionId, - String query, + Function queryProvider, Optional preparedQuery, Session session, URI self, @@ -318,6 +319,8 @@ static QueryStateMachine beginWithTicker( querySpan.setAttribute(TrinoAttributes.QUERY_TYPE, queryType.map(Enum::name).orElse("UNKNOWN")); + String query = queryProvider.apply(session); + QueryStateMachine queryStateMachine = new QueryStateMachine( query, preparedQuery, diff --git a/core/trino-main/src/main/java/io/trino/server/CoordinatorModule.java b/core/trino-main/src/main/java/io/trino/server/CoordinatorModule.java index 2c1afc074bd4..6759385904ec 100644 --- a/core/trino-main/src/main/java/io/trino/server/CoordinatorModule.java +++ b/core/trino-main/src/main/java/io/trino/server/CoordinatorModule.java @@ -113,6 +113,7 @@ import io.trino.server.ui.WorkerResource; import io.trino.spi.VersionEmbedder; import io.trino.sql.PlannerContext; +import io.trino.sql.SensitiveStatementRedactor; import io.trino.sql.analyzer.AnalyzerFactory; import io.trino.sql.analyzer.QueryExplainerFactory; import io.trino.sql.planner.OptimizerStatsMBeanExporter; @@ -304,6 +305,9 @@ List getCompositeOutputDataSizeEstimatorDelegateFac rewriteBinder.addBinding().to(ShowStatsRewrite.class).in(Scopes.SINGLETON); rewriteBinder.addBinding().to(ExplainRewrite.class).in(Scopes.SINGLETON); + // security-sensitive statement redactor + binder.bind(SensitiveStatementRedactor.class).in(Scopes.SINGLETON); + // planner binder.bind(PlanFragmenter.class).in(Scopes.SINGLETON); binder.bind(PlanOptimizersFactory.class).to(PlanOptimizers.class).in(Scopes.SINGLETON); diff --git a/core/trino-main/src/main/java/io/trino/sql/SensitiveStatementRedactor.java b/core/trino-main/src/main/java/io/trino/sql/SensitiveStatementRedactor.java new file mode 100644 index 000000000000..b2c81300f63a --- /dev/null +++ b/core/trino-main/src/main/java/io/trino/sql/SensitiveStatementRedactor.java @@ -0,0 +1,212 @@ +/* + * 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 io.trino.sql; + +import com.google.common.collect.ImmutableMap; +import com.google.inject.Inject; +import io.trino.FeaturesConfig; +import io.trino.Session; +import io.trino.connector.CatalogFactory; +import io.trino.connector.CatalogPropertiesProvider; +import io.trino.security.AccessControl; +import io.trino.spi.catalog.CatalogName; +import io.trino.spi.catalog.CatalogProperties; +import io.trino.spi.connector.ConnectorName; +import io.trino.sql.tree.AstVisitor; +import io.trino.sql.tree.CreateCatalog; +import io.trino.sql.tree.Explain; +import io.trino.sql.tree.ExplainAnalyze; +import io.trino.sql.tree.Expression; +import io.trino.sql.tree.Identifier; +import io.trino.sql.tree.Node; +import io.trino.sql.tree.Property; +import io.trino.sql.tree.Statement; +import io.trino.sql.tree.StringLiteral; + +import java.util.List; +import java.util.Map; +import java.util.Set; + +import static com.google.common.collect.ImmutableList.toImmutableList; +import static io.trino.execution.ParameterExtractor.bindParameters; +import static io.trino.metadata.PropertyUtil.evaluateProperty; +import static io.trino.spi.StandardErrorCode.INVALID_CATALOG_PROPERTY; +import static io.trino.spi.type.VarcharType.VARCHAR; +import static io.trino.sql.analyzer.ExpressionTreeUtils.extractLocation; +import static java.util.Objects.requireNonNull; +import static org.weakref.jmx.$internal.guava.collect.ImmutableSet.toImmutableSet; + +public class SensitiveStatementRedactor +{ + private static final String REDACTED_VALUE = "***"; + + private final boolean enabled; + private final PlannerContext plannerContext; + private final AccessControl accessControl; + private final CatalogPropertiesProvider catalogPropertiesProvider; + private final CatalogFactory catalogFactory; + + @Inject + public SensitiveStatementRedactor( + FeaturesConfig config, + PlannerContext plannerContext, + AccessControl accessControl, + CatalogPropertiesProvider catalogPropertiesProvider, + CatalogFactory catalogFactory) + { + this.enabled = requireNonNull(config, "config is null").isStatementRedactingEnabled(); + this.plannerContext = requireNonNull(plannerContext, "plannerContext is null"); + this.accessControl = requireNonNull(accessControl, "accessControl is null"); + this.catalogPropertiesProvider = requireNonNull(catalogPropertiesProvider, "catalogPropertiesProvider is null"); + this.catalogFactory = requireNonNull(catalogFactory, "catalogFactory is null"); + } + + public String redact(String rawQuery, Statement statement, Session session, List parameters) + { + if (enabled) { + RedactingVisitor visitor = new RedactingVisitor(session, parameters); + Node redactedStatement = visitor.process(statement); + if (visitor.isRedacted()) { + return SqlFormatter.formatSql(redactedStatement); + } + } + return rawQuery; + } + + public String redact(String rawQuery) + { + if (enabled) { + return REDACTED_VALUE; + } + return rawQuery; + } + + private class RedactingVisitor + extends AstVisitor + { + private final Session session; + private final List parameters; + + private boolean redacted; + + public RedactingVisitor(Session session, List parameters) + { + this.session = requireNonNull(session, "session is null"); + this.parameters = requireNonNull(parameters, "parameters is null"); + } + + public boolean isRedacted() + { + return redacted; + } + + @Override + protected Node visitNode(Node node, Void context) + { + return node; + } + + @Override + protected Node visitExplain(Explain explain, Void context) + { + Statement statement = (Statement) process(explain.getStatement()); + return new Explain(explain.getLocation().orElseThrow(), statement, explain.getOptions()); + } + + @Override + protected Node visitExplainAnalyze(ExplainAnalyze explainAnalyze, Void context) + { + Statement statement = (Statement) process(explainAnalyze.getStatement()); + return new ExplainAnalyze(explainAnalyze.getLocation().orElseThrow(), statement, explainAnalyze.isVerbose()); + } + + @Override + protected Node visitCreateCatalog(CreateCatalog createCatalog, Void context) + { + ConnectorName connectorName = new ConnectorName(createCatalog.getConnectorName().getValue()); + CatalogName catalogName = new CatalogName(createCatalog.getCatalogName().getValue()); + List redactedProperties = redact(createCatalog, catalogName, connectorName); + return createCatalog.withProperties(redactedProperties); + } + + private List redact(CreateCatalog statement, CatalogName catalogName, ConnectorName connectorName) + { + redacted = true; + Set sensitiveProperties = getSecuritySensitiveProperties(statement, catalogName, connectorName); + return redactProperties(statement.getProperties(), sensitiveProperties); + } + + private Set getSecuritySensitiveProperties(CreateCatalog statement, CatalogName catalogName, ConnectorName connectorName) + { + try { + Map properties = evaluateProperties(statement); + CatalogProperties catalogProperties = catalogPropertiesProvider.getCatalogProperties(catalogName, connectorName, properties); + return catalogFactory.getSecuritySensitivePropertyNames(catalogProperties); + } + catch (RuntimeException e) { + // To obtain security-sensitive properties, we need to perform a few steps that usually occur during + // the execution phase, such as evaluating properties, resolving secrets, validating the configuration, etc. + // If any exception occurs while performing these steps preemptively, we don't want to fail the entire query + // because it's not the redactor's responsibility. Instead, we take a defensive approach and mask all the properties. + return statement.getProperties().stream() + .map(Property::getName) + .map(Identifier::getValue) + .collect(toImmutableSet()); + } + } + + private Map evaluateProperties(CreateCatalog statement) + { + // Redaction is performed at an early stage of query processing, before the session + // is registered in the language function manager. Since property evaluation may require + // accessing functions for the session, we temporarily register the session here. + plannerContext.getLanguageFunctionManager().registerQuery(session); + try { + ImmutableMap.Builder propertyValues = ImmutableMap.builder(); + for (Property property : statement.getProperties()) { + String propertyName = property.getName().getValue(); + propertyValues.put( + propertyName, + (String) evaluateProperty( + extractLocation(property), + propertyName, + VARCHAR, + property.getNonDefaultValue(), + session, + plannerContext, + accessControl, + bindParameters(statement, parameters), + INVALID_CATALOG_PROPERTY, + "catalog property")); + } + return propertyValues.buildKeepingLast(); + } + finally { + plannerContext.getLanguageFunctionManager().unregisterQuery(session); + } + } + + private List redactProperties(List properties, Set sensitiveProperties) + { + return properties.stream() + .map(property -> { + if (sensitiveProperties.contains(property.getName().getValue())) { + return new Property(property.getName(), new StringLiteral(REDACTED_VALUE)); + } + return property; + }) + .collect(toImmutableList()); + } + } +} diff --git a/core/trino-main/src/test/java/io/trino/dispatcher/TestLocalDispatchQuery.java b/core/trino-main/src/test/java/io/trino/dispatcher/TestLocalDispatchQuery.java index 1a3f052a4b55..0edd6499d63c 100644 --- a/core/trino-main/src/test/java/io/trino/dispatcher/TestLocalDispatchQuery.java +++ b/core/trino-main/src/test/java/io/trino/dispatcher/TestLocalDispatchQuery.java @@ -109,7 +109,7 @@ public void testSubmittedForDispatchedQuery() accessControl.setSystemAccessControls(List.of(AllowAllSystemAccessControl.INSTANCE)); QueryStateMachine queryStateMachine = QueryStateMachine.begin( Optional.empty(), - "sql", + _ -> "sql", Optional.empty(), TEST_SESSION, URI.create("fake://fake-query"), diff --git a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java index 6e3f7e5e38f5..bb95449b7768 100644 --- a/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java +++ b/core/trino-main/src/test/java/io/trino/execution/BaseDataDefinitionTaskTest.java @@ -229,7 +229,7 @@ private static QueryStateMachine stateMachine(TransactionManager transactionMana { return QueryStateMachine.begin( Optional.empty(), - "test", + _ -> "test", Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCallTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCallTask.java index 683cdf651a5d..5928d352b3a6 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCallTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCallTask.java @@ -156,7 +156,7 @@ private QueryStateMachine stateMachine(TransactionManager transactionManager, Me { return QueryStateMachine.begin( Optional.empty(), - "CALL testing_procedure()", + _ -> "CALL testing_procedure()", Optional.empty(), testSessionBuilder() .setCatalog(TEST_CATALOG_NAME) diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCommitTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCommitTask.java index 1e49a2a7150c..d8447c811e29 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCommitTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCommitTask.java @@ -133,7 +133,7 @@ private QueryStateMachine createQueryStateMachine(String query, Session session, { return QueryStateMachine.begin( Optional.empty(), - query, + _ -> query, Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestCreateCatalogTask.java b/core/trino-main/src/test/java/io/trino/execution/TestCreateCatalogTask.java index fc502fdb3eba..3058b0222b01 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestCreateCatalogTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestCreateCatalogTask.java @@ -84,7 +84,7 @@ public void setUp() task = (CreateCatalogTask) tasks.get(CreateCatalog.class); queryStateMachine = QueryStateMachine.begin( Optional.empty(), - "test", + _ -> "test", Optional.empty(), queryRunner.getDefaultSession(), URI.create("fake://uri"), @@ -176,7 +176,7 @@ public void testAddOrReplaceCatalogFail() CreateCatalogTask task = (CreateCatalogTask) tasks.get(CreateCatalog.class); QueryStateMachine queryStateMachine = QueryStateMachine.begin( Optional.empty(), - "test", + _ -> "test", Optional.empty(), queryRunner.getDefaultSession(), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestDeallocateTask.java b/core/trino-main/src/test/java/io/trino/execution/TestDeallocateTask.java index b63ab4a9caba..9c7069ff091e 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestDeallocateTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestDeallocateTask.java @@ -102,7 +102,7 @@ private Set executeDeallocate(String statementName, String sqlString, Se accessControl.setSystemAccessControls(List.of(AllowAllSystemAccessControl.INSTANCE)); QueryStateMachine stateMachine = QueryStateMachine.begin( Optional.empty(), - sqlString, + _ -> sqlString, Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestDropCatalogTask.java b/core/trino-main/src/test/java/io/trino/execution/TestDropCatalogTask.java index fc84d5a368b6..6f27eba5510c 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestDropCatalogTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestDropCatalogTask.java @@ -115,7 +115,7 @@ private QueryStateMachine createNewQuery() { return QueryStateMachine.begin( Optional.empty(), - "test", + _ -> "test", Optional.empty(), testSession(queryRunner.getDefaultSession()), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestPrepareTask.java b/core/trino-main/src/test/java/io/trino/execution/TestPrepareTask.java index 2c1187c65e5a..17b239aeed7e 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestPrepareTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestPrepareTask.java @@ -125,7 +125,7 @@ private Map executePrepare(String statementName, Statement state accessControl.setSystemAccessControls(List.of(AllowAllSystemAccessControl.INSTANCE)); QueryStateMachine stateMachine = QueryStateMachine.begin( Optional.empty(), - sqlString, + _ -> sqlString, Optional.empty(), testSession(session), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestQueryStateMachine.java b/core/trino-main/src/test/java/io/trino/execution/TestQueryStateMachine.java index eed4fdea1461..272ea5bc39a8 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestQueryStateMachine.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestQueryStateMachine.java @@ -852,7 +852,7 @@ public QueryStateMachine build() accessControl.setSystemAccessControls(List.of(AllowAllSystemAccessControl.INSTANCE)); QueryStateMachine stateMachine = QueryStateMachine.beginWithTicker( Optional.empty(), - QUERY, + _ -> QUERY, Optional.empty(), TEST_SESSION, LOCATION, diff --git a/core/trino-main/src/test/java/io/trino/execution/TestResetSessionTask.java b/core/trino-main/src/test/java/io/trino/execution/TestResetSessionTask.java index e145ebd36638..a337de738998 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestResetSessionTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestResetSessionTask.java @@ -106,7 +106,7 @@ public void test() QueryStateMachine stateMachine = QueryStateMachine.begin( Optional.empty(), - "reset foo", + _ -> "reset foo", Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestRoleTasks.java b/core/trino-main/src/test/java/io/trino/execution/TestRoleTasks.java index 310df2570f3e..56846675b41e 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestRoleTasks.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestRoleTasks.java @@ -160,7 +160,7 @@ protected QueryStateMachine execute(String statement, Data { QueryStateMachine stateMachine = QueryStateMachine.begin( Optional.empty(), - statement, + _ -> statement, Optional.empty(), testSessionBuilder() .setIdentity(Identity.ofUser(USER_NAME)) diff --git a/core/trino-main/src/test/java/io/trino/execution/TestRollbackTask.java b/core/trino-main/src/test/java/io/trino/execution/TestRollbackTask.java index 8a6f94660185..ac0ae44dd3dd 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestRollbackTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestRollbackTask.java @@ -123,7 +123,7 @@ private QueryStateMachine createQueryStateMachine(String query, Session session, { return QueryStateMachine.begin( Optional.empty(), - query, + _ -> query, Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSetPathTask.java b/core/trino-main/src/test/java/io/trino/execution/TestSetPathTask.java index 6bede96b4b63..de341182500e 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSetPathTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSetPathTask.java @@ -110,7 +110,7 @@ private QueryStateMachine createQueryStateMachine(String query) { return QueryStateMachine.begin( Optional.empty(), - query, + _ -> query, Optional.empty(), testSession(), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSetSessionAuthorizationTask.java b/core/trino-main/src/test/java/io/trino/execution/TestSetSessionAuthorizationTask.java index fcc737fa765b..aab16b4e4aea 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSetSessionAuthorizationTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSetSessionAuthorizationTask.java @@ -110,7 +110,7 @@ private QueryStateMachine createStateMachine(Optional transaction { QueryStateMachine stateMachine = QueryStateMachine.begin( transactionId, - query, + _ -> query, Optional.empty(), testSessionBuilder().build(), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSetSessionTask.java b/core/trino-main/src/test/java/io/trino/execution/TestSetSessionTask.java index 26a66a699510..8a12242e145e 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSetSessionTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSetSessionTask.java @@ -194,7 +194,7 @@ private void testSetSessionWithParameters(String property, Expression expression QualifiedName qualifiedPropName = QualifiedName.of(CATALOG_NAME, property); QueryStateMachine stateMachine = QueryStateMachine.begin( Optional.empty(), - format("set %s = 'old_value'", qualifiedPropName), + _ -> format("set %s = 'old_value'", qualifiedPropName), Optional.empty(), testSession(), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSetTimeZoneTask.java b/core/trino-main/src/test/java/io/trino/execution/TestSetTimeZoneTask.java index 726cfa443655..8b65299ff525 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSetTimeZoneTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSetTimeZoneTask.java @@ -252,7 +252,7 @@ private QueryStateMachine createQueryStateMachine(String query) { return QueryStateMachine.begin( Optional.empty(), - query, + _ -> query, Optional.empty(), testSession(), URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java b/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java index cc5681b4d435..a52aac9458b0 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestSqlTaskManagerRaceWithCatalogPrune.java @@ -140,6 +140,12 @@ public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName { throw new UnsupportedOperationException("Only implement what is needed by worker catalog manager"); } + + @Override + public Set getSecuritySensitivePropertyNames(CatalogProperties catalogProperties) + { + throw new UnsupportedOperationException("Only implement what is needed by worker catalog manager"); + } }; private static final TaskExecutor NOOP_TASK_EXECUTOR = new TaskExecutor() { @Override diff --git a/core/trino-main/src/test/java/io/trino/execution/TestStartTransactionTask.java b/core/trino-main/src/test/java/io/trino/execution/TestStartTransactionTask.java index e6cd335f0f0e..8f7d12ab3abd 100644 --- a/core/trino-main/src/test/java/io/trino/execution/TestStartTransactionTask.java +++ b/core/trino-main/src/test/java/io/trino/execution/TestStartTransactionTask.java @@ -254,7 +254,7 @@ private QueryStateMachine createQueryStateMachine(String query, Session session, { return QueryStateMachine.begin( Optional.empty(), - query, + _ -> query, Optional.empty(), session, URI.create("fake://uri"), diff --git a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java index ca8b7134d1cb..15a7e54f0d01 100644 --- a/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java +++ b/core/trino-main/src/test/java/io/trino/sql/analyzer/TestFeaturesConfig.java @@ -66,7 +66,8 @@ public void testDefaults() .setHideInaccessibleColumns(false) .setForceSpillingJoin(false) .setColumnarFilterEvaluationEnabled(true) - .setFaultTolerantExecutionExchangeEncryptionEnabled(true)); + .setFaultTolerantExecutionExchangeEncryptionEnabled(true) + .setStatementRedactingEnabled(true)); } @Test @@ -101,6 +102,7 @@ public void testExplicitPropertyMappings() .put("force-spilling-join-operator", "true") .put("experimental.columnar-filter-evaluation.enabled", "false") .put("fault-tolerant-execution-exchange-encryption-enabled", "false") + .put("deprecated.statement-redacting-enabled", "false") .buildOrThrow(); FeaturesConfig expected = new FeaturesConfig() @@ -131,7 +133,8 @@ public void testExplicitPropertyMappings() .setHideInaccessibleColumns(true) .setForceSpillingJoin(true) .setColumnarFilterEvaluationEnabled(false) - .setFaultTolerantExecutionExchangeEncryptionEnabled(false); + .setFaultTolerantExecutionExchangeEncryptionEnabled(false) + .setStatementRedactingEnabled(false); assertFullMapping(properties, expected); } } diff --git a/core/trino-parser/src/main/java/io/trino/sql/tree/CreateCatalog.java b/core/trino-parser/src/main/java/io/trino/sql/tree/CreateCatalog.java index d443a07a831e..6b032a374537 100644 --- a/core/trino-parser/src/main/java/io/trino/sql/tree/CreateCatalog.java +++ b/core/trino-parser/src/main/java/io/trino/sql/tree/CreateCatalog.java @@ -50,6 +50,18 @@ public CreateCatalog( this.comment = requireNonNull(comment, "comment is null"); } + public CreateCatalog withProperties(List properties) + { + return new CreateCatalog( + getLocation().orElseThrow(), + catalogName, + notExists, + connectorName, + properties, + principal, + comment); + } + public Identifier getCatalogName() { return catalogName; diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java index 3088d39af799..d539831de660 100644 --- a/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java @@ -316,7 +316,11 @@ public void testAnalysisFailure() public void testParseError() throws Exception { - assertFailedQuery("You shall not parse!", "line 1:1: mismatched input 'You'. Expecting: 'ALTER', 'ANALYZE', 'CALL', 'COMMENT', 'COMMIT', 'CREATE', 'DEALLOCATE', 'DELETE', 'DENY', 'DESC', 'DESCRIBE', 'DROP', 'EXECUTE', 'EXPLAIN', 'GRANT', 'INSERT', 'MERGE', 'PREPARE', 'REFRESH', 'RESET', 'REVOKE', 'ROLLBACK', 'SET', 'SHOW', 'START', 'TRUNCATE', 'UPDATE', 'USE', 'WITH', "); + assertFailedQuery( + getSession(), + "You shall not parse!", + "line 1:1: mismatched input 'You'. Expecting: 'ALTER', 'ANALYZE', 'CALL', 'COMMENT', 'COMMIT', 'CREATE', 'DEALLOCATE', 'DELETE', 'DENY', 'DESC', 'DESCRIBE', 'DROP', 'EXECUTE', 'EXPLAIN', 'GRANT', 'INSERT', 'MERGE', 'PREPARE', 'REFRESH', 'RESET', 'REVOKE', 'ROLLBACK', 'SET', 'SHOW', 'START', 'TRUNCATE', 'UPDATE', 'USE', 'WITH', ", + "***"); } @Test @@ -395,16 +399,22 @@ private Optional findQueryId(String queryPattern) private void assertFailedQuery(@Language("SQL") String sql, String expectedFailure) throws Exception { - assertFailedQuery(getSession(), sql, expectedFailure); + assertFailedQuery(getSession(), sql, expectedFailure, sql); } private void assertFailedQuery(Session session, @Language("SQL") String sql, String expectedFailure) throws Exception + { + assertFailedQuery(session, sql, expectedFailure, sql); + } + + private void assertFailedQuery(Session session, @Language("SQL") String sql, String expectedFailure, String redactedQuery) + throws Exception { QueryEvents queryEvents = queries.runQueryAndWaitForEvents(sql, session, Optional.of(expectedFailure)).getQueryEvents(); QueryCompletedEvent queryCompletedEvent = queryEvents.getQueryCompletedEvent(); - assertThat(queryCompletedEvent.getMetadata().getQuery()).isEqualTo(sql); + assertThat(queryCompletedEvent.getMetadata().getQuery()).isEqualTo(redactedQuery); QueryFailureInfo failureInfo = queryCompletedEvent.getFailureInfo() .orElseThrow(() -> new AssertionError("Expected query event to be failed")); diff --git a/testing/trino-tests/src/test/java/io/trino/execution/TestRedactSensitiveStatements.java b/testing/trino-tests/src/test/java/io/trino/execution/TestRedactSensitiveStatements.java new file mode 100644 index 000000000000..564c73175495 --- /dev/null +++ b/testing/trino-tests/src/test/java/io/trino/execution/TestRedactSensitiveStatements.java @@ -0,0 +1,292 @@ +/* + * 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 io.trino.execution; + +import com.google.common.collect.ImmutableMap; +import io.trino.Session; +import io.trino.execution.EventsCollector.QueryEvents; +import io.trino.execution.TestEventListenerPlugin.TestingEventListenerPlugin; +import io.trino.plugin.jdbc.JdbcPlugin; +import io.trino.plugin.jdbc.TestingH2JdbcModule; +import io.trino.plugin.tpch.TpchPlugin; +import io.trino.spi.eventlistener.QueryCompletedEvent; +import io.trino.spi.eventlistener.QueryCreatedEvent; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.DistributedQueryRunner; +import io.trino.testing.QueryRunner; +import org.intellij.lang.annotations.Language; +import org.junit.jupiter.api.Test; + +import java.util.Optional; + +import static io.trino.plugin.jdbc.TestingH2JdbcModule.createH2ConnectionUrl; +import static io.trino.plugin.tpch.TpchMetadata.TINY_SCHEMA_NAME; +import static io.trino.testing.TestingNames.randomNameSuffix; +import static io.trino.testing.TestingSession.testSessionBuilder; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestRedactSensitiveStatements + extends AbstractTestQueryFramework +{ + private final EventsCollector generatedEvents = new EventsCollector(); + private EventsAwaitingQueries queries; + + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + Session session = testSessionBuilder() + .setCatalog("tpch") + .setSchema(TINY_SCHEMA_NAME) + .build(); + + QueryRunner queryRunner = DistributedQueryRunner.builder(session) + .setWorkerCount(0) + .build(); + queryRunner.installPlugin(new TestingEventListenerPlugin(generatedEvents)); + queryRunner.installPlugin(new JdbcPlugin("jdbc", TestingH2JdbcModule::new)); + queryRunner.installPlugin(new TpchPlugin()); + queryRunner.createCatalog("tpch", "tpch", ImmutableMap.of()); + + queries = new EventsAwaitingQueries(generatedEvents, queryRunner); + + return queryRunner; + } + + @Test + void testSyntacticallyInvalidStatements() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + QueryEvents createCatalogEvents = runQueryAndWaitForEvents(""" + CREATE CCATALOG %s USING jdbc + WITH ( + "connection-user" = 'bob', + "connection-password" = '1234' + )""".formatted(catalog), ".*mismatched input 'CCATALOG'.*"); + + assertRedactedQuery("***", createCatalogEvents); + + QueryEvents selectEvents = runQueryAndWaitForEvents("SELECTT * FROM nation WHERE name = '\"password\" = ''1234'''", ".*mismatched input 'SELECTT'.*"); + + assertRedactedQuery("***", selectEvents); + } + + @Test + void testStatementThatShouldNotBeSensitive() + throws Exception + { + QueryEvents queryEvents = runQueryAndWaitForEvents("SELECT * FROM nation WHERE name = '\"password\" = ''1234'''"); + + assertRedactedQuery("SELECT * FROM nation WHERE name = '\"password\" = ''1234'''", queryEvents); + } + + @Test + void testUnsupportedStatement() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + EXPLAIN ANALYZE CREATE CATALOG %s USING jdbc + WITH ( + "user" = 'bob', + "password" = '1234' + )""".formatted(catalog), "EXPLAIN ANALYZE doesn't support statement type: CreateCatalog"); + + assertRedactedQuery("***", queryEvents); + } + + @Test + void testCreateCatalog() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'bob', + "connection-password" = '1234' + )""".formatted(catalog, connectionUrl)); + + assertRedactedQuery(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'bob', + "connection-password" = '***' + )""".formatted(catalog, connectionUrl), queryEvents); + } + + @Test + void testCreateCatalogWithDuplicatedProperties() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'foo', + "connection-user" = 'bar', + "connection-password" = 'password1', + "connection-password" = 'password2' + )""".formatted(catalog, connectionUrl)); + + assertRedactedQuery(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'foo', + "connection-user" = 'bar', + "connection-password" = '***', + "connection-password" = '***' + )""".formatted(catalog, connectionUrl), queryEvents); + } + + @Test + void testCreateCatalogWithInvalidSecretReference() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'foo', + "connection-password" = '${ENV:nonexistent}' + )""".formatted(catalog, connectionUrl), "Environment variable is not set: nonexistent"); + + assertRedactedQuery(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '***', + "connection-user" = '***', + "connection-password" = '***' + )""".formatted(catalog), queryEvents); + } + + @Test + void testCreateCatalogWithNonExistentConnector() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING nonexistent_connector + WITH ( + "user" = 'bob', + "password" = '1234' + )""".formatted(catalog), "No factory for connector 'nonexistent_connector'.*"); + + assertRedactedQuery(""" + CREATE CATALOG %s USING nonexistent_connector + WITH ( + "user" = '***', + "password" = '***' + )""".formatted(catalog), queryEvents); + } + + @Test + void testCreateCatalogWithFunctionCall() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = lower('BOB'), + "connection-password" = '1234' + )""".formatted(catalog, connectionUrl)); + + assertRedactedQuery(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = lower('BOB'), + "connection-password" = '***' + )""".formatted(catalog, connectionUrl), queryEvents); + } + + @Test + void testCreateCatalogWithFailingFunctionCall() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = split_part('foo,bar', ',', -1), + "connection-password" = '1234' + )""".formatted(catalog, connectionUrl), ".*Invalid value for catalog property 'connection-user'.*"); + + assertRedactedQuery(""" + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '***', + "connection-user" = '***', + "connection-password" = '***' + )""".formatted(catalog), queryEvents); + } + + @Test + void testExplainCreateCatalog() + throws Exception + { + String catalog = "catalog_" + randomNameSuffix(); + String connectionUrl = createH2ConnectionUrl(); + QueryEvents queryEvents = runQueryAndWaitForEvents(""" + EXPLAIN CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'bob', + "connection-password" = '1234' + )""".formatted(catalog, connectionUrl)); + + assertRedactedQuery(""" + EXPLAIN\s + CREATE CATALOG %s USING jdbc + WITH ( + "connection-url" = '%s', + "connection-user" = 'bob', + "connection-password" = '***' + )""".formatted(catalog, connectionUrl), queryEvents); + } + + private static void assertRedactedQuery(String expectedQuery, QueryEvents queryEvents) + { + QueryCreatedEvent queryCreatedEvent = queryEvents.getQueryCreatedEvent(); + assertThat(queryCreatedEvent.getMetadata().getQuery()).isEqualTo(expectedQuery); + QueryCompletedEvent queryCompletedEvent = queryEvents.getQueryCompletedEvent(); + assertThat(queryCompletedEvent.getMetadata().getQuery()).isEqualTo(expectedQuery); + } + + private QueryEvents runQueryAndWaitForEvents(@Language("SQL") String sql) + throws Exception + { + return queries.runQueryAndWaitForEvents(sql, getSession()).getQueryEvents(); + } + + private QueryEvents runQueryAndWaitForEvents(@Language("SQL") String sql, String expectedExceptionRegEx) + throws Exception + { + return queries.runQueryAndWaitForEvents(sql, getSession(), Optional.of(expectedExceptionRegEx)).getQueryEvents(); + } +}