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

NO AUTO Adds blocklist for load methods #3118

Merged
merged 1 commit into from
Aug 8, 2022
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
1 change: 1 addition & 0 deletions core/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ dependencies {
exclude module: "snakeyaml"
}
compile group: 'org.yaml', name: 'snakeyaml', version: '1.26'
compile group: 'com.github.seancfoley', name: 'ipaddress', version: '5.3.3'
testCompile group: 'com.github.stefanbirkner', name: 'system-rules', version: '1.19.0'

testCompile 'net.sourceforge.jexcelapi:jxl:2.6.12'
Expand Down
40 changes: 38 additions & 2 deletions core/src/main/java/apoc/ApocConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import apoc.export.util.ExportConfig;
import apoc.util.SimpleRateLimiter;
import inet.ipaddr.IPAddressString;
import org.apache.commons.configuration2.Configuration;
import org.apache.commons.configuration2.PropertiesConfiguration;
import org.apache.commons.configuration2.builder.combined.CombinedConfigurationBuilder;
Expand All @@ -20,7 +21,10 @@
import org.neo4j.logging.NullLog;
import org.neo4j.logging.internal.LogService;

import java.io.IOException;
import java.lang.reflect.Field;
import java.net.InetAddress;
import java.net.MalformedURLException;
import java.net.URL;
import java.time.Duration;
import java.time.ZoneId;
Expand All @@ -43,8 +47,8 @@
import static org.neo4j.configuration.GraphDatabaseSettings.transaction_logs_root_path;

