Skip to content

Commit 04b6adb

Browse files
committed
check that certificate of doc matches registered certificate/fingerprint before validating signature
1 parent 7f614e1 commit 04b6adb

File tree

1 file changed

+78
-18
lines changed
  • core/src/main/java/com/onelogin/saml2/util

1 file changed

+78
-18
lines changed

core/src/main/java/com/onelogin/saml2/util/Util.java

Lines changed: 78 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -28,10 +28,12 @@
2828
import java.security.cert.X509Certificate;
2929
import java.security.spec.PKCS8EncodedKeySpec;
3030
import java.util.Calendar;
31+
import java.util.HashMap;
3132
import java.util.HashSet;
3233
import java.util.Iterator;
3334
import java.util.List;
3435
import java.util.Locale;
36+
import java.util.Map;
3537
import java.util.Set;
3638
import java.util.TimeZone;
3739
import java.util.UUID;
@@ -66,7 +68,9 @@
6668
import org.apache.xml.security.encryption.XMLCipher;
6769
import org.apache.xml.security.exceptions.XMLSecurityException;
6870
import org.apache.xml.security.keys.KeyInfo;
71+
import org.apache.xml.security.keys.keyresolver.KeyResolverException;
6972
import org.apache.xml.security.signature.XMLSignature;
73+
import org.apache.xml.security.signature.XMLSignatureException;
7074
import org.apache.xml.security.transforms.Transforms;
7175
import org.apache.xml.security.utils.XMLUtils;
7276
import org.joda.time.DateTime;
@@ -892,12 +896,32 @@ public static boolean validateSign(final Document doc, final List<X509Certificat
892896

893897
if (signatures.getLength() == 1) {
894898
final Node signNode = signatures.item(0);
899+
900+
Map<String,Object> signatureData = getSignatureData(signNode, alg);
901+
if (signatureData.isEmpty()) {
902+
return false;
903+
}
904+
XMLSignature signature = (XMLSignature) signatureData.get("signature");
905+
X509Certificate extractedCert = (X509Certificate) signatureData.get("cert");
906+
String extractedFingerprint = (String) signatureData.get("fingerprint");
907+
895908
if (certList == null || certList.isEmpty()) {
896-
return validateSignNode(signNode, null, fingerprint, alg);
909+
return validateSignNode(signature, null, fingerprint, alg, extractedCert, extractedFingerprint);
897910
} else {
911+
Boolean certMatches = false;
898912
for (X509Certificate cert : certList) {
899-
if (validateSignNode(signNode, cert, fingerprint, alg))
913+
if (cert != null && extractedFingerprint != null && !extractedFingerprint.equals(calculateX509Fingerprint(cert, alg))) {
914+
continue;
915+
} else {
916+
certMatches = true;
917+
}
918+
919+
if (validateSignNode(signature, cert, fingerprint, alg, extractedCert, extractedFingerprint)) {
900920
return true;
921+
}
922+
}
923+
if (certMatches == false) {
924+
LOGGER.warn("Certificate used in the document does not match any registered certificate");
901925
}
902926
}
903927
}
@@ -949,6 +973,33 @@ public static Boolean validateMetadataSign(Document doc, X509Certificate cert, S
949973
return false;
950974
}
951975

976+
private static Map<String,Object> getSignatureData(Node signNode, String alg) {
977+
Map<String,Object> signatureData = new HashMap<>();
978+
try {
979+
Element sigElement = (Element) signNode;
980+
XMLSignature signature = new XMLSignature(sigElement, "", true);
981+
982+
String sigMethodAlg = signature.getSignedInfo().getSignatureMethodURI();
983+
if (!isAlgorithmWhitelisted(sigMethodAlg)){
984+
throw new Exception(sigMethodAlg + " is not a valid supported algorithm");
985+
}
986+
987+
String extractedFingerprint = null;
988+
X509Certificate extractedCert = null;
989+
KeyInfo keyInfo = signature.getKeyInfo();
990+
if (keyInfo != null && keyInfo.containsX509Data()) {
991+
extractedCert = keyInfo.getX509Certificate();
992+
extractedFingerprint = calculateX509Fingerprint(extractedCert, alg);
993+
}
994+
signatureData.put("signature", signature);
995+
signatureData.put("cert", extractedCert);
996+
signatureData.put("fingerprint", extractedFingerprint);
997+
} catch (Exception e) {
998+
LOGGER.warn("Error executing getSignatureData: " + e.getMessage(), e);
999+
}
1000+
return signatureData;
1001+
}
1002+
9521003
/**
9531004
* Validate signature of the Node.
9541005
*
@@ -962,31 +1013,40 @@ public static Boolean validateMetadataSign(Document doc, X509Certificate cert, S
9621013
* The signature algorithm method
9631014
*
9641015
* @return True if the sign is valid, false otherwise.
1016+
*
1017+
* @throws Exception
9651018
*/
9661019
public static Boolean validateSignNode(Node signNode, X509Certificate cert, String fingerprint, String alg) {
967-
Boolean res = false;
968-
try {
969-
Element sigElement = (Element) signNode;
970-
XMLSignature signature = new XMLSignature(sigElement, "", true);
1020+
Map<String,Object> signatureData = getSignatureData(signNode, alg);
1021+
if (signatureData.isEmpty()) {
1022+
return false;
1023+
}
9711024

972-
String sigMethodAlg = signature.getSignedInfo().getSignatureMethodURI();
973-
if (!isAlgorithmWhitelisted(sigMethodAlg)){
974-
throw new Exception(sigMethodAlg + " is not a valid supported algorithm");
975-
}
1025+
XMLSignature signature = (XMLSignature) signatureData.get("signature");
1026+
X509Certificate extractedCert = (X509Certificate) signatureData.get("cert");
1027+
String extractedFingerprint = (String) signatureData.get("fingerprint");
1028+
1029+
return validateSignNode(signature, cert, fingerprint, alg, extractedCert, extractedFingerprint);
1030+
}
9761031

1032+
public static Boolean validateSignNode(XMLSignature signature, X509Certificate cert, String fingerprint, String alg, X509Certificate extractedCert, String extractedFingerprint) {
1033+
Boolean res = false;
1034+
try {
9771035
if (cert != null) {
9781036
res = signature.checkSignatureValue(cert);
979-
} else {
980-
KeyInfo keyInfo = signature.getKeyInfo();
981-
if (fingerprint != null && keyInfo != null && keyInfo.containsX509Data()) {
982-
X509Certificate providedCert = keyInfo.getX509Certificate();
983-
String calculatedFingerprint = calculateX509Fingerprint(providedCert, alg);
984-
for (String fingerprintStr : fingerprint.split(",")) {
985-
if (calculatedFingerprint.equals(fingerprintStr.trim())) {
986-
res = signature.checkSignatureValue(providedCert);
1037+
} else if (extractedCert != null && fingerprint != null && extractedFingerprint != null) {
1038+
Boolean fingerprintMatches = false;
1039+
for (String fingerprintStr : fingerprint.split(",")) {
1040+
if (extractedFingerprint.equals(fingerprintStr.trim())) {
1041+
fingerprintMatches = true;
1042+
if (res = signature.checkSignatureValue(extractedCert)) {
1043+
break;
9871044
}
9881045
}
9891046
}
1047+
if (fingerprintMatches == false) {
1048+
LOGGER.warn("Fingerprint of the certificate used in the document does not match any registered fingerprints");
1049+
}
9901050
}
9911051
} catch (Exception e) {
9921052
LOGGER.warn("Error executing validateSignNode: " + e.getMessage(), e);

0 commit comments

Comments
 (0)