You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@wicket.apache.org by pa...@apache.org on 2020/08/07 20:43:29 UTC
[wicket] branch master updated (5fc33a5 -> bef3fac)
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git.
from 5fc33a5 Updated eclipse settings to reflect minimum java level of 11. (#441)
add e61a083 Added Fetch Metadata checks to CsrfPrevention
add cfde11b WICKET-6786: Initial implementation of Fetch Metadata and integration with CsrfPreventionRequestCycleListener
add 0827a19 Make the current resource isolation policy configurable on CSRF request cycle listener. Sort static imports.
add c4a03da Make constructors of CsrfPreventionRequestCycleListener visible to subclasses. This is required by the WebSocketAwareCsrfPreventionRequestCycleListener
add d6fc569 Fixed the change to checkRequest method signature to preserve API
add 584257d WICKET-6786: Adds new Fetch Metadata request listener. Makes the legacy CsrfPreventionRequestCycleListener be a ResourceIsolationPolicy that can be used in combination with the DefaultResourceIsolationPolicy to add support for legacy browsers that don't send Sec-Fetch headers yet.
add 61f4b93 WICKET-6786: Minor refactor of FetchMetadataRequestCycleListener.
add e9c471e WICKET-6786: Create OriginBasedResourceIsolationPolicy to support legacy browsers that don't send Sec-Fetch-* headers and add it as a default Resource Isolation Policy to the Fetch Metadata listener.
add 1e2acdb WICKET-6786: Fix logging statements.
new 5d9d4b5 WICKET-6786: code reformatting
new c51372b WICKET-6786: reintroduced the configuration options from CsrfPreventionRequestCycleListener
new d01d6a2 Merge branch 'wicket-6786'
new bef3fac WICKET-6786: Fix license header and add some comments
The 4 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
.../http/CsrfPreventionRequestCycleListener.java | 240 ++----------
.../http/DefaultResourceIsolationPolicy.java | 80 ++++
.../http/FetchMetadataRequestCycleListener.java | 415 +++++++++++++++++++++
.../http/OriginBasedResourceIsolationPolicy.java | 283 ++++++++++++++
.../protocol/http/ResourceIsolationOutcome.java | 12 +-
.../protocol/http/ResourceIsolationPolicy.java | 56 +++
.../CsrfPreventionRequestCycleListenerTest.java | 69 +++-
.../FetchMetadataRequestCycleListenerTest.java | 200 ++++++++++
8 files changed, 1122 insertions(+), 233 deletions(-)
create mode 100644 wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
create mode 100644 wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
create mode 100644 wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
copy wicket-cdi/src/main/java/org/apache/wicket/cdi/DetachEvent.java => wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java (74%)
create mode 100644 wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
create mode 100644 wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
[wicket] 01/04: WICKET-6786: code reformatting
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit 5d9d4b598ec070cb863d67124d276e74cd3cdc87
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Fri Aug 7 12:13:32 2020 +0200
WICKET-6786: code reformatting
---
.../http/CsrfPreventionRequestCycleListener.java | 4 +-
.../http/DefaultResourceIsolationPolicy.java | 6 +-
.../http/FetchMetadataRequestCycleListener.java | 212 +++++-----
.../http/OriginBasedResourceIsolationPolicy.java | 446 +++++++++++----------
.../FetchMetadataRequestCycleListenerTest.java | 307 +++++++-------
5 files changed, 497 insertions(+), 478 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
index db39cc5..f4d4040 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/CsrfPreventionRequestCycleListener.java
@@ -579,8 +579,8 @@ public class CsrfPreventionRequestCycleListener extends OriginBasedResourceIsola
{
onAborted(request, origin, page);
log.info(
- "Possible CSRF attack, request URL: {}, Origin: {}, action: aborted with error {} {}",
- request.getRequestURL(), origin, errorCode, errorMessage);
+ "Possible CSRF attack, request URL: {}, Origin: {}, action: aborted with error {} {}",
+ request.getRequestURL(), origin, errorCode, errorMessage);
throw new AbortWithHttpErrorCodeException(errorCode, errorMessage);
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
index de9d155..b1d7cf7 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
@@ -34,11 +34,11 @@ public class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy
{
@Override
- public boolean isRequestAllowed(HttpServletRequest request,
- IRequestablePage targetPage)
+ public boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
{
// request made by a legacy browser with no support for Fetch Metadata
- if (!hasFetchMetadataHeaders(request)) {
+ if (!hasFetchMetadataHeaders(request))
+ {
return true;
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
index 1d1f88d..ea9a8ef 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
@@ -44,117 +44,121 @@ import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
/**
- * The Fetch Metadata Request Cycle Listener is Wicket's implementation of Fetch Metadata.
- * This adds a layer of protection for modern browsers that prevents Cross-Site Request Forgery
- * attacks.
+ * The Fetch Metadata Request Cycle Listener is Wicket's implementation of Fetch Metadata. This adds
+ * a layer of protection for modern browsers that prevents Cross-Site Request Forgery attacks.
*
* This request listener uses the {@link DefaultResourceIsolationPolicy} by default and can be
* customized with additional Resource Isolation Policies.
*
- * This listener can be configured to add exempted URL paths that are intended to be used cross-site.
+ * This listener can be configured to add exempted URL paths that are intended to be used
+ * cross-site.
*
- * Learn more about Fetch Metadata and resource isolation
- * at <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
+ * Learn more about Fetch Metadata and resource isolation at
+ * <a href="https://web.dev/fetch-metadata/">https://web.dev/fetch-metadata/</a>
*
* @author Santiago Diaz - saldiaz@google.com
* @author Ecenaz Jen Ozmen - ecenazo@google.com
*/
-public class FetchMetadataRequestCycleListener implements IRequestCycleListener {
-
- private static final Logger log = LoggerFactory
- .getLogger(FetchMetadataRequestCycleListener.class);
- public static final int ERROR_CODE = 403;
- public static final String ERROR_MESSAGE = "Forbidden";
- public static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", "
- + SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER;
-
- private final Set<String> exemptedPaths = new HashSet<>();
- private final List<ResourceIsolationPolicy> resourceIsolationPolicies = new ArrayList<>();
-
- public FetchMetadataRequestCycleListener(ResourceIsolationPolicy... additionalPolicies) {
- this.resourceIsolationPolicies.addAll(
- asList(
- new DefaultResourceIsolationPolicy(),
- new OriginBasedResourceIsolationPolicy()
- )
- );
-
- this.resourceIsolationPolicies.addAll(asList(additionalPolicies));
- }
-
- public void addExemptedPaths(String... exemptions) {
- Arrays.stream(exemptions)
- .filter(e -> !Strings.isEmpty(e))
- .forEach(exemptedPaths::add);
- }
-
- @Override
- public void onBeginRequest(RequestCycle cycle)
- {
- HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
- .getContainerRequest();
-
- log.debug("Processing request to: {}", containerRequest.getPathInfo());
- }
-
- @Override
- public void onRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler)
- {
- handler = unwrap(handler);
- IPageRequestHandler pageRequestHandler = getPageRequestHandler(handler);
- if (pageRequestHandler == null) {
- return;
- }
-
- IRequestablePage targetedPage = pageRequestHandler.getPage();
- HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
- .getContainerRequest();
-
- String pathInfo = containerRequest.getPathInfo();
- if (exemptedPaths.contains(pathInfo)) {
- if (log.isDebugEnabled()) {
- log.debug("Allowing request to {} because it matches an exempted path",
- new Object[]{pathInfo});
- }
- return;
- }
-
- for (ResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies) {
- if (!resourceIsolationPolicy.isRequestAllowed(containerRequest, targetedPage)) {
- log.debug("Isolation policy {} has rejected a request to {}",
- Classes.simpleName(resourceIsolationPolicy.getClass()), pathInfo);
- throw new AbortWithHttpErrorCodeException(ERROR_CODE, ERROR_MESSAGE);
- }
- }
- }
-
- @Override
- public void onEndRequest(RequestCycle cycle)
- {
- // set vary headers to avoid caching responses processed by Fetch Metadata
- // caching these responses may return 403 responses to legitimate requests
- // or defeat the protection
- if (cycle.getResponse() instanceof WebResponse)
- {
- WebResponse webResponse = (WebResponse)cycle.getResponse();
- if (webResponse.isHeaderSupported())
- {
- webResponse.addHeader(VARY_HEADER, VARY_HEADER_VALUE);
- }
- }
- }
-
- private static IRequestHandler unwrap(IRequestHandler handler) {
- while (handler instanceof IRequestHandlerDelegate) {
- handler = ((IRequestHandlerDelegate)handler).getDelegateHandler();
- }
- return handler;
- }
-
- private IPageRequestHandler getPageRequestHandler(IRequestHandler handler)
- {
- boolean isPageRequestHandler = handler instanceof IPageRequestHandler &&
- !(handler instanceof RenderPageRequestHandler);
- return isPageRequestHandler ? (IPageRequestHandler) handler : null;
- }
+public class FetchMetadataRequestCycleListener implements IRequestCycleListener
+{
+
+ private static final Logger log = LoggerFactory
+ .getLogger(FetchMetadataRequestCycleListener.class);
+ public static final int ERROR_CODE = 403;
+ public static final String ERROR_MESSAGE = "Forbidden";
+ public static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", "
+ + SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER;
+
+ private final Set<String> exemptedPaths = new HashSet<>();
+ private final List<ResourceIsolationPolicy> resourceIsolationPolicies = new ArrayList<>();
+
+ public FetchMetadataRequestCycleListener(ResourceIsolationPolicy... additionalPolicies)
+ {
+ this.resourceIsolationPolicies.addAll(
+ asList(new DefaultResourceIsolationPolicy(), new OriginBasedResourceIsolationPolicy()));
+
+ this.resourceIsolationPolicies.addAll(asList(additionalPolicies));
+ }
+
+ public void addExemptedPaths(String... exemptions)
+ {
+ Arrays.stream(exemptions).filter(e -> !Strings.isEmpty(e)).forEach(exemptedPaths::add);
+ }
+
+ @Override
+ public void onBeginRequest(RequestCycle cycle)
+ {
+ HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
+ .getContainerRequest();
+
+ log.debug("Processing request to: {}", containerRequest.getPathInfo());
+ }
+
+ @Override
+ public void onRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler)
+ {
+ handler = unwrap(handler);
+ IPageRequestHandler pageRequestHandler = getPageRequestHandler(handler);
+ if (pageRequestHandler == null)
+ {
+ return;
+ }
+
+ IRequestablePage targetedPage = pageRequestHandler.getPage();
+ HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
+ .getContainerRequest();
+
+ String pathInfo = containerRequest.getPathInfo();
+ if (exemptedPaths.contains(pathInfo))
+ {
+ if (log.isDebugEnabled())
+ {
+ log.debug("Allowing request to {} because it matches an exempted path",
+ new Object[] { pathInfo });
+ }
+ return;
+ }
+
+ for (ResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies)
+ {
+ if (!resourceIsolationPolicy.isRequestAllowed(containerRequest, targetedPage))
+ {
+ log.debug("Isolation policy {} has rejected a request to {}",
+ Classes.simpleName(resourceIsolationPolicy.getClass()), pathInfo);
+ throw new AbortWithHttpErrorCodeException(ERROR_CODE, ERROR_MESSAGE);
+ }
+ }
+ }
+
+ @Override
+ public void onEndRequest(RequestCycle cycle)
+ {
+ // set vary headers to avoid caching responses processed by Fetch Metadata
+ // caching these responses may return 403 responses to legitimate requests
+ // or defeat the protection
+ if (cycle.getResponse() instanceof WebResponse)
+ {
+ WebResponse webResponse = (WebResponse)cycle.getResponse();
+ if (webResponse.isHeaderSupported())
+ {
+ webResponse.addHeader(VARY_HEADER, VARY_HEADER_VALUE);
+ }
+ }
+ }
+
+ private static IRequestHandler unwrap(IRequestHandler handler)
+ {
+ while (handler instanceof IRequestHandlerDelegate)
+ {
+ handler = ((IRequestHandlerDelegate)handler).getDelegateHandler();
+ }
+ return handler;
+ }
+
+ private IPageRequestHandler getPageRequestHandler(IRequestHandler handler)
+ {
+ boolean isPageRequestHandler = handler instanceof IPageRequestHandler
+ && !(handler instanceof RenderPageRequestHandler);
+ return isPageRequestHandler ? (IPageRequestHandler)handler : null;
+ }
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
index e8fd195..bf07276 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
@@ -29,251 +29,255 @@ import org.apache.wicket.util.string.Strings;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
-public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPolicy {
- private static final Logger log = LoggerFactory
- .getLogger(OriginBasedResourceIsolationPolicy.class);
+public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPolicy
+{
+ private static final Logger log = LoggerFactory
+ .getLogger(OriginBasedResourceIsolationPolicy.class);
- /**
- * A white list of accepted origins (host names/domain names) presented as
- * <domainname>.<TLD>. The domain part can contain subdomains.
- */
- private Collection<String> acceptedOrigins = new ArrayList<>();
+ /**
+ * A white list of accepted origins (host names/domain names) presented as
+ * <domainname>.<TLD>. The domain part can contain subdomains.
+ */
+ private Collection<String> acceptedOrigins = new ArrayList<>();
- /**
- * Adds an origin (host name/domain name) to the white list. An origin is in the form of
- * <domainname>.<TLD>, and can contain a subdomain. Every Origin header that matches
- * a domain from the whitelist is accepted and not checked any further for CSRF issues.
- *
- * E.g. when {@code example.com} is in the white list, this allows requests from (i.e. with an
- * {@code Origin:} header containing) {@code example.com} and {@code blabla.example.com} but
- * rejects requests from {@code blablaexample.com} and {@code example2.com}.
- *
- * @param acceptedOrigin
- * the acceptable origin
- * @return this
- */
- public OriginBasedResourceIsolationPolicy addAcceptedOrigin(String acceptedOrigin)
- {
- Checks.notNull("acceptedOrigin", acceptedOrigin);
+ /**
+ * Adds an origin (host name/domain name) to the white list. An origin is in the form of
+ * <domainname>.<TLD>, and can contain a subdomain. Every Origin header that matches
+ * a domain from the whitelist is accepted and not checked any further for CSRF issues.
+ *
+ * E.g. when {@code example.com} is in the white list, this allows requests from (i.e. with an
+ * {@code Origin:} header containing) {@code example.com} and {@code blabla.example.com} but
+ * rejects requests from {@code blablaexample.com} and {@code example2.com}.
+ *
+ * @param acceptedOrigin
+ * the acceptable origin
+ * @return this
+ */
+ public OriginBasedResourceIsolationPolicy addAcceptedOrigin(String acceptedOrigin)
+ {
+ Checks.notNull("acceptedOrigin", acceptedOrigin);
- // strip any leading dot characters
- final int len = acceptedOrigin.length();
- int i = 0;
- while (i < len && acceptedOrigin.charAt(i) == '.')
- {
- i++;
- }
- acceptedOrigins.add(acceptedOrigin.substring(i));
- return this;
- }
+ // strip any leading dot characters
+ final int len = acceptedOrigin.length();
+ int i = 0;
+ while (i < len && acceptedOrigin.charAt(i) == '.')
+ {
+ i++;
+ }
+ acceptedOrigins.add(acceptedOrigin.substring(i));
+ return this;
+ }
- /**
- * This origin-based listener can be used in combination with the {@link FetchMetadataRequestCycleListener}
- * to add support for legacy browsers that don't send Sec-Fetch-* headers yet.
- * @return whether the request is allowed based on its origin
- */
- @Override
- public boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage) {
- String sourceUri = getSourceUri(request);
+ /**
+ * This origin-based listener can be used in combination with the
+ * {@link FetchMetadataRequestCycleListener} to add support for legacy browsers that don't send
+ * Sec-Fetch-* headers yet.
+ *
+ * @return whether the request is allowed based on its origin
+ */
+ @Override
+ public boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
+ {
+ String sourceUri = getSourceUri(request);
- if (sourceUri == null || sourceUri.isEmpty())
- {
- log.debug("Source URI not present in request to {}", request.getPathInfo());
- return true;
- }
- sourceUri = sourceUri.toLowerCase(Locale.ROOT);
+ if (sourceUri == null || sourceUri.isEmpty())
+ {
+ log.debug("Source URI not present in request to {}", request.getPathInfo());
+ return true;
+ }
+ sourceUri = sourceUri.toLowerCase(Locale.ROOT);
- // if the origin is a know and trusted origin, don't check any further but allow the request
- if (isWhitelistedHost(sourceUri))
- {
- return true;
- }
+ // if the origin is a know and trusted origin, don't check any further but allow the request
+ if (isWhitelistedHost(sourceUri))
+ {
+ return true;
+ }
- // check if the origin HTTP header matches the request URI
- if (!isLocalOrigin(request, sourceUri))
- {
- log.debug("Source URI conflicts with request origin");
- return false;
- }
+ // check if the origin HTTP header matches the request URI
+ if (!isLocalOrigin(request, sourceUri))
+ {
+ log.debug("Source URI conflicts with request origin");
+ return false;
+ }
- return true;
- }
+ return true;
+ }
- /**
- * Checks whether the {@code Origin} HTTP header of the request matches where the request came
- * from.
- *
- * @param containerRequest
- * the current container request
- * @param originHeader
- * the contents of the {@code Origin} HTTP header
- * @return {@code true} when the origin of the request matches the {@code Origin} HTTP header
- */
- protected boolean isLocalOrigin(HttpServletRequest containerRequest, String originHeader)
- {
- // Make comparable strings from Origin and Location
- String origin = normalizeUri(originHeader);
- if (origin == null)
- return false;
+ /**
+ * Checks whether the {@code Origin} HTTP header of the request matches where the request came
+ * from.
+ *
+ * @param containerRequest
+ * the current container request
+ * @param originHeader
+ * the contents of the {@code Origin} HTTP header
+ * @return {@code true} when the origin of the request matches the {@code Origin} HTTP header
+ */
+ protected boolean isLocalOrigin(HttpServletRequest containerRequest, String originHeader)
+ {
+ // Make comparable strings from Origin and Location
+ String origin = normalizeUri(originHeader);
+ if (origin == null)
+ return false;
- String request = getTargetUriFromRequest(containerRequest);
- if (request == null)
- return false;
+ String request = getTargetUriFromRequest(containerRequest);
+ if (request == null)
+ return false;
- return origin.equalsIgnoreCase(request);
- }
+ return origin.equalsIgnoreCase(request);
+ }
- /**
- * Creates a RFC-6454 comparable URI from the {@code request} requested resource.
- *
- * @param request
- * the incoming request
- * @return only the scheme://host[:port] part, or {@code null} when the origin string is not
- * compliant
- */
- protected final String getTargetUriFromRequest(HttpServletRequest request)
- {
- // Build scheme://host:port from request
- StringBuilder target = new StringBuilder();
- String scheme = request.getScheme();
- if (scheme == null)
- {
- return null;
- }
- else
- {
- scheme = scheme.toLowerCase(Locale.ROOT);
- }
- target.append(scheme);
- target.append("://");
+ /**
+ * Creates a RFC-6454 comparable URI from the {@code request} requested resource.
+ *
+ * @param request
+ * the incoming request
+ * @return only the scheme://host[:port] part, or {@code null} when the origin string is not
+ * compliant
+ */
+ protected final String getTargetUriFromRequest(HttpServletRequest request)
+ {
+ // Build scheme://host:port from request
+ StringBuilder target = new StringBuilder();
+ String scheme = request.getScheme();
+ if (scheme == null)
+ {
+ return null;
+ }
+ else
+ {
+ scheme = scheme.toLowerCase(Locale.ROOT);
+ }
+ target.append(scheme);
+ target.append("://");
- String host = request.getServerName();
- if (host == null)
- {
- return null;
- }
- target.append(host);
+ String host = request.getServerName();
+ if (host == null)
+ {
+ return null;
+ }
+ target.append(host);
- int port = request.getServerPort();
- if ("http".equals(scheme) && port != 80 || "https".equals(scheme) && port != 443)
- {
- target.append(':');
- target.append(port);
- }
+ int port = request.getServerPort();
+ if ("http".equals(scheme) && port != 80 || "https".equals(scheme) && port != 443)
+ {
+ target.append(':');
+ target.append(port);
+ }
- return target.toString();
- }
+ return target.toString();
+ }
- /**
- * Resolves the source URI from the request headers ({@code Origin} or {@code Referer}).
- *
- * @param containerRequest
- * the current container request
- * @return the normalized source URI.
- */
- private String getSourceUri(HttpServletRequest containerRequest)
- {
- String sourceUri = containerRequest.getHeader(WebRequest.HEADER_ORIGIN);
- if (Strings.isEmpty(sourceUri))
- {
- sourceUri = containerRequest.getHeader(WebRequest.HEADER_REFERER);
- }
- return normalizeUri(sourceUri);
- }
+ /**
+ * Resolves the source URI from the request headers ({@code Origin} or {@code Referer}).
+ *
+ * @param containerRequest
+ * the current container request
+ * @return the normalized source URI.
+ */
+ private String getSourceUri(HttpServletRequest containerRequest)
+ {
+ String sourceUri = containerRequest.getHeader(WebRequest.HEADER_ORIGIN);
+ if (Strings.isEmpty(sourceUri))
+ {
+ sourceUri = containerRequest.getHeader(WebRequest.HEADER_REFERER);
+ }
+ return normalizeUri(sourceUri);
+ }
- /**
- * Creates a RFC-6454 comparable URI from the {@code uri} string.
- *
- * @param uri
- * the contents of the Origin or Referer HTTP header
- * @return only the scheme://host[:port] part, or {@code null} when the URI string is not
- * compliant
- */
- protected final String normalizeUri(String uri)
- {
- // the request comes from a privacy sensitive context, flag as non-local origin. If
- // alternative action is required, an implementor can override any of the onAborted,
- // onSuppressed or onAllowed and implement such needed action.
+ /**
+ * Creates a RFC-6454 comparable URI from the {@code uri} string.
+ *
+ * @param uri
+ * the contents of the Origin or Referer HTTP header
+ * @return only the scheme://host[:port] part, or {@code null} when the URI string is not
+ * compliant
+ */
+ protected final String normalizeUri(String uri)
+ {
+ // the request comes from a privacy sensitive context, flag as non-local origin. If
+ // alternative action is required, an implementor can override any of the onAborted,
+ // onSuppressed or onAllowed and implement such needed action.
- if (Strings.isEmpty(uri) || "null".equals(uri))
- return null;
+ if (Strings.isEmpty(uri) || "null".equals(uri))
+ return null;
- StringBuilder target = new StringBuilder();
+ StringBuilder target = new StringBuilder();
- try
- {
- URI originUri = new URI(uri);
- String scheme = originUri.getScheme();
- if (scheme == null)
- {
- return null;
- }
- else
- {
- scheme = scheme.toLowerCase(Locale.ROOT);
- }
+ try
+ {
+ URI originUri = new URI(uri);
+ String scheme = originUri.getScheme();
+ if (scheme == null)
+ {
+ return null;
+ }
+ else
+ {
+ scheme = scheme.toLowerCase(Locale.ROOT);
+ }
- target.append(scheme);
- target.append("://");
+ target.append(scheme);
+ target.append("://");
- String host = originUri.getHost();
- if (host == null)
- {
- return null;
- }
- target.append(host);
+ String host = originUri.getHost();
+ if (host == null)
+ {
+ return null;
+ }
+ target.append(host);
- int port = originUri.getPort();
- boolean portIsSpecified = port != -1;
- boolean isAlternateHttpPort = "http".equals(scheme) && port != 80;
- boolean isAlternateHttpsPort = "https".equals(scheme) && port != 443;
+ int port = originUri.getPort();
+ boolean portIsSpecified = port != -1;
+ boolean isAlternateHttpPort = "http".equals(scheme) && port != 80;
+ boolean isAlternateHttpsPort = "https".equals(scheme) && port != 443;
- if (portIsSpecified && (isAlternateHttpPort || isAlternateHttpsPort))
- {
- target.append(':');
- target.append(port);
- }
- return target.toString();
- }
- catch (URISyntaxException e)
- {
- log.debug("Invalid URI provided: {}, marked conflicting", uri);
- return null;
- }
- }
+ if (portIsSpecified && (isAlternateHttpPort || isAlternateHttpsPort))
+ {
+ target.append(':');
+ target.append(port);
+ }
+ return target.toString();
+ }
+ catch (URISyntaxException e)
+ {
+ log.debug("Invalid URI provided: {}, marked conflicting", uri);
+ return null;
+ }
+ }
- /**
- * Checks whether the domain part of the {@code sourceUri} ({@code Origin} or {@code Referer}
- * header) is whitelisted.
- *
- * @param sourceUri
- * the contents of the {@code Origin} or {@code Referer} HTTP header
- * @return {@code true} when the source domain was whitelisted
- */
- protected boolean isWhitelistedHost(final String sourceUri)
- {
- try
- {
- final String sourceHost = new URI(sourceUri).getHost();
- if (Strings.isEmpty(sourceHost))
- return false;
- for (String whitelistedOrigin : acceptedOrigins)
- {
- if (sourceHost.equalsIgnoreCase(whitelistedOrigin) ||
- sourceHost.endsWith("." + whitelistedOrigin))
- {
- log.trace("Origin {} matched whitelisted origin {}, request accepted",
- sourceUri, whitelistedOrigin);
- return true;
- }
- }
- }
- catch (URISyntaxException e)
- {
- log.debug("Origin: {} not parseable as an URI. Whitelisted-origin check skipped.",
- sourceUri);
- }
+ /**
+ * Checks whether the domain part of the {@code sourceUri} ({@code Origin} or {@code Referer}
+ * header) is whitelisted.
+ *
+ * @param sourceUri
+ * the contents of the {@code Origin} or {@code Referer} HTTP header
+ * @return {@code true} when the source domain was whitelisted
+ */
+ protected boolean isWhitelistedHost(final String sourceUri)
+ {
+ try
+ {
+ final String sourceHost = new URI(sourceUri).getHost();
+ if (Strings.isEmpty(sourceHost))
+ return false;
+ for (String whitelistedOrigin : acceptedOrigins)
+ {
+ if (sourceHost.equalsIgnoreCase(whitelistedOrigin)
+ || sourceHost.endsWith("." + whitelistedOrigin))
+ {
+ log.trace("Origin {} matched whitelisted origin {}, request accepted",
+ sourceUri, whitelistedOrigin);
+ return true;
+ }
+ }
+ }
+ catch (URISyntaxException e)
+ {
+ log.debug("Origin: {} not parseable as an URI. Whitelisted-origin check skipped.",
+ sourceUri);
+ }
- return false;
- }
+ return false;
+ }
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
index 487ed12..f2c290c 100644
--- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
@@ -32,152 +32,163 @@ import org.apache.wicket.util.tester.WicketTestCase;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
-public class FetchMetadataRequestCycleListenerTest extends WicketTestCase {
-
- private FetchMetadataRequestCycleListener fetchMetadataListener;
-
- @BeforeEach
- void before() {
- withCustomListener(new FetchMetadataRequestCycleListener());
- }
-
- void withCustomListener(FetchMetadataRequestCycleListener fetchMetadataListener) {
- WebApplication application = tester.getApplication();
-
- this.fetchMetadataListener = fetchMetadataListener;
- application.getRequestCycleListeners().add(fetchMetadataListener);
-
- tester.startPage(FirstPage.class);
- tester.assertRenderedPage(FirstPage.class);
- }
-
- /**
- * Tests whether a request with Sec-Fetch-Site = cross-site is aborted
- */
- @Test
- void crossSiteFMAborted() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
-
- assertRequestAborted();
- }
-
- /**
- * Tests whether embed requests are aborted by fetch metadata checks
- */
- @Test
- void destEmbedFMAborted() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
- tester.addRequestHeader(SEC_FETCH_DEST_HEADER, DEST_EMBED);
-
- assertRequestAborted();
- }
-
- /**
- * Tests whether object requests (sec-fetch-dest :"object" ) are aborted by FM checks
- */
- @Test
- void destObjectAborted() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
- tester.addRequestHeader(SEC_FETCH_DEST_HEADER, DEST_OBJECT);
-
- assertRequestAborted();
- }
-
- /**
- * Tests whether a top level navigation request is allowed by FM checks
- */
- @Test
- void topLevelNavigationAllowedFM() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_ORIGIN);
- tester.addRequestHeader(SEC_FETCH_MODE_HEADER, MODE_NAVIGATE);
-
- assertRequestAccepted();
- }
-
- /**
- * Tests that requests rejected by fetch metadata have the Vary header set
- */
- @Test
- void varyHeaderSetWhenFetchMetadataRejectsRequest() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
- assertRequestAborted();
-
- String vary = tester.getLastResponse().getHeader("Vary");
-
- if (vary == null) {
- throw new AssertionError("Vary header should not be null");
- }
-
- if (!vary.contains(SEC_FETCH_DEST_HEADER) || !vary.contains(SEC_FETCH_MODE_HEADER)
- || !vary.contains(SEC_FETCH_SITE_HEADER)) {
- throw new AssertionError("Unexpected vary header: " + vary);
- }
- }
-
- /**
- * Tests that requests accepted by fetch metadata have the Vary header set
- */
- @Test
- void varyHeaderSetWhenFetchMetadataAcceptsRequest() {
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE);
- assertRequestAccepted();
-
- String vary = tester.getLastResponse().getHeader(VARY_HEADER);
- if (vary == null) {
- throw new AssertionError("Vary header should not be null");
- }
-
- if (!vary.contains(SEC_FETCH_DEST_HEADER) || !vary.contains(SEC_FETCH_MODE_HEADER)
- || !vary.contains(SEC_FETCH_SITE_HEADER)) {
- throw new AssertionError("Unexpected vary header: " + vary);
- }
- }
-
- @Test
- void whenAtLeastOnePolicyRejectsRequest_thenRequestRejected() {
- fetchMetadataListener = new FetchMetadataRequestCycleListener(
- (request, page) -> true,
- (request, page) -> true,
- (request, page) -> false,
- (request, page) -> true
- );
-
- withCustomListener(fetchMetadataListener);
- assertRequestAborted();
- }
-
- @Test
- void whenAllPoliciesAcceptRequest_thenRequestAccepted() {
- fetchMetadataListener = new FetchMetadataRequestCycleListener(
- (request, page) -> true,
- (request, page) -> true,
- (request, page) -> true,
- (request, page) -> true
- );
-
- withCustomListener(fetchMetadataListener);
- assertRequestAccepted();
- }
-
- @Test
- void whenCrossOriginRequestToExempted_thenRequestAccepted() {
- fetchMetadataListener.addExemptedPaths("/wicket/bookmarkable/org.apache.wicket.protocol.http.FirstPage");
- withCustomListener(fetchMetadataListener);
-
- tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
- assertRequestAccepted();
- }
-
- private void assertRequestAborted() {
- tester.clickLink("link");
- assertEquals(tester.getLastResponse().getStatus(),
- FetchMetadataRequestCycleListener.ERROR_CODE);
- assertEquals(tester.getLastResponse().getErrorMessage(),
- FetchMetadataRequestCycleListener.ERROR_MESSAGE);
- }
-
- private void assertRequestAccepted() {
- tester.clickLink("link");
- tester.assertRenderedPage(SecondPage.class);
- }
+public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
+{
+
+ private FetchMetadataRequestCycleListener fetchMetadataListener;
+
+ @BeforeEach
+ void before()
+ {
+ withCustomListener(new FetchMetadataRequestCycleListener());
+ }
+
+ void withCustomListener(FetchMetadataRequestCycleListener fetchMetadataListener)
+ {
+ WebApplication application = tester.getApplication();
+
+ this.fetchMetadataListener = fetchMetadataListener;
+ application.getRequestCycleListeners().add(fetchMetadataListener);
+
+ tester.startPage(FirstPage.class);
+ tester.assertRenderedPage(FirstPage.class);
+ }
+
+ /**
+ * Tests whether a request with Sec-Fetch-Site = cross-site is aborted
+ */
+ @Test
+ void crossSiteFMAborted()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+
+ assertRequestAborted();
+ }
+
+ /**
+ * Tests whether embed requests are aborted by fetch metadata checks
+ */
+ @Test
+ void destEmbedFMAborted()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+ tester.addRequestHeader(SEC_FETCH_DEST_HEADER, DEST_EMBED);
+
+ assertRequestAborted();
+ }
+
+ /**
+ * Tests whether object requests (sec-fetch-dest :"object" ) are aborted by FM checks
+ */
+ @Test
+ void destObjectAborted()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+ tester.addRequestHeader(SEC_FETCH_DEST_HEADER, DEST_OBJECT);
+
+ assertRequestAborted();
+ }
+
+ /**
+ * Tests whether a top level navigation request is allowed by FM checks
+ */
+ @Test
+ void topLevelNavigationAllowedFM()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_ORIGIN);
+ tester.addRequestHeader(SEC_FETCH_MODE_HEADER, MODE_NAVIGATE);
+
+ assertRequestAccepted();
+ }
+
+ /**
+ * Tests that requests rejected by fetch metadata have the Vary header set
+ */
+ @Test
+ void varyHeaderSetWhenFetchMetadataRejectsRequest()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+ assertRequestAborted();
+
+ String vary = tester.getLastResponse().getHeader("Vary");
+
+ if (vary == null)
+ {
+ throw new AssertionError("Vary header should not be null");
+ }
+
+ if (!vary.contains(SEC_FETCH_DEST_HEADER) || !vary.contains(SEC_FETCH_MODE_HEADER)
+ || !vary.contains(SEC_FETCH_SITE_HEADER))
+ {
+ throw new AssertionError("Unexpected vary header: " + vary);
+ }
+ }
+
+ /**
+ * Tests that requests accepted by fetch metadata have the Vary header set
+ */
+ @Test
+ void varyHeaderSetWhenFetchMetadataAcceptsRequest()
+ {
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, SAME_SITE);
+ assertRequestAccepted();
+
+ String vary = tester.getLastResponse().getHeader(VARY_HEADER);
+ if (vary == null)
+ {
+ throw new AssertionError("Vary header should not be null");
+ }
+
+ if (!vary.contains(SEC_FETCH_DEST_HEADER) || !vary.contains(SEC_FETCH_MODE_HEADER)
+ || !vary.contains(SEC_FETCH_SITE_HEADER))
+ {
+ throw new AssertionError("Unexpected vary header: " + vary);
+ }
+ }
+
+ @Test
+ void whenAtLeastOnePolicyRejectsRequest_thenRequestRejected()
+ {
+ fetchMetadataListener = new FetchMetadataRequestCycleListener((request, page) -> true,
+ (request, page) -> true, (request, page) -> false, (request, page) -> true);
+
+ withCustomListener(fetchMetadataListener);
+ assertRequestAborted();
+ }
+
+ @Test
+ void whenAllPoliciesAcceptRequest_thenRequestAccepted()
+ {
+ fetchMetadataListener = new FetchMetadataRequestCycleListener((request, page) -> true,
+ (request, page) -> true, (request, page) -> true, (request, page) -> true);
+
+ withCustomListener(fetchMetadataListener);
+ assertRequestAccepted();
+ }
+
+ @Test
+ void whenCrossOriginRequestToExempted_thenRequestAccepted()
+ {
+ fetchMetadataListener
+ .addExemptedPaths("/wicket/bookmarkable/org.apache.wicket.protocol.http.FirstPage");
+ withCustomListener(fetchMetadataListener);
+
+ tester.addRequestHeader(SEC_FETCH_SITE_HEADER, CROSS_SITE);
+ assertRequestAccepted();
+ }
+
+ private void assertRequestAborted()
+ {
+ tester.clickLink("link");
+ assertEquals(tester.getLastResponse().getStatus(),
+ FetchMetadataRequestCycleListener.ERROR_CODE);
+ assertEquals(tester.getLastResponse().getErrorMessage(),
+ FetchMetadataRequestCycleListener.ERROR_MESSAGE);
+ }
+
+ private void assertRequestAccepted()
+ {
+ tester.clickLink("link");
+ tester.assertRenderedPage(SecondPage.class);
+ }
}
[wicket] 03/04: Merge branch 'wicket-6786'
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit d01d6a2f4376fe9bec2cdd011259d435bd9f0e3f
Merge: 5fc33a5 c51372b
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Fri Aug 7 22:32:31 2020 +0200
Merge branch 'wicket-6786'
.../http/CsrfPreventionRequestCycleListener.java | 240 ++----------
.../http/DefaultResourceIsolationPolicy.java | 80 ++++
.../http/FetchMetadataRequestCycleListener.java | 415 +++++++++++++++++++++
.../http/OriginBasedResourceIsolationPolicy.java | 283 ++++++++++++++
.../protocol/http/ResourceIsolationOutcome.java | 8 +
.../protocol/http/ResourceIsolationPolicy.java | 56 +++
.../CsrfPreventionRequestCycleListenerTest.java | 69 +++-
.../FetchMetadataRequestCycleListenerTest.java | 200 ++++++++++
8 files changed, 1124 insertions(+), 227 deletions(-)
[wicket] 04/04: WICKET-6786: Fix license header and add some
comments
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit bef3facb3b240f60a3455f257eaf1b9db81a9e29
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Fri Aug 7 22:40:01 2020 +0200
WICKET-6786: Fix license header and add some comments
---
.../protocol/http/ResourceIsolationOutcome.java | 26 +++++++++++++++++++---
1 file changed, 23 insertions(+), 3 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
index 9b7dc38..1250bfc 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
@@ -1,8 +1,28 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF 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
+ *
+ * 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.apache.wicket.protocol.http;
+/**
+ * Indicates the outcome for a resource isolation policy for a request. When the outcome is
+ * {@link #UNKNOWN}, the next policy will be consulted.
+ *
+ * @author papegaaij
+ */
public enum ResourceIsolationOutcome
{
- ALLOWED,
- DISALLOWED,
- UNKNOWN
+ ALLOWED, DISALLOWED, UNKNOWN
}
[wicket] 02/04: WICKET-6786: reintroduced the configuration options
from CsrfPreventionRequestCycleListener
Posted by pa...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
papegaaij pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/wicket.git
commit c51372bd51e67276d86c8cf02336aa04e2f7a403
Author: Emond Papegaaij <em...@topicus.nl>
AuthorDate: Fri Aug 7 13:11:36 2020 +0200
WICKET-6786: reintroduced the configuration options from CsrfPreventionRequestCycleListener
---
.../http/DefaultResourceIsolationPolicy.java | 11 +-
.../http/FetchMetadataRequestCycleListener.java | 321 ++++++++++++++++++---
.../http/OriginBasedResourceIsolationPolicy.java | 10 +-
.../protocol/http/ResourceIsolationOutcome.java | 8 +
.../protocol/http/ResourceIsolationPolicy.java | 3 +-
.../FetchMetadataRequestCycleListenerTest.java | 28 +-
6 files changed, 325 insertions(+), 56 deletions(-)
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
index b1d7cf7..e3de5a1 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/DefaultResourceIsolationPolicy.java
@@ -34,12 +34,13 @@ public class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy
{
@Override
- public boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
+ public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request,
+ IRequestablePage targetPage)
{
// request made by a legacy browser with no support for Fetch Metadata
if (!hasFetchMetadataHeaders(request))
{
- return true;
+ return ResourceIsolationOutcome.UNKNOWN;
}
String site = request.getHeader(SEC_FETCH_SITE_HEADER);
@@ -47,11 +48,13 @@ public class DefaultResourceIsolationPolicy implements ResourceIsolationPolicy
// Allow same-site and browser-initiated requests
if (SAME_ORIGIN.equals(site) || SAME_SITE.equals(site) || NONE.equals(site))
{
- return true;
+ return ResourceIsolationOutcome.ALLOWED;
}
// Allow simple top-level navigations except <object> and <embed>
- return isAllowedTopLevelNavigation(request);
+ return isAllowedTopLevelNavigation(request)
+ ? ResourceIsolationOutcome.ALLOWED
+ : ResourceIsolationOutcome.DISALLOWED;
}
private boolean isAllowedTopLevelNavigation(HttpServletRequest request)
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
index ea9a8ef..d1500b9 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListener.java
@@ -26,9 +26,11 @@ import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
-import java.util.Optional;
import java.util.Set;
+
import javax.servlet.http.HttpServletRequest;
+
+import org.apache.wicket.RestartResponseException;
import org.apache.wicket.core.request.handler.IPageRequestHandler;
import org.apache.wicket.core.request.handler.RenderPageRequestHandler;
import org.apache.wicket.request.IRequestHandler;
@@ -61,23 +63,151 @@ import org.slf4j.LoggerFactory;
*/
public class FetchMetadataRequestCycleListener implements IRequestCycleListener
{
-
private static final Logger log = LoggerFactory
.getLogger(FetchMetadataRequestCycleListener.class);
- public static final int ERROR_CODE = 403;
- public static final String ERROR_MESSAGE = "Forbidden";
+
+ public static final String ERROR_MESSAGE = "The request was blocked by a resource isolation policy";
public static final String VARY_HEADER_VALUE = SEC_FETCH_DEST_HEADER + ", "
+ SEC_FETCH_SITE_HEADER + ", " + SEC_FETCH_MODE_HEADER;
+ /**
+ * The action to perform when the outcome of the resource isolation policy is DISALLOWED or
+ * UNKNOWN.
+ */
+ public enum CsrfAction
+ {
+ /** Aborts the request and throws an exception when a CSRF request is detected. */
+ ABORT {
+ @Override
+ public String toString()
+ {
+ return "aborted";
+ }
+ },
+
+ /**
+ * Ignores the action of a CSRF request, and just renders the page it was targeted against.
+ */
+ SUPPRESS {
+ @Override
+ public String toString()
+ {
+ return "suppressed";
+ }
+ },
+
+ /** Detects a CSRF request, logs it and allows the request to continue. */
+ ALLOW {
+ @Override
+ public String toString()
+ {
+ return "allowed";
+ }
+ },
+ }
+
+ /**
+ * Action to perform when none resource isolation policies can determine the validity of the
+ * request.
+ */
+ private CsrfAction unknownOutcomeAction = CsrfAction.ABORT;
+
+ /**
+ * Action to perform when DISALLOWED is reported by a resource isolation policy.
+ */
+ private CsrfAction disallowedOutcomeAction = CsrfAction.ABORT;
+
+ /**
+ * The error code to report when the action to take for a CSRF request is
+ * {@link CsrfAction#ABORT}. Default {@code 403 FORBIDDEN}.
+ */
+ private int errorCode = javax.servlet.http.HttpServletResponse.SC_FORBIDDEN;
+
+ /**
+ * The error message to report when the action to take for a CSRF request is {@code ERROR}.
+ * Default {@code "The request was blocked by a resource isolation policy"}.
+ */
+ private String errorMessage = ERROR_MESSAGE;
+
+
private final Set<String> exemptedPaths = new HashSet<>();
private final List<ResourceIsolationPolicy> resourceIsolationPolicies = new ArrayList<>();
- public FetchMetadataRequestCycleListener(ResourceIsolationPolicy... additionalPolicies)
+ /**
+ * Create a new FetchMetadataRequestCycleListener with the given policies. If no policies are
+ * given, {@link DefaultResourceIsolationPolicy} and {@link OriginBasedResourceIsolationPolicy}
+ * will be used. The policies are checked in order. The first outcome that's not
+ * {@link ResourceIsolationOutcome#UNKNOWN} will be used.
+ *
+ * @param policies
+ * the policies to check requests against.
+ */
+ public FetchMetadataRequestCycleListener(ResourceIsolationPolicy... policies)
+ {
+ this.resourceIsolationPolicies.addAll(asList(policies));
+ if (policies.length == 0)
+ {
+ this.resourceIsolationPolicies.addAll(asList(new DefaultResourceIsolationPolicy(),
+ new OriginBasedResourceIsolationPolicy()));
+ }
+ }
+
+ /**
+ * Sets the action when none of the resource isolation policies can come to an outcome. Default
+ * {@code ABORT}.
+ *
+ * @param action
+ * the alternate action
+ *
+ * @return this (for chaining)
+ */
+ public FetchMetadataRequestCycleListener setUnknownOutcomeAction(CsrfAction action)
{
- this.resourceIsolationPolicies.addAll(
- asList(new DefaultResourceIsolationPolicy(), new OriginBasedResourceIsolationPolicy()));
+ this.unknownOutcomeAction = action;
+ return this;
+ }
- this.resourceIsolationPolicies.addAll(asList(additionalPolicies));
+ /**
+ * Sets the action when a request is disallowed by a resource isolation policy. Default is
+ * {@code ABORT}.
+ *
+ * @param action
+ * the alternate action
+ *
+ * @return this
+ */
+ public FetchMetadataRequestCycleListener setDisallowedOutcomeAction(CsrfAction action)
+ {
+ this.disallowedOutcomeAction = action;
+ return this;
+ }
+
+ /**
+ * Modifies the HTTP error code in the exception when a disallowed request is detected.
+ *
+ * @param errorCode
+ * the alternate HTTP error code, default {@code 403 FORBIDDEN}
+ *
+ * @return this
+ */
+ public FetchMetadataRequestCycleListener setErrorCode(int errorCode)
+ {
+ this.errorCode = errorCode;
+ return this;
+ }
+
+ /**
+ * Modifies the HTTP message in the exception when a disallowed request is detected.
+ *
+ * @param errorMessage
+ * the alternate message
+ *
+ * @return this
+ */
+ public FetchMetadataRequestCycleListener setErrorMessage(String errorMessage)
+ {
+ this.errorMessage = errorMessage;
+ return this;
}
public void addExemptedPaths(String... exemptions)
@@ -94,39 +224,121 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
log.debug("Processing request to: {}", containerRequest.getPathInfo());
}
+ /**
+ * Dynamic override for enabling/disabling the CSRF detection. Might be handy for specific
+ * tenants in a multi-tenant application. When false, the CSRF detection is not performed for
+ * the running request. Default {@code true}
+ *
+ * @return {@code true} when the CSRF checks need to be performed.
+ */
+ protected boolean isEnabled()
+ {
+ return true;
+ }
+
+ /**
+ * Override to limit whether the request to the specific page should be checked for a possible
+ * CSRF attack.
+ *
+ * @param targetedPage
+ * the page that is the target for the action
+ * @return {@code true} when the request to the page should be checked for CSRF issues.
+ */
+ protected boolean isChecked(IRequestablePage targetedPage)
+ {
+ return true;
+ }
+
+ /**
+ * Override to change the request handler types that are checked. Currently only action handlers
+ * (form submits, link clicks, AJAX events) are checked for a matching Origin HTTP header.
+ *
+ * @param handler
+ * the handler that is currently processing
+ * @return true when the Origin HTTP header should be checked for this {@code handler}
+ */
+ protected boolean isChecked(IRequestHandler handler)
+ {
+ return handler instanceof IPageRequestHandler
+ && !(handler instanceof RenderPageRequestHandler);
+ }
+
@Override
public void onRequestHandlerResolved(RequestCycle cycle, IRequestHandler handler)
{
- handler = unwrap(handler);
- IPageRequestHandler pageRequestHandler = getPageRequestHandler(handler);
- if (pageRequestHandler == null)
+ if (!isEnabled())
{
+ log.trace("CSRF listener is disabled, no checks performed");
return;
}
- IRequestablePage targetedPage = pageRequestHandler.getPage();
- HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
- .getContainerRequest();
-
- String pathInfo = containerRequest.getPathInfo();
- if (exemptedPaths.contains(pathInfo))
+ handler = unwrap(handler);
+ if (isChecked(handler))
{
- if (log.isDebugEnabled())
+ IPageRequestHandler pageRequestHandler = (IPageRequestHandler)handler;
+ IRequestablePage targetedPage = pageRequestHandler.getPage();
+ HttpServletRequest containerRequest = (HttpServletRequest)cycle.getRequest()
+ .getContainerRequest();
+
+ if (!isChecked(targetedPage))
{
- log.debug("Allowing request to {} because it matches an exempted path",
- new Object[] { pathInfo });
+ if (log.isDebugEnabled())
+ {
+ log.debug("Targeted page {} was opted out of the CSRF origin checks, allowed",
+ targetedPage.getClass().getName());
+ }
+ return;
}
- return;
- }
- for (ResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies)
- {
- if (!resourceIsolationPolicy.isRequestAllowed(containerRequest, targetedPage))
+ String pathInfo = containerRequest.getPathInfo();
+ if (exemptedPaths.contains(pathInfo))
{
- log.debug("Isolation policy {} has rejected a request to {}",
- Classes.simpleName(resourceIsolationPolicy.getClass()), pathInfo);
- throw new AbortWithHttpErrorCodeException(ERROR_CODE, ERROR_MESSAGE);
+ if (log.isDebugEnabled())
+ {
+ log.debug("Allowing request to {} because it matches an exempted path",
+ new Object[] { pathInfo });
+ }
+ return;
}
+
+ for (ResourceIsolationPolicy resourceIsolationPolicy : resourceIsolationPolicies)
+ {
+ ResourceIsolationOutcome outcome = resourceIsolationPolicy
+ .isRequestAllowed(containerRequest, targetedPage);
+ if (ResourceIsolationOutcome.DISALLOWED.equals(outcome))
+ {
+ log.debug("Isolation policy {} has rejected a request to {}",
+ Classes.simpleName(resourceIsolationPolicy.getClass()), pathInfo);
+ triggerAction(disallowedOutcomeAction, containerRequest, targetedPage);
+ }
+ else if (ResourceIsolationOutcome.ALLOWED.equals(outcome))
+ {
+ return;
+ }
+ }
+ triggerAction(unknownOutcomeAction, containerRequest, targetedPage);
+ }
+ else
+ {
+ if (log.isTraceEnabled())
+ log.trace("Resolved handler {} is not checked, no CSRF check performed",
+ handler.getClass().getName());
+ }
+ }
+
+ private void triggerAction(CsrfAction action, HttpServletRequest request, IRequestablePage page)
+ {
+ switch (action)
+ {
+ case ALLOW :
+ allowHandler(request, page);
+ break;
+ case SUPPRESS :
+ suppressHandler(request, page);
+ break;
+ case ABORT :
+ abortHandler(request, page);
+ break;
}
}
@@ -146,6 +358,52 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
}
}
+ /**
+ * Handles the case where the resource isolation policies resulted in
+ * {@link ResourceIsolationOutcome#UNKNOWN} or {@link ResourceIsolationOutcome#DISALLOWED} and
+ * the action was set to {#link {@link CsrfAction#ALLOW}.
+ *
+ * @param request
+ * the request
+ * @param page
+ * the page that is targeted with this request
+ */
+ protected void allowHandler(HttpServletRequest request, IRequestablePage page)
+ {
+ log.info("Possible CSRF attack, request URL: {}, action: allowed", request.getRequestURL());
+ }
+
+ /**
+ * Supresses the execution of the listener in the request because the outcome results in
+ * {@link CsrfAction#SUPPRESS}.
+ *
+ * @param request
+ * the request
+ * @param page
+ * the page that is targeted with this request
+ */
+ protected void suppressHandler(HttpServletRequest request, IRequestablePage page)
+ {
+ log.info("Possible CSRF attack, request URL: {}, action: suppressed",
+ request.getRequestURL());
+ throw new RestartResponseException(page);
+ }
+
+ /**
+ * Aborts the request because the outcome results in {@link CsrfAction#ABORT}.
+ *
+ * @param request
+ * the request
+ * @param page
+ * the page that is targeted with this request
+ */
+ protected void abortHandler(HttpServletRequest request, IRequestablePage page)
+ {
+ log.info("Possible CSRF attack, request URL: {}, action: aborted with error {} {}",
+ request.getRequestURL(), errorCode, errorMessage);
+ throw new AbortWithHttpErrorCodeException(errorCode, errorMessage);
+ }
+
private static IRequestHandler unwrap(IRequestHandler handler)
{
while (handler instanceof IRequestHandlerDelegate)
@@ -154,11 +412,4 @@ public class FetchMetadataRequestCycleListener implements IRequestCycleListener
}
return handler;
}
-
- private IPageRequestHandler getPageRequestHandler(IRequestHandler handler)
- {
- boolean isPageRequestHandler = handler instanceof IPageRequestHandler
- && !(handler instanceof RenderPageRequestHandler);
- return isPageRequestHandler ? (IPageRequestHandler)handler : null;
- }
}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
index bf07276..3df6bbf 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/OriginBasedResourceIsolationPolicy.java
@@ -76,31 +76,31 @@ public class OriginBasedResourceIsolationPolicy implements ResourceIsolationPoli
* @return whether the request is allowed based on its origin
*/
@Override
- public boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
+ public ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage)
{
String sourceUri = getSourceUri(request);
if (sourceUri == null || sourceUri.isEmpty())
{
log.debug("Source URI not present in request to {}", request.getPathInfo());
- return true;
+ return ResourceIsolationOutcome.UNKNOWN;
}
sourceUri = sourceUri.toLowerCase(Locale.ROOT);
// if the origin is a know and trusted origin, don't check any further but allow the request
if (isWhitelistedHost(sourceUri))
{
- return true;
+ return ResourceIsolationOutcome.ALLOWED;
}
// check if the origin HTTP header matches the request URI
if (!isLocalOrigin(request, sourceUri))
{
log.debug("Source URI conflicts with request origin");
- return false;
+ return ResourceIsolationOutcome.DISALLOWED;
}
- return true;
+ return ResourceIsolationOutcome.ALLOWED;
}
/**
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
new file mode 100644
index 0000000..9b7dc38
--- /dev/null
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationOutcome.java
@@ -0,0 +1,8 @@
+package org.apache.wicket.protocol.http;
+
+public enum ResourceIsolationOutcome
+{
+ ALLOWED,
+ DISALLOWED,
+ UNKNOWN
+}
diff --git a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
index 1b68df1..6995cb2 100644
--- a/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
+++ b/wicket-core/src/main/java/org/apache/wicket/protocol/http/ResourceIsolationPolicy.java
@@ -51,5 +51,6 @@ public interface ResourceIsolationPolicy
String DEST_SCRIPT = "script";
String DEST_IMAGE = "image";
- boolean isRequestAllowed(HttpServletRequest request, IRequestablePage targetPage);
+ ResourceIsolationOutcome isRequestAllowed(HttpServletRequest request,
+ IRequestablePage targetPage);
}
diff --git a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java b/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
index f2c290c..d1a5960 100644
--- a/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
+++ b/wicket-core/src/test/java/org/apache/wicket/protocol/http/FetchMetadataRequestCycleListenerTest.java
@@ -47,6 +47,10 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
{
WebApplication application = tester.getApplication();
+ if (this.fetchMetadataListener != null)
+ {
+ application.getRequestCycleListeners().remove(this.fetchMetadataListener);
+ }
this.fetchMetadataListener = fetchMetadataListener;
application.getRequestCycleListeners().add(fetchMetadataListener);
@@ -147,22 +151,24 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
}
@Test
- void whenAtLeastOnePolicyRejectsRequest_thenRequestRejected()
+ void whenAtFirstNotUnkownRejectsRequest_thenRequestRejected()
{
- fetchMetadataListener = new FetchMetadataRequestCycleListener((request, page) -> true,
- (request, page) -> true, (request, page) -> false, (request, page) -> true);
-
- withCustomListener(fetchMetadataListener);
+ withCustomListener(new FetchMetadataRequestCycleListener(
+ (request, page) -> ResourceIsolationOutcome.UNKNOWN,
+ (request, page) -> ResourceIsolationOutcome.UNKNOWN,
+ (request, page) -> ResourceIsolationOutcome.DISALLOWED,
+ (request, page) -> ResourceIsolationOutcome.ALLOWED));
assertRequestAborted();
}
@Test
- void whenAllPoliciesAcceptRequest_thenRequestAccepted()
+ void whenFirstNotUnknownPolicieAcceptRequest_thenRequestAccepted()
{
- fetchMetadataListener = new FetchMetadataRequestCycleListener((request, page) -> true,
- (request, page) -> true, (request, page) -> true, (request, page) -> true);
-
- withCustomListener(fetchMetadataListener);
+ withCustomListener(new FetchMetadataRequestCycleListener(
+ (request, page) -> ResourceIsolationOutcome.UNKNOWN,
+ (request, page) -> ResourceIsolationOutcome.ALLOWED,
+ (request, page) -> ResourceIsolationOutcome.ALLOWED,
+ (request, page) -> ResourceIsolationOutcome.ALLOWED));
assertRequestAccepted();
}
@@ -181,7 +187,7 @@ public class FetchMetadataRequestCycleListenerTest extends WicketTestCase
{
tester.clickLink("link");
assertEquals(tester.getLastResponse().getStatus(),
- FetchMetadataRequestCycleListener.ERROR_CODE);
+ javax.servlet.http.HttpServletResponse.SC_FORBIDDEN);
assertEquals(tester.getLastResponse().getErrorMessage(),
FetchMetadataRequestCycleListener.ERROR_MESSAGE);
}