REF-28: Verify the signature on incoming LogoutRequests

oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java

@@ -1,27 +1,130 @@
 package dk.gov.oio.saml.service;
 
+import javax.servlet.http.HttpServletRequest;
+
 import org.joda.time.DateTime;
 import org.opensaml.core.config.InitializationException;
 import org.opensaml.messaging.context.MessageContext;
+import org.opensaml.messaging.handler.MessageHandlerException;
 import org.opensaml.saml.common.SAMLObject;
 import org.opensaml.saml.common.messaging.context.SAMLEndpointContext;
 import org.opensaml.saml.common.messaging.context.SAMLPeerEntityContext;
+import org.opensaml.saml.common.messaging.context.SAMLProtocolContext;
 import org.opensaml.saml.common.xml.SAMLConstants;
+import org.opensaml.saml.saml2.binding.security.impl.SAML2HTTPRedirectDeflateSignatureSecurityHandler;
 import org.opensaml.saml.saml2.core.Issuer;
 import org.opensaml.saml.saml2.core.LogoutRequest;
 import org.opensaml.saml.saml2.core.NameID;
 import org.opensaml.saml.saml2.core.SessionIndex;
+import org.opensaml.saml.saml2.metadata.IDPSSODescriptor;
 import org.opensaml.saml.saml2.metadata.SingleSignOnService;
+import org.opensaml.saml.security.impl.SAMLSignatureProfileValidator;
+import org.opensaml.security.credential.UsageType;
+import org.opensaml.security.credential.impl.StaticCredentialResolver;
+import org.opensaml.security.x509.BasicX509Credential;
 import org.opensaml.xmlsec.SignatureSigningParameters;
+import org.opensaml.xmlsec.SignatureValidationParameters;
 import org.opensaml.xmlsec.context.SecurityParametersContext;
-
+import org.opensaml.xmlsec.keyinfo.impl.StaticKeyInfoCredentialResolver;
+import org.opensaml.xmlsec.signature.Signature;
+import org.opensaml.xmlsec.signature.support.SignatureException;
+import org.opensaml.xmlsec.signature.support.SignatureValidator;
+import org.opensaml.xmlsec.signature.support.impl.ExplicitKeySignatureTrustEngine;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import dk.gov.oio.saml.util.ExternalException;
 import dk.gov.oio.saml.util.InternalException;
 import dk.gov.oio.saml.util.SamlHelper;
