Skip to content

Commit

Permalink
Check permission before resolving mapping fields [HZ-2991] [5.3.z]
Browse files Browse the repository at this point in the history 
`SqlConnector#resolveAndValidateFields` may try to resolve field names
from metadata and a sample. This is not covered by any permission check.

Added `SqlConnector#permissionsForResolve` that returns permissions
required to run the `resolveAndValidateFields`. Propagation of
SqlSecurityContext was required for the actuall permission check.

EE PR #... (adds security tests)

Backport of hazelcast#25348
  • Loading branch information
@frant-hartm
frant-hartm committed Oct 2, 2023
1 parent b1e62a5 commit ca0f011
Show file tree
Hide file tree
Showing 12 changed files with 112 additions and 49 deletions.
5 changes: 3 additions & 2 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/PlanExecutor.java
View file
Expand Up @@ -93,6 +93,7 @@
import com.hazelcast.sql.impl.schema.type.Type;
import com.hazelcast.sql.impl.schema.type.TypeKind;
import com.hazelcast.sql.impl.schema.view.View;
import com.hazelcast.sql.impl.security.SqlSecurityContext;
import com.hazelcast.sql.impl.state.QueryResultRegistry;
import com.hazelcast.sql.impl.type.QueryDataType;
import org.apache.calcite.rel.RelNode;
Expand Down Expand Up @@ -171,8 +172,8 @@ public PlanExecutor(
logger = nodeEngine.getLogger(getClass());
}

SqlResult execute(CreateMappingPlan plan) {
catalog.createMapping(plan.mapping(), plan.replace(), plan.ifNotExists());
SqlResult execute(CreateMappingPlan plan, SqlSecurityContext ssc) {
catalog.createMapping(plan.mapping(), plan.replace(), plan.ifNotExists(), ssc);
return UpdateSqlResultImpl.createUpdateCountResult(0);
}

Expand Down
50 changes: 25 additions & 25 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/SqlPlanImpl.java
View file
Expand Up @@ -148,10 +148,10 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE MAPPING", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE MAPPING", timeout);
return planExecutor.execute(this);
return planExecutor.execute(this, ssc);
}
}

