summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java1
-rw-r--r--BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java48
-rw-r--r--BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java16
-rw-r--r--BKUViewer/src/main/java/at/gv/egiz/bku/slxhtml/SLXHTMLValidator.java3
-rw-r--r--utils/src/main/java/at/gv/egiz/dom/DOMUtils.java78
-rw-r--r--utils/src/main/java/at/gv/egiz/slbinding/SLUnmarshaller.java52
-rw-r--r--utils/src/test/java/at/gv/egiz/slbinding/UnmarshallCXSRTest.java6
7 files changed, 114 insertions, 90 deletions
diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java
index 8901969d..d01f8128 100644
--- a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java
+++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/MoccaHttpServletRequestWrapper.java
@@ -5,7 +5,6 @@ import java.io.ByteArrayInputStream;
import java.io.IOException;
import java.io.InputStreamReader;
-import javax.servlet.ReadListener;
import javax.servlet.ServletInputStream;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletRequestWrapper;
diff --git a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java
index 0e98cb79..356401b6 100644
--- a/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java
+++ b/BKUOnline/src/main/java/at/gv/egiz/bku/online/filter/StalSecurityFilter.java
@@ -1,26 +1,20 @@
package at.gv.egiz.bku.online.filter;
-import java.io.ByteArrayInputStream;
import java.io.IOException;
-import java.io.InputStream;
-import java.io.InputStreamReader;
-
import javax.servlet.Filter;
import javax.servlet.FilterChain;
import javax.servlet.FilterConfig;
import javax.servlet.ServletException;
-import javax.servlet.ServletInputStream;
import javax.servlet.ServletRequest;
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
-import javax.xml.parsers.DocumentBuilderFactory;
-import javax.xml.parsers.ParserConfigurationException;
import javax.xml.stream.XMLStreamException;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-import org.xml.sax.SAXException;
+
+import at.gv.egiz.dom.DOMUtils;
public class StalSecurityFilter implements Filter {
@@ -43,7 +37,7 @@ public class StalSecurityFilter implements Filter {
if (stalHttpReq.isInputStreamAvailable()) {
log.trace("Validate STAL request ... ");
- validateStalRequest(stalHttpReq.getInputStream());
+ DOMUtils.validateXMLAgainstXXEAndSSRFAttacks(stalHttpReq.getInputStream());
log.trace("Validate of STAL request completed");
}
@@ -71,7 +65,6 @@ public class StalSecurityFilter implements Filter {
@Override
public void destroy() {
- // TODO Auto-generated method stub
}
@@ -83,40 +76,5 @@ public class StalSecurityFilter implements Filter {
log.error("Can not response with http error message");
}
-
- private void validateStalRequest(InputStream is) throws XMLStreamException, IOException {
-
- 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);
-
- }
-
- try {
- //validate input stream
- 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/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java
index 37889ae5..d34ead45 100644
--- a/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java
+++ b/BKUOnline/src/main/java/at/gv/egiz/mocca/id/DataURLServerServlet.java
@@ -65,6 +65,7 @@ import at.gv.egiz.bku.slcommands.impl.SLCommandImpl;
import at.gv.egiz.bku.slexceptions.SLCommandException;
import at.gv.egiz.bku.utils.DebugInputStream;
import at.gv.egiz.bku.utils.StreamUtil;
+import at.gv.egiz.dom.DOMUtils;
import at.gv.egiz.org.apache.tomcat.util.http.AcceptLanguage;
import at.gv.egiz.slbinding.SLUnmarshaller;
@@ -152,18 +153,9 @@ public class DataURLServerServlet extends HttpServlet {
"(see http://www.w3.org/TR/xmldsig-bestpractices/#be-aware-schema-normalization)", e);
}
- 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 SAX parser security features. -> XML parsing is possible insecure!!!! ", e);
-
- }
-
-
+ //set XML parser flags to prevent XXE, XEE and SSRF attacks
+ DOMUtils.setXMLParserFlagsAgainstXXEAndSSRFAttacks(dbf);
+
DocumentBuilder documentBuilder;
try {
documentBuilder = dbf.newDocumentBuilder();
diff --git a/BKUViewer/src/main/java/at/gv/egiz/bku/slxhtml/SLXHTMLValidator.java b/BKUViewer/src/main/java/at/gv/egiz/bku/slxhtml/SLXHTMLValidator.java
index 95d2b78c..6fea75cb 100644
--- a/BKUViewer/src/main/java/at/gv/egiz/bku/slxhtml/SLXHTMLValidator.java
+++ b/BKUViewer/src/main/java/at/gv/egiz/bku/slxhtml/SLXHTMLValidator.java
@@ -139,6 +139,9 @@ public class SLXHTMLValidator implements at.gv.egiz.bku.viewer.Validator {
spf.setValidating(true);
spf.setXIncludeAware(false);
+ /*
+ * Set parser features to disallow external entities and external dtd load operations
+ */
try {
spf.setFeature("http://xml.org/sax/features/external-general-entities", false);
spf.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
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);
generated by cgit v1.2.3 (git 2.25.1) at 2024-11-21 21:52:06 +0000