saml: Safer DocumentBuilderFactory and ParserPool configuration

This implements safer DocumentBuilderFactory and ParserPool utilities
to be used throughout the codebase to prevent potential XXE exploits.

References:
https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
https://www.blackhat.com/docs/us-15/materials/us-15-Wang-FileCry-The-New-Age-Of-XXE-java-wp.pdf

Signed-off-by: Rohit Yadav <[email protected]>
(cherry picked from commit 8e0e68ef368ebe2793ef80e2c3821eaecb47b593)
Signed-off-by: Rohit Yadav <[email protected]>

Author: rohityadavcloud

plugins/user-authenticators/saml2/pom.xml

@@ -31,6 +31,7 @@
         <dependency>
             <groupId>org.opensaml</groupId>
             <artifactId>opensaml</artifactId>
+            <version>${cs.opensaml.version}</version>
         </dependency>
         <dependency>
             <groupId>org.apache.cloudstack</groupId>

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/api/command/GetServiceProviderMetaDataCmd.java

@@ -30,6 +30,7 @@
 import org.apache.cloudstack.api.response.SAMLMetaDataResponse;
 import org.apache.cloudstack.saml.SAML2AuthManager;
 import org.apache.cloudstack.saml.SAMLProviderMetadata;
+import org.apache.cloudstack.utils.security.ParserUtils;
 import org.apache.log4j.Logger;
 import org.opensaml.Configuration;
 import org.opensaml.DefaultBootstrap;
@@ -239,7 +240,7 @@ public String authenticate(String command, Map<String, Object[]> params, HttpSes
 
         StringWriter stringWriter = new StringWriter();
         try {
-            DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+            DocumentBuilderFactory factory = ParserUtils.getSaferDocumentBuilderFactory();
             DocumentBuilder builder = factory.newDocumentBuilder();
             Document document = builder.newDocument();
             Marshaller out = Configuration.getMarshallerFactory().getMarshaller(spEntityDescriptor);

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAML2AuthManagerImpl.java

@@ -78,7 +78,6 @@
 import org.opensaml.saml2.metadata.provider.MetadataProviderException;
 import org.opensaml.xml.ConfigurationException;
 import org.opensaml.xml.XMLObject;
-import org.opensaml.xml.parse.BasicParserPool;
 import org.opensaml.xml.security.credential.UsageType;
 import org.opensaml.xml.security.keyinfo.KeyInfoHelper;
 import org.springframework.stereotype.Component;
@@ -389,7 +388,7 @@ private boolean setup() {
                 }
             }
             _idpMetaDataProvider.setRequireValidMetadata(true);
-            _idpMetaDataProvider.setParserPool(new BasicParserPool());
+            _idpMetaDataProvider.setParserPool(SAMLUtils.getSaferParserPool());
             _idpMetaDataProvider.initialize();
             _timer.scheduleAtFixedRate(new MetadataRefreshTask(), 0, _refreshInterval * 1000);
 

plugins/user-authenticators/saml2/src/main/java/org/apache/cloudstack/saml/SAMLUtils.java

@@ -42,12 +42,15 @@
 import java.security.spec.InvalidKeySpecException;
 import java.security.spec.PKCS8EncodedKeySpec;
 import java.security.spec.X509EncodedKeySpec;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.zip.Deflater;
 import java.util.zip.DeflaterOutputStream;
 
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletResponse;
+import javax.xml.XMLConstants;
 import javax.xml.parsers.DocumentBuilder;
 import javax.xml.parsers.DocumentBuilderFactory;
 import javax.xml.parsers.ParserConfigurationException;
@@ -56,6 +59,7 @@
 import org.apache.cloudstack.api.ApiConstants;
 import org.apache.cloudstack.api.response.LoginCmdResponse;
 import org.apache.cloudstack.utils.security.CertUtils;
+import org.apache.cloudstack.utils.security.ParserUtils;
 import org.apache.log4j.Logger;
 import org.bouncycastle.operator.OperatorCreationException;
 import org.joda.time.DateTime;
@@ -88,6 +92,7 @@
 import org.opensaml.xml.io.Unmarshaller;
 import org.opensaml.xml.io.UnmarshallerFactory;
 import org.opensaml.xml.io.UnmarshallingException;
+import org.opensaml.xml.parse.BasicParserPool;
 import org.opensaml.xml.signature.SignatureConstants;
 import org.opensaml.xml.util.Base64;
 import org.opensaml.xml.util.XMLHelper;
@@ -227,7 +232,7 @@ public static String encodeSAMLRequest(XMLObject authnRequest)
     public static Response decodeSAMLResponse(String responseMessage)
             throws ConfigurationException, ParserConfigurationException,
             SAXException, IOException, UnmarshallingException {
-        DocumentBuilderFactory documentBuilderFactory = DocumentBuilderFactory.newInstance();
+        DocumentBuilderFactory documentBuilderFactory = ParserUtils.getSaferDocumentBuilderFactory();
         documentBuilderFactory.setNamespaceAware(true);
         DocumentBuilder docBuilder = documentBuilderFactory.newDocumentBuilder();
         byte[] base64DecodedResponse = Base64.decode(responseMessage);
@@ -361,4 +366,19 @@ public static X509Certificate generateRandomX509Certificate(KeyPair keyPair) thr
                 "CN=ApacheCloudStack", "CN=ApacheCloudStack",
                 3, "SHA256WithRSA");
     }
