Skip to content

Commit

Permalink
NO AUTO Adds blocklist for load methods (#3119)
Browse files Browse the repository at this point in the history
  • Loading branch information
ncordon authored Aug 8, 2022
1 parent 2ed8825 commit 7b7fc7d
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 5 deletions.
1 change: 1 addition & 0 deletions build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ dependencies {
// apt 'net.biville.florent:neo4j-sproc-compiler:1.2' // temporarily disabled until byte[] is supported by sproc compiler
apt group: 'org.neo4j', name: 'neo4j', version: neo4jVersionEffective
compile group: 'commons-codec', name: 'commons-codec', version: '1.14'
compile group: 'com.github.seancfoley', name: 'ipaddress', version: '5.3.3'
compileOnly group: 'com.sun.mail', name: 'javax.mail', version: '1.6.0'
testCompile group: 'com.sun.mail', name: 'javax.mail', version: '1.6.0'
compile group: 'com.jayway.jsonpath', name: 'json-path', version: '2.4.0'
Expand Down
84 changes: 84 additions & 0 deletions src/main/java/apoc/ApocConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,28 @@
import apoc.cache.Static;
import apoc.util.FileUtils;
import apoc.util.Util;
import inet.ipaddr.AddressStringException;
import inet.ipaddr.IPAddressString;

import org.neo4j.graphdb.config.Setting;
import org.neo4j.kernel.configuration.Config;
import org.neo4j.kernel.configuration.Settings;
import org.neo4j.kernel.internal.GraphDatabaseAPI;

import java.io.IOException;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URL;
import java.util.Arrays;
import java.util.HashMap;
import java.util.Map;
import java.util.Optional;
import java.util.function.Function;
import java.util.regex.Pattern;
import java.util.List;

import static java.lang.String.format;
import static org.neo4j.kernel.configuration.Settings.setting;

/**
* @author mh
Expand All @@ -25,6 +40,9 @@ public class ApocConfiguration {

private static final Map<String, Object> DEFAULTS = Util.map("import.file.use_neo4j_config", true);

public static final String CYPHER_IP_BLOCKLIST = "unsupported.dbms.cypher_ip_blocklist";
private static List<IPAddressString> blockedIpRanges = Arrays.asList();

static {
PARAM_WHITELIST.put("dbms.security.allow_csv_import_from_file_urls", "import.file.allow_read_from_filesystem");

Expand All @@ -38,6 +56,7 @@ public static void initialize(GraphDatabaseAPI db) {
Static.clear();
Config neo4jConfig = db.getDependencyResolver().resolveDependency(Config.class);
Map<String, String> params = neo4jConfig.getRaw();
blockedIpRanges = neo4jConfig.get(ApocSettings.cypher_ip_blocklist);
apocConfig.clear();
apocConfig.putAll(Util.subMap(params, PREFIX));
mergeDefaults();
Expand Down Expand Up @@ -83,4 +102,69 @@ public static String getImportDir() {
return ApocConfiguration.get("dbms.directories.import", "").toString();
}


public IPAddressString parse( String value )
{
IPAddressString ipAddress = new IPAddressString( value.trim() );
try
{
ipAddress.validate();
}
catch ( AddressStringException e )
{
throw new IllegalArgumentException( format( "'%s' is not a valid CIDR ip", value ), e );
}
return ipAddress;
}


public static void checkAllowedUrl(String url) throws IOException
{
try {
if (blockedIpRanges != null && !blockedIpRanges.isEmpty()) {
URL parsedUrl = new URL( url);
InetAddress inetAddress = InetAddress.getByName( parsedUrl.getHost());

for (IPAddressString blockedIpRange : blockedIpRanges)
{
if (blockedIpRange.contains(new IPAddressString(inetAddress.getHostAddress())))
{
throw new IOException("access to " + inetAddress + " is blocked via the configuration property "
+ CYPHER_IP_BLOCKLIST);
}
}
}
} catch ( MalformedURLException e) {
throw new IOException(e);
}
}
}

class ApocSettings {
public static final Function<String, IPAddressString> CIDR_IP = new Function<String, IPAddressString>()
{
@Override
public IPAddressString apply(String value)
{
IPAddressString ipAddress = new IPAddressString(value.trim());
try
{
ipAddress.validate();
}
catch (AddressStringException e)
{
throw new IllegalArgumentException(format( "'%s' is not a valid CIDR ip", value), e);
}
return ipAddress;
}

@Override
public String toString()
{
return "an ip with subnet in CDIR format. e.g. 127.168.0.1/8";
}
};

public static final Setting<List<IPAddressString>> cypher_ip_blocklist = setting(
"unsupported.dbms.cypher_ip_blocklist", Settings.list(",", CIDR_IP), "");
}
10 changes: 7 additions & 3 deletions src/main/java/apoc/util/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -249,10 +249,14 @@ public static String getConfiguredImportDirectory() {
return ApocConfiguration.get("dbms.directories.import", "import");
}

public static void checkReadAllowed(String url) {
if (isFile(url) && !ApocConfiguration.isEnabled("import.file.enabled"))
throw new RuntimeException(LOAD_FROM_FILE_ERROR);
public static void checkReadAllowed(String url) throws IOException {
if (isFile(url) && !ApocConfiguration.isEnabled("import.file.enabled")) {
throw new RuntimeException( LOAD_FROM_FILE_ERROR );
} else {
ApocConfiguration.checkAllowedUrl(url);
}
}

public static void checkWriteAllowed(ExportConfig exportConfig, String fileName) {
if (!ApocConfiguration.isEnabled("export.file.enabled"))
if (exportConfig == null || (fileName != null && !fileName.equals("")) || !exportConfig.streamStatements()) {
Expand Down
1 change: 1 addition & 0 deletions src/main/java/apoc/util/Util.java
Original file line number Diff line number Diff line change
Expand Up @@ -418,6 +418,7 @@ private static InputStream getFileStreamIntoCompressedFile(InputStream is, Strin
}

public static StreamConnection readHttpInputStream(String urlAddress, Map<String, Object> headers, String payload) throws IOException {
FileUtils.checkReadAllowed(urlAddress);
URLConnection con = openUrlConnection(urlAddress, headers);
writePayload(con, payload);
String newUrl = handleRedirect(con, urlAddress);
Expand Down
23 changes: 21 additions & 2 deletions src/test/java/apoc/load/LoadJsonTest.java
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
package apoc.load;

import apoc.ApocConfiguration;
import apoc.util.TestUtil;
import org.apache.commons.lang.exception.ExceptionUtils;
import org.junit.After;
import org.junit.AfterClass;
import org.junit.Assert;
import org.junit.Before;
import org.junit.BeforeClass;
import org.junit.Ignore;
Expand All @@ -20,6 +22,7 @@
import java.util.List;
import java.util.Map;
import java.util.concurrent.TimeUnit;
import java.util.Arrays;

import static apoc.util.MapUtil.map;
import static apoc.util.TestUtil.testCall;
Expand Down Expand Up @@ -57,6 +60,7 @@ public static void stopServer() {
.setConfig("apoc.import.file.use_neo4j_config", "false")
.setConfig("apoc.json.zip.url","https://github.com/neo4j-contrib/neo4j-apoc-procedures/blob/3.4/src/test/resources/testload.zip?raw=true!person.json")
.setConfig("apoc.json.simpleJson.url", url.toString())
.setConfig(ApocConfiguration.CYPHER_IP_BLOCKLIST, "127.168.0.0/8")
.newGraphDatabase();
TestUtil.registerProcedure(db, LoadJson.class);
}
Expand All @@ -73,6 +77,21 @@ public static void stopServer() {
});
}

@Test public void testLoadJsonFromBlockedIpRange() throws Exception {
List<String> protocols = Arrays.asList("https", "http");

for (String protocol: protocols) {
QueryExecutionException e = Assert.assertThrows(QueryExecutionException.class,
() -> testCall(db,
"CALL apoc.load.json('" + protocol + "://127.168.0.0/test.csv')",
map(),
(r) -> {}
)
);
assertTrue(e.getMessage().contains("access to /127.168.0.0 is blocked via the configuration property unsupported.dbms.cypher_ip_blocklist"));
}
}

@Test public void testLoadMultiJson() throws Exception {
URL url = ClassLoader.getSystemResource("multi.json");
testResult(db, "CALL apoc.load.json({url})",map("url",url.toString()), // 'file:map.json' YIELD value RETURN value
Expand Down Expand Up @@ -268,8 +287,8 @@ public void testLoadJsonByUrlInConfigFileWrongKey() throws Exception {
Throwable except = ExceptionUtils.getRootCause(e);
assertTrue(except instanceof IOException);
final String message = except.getMessage();
assertTrue(message.startsWith("Cannot open file "));
assertTrue(message.endsWith("foo for reading."));
System.out.println("This is the message: " + message);
assertTrue(message.startsWith("no protocol: foo"));
throw e;
}
}
Expand Down

0 comments on commit 7b7fc7d

Please sign in to comment.