Assertion manipulation attack mitigation (#1004)

* Asserition manipulation attack mitigation

* Fix Logout errors

* Add log

Author: sgrampone

gamsaml20/src/main/java/com/genexus/saml20/PostBinding.java

@@ -15,10 +15,10 @@ public class PostBinding extends Binding {
 	private static final Logger logger = LogManager.getLogger(PostBinding.class);
 
 	private Document xmlDoc;
+	private Document verifiedDoc;
 
 	public PostBinding() {
 		logger.trace("PostBinding constructor");
-		xmlDoc = null;
 	}
 	// EXTERNAL OBJECT PUBLIC METHODS  - BEGIN
 
@@ -42,27 +42,34 @@ public static String logout(SamlParms parms, String relayState) {
 	}
 
 	public boolean verifySignatures(SamlParms parms) {
-		return DSig.validateSignatures(this.xmlDoc, parms.getTrustCertPath(), parms.getTrustCertAlias(), parms.getTrustCertPass());
+		String verified = DSig.validateSignatures(this.xmlDoc, parms.getTrustCertPath(), parms.getTrustCertAlias(), parms.getTrustCertPass());
+		if(verified.isEmpty()){
+			return false;
+		}else {
+			this.verifiedDoc = SamlAssertionUtils.loadDocument(verified);
+			logger.debug(MessageFormat.format("verifySignatures - sanitized xmlDoc {0}", Encoding.documentToString(this.xmlDoc)));
+			return true;
+		}
 	}
 
 	public String getLoginAssertions() {
 		logger.trace("getLoginAssertions");
-		return SamlAssertionUtils.getLoginInfo(this.xmlDoc);
+		return SamlAssertionUtils.getLoginInfo(this.verifiedDoc);
 	}
 
 	public String getLogoutAssertions() {
 		logger.trace("getLogoutAssertions");
-		return SamlAssertionUtils.getLogoutInfo(this.xmlDoc);
+		return SamlAssertionUtils.getLogoutInfo(this.verifiedDoc);
 	}
 
 	public String getLoginAttribute(String name) {
 		logger.trace("getLoginAttribute");
-		return SamlAssertionUtils.getLoginAttribute(this.xmlDoc, name).trim();
+		return SamlAssertionUtils.getLoginAttribute(this.verifiedDoc, name).trim();
 	}
 
 	public String getRoles(String name) {
 		logger.debug("getRoles");
-		return SamlAssertionUtils.getRoles(this.xmlDoc, name);
+		return SamlAssertionUtils.getRoles(this.verifiedDoc, name);
 	}
 
 	public boolean isLogout(){

gamsaml20/src/main/java/com/genexus/saml20/utils/DSig.java

@@ -6,6 +6,7 @@
 import org.apache.xml.security.utils.Constants;
 import org.w3c.dom.Document;
 import org.w3c.dom.Element;
+import org.w3c.dom.Node;
 import org.w3c.dom.NodeList;
 
 import javax.xml.xpath.XPath;
@@ -22,40 +23,44 @@ public class DSig {
 
 	private static final Logger logger = LogManager.getLogger(DSig.class);
 
-	public static boolean validateSignatures(Document xmlDoc, String certPath, String certAlias, String certPassword) {
+	public static String validateSignatures(Document xmlDoc, String certPath, String certAlias, String certPassword) {
 		logger.trace("validateSignatures");
+		List<Element> assertions = new ArrayList<Element>();
 		X509Certificate cert = Keys.loadCertificate(certPath, certAlias, certPassword);
 
 		NodeList nodes = findElementsByPath(xmlDoc, "//*[@ID]");
 
 		NodeList signatures = xmlDoc.getElementsByTagNameNS(Constants.SignatureSpecNS, Constants._TAG_SIGNATURE);
 		//check the message is signed - security measure
 		if(signatures.getLength() == 0){
-			return false;
+			return "";
 		}
 		for (int i = 0; i < signatures.getLength(); i++) {
 			Element signedElement = findNodeById(nodes, getSignatureID((Element) signatures.item(i)));
+			assertions.add(signedElement);
 			if (signedElement == null) {
-				return false;
+				return "";
 			}
 			signedElement.setIdAttribute("ID", true);
 			try {
 				XMLSignature signature = new XMLSignature((Element) signatures.item(i), "");
 				//verifies the signature algorithm is one expected - security meassure
 				if (!verifySignatureAlgorithm((Element) signatures.item(i))) {
-					return false;
+					return "";
 				}
 				if (!signature.checkSignatureValue(cert)) {
-					return false;
+					return "";
 				}
 			} catch (Exception e) {
 				logger.error("validateSignatures", e);
-				return false;
+				return "";
 			}
 		}
-		return true;
+		return SamlAssertionUtils.isLogout(xmlDoc) ? SamlAssertionUtils.buildXmlLogout(assertions) : SamlAssertionUtils.buildXmlLogin(assertions, xmlDoc);
 	}
 
+
+
 	private static boolean verifySignatureAlgorithm(Element elem) {
 		logger.trace("verifySignatureAlgorithm");
 		NodeList signatureMethod = elem.getElementsByTagNameNS(Constants.SignatureSpecNS, Constants._TAG_SIGNATUREMETHOD);

gamsaml20/src/main/java/com/genexus/saml20/utils/Encoding.java

@@ -4,7 +4,9 @@
 import org.apache.logging.log4j.Logger;
 import org.bouncycastle.util.encoders.Base64;
 import org.w3c.dom.Document;
+import org.w3c.dom.Element;
 
+import javax.xml.transform.OutputKeys;
 import javax.xml.transform.Transformer;
 import javax.xml.transform.TransformerFactory;
 import javax.xml.transform.dom.DOMSource;
@@ -74,6 +76,24 @@ public static String documentToString(Document doc) {
 		}
 	}
 
+	public static String elementToString(Element element) {
+		try {
+			TransformerFactory tf = TransformerFactory.newInstance();
+			Transformer transformer = tf.newTransformer();
+
+			transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
+			transformer.setOutputProperty(OutputKeys.INDENT, "yes");
+
+			StringWriter writer = new StringWriter();
+			transformer.transform(new DOMSource(element), new StreamResult(writer));
+
+			return writer.toString();
+		} catch (Exception e) {
+			logger.error("elementToString", e);
+			return null;
+		}
+	}
+
 	public static byte[] decodeParameter(String parm) {
 		logger.trace("decodeParameter");
 		try {

gamsaml20/src/main/java/com/genexus/saml20/utils/Keys.java

@@ -112,7 +112,6 @@ private static X509Certificate loadCertificateFromJKS(String path, String alias,
 		logger.debug(MessageFormat.format("path: {0}, alias: {1}", path, alias));
 		Path p = new File(path).toPath();
 		logger.debug("Res path: " + p.toAbsolutePath());
-		System.out.println("Res path: " + p.toAbsolutePath());
 		try (InputStream in = new DataInputStream(Files.newInputStream(p))) {
 			KeyStore ks = KeyStore.getInstance("JKS");
 			ks.load(in, password.toCharArray());

gamsaml20/src/main/java/com/genexus/saml20/utils/SamlAssertionUtils.java

@@ -49,6 +49,40 @@ public static Document loadDocument(String xml) {
 		}
 	}
 
+	public static String buildXmlLogin(List<org.w3c.dom.Element> assertions, Document xmlDoc){
+		//security meassure against assertion manipulation, it assures that every assertion to be used on the app has been signed and verified
+		logger.trace("buildXmlLogin");
+		org.w3c.dom.Element element = xmlDoc.getDocumentElement();
+		Node response  = element.cloneNode(false);
+
+		NodeList status = element.getElementsByTagNameNS(_saml_protocolNS, "Status");
+		response.appendChild(status.item(0));
+
+		for(org.w3c.dom.Element elem: assertions){
+			if(!elem.getLocalName().equals("Response")){
+				Node node = elem.cloneNode(true);
+				response.appendChild(node);
+			}
+		}
+		return Encoding.elementToString((org.w3c.dom.Element) response);
+	}
+
+	public static String buildXmlLogout(List<org.w3c.dom.Element> assertions){
+		logger.trace("buildXmlLogout");
+		if(assertions.isEmpty())
+		{
+			logger.error("buildXmlLogout - There are 0 signed assertions on LogoutResponse");
+			return "";
+		}
+		org.w3c.dom.Element element =  assertions.get(0);
+		Node logoutResponse = element.cloneNode(false);
+		NodeList status = element.getElementsByTagNameNS(_saml_protocolNS, "Status");
+		logoutResponse.appendChild(status.item(0));
+		NodeList issuer = element.getElementsByTagNameNS(_saml_assertionNS, "Issuer");
+		logoutResponse.appendChild(issuer.item(0));
+		return Encoding.elementToString((org.w3c.dom.Element) logoutResponse);
+	}
+
 	public static boolean isLogout(Document xmlDoc){
 		logger.trace("isLogout");
 		try {