+
+    public static BasicParserPool getSaferParserPool() {
+        final Map<String, Boolean> features = new HashMap<>();
+        features.put(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        features.put("http://apache.org/xml/features/disallow-doctype-decl", true);
+        features.put("http://xml.org/sax/features/external-general-entities", false);
+        features.put("http://xml.org/sax/features/external-parameter-entities", false);
+        features.put("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        final BasicParserPool parserPool = new BasicParserPool();
+        parserPool.setXincludeAware(false);
+        parserPool.setIgnoreComments(true);
+        parserPool.setExpandEntityReferences(false);
+        parserPool.setBuilderFeatures(features);
+        return parserPool;
+    }
 }

pom.xml

@@ -159,7 +159,7 @@
         <cs.mysql.version>8.0.19</cs.mysql.version>
         <cs.neethi.version>2.0.4</cs.neethi.version>
         <cs.nitro.version>10.1</cs.nitro.version>
-        <cs.opensaml.version>2.6.4</cs.opensaml.version>
+        <cs.opensaml.version>2.6.6</cs.opensaml.version>
         <cs.rados-java.version>0.6.0</cs.rados-java.version>
         <cs.reflections.version>0.9.12</cs.reflections.version>
         <cs.servicemix.version>3.4.4_1</cs.servicemix.version>

utils/src/main/java/org/apache/cloudstack/utils/security/ParserUtils.java

@@ -0,0 +1,41 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package org.apache.cloudstack.utils.security;
+
+import javax.xml.XMLConstants;
+import javax.xml.parsers.DocumentBuilderFactory;
+import javax.xml.parsers.ParserConfigurationException;
+
+public class ParserUtils {
+    public static DocumentBuilderFactory getSaferDocumentBuilderFactory() throws ParserConfigurationException {
+        DocumentBuilderFactory factory = DocumentBuilderFactory.newInstance();
+
+        // REDHAT https://www.blackhat.com/docs/us-15/materials/us-15-Wang-FileCry-The-New-Age-Of-XXE-java-wp.pdf
+        // OWASP https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+        // and these as well, per Timothy Morgan's 2014 paper: "XML Schema, DTD, and Entity Attacks"
+        factory.setFeature(XMLConstants.FEATURE_SECURE_PROCESSING, true);
+        factory.setFeature("http://apache.org/xml/features/disallow-doctype-decl", true);
+        factory.setFeature("http://xml.org/sax/features/external-general-entities", false);
+        factory.setFeature("http://xml.org/sax/features/external-parameter-entities", false);
+        factory.setFeature("http://apache.org/xml/features/nonvalidating/load-external-dtd", false);
+        factory.setXIncludeAware(false);
+        factory.setExpandEntityReferences(false);
+
+        return factory;
+    }
+}