Skip to content

Commit 3bb2d31

Browse files
sgramponeBeta Bot
authored andcommitted
Cherry pick branch 'genexuslabs:assertion_manipulation_attack' into beta
1 parent 47db56b commit 3bb2d31

File tree

5 files changed

+78
-15
lines changed

5 files changed

+78
-15
lines changed

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

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,10 +15,10 @@ public class PostBinding extends Binding {
1515
private static final Logger logger = LogManager.getLogger(PostBinding.class);
1616

1717
private Document xmlDoc;
18+
private Document verifiedDoc;
1819

1920
public PostBinding() {
2021
logger.trace("PostBinding constructor");
21-
xmlDoc = null;
2222
}
2323
// EXTERNAL OBJECT PUBLIC METHODS - BEGIN
2424

@@ -42,31 +42,39 @@ public static String logout(SamlParms parms, String relayState) {
4242
}
4343

4444
public boolean verifySignatures(SamlParms parms) {
45-
return DSig.validateSignatures(this.xmlDoc, parms.getTrustCertPath(), parms.getTrustCertAlias(), parms.getTrustCertPass());
45+
String verified = DSig.validateSignatures(this.xmlDoc, parms.getTrustCertPath(), parms.getTrustCertAlias(), parms.getTrustCertPass());
46+
if(verified.isEmpty()){
47+
return false;
48+
}else {
49+
this.verifiedDoc = SamlAssertionUtils.loadDocument(verified);
50+
this.xmlDoc = null;
51+
logger.debug(MessageFormat.format("verifySignatures sanitized xmlDoc {0}", Encoding.documentToString(this.xmlDoc)));
52+
return true;
53+
}
4654
}
4755

4856
public String getLoginAssertions() {
4957
logger.trace("getLoginAssertions");
50-
return SamlAssertionUtils.getLoginInfo(this.xmlDoc);
58+
return SamlAssertionUtils.getLoginInfo(this.verifiedDoc);
5159
}
5260

5361
public String getLogoutAssertions() {
5462
logger.trace("getLogoutAssertions");
55-
return SamlAssertionUtils.getLogoutInfo(this.xmlDoc);
63+
return SamlAssertionUtils.getLogoutInfo(this.verifiedDoc);
5664
}
5765

5866
public String getLoginAttribute(String name) {
5967
logger.trace("getLoginAttribute");
60-
return SamlAssertionUtils.getLoginAttribute(this.xmlDoc, name).trim();
68+
return SamlAssertionUtils.getLoginAttribute(this.verifiedDoc, name).trim();
6169
}
6270

6371
public String getRoles(String name) {
6472
logger.debug("getRoles");
65-
return SamlAssertionUtils.getRoles(this.xmlDoc, name);
73+
return SamlAssertionUtils.getRoles(this.verifiedDoc, name);
6674
}
6775

6876
public boolean isLogout(){
69-
return SamlAssertionUtils.isLogout(this.xmlDoc);
77+
return SamlAssertionUtils.isLogout(this.verifiedDoc);
7078
}
7179

7280
// EXTERNAL OBJECT PUBLIC METHODS - END

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

Lines changed: 12 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import org.apache.xml.security.utils.Constants;
77
import org.w3c.dom.Document;
88
import org.w3c.dom.Element;
9+
import org.w3c.dom.Node;
910
import org.w3c.dom.NodeList;
1011

1112
import javax.xml.xpath.XPath;
@@ -22,40 +23,44 @@ public class DSig {
2223

2324
private static final Logger logger = LogManager.getLogger(DSig.class);
2425

25-
public static boolean validateSignatures(Document xmlDoc, String certPath, String certAlias, String certPassword) {
26+
public static String validateSignatures(Document xmlDoc, String certPath, String certAlias, String certPassword) {
2627
logger.trace("validateSignatures");
28+
List<Element> assertions = new ArrayList<Element>();
2729
X509Certificate cert = Keys.loadCertificate(certPath, certAlias, certPassword);
2830

2931
NodeList nodes = findElementsByPath(xmlDoc, "//*[@ID]");
3032

3133
NodeList signatures = xmlDoc.getElementsByTagNameNS(Constants.SignatureSpecNS, Constants._TAG_SIGNATURE);
3234
//check the message is signed - security measure
3335
if(signatures.getLength() == 0){
34-
return false;
36+
return "";
3537
}
3638
for (int i = 0; i < signatures.getLength(); i++) {
3739
Element signedElement = findNodeById(nodes, getSignatureID((Element) signatures.item(i)));
40+
assertions.add(signedElement);
3841
if (signedElement == null) {
39-
return false;
42+
return "";
4043
}
4144
signedElement.setIdAttribute("ID", true);
4245
try {
4346
XMLSignature signature = new XMLSignature((Element) signatures.item(i), "");
4447
//verifies the signature algorithm is one expected - security meassure
4548
if (!verifySignatureAlgorithm((Element) signatures.item(i))) {
46-
return false;
49+
return "";
4750
}
4851
if (!signature.checkSignatureValue(cert)) {
49-
return false;
52+
return "";
5053
}
5154
} catch (Exception e) {
5255
logger.error("validateSignatures", e);
53-
return false;
56+
return "";
5457
}
5558
}
56-
return true;
59+
return SamlAssertionUtils.isLogout(xmlDoc) ? SamlAssertionUtils.buildXmlLogout(assertions) : SamlAssertionUtils.buildXmlLogin(assertions, xmlDoc);
5760
}
5861

62+
63+
5964
private static boolean verifySignatureAlgorithm(Element elem) {
6065
logger.trace("verifySignatureAlgorithm");
6166
NodeList signatureMethod = elem.getElementsByTagNameNS(Constants.SignatureSpecNS, Constants._TAG_SIGNATUREMETHOD);

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
import org.apache.logging.log4j.Logger;
55
import org.bouncycastle.util.encoders.Base64;
66
import org.w3c.dom.Document;
7+
import org.w3c.dom.Element;
78

9+
import javax.xml.transform.OutputKeys;
810
import javax.xml.transform.Transformer;
911
import javax.xml.transform.TransformerFactory;
1012
import javax.xml.transform.dom.DOMSource;
@@ -74,6 +76,24 @@ public static String documentToString(Document doc) {
7476
}
7577
}
7678

79+
public static String elementToString(Element element) {
80+
try {
81+
TransformerFactory tf = TransformerFactory.newInstance();
82+
Transformer transformer = tf.newTransformer();
83+
84+
transformer.setOutputProperty(OutputKeys.OMIT_XML_DECLARATION, "yes");
85+
transformer.setOutputProperty(OutputKeys.INDENT, "yes");
86+
87+
StringWriter writer = new StringWriter();
88+
transformer.transform(new DOMSource(element), new StreamResult(writer));
89+
90+
return writer.toString();
91+
} catch (Exception e) {
92+
logger.error("elementToString", e);
93+
return null;
94+
}
95+
}
96+
7797
public static byte[] decodeParameter(String parm) {
7898
logger.trace("decodeParameter");
7999
try {

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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,6 @@ private static X509Certificate loadCertificateFromJKS(String path, String alias,
112112
logger.debug(MessageFormat.format("path: {0}, alias: {1}", path, alias));
113113
Path p = new File(path).toPath();
114114
logger.debug("Res path: " + p.toAbsolutePath());
115-
System.out.println("Res path: " + p.toAbsolutePath());
116115
try (InputStream in = new DataInputStream(Files.newInputStream(p))) {
117116
KeyStore ks = KeyStore.getInstance("JKS");
118117
ks.load(in, password.toCharArray());

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,37 @@ public static Document loadDocument(String xml) {
4949
}
5050
}
5151

52+
public static String buildXmlLogin(List<org.w3c.dom.Element> assertions, Document xmlDoc){
53+
//security meassure against assertion manipulation, it assures that every assertion to be used on the app has been signed and verified
54+
org.w3c.dom.Element element = xmlDoc.getDocumentElement();
55+
Node response = element.cloneNode(false);
56+
57+
NodeList status = element.getElementsByTagNameNS(_saml_protocolNS, "Status");
58+
response.appendChild(status.item(0));
59+
60+
for(org.w3c.dom.Element elem: assertions){
61+
if(!elem.getLocalName().equals("Response")){
62+
Node node = elem.cloneNode(true);
63+
response.appendChild(node);
64+
}
65+
}
66+
return Encoding.elementToString((org.w3c.dom.Element) response);
67+
}
68+
69+
public static String buildXmlLogout(List<org.w3c.dom.Element> assertions){
70+
if(assertions.isEmpty())
71+
{
72+
return "";
73+
}
74+
org.w3c.dom.Element element = assertions.get(0);
75+
Node logoutResponse = element.cloneNode(false);
76+
NodeList status = element.getElementsByTagNameNS(_saml_protocolNS, "Status");
77+
logoutResponse.appendChild(status.item(0));
78+
NodeList issuer = element.getElementsByTagNameNS(_saml_assertionNS, "Issuer");
79+
logoutResponse.appendChild(issuer.item(0));
80+
return Encoding.elementToString((org.w3c.dom.Element) logoutResponse);
81+
}
82+
5283
public static boolean isLogout(Document xmlDoc){
5384
logger.trace("isLogout");
5485
try {

0 commit comments

Comments
 (0)