diff --git a/uportal-war/src/main/java/org/jasig/portal/security/firewall/README.md b/uportal-war/src/main/java/org/jasig/portal/security/firewall/README.md new file mode 100644 index 00000000000..27d2845bd25 --- /dev/null +++ b/uportal-war/src/main/java/org/jasig/portal/security/firewall/README.md @@ -0,0 +1,177 @@ +RequestParameterPolicyEnforcementFilter +======================================= + +The `RequestParameterPolicyEnforcementFilter` is a blunt generic Java Servlet Filter suitable for + + * in general, blocking malicious requests involving unexpectedly duplicate request parameters or request parameters with unexpected and poorly handled characters in their values. + * in specific, blocking the malicious requests involved in `CVE-2014-4172`, to which certain Java CAS Client versions +are vulnerable, including Java CAS Client versions that have been included in uPortal releases. + +It is a fork of that offered to the `cas-server-security-filter` project at https://github.com/Jasig/cas-server-security-filter/pull/6 . + +It is optionally configurable as to what parameters it checks, what characters it forbids in those checked parameters, +and whether it allows those checked parameters to be multi-valued. + +It has no dependencies. + +Configuration warning : Filter order matters +-------------------------------------------- + +The `RequestParameterPolicyEnforcementFilter` MUST be mapped BEFORE the `CAS Validate Filter` so that it is filtering + the inputs to the Java CAS Client Filter. So, take great care with your `web.xml` `filter-mapping` order. + +Configuration options +--------------------- + +This Filter is optionally configured via Filter `init-param` in `web.xml`. + +In general the Filter is very persnickety about init-params, such that if you give it a configuration that the Filter + is not totally sure it understands, it will fail Filter initialization. + +### parametersToCheck init-param + + +The _optional_ init-param `parametersToCheck` is a whitespace-delimited set of the names of request parameters the +Filter will check. + +The special value `*` instructs the Filter to check all parameters, and is the default behavior. + +### charactersToForbid init-param + +The _optional_ init-param `charactersToForbid` is a whitespace-delimited set of the individual characters the Filter +will forbid. + +The special magic value of exactly `none` instructs the Filter not to forbid any characters. (This is useful for +using the Filter to block multi-valued-ness of parameters without sniffing on any characters.) + +If not present, the Filter will default to forbidding the characters `? # & %` . + +### allowMultiValuedParameters init-param + +The _optional_ init-param `allowMultiValuedParameters` indicates whether the Filter should allow multi-valued +parameters. + +If present the value of this parameter must be exactly `true` or `false`, with `false` as the default. + + + +Configuration Examples +---------------------- + +### The recommended and included configuration + +This filter is wired up in the included `web.xml` to block illicit characters in values of and multiple values of the + request parameters involved in using the Java CAS Client, since these characters should not be in CAS ticket + identifiers and there is no legitimate use case for submission of CAS protocol parameters as multi-valued. + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + parametersToCheck + ticket SAMLArt pgtIou pgtId + + + + ... + + + requestParameterFilter + /Login + /CasProxyServlet + + + +### The no-configuration behavior + +In principle, all `init-param`s configuring the RequestParameterPolicyEnforcementFilter are optional and it can operate with default configuration. + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + ... + + requestParameterFilter + /* + + +In this configuration, the Filter will scrutinize all request parameters, requiring that they not be multi-valued, and requiring that they not contain any of `% ? # &`. + +This configuration is not appropriate in uPortal, where multi-valued parameters and parameters containing interesting characters are valid for some request parameters in some circumstances. That is, the default configuration is over-broad in what it blocks. + +### Allow multi-valued parameters + +Multi-valued parameters are essential for supporting forms with multi-choice selectors where the form submission is +legitimately represented as repeated parameter names with different values. + +So, if you want to scrutinize the characters in all parameters, you might have to relax the requirement that those +parameters not be multi-valued. + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + allowMultiValuedParameters + true + + + ... + + requestParameterFilter + /* + + +This configuration isn't necessary in the included `web.xml` configuration because the CAS protocol parameters do not need to be multi-valued. + + +### Restrictions suitable for fronting a CAS Server + +Likewise, you could use this Filter in front of a CAS Server to prevent unexpected multi-valued submissions of CAS +protocol parameters. + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + parametersToCheck + ticket SAMLArt service renew gateway warn logoutUrl pgtUrl + + + charactersToForbid + none + + + ... + + requestParameterFilter + /* + + +This approach has the advantage of only blocking specific CAS protocol parameters, +so that if you were to map the Filter in front of say the services management UI you can block unexpectedly +multi-valued CAS protocol parameters without blocking submission of the services management edit screen where +multiple user attributes are selected for release to a service (a legitimate case of a multi-valued attribute). + +### An entirely novel configuration + +So, a neat thing about this Filter is that it has nothing to do with CAS and it has no dependencies at all other than the Servlet API, so on that fateful day when you discover that some Java web application has some problem involving illicit submissions of the semicolon character in a request parameter named `query`, you can plop this Filter in front of it and get back to safety. Doing so will almost certainly just work, since this Filter has no external dependencies whatsoever except on the Servlet API that had to be present for that Web Application to be a Java web app anyway. + + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + parametersToCheck + query + + + charactersToForbid + ; + + + ... + + requestParameterFilter + /* + diff --git a/uportal-war/src/main/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilter.java b/uportal-war/src/main/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilter.java new file mode 100644 index 00000000000..5b89d92f3c3 --- /dev/null +++ b/uportal-war/src/main/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilter.java @@ -0,0 +1,474 @@ +/* + * Licensed to Jasig under one or more contributor license + * agreements. See the NOTICE file distributed with this work + * for additional information regarding copyright ownership. + * Jasig licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a + * copy of the License at the following location: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.jasig.portal.security.firewall; + +import javax.servlet.Filter; +import javax.servlet.FilterChain; +import javax.servlet.FilterConfig; +import javax.servlet.ServletException; +import javax.servlet.ServletRequest; +import javax.servlet.ServletResponse; +import javax.servlet.http.HttpServletRequest; +import java.io.IOException; +import java.util.Enumeration; +import java.util.HashSet; +import java.util.Map; +import java.util.Set; + +/** + * This is a Java Servlet Filter that examines specified Request Parameters as to whether they contain specified + * characters and as to whether they are multivalued throws an Exception if they do not meet configured rules. + * + * This is a fork of the Filter offered at https://github.com/Jasig/cas-server-security-filter/pull/6 . + * + * This is forked into the uPortal project source code directly so that it can be used to patch the uPortal usage of + * the Java CAS Client against CVE-2014-4172 without requiring upgrade of the Java CAS Client library itself. + * + * Configuration: + * + * The filter defaults to checking all request parameters for the hash, percent, question mark, + * and ampersand characters, and enforcing no-multi-valued-ness. + * + * You can turn off multi-value checking by setting the init-param "allowMultiValuedParameters" to "true". Setting it + * to "false" is a no-op retaining the default configuration. Setting this parameter to any other value fails filter + * initialization. + * + * You can change the set of request parameters being examined by setting the init-param "parametersToCheck" to a + * whitespace delimited list of parameters to check. Setting it to the special value "*" retains the default + * behavior of checking all. Setting it to a blank value fails filter initialization. Setting it to a String + * containing the asterisk token and any additional token fails filter initialization. + * + * You can change the set of characters looked for by setting the init-param "charactersToForbid" to a whitespace + * delimited list of characters to forbid. Setting it to the special value "none" disables the illicit character + * blocking feature of this Filter (for the case where you only want to use the mutli-valued-ness blocking). + * Setting it to a blank value fails filter initialization. + * Setting it to a value that fails to parse perfectly + * (e.g., a value with multi-character Strings between the whitespace delimiters) + * fails filter initialization. The default set of characters disallowed is percent, hash, question mark, + * and ampersand. + * + * Setting any other init parameter other than these recognized by this Filter will fail Filter initialization. This + * is to protect the adopter from typos or misunderstandings in web.xml configuration such that an intended + * configuration might not have taken effect, since that might have security implications. + * + * Setting the Filter to both allow multi-valued parameters and to disallow no characters would make the Filter a + * no-op, and so fails Filter initialization since you probably meant the Filter to be doing something. + * + * The intent of this filter is rough, brute force blocking of unexpected characters in specific CAS protocol related + * request parameters. This is one option as a workaround for patching in place certain Java CAS Client versions that + * may be vulnerable to certain attacks involving crafted request parameter values that may be mishandled. This is + * also suitable for patching certain CAS Server versions to make more of an effort to detect and block out-of-spec + * CAS protocol requests. Aside from the intent to be useful for those cases, there is nothing CAS-specific about + * this Filter itself. This is a generic Filter for doing some pretty basic generic sanity checking on request + * parameters. It might come in handy the next time this kind of issue arises. + * + * This Filter is written to have no external .jar dependencies aside from the Servlet API necessary to be a Filter. + * + * This class is declared final because it is not designed for extension. + * + * @since uPortal 4.0.15 + */ +public final class RequestParameterPolicyEnforcementFilter implements Filter { + + + /** + * The set of Characters blocked by default on checked parameters. + * Expressed as a whitespace delimited set of characters. + */ + public static final String DEFAULT_CHARACTERS_BLOCKED = "? & # %"; + + /** + * The name of the optional Filter init-param specifying what request parameters ought to be checked. + * The value is a whitespace delimited set of parameters. + * The exact value '*' has the special meaning of matching all parameters, and is the default behavior. + */ + public static final String PARAMETERS_TO_CHECK = "parametersToCheck"; + + /** + * The name of the optional Filter init-param specifying what characters are forbidden in the checked request + * parameters. The value is a whitespace delimited set of such characters. + */ + public static final String CHARACTERS_TO_FORBID = "charactersToForbid"; + + /** + * The name of the optional Filter init-param specifying whether the checked request parameters are allowed + * to have multiple values. Allowable values for this init parameter are `true` and `false`. + */ + public static final String ALLOW_MULTI_VALUED_PARAMETERS = "allowMultiValuedParameters"; + + /** + * Set of parameter names to check. + * Empty set represents special behavior of checking all parameters. + */ + private Set parametersToCheck; + + /** + * Set of characters to forbid in the checked request parameters. + * Empty set represents not forbidding any characters. + */ + private Set charactersToForbid; + + /** + * Should checked parameters be permitted to have multiple values. + */ + private boolean allowMultiValueParameters = false; + + + /* ========================================================================================================== */ + /* Filter methods */ + + @Override + public void init(final FilterConfig filterConfig) throws ServletException { + + // verify there are no init parameters configured that are not recognized + // since an unrecognized init param might be the adopter trying to configure this filter in an important way + // and accidentally ignoring that intent might have security implications. + final Enumeration initParamNames = filterConfig.getInitParameterNames(); + throwIfUnrecognizedParamName(initParamNames); + + final String initParamAllowMultiValuedParameters = filterConfig.getInitParameter(ALLOW_MULTI_VALUED_PARAMETERS); + final String initParamParametersToCheck = filterConfig.getInitParameter(PARAMETERS_TO_CHECK); + final String initParamCharactersToForbid = filterConfig.getInitParameter(CHARACTERS_TO_FORBID); + + try { + this.allowMultiValueParameters = parseStringToBooleanDefaultingToFalse(initParamAllowMultiValuedParameters); + } catch (final Exception e) { + throw new ServletException("Error parsing request parameter [" + ALLOW_MULTI_VALUED_PARAMETERS + + "] with value [" + initParamAllowMultiValuedParameters + "]", e); + } + + try { + this.parametersToCheck = parseParametersToCheck(initParamParametersToCheck); + } catch (final Exception e) { + throw new ServletException("Error parsing request parameter " + PARAMETERS_TO_CHECK + " with value [" + + initParamParametersToCheck + "]", e); + } + + try { + this.charactersToForbid = parseCharactersToForbid(initParamCharactersToForbid); + } catch (final Exception e) { + throw new ServletException("Error parsing request parameter " + CHARACTERS_TO_FORBID + " with value [" + + initParamCharactersToForbid + "]", e); + } + + if (this.allowMultiValueParameters && this.charactersToForbid.isEmpty()) { + throw new ServletException("Configuration to allow multi-value parameters and forbid no characters makes " + + getClass().getSimpleName() + " a no-op, which is probably not what you want, " + + "so failing Filter init."); + } + + } + + + @Override + public void doFilter(final ServletRequest request, final ServletResponse response, final FilterChain chain) + throws IOException, ServletException { + + try { + if (request instanceof HttpServletRequest) { + final HttpServletRequest httpServletRequest = (HttpServletRequest) request; + + // immutable map from String param name --> String[] parameter values + final Map parameterMap = httpServletRequest.getParameterMap(); + + // which parameters *on this request* ought to be checked. + final Set parametersToCheckHere; + + if (this.parametersToCheck.isEmpty()) { + // the special meaning of empty is to check *all* the parameters, so + parametersToCheckHere = parameterMap.keySet(); + } else { + parametersToCheckHere = this.parametersToCheck; + } + + if (! this.allowMultiValueParameters) { + requireNotMultiValued(parametersToCheckHere, parameterMap); + } + + enforceParameterContentCharacterRestrictions(parametersToCheckHere, + this.charactersToForbid, parameterMap); + + + } + } catch (final Exception e ) { + // translate to a ServletException to meet the typed expectations of the Filter API. + throw new ServletException(getClass().getSimpleName() + " is blocking this request. Examine the cause in" + + " this stack trace to understand why.", e); + } + + chain.doFilter(request, response); + } + + @Override + public void destroy() { + // do nothing + } + + /* ========================================================================================================== */ + /* Init parameter parsing */ + + /** + * Examines the Filter init parameter names and throws ServletException if they contain an unrecognized + * init parameter name. + * + * This is a stateless static method. + * + * This method is an implementation detail and is not exposed API. + * This method is only non-private to allow JUnit testing. + * + * @param initParamNames init param names, in practice as read from the FilterConfig. + * @throws ServletException if unrecognized parameter name is present + */ + static void throwIfUnrecognizedParamName(Enumeration initParamNames) throws ServletException { + final Set recognizedParameterNames = new HashSet(); + recognizedParameterNames.add(ALLOW_MULTI_VALUED_PARAMETERS); + recognizedParameterNames.add(PARAMETERS_TO_CHECK); + recognizedParameterNames.add(CHARACTERS_TO_FORBID); + + + while (initParamNames.hasMoreElements()) { + final String initParamName = (String) initParamNames.nextElement(); + if (! recognizedParameterNames.contains(initParamName)) { + throw new ServletException("Unrecognized init parameter [" + initParamName + "]. Failing safe. Typo" + + " in the web.xml configuration? " + + " Misunderstanding about the configuration " + + RequestParameterPolicyEnforcementFilter.class.getSimpleName() + " expects?"); + } + } + } + + /** + * Parse a String to a boolean. + * + * "true" --> true + * "false" --> false + * null --> false + * Anything else --> throw IllegalArgumentException. + * + * This is a stateless static method. + * + * This method is an implementation detail and is not exposed API. + * This method is only non-private to allow JUnit testing. + * + * @param stringToParse a String to parse to a boolean as specified + * @return true or false + * @throws IllegalArgumentException if the String is not true, false, or null. + */ + static boolean parseStringToBooleanDefaultingToFalse(final String stringToParse) { + + if ("true".equals(stringToParse)) { + return true; + } else if ("false".equals(stringToParse)) { + return false; + } else if (null == stringToParse) { + return false; + } + + throw new IllegalArgumentException("String [" + stringToParse + + "] could not parse to a boolean because it was not precisely 'true' or 'false'."); + + } + + + /** + * Parse the whitespace delimited String of parameters to check. + * + * If the String is null, return the empty set. + * If the whitespace delimited String contains no tokens, throw IllegalArgumentException. + * If the sole token is an asterisk, return the empty set. + * If the asterisk token is encountered among other tokens, throw IllegalArgumentException. + * + * This method returning an empty Set has the special meaning of "check all parameters". + * + * This is a stateless static method. + * + * This method is an implementation detail and is not exposed API. + * This method is only non-private to allow JUnit testing. + * + * @param initParamValue null, or a non-blank whitespace delimited list of parameters to check + * @return a Set of String names of parameters to check, or an empty set representing check-them-all. + * + * @throws IllegalArgumentException when the init param value is out of spec + */ + static Set parseParametersToCheck(final String initParamValue) { + + final Set parameterNames = new HashSet(); + + if (null == initParamValue) { + return parameterNames; + } + + final String[] tokens = initParamValue.split("\\s+"); + + if ( 0 == tokens.length) { + throw new IllegalArgumentException("[" + initParamValue + + "] had no tokens but should have had at least one token."); + } + + if (1 == tokens.length && "*".equals(tokens[0])) { + return parameterNames; + } + + for (final String parameterName : tokens) { + + if ("*".equals(parameterName)) { + throw new IllegalArgumentException("Star token encountered among other tokens in parsing [" + initParamValue + "]"); + } + + parameterNames.add(parameterName); + } + + return parameterNames; + } + + /** + * Parse a whitespace delimited set of Characters from a String. + * + * If the String is null (init param was not set) default to DEFAULT_CHARACTERS_BLOCKED. + * If the String is "none" parse to empty set meaning block no characters. + * If the String is empty throw, to avoid configurer accidentally configuring not to block any characters. + * + * @param paramValue value of the init param to parse + * @return non-null Set of zero or more Characters to block + */ + static Set parseCharactersToForbid(String paramValue) { + + final Set charactersToForbid = new HashSet(); + + if (null == paramValue) { + paramValue = DEFAULT_CHARACTERS_BLOCKED; + } + + if ("none".equals(paramValue)) { + return charactersToForbid; + } + + final String[] tokens = paramValue.split("\\s+"); + + if (0 == tokens.length) { + throw new IllegalArgumentException("Expected tokens when parsing [" + paramValue + "] but found no tokens." + + " If you really want to configure no characters, use the magic value 'none'."); + } + + for (final String token : tokens) { + + if (token.length() > 1) { + throw new IllegalArgumentException("Expected tokens of length 1 but found [" + token + "] when " + + "parsing [" + paramValue + "]"); + } + + final Character character = token.charAt(0); + charactersToForbid.add(character); + } + + return charactersToForbid; + } + + /* ========================================================================================================== */ + /* Filtering requests */ + + /** + * For each parameter to check, verify that it has zero or one value. + * + * The Set of parameters to check MAY be empty. + * The parameter map MAY NOT contain any given parameter to check. + * + * This method is an implementation detail and is not exposed API. + * This method is only non-private to allow JUnit testing. + * + * + * Static, stateless method. + * + * @param parametersToCheck non-null potentially empty Set of String names of parameters + * @param parameterMap non-null Map from String name of parameter to String[] values + * + * @throws IllegalStateException if a parameterToCheck is present in the parameterMap with multiple values. + */ + static void requireNotMultiValued(final Set parametersToCheck, final Map parameterMap) { + + for (final String parameterName : parametersToCheck) { + if (parameterMap.containsKey(parameterName)) { + final String[] values = (String[]) parameterMap.get(parameterName); + if ( values.length > 1 ) { + throw new IllegalStateException("Parameter [" + parameterName + "] had multiple values [" + + values + "] but at most one value is allowable."); + } + } + } + + } + + /** + * For each parameter to check, for each value of that parameter, check that the value does not contain + * forbidden characters. + * + * This is a stateless static method. + * + * This method is an implementation detail and is not exposed API. + * This method is only non-private to allow JUnit testing. + * + * @param parametersToCheck Set of String request parameter names to look for + * @param charactersToForbid Set of Character characters to forbid + * @param parameterMap String --> String[] Map, in practice as read from ServletRequest + */ + static void enforceParameterContentCharacterRestrictions( + final Set parametersToCheck, final Set charactersToForbid, final Map parameterMap) { + + if (charactersToForbid.isEmpty()) { + // short circuit + return; + } + + for (final String parameterToCheck : parametersToCheck) { + + final String[] parameterValues = (String[]) parameterMap.get(parameterToCheck); + + if (null != parameterValues) { + + for (final String parameterValue : parameterValues) { + + for (final Character forbiddenCharacter : charactersToForbid) { + + final StringBuilder characterAsStringBuilder = new StringBuilder(); + characterAsStringBuilder.append(forbiddenCharacter); + + if (parameterValue.contains(characterAsStringBuilder)) { + throw new IllegalArgumentException("Disallowed character [" + forbiddenCharacter + + "] found in value [" + parameterValue + "] of parameter named [" + + parameterToCheck + "]"); + } + + // that forbiddenCharacter was not in this parameterValue + } + + // none of the charactersToForbid were in this parameterValue + } + + // none of the values of this parameterToCheck had a forbidden character + } // or this parameterToCheck had null value + + } + + // none of the values of any of the parametersToCheck had a forbidden character + // hurray! allow flow to continue without throwing an Exception. + } + + +} diff --git a/uportal-war/src/main/webapp/WEB-INF/web.xml b/uportal-war/src/main/webapp/WEB-INF/web.xml index 06e2149ee15..772a9e463ea 100644 --- a/uportal-war/src/main/webapp/WEB-INF/web.xml +++ b/uportal-war/src/main/webapp/WEB-INF/web.xml @@ -107,6 +107,15 @@ org.jasig.portal.utils.web.RequestAttributeMutexListener + + + requestParameterFilter + org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter + + parametersToCheck + ticket SAMLArt pgtIou pgtId + + CAS Validate Filter @@ -284,7 +293,14 @@ RenderingDispatcherServlet JsonRenderingDispatcherServlet - + + + requestParameterFilter + /Login + /CasProxyServlet + + + CAS Validate Filter /Login diff --git a/uportal-war/src/test/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilterTests.java b/uportal-war/src/test/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilterTests.java new file mode 100644 index 00000000000..3277273d2fb --- /dev/null +++ b/uportal-war/src/test/java/org/jasig/portal/security/firewall/RequestParameterPolicyEnforcementFilterTests.java @@ -0,0 +1,729 @@ +/* + * Licensed to Jasig under one or more contributor license + * agreements. See the NOTICE file distributed with this work + * for additional information regarding copyright ownership. + * Jasig licenses this file to you under the Apache License, + * Version 2.0 (the "License"); you may not use this file + * except in compliance with the License. You may obtain a + * copy of the License at the following location: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.jasig.portal.security.firewall; + +import java.io.IOException; +import java.util.*; + +import org.junit.Assert; +import org.junit.Test; + +import javax.servlet.*; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Tests the {@link org.jasig.portal.security.firewall.RequestParameterPolicyEnforcementFilter}. + * + * There are two kinds of testcases here. + * + * First there are testcases for the Filter as a whole against the Filter API. The advantage of these is that they + * are testing at the level we care about, the way the filter will actually be used, + * against the API it really exposes to adopters. So, great. The disadvantage of these is that it's + * + * Then there are testcases for bits of the implementation of the filter (namely, configuration parsing and policy + * enforcement). + * + * @since uPortal 4.0.15 + */ +public final class RequestParameterPolicyEnforcementFilterTests { + + /* ========================================================================================================== */ + + /* Tests for the Filter as a whole. + */ + + + /** + * Test that the Filter throws on init when unrecognized Filter init-param. + * @throws ServletException on test success. + */ + @Test(expected = ServletException.class) + public void testUnrecognizedInitParamFailsFilterInit() throws ServletException { + + final Set initParameterNames = new HashSet(); + initParameterNames.add("unrecognizedInitParameterName"); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + filter.init(filterConfig); + } + + /** + * Test that if you configure the filter to forbid no characters and also to allow multi-valued parameters, + * filter init fails because the filter would be a no-op. + */ + @Test(expected = ServletException.class) + public void testNoOpConfigurationFailsFilterInit() throws ServletException { + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + + // mock up filter config. + final Set initParameterNames = new HashSet(); + initParameterNames.add(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS); + initParameterNames.add(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS)) + .thenReturn("true"); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID)) + .thenReturn("none"); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK)) + .thenReturn(null); + + filter.init(filterConfig); + + + + } + + /** + * Test that, in the default configuration, when presented with a multi-valued parameter that configured to check + * and configured to require not multi valued, rejects request. + */ + @Test(expected = ServletException.class) + public void testRejectsMultiValuedRequestParameter() throws IOException, ServletException { + + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + + // mock up filter config. Default configuration with no init-params for this use case. + final Set initParameterNames = new HashSet(); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK)) + .thenReturn(null); + + + // init the filter + try { + filter.init(filterConfig); + } catch (Exception e) { + Assert.fail("Should not have failed filter init."); + } + + // mock up a request to filter + + final Map requestParameterMap = new HashMap(); + requestParameterMap.put("someName", new String[] {"someValue", "someOtherValue"}); + + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameterMap()).thenReturn(requestParameterMap); + + final HttpServletResponse response = mock(HttpServletResponse.class); + + final FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + } + + /** + * Test that, when configured to allow multi-valued parameters, + * when presented with a multi-valued parameter that configured to check + * , rejects request. + */ + @Test + public void testAcceptsMultiValuedRequestParameter() throws IOException, ServletException { + + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + + // mock up filter config. Configure to allow multi-valued parameters. + final Set initParameterNames = new HashSet(); + initParameterNames.add(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS)) + .thenReturn("true"); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK)) + .thenReturn(null); + + + // init the filter + try { + filter.init(filterConfig); + } catch (Exception e) { + Assert.fail("Should not have failed filter init."); + } + + // mock up a request to filter, with a multi-valued checked parameter + + final Map requestParameterMap = new HashMap(); + requestParameterMap.put("someName", new String[] {"someValue", "someOtherValue"}); + + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameterMap()).thenReturn(requestParameterMap); + + final HttpServletResponse response = mock(HttpServletResponse.class); + + final FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + } + + /** + * Test that, in the default configuration, when presented with a request parameter with an illicit character in + * it, blocks the request. + * @throws IOException + * @throws ServletException + */ + @Test(expected = ServletException.class) + public void testRejectsRequestWithIllicitCharacterInCheckedParameter() throws IOException, ServletException { + + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + + // mock up filter config. Default configuration with no init-params for this use case. + final Set initParameterNames = new HashSet(); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK)) + .thenReturn(null); + + + // init the filter + try { + filter.init(filterConfig); + } catch (Exception e) { + Assert.fail("Should not have failed filter init."); + } + + // mock up a request to filter + + final Map requestParameterMap = new HashMap(); + // percent character is illicit by default, so, illicit character in this parameter value + requestParameterMap.put("someName", new String[] {"someValue%40gmail.com"}); + + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameterMap()).thenReturn(requestParameterMap); + + final HttpServletResponse response = mock(HttpServletResponse.class); + + final FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + } + + /** + * Test that when configured to only check some parameters, does not throw on forbidden character in unchecked + * parameter. + */ + @Test + public void testAllowsUncheckedParametersToHaveIllicitCharacters() throws IOException, ServletException { + + final RequestParameterPolicyEnforcementFilter filter = new RequestParameterPolicyEnforcementFilter(); + + // mock up filter config. Default configuration with no init-params for this use case. + final Set initParameterNames = new HashSet(); + initParameterNames.add(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK); + final Enumeration parameterNamesEnumeration = Collections.enumeration(initParameterNames); + final FilterConfig filterConfig = mock(FilterConfig.class); + when(filterConfig.getInitParameterNames()).thenReturn(parameterNamesEnumeration); + + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID)) + .thenReturn(null); + when(filterConfig.getInitParameter(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK)) + .thenReturn("ticket"); + + + // init the filter + try { + filter.init(filterConfig); + } catch (Exception e) { + Assert.fail("Should not have failed filter init."); + } + + // mock up a request to filter + + final Map requestParameterMap = new HashMap(); + // percent character is illicit by default, so, illicit character in this parameter value + // but this parameter name is unchecked + requestParameterMap.put("uncheckedName", new String[] {"someValue%40gmail.com"}); + + final HttpServletRequest request = mock(HttpServletRequest.class); + when(request.getParameterMap()).thenReturn(requestParameterMap); + + final HttpServletResponse response = mock(HttpServletResponse.class); + + final FilterChain chain = mock(FilterChain.class); + + filter.doFilter(request, response, chain); + + } + + + /* ========================================================================================================== */ + + /* Tests for throwIfUnrecognizedInitParamNames() + * These test that the fail-safe on unrecognized (and thus un-honored) configuration works as intended. + */ + + /** + * Tests that the method checking for unrecognized parameters accepts the expected parameters. + * @throws ServletException on test failure + */ + @Test + public void testAcceptsExpectedParameterNames() throws ServletException { + + final Set parameterNames = new HashSet(); + parameterNames.add(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID); + parameterNames.add(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK); + parameterNames.add(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS); + final Enumeration parameterNamesEnumeration = Collections.enumeration(parameterNames); + + RequestParameterPolicyEnforcementFilter.throwIfUnrecognizedParamName(parameterNamesEnumeration); + } + + /** + * Test that the method checking for unrecognized parameters throws on an unrecognized parameter. + * @throws ServletException on test success + */ + @Test(expected = ServletException.class) + public void testRejectsUnExpectedParameterName() throws ServletException { + + final Set parameterNames = new HashSet(); + parameterNames.add(RequestParameterPolicyEnforcementFilter.CHARACTERS_TO_FORBID); + parameterNames.add(RequestParameterPolicyEnforcementFilter.PARAMETERS_TO_CHECK); + parameterNames.add(RequestParameterPolicyEnforcementFilter.ALLOW_MULTI_VALUED_PARAMETERS); + parameterNames.add("unexpectedParameterName"); + final Enumeration parameterNamesEnumeration = Collections.enumeration(parameterNames); + + RequestParameterPolicyEnforcementFilter.throwIfUnrecognizedParamName(parameterNamesEnumeration); + } + + + + /* ========================================================================================================== */ + + /* Tests for parseStringToBooleanDefaultingToFalse() + * These test that the boolean init parameter parsing works properly. + */ + + /** + * Test that true parses to true. + */ + @Test + public void testParsesTrueToTrue() { + + Assert.assertTrue(RequestParameterPolicyEnforcementFilter.parseStringToBooleanDefaultingToFalse("true")); + + } + + /** + * Test that false parses to false. + */ + @Test + public void testParsesFalseToFalse() { + + Assert.assertFalse(RequestParameterPolicyEnforcementFilter.parseStringToBooleanDefaultingToFalse("false")); + + } + + /** + * Test that null parses to false. + */ + @Test + public void testParsesNullToFalse() { + + Assert.assertFalse(RequestParameterPolicyEnforcementFilter.parseStringToBooleanDefaultingToFalse(null)); + + } + + /** + * Test that maybe parses to illegal argument exception. + */ + @Test(expected = Exception.class) + public void testParsingMaybeYieldsException() { + + RequestParameterPolicyEnforcementFilter.parseStringToBooleanDefaultingToFalse("maybe"); + + } + + + /* ========================================================================================================== */ + /* Tests for parseParametersToCheck(). + * Ensure that the Filter properly understands which parameters it ought to be checking. + */ + + /** + * Test that a null parameter value parses to the empty set. + */ + @Test + public void testParsesNullToEmptySet() { + + final Set returnedSet = RequestParameterPolicyEnforcementFilter.parseParametersToCheck(null); + + Assert.assertTrue(returnedSet.isEmpty()); + + } + + /** + * Test that a whitespace delimited list of parameter names parses as expected. + */ + @Test + public void testParsesWhiteSpaceDelimitedStringToSet() { + + final String parameterValue = "service renew gateway"; + + final Set expectedSet = new HashSet(); + expectedSet.add("service"); + expectedSet.add("renew"); + expectedSet.add("gateway"); + + final Set returnedSet = RequestParameterPolicyEnforcementFilter.parseParametersToCheck(parameterValue); + + Assert.assertEquals(expectedSet, returnedSet); + } + + /** + * Test that blank parses to exception. + */ + @Test(expected = Exception.class) + public void testParsingBlankParametersToCheckThrowsException() { + + RequestParameterPolicyEnforcementFilter.parseParametersToCheck(" "); + + } + + /** + * Test the special parsing behavior of star parses to empty Set. + */ + @Test + public void testAsteriskParsesToEmptySetOfParametersToCheck() { + + final Set expectedSet = new HashSet(); + + final Set returnedSet = RequestParameterPolicyEnforcementFilter.parseParametersToCheck("*"); + + Assert.assertEquals(expectedSet, returnedSet); + + } + + /** + * Test that encountering the star token among other tokens yields exception. + */ + @Test(expected = Exception.class) + public void testParsingAsteriskWithOtherTokensThrowsException() { + + RequestParameterPolicyEnforcementFilter.parseParametersToCheck("renew * gateway"); + + } + + + /* ========================================================================================================== */ + /* Tests for parseCharactersToForbid(). + * Ensure that the Filter properly understands which characters it ought to be forbidding. + */ + + /** + * Test that when the parameter is not set (is null) parses to a default set of character. + */ + @Test + public void testParsingNullYieldsPercentHashAmpersandAndQuestionMark() { + + final Set expected = new HashSet(); + expected.add('%'); + expected.add('#'); + expected.add('&'); + expected.add('?'); + + final Set actual = RequestParameterPolicyEnforcementFilter.parseCharactersToForbid(null); + + Assert.assertEquals(expected, actual); + + } + + /** + * Test that when the parameter is set but blank throws an exception. + */ + @Test(expected = Exception.class) + public void testParsingBlankThrowsException() { + RequestParameterPolicyEnforcementFilter.parseCharactersToForbid(" "); + } + + /** + * Test that when the parameter is set to the special value "none" returns empty Set. + */ + @Test + public void testParsesLiteralNoneToEmptySet() { + + final Set expected = new HashSet(); + + final Set actual = RequestParameterPolicyEnforcementFilter.parseCharactersToForbid("none"); + + Assert.assertEquals(expected, actual); + } + + /** + * Test that parsing some characters works as expected. + */ + @Test + public void testParsingSomeCharactersWorks() { + final Set expected = new HashSet(); + expected.add('&'); + expected.add('%'); + expected.add('*'); + expected.add('#'); + expected.add('@'); + + final Set actual = RequestParameterPolicyEnforcementFilter.parseCharactersToForbid("& % * # @"); + + Assert.assertEquals(expected, actual); + } + + /** + * The tokens are supposed to be single characters. If they are longer than that, the deployer may be confused as + * to how to configure this filter. + */ + @Test(expected = Exception.class) + public void testParsingMulticharacterTokensThrows() { + RequestParameterPolicyEnforcementFilter.parseCharactersToForbid("& %*# @"); + } + + + /* ========================================================================================================== */ + + /* Tests for requireNotMultiValued(). + * Ensure that runtime enforcement of not-multi-valued-ness work properly. + */ + + + + + /** + * Test that enforcing no-multi-value detects multi-valued parameter and throws. + */ + @Test(expected = IllegalStateException.class) + public void testRequireNotMultiValueBlocksMultiValue() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("dogName"); + + // set up a parameter map with a multi-valued parameter + final Map parameterMap = new HashMap(); + parameterMap.put("dogName", new String[] {"Johan", "Cubby"}); + + RequestParameterPolicyEnforcementFilter.requireNotMultiValued(parametersToCheck, parameterMap); + } + + /** + * Test that enforcing no-multi-value allows single-valued parameter. + */ + @Test + public void testRequireNotMultiValuedAllowsSingleValued() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("dogName"); + + // set up a parameter map with single-valued parameter + final Map parameterMap = new HashMap(); + parameterMap.put("dogName", new String[] {"Abbie"}); + + RequestParameterPolicyEnforcementFilter.requireNotMultiValued(parametersToCheck, parameterMap); + + } + + /** + * Test that enforcing no-multi-value allows not-present parameter. + */ + @Test + public void testRequireNotMultiValuedIgnoresMissingParameter() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("dogName"); + + // set up a parameter map with no entries + final Map parameterMap = new HashMap(); + + RequestParameterPolicyEnforcementFilter.requireNotMultiValued(parametersToCheck, parameterMap); + + } + + /** + * Test that enforcing no-multi-value allows multi-valued parameters not among those to check. + */ + @Test + public void testRequireNotMultiValueAllowsUncheckedMultiValue() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("dogName"); + + // set up a parameter map with a multi-valued parameter with a name not matching those to check + final Map parameterMap = new HashMap(); + parameterMap.put("catName", new String[] {"Reggie", "Shenanigans"}); + + RequestParameterPolicyEnforcementFilter.requireNotMultiValued(parametersToCheck, parameterMap); + } + + /* ========================================================================================================== */ + + /* Tests for enforceParameterContentCharacterRestrictions(). + * Ensure that runtime enforcement of what characters parameters contain works properly. + */ + + /** + * Test that allows parameters not containing forbidden characters. + */ + @Test + public void testAllowsParametersNotContainingForbiddenCharacters() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("catName"); + parametersToCheck.add("dogName"); + + final Set charactersToForbid = new HashSet(); + charactersToForbid.add('&'); + charactersToForbid.add('%'); + + final Map parameterMap = new HashMap(); + parameterMap.put("catName", new String[] {"Reggie", "Shenanigans"}); + parameterMap.put("dogName", new String[] {"Brutus", "Johan", "Cubby", "Abbie"}); + parameterMap.put("plantName", new String[] {"Rex"}); + + RequestParameterPolicyEnforcementFilter.enforceParameterContentCharacterRestrictions(parametersToCheck, + charactersToForbid, parameterMap); + + } + + /** + * Test that when a checked parameter contains a forbidden character, throws. + */ + @Test(expected = Exception.class) + public void testThrowsOnParameterContainingForbiddenCharacter() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("catName"); + parametersToCheck.add("plantName"); + + final Set charactersToForbid = new HashSet(); + charactersToForbid.add('&'); + charactersToForbid.add('%'); + + final Map parameterMap = new HashMap(); + parameterMap.put("catName", new String[] {"Reggie", "Shenanigans"}); + parameterMap.put("dogName", new String[] {"Brutus", "Johan", "Cubby", "Abbie"}); + // plantName is checked, and contains a forbidden character + parameterMap.put("plantName", new String[] {"Rex&p0wned=true"}); + + RequestParameterPolicyEnforcementFilter.enforceParameterContentCharacterRestrictions(parametersToCheck, + charactersToForbid, parameterMap); + + } + + /** + * Test that when a checked parameter contains a forbidden character in a non-first value, still throws. + */ + @Test(expected = Exception.class) + public void testThrowsOnMultipleParameterContainingForbiddenCharacter() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("catName"); + parametersToCheck.add("dogName"); + + final Set charactersToForbid = new HashSet(); + charactersToForbid.add('!'); + charactersToForbid.add('$'); + + final Map parameterMap = new HashMap(); + parameterMap.put("catName", new String[] {"Reggie", "Shenanigans"}); + // dogName is checked, and contains a forbidden character + parameterMap.put("dogName", new String[] {"Brutus", "Johan", "Cub!by", "Abbie"}); + parameterMap.put("plantName", new String[] {"Rex"}); + + RequestParameterPolicyEnforcementFilter.enforceParameterContentCharacterRestrictions(parametersToCheck, + charactersToForbid, parameterMap); + + } + + /** + * Test that when an unchecked parameter contains a character that would be forbidden were the parameter checked, + * does not throw. + */ + @Test + public void testAllowsUncheckedParameterContainingForbiddenCharacter() { + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("catName"); + parametersToCheck.add("dogName"); + + final Set charactersToForbid = new HashSet(); + charactersToForbid.add('&'); + charactersToForbid.add('$'); + + final Map parameterMap = new HashMap(); + parameterMap.put("catName", new String[] {"Reggie", "Shenanigans"}); + parameterMap.put("dogName", new String[] {"Brutus", "Johan", "Cubby", "Abbie"}); + + // plantName is not checked + parameterMap.put("plantName", new String[] {"Rex&ownage=true"}); + + RequestParameterPolicyEnforcementFilter.enforceParameterContentCharacterRestrictions(parametersToCheck, + charactersToForbid, parameterMap); + + } + + /** + * Test that not all parametersToCheck need be present on the request. + */ + @Test + public void testAllowsCheckedParameterNotPresent() { + // this test added in response to a stupid NullPointerException defect, to prevent regression. + + final Set parametersToCheck = new HashSet(); + parametersToCheck.add("catName"); + parametersToCheck.add("dogName"); + + final Set charactersToForbid = new HashSet(); + charactersToForbid.add('&'); + charactersToForbid.add('$'); + + final Map parameterMap = new HashMap(); + parameterMap.put("dogName", new String[] {"Brutus", "Johan", "Cubby", "Abbie"}); + + RequestParameterPolicyEnforcementFilter.enforceParameterContentCharacterRestrictions(parametersToCheck, + charactersToForbid, parameterMap); + + } + +}