Juan Hernandez has uploaded a new change for review.

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

restapi: CSRF protection filter

This patch introduces a filter that protects the RESTAPI from CSRF
attacks. Protection is enabled/disabled globally, using the new
CSRFProtection configuration parameter. By default this parameter is
"false", so the protection isn't enabled. This can be changed with the
"engine-config" tool, as follows:

  # engine-config -s CSRFProtection=true

If the protection is enabled globally, then the caller can enable for a
particular session using the "csrf-protection" preference:

  GET /ovirt-engine/api HTTP/1.1
  Authorization: Basic P/c1qcSSGuTlxUCTEUCosZfZ
  Host: ovirt.example.com
  Prefer: persistent-auth, csrf-protection

If this preference isn't specified then the session won't be protected,
even if it is enabled globally.

For protected sessions the caller must always include the "JSESSIONID"
header, which should contain the value of the session identifier:

  GET /ovirt-engine/api HTTP/1.1
  Cookie: JSESSIONID=y+FXYivGm2rdajrNhTRatNjl
  Prefer: persistent-auth, csrf-protection
  JSESSIONID: y+FXYivGm2rdajrNhTRatNjl

If the protection is enabled and the caller fails to send this token
then the request will be rejected and logged.

Change-Id: I5700192b62e514091c9f29910596f312c068c5b2
Bug-Url: https://bugzilla.redhat.com/1077441
Signed-off-by: Juan Hernandez <[email protected]>
(cherry picked from commit 9491d9e6e34e1115fac9cfe4309c81cf554efdba)
---
M 
backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
M backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
A 
backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java
M 
backend/manager/modules/restapi/interface/common/jaxrs/src/main/modules/org/ovirt/engine/api/interface-common-jaxrs/main/module.xml
M backend/manager/modules/restapi/webapp/src/main/webapp/WEB-INF/web.xml
M ear/src/main/resources/META-INF/MANIFEST.MF
M packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
M packaging/etc/engine-config/engine-config.properties
8 files changed, 243 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.ovirt.org:29418/ovirt-engine refs/changes/49/29849/1

diff --git 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
index 7f2b954..c8ec2d3 100644
--- 
a/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
+++ 
b/backend/manager/modules/common/src/main/java/org/ovirt/engine/core/common/config/ConfigValues.java
@@ -1857,5 +1857,13 @@
     @DefaultValueAttribute("")
     CustomFencePowerWaitParam,
 
+    /**
+     * If the values is {@code true} then the RESTAPI will accept requests to 
create CSRF protected sessions, otherwise
+     * all sessions will be unprotected, regardless of what the client 
requests.
+     */
+    @TypeConverterAttribute(Boolean.class)
+    @DefaultValueAttribute("false")
+    CSRFProtection,
+
     Invalid
 }
diff --git a/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml 
b/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
index dff5807..186f393 100644
--- a/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
+++ b/backend/manager/modules/restapi/interface/common/jaxrs/pom.xml
@@ -45,6 +45,11 @@
     </dependency>
 
     <dependency>
+      <groupId>org.apache.httpcomponents</groupId>
+      <artifactId>httpcore</artifactId>
+    </dependency>
+
+    <dependency>
       <groupId>org.jboss.spec.javax.security.jacc</groupId>
       <artifactId>jboss-jacc-api_1.4_spec</artifactId>
       <version>1.0.1.Final</version>
@@ -52,6 +57,11 @@
     </dependency>
 
     <dependency>
+      <groupId>org.jboss.spec.javax.servlet</groupId>
+      <artifactId>jboss-servlet-api_3.0_spec</artifactId>
+    </dependency>
+
+    <dependency>
       <groupId>org.slf4j</groupId>
       <artifactId>slf4j-api</artifactId>
     </dependency>
diff --git 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java
 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java