public class ApocConfig extends LifecycleAdapter {

public static final String SUN_JAVA_COMMAND = "sun.java.command";
public static final String CYPHER_IP_BLOCKLIST = "unsupported.dbms.cypher_ip_blocklist";
public static final String APOC_IMPORT_FILE_ENABLED = "apoc.import.file.enabled";
public static final String APOC_EXPORT_FILE_ENABLED = "apoc.export.file.enabled";
public static final String APOC_IMPORT_FILE_USE_NEO4J_CONFIG = "apoc.import.file.use_neo4j_config";
Expand Down Expand Up @@ -99,13 +103,17 @@ public class ApocConfig extends LifecycleAdapter {
private LoggingType loggingType;
private SimpleRateLimiter rateLimiter;
private GraphDatabaseService systemDb;

private List<IPAddressString> blockedIpRanges = List.of();

/**
* keep track if this instance is already initialized so dependent class can wait if needed
*/
private boolean initialized = false;

public ApocConfig(Config neo4jConfig, LogService log, GlobalProcedures globalProceduresRegistry, DatabaseManagementService databaseManagementService) {
this.neo4jConfig = neo4jConfig;
this.blockedIpRanges = neo4jConfig.get(ApocSettings.cypher_ip_blocklist);
this.log = log.getInternalLog(ApocConfig.class);
this.databaseManagementService = databaseManagementService;
theInstance = this;
Expand Down Expand Up @@ -259,9 +267,37 @@ public void isImportFileEnabled() {
}
}

public void checkReadAllowed(String url) {
/*
TODO
This needs to be cleaned up in 5.0
WebURLAccessRule was added in the database in 4.4.5, so it would
break backwards compatibility with 4.4.xx previous versions
*/
private void checkAllowedUrl(String url) throws IOException {
try {
if (blockedIpRanges != null && !blockedIpRanges.isEmpty()) {
URL parsedUrl = new URL(url);
InetAddress inetAddress = InetAddress.getByName(parsedUrl.getHost());

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

public void checkReadAllowed(String url) throws IOException {
if (isFile(url)) {
isImportFileEnabled();
} else {
checkAllowedUrl(url);
}
}

Expand Down
41 changes: 41 additions & 0 deletions core/src/main/java/apoc/ApocSettings.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
package apoc;

import inet.ipaddr.AddressStringException;
import inet.ipaddr.IPAddressString;

import org.neo4j.annotations.service.ServiceProvider;
import org.neo4j.configuration.Description;
import org.neo4j.configuration.SettingValueParser;
import org.neo4j.configuration.SettingsDeclaration;
import org.neo4j.graphdb.config.Setting;

import java.time.Duration;
import java.util.List;

import static apoc.ApocConfig.*;
import static org.neo4j.configuration.SettingImpl.newBuilder;
Expand All @@ -19,6 +23,43 @@
*/
@ServiceProvider
public class ApocSettings implements SettingsDeclaration {
/*
TODO
This needs to be cleaned up in 5.0, along with cypher_ip_blocklist
The option was added in the database in 4.4.5, so it would
break backwards compatibility with 4.4.xx previous versions
*/
public static final SettingValueParser<IPAddressString> CIDR_IP = new SettingValueParser<>()
{
@Override
public IPAddressString parse( String value )
{
IPAddressString ipAddress = new IPAddressString( value.trim() );
try
{
ipAddress.validate();
}
catch ( AddressStringException e )
{
throw new IllegalArgumentException( String.format( "'%s' is not a valid CIDR ip", value ), e );
}
return ipAddress;
}

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

@Override
public Class<IPAddressString> getType()
{
return IPAddressString.class;
}
};

public static final Setting<List<IPAddressString>> cypher_ip_blocklist = newBuilder( CYPHER_IP_BLOCKLIST, listOf( CIDR_IP ), List.of() ).build();

public static final Setting<Boolean> apoc_export_file_enabled = newBuilder( APOC_EXPORT_FILE_ENABLED, BOOL, false ).build();

Expand Down
5 changes: 3 additions & 2 deletions core/src/main/java/apoc/load/Xml.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import apoc.util.CompressionAlgo;
import apoc.util.CompressionConfig;
import apoc.util.FileUtils;
import apoc.util.Util;
import org.apache.commons.lang3.BooleanUtils;
import org.apache.commons.lang3.StringUtils;
import org.neo4j.graphdb.Label;
Expand Down Expand Up @@ -160,8 +161,8 @@ private XMLStreamReader getXMLStreamReader(Object urlOrBinary, XmlImportConfig c
String url = (String) urlOrBinary;
apocConfig.checkReadAllowed(url);
url = FileUtils.changeFileUrlIfImportDirectoryConstrained(url);
URLConnection urlConnection = new URL(url).openConnection();
inputStream = urlConnection.getInputStream();
var sc = Util.openInputStream(url, null, null, null);
inputStream = sc.getStream();
} else if (urlOrBinary instanceof byte[]) {
inputStream = getInputStreamFromBinary((byte[]) urlOrBinary, config.getCompressionAlgo());
} else {
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/apoc/util/FileUtils.java
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ public class FileUtils {
public enum SupportedProtocols {
http(true, null),
https(true, null),
ftp(true, null),
s3(Util.classExists("com.amazonaws.services.s3.AmazonS3"),
Util.createInstanceOrNull("apoc.util.s3.S3UrlStreamHandlerFactory")),
gs(Util.classExists("com.google.cloud.storage.Storage"),
Expand All @@ -65,6 +66,7 @@ public StreamConnection getStreamConnection(String urlAddress, Map<String, Objec
return FileUtils.openS3InputStream(new URL(urlAddress));
case hdfs:
return FileUtils.openHdfsInputStream(new URL(urlAddress));
case ftp:
case http:
case https:
case gs:
Expand Down
2 changes: 2 additions & 0 deletions core/src/main/java/apoc/util/Util.java
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
package apoc.util;

import apoc.ApocConfig;
import apoc.Pools;
import apoc.convert.Convert;
import apoc.export.util.CountingInputStream;
Expand Down Expand Up @@ -420,6 +421,7 @@ private static InputStream getFileStreamIntoCompressedFile(InputStream is, Strin
}

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

import apoc.ApocSettings;
import apoc.util.CompressionAlgo;
import apoc.util.JsonUtil;
import apoc.util.TestUtil;
import apoc.util.Util;
import org.apache.commons.lang.exception.ExceptionUtils;
import inet.ipaddr.IPAddressString;
import org.junit.*;
import org.mockserver.client.MockServerClient;
import org.mockserver.integration.ClientAndServer;
import org.mockserver.model.Header;

import org.neo4j.graphdb.QueryExecutionException;
import org.neo4j.graphdb.Result;
import org.neo4j.internal.helpers.collection.Iterators;
Expand Down Expand Up @@ -56,7 +59,8 @@ public static void stopServer() {
}

@Rule
public DbmsRule db = new ImpermanentDbmsRule();
public DbmsRule db = new ImpermanentDbmsRule()
.withSetting(ApocSettings.cypher_ip_blocklist, List.of(new IPAddressString("127.168.0.0/8")));
// .withSetting(ApocSettings.apoc_import_file_enabled, true)
// .withSetting(ApocSettings.apoc_import_file_use__neo4j__config, false);

Expand All @@ -76,6 +80,21 @@ public static void stopServer() {
});
}

@Test public void testLoadJsonFromBlockedIpRange() throws Exception {
var protocols = List.of("https", "http", "ftp");

for (var 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 testLoadMultiJsonWithBinary() {
testResult(db, "CALL apoc.load.jsonParams($url, null, null, null, $config)",
Expand Down
2 changes: 1 addition & 1 deletion readme.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ You can build the library by running the following commands:
git clone http://github.com/neo4j-contrib/neo4j-apoc-procedures
cd neo4j-apoc-procedures
./gradlew shadow
cp build/libs/apoc-<version>-all.jar $NEO4J_HOME/plugins/
cp build/full/libs/apoc-<version>-all.jar $NEO4J_HOME/plugins/
$NEO4J_HOME/bin/neo4j restart
----

Expand Down