Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[PL7XdSMV] apoc.config.list and apoc.config.map leak evars #3400

Merged
merged 1 commit into from
Jan 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 25 additions & 0 deletions docs/asciidoc/modules/ROOT/partials/usage/allowed.configs.adoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
[NOTE]
====
Please note that this procedure only gets the following configurations, to avoid retrieving sensitive data:
- `apoc.import.file.enabled`,
- `apoc.import.file.use_neo4j_config`,
- `apoc.import.file.allow_read_from_filesystem`,
- `apoc.export.file.enabled`,
- `apoc.trigger.enabled`,
- `apoc.trigger.refresh`,
- `apoc.uuid.enabled`,
- `apoc.uuid.enabled.<databaseName>`,
- `apoc.ttl.enabled`,
- `apoc.ttl.enabled.<databaseName>`,
- `apoc.ttl.schedule`
- `apoc.ttl.limit`
- `apoc.jobs.scheduled.num_threads`,
- `apoc.jobs.pool.num_threads`,
- `apoc.jobs.queue.size`
- `apoc.http.timeout.connect`
- `apoc.http.timeout.read`
- `apoc.custom.procedures.refresh`
- `apoc.spatial.geocode.osm.throttle`
- `apoc.spatial.geocode.google.throttle`
====
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,6 @@ RETURN key, value;
| "apoc.import.file.allow_read_from_filesystem" | "true"
| "apoc.trigger.enabled" | "false"
| "apoc.uuid.enabled" | "false"
|===
|===

include::partial$usage/allowed.configs.adoc[]
Original file line number Diff line number Diff line change
Expand Up @@ -12,4 +12,7 @@ RETURN apoc.map.clean(value, keys, []) AS value;
|===
| value
| {`apoc.uuid.enabled`: "false", `apoc.import.file.allow_read_from_filesystem`: "true", `apoc.ttl.enabled`: "true", `apoc.trigger.enabled`: "false", `apoc.ttl.limit`: "1000", `apoc.import.file.enabled`: "false", `apoc.ttl.schedule`: "PT1M", `apoc.export.file.enabled`: "false", apoc_ttl_enabled: "true", `apoc.import.file.use_neo4j_config`: "true"}
|===
|===


include::partial$usage/allowed.configs.adoc[]
67 changes: 65 additions & 2 deletions extended/src/main/java/apoc/config/Config.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,72 @@
import org.neo4j.procedure.Procedure;

import java.util.Map;
import java.util.Set;
import java.util.stream.Collectors;
import java.util.stream.Stream;

import static apoc.ApocConfig.APOC_CONFIG_JOBS_POOL_NUM_THREADS;
import static apoc.ApocConfig.APOC_CONFIG_JOBS_QUEUE_SIZE;
import static apoc.ApocConfig.APOC_CONFIG_JOBS_SCHEDULED_NUM_THREADS;
import static apoc.ApocConfig.APOC_EXPORT_FILE_ENABLED;
import static apoc.ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM;
import static apoc.ApocConfig.APOC_IMPORT_FILE_ENABLED;
import static apoc.ApocConfig.APOC_IMPORT_FILE_USE_NEO4J_CONFIG;
import static apoc.ApocConfig.APOC_TRIGGER_ENABLED;
import static apoc.ExtendedApocConfig.APOC_TTL_ENABLED;
import static apoc.ExtendedApocConfig.APOC_TTL_LIMIT;
import static apoc.ExtendedApocConfig.APOC_TTL_SCHEDULE;
import static apoc.ExtendedApocConfig.APOC_UUID_ENABLED;
import static apoc.ExtendedApocConfig.APOC_UUID_FORMAT;
import static apoc.custom.CypherProceduresHandler.CUSTOM_PROCEDURES_REFRESH;

