diff --git a/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java b/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java index 9a28137694..5feda2b568 100755 --- a/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java +++ b/saml-core/src/main/java/org/keycloak/saml/SPMetadataDescriptor.java @@ -23,10 +23,10 @@ package org.keycloak.saml; */ public class SPMetadataDescriptor { - public static String getSPDescriptor(String binding, String assertionEndpoint, String logoutEndpoint, boolean wantAuthnRequestsSigned, String entityId, String nameIDPolicyFormat, String signingCerts) { + public static String getSPDescriptor(String binding, String assertionEndpoint, String logoutEndpoint, boolean wantAuthnRequestsSigned, boolean wantAssertionsSigned, String entityId, String nameIDPolicyFormat, String signingCerts) { String descriptor = "\n" + - " \n"; if (wantAuthnRequestsSigned && signingCerts != null) { descriptor += signingCerts; diff --git a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java index 89d668052e..bb15d233bf 100755 --- a/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java +++ b/saml-core/src/main/java/org/keycloak/saml/processing/core/saml/v2/util/AssertionUtil.java @@ -32,6 +32,7 @@ import org.keycloak.dom.saml.v2.assertion.StatementAbstractType; import org.keycloak.dom.saml.v2.assertion.SubjectType; import org.keycloak.dom.saml.v2.assertion.SubjectType.STSubType; import org.keycloak.dom.saml.v2.protocol.ResponseType; +import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.common.ErrorCodes; import org.keycloak.saml.common.PicketLinkLogger; import org.keycloak.saml.common.PicketLinkLoggerFactory; @@ -41,7 +42,6 @@ import org.keycloak.saml.common.exceptions.ParsingException; import org.keycloak.saml.common.exceptions.ProcessingException; import org.keycloak.saml.common.exceptions.fed.IssueInstantMissingException; import org.keycloak.saml.common.util.DocumentUtil; -import org.keycloak.saml.common.util.StaxParserUtil; import org.keycloak.saml.common.util.StaxUtil; import org.keycloak.saml.processing.api.saml.v2.response.SAML2Response; import org.keycloak.saml.processing.api.saml.v2.sig.SAML2Signature; @@ -287,6 +287,22 @@ public class AssertionUtil { return false; } + /** + * Given an assertion element, validate the signature. + */ + public static boolean isSignatureValid(Element assertionElement, KeyLocator keyLocator) { + try { + Document doc = DocumentUtil.createDocument(); + Node n = doc.importNode(assertionElement, true); + doc.appendChild(n); + + return new SAML2Signature().validate(doc, keyLocator); + } catch (Exception e) { + logger.signatureAssertionValidationError(e); + } + return false; + } + /** * Check whether the assertion has expired * @@ -541,7 +557,23 @@ public class AssertionUtil { return responseType.getAssertions().get(0).getAssertion(); } - public static ResponseType decryptAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException { + public static boolean isAssertionEncrypted(ResponseType responseType) throws ProcessingException { + List assertions = responseType.getAssertions(); + + if (assertions.isEmpty()) { + throw new ProcessingException("No assertion from response."); + } + + ResponseType.RTChoiceType rtChoiceType = assertions.get(0); + return rtChoiceType.getEncryptedAssertion() != null; + } + + /** + * This method modifies the given responseType, and replaces the encrypted assertion with a decrypted version. + * + * It returns the assertion element as it was decrypted. This can be used in sginature verification. + */ + public static Element decryptAssertion(ResponseType responseType, PrivateKey privateKey) throws ParsingException, ProcessingException, ConfigurationException { SAML2Response saml2Response = new SAML2Response(); Document doc = saml2Response.convert(responseType); @@ -560,11 +592,11 @@ public class AssertionUtil { SAMLParser parser = new SAMLParser(); JAXPValidationUtil.checkSchemaValidation(decryptedDocumentElement); - AssertionType assertion = (AssertionType) parser.parse(StaxParserUtil.getXMLEventReader(DocumentUtil + AssertionType assertion = (AssertionType) parser.parse(parser.createEventReader(DocumentUtil .getNodeAsStream(decryptedDocumentElement))); responseType.replaceAssertion(oldID, new ResponseType.RTChoiceType(assertion)); - return responseType; + return decryptedDocumentElement; } } \ No newline at end of file diff --git a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java index 38e57cb256..60893ba535 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLEndpoint.java @@ -49,9 +49,12 @@ import org.keycloak.protocol.saml.SamlProtocolUtils; import org.keycloak.saml.SAML2LogoutResponseBuilder; import org.keycloak.saml.SAMLRequestParser; import org.keycloak.saml.common.constants.GeneralConstants; +import org.keycloak.saml.common.constants.JBossSAMLConstants; import org.keycloak.saml.common.constants.JBossSAMLURIConstants; import org.keycloak.saml.common.exceptions.ConfigurationException; import org.keycloak.saml.common.exceptions.ProcessingException; +import org.keycloak.saml.common.util.DocumentUtil; +import org.keycloak.saml.processing.api.saml.v2.sig.SAML2Signature; import org.keycloak.saml.processing.core.saml.v2.common.SAMLDocumentHolder; import org.keycloak.saml.processing.core.saml.v2.constants.X500SAMLProfileConstants; import org.keycloak.saml.processing.core.saml.v2.util.AssertionUtil; @@ -74,6 +77,7 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriBuilder; import javax.ws.rs.core.UriInfo; +import javax.xml.namespace.QName; import java.io.IOException; import java.security.Key; import java.security.cert.X509Certificate; @@ -83,6 +87,8 @@ import java.util.List; import org.keycloak.rotation.HardcodedKeyLocator; import org.keycloak.rotation.KeyLocator; import org.keycloak.saml.processing.core.util.KeycloakKeySamlExtensionGenerator; +import org.w3c.dom.Document; +import org.w3c.dom.Element; import java.util.*; @@ -344,7 +350,38 @@ public class SAMLEndpoint { if (responseType.getAssertions() == null || responseType.getAssertions().isEmpty()) { return callback.error(relayState, Messages.IDENTITY_PROVIDER_UNEXPECTED_ERROR); } - AssertionType assertion = AssertionUtil.getAssertion(responseType, keys.getPrivateKey()); + + boolean assertionIsEncrypted = AssertionUtil.isAssertionEncrypted(responseType); + + if (config.isWantAssertionsEncrypted() && !assertionIsEncrypted) { + logger.error("The assertion is not encrypted, which is required."); + event.event(EventType.IDENTITY_PROVIDER_RESPONSE); + event.error(Errors.INVALID_SAML_RESPONSE); + return ErrorPage.error(session, Messages.INVALID_REQUESTER); + } + + Element assertionElement; + + if (assertionIsEncrypted) { + // This methods writes the parsed and decrypted assertion back on the responseType parameter: + assertionElement = AssertionUtil.decryptAssertion(responseType, keys.getPrivateKey()); + } else { + /* We verify the assertion using original document to handle cases where the IdP + includes whitespace and/or newlines inside tags. */ + assertionElement = DocumentUtil.getElement(holder.getSamlDocument(), new QName(JBossSAMLConstants.ASSERTION.get())); + } + + if (config.isWantAssertionsSigned() && config.isValidateSignature()) { + if (!AssertionUtil.isSignatureValid(assertionElement, getIDPKeyLocator())) { + logger.error("validation failed"); + event.event(EventType.IDENTITY_PROVIDER_RESPONSE); + event.error(Errors.INVALID_SIGNATURE); + return ErrorPage.error(session, Messages.INVALID_REQUESTER); + } + } + + AssertionType assertion = responseType.getAssertions().get(0).getAssertion(); + SubjectType subject = assertion.getSubject(); SubjectType.STSubType subType = subject.getSubType(); NameIDType subjectNameID = (NameIDType) subType.getBaseID(); 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 1f8f7933d8..bdfea6a5ef 100755 --- a/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java +++ b/services/src/main/java/org/keycloak/broker/saml/SAMLIdentityProvider.java @@ -233,6 +233,7 @@ public class SAMLIdentityProvider extends AbstractIdentityProvider {{:: 'want-authn-requests-signed.tooltip' | translate}} +
+ +
+ +
+ {{:: 'want-assertions-signed.tooltip' | translate}} +
+
+ +
+ +
+ {{:: 'want-assertions-encrypted.tooltip' | translate}} +