Skip to content

Commit

Permalink
[PL7XdSMV] apoc.config.list and apoc.config.map leak evars (#3400)
Browse files Browse the repository at this point in the history
  • Loading branch information
vga91 committed Jan 31, 2023
1 parent 1b8bbdc commit ad1835b
Show file tree
Hide file tree
Showing 5 changed files with 138 additions and 9 deletions.
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 @@ -12,16 +12,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 @@ -40,16 +96,23 @@ public ConfigResult(String key, Object value) {
@Procedure
public Stream<ConfigResult> 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)));
}

@Admin
@Description("apoc.config.map | Lists the Neo4j configuration as map")
@Procedure
public Stream<MapResult> 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"));
});
}

}

0 comments on commit ad1835b

Please sign in to comment.