NIFI-4353

- Added XmlUtils class.
- Added unit test.
- Added XXE test resource.
- Refactored JAXB unmarshalling globally to prevent XXE attacks.
- Refactored duplicated/legacy code.
- Cleaned up commented code.
- Switched from FileInputStream back to StreamSource in AuthorizerFactoryBean.
- This closes #2134

Author: alopresto

nifi-commons/nifi-security-utils/pom.xml

@@ -60,5 +60,18 @@
             <artifactId>nifi-properties</artifactId>
         </dependency>
     </dependencies>
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.rat</groupId>
+                <artifactId>apache-rat-plugin</artifactId>
+                <configuration>
+                    <excludes combine.children="append">
+                        <exclude>src/test/resources/xxe_template.xml</exclude>
+                    </excludes>
+                </configuration>
+            </plugin>
+        </plugins>
+    </build>
 </project>
 

nifi-commons/nifi-security-utils/src/main/java/org/apache/nifi/security/xml/XmlUtils.java

@@ -0,0 +1,44 @@
+/*
+ * 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 javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.stream.StreamSource;
+import java.io.InputStream;
+
+public class XmlUtils {
+
+    public static XMLStreamReader createSafeReader(InputStream inputStream) throws XMLStreamException {
+        if (inputStream == null) {
+            throw new IllegalArgumentException("The provided input stream cannot be null");
+        }
+        return createSafeReader(new StreamSource(inputStream));
+    }
+
+    public static XMLStreamReader createSafeReader(StreamSource source) throws XMLStreamException {
+        if (source == null) {
+            throw new IllegalArgumentException("The provided source cannot be null");
+        }
+
+        XMLInputFactory xif = XMLInputFactory.newFactory();
+        xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+        xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+        return xif.createXMLStreamReader(source);
+    }
+}

nifi-commons/nifi-security-utils/src/test/groovy/org/apache/nifi/security/xml/XmlUtilsTest.groovy

@@ -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 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
+import org.slf4j.Logger
+import org.slf4j.LoggerFactory
+
+import javax.xml.bind.JAXBContext
+import javax.xml.bind.UnmarshalException
+import javax.xml.bind.Unmarshaller
+import javax.xml.bind.annotation.XmlAccessType
+import javax.xml.bind.annotation.XmlAccessorType
+import javax.xml.bind.annotation.XmlAttribute
+import javax.xml.bind.annotation.XmlRootElement
+import javax.xml.stream.XMLStreamReader
+
+import static groovy.test.GroovyAssert.shouldFail
+
+@RunWith(JUnit4.class)
+class XmlUtilsTest {
+    private static final Logger logger = LoggerFactory.getLogger(XmlUtilsTest.class)
+
+    @BeforeClass
+    static void setUpOnce() throws Exception {
+        logger.metaClass.methodMissing = { String name, args ->
+            logger.info("[${name?.toUpperCase()}] ${(args as List).join(" ")}")
+        }
+    }
+
+    @Before
+    void setUp() throws Exception {
+
+    }
+
+    @After
+    void tearDown() throws Exception {
+
+    }
+
+    @Test
+    void testShouldHandleXXEInUnmarshal() {
+        // Arrange
+        final String XXE_TEMPLATE_FILEPATH = "src/test/resources/xxe_template.xml"
+        InputStream templateStream = new File(XXE_TEMPLATE_FILEPATH).newInputStream()
+
+        JAXBContext context = JAXBContext.newInstance(XmlObject.class)
+
+        // Act
+        def msg = shouldFail(UnmarshalException) {
+            Unmarshaller unmarshaller = context.createUnmarshaller()
+            XMLStreamReader xsr = XmlUtils.createSafeReader(templateStream)
+            def parsed = unmarshaller.unmarshal(xsr, XmlObject.class)
+            logger.info("Unmarshalled ${parsed.toString()}")
+        }
+
+        // Assert
+        logger.expected(msg)
+        assert msg =~ "XMLStreamException: ParseError "
+    }
+}
+
+@XmlAccessorType( XmlAccessType.NONE )
+@XmlRootElement(name = "object")
+class XmlObject {
+    @XmlAttribute
+    String name
+
+    @XmlAttribute
+    String description
+
+    @XmlAttribute
+    String groupId
+
+    @XmlAttribute
+    String timestamp
+}

nifi-commons/nifi-security-utils/src/test/resources/xxe_template.xml

@@ -0,0 +1,7 @@
+<?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>

nifi-commons/nifi-site-to-site-client/src/main/java/org/apache/nifi/remote/cluster/ClusterNodeInformation.java

@@ -19,13 +19,15 @@
 import java.io.InputStream;
 import java.io.OutputStream;
 import java.util.Collection;
-
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
 import javax.xml.bind.Marshaller;
 import javax.xml.bind.Unmarshaller;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import org.apache.nifi.security.xml.XmlUtils;
 
 @XmlRootElement
 public class ClusterNodeInformation {
@@ -61,7 +63,12 @@ public void marshal(final OutputStream os) throws JAXBException {
     }
 
     public static ClusterNodeInformation unmarshal(final InputStream is) throws JAXBException {
-        final Unmarshaller unmarshaller = JAXB_CONTEXT.createUnmarshaller();
-        return (ClusterNodeInformation) unmarshaller.unmarshal(is);
+        try {
+            final Unmarshaller unmarshaller = JAXB_CONTEXT.createUnmarshaller();
+            final XMLStreamReader xsr = XmlUtils.createSafeReader(is);
+            return (ClusterNodeInformation) unmarshaller.unmarshal(xsr);
+        } catch (XMLStreamException e) {
+            throw new JAXBException("Error unmarshalling the cluster node information", e);
+        }
     }
 }

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/pom.xml

@@ -87,6 +87,10 @@
             <groupId>org.apache.nifi</groupId>
             <artifactId>nifi-framework-authorization</artifactId>
         </dependency>
+        <dependency>
+            <groupId>org.apache.nifi</groupId>
+            <artifactId>nifi-security-utils</artifactId>
+        </dependency>
     </dependencies>
 
 </project>

nifi-nar-bundles/nifi-framework-bundle/nifi-framework/nifi-authorizer/src/main/java/org/apache/nifi/authorization/AuthorizerFactoryBean.java

@@ -16,6 +16,25 @@
  */
 package org.apache.nifi.authorization;
 
