This is an automated email from the ASF dual-hosted git repository. coheigea pushed a commit to branch 2_2_x-fixes in repository https://gitbox.apache.org/repos/asf/ws-wss4j.git
The following commit(s) were added to refs/heads/2_2_x-fixes by this push: new 9e0a5d1 WSS-672 - Make sure to process all elements of the SAML Signature KeyInfo 9e0a5d1 is described below commit 9e0a5d1c0669c7443fe75dfa34231908aaa989c3 Author: Colm O hEigeartaigh <cohei...@apache.org> AuthorDate: Fri May 15 10:16:57 2020 +0100 WSS-672 - Make sure to process all elements of the SAML Signature KeyInfo --- .../org/apache/wss4j/common/saml/SAMLUtil.java | 36 ++++++++++++++++++---- 1 file changed, 30 insertions(+), 6 deletions(-) diff --git a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SAMLUtil.java b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SAMLUtil.java index 9e2936c..66a7d71 100644 --- a/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SAMLUtil.java +++ b/ws-security-common/src/main/java/org/apache/wss4j/common/saml/SAMLUtil.java @@ -46,6 +46,18 @@ import org.w3c.dom.Element; */ public final class SAMLUtil { + /** + * This constant defines the maximum amount of child elements of a Signature KeyInfo, associated with a + * signed SAML assertion. Any other child element will be ignored. + */ + private static final int MAX_KEYINFO_CONTENT_LIST_SIZE = 3; + + /** + * This constant defines the maximum amount of child elements of a X509Data KeyInfo, associated with a + * signed SAML assertion. Any other child element will be ignored. + */ + private static final int MAX_X509DATA_SIZE = 5; + private static final String SIG_NS = "http://www.w3.org/2000/09/xmldsig#"; private SAMLUtil() { @@ -195,7 +207,6 @@ public final class SAMLUtil { // Next marshal the KeyInfo DOM element into a javax KeyInfo object and get the // (public key) credential // - X509Certificate[] certs = null; KeyInfoFactory keyInfoFactory = null; try { keyInfoFactory = KeyInfoFactory.getInstance("DOM", "ApacheXMLDSig"); @@ -209,19 +220,27 @@ public final class SAMLUtil { keyInfoFactory.unmarshalKeyInfo(keyInfoStructure); List<?> list = keyInfo.getContent(); + X509Certificate[] certs = null; + PublicKey publicKey = null; for (int i = 0; i < list.size(); i++) { + // Put a hard bound on how many child elements there can be of KeyInfo + if (i >= MAX_KEYINFO_CONTENT_LIST_SIZE) { + break; + } XMLStructure xmlStructure = (XMLStructure) list.get(i); - if (xmlStructure instanceof KeyValue) { - PublicKey publicKey = ((KeyValue)xmlStructure).getPublicKey(); - return new SAMLKeyInfo(publicKey); + if (xmlStructure instanceof KeyValue && publicKey == null) { + publicKey = ((KeyValue)xmlStructure).getPublicKey(); } else if (xmlStructure instanceof X509Data) { List<?> x509Data = ((X509Data)xmlStructure).getContent(); for (int j = 0; j < x509Data.size(); j++) { + // Put a hard bound on how many child elements there can be of X509Data + if (j >= MAX_X509DATA_SIZE || certs != null) { + break; + } Object x509obj = x509Data.get(j); if (x509obj instanceof X509Certificate) { certs = new X509Certificate[1]; certs[0] = (X509Certificate)x509obj; - return new SAMLKeyInfo(certs); } else if (x509obj instanceof X509IssuerSerial) { if (sigCrypto == null) { throw new WSSecurityException( @@ -240,11 +259,16 @@ public final class SAMLUtil { new Object[] {"cannot get certificate or key"} ); } - return new SAMLKeyInfo(certs); } } } } + + if (certs != null || publicKey != null) { + SAMLKeyInfo samlKeyInfo = new SAMLKeyInfo(certs); + samlKeyInfo.setPublicKey(publicKey); + return samlKeyInfo; + } } catch (Exception ex) { throw new WSSecurityException( WSSecurityException.ErrorCode.FAILURE, ex, "invalidSAMLsecurity",