Merge pull request #1036 from patriot1burke/master

validate destination

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";
 }

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";

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;
+
+    }
+
+
 }

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.event(EventType.LOGOUT);
-                event.error(Errors.INVALID_TOKEN);
+                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) {
+        protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
-            return SAMLRequestParser.parsePostBinding(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) {
+        protected SAMLDocumentHolder extractRequestDocument(String samlRequest) {
-            return SAMLRequestParser.parseRedirectBinding(samlRequest);
+            return SAMLRequestParser.parseRequestRedirectBinding(samlRequest);
+        }
+
+        @Override
+        protected SAMLDocumentHolder extractResponseDocument(String response) {
+            return SAMLRequestParser.parseRequestRedirectBinding(response);
         }
 
         @Override