You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nifi.apache.org by al...@apache.org on 2019/06/12 23:41:05 UTC
[nifi] branch master updated: NIFI-6301 - Added a
SafeXMLConfiguration which disables XML DTDs which may call external
entities. NIFI-6301 - Fixed unit test. Added comments. NIFI-6301 - Removed
unused rule from test. NIFI-6301 - Changed read() methods to use a boolean
instead. Updated comments. NIFI-6301 - Fixing checkstyle errors. NIFI-6301
- Added an XXE Validator to do a simple regex check on files that contain
XXE declarations. Added unit tests and related XML test files. NIFI-6301 -
Forgot license headers. Fixed a [...]
This is an automated email from the ASF dual-hosted git repository.
alopresto pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nifi.git
The following commit(s) were added to refs/heads/master by this push:
new 75fb34c NIFI-6301 - Added a SafeXMLConfiguration which disables XML DTDs which may call external entities. NIFI-6301 - Fixed unit test. Added comments. NIFI-6301 - Removed unused rule from test. NIFI-6301 - Changed read() methods to use a boolean instead. Updated comments. NIFI-6301 - Fixing checkstyle errors. NIFI-6301 - Added an XXE Validator to do a simple regex check on files that contain XXE declarations. Added unit tests and related XML test files. NIFI-6301 - Forgot licen [...]
75fb34c is described below
commit 75fb34c8ee4fd364fbfa81c433b2cc26304a33f7
Author: thenatog <th...@gmail.com>
AuthorDate: Wed May 29 18:34:52 2019 -0400
NIFI-6301 - Added a SafeXMLConfiguration which disables XML DTDs which may call external entities.
NIFI-6301 - Fixed unit test. Added comments.
NIFI-6301 - Removed unused rule from test.
NIFI-6301 - Changed read() methods to use a boolean instead. Updated comments.
NIFI-6301 - Fixing checkstyle errors.
NIFI-6301 - Added an XXE Validator to do a simple regex check on files that contain XXE declarations. Added unit tests and related XML test files.
NIFI-6301 - Forgot license headers. Fixed a unit test.
NIFI-6301 - Refactored duplicate code for error handling.
NIFI-6301 - Removed unused imports.
NIFI-6301 - Slightly changed messaging language.
NIFI-6301 - Added logic to catch multiline XXE attacks.
Added logging messages.
Updated unit test.
This closes #3507.
Co-authored-by: Andy LoPresto <al...@apache.org>
Signed-off-by: Andy LoPresto <al...@apache.org>
---
nifi-commons/nifi-security-utils/pom.xml | 5 +
.../nifi/security/xml/SafeXMLConfiguration.java | 158 +++++++++++++++++++++
.../org/apache/nifi/security/xml/XXEValidator.java | 96 +++++++++++++
.../apache/nifi/security/xml/XmlUtilsTest.groovy | 2 +-
.../org/apache/nifi/security/XXEValidatorTest.java | 148 +++++++++++++++++++
.../src/test/resources/local_xxe_file.xml | 22 +++
.../src/test/resources/multiline_xxe_file.xml | 26 ++++
.../src/test/resources/no_xxe.xml | 22 +++
.../src/test/resources/remote_xxe_file.xml | 25 ++++
.../src/test/resources/whitespace_xxe_file.xml | 25 ++++
.../src/test/resources/xxe_template.xml | 7 -
.../apache/nifi/lookup/XMLFileLookupService.java | 9 +-
.../CommonsConfigurationLookupService.java | 2 +
.../nifi/lookup/TestXMLFileLookupService.java | 30 +++-
.../src/test/resources/test-xxe.xml | 25 ++++
15 files changed, 584 insertions(+), 18 deletions(-)
diff --git a/nifi-commons/nifi-security-utils/pom.xml b/nifi-commons/nifi-security-utils/pom.xml
index 38486b5..b587e1c 100644
--- a/nifi-commons/nifi-security-utils/pom.xml
+++ b/nifi-commons/nifi-security-utils/pom.xml
@@ -84,6 +84,11 @@
<artifactId>spock-core</artifactId>
<scope>test</scope>
</dependency>
+ <dependency>
+ <groupId>org.apache.commons</groupId>
+ <artifactId>commons-configuration2</artifactId>
+ <version>2.4</version>
+ </dependency>
</dependencies>
<build>
diff --git a/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/SafeXMLConfiguration.java b/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/SafeXMLConfiguration.java
new file mode 100644
index 0000000..0e0cd7e
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/SafeXMLConfiguration.java
@@ -0,0 +1,158 @@
+/*
+ * 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.nifi.security.xml;
+
+import org.apache.commons.configuration2.HierarchicalConfiguration;
+import org.apache.commons.configuration2.XMLConfiguration;
+import org.apache.commons.configuration2.ex.ConfigurationException;
+import org.apache.commons.configuration2.io.FileLocator;
+import org.apache.commons.configuration2.tree.ImmutableNode;
+import org.xml.sax.SAXException;
+import org.xml.sax.SAXParseException;
+import org.xml.sax.helpers.DefaultHandler;
+
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.Reader;
+
+/**
+ * For security reasons, this class overrides the Apache commons 'XMLConfiguration' class to disable processing of XML external entity (XXE) declarations.
+ * This class should be used in all cases where an XML configuration file will be used by NiFi. It is currently used by the XMLFileLookupService.
+ */
+public class SafeXMLConfiguration extends XMLConfiguration {
+
+ // Schema Language key for the parser
+ private static final String JAXP_SCHEMA_LANGUAGE =
+ "http://java.sun.com/xml/jaxp/properties/schemaLanguage";
+
+ // Schema Language for the parser
+ private static final String W3C_XML_SCHEMA =
+ "http://www.w3.org/2001/XMLSchema";
+
+ // These features are used to disable processing external entities in the DocumentBuilderFactory to protect against XXE attacks
+ private static final String DISALLOW_DOCTYPES = "http://apache.org/xml/features/disallow-doctype-decl";
+ private static final String ALLOW_EXTERNAL_GENERAL_ENTITIES = "http://xml.org/sax/features/external-general-entities";
+ private static final String ALLOW_EXTERNAL_PARAM_ENTITIES = "http://xml.org/sax/features/external-parameter-entities";
+ private static final String ALLOW_EXTERNAL_DTD = "http://apache.org/xml/features/nonvalidating/load-external-dtd";
+
+ private static final String XXE_ERROR_MESSAGE = "XML configuration file contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.";
+
+ public SafeXMLConfiguration() {
+ super();
+ }
+
+ public SafeXMLConfiguration(HierarchicalConfiguration<ImmutableNode> c) {
+ super(c);
+ }
+
+ @Override
+ public void initFileLocator(FileLocator loc) {
+ super.initFileLocator(loc);
+ }
+
+ /**
+ * This overridden createDocumentBuilder() method sets the appropriate factory attributes to disable XXE parsing.
+ *
+ * @return Returns a safe DocumentBuilder
+ * @throws ParserConfigurationException A configuration error
+ */
+ @Override
+ public DocumentBuilder createDocumentBuilder() throws ParserConfigurationException {
+ if (getDocumentBuilder() != null) {
+ return getDocumentBuilder();
+ }
+
+ final DocumentBuilderFactory factory = DocumentBuilderFactory
+ .newInstance();
+
+ if (isValidating()) {
+ factory.setValidating(true);
+ if (isSchemaValidation()) {
+ factory.setNamespaceAware(true);
+ factory.setAttribute(JAXP_SCHEMA_LANGUAGE, W3C_XML_SCHEMA);
+ }
+ }
+
+ // Disable DTDs and external entities to protect against XXE
+ factory.setAttribute(DISALLOW_DOCTYPES, true);
+ factory.setAttribute(ALLOW_EXTERNAL_GENERAL_ENTITIES, false);
+ factory.setAttribute(ALLOW_EXTERNAL_PARAM_ENTITIES, false);
+ factory.setAttribute(ALLOW_EXTERNAL_DTD, false);
+
+ final DocumentBuilder result;
+ result = factory.newDocumentBuilder();
+ result.setEntityResolver(super.getEntityResolver());
+
+ if (isValidating()) {
+ // register an error handler which detects validation errors
+ result.setErrorHandler(new DefaultHandler() {
+ @Override
+ public void error(SAXParseException ex) throws SAXException {
+ throw ex;
+ }
+ });
+ }
+
+ return result;
+ }
+
+ @Override
+ public void read(Reader in) throws ConfigurationException, IOException {
+ delegateRead(() -> {
+ super.read(in);
+ });
+ }
+
+ @Override
+ public void read(InputStream in) throws ConfigurationException, IOException {
+ delegateRead(() -> {
+ super.read(in);
+ });
+ }
+
+ private void delegateRead(XMLReader superRead) throws ConfigurationException, IOException {
+ try {
+ superRead.read();
+ } catch (ConfigurationException e) {
+ if (isXXERelatedException(e)) {
+ // Wrap this exception to tell the user their XML should not contain XXEs
+ throw new ConfigurationException(XXE_ERROR_MESSAGE, e);
+ } else {
+ // Throw the original exception as it occurred for an unrelated reason
+ throw e;
+ }
+ }
+ }
+
+ /**
+ * Determine if the ConfigurationException was thrown because the XML configuration file contains an external entity (XXE).
+ *
+ * @param e A ConfigurationException that was thrown when parsing the XML configuration file.
+ * @return true if the ConfigurationException was a result of attempting to parse an external entity (which is not allowed for security reasons). Returns false otherwise.
+ */
+ private boolean isXXERelatedException(ConfigurationException e) {
+ return (e.getCause() instanceof SAXParseException && e.getCause().getMessage().contains("DOCTYPE is disallowed"));
+ }
+
+ @FunctionalInterface
+ interface XMLReader {
+ void read() throws ConfigurationException, IOException;
+ }
+}
\ No newline at end of file
diff --git a/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/XXEValidator.java b/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/XXEValidator.java
new file mode 100644
index 0000000..4d54b19
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/XXEValidator.java
@@ -0,0 +1,96 @@
+/*
+ * 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.nifi.security.xml;
+
+import java.io.BufferedReader;
+import java.io.IOException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/* This validator is a quick method to block XML files being used which contain external entity ("!ENTITY") declarations.
+ */
+public class XXEValidator implements Validator {
+ private static final Logger logger = LoggerFactory.getLogger(XXEValidator.class);
+
+ // This pattern looks complicated but allows for arbitrary space/newlines between characters in the <!ENTITY element
+ private final Pattern xxePattern = Pattern.compile("(?i)<\\s*!\\s*E\\s*N\\s*T\\s*I\\s*T\\s*Y");
+
+ @Override
+ public ValidationResult validate(final String subject, final String input, final ValidationContext validationContext) {
+ Path xmlFilePath = Paths.get(input);
+ String line;
+ boolean containsXXE = false;
+
+ final String xmlFilePathString = xmlFilePath.toString();
+ logger.info("Validating {} for XXE attack", xmlFilePathString);
+
+ if (Files.exists(xmlFilePath)) {
+ try (BufferedReader reader = Files.newBufferedReader(xmlFilePath)) {
+ StringBuilder sb = new StringBuilder();
+
+ while ((line = reader.readLine()) != null) {
+ Matcher matcher = xxePattern.matcher(line);
+
+ if (matcher.find()) {
+ // We found an external entity declaration. Stop reading the file and return an invalid property result.
+ containsXXE = true;
+ logger.warn("Detected XXE attack in {}", xmlFilePathString);
+ break;
+ }
+
+ sb.append(line).append("\n");
+ }
+
+ // Some attacks will be crafted with newlines between the characters which trigger the attack
+ if (!containsXXE) {
+ logger.debug("No XXE attack detected in {} line-by-line; checking concatenated document", xmlFilePathString);
+ Matcher matcher = xxePattern.matcher(sb.toString());
+ containsXXE = matcher.find();
+ if (containsXXE) {
+ logger.warn("Detected multiline XXE attack in {}", xmlFilePathString);
+ } else {
+ logger.debug("No XXE attack detected in full file {}", xmlFilePathString);
+ }
+ }
+
+ if (containsXXE) {
+ return new ValidationResult.Builder().subject(subject).input(input).valid(false)
+ .explanation("XML file " + input + " contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.")
+ .build();
+ } else {
+ return new ValidationResult.Builder().subject(subject).input(input).valid(true).explanation("No XXE attack detected.").build();
+ }
+ } catch (IOException e) {
+ return new ValidationResult.Builder().subject(subject).input(input).valid(false)
+ .explanation(input + " is not valid because: " + e.getLocalizedMessage())
+ .build();
+ }
+ } else {
+ return new ValidationResult.Builder().subject(subject).input(input).valid(false)
+ .explanation("File not found: " + input + " could not be found.")
+ .build();
+ }
+ }
+}
\ No newline at end of file
diff --git a/nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/xml/XmlUtilsTest.groovy b/nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/xml/XmlUtilsTest.groovy
index 6a1286f..87429fc 100644
--- a/nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/xml/XmlUtilsTest.groovy
+++ b/nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/xml/XmlUtilsTest.groovy
@@ -60,7 +60,7 @@ class XmlUtilsTest {
@Test
void testShouldHandleXXEInUnmarshal() {
// Arrange
- final String XXE_TEMPLATE_FILEPATH = "src/test/resources/xxe_template.xml"
+ final String XXE_TEMPLATE_FILEPATH = "src/test/resources/local_xxe_file.xml"
InputStream templateStream = new File(XXE_TEMPLATE_FILEPATH).newInputStream()
JAXBContext context = JAXBContext.newInstance(XmlObject.class)
diff --git a/nifi-commons/nifi-security-utils/src/test/java/org/apache/nifi/security/XXEValidatorTest.java b/nifi-commons/nifi-security-utils/src/test/java/org/apache/nifi/security/XXEValidatorTest.java
new file mode 100644
index 0000000..9c39e28
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/java/org/apache/nifi/security/XXEValidatorTest.java
@@ -0,0 +1,148 @@
+/*
+ * 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.nifi.security;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.mockito.Mockito.mock;
+
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.security.xml.XXEValidator;
+import org.junit.After;
+import org.junit.Before;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.JUnit4;
+
+@RunWith(JUnit4.class)
+public class XXEValidatorTest {
+ private String simpleXMLFile = "src/test/resources/no_xxe.xml";
+ private String remoteXXEFile = "src/test/resources/remote_xxe_file.xml";
+ private String localXXEFile = "src/test/resources/local_xxe_file.xml";
+ private String multilineXXEFile = "src/test/resources/multiline_xxe_file.xml";
+ private String whitespaceXXEFile = "src/test/resources/whitespace_xxe_file.xml";
+ private String configurationKey = "Configuration Name";
+ private ValidationContext validationContext;
+
+ @BeforeClass
+ public static void setUpOnce() throws Exception {
+
+ }
+
+ @Before
+ public void setUp() throws Exception {
+ validationContext = mock(ValidationContext.class);
+ }
+
+ @After
+ public void tearDown() throws Exception {
+
+ }
+
+ @Test
+ public void testXmlFileWithNoXXEIsValid() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = simpleXMLFile;
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertTrue(val.isValid());
+ }
+
+
+ @Test
+ public void testXmlFileWithRemoteXXEIsNotValid() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = remoteXXEFile;
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertFalse(val.isValid());
+ assertEquals("XML file " + parameterInput + " contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.", val.getExplanation());
+ }
+
+ @Test
+ public void testXmlFileWithLocalXXEIsNotValid() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = localXXEFile;
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertFalse(val.isValid());
+ assertEquals("XML file " + parameterInput + " contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.", val.getExplanation());
+ }
+
+ @Test
+ public void testXmlFileWithMultilineXXEIsInvalid() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = multilineXXEFile;
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertFalse(val.isValid());
+ assertEquals("XML file " + parameterInput + " contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.", val.getExplanation());
+ }
+
+ @Test
+ public void testXmlFileWithXXEAndWhitespaceIsInvalid() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = whitespaceXXEFile;
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertFalse(val.isValid());
+ assertEquals("XML file " + parameterInput + " contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled.", val.getExplanation());
+ }
+
+ @Test
+ public void testMissingXmlFile() {
+ // Arrange
+ String parameterKey = configurationKey;
+ String parameterInput = "missing_file.xml";
+ XXEValidator a = new XXEValidator();
+
+ // Act
+ ValidationResult val = a.validate(parameterKey, parameterInput, validationContext);
+
+ //Assert
+ assertFalse(val.isValid());
+ assertEquals("File not found: missing_file.xml could not be found.", val.getExplanation());
+ }
+}
\ No newline at end of file
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/local_xxe_file.xml b/nifi-commons/nifi-security-utils/src/test/resources/local_xxe_file.xml
new file mode 100644
index 0000000..a61a929
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/resources/local_xxe_file.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?><!DOCTYPE object [<!ENTITY xxe SYSTEM "file:///etc/passwd" >]>
+<!--
+ 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.
+-->
+<object>
+ <name>&xxe;</name>
+ <description>Arbitrary XML that has an XXE attack present.</description>
+ <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
+ <timestamp>09/05/2017 14:51:01 PDT</timestamp>
+</object>
+ TestXMLFileLookupService.java:82
\ No newline at end of file
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/multiline_xxe_file.xml b/nifi-commons/nifi-security-utils/src/test/resources/multiline_xxe_file.xml
new file mode 100644
index 0000000..daf9afa
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/resources/multiline_xxe_file.xml
@@ -0,0 +1,26 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+ 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.
+-->
+<!DOCTYPE root [
+ <!
+ ENTITY sp SYSTEM "http://127.0.0.1:8897">
+ %sp;
+ ]>
+<root>
+ <name>&sp;</name>
+ <description>Arbitrary XML that has an XXE attack present.</description>
+ <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
+ <timestamp>09/05/2017 14:51:01 PDT</timestamp>
+</root>
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/no_xxe.xml b/nifi-commons/nifi-security-utils/src/test/resources/no_xxe.xml
new file mode 100644
index 0000000..56e59ef
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/resources/no_xxe.xml
@@ -0,0 +1,22 @@
+<?xml version="1.0" ?>
+<!--
+ 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.
+-->
+<test>
+ <properties>
+ <property>this is property 1</property>
+ <property>this is property 2</property>
+ <property value="this is property 3" />
+ </properties>
+</test>
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/remote_xxe_file.xml b/nifi-commons/nifi-security-utils/src/test/resources/remote_xxe_file.xml
new file mode 100644
index 0000000..47f1780
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/resources/remote_xxe_file.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+ 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.
+-->
+<!DOCTYPE root [
+ <!ENTITY sp SYSTEM "http://127.0.0.1:8897">
+ %sp;
+ ]>
+<root>
+ <name>&sp;</name>
+ <description>Arbitrary XML that has an XXE attack present.</description>
+ <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
+ <timestamp>09/05/2017 14:51:01 PDT</timestamp>
+</root>
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/whitespace_xxe_file.xml b/nifi-commons/nifi-security-utils/src/test/resources/whitespace_xxe_file.xml
new file mode 100644
index 0000000..960020e
--- /dev/null
+++ b/nifi-commons/nifi-security-utils/src/test/resources/whitespace_xxe_file.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+ 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.
+-->
+<!DOCTYPE root [
+ < !ENTITY sp SYSTEM "http://127.0.0.1:8897">
+ %sp;
+ ]>
+<root>
+ <name>&sp;</name>
+ <description>Arbitrary XML that has an XXE attack present.</description>
+ <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
+ <timestamp>09/05/2017 14:51:01 PDT</timestamp>
+</root>
diff --git a/nifi-commons/nifi-security-utils/src/test/resources/xxe_template.xml b/nifi-commons/nifi-security-utils/src/test/resources/xxe_template.xml
deleted file mode 100644
index 372d507..0000000
--- a/nifi-commons/nifi-security-utils/src/test/resources/xxe_template.xml
+++ /dev/null
@@ -1,7 +0,0 @@
-<?xml version="1.0" encoding="UTF-8" standalone="yes"?><!DOCTYPE object [<!ENTITY xxe SYSTEM "file:///etc/passwd" >]>
-<object>
- <name>&xxe;</name>
- <description>Arbitrary XML that has an XXE attack present.</description>
- <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
- <timestamp>09/05/2017 14:51:01 PDT</timestamp>
-</object>
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/XMLFileLookupService.java b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/XMLFileLookupService.java
index b8bcd85..ce9c377 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/XMLFileLookupService.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/XMLFileLookupService.java
@@ -16,17 +16,18 @@
*/
package org.apache.nifi.lookup;
-import org.apache.commons.configuration2.XMLConfiguration;
-
import org.apache.nifi.annotation.documentation.CapabilityDescription;
import org.apache.nifi.annotation.documentation.Tags;
import org.apache.nifi.lookup.configuration2.CommonsConfigurationLookupService;
+import org.apache.nifi.security.xml.SafeXMLConfiguration;
+
@Tags({"lookup", "cache", "enrich", "join", "xml", "reloadable", "key", "value"})
@CapabilityDescription("A reloadable XML file-based lookup service." +
" This service uses Apache Commons Configuration." +
" Example XML configuration file and how to access specific configuration can be found at" +
- " http://commons.apache.org/proper/commons-configuration/userguide/howto_hierarchical.html")
-public class XMLFileLookupService extends CommonsConfigurationLookupService<XMLConfiguration> {
+ " http://commons.apache.org/proper/commons-configuration/userguide/howto_hierarchical.html." +
+ " External entity processing is disabled.")
+public class XMLFileLookupService extends CommonsConfigurationLookupService<SafeXMLConfiguration> {
}
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/configuration2/CommonsConfigurationLookupService.java b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/configuration2/CommonsConfigurationLookupService.java
index 3957f80..3307e5f 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/configuration2/CommonsConfigurationLookupService.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/main/java/org/apache/nifi/lookup/configuration2/CommonsConfigurationLookupService.java
@@ -47,6 +47,7 @@ import org.apache.nifi.lookup.LookupFailureException;
import org.apache.nifi.lookup.StringLookupService;
import org.apache.nifi.processor.util.StandardValidators;
import org.apache.nifi.reporting.InitializationException;
+import org.apache.nifi.security.xml.XXEValidator;
/**
* This abstract class defines a generic {@link LookupService} backed by an
@@ -66,6 +67,7 @@ public abstract class CommonsConfigurationLookupService<T extends FileBasedConfi
.description("A configuration file")
.required(true)
.addValidator(StandardValidators.FILE_EXISTS_VALIDATOR)
+ .addValidator(new XXEValidator())
.expressionLanguageSupported(ExpressionLanguageScope.VARIABLE_REGISTRY)
.build();
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/java/org/apache/nifi/lookup/TestXMLFileLookupService.java b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/java/org/apache/nifi/lookup/TestXMLFileLookupService.java
index 34c7b87..c6f5d92 100644
--- a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/java/org/apache/nifi/lookup/TestXMLFileLookupService.java
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/java/org/apache/nifi/lookup/TestXMLFileLookupService.java
@@ -16,19 +16,18 @@
*/
package org.apache.nifi.lookup;
+import static org.hamcrest.CoreMatchers.instanceOf;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
import java.util.Collections;
import java.util.Optional;
-
import org.apache.nifi.reporting.InitializationException;
import org.apache.nifi.util.TestRunner;
import org.apache.nifi.util.TestRunners;
-
import org.junit.Test;
-import static org.hamcrest.CoreMatchers.instanceOf;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertThat;
-
public class TestXMLFileLookupService {
final static Optional<String> EMPTY_STRING = Optional.empty();
@@ -63,4 +62,23 @@ public class TestXMLFileLookupService {
assertEquals(EMPTY_STRING, property4);
}
+ @Test
+ public void testXXEProtection() throws InitializationException {
+
+ // Arrange
+ final TestRunner runner = TestRunners.newTestRunner(TestProcessor.class);
+ final XMLFileLookupService service = new XMLFileLookupService();
+ runner.addControllerService("xml-file-lookup-service", service);
+ runner.setProperty(service, XMLFileLookupService.CONFIGURATION_FILE, "src/test/resources/test-xxe.xml");
+
+ try {
+ // Act
+ // Service will fail to enable because test-xxe.xml contains a DTD
+ runner.enableControllerService(service);
+
+ } catch (final Throwable e) {
+ // Assert
+ assertTrue(e.getMessage().contains("contained an external entity. To prevent XXE vulnerabilities, NiFi has external entity processing disabled."));
+ }
+ }
}
diff --git a/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/resources/test-xxe.xml b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/resources/test-xxe.xml
new file mode 100644
index 0000000..d9d18b9
--- /dev/null
+++ b/nifi-nar-bundles/nifi-standard-services/nifi-lookup-services-bundle/nifi-lookup-services/src/test/resources/test-xxe.xml
@@ -0,0 +1,25 @@
+<?xml version="1.0" encoding="UTF-8" standalone="yes"?>
+<!--
+ 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.
+-->
+<!DOCTYPE root [
+ <!ENTITY sp SYSTEM "http://127.0.0.1:8897">
+ %sp;
+]>
+<root>
+ <name>&sp;</name>
+ <description>Arbitrary XML that has an XXE attack present.</description>
+ <groupId>3a204982-015e-1000-eaa2-19d352ec8394</groupId>
+ <timestamp>09/05/2017 14:51:01 PDT</timestamp>
+</root>