Skip to content

Commit

Permalink
Introduce redaction for sensitive statements
Browse files Browse the repository at this point in the history
This commit introduces redacting of security-sensitive information in
statements containing connector 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 valid query references a nonexistent connector, all properties
are masked.
* If a query fails before or during parsing, the entire query is masked

The redacted form is created in DispatchManager and is propagated to
all places that create QueryInfo and BasicQueryInfo. 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.
  • Loading branch information
piotrrzysko committed Dec 23, 2024
1 parent 8018b45 commit 11a7ed1
Show file tree
Hide file tree
Showing 11 changed files with 415 additions and 4 deletions.
14 changes: 14 additions & 0 deletions core/trino-main/src/main/java/io/trino/FeaturesConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ public class FeaturesConfig

private boolean faultTolerantExecutionExchangeEncryptionEnabled = true;

private boolean statementRedactingEnabled = true;

public enum DataIntegrityVerification
{
NONE,
Expand Down Expand Up @@ -514,6 +516,18 @@ public FeaturesConfig setFaultTolerantExecutionExchangeEncryptionEnabled(boolean
return this;
}

public boolean isStatementRedactingEnabled()
{
return statementRedactingEnabled;
}

@Config("statement-redacting-enabled")
public FeaturesConfig setStatementRedactingEnabled(boolean statementRedactingEnabled)
{
this.statementRedactingEnabled = statementRedactingEnabled;
return this;
}

public void applyFaultTolerantExecutionDefaults()
{
exchangeCompressionCodec = LZ4;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@
import io.trino.spi.connector.ConnectorFactory;
import io.trino.spi.connector.ConnectorName;

import java.util.Set;

@ThreadSafe
public interface CatalogFactory
{
Expand All @@ -28,4 +30,6 @@ public interface CatalogFactory
CatalogConnector createCatalog(CatalogProperties catalogProperties);

CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector);

Set<String> getRedactablePropertyNames(ConnectorName connectorName, Set<String> propertyNames);
}
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@

import java.util.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;

Expand Down Expand Up @@ -144,6 +145,18 @@ public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName
return createCatalog(catalogHandle, connectorName, connector, Optional.empty());
}

@Override
public Set<String> getRedactablePropertyNames(ConnectorName connectorName, Set<String> propertyNames)
{
ConnectorFactory connectorFactory = connectorFactories.get(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 propertyNames;
}
return connectorFactory.getRedactablePropertyNames(propertyNames);
}

private CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName connectorName, Connector connector, Optional<CatalogProperties> catalogProperties)
{
Tracer tracer = createTracer(catalogHandle);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -51,6 +52,12 @@ public CatalogConnector createCatalog(CatalogHandle catalogHandle, ConnectorName
return getDelegate().createCatalog(catalogHandle, connectorName, connector);
}

@Override
public Set<String> getRedactablePropertyNames(ConnectorName connectorName, Set<String> propertyNames)
{
return getDelegate().getRedactablePropertyNames(connectorName, propertyNames);
}

private CatalogFactory getDelegate()
{
CatalogFactory catalogFactory = delegate.get();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@
import io.trino.spi.TrinoException;
import io.trino.spi.resourcegroups.SelectionContext;
import io.trino.spi.resourcegroups.SelectionCriteria;
import io.trino.sql.SensitiveStatementRedactor;
import jakarta.annotation.PostConstruct;
import jakarta.annotation.PreDestroy;
import org.weakref.jmx.Flatten;
Expand Down Expand Up @@ -84,6 +85,7 @@ public class DispatchManager
private final SessionPropertyDefaults sessionPropertyDefaults;
private final SessionPropertyManager sessionPropertyManager;
private final Tracer tracer;
private final SensitiveStatementRedactor sensitiveStatementRedactor;

private final int maxQueryLength;

Expand All @@ -107,6 +109,7 @@ public DispatchManager(
SessionPropertyDefaults sessionPropertyDefaults,
SessionPropertyManager sessionPropertyManager,
Tracer tracer,
SensitiveStatementRedactor sensitiveStatementRedactor,
QueryManagerConfig queryManagerConfig,
DispatchExecutor dispatchExecutor,
QueryMonitor queryMonitor)
Expand All @@ -121,6 +124,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();

Expand Down Expand Up @@ -207,6 +211,7 @@ private <C> void createQueryInternal(QueryId queryId, Span querySpan, Slug slug,
{
Session session = null;
PreparedQuery preparedQuery = null;
String redactedQuery = null;
try {
if (query.length() > maxQueryLength) {
int queryLength = query.length();
Expand All @@ -223,6 +228,9 @@ private <C> void createQueryInternal(QueryId queryId, Span querySpan, Slug slug,
// prepare query
preparedQuery = queryPreparer.prepareQuery(session, query);

// redact security-sensitive information that query may contain
redactedQuery = sensitiveStatementRedactor.redact(query, preparedQuery.getStatement());

// select resource group
Optional<String> queryType = getQueryType(preparedQuery.getStatement()).map(Enum::name);
SelectionContext<C> selectionContext = resourceGroupManager.selectGroup(new SelectionCriteria(
Expand All @@ -240,7 +248,7 @@ private <C> void createQueryInternal(QueryId queryId, Span querySpan, Slug slug,
DispatchQuery dispatchQuery = dispatchQueryFactory.createDispatchQuery(
session,
sessionContext.getTransactionId(),
query,
redactedQuery,
preparedQuery,
slug,
selectionContext.getResourceGroupId());
Expand All @@ -266,8 +274,16 @@ private <C> void createQueryInternal(QueryId queryId, Span querySpan, Slug slug,
.setSource(sessionContext.getSource().orElse(null))
.build();
}
if (redactedQuery == null) {
redactedQuery = sensitiveStatementRedactor.redact(query);
}
Optional<String> 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -304,6 +305,9 @@ List<OutputStatsEstimatorFactory> 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);
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,131 @@
/*
* 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.inject.Inject;
import io.trino.FeaturesConfig;
import io.trino.connector.CatalogFactory;
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.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.Set;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

public class SensitiveStatementRedactor
{
public static final String REDACTED_VALUE = "***";

private final boolean enabled;
private final CatalogFactory catalogFactory;

@Inject
public SensitiveStatementRedactor(FeaturesConfig config, CatalogFactory catalogFactory)
{
this.enabled = config.isStatementRedactingEnabled();
this.catalogFactory = catalogFactory;
}

public String redact(String rawQuery, Statement statement)
{
if (enabled) {
RedactingVisitor visitor = new RedactingVisitor();
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<Node, Void>
{
private boolean redacted;

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());
List<Property> redactedProperties = redact(connectorName, createCatalog.getProperties());
return createCatalog.withProperties(redactedProperties);
}

private List<Property> redact(ConnectorName connectorName, List<Property> properties)
{
redacted = true;
Set<String> propertyNames = properties.stream()
.map(Property::getName)
.map(Identifier::getValue)
.collect(toImmutableSet());
Set<String> redactableProperties = catalogFactory.getRedactablePropertyNames(connectorName, propertyNames);
return redactProperties(properties, redactableProperties);
}

private List<Property> redactProperties(List<Property> properties, Set<String> redactableProperties)
{
return properties.stream()
.map(property -> {
if (redactableProperties.contains(property.getName().getValue())) {
return new Property(property.getName(), new StringLiteral(REDACTED_VALUE));
}
return property;
})
.collect(toImmutableList());
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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<String> getRedactablePropertyNames(ConnectorName connectorName, Set<String> propertyNames)
{
throw new UnsupportedOperationException("Only implement what is needed by worker catalog manager");
}
};
private static final TaskExecutor NOOP_TASK_EXECUTOR = new TaskExecutor() {
@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ public void testDefaults()
.setHideInaccessibleColumns(false)
.setForceSpillingJoin(false)
.setColumnarFilterEvaluationEnabled(true)
.setFaultTolerantExecutionExchangeEncryptionEnabled(true));
.setFaultTolerantExecutionExchangeEncryptionEnabled(true)
.setStatementRedactingEnabled(true));
}

@Test
Expand Down Expand Up @@ -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("statement-redacting-enabled", "false")
.buildOrThrow();

FeaturesConfig expected = new FeaturesConfig()
Expand Down Expand Up @@ -131,7 +133,8 @@ public void testExplicitPropertyMappings()
.setHideInaccessibleColumns(true)
.setForceSpillingJoin(true)
.setColumnarFilterEvaluationEnabled(false)
.setFaultTolerantExecutionExchangeEncryptionEnabled(false);
.setFaultTolerantExecutionExchangeEncryptionEnabled(false)
.setStatementRedactingEnabled(false);
assertFullMapping(properties, expected);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,18 @@ public CreateCatalog(
this.comment = requireNonNull(comment, "comment is null");
}

public CreateCatalog withProperties(List<Property> properties)
{
return new CreateCatalog(
getLocation().orElseThrow(),
catalogName,
notExists,
connectorName,
properties,
principal,
comment);
}

public Identifier getCatalogName()
{
return catalogName;
Expand Down
Loading

0 comments on commit 11a7ed1

Please sign in to comment.