+import dk.gov.oio.saml.util.StringUtil;
+import net.shibboleth.utilities.java.support.component.ComponentInitializationException;
 import net.shibboleth.utilities.java.support.security.RandomIdentifierGenerationStrategy;
 
 public class LogoutRequestService {
-    public void validateLogoutRequest() {
-        return;
+    private static final Logger log = LoggerFactory.getLogger(LogoutRequestService.class);
+
+    /**
+     * Verify that an incoming LogoutRequest was issued by the configured IdP and signed with its signing key.
+     *
+     * <p>Accepts a signature on the message itself (POST and SOAP) or on the query string (HTTP-Redirect
+     * binding), and rejects a request carrying neither. Must be called before any session is terminated.</p>
+     */
+    public static void validateLogoutRequest(HttpServletRequest httpServletRequest, MessageContext<SAMLObject> messageContext, LogoutRequest logoutRequest) throws ExternalException, InternalException {
+        validateIssuer(logoutRequest);
+
+        if (logoutRequest.isSigned()) {
+            validateMessageSignature(logoutRequest);
+        }
+        else if (StringUtil.isNotEmpty(httpServletRequest.getParameter("Signature"))) {
+            validateQueryStringSignature(httpServletRequest, messageContext);
+        }
+        else {
+            throw new ExternalException("LogoutRequest was not signed");
+        }
+    }
+
+    private static void validateIssuer(LogoutRequest logoutRequest) throws ExternalException {
+        Issuer issuer = logoutRequest.getIssuer();
+        String idpEntityID = OIOSAML3Service.getConfig().getIdpEntityID();
+
+        if (issuer == null || !idpEntityID.equals(issuer.getValue())) {
+            log.warn("LogoutRequest issuer '{}' does not match the configured IdP '{}'", (issuer != null) ? issuer.getValue() : null, idpEntityID);
+            throw new ExternalException("LogoutRequest was not issued by the configured IdP");
+        }
+    }
+
+    private static void validateMessageSignature(LogoutRequest logoutRequest) throws ExternalException, InternalException {
+        Signature signature = logoutRequest.getSignature();
+        try {
+            // Establishes that the signature is bound to this message, see SAMLSignatureProfileValidator
+            new SAMLSignatureProfileValidator().validate(signature);
+            SignatureValidator.validate(signature, getIdPSigningCredential());
+        }
+        catch (SignatureException e) {
+            throw new ExternalException("LogoutRequest signature could not be validated", e);
+        }
+    }
+
+    private static void validateQueryStringSignature(HttpServletRequest httpServletRequest, MessageContext<SAMLObject> messageContext) throws ExternalException, InternalException {
+        SAMLPeerEntityContext peerEntityContext = messageContext.getSubcontext(SAMLPeerEntityContext.class, true);
+        peerEntityContext.setEntityId(OIOSAML3Service.getConfig().getIdpEntityID());
+        peerEntityContext.setRole(IDPSSODescriptor.DEFAULT_ELEMENT_NAME);
+
+        messageContext.getSubcontext(SAMLProtocolContext.class, true).setProtocol(SAMLConstants.SAML20P_NS);
+
+        SignatureValidationParameters validationParameters = new SignatureValidationParameters();
+        validationParameters.setSignatureTrustEngine(new ExplicitKeySignatureTrustEngine(
+                new StaticCredentialResolver(getIdPSigningCredential()),
+                new StaticKeyInfoCredentialResolver(getIdPSigningCredential())));
+        messageContext.getSubcontext(SecurityParametersContext.class, true).setSignatureValidationParameters(validationParameters);
+
+        SAML2HTTPRedirectDeflateSignatureSecurityHandler signatureHandler = new SAML2HTTPRedirectDeflateSignatureSecurityHandler();
+        try {
+            signatureHandler.setHttpServletRequest(httpServletRequest);
+            signatureHandler.initialize();
+            signatureHandler.invoke(messageContext);
+        }
+        catch (ComponentInitializationException e) {
+            throw new InternalException("Could not initialize SAML2HTTPRedirectDeflateSignatureSecurityHandler", e);
+        }
+        catch (MessageHandlerException e) {
+            throw new ExternalException("LogoutRequest signature could not be validated", e);
+        }
+        finally {
+            if (signatureHandler.isInitialized() && !signatureHandler.isDestroyed()) {
+                signatureHandler.destroy();
+            }
+        }
+
+        // The handler leaves the peer unauthenticated if it did not handle the message, for instance when the
+        // binding does not match, so a completed invoke is not on its own proof that the signature was checked
+        if (!peerEntityContext.isAuthenticated()) {
+            throw new ExternalException("LogoutRequest signature was not verified");
+        }
+    }
+
+    private static BasicX509Credential getIdPSigningCredential() throws ExternalException, InternalException {
+        return new BasicX509Credential(IdPMetadataService.getInstance().getIdPMetadata().getValidX509Certificate(UsageType.SIGNING));
     }
 
     public static MessageContext<SAMLObject> createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination, String index) throws InitializationException, InternalException {

oiosaml/src/main/java/dk/gov/oio/saml/servlet/LogoutRequestHandler.java

@@ -41,6 +41,9 @@ public void handleGet(HttpServletRequest httpServletRequest, HttpServletResponse
         MessageContext<SAMLObject> context = decodeGet(httpServletRequest);
         LogoutRequest logoutRequest = getSamlObject(context, LogoutRequest.class);
 
+        // Nothing on the session may be touched before the request is known to come from the IdP
+        LogoutRequestService.validateLogoutRequest(httpServletRequest, context, logoutRequest);
+
         MessageContext<SAMLObject> outgoingMessage = handleRequest(httpServletRequest, new LogoutRequestWrapper(logoutRequest));
         try {
             sendPost(httpServletResponse, outgoingMessage);
@@ -61,6 +64,9 @@ public void handleSOAP(HttpServletRequest httpServletRequest, HttpServletRespons
         MessageContext<SAMLObject> context = decodeSOAP(httpServletRequest);
         LogoutRequest logoutRequest = getSamlObject(context, LogoutRequest.class);
 
+        // Nothing on the session may be touched before the request is known to come from the IdP
+        LogoutRequestService.validateLogoutRequest(httpServletRequest, context, logoutRequest);
+
         MessageContext<SAMLObject> outgoingMessage = handleRequest(httpServletRequest, new LogoutRequestWrapper(logoutRequest));
         try {
             sendSOAP(httpServletResponse, outgoingMessage);

oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java

@@ -171,8 +171,9 @@ public void testIdPSOAPLogoutRequestWhenLoggedIn() throws Exception {
 
         // Marshall and serialize
         Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage());
+        // Serialized as is: indenting or otherwise reformatting the message would break its signature
         final String soapXml = "<soapenv:Envelope xmlns:soapenv=\"http://schemas.xmlsoap.org/soap/envelope/\"><soapenv:Body>" +
-                StringUtil.elementToString(marshalledMessage) + "</soapenv:Body></soapenv:Envelope>";
+                SerializeSupport.nodeToString(marshalledMessage).replaceFirst("^<\\?xml[^>]*\\?>", "") + "</soapenv:Body></soapenv:Envelope>";
 
         InputStream inputStream = new ByteArrayInputStream(soapXml.getBytes("UTF-8"));
 
@@ -229,6 +230,103 @@ public int read() throws IOException {
         Assertions.assertEquals(TestConstants.IDP_LOGOUT_RESPONSE_URL, logoutResponse.getDestination());
     }
 
+    @DisplayName("Test that an IdP can request a logout with a signature on the query string")
+    @Test
+    public void testIdPLogoutRequestSignedOnQueryString() throws Exception {
+        HttpSession session = Mockito.mock(HttpSession.class);
+        AssertionWrapper assertionWrapper = Mockito.mock(AssertionWrapper.class);
+        SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler();
+
+        // Create LogoutRequest without a signature on the message itself, the HTTP-Redirect binding signs
+        // the query string instead
+        String nameID = "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7";
+        MessageContext<SAMLObject> messageContext = IdpUtil.createMessageWithLogoutRequest(nameID, NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true);
+        String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex();
+        String redirectUrl = IdpUtil.encodeAsRedirectUrl(messageContext);
+
+        Mockito.when(sessionHandler.getAssertion(sessionIndex)).thenReturn(assertionWrapper);
+        Mockito.when(sessionHandler.isAuthenticated(session)).thenReturn(true);
+        Mockito.when(sessionHandler.getAuthnRequest(session)).thenReturn(null);
+
+        // Mock HttpServletRequest
+        HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+        Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL));
+        Mockito.when(request.getSession()).thenReturn(session);
+        Mockito.when(request.getMethod()).thenReturn("GET");
+        IdpUtil.stubRedirectRequest(request, redirectUrl);
+
+        // Mock HttpServletResponse
+        ServletOutputStream outputStreamMock = Mockito.mock(ServletOutputStream.class);
+        HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+        Mockito.when(response.getOutputStream()).thenReturn(outputStreamMock);
+
+        new LogoutRequestHandler().handleGet(request, response);
+
+        Mockito.verify(sessionHandler).logout(session, assertionWrapper);
+        Mockito.verify(session).invalidate();
+    }
+
+    @DisplayName("Test that a LogoutRequest without a signature is rejected")
+    @Test
+    public void testRejectUnsignedLogoutRequest() throws Exception {
+        assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest(
+                "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7",
+                NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, false, true));
+    }
+
+    @DisplayName("Test that a LogoutRequest signed with an unknown key is rejected")
+    @Test
+    public void testRejectLogoutRequestSignedWithUnknownKey() throws Exception {
+        assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest(
+                "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7",
+                NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, false));
+    }
+
+    @DisplayName("Test that a LogoutRequest from another issuer is rejected")
+    @Test
+    public void testRejectLogoutRequestFromUnknownIssuer() throws Exception {
+        assertLogoutRequestRejected(IdpUtil.createMessageWithLogoutRequest(
+                "https://data.gov.dk/model/core/eid/person/uuid/37a5a1aa-67ce-4f70-b7c0-b8e678d585f7",
+                NameID.PERSISTENT, TestConstants.SP_LOGOUT_REQUEST_URL, true, true, "https://not-the-configured-idp"));
+    }
+
+    /**
+     * Send the LogoutRequest to the handler and require that it is refused without any session being touched.
+     */
+    private void assertLogoutRequestRejected(MessageContext<SAMLObject> messageContext) throws Exception {
+        HttpSession session = Mockito.mock(HttpSession.class);
+        AssertionWrapper assertionWrapper = Mockito.mock(AssertionWrapper.class);
+        SessionHandler sessionHandler = OIOSAML3Service.getSessionHandlerFactory().getHandler();
+
+        String sessionIndex = ((LogoutRequest) messageContext.getMessage()).getSessionIndexes().get(0).getSessionIndex();
+        Mockito.when(sessionHandler.getAssertion(sessionIndex)).thenReturn(assertionWrapper);
+        Mockito.when(sessionHandler.isAuthenticated(session)).thenReturn(true);
+        Mockito.when(sessionHandler.getAuthnRequest(session)).thenReturn(null);
+
+        // Marshall, deflate and base64 encode as the HTTP-Redirect binding does
+        Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage());
+        ByteArrayOutputStream bytesOut = new ByteArrayOutputStream();
+        DeflaterOutputStream deflaterStream = new DeflaterOutputStream(bytesOut, new Deflater(8, true));
+        deflaterStream.write(SerializeSupport.nodeToString(marshalledMessage).getBytes("UTF-8"));
+        deflaterStream.finish();
+
+        // Mock HttpServletRequest
+        HttpServletRequest request = Mockito.mock(HttpServletRequest.class);
+        Mockito.when(request.getRequestURL()).thenReturn(new StringBuffer(TestConstants.SP_ASSERTION_CONSUMER_URL));
+        Mockito.when(request.getSession()).thenReturn(session);
+        Mockito.when(request.getMethod()).thenReturn("GET");
+        Mockito.when(request.getParameter("RelayState")).thenReturn(null);
+        Mockito.when(request.getParameter("SAMLRequest")).thenReturn(Base64Support.encode(bytesOut.toByteArray(), Base64Support.UNCHUNKED));
+
+        HttpServletResponse response = Mockito.mock(HttpServletResponse.class);
+
+        Assertions.assertThrows(ExternalException.class, () -> new LogoutRequestHandler().handleGet(request, response));
+
+        // The session handler mock is shared between tests, so verify against this tests own session
+        Mockito.verify(sessionHandler, Mockito.never()).logout(Mockito.eq(session), Mockito.any(AssertionWrapper.class));
+        Mockito.verify(session, Mockito.never()).invalidate();
+    }
+
     @DisplayName("Test that a user that is not logged in can safely attempt a logout")
     @Test
     public void testLogoutRequestWhenNotLoggedIn() throws InternalException, IOException, ExternalException {
@@ -268,8 +366,9 @@ public void testSOAPLogoutRequestWhenNotLoggedIn() throws Exception {
 
         // Marshall and serialize
         Element marshalledMessage = XMLObjectSupport.marshall(messageContext.getMessage());
+        // Serialized as is: indenting or otherwise reformatting the message would break its signature
         final String soapXml = "<soapenv:Envelope xmlns:soapenv=\"http://schemas.xmlsoap.org/soap/envelope/\"><soapenv:Body>" +
-                StringUtil.elementToString(marshalledMessage) + "</soapenv:Body></soapenv:Envelope>";
+                SerializeSupport.nodeToString(marshalledMessage).replaceFirst("^<\\?xml[^>]*\\?>", "") + "</soapenv:Body></soapenv:Envelope>";
 
         InputStream inputStream = new ByteArrayInputStream(soapXml.getBytes("UTF-8"));