You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@camel.apache.org by da...@apache.org on 2019/01/08 17:36:51 UTC
[camel] branch jail created (now 85ce135)
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a change to branch jail
in repository https://gitbox.apache.org/repos/asf/camel.git.
at 85ce135 CAMEL-13042: File producer should by default only allow to write files in the starting directory (or subs). Added new option to turn this on|off.
This branch includes the following new commits:
new 85ce135 CAMEL-13042: File producer should by default only allow to write files in the starting directory (or subs). Added new option to turn this on|off.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
[camel] 01/01: CAMEL-13042: File producer should by default only
allow to write files in the starting directory (or subs). Added new option
to turn this on|off.
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davsclaus pushed a commit to branch jail
in repository https://gitbox.apache.org/repos/asf/camel.git
commit 85ce135a59c19c12d351a529bd517e31cebdc9ea
Author: Claus Ibsen <cl...@gmail.com>
AuthorDate: Tue Jan 8 17:41:43 2019 +0100
CAMEL-13042: File producer should by default only allow to write files in the starting directory (or subs). Added new option to turn this on|off.
---
camel-core/src/main/docs/file-component.adoc | 3 +-
.../camel/component/file/GenericFileEndpoint.java | 15 +++++
.../camel/component/file/GenericFileProducer.java | 14 ++++-
.../component/file/FileProducerExpressionTest.java | 2 +-
.../FileProducerJailStartingDirectoryTest.java | 73 ++++++++++++++++++++++
.../file/remote/FtpProducerExpressionTest.java | 2 +-
.../FtpProducerJailStartingDirectoryTest.java | 68 ++++++++++++++++++++
.../camel-jsch/src/main/docs/scp-component.adoc | 3 +-
8 files changed, 174 insertions(+), 6 deletions(-)
diff --git a/camel-core/src/main/docs/file-component.adoc b/camel-core/src/main/docs/file-component.adoc
index 3f16527..25a5c49 100644
--- a/camel-core/src/main/docs/file-component.adoc
+++ b/camel-core/src/main/docs/file-component.adoc
@@ -72,7 +72,7 @@ with the following path and query parameters:
|===
-==== Query Parameters (86 parameters):
+==== Query Parameters (87 parameters):
[width="100%",cols="2,5,^1,2",options="header"]
@@ -102,6 +102,7 @@ with the following path and query parameters:
| *startingDirectoryMustExist* (consumer) | Whether the starting directory must exist. Mind that the autoCreate option is default enabled, which means the starting directory is normally auto created if it doesn't exist. You can disable autoCreate and enable this to ensure the starting directory must exist. Will thrown an exception if the directory doesn't exist. | false | boolean
| *fileExist* (producer) | What to do if a file already exists with the same name. Override, which is the default, replaces the existing file. Append - adds content to the existing file. Fail - throws a GenericFileOperationException, indicating that there is already an existing file. Ignore - silently ignores the problem and does not override the existing file, but assumes everything is okay. Move - option requires to use the moveExisting option to be configured as well. The option eager [...]
| *flatten* (producer) | Flatten is used to flatten the file name path to strip any leading paths, so it's just the file name. This allows you to consume recursively into sub-directories, but when you eg write the files to another directory they will be written in a single directory. Setting this to true on the producer enforces that any file name in CamelFileName header will be stripped for any leading paths. | false | boolean
+| *jailStartingDirectory* (producer) | Used for jailing (restricting) writing files to the starting directory (and sub) only. This is enabled by default to not allow Camel to write files to outside directories (to be more secured out of the box). You can turn this off to allow writing files to directories outside the starting directory, such as parent or root folders. | true | boolean
| *moveExisting* (producer) | Expression (such as File Language) used to compute file name to use when fileExist=Move is configured. To move files into a backup subdirectory just enter backup. This option only supports the following File Language tokens: file:name, file:name.ext, file:name.noext, file:onlyname, file:onlyname.noext, file:ext, and file:parent. Notice the file:parent is not supported by the FTP component, as the FTP component can only move any existing files to a relative d [...]
| *tempFileName* (producer) | The same as tempPrefix option but offering a more fine grained control on the naming of the temporary filename as it uses the File Language. | | String
| *tempPrefix* (producer) | This option is used to write the file using a temporary name and then, after the write is complete, rename it to the real name. Can be used to identify files being written and also avoid consumers (not using exclusive read locks) reading in progress files. Is often used by FTP when uploading big files. | | String
diff --git a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
index c278890..642f4c0 100644
--- a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
+++ b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileEndpoint.java
@@ -94,6 +94,8 @@ public abstract class GenericFileEndpoint<T> extends ScheduledPollEndpoint imple
protected boolean keepLastModified;
@UriParam(label = "producer,advanced")
protected boolean allowNullBody;
+ @UriParam(label = "producer", defaultValue = "true")
+ protected boolean jailStartingDirectory = true;
// consumer options
@@ -1251,6 +1253,19 @@ public abstract class GenericFileEndpoint<T> extends ScheduledPollEndpoint imple
this.allowNullBody = allowNullBody;
}
+ public boolean isJailStartingDirectory() {
+ return jailStartingDirectory;
+ }
+
+ /**
+ * Used for jailing (restricting) writing files to the starting directory (and sub) only.
+ * This is enabled by default to not allow Camel to write files to outside directories (to be more secured out of the box).
+ * You can turn this off to allow writing files to directories outside the starting directory, such as parent or root folders.
+ */
+ public void setJailStartingDirectory(boolean jailStartingDirectory) {
+ this.jailStartingDirectory = jailStartingDirectory;
+ }
+
public ExceptionHandler getOnCompletionExceptionHandler() {
return onCompletionExceptionHandler;
}
diff --git a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
index 144fca6..f46c32a 100644
--- a/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
+++ b/camel-core/src/main/java/org/apache/camel/component/file/GenericFileProducer.java
@@ -22,6 +22,7 @@ import java.util.Map;
import java.util.concurrent.locks.Lock;
import java.util.concurrent.locks.ReentrantLock;
+import org.apache.camel.CamelExchangeException;
import org.apache.camel.Exchange;
import org.apache.camel.Expression;
import org.apache.camel.support.DefaultExchange;
@@ -303,7 +304,7 @@ public class GenericFileProducer<T> extends DefaultProducer {
log.debug("Wrote [{}] to [{}]", fileName, getEndpoint());
}
- public String createFileName(Exchange exchange) {
+ public String createFileName(Exchange exchange) throws CamelExchangeException {
String answer;
// overrule takes precedence
@@ -325,7 +326,7 @@ public class GenericFileProducer<T> extends DefaultProducer {
exchange.getIn().setHeader(Exchange.FILE_NAME, value);
}
- if (value != null && value instanceof String && StringHelper.hasStartToken((String) value, "simple")) {
+ if (value instanceof String && StringHelper.hasStartToken((String) value, "simple")) {
log.warn("Simple expression: {} detected in header: {} of type String. This feature has been removed (see CAMEL-6748).", value, Exchange.FILE_NAME);
}
@@ -372,6 +373,15 @@ public class GenericFileProducer<T> extends DefaultProducer {
answer = baseDir + endpoint.getGeneratedFileName(exchange.getIn());
}
+ if (endpoint.isJailStartingDirectory()) {
+ // check for file must be within starting directory (need to compact first as the name can be using relative paths via ../ etc)
+ String compatchAnswer = FileUtil.compactPath(answer);
+ String compatchBaseDir = FileUtil.compactPath(baseDir);
+ if (!compatchAnswer.startsWith(compatchBaseDir)) {
+ throw new IllegalArgumentException("Cannot write file with name: " + compatchAnswer + " as the filename is jailed to the starting directory: " + compatchBaseDir);
+ }
+ }
+
if (endpoint.getConfiguration().needToNormalize()) {
// must normalize path to cater for Windows and other OS
answer = normalizePath(answer);
diff --git a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
index 9907f7c..788a6aa 100644
--- a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
+++ b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerExpressionTest.java
@@ -81,7 +81,7 @@ public class FileProducerExpressionTest extends ContextTestSupport {
@Test
public void testProducerComplexByExpression() throws Exception {
String expression = "../filelanguageinbox/myfile-${bean:myguidgenerator.guid}-${date:now:yyyyMMdd}.txt";
- template.sendBody("file://target/filelanguage?fileName=" + expression, "Hello World");
+ template.sendBody("file://target/filelanguage?jailStartingDirectory=false&fileName=" + expression, "Hello World");
String date = new SimpleDateFormat("yyyyMMdd").format(new Date());
assertFileExists("target/filelanguageinbox/myfile-123-" + date + ".txt");
diff --git a/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java
new file mode 100644
index 0000000..e303c35
--- /dev/null
+++ b/camel-core/src/test/java/org/apache/camel/component/file/FileProducerJailStartingDirectoryTest.java
@@ -0,0 +1,73 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.file;
+
+import org.apache.camel.ContextTestSupport;
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.junit.Before;
+import org.junit.Test;
+
+public class FileProducerJailStartingDirectoryTest extends ContextTestSupport {
+
+ @Override
+ @Before
+ public void setUp() throws Exception {
+ deleteDirectory("target/jail");
+ super.setUp();
+ }
+
+ @Test
+ public void testJailed() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(0);
+
+ try {
+ template.sendBodyAndHeader("direct:start", "Hello World", Exchange.FILE_NAME, "hello.txt");
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ IllegalArgumentException iae = assertIsInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertTrue(iae.getMessage().contains("as the filename is jailed to the starting directory"));
+ }
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Test
+ public void testNotJailed() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(1);
+
+ template.sendBodyAndHeader("direct:start", "Bye World", Exchange.FILE_NAME, "outbox/bye.txt");
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ from("direct:start")
+ .setHeader(Exchange.FILE_NAME, simple("../${file:name}"))
+ .to("file:target/jail/outbox")
+ .to("mock:result");
+ }
+ };
+ }
+}
diff --git a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
index 435a1b4..06b8b38 100644
--- a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
+++ b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerExpressionTest.java
@@ -85,7 +85,7 @@ public class FtpProducerExpressionTest extends FtpServerTestSupport {
@Test
public void testProducerComplexByExpression() throws Exception {
// need one extra subdirectory (=foo) to be able to start with .. in the fileName option
- String url = "ftp://admin@localhost:" + getPort() + "/filelanguage/foo?password=admin";
+ String url = "ftp://admin@localhost:" + getPort() + "/filelanguage/foo?password=admin&jailStartingDirectory=false";
String expression = "../filelanguageinbox/myfile-${bean:myguidgenerator.guid}-${date:now:yyyyMMdd}.txt";
template.sendBody(url + "&fileName=" + expression, "Hello World");
diff --git a/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java
new file mode 100644
index 0000000..de01b5a
--- /dev/null
+++ b/components/camel-ftp/src/test/java/org/apache/camel/component/file/remote/FtpProducerJailStartingDirectoryTest.java
@@ -0,0 +1,68 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.camel.component.file.remote;
+
+import org.apache.camel.Exchange;
+import org.apache.camel.builder.RouteBuilder;
+import org.apache.camel.component.mock.MockEndpoint;
+import org.junit.Test;
+
+public class FtpProducerJailStartingDirectoryTest extends FtpServerTestSupport {
+
+ private String getFtpUrl() {
+ return "ftp://admin@localhost:" + getPort() + "/upload/jail?binary=false&password=admin&tempPrefix=.uploading";
+ }
+
+ @Test
+ public void testJailed() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(0);
+
+ try {
+ template.sendBodyAndHeader("direct:start", "Hello World", Exchange.FILE_NAME, "hello.txt");
+ fail("Should have thrown exception");
+ } catch (Exception e) {
+ IllegalArgumentException iae = assertIsInstanceOf(IllegalArgumentException.class, e.getCause());
+ assertTrue(iae.getMessage().contains("as the filename is jailed to the starting directory"));
+ }
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Test
+ public void testNotJailed() throws Exception {
+ MockEndpoint mock = getMockEndpoint("mock:result");
+ mock.expectedMessageCount(1);
+
+ template.sendBodyAndHeader("direct:start", "Bye World", Exchange.FILE_NAME, "jail/bye.txt");
+
+ assertMockEndpointsSatisfied();
+ }
+
+ @Override
+ protected RouteBuilder createRouteBuilder() throws Exception {
+ return new RouteBuilder() {
+ @Override
+ public void configure() throws Exception {
+ from("direct:start")
+ .setHeader(Exchange.FILE_NAME, simple("../${file:name}"))
+ .to(getFtpUrl())
+ .to("mock:result");
+ }
+ };
+ }
+}
\ No newline at end of file
diff --git a/components/camel-jsch/src/main/docs/scp-component.adoc b/components/camel-jsch/src/main/docs/scp-component.adoc
index 7303285..4766037 100644
--- a/components/camel-jsch/src/main/docs/scp-component.adoc
+++ b/components/camel-jsch/src/main/docs/scp-component.adoc
@@ -83,7 +83,7 @@ with the following path and query parameters:
|===
-==== Query Parameters (21 parameters):
+==== Query Parameters (22 parameters):
[width="100%",cols="2,5,^1,2",options="header"]
@@ -93,6 +93,7 @@ with the following path and query parameters:
| *chmod* (producer) | Allows you to set chmod on the stored file. For example chmod=664. | 664 | String
| *fileName* (producer) | Use Expression such as File Language to dynamically set the filename. For consumers, it's used as a filename filter. For producers, it's used to evaluate the filename to write. If an expression is set, it take precedence over the CamelFileName header. (Note: The header itself can also be an Expression). The expression options support both String and Expression types. If the expression is a String type, it is always evaluated using the File Language. If the expre [...]
| *flatten* (producer) | Flatten is used to flatten the file name path to strip any leading paths, so it's just the file name. This allows you to consume recursively into sub-directories, but when you eg write the files to another directory they will be written in a single directory. Setting this to true on the producer enforces that any file name in CamelFileName header will be stripped for any leading paths. | false | boolean
+| *jailStartingDirectory* (producer) | Used for jailing (restricting) writing files to the starting directory (and sub) only. This is enabled by default to not allow Camel to write files to outside directories (to be more secured out of the box). You can turn this off to allow writing files to directories outside the starting directory, such as parent or root folders. | true | boolean
| *strictHostKeyChecking* (producer) | Sets whether to use strict host key checking. Possible values are: no, yes | no | String
| *allowNullBody* (producer) | Used to specify if a null body is allowed during file writing. If set to true then an empty file will be created, when set to false, and attempting to send a null body to the file component, a GenericFileWriteException of 'Cannot write null body to file.' will be thrown. If the fileExist option is set to 'Override', then the file will be truncated, and if set to append the file will remain unchanged. | false | boolean
| *disconnectOnBatchComplete* (producer) | Whether or not to disconnect from remote FTP server right after a Batch upload is complete. disconnectOnBatchComplete will only disconnect the current connection to the FTP server. | false | boolean