Use raw url parameters in redirect signature validation

This patch implements the 'use original URL-encoded values it receives
on the query string' to verify the signature when http redirect is used.

We encountered this problem when integrating with Microsoft ADFS 2.0.
The server uses UPPERCASE in url encodings, this conflicts with the
lowercase url encoding used here (ex %2B vs %2b).

Solution
Added extra parameter to the classes where the raw parameters were needed.
Basically, just a map containing the raw URL parameters. Keyed on the name
and with key=value as the value. This is to preserve all variations
like foo=bar, blank= and empty (ex foo=bar&blank=&empty).

Existing HttpRequest abstraction not modified, as the use of
raw url parameters is unusual in most situations.

Note: The SigAlg parameter is now mandatory, test modified.
There was a default SigAlg added to the url in the verification step
when missing. This conflicted with the raw url parameters solution.

From saml-bindings-2.0 (http://docs.oasis-open.org/security/saml/v2.0/)

3.4.4.1 DEFLATE Encoding
Identification: urn:oasis:names:tc:SAML:2.0:bindings:URL-Encoding:DEFLATE

To construct the signature, a string consisting of the concatenation
of the RelayState (if present), SigAlg, and SAMLRequest (or SAMLResponse)
query string parameters (each one URLencoded) is constructed in one of the
following ways (ordered as below):
SAMLRequest=value&RelayState=value&SigAlg=value
SAMLResponse=value&RelayState=value&SigAlg=value

Further, note that URL-encoding is not canonical; that is, there are
multiple legal encodings for a given value. The relying party MUST
therefore perform the verification step using the original URL-encoded
values it received on the query string. It is not sufficient to
re-encode the parameters after they have been processed by software
because the resulting encoding may not match the signer's encoding

Finally, note that if there is no RelayState value, the entire parameter
should be omitted from the signature computation (and not included as
an empty parameter name).

Author: merit\rembjo0

core/src/main/java/com/onelogin/saml2/logout/LogoutRequest.java

@@ -12,6 +12,7 @@
 
 import javax.xml.xpath.XPathExpressionException;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.text.StrSubstitutor;
 import org.joda.time.DateTime;
 import org.slf4j.Logger;
@@ -60,6 +61,11 @@ public class LogoutRequest {
      */
 	private final HttpRequest request;
 
+	/**
+	 * Map with raw request parameters as received in the queryString. Required for accurate signature verification.
+	 */
+	private Map<String, String> rawRequestParams;
+	
 	/**
      * NameID.
      */	
@@ -92,16 +98,19 @@ public class LogoutRequest {
 	 *              OneLogin_Saml2_Settings
 	 * @param request
      *              the HttpRequest object to be processed (Contains GET and POST parameters, request URL, ...).
+     * @param rawRequestParams
+     *              map with 'raw' url request parameters for signature validation (keyed on name, value is key & value)          
 	 * @param nameId
 	 *              The NameID that will be set in the LogoutRequest.
 	 * @param sessionIndex
 	 *              The SessionIndex (taken from the SAML Response in the SSO process).
 	 * @throws XMLEntityException 
 	 *
 	 */
-	public LogoutRequest(Saml2Settings settings, HttpRequest request, String nameId, String sessionIndex) throws XMLEntityException {
+	public LogoutRequest(Saml2Settings settings, HttpRequest request, Map<String, String> rawRequestParams, String nameId, String sessionIndex) throws XMLEntityException {
 		this.settings = settings;
 		this.request = request;
+		this.rawRequestParams = rawRequestParams;
 
 		String samlLogoutRequest = null;
 
@@ -133,7 +142,7 @@ public LogoutRequest(Saml2Settings settings, HttpRequest request, String nameId,
 	 * @throws XMLEntityException 
 	 */
 	public LogoutRequest(Saml2Settings settings) throws XMLEntityException {
-		this(settings, null, null, null);
+		this(settings, null, null, null, null);
 	}
 
 	/**
@@ -143,11 +152,13 @@ public LogoutRequest(Saml2Settings settings) throws XMLEntityException {
 	 *            OneLogin_Saml2_Settings
 	 * @param request
      *              the HttpRequest object to be processed (Contains GET and POST parameters, request URL, ...).
+     * @param rawRequestParams
+     *              map with 'raw' url request parameters for signature validation (keyed on name, value is key & value)
      *
 	 * @throws XMLEntityException 
 	 */
-	public LogoutRequest(Saml2Settings settings, HttpRequest request) throws XMLEntityException {
-		this(settings, request, null, null);
+	public LogoutRequest(Saml2Settings settings, HttpRequest request, Map<String, String> rawRequestParams) throws XMLEntityException {
+		this(settings, request, rawRequestParams, null, null);
 	}
 
 	/**
@@ -334,15 +345,19 @@ public Boolean isValid() throws Exception {
 				if (signAlg == null || signAlg.isEmpty()) {
 					signAlg = Constants.RSA_SHA1;
 				}
-				String relayState = request.getParameter("RelayState");
-
-				String signedQuery = "SAMLRequest=" + Util.urlEncoder(request.getParameter("SAMLRequest"));
-
-				if (relayState != null && !relayState.isEmpty()) {
-					signedQuery += "&RelayState=" + Util.urlEncoder(relayState);
+				
+				String rawSamlRequest = rawRequestParams.get("SAMLRequest");
+				String rawRelayState = rawRequestParams.get("RelayState");
+				String rawSigAlg = rawRequestParams.get("SigAlg");
+				
+				String signedQuery = "";
+				signedQuery += rawSamlRequest;
+				
+				if (StringUtils.isNotBlank(rawRelayState)) {
+					signedQuery += "&" + rawRelayState;
 				}
 
-				signedQuery += "&SigAlg=" + Util.urlEncoder(signAlg);
+				signedQuery += "&" + rawSigAlg;
 
 				if (!Util.validateBinarySignature(signedQuery, Util.base64decoder(signature), cert, signAlg)) {
 					throw new ValidationError("Signature validation failed. Logout Request rejected", ValidationError.INVALID_SIGNATURE);

core/src/main/java/com/onelogin/saml2/logout/LogoutResponse.java

@@ -7,9 +7,11 @@
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
+import java.util.regex.Pattern;
 
 import javax.xml.xpath.XPathExpressionException;
 
+import org.apache.commons.lang3.StringUtils;
 import org.apache.commons.lang3.text.StrSubstitutor;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -61,6 +63,11 @@ public class LogoutResponse {
      */
 	private final HttpRequest request;
 
+	/**
+	 * Map with raw request parameters as received in the queryString. Required for accurate signature verification.
+	 */
+	private Map<String, String> rawRequestParams;
+	
 	/**
 	 * URL of the current host + current view
 	 */
@@ -88,11 +95,14 @@ public class LogoutResponse {
 	 *              OneLogin_Saml2_Settings
 	 * @param request
      *              the HttpRequest object to be processed (Contains GET and POST parameters, request URL, ...).
+     * @param rawRequestParams
+     *              map with 'raw' url request parameters for signature validation (keyed on name, value is key & value)
      *
 	 */
-	public LogoutResponse(Saml2Settings settings, HttpRequest request) {
+	public LogoutResponse(Saml2Settings settings, HttpRequest request, Map<String, String> rawRequestParams) {
 		this.settings = settings;
 		this.request = request;
+		this.rawRequestParams = rawRequestParams;
 
 		String samlLogoutResponse = null;
 		if (request != null) {
@@ -214,20 +224,25 @@ public Boolean isValid(String requestId) {
 				if (cert == null) {
 					throw new SettingsException("In order to validate the sign on the Logout Response, the x509cert of the IdP is required", SettingsException.CERT_NOT_FOUND);
 				}
-
+				
+				
 				String signAlg = request.getParameter("SigAlg");
 				if (signAlg == null || signAlg.isEmpty()) {
 					signAlg = Constants.RSA_SHA1;
 				}
 
-				String signedQuery = "SAMLResponse=" + Util.urlEncoder(request.getParameter("SAMLResponse"));
-
-				String relayState = request.getParameter("RelayState");
-				if (relayState != null && !relayState.isEmpty()) {
-					signedQuery += "&RelayState=" + Util.urlEncoder(relayState);
+				String rawSamlResponse = rawRequestParams.get("SAMLResponse");
+				String rawRelayState = rawRequestParams.get("RelayState");
+				String rawSigAlg = rawRequestParams.get("SigAlg");
+				
+				String signedQuery = "";
+				signedQuery += rawSamlResponse;
+				
+				if (StringUtils.isNotBlank(rawRelayState)) {
+					signedQuery += "&" + rawRelayState;
 				}
 
-				signedQuery += "&SigAlg=" + Util.urlEncoder(signAlg);
+				signedQuery += "&" + rawSigAlg;
 
 				if (!Util.validateBinarySignature(signedQuery, Util.base64decoder(signature), cert, signAlg)) {
 					throw new ValidationError("Signature validation failed. Logout Response rejected", ValidationError.INVALID_SIGNATURE);

core/src/main/java/com/onelogin/saml2/util/QueryStringSplitter.java

@@ -0,0 +1,65 @@
+package com.onelogin.saml2.util;
+
+import java.util.Collections;
+import java.util.LinkedHashMap;
+import java.util.Map;
+import java.util.regex.Matcher;
+import java.util.regex.Pattern;
+
+import org.apache.commons.lang3.StringUtils;
+
+/**
+ * Utility for splitting raw url a raw url query string
+ * (foo=bar&hello=world&data=Hi%20there) into
+ *
+ */
+public final class QueryStringSplitter {
+
+	// Group 1 contains key & value, group 2 contains key only
+	private static final Pattern RE_QUERY = Pattern.compile("(([^=&]+)(?:=[^&]*)?)(?:&|$)");
+
+	private QueryStringSplitter() {
+		// static methods only, no instance
+	}
+
+	/**
+	 * Splits a raw url query string into a map where:
+	 * <ul>
+	 * <li>the key is the parameter name
+	 * <li>the value is the key & value
+	 * </ul>
+	 * <b>Note:</b> For keys with multiple values, the first value is used.
+	 * <p>
+	 * Given the string "foo=bar&hello=world&empty&blank=&data=Hi%20there" it
+	 * will return the map:
+	 * <ul>
+	 * <li>foo: foo=bar
+	 * <li>hello: hello=world
+	 * <li>empty: empty
+	 * <li>blank: blank=
+	 * <li>data: data=Hi%20there
+	 * </ul>
+	 * 
+	 * @param queryString
+	 *            raw url query string
+	 * 
+	 * @returns Unmodifiable map, empty if input is empty or null
+	 */
+	public static Map<String, String> split(final String queryString) {
+		if (StringUtils.isBlank(queryString)) {
+			return Collections.emptyMap();
+		}
+
+		Map<String, String> results = new LinkedHashMap<>();
+		Matcher matcher = RE_QUERY.matcher(queryString);
+		while (matcher.find()) {
+			String key = matcher.group(2);
+			if (!results.containsKey(key)) {
+				results.put(key, matcher.group(1));
+			}
+		}
+
+		return Collections.unmodifiableMap(results);
+	}
+
+}