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(); + } +}