From ee647fb798359ef79f335ac04f7b5ab9c8048256 Mon Sep 17 00:00:00 2001 From: kolzeq Date: Tue, 28 Jun 2016 16:15:53 +0200 Subject: [PATCH] [CONJ-305] Add LOAD DATA LOCAL INFILE interceptors --- documentation/changelog.creole | 48 +++++++++++++++- .../use-mariadb-connector-j-driver.creole | 39 ++++++++++++- .../mariadb/jdbc/LocalInfileInterceptor.java | 57 +++++++++++++++++++ .../protocol/AbstractQueryProtocol.java | 15 ++++- .../jdbc/LocalInfileInputStreamTest.java | 25 +++++++- .../jdbc/LocalInfileInterceptorImpl.java | 9 +++ .../org.mariadb.jdbc.LocalInfileInterceptor | 1 + src/test/resources/test2.txt | 2 + 8 files changed, 190 insertions(+), 6 deletions(-) create mode 100644 src/main/java/org/mariadb/jdbc/LocalInfileInterceptor.java create mode 100644 src/test/java/org/mariadb/jdbc/LocalInfileInterceptorImpl.java create mode 100644 src/test/resources/META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor create mode 100644 src/test/resources/test2.txt diff --git a/documentation/changelog.creole b/documentation/changelog.creole index 87d431ebd..aaeb1b5a8 100644 --- a/documentation/changelog.creole +++ b/documentation/changelog.creole @@ -81,13 +81,59 @@ New Options : Client logging can be enable, permitting to log query information, execution time and different failover information. This implementation need the standard SLF4J dependency. //TODO finish documentation on logging to explain the differents information logged according to level. - New Options : |=log|Enable log information. require Slf4j version > 1.4 dependency.\\//Default: false. Since 1.5.0//| |=maxQuerySizeToLog|Only the first characters corresponding to this options size will be displayed in logs\\//Default: 1024. Since 1.5.0//| |=slowQueryThresholdNanos|Will log query with execution time superior to this value (if defined )\\//Default: 1024. Since 1.5.0//| |=profileSql|log query execution time.\\//Default: false. Since 1.5.0//| + +=== "LOAD DATA INFILE" +LOAD DATA INFILE The fastest way to load many datas is using query [[https://mariadb +.com/kb/en/mariadb/load-data-infile/|LOAD DATA INFILE]]. +\\Problem is using "LOAD DATA LOCAL INFILE" (ie : loading a file from client), may be a security problem : +* A "man in the middle" proxy server can change the actual file asked from server so client will send a Local file to + this proxy. +* if someone has can execute query from client, he can have access to any file on client (according to the rights of the user running the client process). + +see [[./use-mariadb-connector-j-driver.creole#load-data-infile|load-data-infile documentation]] for more informations. + +Interceptors can now filter LOAD DATA LOCAL INFILE query according to filename. + +Thoses interceptors must implement interface {{{org.mariadb.jdbc.LocalInfileInterceptor}}}. +Interceptors are using the [[http://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html|ServiceLoader]] pattern, so interceptors must be defined in file META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor. + +Example : +create file META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor with content org.project.LocalInfileInterceptorImpl. +{{{ +public class LocalInfileInterceptorImpl implements LocalInfileInterceptor { + @Override + public boolean validate(String fileName) { + File file = new File(fileName); + String absolutePath = file.getAbsolutePath(); + String filePath = absolutePath.substring(0,absolutePath.lastIndexOf(File.separator)); + return filePath.equals("/var/tmp/exchanges"); + } +} +}}} + +You can get ride of defining the META-INF/services file using [[https://github.com/google/auto/tree/master/service|google auto-service] framework +Using the previous example, just add {{{@AutoService(LocalInfileInterceptor.class)}}}, and your interceptor will be automatically defined. +{{{ +@AutoService(LocalInfileInterceptor.class) +public class LocalInfileInterceptorImpl implements LocalInfileInterceptor { + @Override + public boolean validate(String fileName) { + File file = new File(fileName); + String absolutePath = file.getAbsolutePath(); + String filePath = absolutePath.substring(0,absolutePath.lastIndexOf(File.separator)); + return filePath.equals("/var/tmp/exchanges"); + } +} +}}} + + + === Minor evolution * CONJ-260 : Add jdbc nString, nCharacterStream, nClob implementation diff --git a/documentation/use-mariadb-connector-j-driver.creole b/documentation/use-mariadb-connector-j-driver.creole index 8e113052e..16b646d5b 100644 --- a/documentation/use-mariadb-connector-j-driver.creole +++ b/documentation/use-mariadb-connector-j-driver.creole @@ -157,7 +157,8 @@ The following options are currently supported. == "LOAD DATA INFILE" The fastest way to load many datas is using query [[https://mariadb.com/kb/en/mariadb/load-data-infile/|LOAD DATA INFILE]]. \\Problem is using "LOAD DATA LOCAL INFILE" (ie : loading a file from client), may be a security problem : -* if server sources has been changed, server mays asked for a different file than the file in query. +* A "man in the middle" proxy server can change the actual file asked from server so client will send a Local file to + this proxy. * if someone has can execute query from client, he can have access to any file on client (according to the rights of the user running the client process). A specific option "allowLocalInfile" (default to true) can deactivate functionality on client side. @@ -185,6 +186,42 @@ Code example: }}} +Since 1.5.0 version, Interceptors can now filter LOAD DATA LOCAL INFILE query according to filename. + +Thoses interceptors must implement interface {{{org.mariadb.jdbc.LocalInfileInterceptor}}}. +Interceptors are using the [[http://docs.oracle.com/javase/7/docs/api/java/util/ServiceLoader.html|ServiceLoader]] pattern, so interceptors must be defined in file META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor. + +Example : +create file META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor with content org.project.LocalInfileInterceptorImpl. +{{{ + public class LocalInfileInterceptorImpl implements LocalInfileInterceptor { + @Override + public boolean validate(String fileName) { + File file = new File(fileName); + String absolutePath = file.getAbsolutePath(); + String filePath = absolutePath.substring(0,absolutePath.lastIndexOf(File.separator)); + return filePath.equals("/var/tmp/exchanges"); + } + } +}}} + +You can get ride of defining the META-INF/services file using [[https://github.com/google/auto/tree/master/service|google auto-service] framework +Using the previous example, just add {{{@AutoService(LocalInfileInterceptor.class)}}}, and your interceptor will be automatically defined. +{{{ + @AutoService(LocalInfileInterceptor.class) + public class LocalInfileInterceptorImpl implements LocalInfileInterceptor { + @Override + public boolean validate(String fileName) { + File file = new File(fileName); + String absolutePath = file.getAbsolutePath(); + String filePath = absolutePath.substring(0,absolutePath.lastIndexOf(File.separator)); + return filePath.equals("/var/tmp/exchanges"); + } + } +}}} + + + == Streaming result sets By default, {{{Statement.executeQuery()}}} will read the full result set from the server before returning. With large result sets, this will require large diff --git a/src/main/java/org/mariadb/jdbc/LocalInfileInterceptor.java b/src/main/java/org/mariadb/jdbc/LocalInfileInterceptor.java new file mode 100644 index 000000000..230c05cfb --- /dev/null +++ b/src/main/java/org/mariadb/jdbc/LocalInfileInterceptor.java @@ -0,0 +1,57 @@ +/* +MariaDB Client for Java + +Copyright (c) 2012-2014 Monty Program Ab. +Copyright (c) 2015-2016 MariaDB Ab. + +This library is free software; you can redistribute it and/or modify it under +the terms of the GNU Lesser General Public License as published by the Free +Software Foundation; either version 2.1 of the License, or (at your option) +any later version. + +This library is distributed in the hope that it will be useful, but +WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or +FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License +for more details. + +You should have received a copy of the GNU Lesser General Public License along +with this library; if not, write to Monty Program Ab info@montyprogram.com. + +This particular MariaDB Client for Java file is work +derived from a Drizzle-JDBC. Drizzle-JDBC file which is covered by subject to +the following copyright and notice provisions: + +Copyright (c) 2009-2011, Marcus Eriksson + +Redistribution and use in source and binary forms, with or without modification, +are permitted provided that the following conditions are met: +Redistributions of source code must retain the above copyright notice, this list +of conditions and the following disclaimer. + +Redistributions in binary form must reproduce the above copyright notice, this +list of conditions and the following disclaimer in the documentation and/or +other materials provided with the distribution. + +Neither the name of the driver nor the names of its contributors may not be +used to endorse or promote products derived from this software without specific +prior written permission. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED +WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. +IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, +INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT +NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR +PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) +ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY +OF SUCH DAMAGE. +*/ + +package org.mariadb.jdbc; + +public interface LocalInfileInterceptor { + + boolean validate(String fileName) ; + +} diff --git a/src/main/java/org/mariadb/jdbc/internal/protocol/AbstractQueryProtocol.java b/src/main/java/org/mariadb/jdbc/internal/protocol/AbstractQueryProtocol.java index 929e9c9aa..b08c2d23f 100644 --- a/src/main/java/org/mariadb/jdbc/internal/protocol/AbstractQueryProtocol.java +++ b/src/main/java/org/mariadb/jdbc/internal/protocol/AbstractQueryProtocol.java @@ -1,7 +1,6 @@ package org.mariadb.jdbc.internal.protocol; import org.mariadb.jdbc.MariaDbConnection; -import org.mariadb.jdbc.MariaDbStatement; import org.mariadb.jdbc.UrlParser; import org.mariadb.jdbc.internal.logging.Logger; import org.mariadb.jdbc.internal.logging.LoggerFactory; @@ -19,6 +18,7 @@ import org.mariadb.jdbc.internal.packet.dao.ColumnInformation; import org.mariadb.jdbc.internal.MariaDbType; import org.mariadb.jdbc.internal.util.dao.ServerPrepareResult; +import org.mariadb.jdbc.LocalInfileInterceptor; import java.io.*; import java.net.SocketException; @@ -31,6 +31,7 @@ import java.sql.SQLException; import java.sql.Statement; import java.util.List; +import java.util.ServiceLoader; import java.util.concurrent.locks.ReentrantLock; /* @@ -334,6 +335,7 @@ private void sendLocalFile(ExecutionResult executionResult, String fileName) thr int seq = 2; InputStream is; if (localInfileInputStream == null) { + if (!getUrlParser().getOptions().allowLocalInfile) { writer.writeEmptyPacket(seq++); throw new QueryException( @@ -342,6 +344,17 @@ private void sendLocalFile(ExecutionResult executionResult, String fileName) thr ExceptionMapper.SqlStates.FEATURE_NOT_SUPPORTED.getSqlState()); } + //validate all defined interceptors + ServiceLoader loader = ServiceLoader.load(LocalInfileInterceptor.class); + for (LocalInfileInterceptor interceptor : loader) { + if (!interceptor.validate(fileName)) { + writer.writeEmptyPacket(seq++); + throw new QueryException("LOCAL DATA LOCAL INFILE request to send local file named \"" + + fileName + "\" not validated by interceptor \"" + interceptor.getClass().getName() + + "\""); + } + } + try { URL url = new URL(fileName); is = url.openStream(); diff --git a/src/test/java/org/mariadb/jdbc/LocalInfileInputStreamTest.java b/src/test/java/org/mariadb/jdbc/LocalInfileInputStreamTest.java index b402ac075..340c588d2 100644 --- a/src/test/java/org/mariadb/jdbc/LocalInfileInputStreamTest.java +++ b/src/test/java/org/mariadb/jdbc/LocalInfileInputStreamTest.java @@ -14,6 +14,8 @@ import java.sql.Statement; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.junit.Assert.fail; public class LocalInfileInputStreamTest extends BaseTest { /** @@ -59,8 +61,25 @@ public void testLocalInfileInputStream() throws SQLException { @Test public void testLocalInfile() throws SQLException { - Statement st = sharedConnection.createStatement(); ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + testLocalInfile(classLoader.getResource("test.txt").getPath()); + } + + @Test + public void testLocalInfileInterceptor() throws SQLException { + ClassLoader classLoader = Thread.currentThread().getContextClassLoader(); + try { + testLocalInfile(classLoader.getResource("test2.txt").getPath()); + fail("Must have been intercepted"); + } catch (SQLException sqle) { + assertTrue(sqle.getMessage().contains("LOCAL DATA LOCAL INFILE request to send local file named") + && sqle.getMessage().contains("not validated by interceptor \"org.mariadb.jdbc.LocalInfileInterceptorImpl\"")); + } + } + + + public void testLocalInfile(String file) throws SQLException { + Statement st = sharedConnection.createStatement(); ResultSet rs = st.executeQuery("select @@version_compile_os"); if (!rs.next()) { return; @@ -68,13 +87,13 @@ public void testLocalInfile() throws SQLException { String os = rs.getString(1); if (os.toLowerCase().startsWith("win") || System.getProperty("os.name").startsWith("Windows")) { - st.executeUpdate("LOAD DATA LOCAL INFILE '" + classLoader.getResource("test.txt").getPath() + st.executeUpdate("LOAD DATA LOCAL INFILE '" + file + "' INTO TABLE tt_local " + " FIELDS TERMINATED BY ',' OPTIONALLY ENCLOSED BY '\"'" + " LINES TERMINATED BY '\\r\\n' " + " (id, test)"); } else { - st.executeUpdate("LOAD DATA LOCAL INFILE '" + classLoader.getResource("test.txt").getPath() + st.executeUpdate("LOAD DATA LOCAL INFILE '" + file + "' INTO TABLE tt_local " + " FIELDS TERMINATED BY ',' OPTIONALLY ENCLOSED BY '\"'" + " LINES TERMINATED BY '\\n' " diff --git a/src/test/java/org/mariadb/jdbc/LocalInfileInterceptorImpl.java b/src/test/java/org/mariadb/jdbc/LocalInfileInterceptorImpl.java new file mode 100644 index 000000000..f238dbe30 --- /dev/null +++ b/src/test/java/org/mariadb/jdbc/LocalInfileInterceptorImpl.java @@ -0,0 +1,9 @@ +package org.mariadb.jdbc; + +public class LocalInfileInterceptorImpl implements LocalInfileInterceptor { + + @Override + public boolean validate(String fileName) { + return fileName != null && fileName.contains("test.txt"); + } +} diff --git a/src/test/resources/META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor b/src/test/resources/META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor new file mode 100644 index 000000000..569dde8d1 --- /dev/null +++ b/src/test/resources/META-INF/services/org.mariadb.jdbc.LocalInfileInterceptor @@ -0,0 +1 @@ +org.mariadb.jdbc.LocalInfileInterceptorImpl \ No newline at end of file diff --git a/src/test/resources/test2.txt b/src/test/resources/test2.txt new file mode 100644 index 000000000..52bbb35c0 --- /dev/null +++ b/src/test/resources/test2.txt @@ -0,0 +1,2 @@ +1,"hello" +2,"world"