diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java b/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java index 4634ab9..66d4d7f 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/service/LogoutRequestService.java +++ b/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. + * + *

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.

+ */ + public static void validateLogoutRequest(HttpServletRequest httpServletRequest, MessageContext 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 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 createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination, String index) throws InitializationException, InternalException { diff --git a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/LogoutRequestHandler.java b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/LogoutRequestHandler.java index 1bc2bbb..a7ca2bb 100644 --- a/oiosaml/src/main/java/dk/gov/oio/saml/servlet/LogoutRequestHandler.java +++ b/oiosaml/src/main/java/dk/gov/oio/saml/servlet/LogoutRequestHandler.java @@ -41,6 +41,9 @@ public void handleGet(HttpServletRequest httpServletRequest, HttpServletResponse MessageContext 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 outgoingMessage = handleRequest(httpServletRequest, new LogoutRequestWrapper(logoutRequest)); try { sendPost(httpServletResponse, outgoingMessage); @@ -61,6 +64,9 @@ public void handleSOAP(HttpServletRequest httpServletRequest, HttpServletRespons MessageContext 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 outgoingMessage = handleRequest(httpServletRequest, new LogoutRequestWrapper(logoutRequest)); try { sendSOAP(httpServletResponse, outgoingMessage); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java b/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java index e52be4e..3399499 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/servlet/LogoutRequestHandlerTest.java +++ b/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 = "" + - StringUtil.elementToString(marshalledMessage) + ""; + SerializeSupport.nodeToString(marshalledMessage).replaceFirst("^<\\?xml[^>]*\\?>", "") + ""; 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 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 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 = "" + - StringUtil.elementToString(marshalledMessage) + ""; + SerializeSupport.nodeToString(marshalledMessage).replaceFirst("^<\\?xml[^>]*\\?>", "") + ""; InputStream inputStream = new ByteArrayInputStream(soapXml.getBytes("UTF-8")); diff --git a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java index 427dcb7..c11454d 100644 --- a/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java +++ b/oiosaml/src/test/java/dk/gov/oio/saml/util/IdpUtil.java @@ -1,6 +1,10 @@ package dk.gov.oio.saml.util; import java.io.FileInputStream; +import java.net.URL; +import java.net.URLDecoder; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; import java.security.KeyStore; import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; @@ -18,6 +22,10 @@ import org.opensaml.core.criterion.EntityIdCriterion; import org.opensaml.core.xml.XMLObjectBuilderFactory; import org.opensaml.core.xml.config.XMLObjectProviderRegistrySupport; +import org.opensaml.core.xml.util.XMLObjectSupport; +import org.opensaml.saml.saml2.binding.encoding.impl.HTTPRedirectDeflateEncoder; +import org.mockito.ArgumentCaptor; +import org.mockito.Mockito; import org.opensaml.core.xml.schema.XSAny; import org.opensaml.core.xml.schema.impl.XSAnyBuilder; import org.opensaml.messaging.context.MessageContext; @@ -177,11 +185,28 @@ private static LogoutResponse createLogoutResponse(String destination, LogoutReq } public static MessageContext createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination) throws Exception { + return createMessageWithLogoutRequest(nameID, nameIDFormat, destination, true, true); + } + + /** + * Create a LogoutRequest as the IdP would send it. + * + * @param signMessage sign the LogoutRequest itself, as done for the POST and SOAP bindings + * @param validSignature sign with the IdP key from metadata, or with an unrelated key + */ + public static MessageContext createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination, boolean signMessage, boolean validSignature) throws Exception { + return createMessageWithLogoutRequest(nameID, nameIDFormat, destination, signMessage, validSignature, TestConstants.IDP_ENTITY_ID); + } + + public static MessageContext createMessageWithLogoutRequest(String nameID, String nameIDFormat, String destination, boolean signMessage, boolean validSignature, String issuerEntityID) throws Exception { // Create message context MessageContext messageContext = new MessageContext<>(); // Create AuthnRequest - LogoutRequest outgoingLogoutRequest = createLogoutRequest(nameID, nameIDFormat, destination); + LogoutRequest outgoingLogoutRequest = createLogoutRequest(nameID, nameIDFormat, destination, issuerEntityID); + if (signMessage) { + signLogoutRequest(outgoingLogoutRequest, validSignature); + } messageContext.setMessage(outgoingLogoutRequest); // Destination @@ -203,7 +228,45 @@ public static MessageContext createMessageWithLogoutRequest(String n return messageContext; } + /** + * Encode a message for the HTTP-Redirect binding, signing the query string when the message context + * carries signing parameters, and return the redirect URL the IdP would send the user agent to. + */ + public static String encodeAsRedirectUrl(MessageContext messageContext) throws Exception { + HttpServletResponse httpServletResponse = Mockito.mock(HttpServletResponse.class); + + HTTPRedirectDeflateEncoder encoder = new HTTPRedirectDeflateEncoder(); + encoder.setMessageContext(messageContext); + encoder.setHttpServletResponse(httpServletResponse); + encoder.initialize(); + encoder.encode(); + + ArgumentCaptor redirectUrl = ArgumentCaptor.forClass(String.class); + Mockito.verify(httpServletResponse).sendRedirect(redirectUrl.capture()); + + return redirectUrl.getValue(); + } + + /** + * Stub query string and parameters on a mocked request, as a container would present the redirect URL. + */ + public static void stubRedirectRequest(HttpServletRequest httpServletRequest, String redirectUrl) throws Exception { + String queryString = new URL(redirectUrl).getQuery(); + Mockito.when(httpServletRequest.getQueryString()).thenReturn(queryString); + + for (String parameter : queryString.split("&")) { + int separator = parameter.indexOf('='); + String name = parameter.substring(0, separator); + String value = URLDecoder.decode(parameter.substring(separator + 1), "UTF-8"); + Mockito.when(httpServletRequest.getParameter(name)).thenReturn(value); + } + } + public static LogoutRequest createLogoutRequest(String nameID, String nameIDFormat, String destination) throws InitializationException { + return createLogoutRequest(nameID, nameIDFormat, destination, OIOSAML3Service.getConfig().getSpEntityID()); + } + + public static LogoutRequest createLogoutRequest(String nameID, String nameIDFormat, String destination, String issuerEntityID) throws InitializationException { LogoutRequest outgoingLR = SamlHelper.build(LogoutRequest.class); // Set ID @@ -218,7 +281,7 @@ public static LogoutRequest createLogoutRequest(String nameID, String nameIDForm Issuer issuer = SamlHelper.build(Issuer.class); outgoingLR.setIssuer(issuer); - issuer.setValue(OIOSAML3Service.getConfig().getSpEntityID()); + issuer.setValue(issuerEntityID); // NameID NameID nameIDObj = SamlHelper.build(NameID.class); @@ -263,6 +326,22 @@ private static X509Certificate getSPCertificate(boolean validCert) throws Except return (X509Certificate) instance.generateCertificate(fis); } + public static void signLogoutRequest(LogoutRequest logoutRequest, boolean validSignature) throws Exception { + Signature signature = buildSAMLObject(Signature.class); + + BasicX509Credential x509Credential = getX509Credential(validSignature); + + signature.setSigningCredential(x509Credential); + signature.setCanonicalizationAlgorithm(CanonicalizationMethod.EXCLUSIVE); + signature.setSignatureAlgorithm(new SignatureRSASHA256().getURI()); + signature.setKeyInfo(getPublicKeyInfo(x509Credential)); + + logoutRequest.setSignature(signature); + + XMLObjectSupport.marshall(logoutRequest); + Signer.signObject(signature); + } + private static void SignAssertion(Assertion assertion, boolean validSignature) throws Exception { Signature signature = buildSAMLObject(Signature.class);