FALCON-2273 Disallow external entity injection and clean up some log messages
Added to code to disable external entity resolution and not log messages with info about location of password files
Author: Venkat Ranganathan <venkat@hortonworks.com>
Reviewers: @sandeepSamudrala, @pallavi-rao
Closes #357 from vrangan/FALCON-2273 and squashes the following commits:
26e141d [Venkat Ranganathan] FALCON-2273: Disallow external entity injection and clean up some log messages
30140db [Venkat Ranganathan] Merge remote-tracking branch 'apache/master'
940be61 [Venkat Ranganathan] Merge remote-tracking branch 'apache/master'
ed3a86e [Venkat Ranganathan] Merge remote-tracking branch 'apache/master'
c082fa6 [Venkat Ranganathan] Merge remote-tracking branch 'apache/master'
f01b8dc [Venkat Ranganathan] Merge remote-tracking branch 'apache/master'
diff --git a/common-types/src/main/java/org/apache/falcon/entity/v0/Entity.java b/common-types/src/main/java/org/apache/falcon/entity/v0/Entity.java
index ba6f2e5..98faf82 100644
--- a/common-types/src/main/java/org/apache/falcon/entity/v0/Entity.java
+++ b/common-types/src/main/java/org/apache/falcon/entity/v0/Entity.java
@@ -20,6 +20,8 @@
import javax.xml.bind.Marshaller;
import javax.xml.bind.Unmarshaller;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamReader;
import java.io.StringReader;
import java.io.StringWriter;
@@ -82,7 +84,9 @@
public static Entity fromString(EntityType type, String str) {
try {
Unmarshaller unmarshaler = type.getUnmarshaller();
- return (Entity) unmarshaler.unmarshal(new StringReader(str));
+ XMLInputFactory xif = SchemaHelper.createXmlInputFactory();
+ XMLStreamReader xsr = xif.createXMLStreamReader(new StringReader(str));
+ return (Entity) unmarshaler.unmarshal(xsr);
} catch (Exception e) {
throw new RuntimeException(e);
}
diff --git a/common-types/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java b/common-types/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java
index 1c02f37..ced43fd 100644
--- a/common-types/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java
+++ b/common-types/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java
@@ -18,6 +18,7 @@
package org.apache.falcon.entity.v0;
+import javax.xml.stream.XMLInputFactory;
import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
@@ -68,4 +69,15 @@
throw new RuntimeException(e);
}
}
+
+ /**
+ * Return the xml input factory that has the properties set for secure handling of data.
+ * @return xif
+ */
+ public static XMLInputFactory createXmlInputFactory() {
+ XMLInputFactory xif = XMLInputFactory.newFactory();
+ xif.setProperty(XMLInputFactory.IS_SUPPORTING_EXTERNAL_ENTITIES, false);
+ xif.setProperty(XMLInputFactory.SUPPORT_DTD, false);
+ return xif;
+ }
}
diff --git a/common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java b/common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java
index 1479133..0178ccb 100644
--- a/common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java
+++ b/common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java
@@ -382,13 +382,11 @@
Path path = new Path(passwordFilePath);
FileSystem fs = HadoopClientFactory.get().createProxiedFileSystem(path.toUri());
if (!fs.exists(path)) {
- throw new IOException("The password file does not exist! "
- + passwordFilePath);
+ throw new IOException("The password file does not exist! ");
}
if (!fs.isFile(path)) {
- throw new IOException("The password file cannot be a directory! "
- + passwordFilePath);
+ throw new IOException("The password file cannot be a directory! ");
}
InputStream is = fs.open(path);
diff --git a/common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java b/common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java
index 05b204d..f4a6372 100644
--- a/common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java
+++ b/common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java
@@ -24,6 +24,7 @@
import org.apache.falcon.entity.v0.AccessControlList;
import org.apache.falcon.entity.v0.Entity;
import org.apache.falcon.entity.v0.EntityType;
+import org.apache.falcon.entity.v0.SchemaHelper;
import org.apache.falcon.security.CurrentUser;
import org.apache.falcon.security.SecurityUtil;
import org.apache.hadoop.security.UserGroupInformation;
@@ -32,6 +33,8 @@
import org.slf4j.LoggerFactory;
import javax.xml.bind.Unmarshaller;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamReader;
import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStream;
@@ -88,9 +91,11 @@
@SuppressWarnings("unchecked")
public T parse(InputStream xmlStream) throws FalconException {
try {
+ XMLInputFactory xif = SchemaHelper.createXmlInputFactory();
+ XMLStreamReader xsr = xif.createXMLStreamReader(xmlStream);
// parse against schema
Unmarshaller unmarshaller = entityType.getUnmarshaller();
- T entity = (T) unmarshaller.unmarshal(xmlStream);
+ T entity = (T) unmarshaller.unmarshal(xsr);
LOG.info("Parsed Entity: {}", entity.getName());
return entity;
} catch (Exception e) {
diff --git a/common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java b/common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java
index 19e10bd..11cdc05 100644
--- a/common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java
+++ b/common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java
@@ -24,6 +24,7 @@
import org.apache.falcon.entity.v0.AccessControlList;
import org.apache.falcon.entity.v0.Entity;
import org.apache.falcon.entity.v0.EntityType;
+import org.apache.falcon.entity.v0.SchemaHelper;
import org.apache.falcon.entity.v0.cluster.Cluster;
import org.apache.falcon.entity.v0.datasource.Datasource;
import org.apache.falcon.hadoop.HadoopClientFactory;
@@ -41,6 +42,9 @@
import org.slf4j.LoggerFactory;
import javax.xml.bind.JAXBException;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamException;
+import javax.xml.stream.XMLStreamReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
@@ -466,8 +470,12 @@
throws IOException, FalconException {
InputStream in = fs.open(new Path(storePath, type + Path.SEPARATOR + URLEncoder.encode(name, UTF_8) + ".xml"));
+ XMLInputFactory xif = SchemaHelper.createXmlInputFactory();
try {
- return (T) type.getUnmarshaller().unmarshal(in);
+ XMLStreamReader xsr = xif.createXMLStreamReader(in);
+ return (T) type.getUnmarshaller().unmarshal(xsr);
+ } catch (XMLStreamException xse) {
+ throw new StoreAccessException("Unable to un-marshall xml definition for " + type + "/" + name, xse);
} catch (JAXBException e) {
throw new StoreAccessException("Unable to un-marshall xml definition for " + type + "/" + name, e);
} finally {
diff --git a/extensions/src/main/java/org/apache/falcon/extensions/util/ExtensionProcessBuilderUtils.java b/extensions/src/main/java/org/apache/falcon/extensions/util/ExtensionProcessBuilderUtils.java
index 286df3e..c8e870b 100644
--- a/extensions/src/main/java/org/apache/falcon/extensions/util/ExtensionProcessBuilderUtils.java
+++ b/extensions/src/main/java/org/apache/falcon/extensions/util/ExtensionProcessBuilderUtils.java
@@ -39,6 +39,8 @@
import javax.xml.bind.Unmarshaller;
import javax.xml.bind.ValidationEvent;
import javax.xml.bind.ValidationEventHandler;
+import javax.xml.stream.XMLInputFactory;
+import javax.xml.stream.XMLStreamReader;
import java.io.StringReader;
import java.util.ArrayList;
import java.util.List;
@@ -96,8 +98,10 @@
}
}
);
+ XMLInputFactory xif = SchemaHelper.createXmlInputFactory();
+ XMLStreamReader xsr = xif.createXMLStreamReader(new StringReader(processTemplate));
process = (org.apache.falcon.entity.v0.process.Process)
- unmarshaller.unmarshal(new StringReader(processTemplate));
+ unmarshaller.unmarshal(xsr);
} catch (Exception e) {
throw new FalconException(e);
}
