From 9984caa2fefa552e98c65601257cf3c5ff02338e Mon Sep 17 00:00:00 2001 From: Bill Burke Date: Wed, 11 Mar 2015 19:27:01 -0400 Subject: [PATCH] validate destination --- .../java/org/keycloak/events/Details.java | 1 + .../main/java/org/keycloak/events/Errors.java | 3 + .../protocol/saml/SAMLRequestParser.java | 35 +++++++++- .../keycloak/protocol/saml/SamlService.java | 64 +++++++++++++++---- 4 files changed, 87 insertions(+), 16 deletions(-) diff --git a/events/api/src/main/java/org/keycloak/events/Details.java b/events/api/src/main/java/org/keycloak/events/Details.java index fa1e57a019..d24390c242 100755 --- a/events/api/src/main/java/org/keycloak/events/Details.java +++ b/events/api/src/main/java/org/keycloak/events/Details.java @@ -22,4 +22,5 @@ public interface Details { String VALIDATE_ACCESS_TOKEN = "validate_access_token"; String UPDATED_REFRESH_TOKEN_ID = "updated_refresh_token_id"; String NODE_HOST = "node_host"; + String REASON = "reason"; } diff --git a/events/api/src/main/java/org/keycloak/events/Errors.java b/events/api/src/main/java/org/keycloak/events/Errors.java index 7f2cb41137..f6531ebcaa 100755 --- a/events/api/src/main/java/org/keycloak/events/Errors.java +++ b/events/api/src/main/java/org/keycloak/events/Errors.java @@ -24,6 +24,9 @@ public interface Errors { String INVALID_REDIRECT_URI = "invalid_redirect_uri"; String INVALID_CODE = "invalid_code"; String INVALID_TOKEN = "invalid_token"; + String INVALID_SAML_AUTHN_REQUEST = "invalid_authn_request"; + String INVALID_SAML_LOGOUT_REQUEST = "invalid_logout_request"; + String INVALID_SAML_LOGOUT_RESPONSE = "invalid_logout_response"; String INVALID_SIGNATURE = "invalid_signature"; String INVALID_REGISTRATION = "invalid_registration"; String INVALID_FORM = "invalid_form"; diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SAMLRequestParser.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SAMLRequestParser.java index db198920f0..a66397f692 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SAMLRequestParser.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SAMLRequestParser.java @@ -3,6 +3,7 @@ package org.keycloak.protocol.saml; import org.picketlink.common.PicketLinkLogger; import org.picketlink.common.PicketLinkLoggerFactory; import org.picketlink.identity.federation.api.saml.v2.request.SAML2Request; +import org.picketlink.identity.federation.api.saml.v2.response.SAML2Response; import org.picketlink.identity.federation.core.saml.v2.common.SAMLDocumentHolder; import org.picketlink.identity.federation.web.util.PostBindingUtil; import org.picketlink.identity.federation.web.util.RedirectBindingUtil; @@ -17,7 +18,7 @@ import java.io.InputStream; public class SAMLRequestParser { private static final PicketLinkLogger logger = PicketLinkLoggerFactory.getLogger(); - public static SAMLDocumentHolder parseRedirectBinding(String samlMessage) { + public static SAMLDocumentHolder parseRequestRedirectBinding(String samlMessage) { InputStream is; is = RedirectBindingUtil.base64DeflateDecode(samlMessage); SAML2Request saml2Request = new SAML2Request(); @@ -31,7 +32,7 @@ public class SAMLRequestParser { } - public static SAMLDocumentHolder parsePostBinding(String samlMessage) { + public static SAMLDocumentHolder parseRequestPostBinding(String samlMessage) { InputStream is; byte[] samlBytes = PostBindingUtil.base64Decode(samlMessage); is = new ByteArrayInputStream(samlBytes); @@ -44,4 +45,34 @@ public class SAMLRequestParser { } return null; } + + public static SAMLDocumentHolder parseResponsePostBinding(String samlMessage) { + InputStream is; + byte[] samlBytes = PostBindingUtil.base64Decode(samlMessage); + is = new ByteArrayInputStream(samlBytes); + SAML2Response response = new SAML2Response(); + try { + response.getSAML2ObjectFromStream(is); + return response.getSamlDocumentHolder(); + } catch (Exception e) { + logger.samlBase64DecodingError(e); + } + return null; + } + + public static SAMLDocumentHolder parseResponseRedirectBinding(String samlMessage) { + InputStream is; + is = RedirectBindingUtil.base64DeflateDecode(samlMessage); + SAML2Response response = new SAML2Response(); + try { + response.getSAML2ObjectFromStream(is); + return response.getSamlDocumentHolder(); + } catch (Exception e) { + logger.samlBase64DecodingError(e); + } + return null; + + } + + } diff --git a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java index 5c96cab575..080b7e8444 100755 --- a/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java +++ b/saml/saml-protocol/src/main/java/org/keycloak/protocol/saml/SamlService.java @@ -6,6 +6,7 @@ import org.jboss.resteasy.spi.HttpRequest; import org.jboss.resteasy.spi.HttpResponse; import org.keycloak.ClientConnection; import org.keycloak.VerificationException; +import org.keycloak.events.Details; import org.keycloak.events.Errors; import org.keycloak.events.EventBuilder; import org.keycloak.events.EventType; @@ -32,6 +33,8 @@ import org.picketlink.identity.federation.saml.v2.protocol.AuthnRequestType; import org.picketlink.identity.federation.saml.v2.protocol.LogoutRequestType; import org.picketlink.identity.federation.saml.v2.protocol.NameIDPolicyType; import org.picketlink.identity.federation.saml.v2.protocol.RequestAbstractType; +import org.picketlink.identity.federation.saml.v2.protocol.StatusResponseType; +import org.picketlink.identity.federation.web.util.PostBindingUtil; import org.picketlink.identity.federation.web.util.RedirectBindingUtil; import javax.ws.rs.Consumes; @@ -120,10 +123,20 @@ public class SamlService { } protected Response handleSamlResponse(String samlResponse, String relayState) { + event.event(EventType.LOGOUT); + SAMLDocumentHolder holder = extractResponseDocument(samlResponse); + StatusResponseType statusResponse = (StatusResponseType)holder.getSamlObject(); + // validate destination + if (!uriInfo.getAbsolutePath().toString().equals(statusResponse.getDestination())) { + event.error(Errors.INVALID_SAML_LOGOUT_RESPONSE); + event.detail(Details.REASON, "invalid_destination"); + return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Invalid request."); + } + AuthenticationManager.AuthResult authResult = authManager.authenticateIdentityCookie(session, realm, uriInfo, clientConnection, headers, false); if (authResult == null) { logger.warn("Unknown saml response."); - event.event(EventType.LOGIN); + event.event(EventType.LOGOUT); event.error(Errors.INVALID_TOKEN); return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Invalid Request"); } @@ -132,16 +145,18 @@ public class SamlService { if (userSession.getState() != UserSessionModel.State.LOGGING_OUT) { logger.warn("Unknown saml response."); logger.warn("UserSession is not tagged as logging out."); - event.event(EventType.LOGIN); - event.error(Errors.INVALID_TOKEN); + event.event(EventType.LOGOUT); + event.error(Errors.INVALID_SAML_LOGOUT_RESPONSE); return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Invalid Request"); } logger.debug("logout response"); - return authManager.browserLogout(session, realm, userSession, uriInfo, clientConnection); + Response response = authManager.browserLogout(session, realm, userSession, uriInfo, clientConnection); + event.success(); + return response; } protected Response handleSamlRequest(String samlRequest, String relayState) { - SAMLDocumentHolder documentHolder = extractDocument(samlRequest); + SAMLDocumentHolder documentHolder = extractRequestDocument(samlRequest); if (documentHolder == null) { event.event(EventType.LOGIN); event.error(Errors.INVALID_TOKEN); @@ -206,10 +221,16 @@ public class SamlService { protected abstract void verifySignature(SAMLDocumentHolder documentHolder, ClientModel client) throws VerificationException; - protected abstract SAMLDocumentHolder extractDocument(String samlRequest); + protected abstract SAMLDocumentHolder extractRequestDocument(String samlRequest); + protected abstract SAMLDocumentHolder extractResponseDocument(String response); protected Response loginRequest(String relayState, AuthnRequestType requestAbstractType, ClientModel client) { - + // validate destination + if (!uriInfo.getAbsolutePath().equals(requestAbstractType.getDestination())) { + event.error(Errors.INVALID_SAML_AUTHN_REQUEST); + event.detail(Details.REASON, "invalid_destination"); + return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Invalid request."); + } String bindingType = getBindingType(requestAbstractType); if ("true".equals(client.getAttribute(SamlProtocol.SAML_FORCE_POST_BINDING))) bindingType = SamlProtocol.SAML_POST_BINDING; String redirect = null; @@ -251,7 +272,8 @@ public class SamlService { if(isSupportedNameIdFormat(nameIdFormat)) { clientSession.setNote(GeneralConstants.NAMEID_FORMAT, nameIdFormat); } else { - event.error(Errors.INVALID_TOKEN); + event.error(Errors.INVALID_SAML_AUTHN_REQUEST); + event.detail(Details.REASON, "unsupported_nameid_format"); return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Unsupported NameIDFormat."); } } @@ -312,9 +334,14 @@ public class SamlService { protected abstract String getBindingType(); protected Response logoutRequest(LogoutRequestType logoutRequest, ClientModel client, String relayState) { + // validate destination + if (!uriInfo.getAbsolutePath().equals(logoutRequest.getDestination())) { + event.error(Errors.INVALID_SAML_LOGOUT_REQUEST); + event.detail(Details.REASON, "invalid_destination"); + return Flows.forwardToSecurityFailurePage(session, realm, uriInfo, "Invalid request."); + } + // authenticate identity cookie, but ignore an access token timeout as we're logging out anyways. - - AuthenticationManager.AuthResult authResult = authManager.authenticateIdentityCookie(session, realm, uriInfo, clientConnection, headers, false); if (authResult != null) { String logoutBinding = getBindingType(); @@ -387,8 +414,12 @@ public class SamlService { } @Override - protected SAMLDocumentHolder extractDocument(String samlRequest) { - return SAMLRequestParser.parsePostBinding(samlRequest); + protected SAMLDocumentHolder extractRequestDocument(String samlRequest) { + return SAMLRequestParser.parseRequestPostBinding(samlRequest); + } + @Override + protected SAMLDocumentHolder extractResponseDocument(String response) { + return SAMLRequestParser.parseResponsePostBinding(response); } @Override @@ -455,8 +486,13 @@ public class SamlService { } @Override - protected SAMLDocumentHolder extractDocument(String samlRequest) { - return SAMLRequestParser.parseRedirectBinding(samlRequest); + protected SAMLDocumentHolder extractRequestDocument(String samlRequest) { + return SAMLRequestParser.parseRequestRedirectBinding(samlRequest); + } + + @Override + protected SAMLDocumentHolder extractResponseDocument(String response) { + return SAMLRequestParser.parseRequestRedirectBinding(response); } @Override