NIFI-4357 Global improvement of XML unmarshalling

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

@@ -0,0 +1,45 @@
+/*
+ * 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.InputStream;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.transform.Source;
+import javax.xml.transform.stream.StreamSource;
+
+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(Source 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);

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

@@ -16,6 +16,39 @@
  */
 package org.apache.nifi.authorization;
 
+import java.io.ByteArrayInputStream;
+import java.io.File;
+import java.io.IOException;
+import java.io.StringWriter;
+import java.nio.charset.StandardCharsets;
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.Date;
+import java.util.HashSet;
+import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+import javax.xml.XMLConstants;
+import javax.xml.bind.JAXBContext;
+import javax.xml.bind.JAXBElement;
+import javax.xml.bind.JAXBException;
+import javax.xml.bind.Marshaller;
+import javax.xml.bind.Unmarshaller;
+import javax.xml.parsers.DocumentBuilder;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLOutputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
+import javax.xml.stream.XMLStreamWriter;
+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;
@@ -30,6 +63,7 @@
 import org.apache.nifi.authorization.util.IdentityMapping;
 import org.apache.nifi.authorization.util.IdentityMappingUtil;
 import org.apache.nifi.components.PropertyValue;
+import org.apache.nifi.security.xml.XmlUtils;
 import org.apache.nifi.user.generated.Users;
 import org.apache.nifi.util.NiFiProperties;
 import org.apache.nifi.util.file.FileUtils;
@@ -42,39 +76,6 @@
 import org.w3c.dom.NodeList;
 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.Marshaller;
-import javax.xml.bind.Unmarshaller;
-import javax.xml.parsers.DocumentBuilder;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
-import javax.xml.stream.XMLOutputFactory;
-import javax.xml.stream.XMLStreamException;
-import javax.xml.stream.XMLStreamWriter;
-import javax.xml.transform.stream.StreamSource;
-import javax.xml.validation.Schema;
-import javax.xml.validation.SchemaFactory;
-import java.io.ByteArrayInputStream;
-import java.io.File;
-import java.io.IOException;
-import java.io.StringWriter;
-import java.nio.charset.StandardCharsets;
-import java.util.ArrayList;
-import java.util.Collections;
-import java.util.Comparator;
-import java.util.Date;
-import java.util.HashSet;
-import java.util.Iterator;
-import java.util.List;
-import java.util.Map;
-import java.util.Set;
-import java.util.concurrent.atomic.AtomicReference;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-
 public class FileAccessPolicyProvider implements ConfigurableAccessPolicyProvider {
 
     private static final Logger logger = LoggerFactory.getLogger(FileAccessPolicyProvider.class);
@@ -528,11 +529,17 @@ private void saveAuthorizations(final Authorizations authorizations) throws JAXB
     }
 
     private Authorizations unmarshallAuthorizations() throws JAXBException {
-        final Unmarshaller unmarshaller = JAXB_AUTHORIZATIONS_CONTEXT.createUnmarshaller();
-        unmarshaller.setSchema(authorizationsSchema);
+        try {
+            final XMLStreamReader xsr = XmlUtils.createSafeReader(new StreamSource(authorizationsFile));
+            final Unmarshaller unmarshaller = JAXB_AUTHORIZATIONS_CONTEXT.createUnmarshaller();
+            unmarshaller.setSchema(authorizationsSchema);
 
-        final JAXBElement<Authorizations> element = unmarshaller.unmarshal(new StreamSource(authorizationsFile), Authorizations.class);
-        return element.getValue();
+            final JAXBElement<Authorizations> element = unmarshaller.unmarshal(xsr, Authorizations.class);
+            return element.getValue();
+        } catch (XMLStreamException e) {
+            logger.error("Encountered an error reading authorizations file: ", e);
+            throw new JAXBException("Error reading authorizations file", e);
+        }
     }
 
     /**
@@ -626,8 +633,15 @@ private void convertLegacyAuthorizedUsers(final Authorizations authorizations) t
         final Unmarshaller unmarshaller = JAXB_USERS_CONTEXT.createUnmarshaller();
         unmarshaller.setSchema(usersSchema);
 
+        final XMLStreamReader xsr;
+        try {
+            xsr = XmlUtils.createSafeReader(new StreamSource(authorizedUsersFile));
+        } catch (XMLStreamException e) {
+            logger.error("Encountered an error reading authorized users file: ", e);
+            throw new JAXBException("Error reading authorized users file", e);
+        }
         final JAXBElement<Users> element = unmarshaller.unmarshal(
-                new StreamSource(authorizedUsersFile), org.apache.nifi.user.generated.Users.class);
+                xsr, org.apache.nifi.user.generated.Users.class);
 
         final org.apache.nifi.user.generated.Users users = element.getValue();
         if (users.getUser().isEmpty()) {