some small refactoring and code documentation

3 files changed, 104 insertions, 32 deletions

diff --git a/utils/src/main/java/at/gv/egiz/dom/DOMUtils.java b/utils/src/main/java/at/gv/egiz/dom/DOMUtils.java
index eae8f05e..2054021a 100644
--- a/utils/src/main/java/at/gv/egiz/dom/DOMUtils.java
+++ b/utils/src/main/java/at/gv/egiz/dom/DOMUtils.java
@@ -33,7 +33,8 @@ import java.io.StringWriter;
 

 import javax.xml.parsers.DocumentBuilder;

 import javax.xml.parsers.DocumentBuilderFactory;

-import javax.xml.parsers.ParserConfigurationException;

+import javax.xml.parsers.ParserConfigurationException;
+import javax.xml.stream.XMLStreamException;
 import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerException;
@@ -47,7 +48,8 @@ import org.w3c.dom.Document;
 import org.w3c.dom.Node;
 import org.w3c.dom.Text;

 import org.w3c.dom.bootstrap.DOMImplementationRegistry;

-import org.w3c.dom.ls.DOMImplementationLS;

+import org.w3c.dom.ls.DOMImplementationLS;
+import org.xml.sax.SAXException;

 
 public final class DOMUtils {

 

@@ -160,6 +162,76 @@ public final class DOMUtils {
     

     base64OutputStream.close();

     return doc.createTextNode(outputStream.toString("ASCII"));

-  }

+  }
+  
+  /**
+   * Set XML parser features to {@link DocumentBuilderFactory} to prevent XXE, XEE and SSRF attacks
+   * <br>
+   * <br>
+   * These features are set by this method:
+   * <ul>
+   * 	<li>http://xml.org/sax/features/external-general-entities --> false</li>
+   * 	<li>http://xml.org/sax/features/external-parameter-entities --> false</li>
+   * 	<li>http://apache.org/xml/features/nonvalidating/load-external-dtd --> false</li>
+   * 	<li>http://apache.org/xml/features/disallow-doctype-decl --> true</li>
+   * </ul>
+   *  
+   * 
+   * @param dbf {@link DocumentBuilderFactory} on which the features should be registered
+   */
+  public static void setXMLParserFlagsAgainstXXEAndSSRFAttacks(DocumentBuilderFactory dbf) {
+	  try {
+			dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
+			dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+			dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+			dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+						
+		} catch (ParserConfigurationException e) {
+			log.error("Can NOT set Xerces parser security features. -> XML parsing is possible insecure!!!! ", e);
+			
+		}
+	  
+  }
+  
+  /**
+   * Parse an {@link InputStream} that contains a XML document into a {@link Document}
+   * This method set all features to prevent XXE, XEE and SSRF attacks
+   * 
+   * These features are set by this method:
+   * <ul>
+   * 	<li>http://xml.org/sax/features/external-general-entities --> false</li>
+   * 	<li>http://xml.org/sax/features/external-parameter-entities --> false</li>
+   * 	<li>http://apache.org/xml/features/nonvalidating/load-external-dtd --> false</li>
+   * 	<li>http://apache.org/xml/features/disallow-doctype-decl --> true</li>
+   * </ul>
+   * 
+   * @param is {@link InputStream} that contains a serialized XML document 
+   * @return Deserialized {@link Document} from input XML 
+   * @throws XMLStreamException XML parser has an error 
+   * @throws IOException 
+   */
+	public static Document validateXMLAgainstXXEAndSSRFAttacks(InputStream is) throws XMLStreamException, IOException {		
+		DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();		
+		setXMLParserFlagsAgainstXXEAndSSRFAttacks(dbf);
+		    	    	        
+		try {        	
+			//validate input stream        	
+			return dbf.newDocumentBuilder().parse(is);
+            		    		    		
+		} catch (SAXException e) {
+			log.error("XML data validation FAILED with msg: " + e.getMessage(), e);
+			throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e);
+
+		} catch (ParserConfigurationException e) {
+			log.error("XML data validation FAILED with msg: " + e.getMessage(), e);
+			throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e);
+    		
+		} catch (IOException e) {
+			log.error("XML data validation FAILED with msg: " + e.getMessage(), e);
+			throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e);
+    		
+		}
+		
+	}

 

 }

diff --git a/utils/src/main/java/at/gv/egiz/slbinding/SLUnmarshaller.java b/utils/src/main/java/at/gv/egiz/slbinding/SLUnmarshaller.java
index 2d1808c8..489c36d8 100644
--- a/utils/src/main/java/at/gv/egiz/slbinding/SLUnmarshaller.java
+++ b/utils/src/main/java/at/gv/egiz/slbinding/SLUnmarshaller.java
@@ -57,6 +57,7 @@ import org.slf4j.LoggerFactory;
 import org.xml.sax.SAXException;
 
 import at.gv.egiz.bku.utils.ClasspathURLStreamHandler;
