Skip to content
Permalink
Browse files Browse the repository at this point in the history
XWIKI-10309: Check URL domains based on a whitelist (#1592)
Introduce a new property for listing the trusted domains and API to
check an URL against that list and the aliases used in subwikis.

  * Add new property url.trustedDomains in xwiki.properties
  * Add new API in URLConfiguration to retrieve this configuration value
  * Create a new URLSecurityManager responsible to check if an URL can
    be trusted based on this property and on the subwikis configurations
  * Introduce a new listener to invalidate the cache of
    URLSecurityManager whenever a XWikiServerClass xobject is
added/updated/deleted
  * Move URL API implementations to URL default module
  * Add a new property url.enableTrustedDomains as a global switch off the
    checks on domains to avoid breaking behaviours on existing instances
  * Add a constant property in URLSecurityManager to be set in
    ExecutionContext to allow temporary switch off the check for
extensions
  * Use both those switches in DefaultURLSecurityManager to prevent
    performing the check when needed
  • Loading branch information
surli committed Apr 15, 2021
1 parent 5ce100d commit 5251c02
Show file tree
Hide file tree
Showing 40 changed files with 640 additions and 9 deletions.
Expand Up @@ -41,6 +41,8 @@
import org.xwiki.security.authentication.AuthenticationFailureManager;
import com.xpn.xwiki.internal.user.UserAuthenticatedEventNotifier;

import com.xpn.xwiki.web.XWikiResponse;

public class MyFormAuthenticator extends FormAuthenticator implements XWikiAuthenticator
{
private static final Logger LOGGER = LoggerFactory.getLogger(MyFormAuthenticator.class);
Expand Down Expand Up @@ -244,8 +246,8 @@ public boolean processLogin(String username, String password, String rememberme,

Boolean bAjax = (Boolean) context.get("ajax");
if ((bAjax == null) || (!bAjax.booleanValue())) {
String continueToURL = getContinueToURL(request);
// This is the url that the user was initially accessing before being prompted for login.
String continueToURL = getContinueToURL(request);
response.sendRedirect(response.encodeRedirectURL(continueToURL));
}
} else {
Expand Down
Expand Up @@ -981,6 +981,12 @@ protected boolean sendGlobalRedirect(XWikiResponse response, String url, XWikiCo
return false;
}

/**
* Perform a redirect to the given URL.
* @param response the response to use to perform the redirect
* @param url the location of the redirect
* @throws XWikiException in case of IOException when performing the redirect.
*/
protected void sendRedirect(XWikiResponse response, String url) throws XWikiException
{
try {
Expand Down
Expand Up @@ -21,8 +21,10 @@

import java.io.IOException;
import java.io.PrintWriter;
import java.net.URL;
import java.util.Collection;
import java.util.Locale;
import java.util.regex.Pattern;

import javax.servlet.ServletOutputStream;
import javax.servlet.http.Cookie;
Expand All @@ -31,10 +33,12 @@
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import org.xwiki.url.URLSecurityManager;

public class XWikiServletResponse implements XWikiResponse
{
private static final Logger LOGGER = LoggerFactory.getLogger(XWikiServletResponse.class);
private static final Pattern ABSOLUTE_URL_PATTERN = Pattern.compile("[a-z0-9]+://.*");

private HttpServletResponse response;

Expand Down Expand Up @@ -66,9 +70,25 @@ public void sendRedirect(String redirect) throws IOException
LOGGER.warn("Possible HTTP Response Splitting attack, attempting to redirect to [{}]", redirect);
return;
}

// check for trusted domains, only if the given location is an absolute URL.
if (ABSOLUTE_URL_PATTERN.matcher(redirect).matches()) {
if (!getURLSecurityManager().isDomainTrusted(new URL(redirect))) {
LOGGER.warn(
"Possible phishing attack, attempting to redirect to [{}], this request has been blocked. "
+ "If the request was legitimate, add the domain related to this request in the list "
+ "of trusted domains in the configuration.", redirect);
return;
}
}
this.response.sendRedirect(redirect);
}

private URLSecurityManager getURLSecurityManager()
{
return Utils.getComponent(URLSecurityManager.class);
}

@Override
public void setContentType(String type)
{
Expand Down
1 change: 1 addition & 0 deletions xwiki-platform-core/xwiki-platform-url/pom.xml
Expand Up @@ -35,6 +35,7 @@
<!-- Sorted Alphabetically -->
<module>xwiki-platform-url-api</module>
<module>xwiki-platform-url-container</module>
<module>xwiki-platform-url-default</module>
<module>xwiki-platform-url-schemes</module>
</modules>
</project>
Expand Up @@ -32,9 +32,7 @@
<packaging>jar</packaging>
<description>Allows configuration of the URL scheme used by XWiki to parse/serialize URLs</description>
<properties>
<!-- The reason for this low TPC value is because this module is tested using integration tests in the various
URL Scheme modules -->
<xwiki.jacoco.instructionRatio>0.45</xwiki.jacoco.instructionRatio>
<xwiki.jacoco.instructionRatio>0.85</xwiki.jacoco.instructionRatio>
<!-- Name to display by the Extension Manager -->
<xwiki.extension.name>URL API</xwiki.extension.name>
</properties>
Expand Down
Expand Up @@ -19,7 +19,11 @@
*/
package org.xwiki.url;

import java.util.Collections;
import java.util.List;

import org.xwiki.component.annotation.Role;
import org.xwiki.stability.Unstable;

/**
* Configuration options for the URL module.
Expand Down Expand Up @@ -47,4 +51,30 @@ default boolean useResourceLastModificationDate()
{
return true;
}

/**
* Specify the list of domains that are considered as trusted by the administrators of the wiki: those domains can
* be used safely for redirections from the wiki or for performing other requests on them.
* @return the list of trusted domains that can be used in the wiki.
* @since 13.3RC1
* @since 12.10.7
*/
@Unstable
default List<String> getTrustedDomains()
{
return Collections.emptyList();
}

/**
* Define if the trusted domains check should be performed or not. This option is provided only to allow bypassing
* security checks globally on the wiki in case of problems.
* @return {@code true} if the security check on domains should be performed. {@code false} otherwise.
* @since 13.3RC1
* @since 12.10.7
*/
@Unstable
default boolean isTrustedDomainsEnabled()
{
return true;
}
}
@@ -0,0 +1,56 @@
/*
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
*/
package org.xwiki.url;

import java.net.URL;

import org.xwiki.component.annotation.Role;
import org.xwiki.stability.Unstable;

/**
* Dedicated component to perform security checks on URLs.
*
* @version $Id$
* @since 13.3RC1
* @since 12.10.7
*/
@Role
@Unstable
public interface URLSecurityManager
{
/**
* Constant to be used in {@link org.xwiki.context.ExecutionContext} with the value {@code "true"} to bypass a
* check of {@link #isDomainTrusted(URL)}.
*/
String BYPASS_DOMAIN_SECURITY_CHECK_CONTEXT_PROPERTY = "bypassDomainSecurityCheck";

/**
* Check if the given {@link URL} can be trusted based on the trusted domains of the wiki.
* This method check on both the list of trusted domains given by the configuration
* (see {@link URLConfiguration#getTrustedDomains()}) and the list of aliases used by the wiki descriptors.
* Note that this method always returns {@code true} if {@link URLConfiguration#isTrustedDomainsEnabled()} returns
* {@code true}. Also the method will return {@code true} whenever the {@link org.xwiki.context.ExecutionContext}
* contains a property named {@link #BYPASS_DOMAIN_SECURITY_CHECK_CONTEXT_PROPERTY} with the value {@code "true"}.
*
* @param urlToCheck the URL for which we want to know if the domain is trusted or not.
* @return {@code true} if the URL domain can be trusted or if the check is skipped, {@code false} otherwise
*/
boolean isDomainTrusted(URL urlToCheck);
}
@@ -0,0 +1,63 @@
<?xml version="1.0" encoding="UTF-8"?>

<!--
* See the NOTICE file distributed with this work for additional
* information regarding copyright ownership.
*
* This is free software; you can redistribute it and/or modify it
* under the terms of the GNU Lesser General Public License as
* published by the Free Software Foundation; either version 2.1 of
* the License, or (at your option) any later version.
*
* This software is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
* Lesser General Public License for more details.
*
* You should have received a copy of the GNU Lesser General Public
* License along with this software; if not, write to the Free
* Software Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA
* 02110-1301 USA, or see the FSF site: http://www.fsf.org.
-->

<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-url</artifactId>
<version>13.3-SNAPSHOT</version>
</parent>
<artifactId>xwiki-platform-url-default</artifactId>
<name>XWiki Platform - URL - Default</name>
<packaging>jar</packaging>
<description>Default implementations of the API defined in xwiki-platform-url-api</description>
<properties>
<!-- The reason for this low TPC value is because this module is tested using integration tests in the various
URL Scheme modules -->
<xwiki.jacoco.instructionRatio>0.31</xwiki.jacoco.instructionRatio>
</properties>
<dependencies>
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-url-api</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-oldcore</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>org.xwiki.platform</groupId>
<artifactId>xwiki-platform-wiki-api</artifactId>
<version>${project.version}</version>
</dependency>
<!-- Testing Dependencies -->
<dependency>
<groupId>org.xwiki.commons</groupId>
<artifactId>xwiki-commons-tool-test-component</artifactId>
<version>${commons.version}</version>
<scope>test</scope>
</dependency>
</dependencies>
</project>
Expand Up @@ -19,6 +19,9 @@
*/
package org.xwiki.url.internal;

import java.util.Collections;
import java.util.List;

import javax.inject.Inject;
import javax.inject.Provider;
import javax.inject.Singleton;
Expand Down Expand Up @@ -60,4 +63,16 @@ public boolean useResourceLastModificationDate()
{
return this.configuration.get().getProperty(PREFIX + "useResourceLastModificationDate", true);
}

@Override
public List<String> getTrustedDomains()
{
return this.configuration.get().getProperty(PREFIX + "trustedDomains", Collections.emptyList());
}

@Override
public boolean isTrustedDomainsEnabled()
{
return this.configuration.get().getProperty(PREFIX + "trustedDomainsEnabled", true);
}
}

0 comments on commit 5251c02

Please sign in to comment.