From b83132ea8f998bc56fd84d3cb27255c4156c5b8f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nacho=20Cord=C3=B3n?= Date: Wed, 2 Feb 2022 11:31:16 +0000 Subject: [PATCH] Adds blocklist for load methods --- core/build.gradle | 1 + core/src/main/java/apoc/ApocConfig.java | 40 +++++++++++++++++- core/src/main/java/apoc/ApocSettings.java | 41 +++++++++++++++++++ core/src/main/java/apoc/load/Xml.java | 5 ++- core/src/main/java/apoc/util/FileUtils.java | 2 + core/src/main/java/apoc/util/Util.java | 2 + .../src/test/java/apoc/load/LoadJsonTest.java | 21 +++++++++- readme.adoc | 2 +- 8 files changed, 108 insertions(+), 6 deletions(-) diff --git a/core/build.gradle b/core/build.gradle index 6ab566b8cc..6033fe6a03 100644 --- a/core/build.gradle +++ b/core/build.gradle @@ -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' diff --git a/core/src/main/java/apoc/ApocConfig.java b/core/src/main/java/apoc/ApocConfig.java index 8da279157e..149b369115 100644 --- a/core/src/main/java/apoc/ApocConfig.java +++ b/core/src/main/java/apoc/ApocConfig.java @@ -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; @@ -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; @@ -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"; @@ -99,6 +103,9 @@ public class ApocConfig extends LifecycleAdapter { private LoggingType loggingType; private SimpleRateLimiter rateLimiter; private GraphDatabaseService systemDb; + + private List blockedIpRanges = List.of(); + /** * keep track if this instance is already initialized so dependent class can wait if needed */ @@ -106,6 +113,7 @@ public class ApocConfig extends LifecycleAdapter { 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; @@ -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); } } diff --git a/core/src/main/java/apoc/ApocSettings.java b/core/src/main/java/apoc/ApocSettings.java index e8903064c1..7b5606abca 100644 --- a/core/src/main/java/apoc/ApocSettings.java +++ b/core/src/main/java/apoc/ApocSettings.java @@ -1,5 +1,8 @@ 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; @@ -7,6 +10,7 @@ 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; @@ -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 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 getType() + { + return IPAddressString.class; + } + }; + + public static final Setting> cypher_ip_blocklist = newBuilder( CYPHER_IP_BLOCKLIST, listOf( CIDR_IP ), List.of() ).build(); public static final Setting apoc_export_file_enabled = newBuilder( APOC_EXPORT_FILE_ENABLED, BOOL, false ).build(); diff --git a/core/src/main/java/apoc/load/Xml.java b/core/src/main/java/apoc/load/Xml.java index 8d5f68debc..9812f3a65e 100644 --- a/core/src/main/java/apoc/load/Xml.java +++ b/core/src/main/java/apoc/load/Xml.java @@ -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; @@ -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 { diff --git a/core/src/main/java/apoc/util/FileUtils.java b/core/src/main/java/apoc/util/FileUtils.java index e1db488c99..9ca292b159 100644 --- a/core/src/main/java/apoc/util/FileUtils.java +++ b/core/src/main/java/apoc/util/FileUtils.java @@ -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"), @@ -65,6 +66,7 @@ public StreamConnection getStreamConnection(String urlAddress, Map headers, String payload) throws IOException { + ApocConfig.apocConfig().checkReadAllowed(urlAddress); URLConnection con = openUrlConnection(urlAddress, headers); writePayload(con, payload); String newUrl = handleRedirect(con, urlAddress); diff --git a/core/src/test/java/apoc/load/LoadJsonTest.java b/core/src/test/java/apoc/load/LoadJsonTest.java index 7780ebba5a..5acf5bf7c7 100644 --- a/core/src/test/java/apoc/load/LoadJsonTest.java +++ b/core/src/test/java/apoc/load/LoadJsonTest.java @@ -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; @@ -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); @@ -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)", diff --git a/readme.adoc b/readme.adoc index 61a1eb2737..1e7000608c 100644 --- a/readme.adoc +++ b/readme.adoc @@ -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--all.jar $NEO4J_HOME/plugins/ +cp build/full/libs/apoc--all.jar $NEO4J_HOME/plugins/ $NEO4J_HOME/bin/neo4j restart ----