+import at.gv.egiz.dom.DOMUtils;
 import at.gv.egiz.validation.ReportingValidationEventHandler;
 
 public class SLUnmarshaller {
@@ -241,43 +242,28 @@ public class SLUnmarshaller {
    * @throws JAXBException
    */
 public Object unmarshal(StreamSource source) throws XMLStreamException, JAXBException {
+    Reader inputReader = source.getReader(); 
     
-    ReportingValidationEventHandler validationEventHandler = new ReportingValidationEventHandler();
-    Reader inputReader = source.getReader();
-        
-    if (inputReader instanceof InputStreamReader) {
-    	DocumentBuilderFactory dbf = DocumentBuilderFactory.newInstance();
-    	
-    	try {
-			dbf.setFeature("http://xml.org/sax/features/external-general-entities", false);
-			dbf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
-			dbf.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
-			dbf.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
-						
-		} catch (ParserConfigurationException e) {
-			log.error("Can NOT set Xerces parser security features. -> XML parsing is possible insecure!!!! ", e);
-			
-		}
-    	    	        
+    /* Validate XML against XXE, XEE, and SSRF
+     * 
+     * This pre-validation step is required because com.sun.xml.stream.sjsxp-1.0.2 XML stream parser library does not 
+     * support all XML parser features to prevent these types of attacks  
+     */
+    if (inputReader instanceof InputStreamReader) {    	    	        
         try {        	
         	//create copy of input stream
         	InputStreamReader isReader = (InputStreamReader) inputReader;
         	String encoding = isReader.getEncoding();        	
         	byte[] backup = IOUtils.toByteArray(isReader, encoding);
 
-        	//validate input stream        	
-        	dbf.newDocumentBuilder().parse(new ByteArrayInputStream(backup));
+        	//validate input stream
+        	DOMUtils.validateXMLAgainstXXEAndSSRFAttacks(new ByteArrayInputStream(backup));
         	    		
     		//create new inputStreamReader for reak processing
     		inputReader = new InputStreamReader(new ByteArrayInputStream(backup), encoding);
     		
     		
-    	} catch (SAXException e) {
-    		log.error("XML data validation FAILED with msg: " + e.getMessage(), e);
-    		throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e);
-
-
-    	} catch (ParserConfigurationException e) {
+    	} catch (XMLStreamException e) {
     		log.error("XML data validation FAILED with msg: " + e.getMessage(), e);
     		throw new XMLStreamException("XML data validation FAILED with msg: " + e.getMessage(), e);
     		
@@ -294,20 +280,28 @@ public Object unmarshal(StreamSource source) throws XMLStreamException, JAXBExce
     	
     }
         
-    //parse XML with original functionality
+    /* !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+     * parse XML with original functionality
+     * 
+     * This code implements the the original mocca XML processing by using 
+     *  com.sun.xml.stream.sjsxp-1.0.2 XML stream parser library. Currently, this library is required to get full 
+     *  security-layer specific XML processing. However, there this lib does not fully support XXE, XEE and SSRF
+     *  prevention mechanisms (e.g.: XMLInputFactory.SUPPORT_DTD flag is not used)    
+     * 
+     * !!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!
+     */
     XMLInputFactory inputFactory = XMLInputFactory.newInstance();
     
     //disallow DTD and external entities
     inputFactory.setProperty(XMLInputFactory.SUPPORT_DTD, false);
     inputFactory.setProperty("javax.xml.stream.isSupportingExternalEntities", false);
-    
-    
-    
+        
     XMLEventReader eventReader = inputFactory.createXMLEventReader(inputReader);
     RedirectEventFilter redirectEventFilter = new RedirectEventFilter();
     XMLEventReader filteredReader = inputFactory.createFilteredReader(eventReader, redirectEventFilter);
 
     Unmarshaller unmarshaller = jaxbContext.createUnmarshaller();
+    ReportingValidationEventHandler validationEventHandler = new ReportingValidationEventHandler();
     unmarshaller.setEventHandler(validationEventHandler);
 
     unmarshaller.setListener(new RedirectUnmarshallerListener(redirectEventFilter));
diff --git a/utils/src/test/java/at/gv/egiz/slbinding/UnmarshallCXSRTest.java b/utils/src/test/java/at/gv/egiz/slbinding/UnmarshallCXSRTest.java
index 62a8d622..5f97be0f 100644
--- a/utils/src/test/java/at/gv/egiz/slbinding/UnmarshallCXSRTest.java
+++ b/utils/src/test/java/at/gv/egiz/slbinding/UnmarshallCXSRTest.java
@@ -77,6 +77,12 @@ public class UnmarshallCXSRTest {
 	    Object value = ((JAXBElement<?>) object).getValue();	    
 	    assertFalse(value.getClass().getName(), value instanceof CreateXMLSignatureResponseType);
 		
+	    /* If the parser has no exception and no CreateXMLSignatureResponseType than the test fails, because 
+	     * the tested XML document contains a CreateXMLSignatureResponseType and an XXE, SSRF attack vector.
+	     * Consequently, the parser result has to be an error
+	     */
+	    assertFalse(true);
+	    
 	} catch (XMLStreamException e) {
 		assertTrue(e.getClass().getName(), e instanceof XMLStreamException);