[java-opensaml] 02/03: OSJ-271: HTTPRedirectDeflateEncoder includes query parameters in ...
Brent Putman
putmanb at georgetown.edu
Fri Mar 22 19:30:22 EDT 2019
This is an automated email from the git hooks/post-receive script.
putmanb pushed a commit to branch master
in repository java-opensaml.
View the commit online:
http://git.shibboleth.net/view/?p=java-opensaml.git;a=commit;h=4d0b0314f045a1b609bbdf5af13f167d8d3fff58
commit 4d0b0314f045a1b609bbdf5af13f167d8d3fff58
Author: Brent Putman <putmanb at georgetown.edu>
AuthorDate: Wed Mar 20 21:53:28 2019 -0400
OSJ-271: HTTPRedirectDeflateEncoder includes query parameters in ...
HTTPRedirectDeflateEncoder includes query parameters in signature
calculation.
---
.../encoding/impl/HTTPRedirectDeflateEncoder.java | 18 +++
.../impl/HTTPRedirectDeflateEncoderTest.java | 146 +++++++++++++++++++++
2 files changed, 164 insertions(+)
diff --git a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
index 09ebb70..b13a606 100644
--- a/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
+++ b/opensaml-saml-impl/src/main/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoder.java
@@ -21,6 +21,7 @@ import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.UnsupportedEncodingException;
import java.net.MalformedURLException;
+import java.util.ArrayList;
import java.util.Iterator;
import java.util.List;
import java.util.Set;
@@ -54,6 +55,7 @@ import org.opensaml.xmlsec.crypto.XMLSigningUtil;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import com.google.common.collect.Lists;
import com.google.common.collect.Sets;
/**
@@ -171,6 +173,13 @@ public class HTTPRedirectDeflateEncoder extends BaseSAML2MessageEncoder {
final List<Pair<String, String>> queryParams = urlBuilder.getQueryParams();
removeDisallowedQueryParams(queryParams);
+ // This is a copy of any existing allowed params that were preserved. Note that they will not be signed.
+ final List<Pair<String, String>> originalParams = new ArrayList<>(queryParams);
+
+ // We clear here so that existing params will not be signed, but can still use the URLBuilder#buildQueryString()
+ // to build the string that will potentially be signed later. Add originalParms back in later.
+ queryParams.clear();
+
final SAMLObject outboundMessage = messageContext.getMessage();
if (outboundMessage instanceof RequestAbstractType) {
@@ -197,8 +206,17 @@ public class HTTPRedirectDeflateEncoder extends BaseSAML2MessageEncoder {
queryParams.add(new Pair<>("Signature", generateSignature(
signingParameters.getSigningCredential(), sigAlgURI, sigMaterial)));
+
+ // Add original params to the beginning of the list preserving their original order.
+ if (!originalParams.isEmpty()) {
+ for (final Pair<String, String> param : Lists.reverse(originalParams)) {
+ queryParams.add(0, param);
+ }
+ }
+
} else {
log.debug("No signing credential was supplied, skipping HTTP-Redirect DEFLATE signing");
+ queryParams.addAll(originalParams);
}
return urlBuilder.buildURL();
diff --git a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoderTest.java b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoderTest.java
index 99cb0a4..8e852b2 100644
--- a/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoderTest.java
+++ b/opensaml-saml-impl/src/test/java/org/opensaml/saml/saml2/binding/encoding/impl/HTTPRedirectDeflateEncoderTest.java
@@ -383,4 +383,150 @@ public class HTTPRedirectDeflateEncoderTest extends XMLObjectBaseTestCase {
// Note: to test that actual signature is cryptographically correct, really need a known good test vector.
// Need to verify that we're signing over the right data in the right byte[] encoded form.
}
+
+ /**
+ * Tests encoding a SAML message to an servlet response with simple sign,
+ * where the destination URL had existing non-disallowed query parameters.
+ *
+ * @throws Exception
+ */
+ @Test
+ @SuppressWarnings("unchecked")
+ public void OSJ271() throws Exception {
+ // First we generate the signature with a redirect URL that does not have query params.
+
+ SAMLObjectBuilder<StatusCode> statusCodeBuilder = (SAMLObjectBuilder<StatusCode>) builderFactory
+ .getBuilder(StatusCode.DEFAULT_ELEMENT_NAME);
+ StatusCode statusCode = statusCodeBuilder.buildObject();
+ statusCode.setValue(StatusCode.SUCCESS);
+
+ SAMLObjectBuilder<Status> statusBuilder = (SAMLObjectBuilder<Status>) builderFactory
+ .getBuilder(Status.DEFAULT_ELEMENT_NAME);
+ Status responseStatus = statusBuilder.buildObject();
+ responseStatus.setStatusCode(statusCode);
+
+ SAMLObjectBuilder<Response> responseBuilder = (SAMLObjectBuilder<Response>) builderFactory
+ .getBuilder(Response.DEFAULT_ELEMENT_NAME);
+ Response samlMessage = responseBuilder.buildObject();
+ samlMessage.setID("foo");
+ samlMessage.setVersion(SAMLVersion.VERSION_20);
+ samlMessage.setIssueInstant(Instant.ofEpochMilli(0));
+ samlMessage.setStatus(responseStatus);
+
+ SAMLObjectBuilder<Endpoint> endpointBuilder = (SAMLObjectBuilder<Endpoint>) builderFactory
+ .getBuilder(AssertionConsumerService.DEFAULT_ELEMENT_NAME);
+ Endpoint samlEndpoint = endpointBuilder.buildObject();
+ samlEndpoint.setLocation("http://example.org");
+ samlEndpoint.setResponseLocation("http://example.org/response");
+
+ MessageContext<SAMLObject> messageContext = new MessageContext<>();
+ messageContext.setMessage(samlMessage);
+ SAMLBindingSupport.setRelayState(messageContext, "relay");
+ messageContext.getSubcontext(SAMLPeerEntityContext.class, true)
+ .getSubcontext(SAMLEndpointContext.class, true).setEndpoint(samlEndpoint);
+ KeyPair kp = KeySupport.generateKeyPair("RSA", 1024, null);
+
+ SignatureSigningParameters signingParameters = new SignatureSigningParameters();
+ signingParameters.setSigningCredential(CredentialSupport.getSimpleCredential(kp.getPublic(), kp.getPrivate()));
+ signingParameters.setSignatureAlgorithm(SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1);
+ messageContext.getSubcontext(SecurityParametersContext.class, true).setSignatureSigningParameters(signingParameters);
+
+ // NOTE: So that we can get an exact signature comparison without and with query params, we do not invoke
+ // the SAMLOutboundDestinationHandler, which would change the data being signed. Not correct vis-a-vis actual
+ //SAML protocol usage, but for purposes of this test it doesn't matter.
+
+ MockHttpServletResponse response = new MockHttpServletResponse();
+
+ HTTPRedirectDeflateEncoder encoder = new HTTPRedirectDeflateEncoder();
+ encoder.setMessageContext(messageContext);
+ encoder.setHttpServletResponse(response);
+
+ encoder.initialize();
+ encoder.prepareContext();
+ encoder.encode();
+
+ Assert.assertNotNull(response.getRedirectedUrl());
+ URLBuilder urlBuilder = new URLBuilder(response.getRedirectedUrl());
+ Assert.assertEquals(urlBuilder.getScheme(), "http");
+ Assert.assertEquals(urlBuilder.getHost(), "example.org");
+ Assert.assertEquals(urlBuilder.getPath(), "/response");
+
+ Map<String,String> queryParams = URISupport.buildQueryMap(urlBuilder.getQueryParams());
+ Assert.assertTrue(queryParams.containsKey("Signature"));
+ Assert.assertNotNull(queryParams.get("Signature"));
+ Assert.assertTrue(queryParams.containsKey("SigAlg"));
+ Assert.assertEquals(queryParams.get("SigAlg"), SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1);
+ Assert.assertTrue(queryParams.containsKey("RelayState"));
+ Assert.assertEquals(queryParams.get("RelayState"), "relay");
+ Assert.assertTrue(queryParams.containsKey("SAMLResponse"));
+ try (InflaterInputStream inflater =
+ new InflaterInputStream(
+ new ByteArrayInputStream(
+ Base64Support.decode(queryParams.get("SAMLResponse"))), new Inflater(true))) {
+
+ Document outboundResponse = parserPool.parse(inflater);
+ assertXMLEquals(outboundResponse, samlMessage);
+ }
+
+ // Note: to test that actual signature is cryptographically correct, really need a known good test vector.
+ // Need to verify that we're signing over the right data in the right byte[] encoded form.
+
+ String signatureWithoutParams = queryParams.get("Signature");
+
+ // Now repeat with a redirect location that does have query params.
+
+ samlEndpoint.setResponseLocation("http://example.org/response?foo=bar&abc=123");
+
+ messageContext = new MessageContext<>();
+ messageContext.setMessage(samlMessage);
+ SAMLBindingSupport.setRelayState(messageContext, "relay");
+ messageContext.getSubcontext(SAMLPeerEntityContext.class, true)
+ .getSubcontext(SAMLEndpointContext.class, true).setEndpoint(samlEndpoint);
+
+ messageContext.getSubcontext(SecurityParametersContext.class, true).setSignatureSigningParameters(signingParameters);
+
+ response = new MockHttpServletResponse();
+
+ encoder = new HTTPRedirectDeflateEncoder();
+ encoder.setMessageContext(messageContext);
+ encoder.setHttpServletResponse(response);
+
+ encoder.initialize();
+ encoder.prepareContext();
+ encoder.encode();
+
+ Assert.assertNotNull(response.getRedirectedUrl());
+ urlBuilder = new URLBuilder(response.getRedirectedUrl());
+ Assert.assertEquals(urlBuilder.getScheme(), "http");
+ Assert.assertEquals(urlBuilder.getHost(), "example.org");
+ Assert.assertEquals(urlBuilder.getPath(), "/response");
+
+ queryParams = URISupport.buildQueryMap(urlBuilder.getQueryParams());
+ Assert.assertTrue(queryParams.containsKey("foo"));
+ Assert.assertEquals(queryParams.get("foo"), "bar");
+ Assert.assertTrue(queryParams.containsKey("abc"));
+ Assert.assertEquals(queryParams.get("abc"), "123");
+
+ Assert.assertTrue(queryParams.containsKey("Signature"));
+ Assert.assertNotNull(queryParams.get("Signature"));
+ Assert.assertTrue(queryParams.containsKey("SigAlg"));
+ Assert.assertEquals(queryParams.get("SigAlg"), SignatureConstants.ALGO_ID_SIGNATURE_RSA_SHA1);
+ Assert.assertTrue(queryParams.containsKey("RelayState"));
+ Assert.assertEquals(queryParams.get("RelayState"), "relay");
+ Assert.assertTrue(queryParams.containsKey("SAMLResponse"));
+ try (InflaterInputStream inflater =
+ new InflaterInputStream(
+ new ByteArrayInputStream(
+ Base64Support.decode(queryParams.get("SAMLResponse"))), new Inflater(true))) {
+
+ Document outboundResponse = parserPool.parse(inflater);
+ assertXMLEquals(outboundResponse, samlMessage);
+ }
+
+ String signatureWithParams = queryParams.get("Signature");
+
+ // Since the new query params should not be signed, the signature should not change.
+ Assert.assertEquals(signatureWithoutParams, signatureWithParams);
+
+ }
}
\ No newline at end of file
--
To stop receiving notification emails like this one, please contact
the administrator of this repository.
More information about the commits
mailing list