From 517c27e644b19c613a052f74f2cf6afa08777270 Mon Sep 17 00:00:00 2001
From: Chris Ridd <chris.ridd@forgerock.com>
Date: Thu, 02 Apr 2015 15:24:50 +0000
Subject: [PATCH] FR-655 OPENDJ-1916: securely configure XMLReader
---
opendj-server-legacy/src/dsml/org/opends/dsml/protocol/DSMLServlet.java | 86 ++++++++++++++++++++++++++++++++++++++----
1 files changed, 77 insertions(+), 9 deletions(-)
diff --git a/opendj-server-legacy/src/dsml/org/opends/dsml/protocol/DSMLServlet.java b/opendj-server-legacy/src/dsml/org/opends/dsml/protocol/DSMLServlet.java
index 911758a..d66447e 100644
--- a/opendj-server-legacy/src/dsml/org/opends/dsml/protocol/DSMLServlet.java
+++ b/opendj-server-legacy/src/dsml/org/opends/dsml/protocol/DSMLServlet.java
@@ -41,6 +41,7 @@
import java.io.OutputStream;
import java.net.URL;
import java.text.ParseException;
+import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.ArrayList;
import java.util.Enumeration;
@@ -49,12 +50,15 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.StringTokenizer;
+import java.util.logging.Level;
+import java.util.logging.Logger;
import javax.servlet.http.HttpServlet;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
import javax.servlet.ServletConfig;
import javax.servlet.ServletException;
+import javax.xml.XMLConstants;
import javax.xml.bind.JAXBContext;
import javax.xml.bind.JAXBElement;
import javax.xml.bind.JAXBException;
@@ -62,6 +66,8 @@
import javax.xml.bind.Unmarshaller;
import javax.xml.parsers.DocumentBuilder;
import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.parsers.SAXParserFactory;
import javax.xml.soap.*;
import javax.xml.soap.SOAPConstants;
import javax.xml.validation.Schema;
@@ -91,9 +97,10 @@
import org.w3c.dom.Document;
import org.xml.sax.Attributes;
import org.xml.sax.helpers.DefaultHandler;
-import org.xml.sax.helpers.XMLReaderFactory;
import org.xml.sax.InputSource;
import org.xml.sax.SAXException;
+import org.xml.sax.SAXNotRecognizedException;
+import org.xml.sax.SAXNotSupportedException;
import org.xml.sax.XMLReader;
@@ -145,6 +152,8 @@
* using SOAP or JAXB.
*/
private DSMLContentHandler contentHandler;
+ /** Prevent multiple logging when trying to set unavailable/unsupported parser features */
+ private static AtomicBoolean logFeatureWarnings = new AtomicBoolean(false);
private String hostName;
private Integer port;
@@ -234,6 +243,8 @@
messageFactorySOAP_1_2 = MessageFactory.newInstance(SOAPConstants.SOAP_1_2_PROTOCOL);
DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
dbf.setNamespaceAware(true);
+ dbf.setXIncludeAware(false);
+ dbf.setExpandEntityReferences(false);
db = dbf.newDocumentBuilder();
this.contentHandler = new DSMLContentHandler();
@@ -585,12 +596,45 @@
}
+
+
+ /**
+ * Safely set a feature on an XMLReader instance.
+ *
+ * @param xmlReader The reader to configure.
+ * @param feature The feature string to set.
+ * @param flag The value to set the feature to.
+ */
+ private void safeSetFeature(XMLReader xmlReader, String feature, Boolean flag)
+ {
+ try
+ {
+ xmlReader.setFeature(feature, flag);
+ }
+ catch (SAXNotSupportedException e)
+ {
+ if (logFeatureWarnings.compareAndSet(false, true))
+ {
+ Logger.getLogger(PKG_NAME).log(Level.SEVERE, "XMLReader unsupported feature " + feature);
+ }
+ }
+ catch (SAXNotRecognizedException e)
+ {
+ if (logFeatureWarnings.compareAndSet(false, true))
+ {
+ Logger.getLogger(PKG_NAME).log(Level.SEVERE, "XMLReader unrecognized feature " + feature);
+ }
+ }
+ }
+
+
+
/**
* Returns an error response after a parsing error. The response has the
* requestID of the batch request, the error response message of the parsing
* exception message and the type 'malformed request'.
*
- * @param is the xml InputStream to parse
+ * @param is the XML InputStream to parse
* @param batchResponse the JAXB object to fill in
* @param parserErrorMessage the parsing error message
*
@@ -602,17 +646,41 @@
String parserErrorMessage) {
ErrorResponse errorResponse = objFactory.createErrorResponse();
- try {
+ try
+ {
// try alternative XML parsing using SAX to retrieve requestID value
- XMLReader xmlReader = XMLReaderFactory.createXMLReader();
+ SAXParserFactory saxParserFactory = SAXParserFactory.newInstance();
+ // Ensure we are doing basic secure processing.
+ saxParserFactory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+ saxParserFactory.setXIncludeAware(false);
+ saxParserFactory.setNamespaceAware(true);
+
+ // Configure a safe XMLReader appropriate for SOAP.
+ XMLReader xmlReader = saxParserFactory.newSAXParser().getXMLReader();
+ safeSetFeature(xmlReader, "http://xml.org/sax/features/validation", false);
+ safeSetFeature(xmlReader, "http://xml.org/sax/features/external-general-entities", false);
+ safeSetFeature(xmlReader, "http://xml.org/sax/features/external-parameter-entities", false);
+ safeSetFeature(xmlReader, "http://xml.org/sax/features/namespaces", true);
+ safeSetFeature(xmlReader, "http://apache.org/xml/features/disallow-doctype-decl", true);
+
// clear previous match
this.contentHandler.requestID = null;
xmlReader.setContentHandler(this.contentHandler);
is.reset();
xmlReader.parse(new InputSource(is));
- } catch (Throwable e) {
- // document cannot be parsed, so will jump here
+ }
+ catch (ParserConfigurationException e)
+ {
+ // ignore
+ }
+ catch (SAXException e)
+ {
+ // ignore
+ }
+ catch (IOException e)
+ {
+ // ignore
}
if ( parserErrorMessage!= null ) {
errorResponse.setMessage(parserErrorMessage);
@@ -810,9 +878,9 @@
return nextID;
}
- /**
- * This class is used when a xml request is malformed to retrieve the
- * requestID value using an event xml parser.
+ /**
+ * This class is used when an XML request is malformed to retrieve the
+ * requestID value using an event XML parser.
*/
private static class DSMLContentHandler extends DefaultHandler {
private String requestID;
--
Gitblit v1.10.0