Juan Hernandez has uploaded a new change for review.

Change subject: restapi: Add CSRF protection filter
......................................................................

restapi: Add CSRF protection filter

Currently we don't have any mechanism to protect the RESTAPI from CSRF
attacks. This patch adds a filter that increases the protection from
this attacks requiring callers to include a copy of the session
identifier in a request header. This way only applications that have
legitimately obtained a session cookie will be able to send valid
requests. A valid request will now look like this:

  GET /ovirt-engine/api/... HTTP/1.1
  Host: ovirt.example.com
  Cookie: JSESSIONID=ia3SKGzjiOOQJMdvZigKQROO.undefined
  JSESSIONID: ia3SKGzjiOOQJMdvZigKQROO.undefined

Requests that don't contain this JSESSIONID header will be rejected with
error code 403 (forbidden). In addition a warning message will be sent
to the log explaining that the request has been rejected because it
looks like a CSRF attack:

  2014-04-08 19:56:37,387 WARN
  [org.ovirt.engine.api.restapi.security.CSRFProtectionFilter]
  (ajp--127.0.0.1-8702-2) Request for "/ovirt-engine/api/events" from
  address "192.168.122.2" has been rejected because it isn't trusted
  and it contains a session cookie but no session header, it's
  probably a CSRF attack.

This change will break backwards compatibility with old callers, as they
aren't aware of the new requirement. To handle that the filter uses a
configurable script that decides if a request is trusted. Trusted
requests don't need to use the JSESSIONID header. This script doesn't
exist by default, so no request will be trusted by default. In order to
use it the administrator will have to create it. The default location is
/etc/ovirt-engine/restapi-csrf-trust.js, but can be changed with the
RESTAPI_CSRF_TRUST_SCRIPT parameter:

  # cat >/etc/ovirt-engine/engine.conf.d/99-csrf-protection.conf <<.
  RESTAPI_CSRF_TRUST_SCRIPT=/etc/ovirt-engine/my.js
  .

The script must contain a "isTrusted" function that receives the HTTP
request as parameter and returns a boolean indicating if the request is
trusted. For example, the following script disables completely the CSRF
protection:

  function isTrusted(request) {
    return true;
  }

The following script disables the CSRF protection but only for the
callers from a certain IP address:

  function isTrusted(request) {
    return request.getRemoteAddr() == "192.168.122.2";
  }

Change-Id: I68f03eeefe5bcb1956036b4a80fef4400c467346
Signed-off-by: Juan Hernandez <[email protected]>
---
M backend/manager/modules/restapi/jaxrs/pom.xml
A 
backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/CSRFProtectionFilter.java
M backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
M packaging/services/ovirt-engine/ovirt-engine.conf.in
4 files changed, 287 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/78/26578/1

diff --git a/backend/manager/modules/restapi/jaxrs/pom.xml 
b/backend/manager/modules/restapi/jaxrs/pom.xml
index 7044908..6aee681 100644
--- a/backend/manager/modules/restapi/jaxrs/pom.xml
+++ b/backend/manager/modules/restapi/jaxrs/pom.xml
@@ -68,6 +68,16 @@
     </dependency>
 
     <dependency>
+      <groupId>org.slf4j</groupId>
+      <artifactId>slf4j-api</artifactId>
+    </dependency>
+
+    <dependency>
+      <groupId>commons-lang</groupId>
+      <artifactId>commons-lang</artifactId>
+    </dependency>
+
+    <dependency>
       <groupId>org.easymock</groupId>
       <artifactId>easymockclassextension</artifactId>
       <version>${easymock.version}</version>
diff --git 
a/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/CSRFProtectionFilter.java
 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/CSRFProtectionFilter.java