/**
* @author mh
* @since 28.10.16
*/
@Extended
public class Config {

// some config keys are hard-coded because belong to `core`, which is no longer accessed from `extended`
private static final Set<String> WHITELIST_CONFIGS = Set.of(
// apoc.import.
APOC_IMPORT_FILE_ENABLED,
APOC_IMPORT_FILE_USE_NEO4J_CONFIG,
APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM,

// apoc.export.
APOC_EXPORT_FILE_ENABLED,

// apoc.trigger.
APOC_TRIGGER_ENABLED,
"apoc.trigger.refresh",

// apoc.uuid.
APOC_UUID_ENABLED,
APOC_UUID_FORMAT,

// apoc.ttl.
APOC_TTL_SCHEDULE,
APOC_TTL_ENABLED,
APOC_TTL_LIMIT,

// apoc.jobs.
APOC_CONFIG_JOBS_SCHEDULED_NUM_THREADS,
APOC_CONFIG_JOBS_POOL_NUM_THREADS,
APOC_CONFIG_JOBS_QUEUE_SIZE,

// apoc.http.
"apoc.http.timeout.connect",
"apoc.http.timeout.read",

// apoc.custom.
CUSTOM_PROCEDURES_REFRESH,

// apoc.spatial. - other configs can have sensitive credentials
"apoc.spatial.geocode.osm.throttle",
"apoc.spatial.geocode.google.throttle"
);

public static class ConfigResult {
public final String key;
public final Object value;
Expand All @@ -48,16 +104,23 @@ public ConfigResult(String key, Object value) {
public Stream<ConfigResult> list() {
Util.checkAdmin(securityContext, callContext,"apoc.config.list");
Configuration config = dependencyResolver.resolveDependency(ApocConfig.class).getConfig();
return Iterators.stream(config.getKeys()).map(s -> new ConfigResult(s, config.getString(s)));
return getApocConfigs(config)
.map(s -> new ConfigResult(s, config.getString(s)));
}

@Description("apoc.config.map | Lists the Neo4j configuration as map")
@Procedure
public Stream<MapResult> map() {
Util.checkAdmin(securityContext,callContext, "apoc.config.map");
Configuration config = dependencyResolver.resolveDependency(ApocConfig.class).getConfig();
Map<String, Object> configMap = Iterators.stream(config.getKeys())
Map<String, Object> configMap = getApocConfigs(config)
.collect(Collectors.toMap(s -> s, s -> config.getString(s)));
return Stream.of(new MapResult(configMap));
}

private static Stream<String> getApocConfigs(Configuration config) {
// we use startsWith(..) because we can have e.g. a config `apoc.uuid.enabled.<dbName>`
return Iterators.stream(config.getKeys())
.filter(conf -> WHITELIST_CONFIGS.stream().anyMatch(conf::startsWith));
}
}
46 changes: 41 additions & 5 deletions extended/src/test/java/apoc/config/ConfigTest.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
package apoc.config;

import apoc.util.TestUtil;
import org.junit.Assert;
import apoc.util.collection.Iterators;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
Expand All @@ -11,15 +11,41 @@
import org.neo4j.test.rule.DbmsRule;
import org.neo4j.test.rule.ImpermanentDbmsRule;

import java.util.Map;
import java.util.Set;

import static apoc.ApocConfig.APOC_CONFIG_JOBS_SCHEDULED_NUM_THREADS;
import static apoc.ApocConfig.APOC_EXPORT_FILE_ENABLED;
import static apoc.ApocConfig.APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM;
import static apoc.ApocConfig.APOC_IMPORT_FILE_ENABLED;
import static apoc.ApocConfig.APOC_IMPORT_FILE_USE_NEO4J_CONFIG;
import static apoc.ApocConfig.APOC_TRIGGER_ENABLED;
import static apoc.trigger.TriggerHandler.TRIGGER_REFRESH;
import static org.junit.Assert.assertEquals;

/**
* @author mh
* @since 28.10.16
*/
public class ConfigTest {
// apoc.export.file.enabled, apoc.import.file.enabled, apoc.import.file.use_neo4j_config, apoc.trigger.enabled and apoc.import.file.allow_read_from_filesystem
// are always set by default
private static final Map<String, String> EXPECTED_APOC_CONFS = Map.of(APOC_EXPORT_FILE_ENABLED, "false",
APOC_IMPORT_FILE_ALLOW__READ__FROM__FILESYSTEM, "true",
APOC_IMPORT_FILE_ENABLED, "false",
APOC_IMPORT_FILE_USE_NEO4J_CONFIG, "true",
APOC_CONFIG_JOBS_SCHEDULED_NUM_THREADS, "4",
TRIGGER_REFRESH, "2000",
APOC_TRIGGER_ENABLED, "false");

@Rule
public final ProvideSystemProperty systemPropertyRule
= new ProvideSystemProperty("foo", "bar");
public final ProvideSystemProperty systemPropertyRule
= new ProvideSystemProperty("foo", "bar")
.and("apoc.import.enabled", "true")
.and("apoc.trigger.refresh", "2000")
.and("apoc.jobs.scheduled.num_threads", "4")
.and("apoc.static.test", "one")
.and("apoc.jdbc.cassandra_songs.url", "jdbc:cassandra://localhost:9042/playlist");

@Rule
public DbmsRule db = new ImpermanentDbmsRule();
Expand All @@ -35,8 +61,18 @@ public void tearDown() {
}

@Test
public void listTest(){
TestUtil.testCall(db, "CALL apoc.config.list() yield key with * where key STARTS WITH 'foo' RETURN *",(row) -> Assert.assertEquals("foo",row.get("key")));
public void configListTest(){
TestUtil.testResult(db, "CALL apoc.config.list() YIELD key RETURN * ORDER BY key", r -> {
final Set<String> actualConfs = Iterators.asSet(r.columnAs("key"));
assertEquals(EXPECTED_APOC_CONFS.keySet(), actualConfs);
});
}

@Test
public void configMapTest(){
TestUtil.testCall(db, "CALL apoc.config.map()", r -> {
assertEquals(EXPECTED_APOC_CONFS, r.get("value"));
});
}

}