diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/sig/SAML2Signature.java b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/sig/SAML2Signature.java index 09ce9d62f8..dd21333e3b 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/sig/SAML2Signature.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/api/saml/v2/sig/SAML2Signature.java @@ -134,14 +134,11 @@ public class SAML2Signature { dto.setSignatureMethod(signatureMethod); dto.setReferenceURI(referenceURI); dto.setNextSibling(sibling); - - if (x509Certificate != null) { - dto.setX509Certificate(x509Certificate); - } + dto.setX509Certificate(x509Certificate); return XMLSignatureUtil.sign(dto, canonicalizationMethodType); } - return XMLSignatureUtil.sign(doc, keyName, keyPair, digestMethod, signatureMethod, referenceURI, canonicalizationMethodType); + return XMLSignatureUtil.sign(doc, keyName, keyPair, digestMethod, signatureMethod, referenceURI, x509Certificate, canonicalizationMethodType); } /** @@ -224,4 +221,4 @@ public class SAML2Signature { element.setIdAttribute(JBossSAMLConstants.ID.get(), true); } -} \ No newline at end of file +} diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java index b8a6ad6c2f..b9ea4d622c 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/util/XMLSignatureUtil.java @@ -723,9 +723,7 @@ public class XMLSignatureUtil { if (x509Certificate != null) { items.add(keyInfoFactory.newX509Data(Collections.singletonList(x509Certificate))); - } - - if (publicKey != null) { + } else if (publicKey != null) { items.add(keyInfoFactory.newKeyValue(publicKey)); } diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java index 07f2c727da..7bea27e1cc 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java @@ -89,6 +89,7 @@ import javax.xml.stream.XMLStreamWriter; import java.io.StringWriter; import java.net.URI; import java.security.KeyPair; +import java.security.cert.X509Certificate; import java.util.ArrayList; import java.util.Arrays; import java.util.Iterator; @@ -457,13 +458,15 @@ public class SAMLIdentityProvider extends AbstractIdentityProviderStian Thorgersen */ @@ -1044,7 +1055,7 @@ public class IdentityProviderTest extends AbstractAdminTest { } @Test - public void testSamlExportSignatureOn() throws URISyntaxException, IOException, ConfigurationException, ParsingException, ProcessingException { + public void testSamlExportSignatureOn() throws URISyntaxException, IOException, ConfigurationException, ParsingException, ProcessingException, CertificateEncodingException { // Use import-config to convert IDPSSODescriptor file into key value pairs // to use when creating a SAML Identity Provider MultipartFormDataOutput form = new MultipartFormDataOutput(); @@ -1059,6 +1070,7 @@ public class IdentityProviderTest extends AbstractAdminTest { // Explicitly enable SP Metadata Signature result.put(SAMLIdentityProviderConfig.SIGN_SP_METADATA, "true"); + result.put(SAMLIdentityProviderConfig.XML_SIG_KEY_INFO_KEY_NAME_TRANSFORMER, XmlKeyInfoKeyNameTransformer.CERT_SUBJECT.name()); // Create new SAML identity provider using configuration retrieved from import-config IdentityProviderRepresentation idpRep = createRep("saml", "saml", true, result); @@ -1073,6 +1085,32 @@ public class IdentityProviderTest extends AbstractAdminTest { Document document = DocumentUtil.getDocument(body); Element signatureElement = DocumentUtil.getDirectChildElement(document.getDocumentElement(), XMLDSIG_NSURI.get(), "Signature"); - Assert.assertNotNull(signatureElement); + Assert.assertThat("Signature not null", signatureElement, notNullValue()); + + Element keyInfoElement = DocumentUtil.getDirectChildElement(signatureElement, XMLDSIG_NSURI.get(), "KeyInfo"); + Assert.assertThat("KeyInfo not null", keyInfoElement, notNullValue()); + + Element x509DataElement = DocumentUtil.getDirectChildElement(keyInfoElement, XMLDSIG_NSURI.get(), "X509Data"); + Assert.assertThat("X509Data not null", x509DataElement, notNullValue()); + + Element x509CertificateElement = DocumentUtil.getDirectChildElement(x509DataElement, XMLDSIG_NSURI.get(), "X509Certificate"); + Assert.assertThat("X509Certificate not null", x509CertificateElement, notNullValue()); + + Element keyNameElement = DocumentUtil.getDirectChildElement(keyInfoElement, XMLDSIG_NSURI.get(), "KeyName"); + Assert.assertThat("KeyName not null", keyNameElement, notNullValue()); + + String activeSigCert = KeyUtils.findActiveSigningKey(realm, Constants.DEFAULT_SIGNATURE_ALGORITHM).getCertificate(); + Assert.assertThat("activeSigCert not null", activeSigCert, notNullValue()); + + X509Certificate activeX509SigCert = XMLSignatureUtil.getX509CertificateFromKeyInfoString(activeSigCert); + Assert.assertThat("KeyName matches subject DN", + keyNameElement.getTextContent().trim(), equalTo(activeX509SigCert.getSubjectDN().getName())); + + Assert.assertThat("Signing cert matches active realm cert", + x509CertificateElement.getTextContent().trim(), equalTo(Base64.getEncoder().encodeToString(activeX509SigCert.getEncoded()))); + + PublicKey activePublicSigKey = activeX509SigCert.getPublicKey(); + Assert.assertThat("Metadata signature is valid", + new SAML2Signature().validate(document, new HardcodedKeyLocator(activePublicSigKey)), is(true)); } }