new file mode 100644
index 0000000..a8789a1
--- /dev/null
+++ 
b/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/security/CSRFProtectionFilter.java
@@ -0,0 +1,237 @@
+/*
+* Copyright (c) 2014 Red Hat, Inc.
+*
+* Licensed 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.ovirt.engine.api.restapi.security;
+
+import java.io.File;
+import java.io.FileInputStream;
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.io.Reader;
+import javax.script.Invocable;
+import javax.script.ScriptEngine;
+import javax.script.ScriptEngineManager;
+import javax.script.ScriptException;
+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 javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpSession;
+
+import org.apache.commons.lang.ObjectUtils;
+import org.ovirt.engine.core.utils.EngineLocalConfig;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This filter checks that the request contains matching session cookies and 
header, except for those requests that
+ * are maked as trusted by the trust script. In order to configure the filter 
add the following to the the
+ * {@code web.xml} file of the application:
+ *
+ * <pre>
+ * &lt;filter&gt;
+ *   &lt;filter-name&gt;CSRFProtectionFilter&lt;/filter-name&gt;
+ *   
&lt;filter-class&gt;org.ovirt.engine.api.restapi.security.auth.CSRFProtectionFilter&lt;/filter-class&gt;
+ *   &lt;init-param&gt;
+ *     &lt;param-name&gt;trust-script&lt;/param-name&gt;
+ *     
&lt;param-value&gt;/etc/ovirt-engine/restapi-trust.js&lt;/param-value&gt;
+ *   &lt;/init-param&gt;
+ * &lt;/filter&gt;
+ * &lt;filter-mapping&gt;
+ *   &lt;filter-name&gt;CSRFProtectionFilter&lt;/filter-name&gt;
+ *   &lt;url-pattern&gt;/*&lt;/url-pattern&gt;
+ * &lt;/filter-mapping&gt;
+ * </pre>
+ *
+ * The {@code trust-script} parameter is optional. If given it must be the 
absolute path name of a JavaScript script
+ * containing a function named {@code isTrusted} that receives the request as 
parameter and returns a boolean that
+ * indicates if the request is trusted. Trusted requests don't need to include 
the session id in the {@code JSESSIONID}
+ * header. For example, the following script will completely disable the CSRF 
protection:
+ *
+ * <pre>
+ * function isTrusted(request) {
+ *     return true;
+ * }
+ * </pre>
+ *
+ * A typical requirement will be to disable the CSRF protection only for a 
particular source IP address where we now
+ * a trusted application is running:
+ *
+ * <pre>
+ * function isTrusted(request) {
+ *     return request.getRemoteAddr() == "192.168.122.2";
+ * }
+ * </pre>
+ */
+@SuppressWarnings("unused")
+public class CSRFProtectionFilter implements Filter {
+    /**
+     * The logger used by the class.
+     */
+    private static final Logger log = 
LoggerFactory.getLogger(CSRFProtectionFilter.class);
+
+    /**
+     * The name of the parameter that contains the location of the trust 
script.
+     */
+    private static final String TRUST_SCRIPT_PARAMETER_NAME = "trust-script";
+
+    /**
+     * The name of the header that should contain the session identifier.
+     */
+    private static final String SESSION_ID_HEADER_NAME = "JSESSIONID";
+
+    /**
+     * The name of the function inside the script used to check if the request 
is trusted.
+     */
+    private static final String IS_TRUSTED_FUNCTION_NAME = "isTrusted";
+
+    /**
+     * The script containing the functions used by the filter.
+     */
+    private Invocable trustScript;
+
+    @Override
+    public void init(FilterConfig config) throws ServletException {
+        // Find the location of the trust script file:
+        String trustScriptPath = 
config.getInitParameter(TRUST_SCRIPT_PARAMETER_NAME);
+        if (trustScriptPath == null) {
+            log.info(
+                "The trust script parameter \"{}\" doesn't have a value, no 
request will be trusted.",
+                TRUST_SCRIPT_PARAMETER_NAME
+            );
+            return;
+        }
+        trustScriptPath = 
EngineLocalConfig.getInstance().expandString(trustScriptPath.replaceAll("%\\{", 
"\\${"));
+
+        // Check that the trust script exists and is accessible:
+        File trustScriptFile = new File(trustScriptPath);
+        if (!trustScriptFile.exists() || !trustScriptFile.canRead()) {
+            log.info(
+                "The trust script \"{}\" doesn't exist or can't be read, no 
request will be trusted.",
+                trustScriptFile.getAbsolutePath()
+            );
+            return;
+        }
+
+        // Create the scripting engine:
+        ScriptEngineManager scriptManager = new ScriptEngineManager();
+        ScriptEngine scriptEngine = 
scriptManager.getEngineByName("JavaScript");
+        if (scriptEngine == null) {
+            log.error(
+                "Can't create the scripting engine needed to execute trust 
script \"{}\", no request will be trusted.",
+                trustScriptFile.getAbsolutePath()
+            );
+            return;
+        }
+
+        // Evaluate the script, so that the required functions are defined:
+        try (Reader scriptReader = new InputStreamReader(new 
FileInputStream(trustScriptFile), "UTF-8")) {
+            scriptEngine.eval(scriptReader);
+        }
+        catch (IOException|ScriptException exception) {
+            log.error(
+                "Execution of the trust script file \"{}\" failed, no request 
will be trusted.",
+                trustScriptFile.getAbsolutePath(), exception
+            );
+            return;
+        }
+
+        // Save the reference to the object that allows us to invoke functions 
contained in the script:
+        trustScript = (Invocable) scriptEngine;
+    }
+
+    @Override
+    public void destroy() {
+    }
+
+    @Override
+    public void doFilter(ServletRequest request, ServletResponse response, 
FilterChain chain)
+        throws IOException, ServletException {
+        doFilter((HttpServletRequest) request, (HttpServletResponse) response, 
chain);
+    }
+
+    private void doFilter(HttpServletRequest request, HttpServletResponse 
response, FilterChain chain)
+        throws IOException, ServletException {
+        // Nothing to check if there isn't a session yet:
+        HttpSession session = request.getSession(false);
+        if (session == null) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        // Nothing to check if there if the trust script declares the request 
trusted:
+        if (isTrusted(request)) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        // Check that the header containing the session is included in the 
request:
+        String headerId = request.getHeader(SESSION_ID_HEADER_NAME);
+        if (headerId == null) {
+            log.warn(
+                "Request for \"{}\" from address \"{}\" has been rejected 
because it isn't trusted and it contains " +
+                "a session cookie but no session header, it's probably a CSRF 
attack.",
+                request.getContextPath() + request.getPathInfo(), 
request.getRemoteAddr()
+            );
+            response.sendError(HttpServletResponse.SC_FORBIDDEN);
+            return;
+        }
+
+        // Check that the session id from the request matches the actual 
session id:
+        String sessionId = session.getId();
+        if (!ObjectUtils.equals(sessionId, headerId)) {
+            log.warn(
+                "Request for \"{}\" from address \"{}\" has been rejected 
because it ins't trusted and it contains " +
+                "a session cookie and a session header with different values, 
it's probably a CSRF attack.",
+                request.getContextPath() + request.getPathInfo(), 
request.getRemoteAddr()
+            );
+            response.sendError(HttpServletResponse.SC_FORBIDDEN);
+            return;
+        }
+
+        // All checks passed, let the request go to the next filter:
+        chain.doFilter(request, response);
+    }
+
+    /**
+     * Checks if the given request is trusted according to the trust script.
+     *
+     * @param request the request to check
+     * @return {@code true} iff the script didn't fail and it returned {@code 
true}, {@code false} otherwise
+     */
+    private boolean isTrusted(HttpServletRequest request) {
+        boolean isTrusted = false;
+        if (trustScript != null) {
+            try {
+                Object scriptResult = 
trustScript.invokeFunction(IS_TRUSTED_FUNCTION_NAME, request);
+                if (scriptResult != null && scriptResult instanceof Boolean) {
+                    isTrusted = (boolean) scriptResult;
+                }
+            }
+            catch (Exception exception) {
+                log.error(
+                    "Execution of the trust function \"{}\" failed, the 
request won't be trusted.",
+                    IS_TRUSTED_FUNCTION_NAME, exception
+                );
+            }
+        }
+        return isTrusted;
+    }
+}
diff --git 
a/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml 
b/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
index 89b79f1..5457f63 100644
--- a/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
+++ b/backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
@@ -22,6 +22,20 @@
     <url-pattern>/*</url-pattern>
   </filter-mapping>
 
+  <!-- Reject requests that don't contain a CSRF protection token: -->
+  <filter>
+    <filter-name>CSRFProtectionFilter</filter-name>
+    
<filter-class>org.ovirt.engine.api.restapi.security.CSRFProtectionFilter</filter-class>
+    <init-param>
+      <param-name>trust-script</param-name>
+      <param-value>%{RESTAPI_CSRF_TRUST_SCRIPT}</param-value>
+    </init-param>
+  </filter>
+  <filter-mapping>
+    <filter-name>CSRFProtectionFilter</filter-name>
+    <url-pattern>/*</url-pattern>
+  </filter-mapping>
+
   <!-- confidentiality -->
   <security-constraint>
     <web-resource-collection>
diff --git a/packaging/services/ovirt-engine/ovirt-engine.conf.in 
b/packaging/services/ovirt-engine/ovirt-engine.conf.in
index f86cf84..915660e 100644
--- a/packaging/services/ovirt-engine/ovirt-engine.conf.in
+++ b/packaging/services/ovirt-engine/ovirt-engine.conf.in
@@ -214,3 +214,29 @@
 
 
 ENGINE_EXTENSION_PATH="${ENGINE_USR}/extensions.d:${ENGINE_ETC}/extensions.d"
+
+#
+# Location of the script used by the CSRF protection filter to check if
+# requests are trusted. This file is optional and should contain a JavaScript
+# script with a function named "isTrusted" that receives an HTTP request and
+# returns a boolean indicating if the request is trusted. For example, the
+# following script will efectively disable the CSRF protection:
+#
+#   function isTrusted(request) {
+#     return true;
+#   }
+#
+# A typical use case is to consider trusted the requests coming from a
+# particular IP address, in this case the a script similar to the following can
+# be used:
+#
+#   function isTrusted(request) {
+#     return request.getRemoteAddr() == "192.268.122.2"
+#   }
+#
+# If no script is provided, or it doesn't exist, then no request will be
+# trusted, and the caller will need to include the value of the session cookie
+# in a header named JSESSIONID, otherwise the request will be rejected with
+# error code 403.
+#
+RESTAPI_CSRF_TRUST_SCRIPT="${ENGINE_ETC}/restapi-csrf-trust.js"


-- 
To view, visit http://gerrit.ovirt.org/26578
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I68f03eeefe5bcb1956036b4a80fef4400c467346
Gerrit-PatchSet: 1
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Juan Hernandez <[email protected]>
_______________________________________________
Engine-patches mailing list
[email protected]
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to