+import java.io.File;
+import java.lang.reflect.Constructor;
+import java.lang.reflect.Field;
+import java.lang.reflect.InvocationTargetException;
+import java.lang.reflect.Method;
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import javax.xml.XMLConstants;
+import javax.xml.bind.JAXBContext;
+import javax.xml.bind.JAXBElement;
+import javax.xml.bind.JAXBException;
+import javax.xml.bind.Unmarshaller;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.stream.StreamSource;
+import javax.xml.validation.Schema;
+import javax.xml.validation.SchemaFactory;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.nifi.authorization.annotation.AuthorizerContext;
 import org.apache.nifi.authorization.exception.AuthorizationAccessException;
@@ -25,6 +44,7 @@
 import org.apache.nifi.authorization.generated.Property;
 import org.apache.nifi.bundle.Bundle;
 import org.apache.nifi.nar.ExtensionManager;
+import org.apache.nifi.security.xml.XmlUtils;
 import org.apache.nifi.util.NiFiProperties;
 import org.apache.nifi.util.file.classloader.ClassLoaderUtils;
 import org.slf4j.Logger;
@@ -33,25 +53,6 @@
 import org.springframework.beans.factory.FactoryBean;
 import org.xml.sax.SAXException;
 
-import javax.xml.XMLConstants;
-import javax.xml.bind.JAXBContext;
-import javax.xml.bind.JAXBElement;
-import javax.xml.bind.JAXBException;
-import javax.xml.bind.Unmarshaller;
-import javax.xml.transform.stream.StreamSource;
-import javax.xml.validation.Schema;
-import javax.xml.validation.SchemaFactory;
-import java.io.File;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Field;
-import java.lang.reflect.InvocationTargetException;
-import java.lang.reflect.Method;
-import java.net.URL;
-import java.net.URLClassLoader;
-import java.util.HashMap;
-import java.util.List;
-import java.util.Map;
-
 /**
  * Factory bean for loading the configured authorizer.
  */
@@ -168,9 +169,10 @@ private Authorizers loadAuthorizersConfiguration() throws Exception {
                 final Schema schema = schemaFactory.newSchema(Authorizers.class.getResource(AUTHORIZERS_XSD));
 
                 // attempt to unmarshal
+                final XMLStreamReader xsr = XmlUtils.createSafeReader(new StreamSource(authorizersConfigurationFile));
                 final Unmarshaller unmarshaller = JAXB_CONTEXT.createUnmarshaller();
                 unmarshaller.setSchema(schema);
-                final JAXBElement<Authorizers> element = unmarshaller.unmarshal(new StreamSource(authorizersConfigurationFile), Authorizers.class);
+                final JAXBElement<Authorizers> element = unmarshaller.unmarshal(xsr, Authorizers.class);
                 return element.getValue();
             } catch (SAXException | JAXBException e) {
                 throw new Exception("Unable to load the authorizer configuration file at: " + authorizersConfigurationFile.getAbsolutePath(), e);