new file mode 100644
index 0000000..6dd8ace
--- /dev/null
+++ 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/java/org/ovirt/engine/api/common/security/CSRFProtectionFilter.java
@@ -0,0 +1,209 @@
+/*
+* 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.common.security;
+
+import java.io.IOException;
+import java.util.Enumeration;
+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.http.HeaderElement;
+import org.apache.http.message.BasicHeaderValueParser;
+import org.ovirt.engine.core.common.config.Config;
+import org.ovirt.engine.core.common.config.ConfigValues;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+/**
+ * This filter implements CSRF protection. In order to activate the protection 
the client system has to activate it
+ * adding the {@code csrf-protection} element to the {@code Prefer} header 
sent in the first request for the session:
+ *
+ * <pre>
+ * GET /ovirt-engine/api HTTP/1.1
+ * Authorization: Basic P/c1qcSSGuTlxUCTEUCosZfZ
+ * Host: ovirt.example.com
+ * Prefer: persistent-auth, csrf-protection
+ * </pre>
+ *
+ * The server will then require that the session identifier is sent with every 
request, inside the {@code JSESSIONID}
+ * header:
+ *
+ * <pre>
+ * GET /ovirt-engine/api HTTP/1.1
+ * Cookie: JSESSIONID=y+FXYivGm2rdajrNhTRatNjl
+ * Prefer: persistent-auth, csrf-protection
+ * JSESSIONID: y+FXYivGm2rdajrNhTRatNjl
+ * </pre>
+ *
+ * Requests for sessions where protection has been enabled will be checked. If 
the session identifier header isn't
+ * provided or incorrect they will be rejected with code 403 (forbidden) and 
logged as warnings.
+ */
+@SuppressWarnings("unused")
+public class CSRFProtectionFilter implements Filter {
+    /**
+     * The log used by the filter.
+     */
+    private static final Logger log = 
LoggerFactory.getLogger(CSRFProtectionFilter.class);
+
+    /**
+     * The name of the header that contains preferences.
+     */
+    private static final String PREFER_HEADER = "Prefer";
+
+    /**
+     * The name of the header element that is used to request protection.
+     */
+    private static final String PREFER_ELEMENT = "csrf-protection";
+
+    /**
+     * The name of the header that contains the session id.
+     */
+    private static final String SESSION_ID_HEADER = "JSESSIONID";
+
+    /**
+     * The name of the session attribute that contains the boolean flag that 
indicates if the protection is enabled
+     * for the session.
+     */
+    private static final String ENABLED_ATTRIBUTE = 
CSRFProtectionFilter.class.getName() + ".enabled";
+
+    @Override
+    public void init(FilterConfig config) throws ServletException {
+    }
+
+    @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 {
+        // If protection is globally disabled then we don't need to do 
anything else, jump directly to the next filter
+        // in the chain:
+        boolean enabled = Config.getValue(ConfigValues.CSRFProtection);
+        if (!enabled) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        // If there is already a session then we need to process it 
immediately, before letting other filters or the
+        // application see or touch the request:
+        HttpSession session = request.getSession(false);
+        if (session != null) {
+            doFilterExistingSession(session, request, response, chain);
+            return;
+        }
+
+        // At this point we know that protection is globally enabled, and that 
there isn't a session already created. So
+        // we should first let the other filters and the application do their 
work. As a result a new session may be
+        // created. In that case we need to check if protection has been 
requested for that session and store the result
+        // for use in future requests.
+        try {
+            chain.doFilter(request, response);
+        }
+        finally {
+            session = request.getSession(false);
+            if (session != null) {
+                enabled = isProtectionRequested(request);
+                session.setAttribute(ENABLED_ATTRIBUTE, enabled);
+            }
+        }
+    }
+
+    private void doFilterExistingSession(HttpSession session, 
HttpServletRequest request, HttpServletResponse response,
+            FilterChain chain) throws IOException, ServletException {
+        // Check if the protection is enabled for this session, if it isn't 
then jump to the next filter:
+        boolean enabled = (Boolean) session.getAttribute(ENABLED_ATTRIBUTE);
+        if (!enabled) {
+            chain.doFilter(request, response);
+            return;
+        }
+
+        // Check if the request contains a session id header, if it doesn't 
then it must be rejected immediately:
+        String sessionIdHeader = request.getHeader(SESSION_ID_HEADER);
+        if (sessionIdHeader == null) {
+            log.warn(
+                "Request for path \"{}\" from IP address {} has been rejected 
because CSRF protection is enabled " +
+                "for the session but the the session id header \"{}\" hasn't 
been provided.",
+                request.getContextPath() + request.getPathInfo(),
+                request.getRemoteAddr(),
+                SESSION_ID_HEADER
+            );
+            response.sendError(HttpServletResponse.SC_FORBIDDEN);
+            return;
+        }
+
+        // Check if the actual session id matches the session id header:
+        String actualSessionId = session.getId();
+        if (!sessionIdHeader.equals(actualSessionId)) {
+            log.warn(
+                "Request for path \"{}\" from IP address {} has been rejected 
because CSRF protection is enabled " +
+                "for the session but the value of the session id header \"{}\" 
doesn't match the actual session " +
+                "id.",
+                request.getContextPath() + request.getPathInfo(),
+                request.getRemoteAddr(),
+                SESSION_ID_HEADER
+            );
+            response.sendError(HttpServletResponse.SC_FORBIDDEN);
+            return;
+        }
+
+        // Everything is OK, let the request go to the next filter:
+        chain.doFilter(request, response);
+    }
+
+    /**
+     * Checks if the headers contained in the given request indicate that the 
user wants to enable protection. This
+     * means checking if the {@code Prefer} header exists and has at least one 
{@code csrf-protection} element. For
+     * example:
+     *
+     * <pre>
+     * GET /ovirt-engine/api HTTP/1.1
+     * Host: ovirt.example.com
+     * Prefer: persistent-auth, csrf-protection
+     * </pre>
+     *
+     * @param request the HTTP request to check
+     * @return {@code true} if the request contains headers that indicate that 
protection should be enabled,
+     *   {@code false} otherwise
+     */
+    private boolean isProtectionRequested(HttpServletRequest request) {
+        Enumeration<String> headerValues = request.getHeaders(PREFER_HEADER);
+        while (headerValues.hasMoreElements()) {
+            String headerValue = headerValues.nextElement();
+            HeaderElement[] headerElements = 
BasicHeaderValueParser.parseElements(headerValue, null);
+            for (HeaderElement headerElement : headerElements) {
+                String elementName = headerElement.getName();
+                if (PREFER_ELEMENT.equalsIgnoreCase(elementName)) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+}
diff --git 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/modules/org/ovirt/engine/api/interface-common-jaxrs/main/module.xml
 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/modules/org/ovirt/engine/api/interface-common-jaxrs/main/module.xml
index 2391ac6..ec909d0 100644
--- 
a/backend/manager/modules/restapi/interface/common/jaxrs/src/main/modules/org/ovirt/engine/api/interface-common-jaxrs/main/module.xml
+++ 
b/backend/manager/modules/restapi/interface/common/jaxrs/src/main/modules/org/ovirt/engine/api/interface-common-jaxrs/main/module.xml
@@ -24,12 +24,15 @@
 
   <dependencies>
     <module name="javax.api"/>
+    <module name="javax.servlet.api"/>
     <module name="javax.ws.rs.api"/>
     <module name="javax.xml.bind.api"/>
     <module name="org.apache.commons.codec"/>
     <module name="org.apache.commons.lang"/>
+    <module name="org.apache.httpcomponents"/>
     <module name="org.jboss.resteasy.resteasy-jaxrs"/>
     <module name="org.ovirt.engine.api.restapi-definition"/>
+    <module name="org.ovirt.engine.core.common"/>
     <module name="org.slf4j"/>
   </dependencies>
 
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 d31d956..8ed8ff6 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
@@ -23,6 +23,15 @@
   version="3.0">
 
   <filter>
+    <filter-name>CSRFProtection</filter-name>
+    
<filter-class>org.ovirt.engine.api.common.security.CSRFProtectionFilter</filter-class>
+  </filter>
+  <filter-mapping>
+    <filter-name>CSRFProtection</filter-name>
+    <url-pattern>/*</url-pattern>
+  </filter-mapping>
+
+  <filter>
     <filter-name>RestApiSessionValidationFilter</filter-name>
     
<filter-class>org.ovirt.engine.core.aaa.filters.RestApiSessionValidationFilter</filter-class>
   </filter>
diff --git a/ear/src/main/resources/META-INF/MANIFEST.MF 
b/ear/src/main/resources/META-INF/MANIFEST.MF
index 2b122ef..853f6c2 100644
--- a/ear/src/main/resources/META-INF/MANIFEST.MF
+++ b/ear/src/main/resources/META-INF/MANIFEST.MF
@@ -6,6 +6,7 @@
  org.apache.commons.collections,
  org.apache.commons.httpclient,
  org.apache.commons.lang,
+ org.apache.httpcomponents,
  org.apache.log4j,
  org.apache.xmlrpc,
  org.codehaus.jackson.jackson-core-asl,
diff --git a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql 
b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
index 94a9592..3c05e85 100644
--- a/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
+++ b/packaging/dbscripts/upgrade/pre_upgrade/0000_config.sql
@@ -787,6 +787,7 @@
 
 select fn_db_add_config_value('AlertOnNumberOfLVs','300','general');
 
+select fn_db_add_config_value('CSRFProtection','false','general');
 
 
------------------------------------------------------------------------------------
 --                  Update with override section
diff --git a/packaging/etc/engine-config/engine-config.properties 
b/packaging/etc/engine-config/engine-config.properties
index e1d3e07..3c8dbc9 100644
--- a/packaging/etc/engine-config/engine-config.properties
+++ b/packaging/etc/engine-config/engine-config.properties
@@ -393,6 +393,8 @@
 MaxNumOfTriesToRunFailedAutoStartVm.type=Integer
 RetryToRunAutoStartVmIntervalInSeconds.description="How often to try to 
restart highly available VM that went down unexpectedly (in seconds)"
 RetryToRunAutoStartVmIntervalInSeconds.type=Integer
+CSRFProtection.type=Boolean
+CSRFProtection.description="Enables CSRF (Cross Site Request Forgery) 
protection in RESTAPI."
 # PM Health Check
 PMHealthCheckEnabled.type=Boolean
 PMHealthCheckEnabled.description="Enable/Disable Power Management Health Check 
feature."


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

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

Reply via email to