mirror of
				https://github.com/apache/cloudstack.git
				synced 2025-10-26 08:42:29 +01:00 
			
		
		
		
	saml: Safer DocumentBuilderFactory and ParserPool configuration (#183)
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 <rohit.yadav@shapeblue.com>
This commit is contained in:
		
							parent
							
								
									7899f5cce6
								
							
						
					
					
						commit
						7c7ee05cef
					
				| @ -31,6 +31,7 @@ | ||||
|         <dependency> | ||||
|             <groupId>org.opensaml</groupId> | ||||
|             <artifactId>opensaml</artifactId> | ||||
|             <version>${cs.opensaml.version}</version> | ||||
|         </dependency> | ||||
|         <dependency> | ||||
|             <groupId>org.apache.cloudstack</groupId> | ||||
|  | ||||
| @ -30,6 +30,7 @@ import org.apache.cloudstack.api.auth.PluggableAPIAuthenticator; | ||||
| 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 class GetServiceProviderMetaDataCmd extends BaseCmd implements APIAuthent | ||||
| 
 | ||||
|         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); | ||||
|  | ||||
| @ -78,7 +78,6 @@ import org.opensaml.saml2.metadata.provider.HTTPMetadataProvider; | ||||
| 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 @@ public class SAML2AuthManagerImpl extends AdapterBase implements SAML2AuthManage | ||||
|                 } | ||||
|             } | ||||
|             _idpMetaDataProvider.setRequireValidMetadata(true); | ||||
|             _idpMetaDataProvider.setParserPool(new BasicParserPool()); | ||||
|             _idpMetaDataProvider.setParserPool(SAMLUtils.getSaferParserPool()); | ||||
|             _idpMetaDataProvider.initialize(); | ||||
|             _timer.scheduleAtFixedRate(new MetadataRefreshTask(), 0, _refreshInterval * 1000); | ||||
| 
 | ||||
|  | ||||
| @ -42,12 +42,15 @@ import java.security.cert.X509Certificate; | ||||
| 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 javax.xml.stream.FactoryConfigurationError; | ||||
| 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.MarshallingException; | ||||
| 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 class SAMLUtils { | ||||
|     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 class SAMLUtils { | ||||
|                 "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; | ||||
|     } | ||||
| } | ||||
|  | ||||
							
								
								
									
										2
									
								
								pom.xml
									
									
									
									
									
								
							
							
						
						
									
										2
									
								
								pom.xml
									
									
									
									
									
								
							| @ -160,7 +160,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.java-linstor.version>0.3.0</cs.java-linstor.version> | ||||
|         <cs.reflections.version>0.9.12</cs.reflections.version> | ||||
|  | ||||
| @ -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; | ||||
|     } | ||||
| } | ||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user