Expand Down Expand Up @@ -197,7 +197,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP MAPPING", arguments);
SqlPlanImpl.ensureNoTimeout("DROP MAPPING", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -278,7 +278,7 @@ public void checkPermissions(SqlSecurityContext context) {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE DATA CONNECTION", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE DATA CONNECTION", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -327,7 +327,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoTimeout("DROP DATA CONNECTION", timeout);
return planExecutor.execute(this);
}
Expand Down Expand Up @@ -403,7 +403,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE INDEX", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE INDEX", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -452,7 +452,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
throw QueryException.error("DROP INDEX is not supported.");
}
}
Expand Down Expand Up @@ -530,7 +530,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoTimeout("CREATE JOB", timeout);
return planExecutor.execute(this, arguments);
}
Expand Down Expand Up @@ -580,7 +580,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("ALTER JOB", arguments);
SqlPlanImpl.ensureNoTimeout("ALTER JOB", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -631,7 +631,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP JOB", arguments);
SqlPlanImpl.ensureNoTimeout("DROP JOB", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -675,7 +675,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE SNAPSHOT", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE SNAPSHOT", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -719,7 +719,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP SNAPSHOT", arguments);
SqlPlanImpl.ensureNoTimeout("DROP SNAPSHOT", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -789,7 +789,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE VIEW", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE VIEW", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -838,7 +838,7 @@ public void checkPermissions(SqlSecurityContext context) {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP VIEW", arguments);
SqlPlanImpl.ensureNoTimeout("DROP VIEW", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -911,7 +911,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("CREATE TYPE", arguments);
SqlPlanImpl.ensureNoTimeout("CREATE TYPE", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -960,7 +960,7 @@ public void checkPermissions(SqlSecurityContext context) {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("DROP TYPE", arguments);
SqlPlanImpl.ensureNoTimeout("DROP TYPE", timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -1004,7 +1004,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoArguments("SHOW " + showTarget, arguments);
SqlPlanImpl.ensureNoTimeout("SHOW " + showTarget, timeout);
return planExecutor.execute(this);
Expand Down Expand Up @@ -1040,7 +1040,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
SqlPlanImpl.ensureNoTimeout("EXPLAIN", timeout);
return planExecutor.execute(this);
}
Expand Down Expand Up @@ -1121,7 +1121,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -1201,7 +1201,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -1289,7 +1289,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, queryId, arguments, timeout);
}
}
Expand Down Expand Up @@ -1363,7 +1363,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1429,7 +1429,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1511,7 +1511,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down Expand Up @@ -1586,7 +1586,7 @@ public boolean producesRows() {
}

@Override
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout) {
public SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc) {
return planExecutor.execute(this, arguments, timeout);
}
}
Expand Down
18 changes: 18 additions & 0 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/SqlConnector.java
View file
Expand Up @@ -38,12 +38,14 @@
import javax.annotation.Nullable;
import java.io.Serializable;
import java.lang.reflect.Method;
import java.security.Permission;
import java.util.Arrays;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.function.Consumer;

import static java.util.Collections.emptyList;
import static java.util.Collections.emptyMap;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -231,6 +233,22 @@ List<MappingField> resolveAndValidateFields(
@Nonnull List<MappingField> userFields
);

/**
* Returns the required permissions to execute
* {@link #resolveAndValidateFields(NodeEngine, SqlExternalResource, List)} method.
* <p>
* Implementors of {@link SqlConnector} don't need to override this method when {@code resolveAndValidateFields}
* doesn't support field resolution or when validation doesn't access the external resource.
* <p>
* The permissions are usually the same as required permissions to read from the external resource.
*
* @return list of permissions required to run {@link #resolveAndValidateFields}
*/
@Nonnull
default List<Permission> permissionsForResolve(SqlExternalResource resource, NodeEngine nodeEngine) {
return emptyList();
}

/**
* Creates a {@link Table} object with the given fields. Should return
* quickly; specifically it should not attempt to connect to the remote
Expand Down
10 changes: 10 additions & 0 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/connector/file/FileSqlConnector.java
View file
Expand Up @@ -31,10 +31,14 @@

import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import java.security.Permission;
import java.util.List;
import java.util.Map;

import static com.hazelcast.jet.core.Edge.between;
import static com.hazelcast.security.permission.ActionConstants.ACTION_READ;
import static com.hazelcast.security.permission.ConnectorPermission.file;
import static java.util.Collections.singletonList;

public class FileSqlConnector implements SqlConnector {

Expand Down Expand Up @@ -83,6 +87,12 @@ static List<MappingField> resolveAndValidateFields(
return METADATA_RESOLVERS.resolveAndValidateFields(userFields, options);
}

@Nonnull
@Override
public List<Permission> permissionsForResolve(SqlExternalResource resource, NodeEngine nodeEngine) {
return singletonList(file(resource.options().get(OPTION_PATH), ACTION_READ));
}

@Nonnull
@Override
public Table createTable(
Expand Down
29 changes: 20 additions & 9 deletions hazelcast-sql/src/main/java/com/hazelcast/jet/sql/impl/schema/TableResolverImpl.java
View file
Expand Up @@ -41,8 +41,10 @@
import com.hazelcast.sql.impl.schema.dataconnection.DataConnectionCatalogEntry;
import com.hazelcast.sql.impl.schema.type.Type;
import com.hazelcast.sql.impl.schema.view.View;
import com.hazelcast.sql.impl.security.SqlSecurityContext;

import javax.annotation.Nonnull;
import java.security.Permission;
import java.util.ArrayList;
import java.util.Collection;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -127,8 +129,8 @@ public void entryRemoved(EntryEvent<String, Object> event) {

// region mapping

public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists) {
Mapping resolved = resolveMapping(mapping);
public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists, SqlSecurityContext securityContext) {
Mapping resolved = resolveMapping(mapping, securityContext);

String name = resolved.name();
if (ifNotExists) {
Expand All @@ -141,7 +143,7 @@ public void createMapping(Mapping mapping, boolean replace, boolean ifNotExists)
}
}

private Mapping resolveMapping(Mapping mapping) {
private Mapping resolveMapping(Mapping mapping, SqlSecurityContext securityContext) {
Map<String, String> options = mapping.options();
String type = mapping.connectorType();
String dataConnection = mapping.dataConnection();
Expand All @@ -157,14 +159,23 @@ private Mapping resolveMapping(Mapping mapping) {
? connector.defaultObjectType()
: mapping.objectType();
checkNotNull(objectType, "objectType cannot be null");

SqlExternalResource externalResource = new SqlExternalResource(
mapping.externalName(),
mapping.dataConnection(),
connector.typeName(),
objectType,
options
);

List<Permission> permissions = connector.permissionsForResolve(externalResource, nodeEngine);
for (Permission permission : permissions) {
securityContext.checkPermission(permission);
}

resolvedFields = connector.resolveAndValidateFields(
nodeEngine,
new SqlExternalResource(
mapping.externalName(),
mapping.dataConnection(),
connector.typeName(),
objectType,
options),
externalResource,
mapping.fields()
);

Expand Down
2 changes: 1 addition & 1 deletion hazelcast-sql/src/main/java/com/hazelcast/sql/impl/SqlServiceImpl.java
View file
Expand Up @@ -263,7 +263,7 @@ private SqlResult query0(
}

// TODO: pageSize ?
return plan.execute(queryId, args0, timeout);
return plan.execute(queryId, args0, timeout, securityContext);
}

private SqlPlan prepare(String schema, String sql, List<Object> arguments, SqlExpectedResultType expectedResultType) {
Expand Down
2 changes: 1 addition & 1 deletion hazelcast-sql/src/main/java/com/hazelcast/sql/impl/optimizer/SqlPlan.java
View file
Expand Up @@ -71,5 +71,5 @@ public long getPlanLastUsed() {
*/
public abstract boolean producesRows();

public abstract SqlResult execute(QueryId queryId, List<Object> arguments, long timeout);
public abstract SqlResult execute(QueryId queryId, List<Object> arguments, long timeout, SqlSecurityContext ssc);
}
4 changes: 2 additions & 2 deletions hazelcast-sql/src/test/java/com/hazelcast/jet/sql/impl/PlanExecutorTest.java
View file
Expand Up @@ -102,11 +102,11 @@ public void test_createMappingExecution(boolean replace, boolean ifNotExists) {
CreateMappingPlan plan = new CreateMappingPlan(planKey(), mapping, replace, ifNotExists, planExecutor);

// when
SqlResult result = planExecutor.execute(plan);
SqlResult result = planExecutor.execute(plan, null);

// then
assertThat(result.updateCount()).isEqualTo(0);
verify(catalog).createMapping(mapping, replace, ifNotExists);
verify(catalog).createMapping(mapping, replace, ifNotExists, null);
}

@Test
Expand Down

0 comments on commit ca0f011

Please sign in to comment.