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 <ven...@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' (cherry picked from commit 7628ca1edf9c440c9514b74e283d2ae77811dafe) Signed-off-by: Pallavi Rao <pallavi....@inmobi.com> Project: http://git-wip-us.apache.org/repos/asf/falcon/repo Commit: http://git-wip-us.apache.org/repos/asf/falcon/commit/7fe2c332 Tree: http://git-wip-us.apache.org/repos/asf/falcon/tree/7fe2c332 Diff: http://git-wip-us.apache.org/repos/asf/falcon/diff/7fe2c332 Branch: refs/heads/master Commit: 7fe2c33245b3032188a94f20ea83797a2843e417 Parents: 1565bde Author: Venkat Ranganathan <ven...@hortonworks.com> Authored: Thu Feb 9 09:54:03 2017 +0530 Committer: Pallavi Rao <pallavi....@inmobi.com> Committed: Thu Feb 9 09:54:23 2017 +0530 ---------------------------------------------------------------------- .../main/java/org/apache/falcon/entity/v0/Entity.java | 6 +++++- .../java/org/apache/falcon/entity/v0/SchemaHelper.java | 12 ++++++++++++ .../java/org/apache/falcon/entity/DatasourceHelper.java | 6 ++---- .../org/apache/falcon/entity/parser/EntityParser.java | 7 ++++++- .../apache/falcon/entity/store/ConfigurationStore.java | 10 +++++++++- .../extensions/util/ExtensionProcessBuilderUtils.java | 6 +++++- 6 files changed, 39 insertions(+), 8 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/common-types/src/main/java/org/apache/falcon/entity/v0/Entity.java ---------------------------------------------------------------------- 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 @@ package org.apache.falcon.entity.v0; 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 abstract class Entity { 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); } http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/common-types/src/main/java/org/apache/falcon/entity/v0/SchemaHelper.java ---------------------------------------------------------------------- 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 @@ public final class SchemaHelper { 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; + } } http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/common/src/main/java/org/apache/falcon/entity/DatasourceHelper.java ---------------------------------------------------------------------- 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 @@ public final class DatasourceHelper { 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); http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/common/src/main/java/org/apache/falcon/entity/parser/EntityParser.java ---------------------------------------------------------------------- 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.store.ConfigurationStore; 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.Logger; 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 @@ public abstract class EntityParser<T extends Entity> { @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) { http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/common/src/main/java/org/apache/falcon/entity/store/ConfigurationStore.java ---------------------------------------------------------------------- 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.EntityUtil; 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.Logger; 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 @@ public final class ConfigurationStore implements FalconService { 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 { http://git-wip-us.apache.org/repos/asf/falcon/blob/7fe2c332/extensions/src/main/java/org/apache/falcon/extensions/util/ExtensionProcessBuilderUtils.java ---------------------------------------------------------------------- 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 org.apache.falcon.util.NotificationType; 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 @@ public final class ExtensionProcessBuilderUtils { } } ); + 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); }
