Verify CSRF token before accepting new CSRF connection (#11635)

* Can't open push connection during client-side init because CSRF token
is not available at that point. This allows simplifying the
initialization because the push state will not be checked until the
first response has been processed.
* Add helper for checking the CSRF token

Change-Id: I31da1ac669dc9a581cbd66f58c07f10ea4b8b676

11 files changed, 101 insertions, 103 deletions

diff --git a/WebContent/VAADIN/vaadinBootstrap.js b/WebContent/VAADIN/vaadinBootstrap.js
index ae50289477..b2995dd0bd 100644
--- a/WebContent/VAADIN/vaadinBootstrap.js
+++ b/WebContent/VAADIN/vaadinBootstrap.js
@@ -120,12 +120,6 @@
 					url += '&theme=' + encodeURIComponent(theme);
 				}
 				
-				// Tell the UI what pushMode it is configured to use
-				var pushMode = getConfig('pushMode');
-				if (pushMode !== undefined) {
-					url += '&v-pushMode=' + encodeURIComponent(pushMode);
-				}
-				
 				var extraParams = getConfig('extraParams')
 				if (extraParams !== undefined) {
 					url += extraParams;
diff --git a/client/src/com/vaadin/client/ApplicationConfiguration.java b/client/src/com/vaadin/client/ApplicationConfiguration.java
index e1f460ff48..adf5e1de9d 100644
--- a/client/src/com/vaadin/client/ApplicationConfiguration.java
+++ b/client/src/com/vaadin/client/ApplicationConfiguration.java
@@ -46,7 +46,6 @@ import com.vaadin.client.metadata.NoDataException;
 import com.vaadin.client.metadata.TypeData;
 import com.vaadin.client.ui.UnknownComponentConnector;
 import com.vaadin.shared.ApplicationConstants;
-import com.vaadin.shared.communication.PushMode;
 import com.vaadin.shared.ui.ui.UIConstants;
 
 public class ApplicationConfiguration implements EntryPoint {
@@ -212,7 +211,6 @@ public class ApplicationConfiguration implements EntryPoint {
     private ErrorMessage authorizationError;
     private ErrorMessage sessionExpiredError;
     private int heartbeatInterval;
-    private PushMode pushMode;
 
     private HashMap<Integer, String> unknownComponents;
 
@@ -325,10 +323,6 @@ public class ApplicationConfiguration implements EntryPoint {
         return heartbeatInterval;
     }
 
-    public PushMode getPushMode() {
-        return pushMode;
-    }
-
     public JavaScriptObject getVersionInfoJSObject() {
         return getJsoConfiguration(id).getVersionInfoJSObject();
     }
@@ -382,14 +376,6 @@ public class ApplicationConfiguration implements EntryPoint {
         heartbeatInterval = jsoConfiguration
                 .getConfigInteger("heartbeatInterval");
 
-        String pushMode = jsoConfiguration.getConfigString("pushMode");
-        if (pushMode != null) {
-            this.pushMode = Enum
-                    .valueOf(PushMode.class, pushMode.toUpperCase());
-        } else {
-            this.pushMode = PushMode.DISABLED;
-        }
-
         communicationError = jsoConfiguration.getConfigError("comErrMsg");
         authorizationError = jsoConfiguration.getConfigError("authErrMsg");
         sessionExpiredError = jsoConfiguration.getConfigError("sessExpMsg");
diff --git a/client/src/com/vaadin/client/ApplicationConnection.java b/client/src/com/vaadin/client/ApplicationConnection.java
index dc8dbcaf43..85cf0f0b46 100644
--- a/client/src/com/vaadin/client/ApplicationConnection.java
+++ b/client/src/com/vaadin/client/ApplicationConnection.java
@@ -156,8 +156,8 @@ public class ApplicationConnection {
      */
     public static final String UIDL_REFRESH_TOKEN = "Vaadin-Refresh";
 
-    // will hold the UIDL security key (for XSS protection) once received
-    private String uidlSecurityKey = "init";
+    // will hold the CSRF token once received
+    private String csrfToken = "init";
 
     private final HashMap<String, String> resourcesMap = new HashMap<String, String>();
 
@@ -182,19 +182,6 @@ public class ApplicationConnection {
 
     protected boolean applicationRunning = false;
 
-    /**
-     * Keep track of whether the initialization JSON has been handled. We should
-     * not process any push messages until the initial JSON has been processed.
-     */
-    private boolean initJsonHandled = false;
-
-    /**
-     * Keep track of any push messages that arrive before
-     * {@link #initJsonHandled} is set to true.
-     */
-    private JsArrayString incommingPushMessageQueue = JsArrayString
-            .createArray().cast();
-
     private boolean hasActiveRequest = false;
 
     /**
@@ -455,8 +442,6 @@ public class ApplicationConnection {
 
         scheduleHeartbeat();
 
-        setPushEnabled(getConfiguration().getPushMode().isEnabled());
-
         Window.addWindowClosingHandler(new ClosingHandler() {
             @Override
             public void onWindowClosing(ClosingEvent event) {
@@ -715,7 +700,7 @@ public class ApplicationConnection {
             final String extraParams) {
         startRequest();
         // Security: double cookie submission pattern
-        final String payload = uidlSecurityKey + VAR_BURST_SEPARATOR
+        final String payload = getCsrfToken() + VAR_BURST_SEPARATOR
                 + requestData;
         VConsole.log("Making UIDL Request with params: " + payload);
         String uri = translateVaadinUri(ApplicationConstants.APP_PROTOCOL_PREFIX
@@ -1127,25 +1112,6 @@ public class ApplicationConnection {
             runPostRequestHooks(configuration.getRootPanelId());
         }
 
-        if (!initJsonHandled) {
-            /*
-             * Assume that the first request that is fully handled is the one
-             * with the initialization data.
-             */
-            initJsonHandled = true;
-
-            int queueLength = incommingPushMessageQueue.length();
-            if (queueLength > 0) {
-                VConsole.log("Init handled, processing " + queueLength
-                        + " enqueued messages");
-                for (int i = 0; i < queueLength; i++) {
-                    handlePushMessage(incommingPushMessageQueue.get(i));
-                }
-                incommingPushMessageQueue.setLength(0);
-            }
-
-        }
-
         // deferring to avoid flickering
         Scheduler.get().scheduleDeferred(new Command() {
             @Override
@@ -1315,7 +1281,7 @@ public class ApplicationConnection {
 
         // Get security key
         if (json.containsKey(ApplicationConstants.UIDL_SECURITY_TOKEN_ID)) {
-            uidlSecurityKey = json
+            csrfToken = json
                     .getString(ApplicationConstants.UIDL_SECURITY_TOKEN_ID);
         }
         VConsole.log(" * Handling resources from server");
@@ -3034,7 +3000,17 @@ public class ApplicationConnection {
     private ConnectorMap connectorMap = GWT.create(ConnectorMap.class);
 
     protected String getUidlSecurityKey() {
-        return uidlSecurityKey;
+        return getCsrfToken();
+    }
+
+    /**
+     * Gets the token (aka double submit cookie) that the server uses to protect
+     * against Cross Site Request Forgery attacks.
+     * 
+     * @return the CSRF token string
+     */
+    public String getCsrfToken() {
+        return csrfToken;
     }
 
     /**
@@ -3443,11 +3419,6 @@ public class ApplicationConnection {
     }
 
     public void handlePushMessage(String message) {
-        if (initJsonHandled) {
-            handleJSONText(message, 200);
-        } else {
-            VConsole.log("Enqueuing push message has init has not yet been handled");
-            incommingPushMessageQueue.push(message);
-        }
+        handleJSONText(message, 200);
     }
 }
diff --git a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
index ef5fc56347..bd666cb464 100644
--- a/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
+++ b/client/src/com/vaadin/client/communication/AtmospherePushConnection.java
@@ -113,6 +113,8 @@ public class AtmospherePushConnection implements PushConnection {
                         + ApplicationConstants.PUSH_PATH + '/');
         String extraParams = UIConstants.UI_ID_PARAMETER + "="
                 + connection.getConfiguration().getUIId();
+        extraParams += "&" + ApplicationConstants.CSRF_TOKEN_PARAMETER + "="
+                + connection.getCsrfToken();
 
         // uri is needed to identify the right connection when closing
         uri = ApplicationConnection.addGetParameters(baseUrl, extraParams);
diff --git a/server/src/com/vaadin/server/BootstrapHandler.java b/server/src/com/vaadin/server/BootstrapHandler.java
index e83b2dfdef..dddfb385a6 100644
--- a/server/src/com/vaadin/server/BootstrapHandler.java
+++ b/server/src/com/vaadin/server/BootstrapHandler.java
@@ -526,8 +526,6 @@ public abstract class BootstrapHandler extends SynchronizedRequestHandler {
         appConfig.put("heartbeatInterval", vaadinService
                 .getDeploymentConfiguration().getHeartbeatInterval());
 
-        appConfig.put("pushMode", context.getPushMode().toString());
-
         String serviceUrl = getServiceUrl(context);
         if (serviceUrl != null) {
             appConfig.put(ApplicationConstants.SERVICE_URL, serviceUrl);
diff --git a/server/src/com/vaadin/server/VaadinService.java b/server/src/com/vaadin/server/VaadinService.java
index 6088fa55ca..11060c6f73 100644
--- a/server/src/com/vaadin/server/VaadinService.java
+++ b/server/src/com/vaadin/server/VaadinService.java
@@ -52,6 +52,7 @@ import com.vaadin.server.communication.HeartbeatHandler;
 import com.vaadin.server.communication.PublishedFileHandler;
 import com.vaadin.server.communication.SessionRequestHandler;
 import com.vaadin.server.communication.UidlRequestHandler;
+import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.JsonConstants;
 import com.vaadin.shared.ui.ui.UIConstants;
 import com.vaadin.ui.UI;
@@ -1481,4 +1482,42 @@ public abstract class VaadinService implements Serializable {
         return false;
     }
 
+    /**
+     * Verifies that the given CSRF token (aka double submit cookie) is valid
+     * for the given session. This is used to protect against Cross Site Request
+     * Forgery attacks.
+     * <p>
+     * This protection is enabled by default, but it might need to be disabled
+     * to allow a certain type of testing. For these cases, the check can be
+     * disabled by setting the init parameter
+     * {@value Constants#SERVLET_PARAMETER_DISABLE_XSRF_PROTECTION} to
+     * <code>true</code>.
+     * 
+     * @see DeploymentConfiguration#isXsrfProtectionEnabled()
+     * 
+     * @since 7.1
+     * 
+     * @param session
+     *            the vaadin session for which the check should be done
+     * @param requestToken
+     *            the CSRF token provided in the request
+     * @return <code>true</code> if the token is valid or if the protection is
+     *         disabled; <code>false</code> if protection is enabled and the
+     *         token is invalid
+     */
+    public static boolean isCsrfTokenValid(VaadinSession session,
+            String requestToken) {
+
+        if (session.getService().getDeploymentConfiguration()
+                .isXsrfProtectionEnabled()) {
+            String keyInSession = (String) session.getSession().getAttribute(
+                    ApplicationConstants.UIDL_SECURITY_TOKEN_ID);
+
+            if (keyInSession == null || !keyInSession.equals(requestToken)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
 }
diff --git a/server/src/com/vaadin/server/communication/PushHandler.java b/server/src/com/vaadin/server/communication/PushHandler.java
index bdc8c723a5..c9d451509c 100644
--- a/server/src/com/vaadin/server/communication/PushHandler.java
+++ b/server/src/com/vaadin/server/communication/PushHandler.java
@@ -38,6 +38,7 @@ import com.vaadin.server.VaadinServletRequest;
 import com.vaadin.server.VaadinServletService;
 import com.vaadin.server.VaadinSession;
 import com.vaadin.server.WebBrowser;
+import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.communication.PushMode;
 import com.vaadin.ui.UI;
 
@@ -70,15 +71,34 @@ public class PushHandler implements AtmosphereHandler {
                     "New push connection with transport {0}",
                     resource.transport());
             resource.getResponse().setContentType("text/plain; charset=UTF-8");
+
+            VaadinSession session = ui.getSession();
             if (resource.transport() == TRANSPORT.STREAMING) {
                 // IE8 requires a longer padding to work properly if the
                 // initial message is small (#11573). Chrome does not work
                 // without the original padding...
-                WebBrowser browser = ui.getSession().getBrowser();
+                WebBrowser browser = session.getBrowser();
                 if (browser.isIE() && browser.getBrowserMajorVersion() == 8) {
                     resource.padding(LONG_PADDING);
                 }
             }
+
+            String requestToken = resource.getRequest().getParameter(
+                    ApplicationConstants.CSRF_TOKEN_PARAMETER);
+            if (!VaadinService.isCsrfTokenValid(session, requestToken)) {
+                getLogger()
+                        .log(Level.WARNING,
+                                "Invalid CSRF token in new connection received from {0}",
+                                resource.getRequest().getRemoteHost());
+                // Refresh on client side, create connection just for
+                // sending a message
+                AtmospherePushConnection connection = new AtmospherePushConnection(
+                        ui);
+                connection.connect(resource);
+                sendRefresh(connection);
+                return;
+            }
+
             resource.suspend();
 
             AtmospherePushConnection connection = new AtmospherePushConnection(
@@ -122,19 +142,13 @@ public class PushHandler implements AtmosphereHandler {
                 getLogger().log(Level.SEVERE, "Error writing JSON to response",
                         e);
                 // Refresh on client side
-                connection
-                        .sendMessage(VaadinService
-                                .createCriticalNotificationJSON(null, null,
-                                        null, null));
+                sendRefresh(connection);
             } catch (InvalidUIDLSecurityKeyException e) {
                 getLogger().log(Level.WARNING,
                         "Invalid security key received from {0}",
                         resource.getRequest().getRemoteHost());
                 // Refresh on client side
-                connection
-                        .sendMessage(VaadinService
-                                .createCriticalNotificationJSON(null, null,
-                                        null, null));
+                sendRefresh(connection);
             }
         }
     };
@@ -314,6 +328,11 @@ public class PushHandler implements AtmosphereHandler {
     public void destroy() {
     }
 
+    private static void sendRefresh(AtmospherePushConnection connection) {
+        connection.sendMessage(VaadinService.createCriticalNotificationJSON(
+                null, null, null, null));
+    }
+
     private static final Logger getLogger() {
         return Logger.getLogger(PushHandler.class.getName());
     }
diff --git a/server/src/com/vaadin/server/communication/ServerRpcHandler.java b/server/src/com/vaadin/server/communication/ServerRpcHandler.java
index 11de224e5e..64f8dfb223 100644
--- a/server/src/com/vaadin/server/communication/ServerRpcHandler.java
+++ b/server/src/com/vaadin/server/communication/ServerRpcHandler.java
@@ -41,6 +41,7 @@ import com.vaadin.server.ServerRpcManager;
 import com.vaadin.server.ServerRpcManager.RpcInvocationException;
 import com.vaadin.server.ServerRpcMethodInvocation;
 import com.vaadin.server.VaadinRequest;
+import com.vaadin.server.VaadinService;
 import com.vaadin.server.VariableOwner;
 import com.vaadin.shared.ApplicationConstants;
 import com.vaadin.shared.Connector;
@@ -108,29 +109,8 @@ public class ServerRpcHandler implements Serializable {
 
         // Security: double cookie submission pattern unless disabled by
         // property
-        if (ui.getSession().getConfiguration().isXsrfProtectionEnabled()) {
-            if (bursts.length == 1 && "init".equals(bursts[0])) {
-                // init request; don't handle any variables, key sent in
-                // response.
-                // TODO This seems to be dead code
-                request.setAttribute(
-                        LegacyCommunicationManager.WRITE_SECURITY_TOKEN_FLAG,
-                        true);
-                return;
-            } else {
-                // ApplicationServlet has stored the security token in the
-                // session; check that it matched the one sent in the UIDL
-                String sessId = (String) ui
-                        .getSession()
-                        .getSession()
-                        .getAttribute(
-                                ApplicationConstants.UIDL_SECURITY_TOKEN_ID);
-
-                if (sessId == null || !sessId.equals(bursts[0])) {
-                    throw new InvalidUIDLSecurityKeyException("");
-                }
-            }
-
+        if (!VaadinService.isCsrfTokenValid(ui.getSession(), bursts[0])) {
+            throw new InvalidUIDLSecurityKeyException("");
         }
         handleBurst(ui, unescapeBurst(bursts[1]));
     }
diff --git a/server/src/com/vaadin/server/communication/UIInitHandler.java b/server/src/com/vaadin/server/communication/UIInitHandler.java
index efe11e02e9..7c8fc3a0d8 100644
--- a/server/src/com/vaadin/server/communication/UIInitHandler.java
+++ b/server/src/com/vaadin/server/communication/UIInitHandler.java
@@ -37,6 +37,7 @@ import com.vaadin.server.VaadinRequest;
 import com.vaadin.server.VaadinResponse;
 import com.vaadin.server.VaadinService;
 import com.vaadin.server.VaadinSession;
+import com.vaadin.shared.communication.PushMode;
 import com.vaadin.shared.ui.ui.UIConstants;
 import com.vaadin.ui.UI;
 
@@ -207,6 +208,13 @@ public abstract class UIInitHandler extends SynchronizedRequestHandler {
 
         ui.doInit(request, uiId.intValue());
 
+        PushMode pushMode = provider.getPushMode(event);
+        if (pushMode == null) {
+            pushMode = session.getService().getDeploymentConfiguration()
+                    .getPushMode();
+        }
+        ui.setPushMode(pushMode);
+
         session.addUI(ui);
 
         // Remember if it should be remembered
diff --git a/server/src/com/vaadin/ui/UI.java b/server/src/com/vaadin/ui/UI.java
index 0ad2787cb6..adba9bd83e 100644
--- a/server/src/com/vaadin/ui/UI.java
+++ b/server/src/com/vaadin/ui/UI.java
@@ -543,10 +543,6 @@ public abstract class UI extends AbstractSingleComponentContainer implements
         // Actual theme - used for finding CustomLayout templates
         theme = request.getParameter("theme");
 
-        PushMode pushMode = PushMode.valueOf(request.getParameter("v-pushMode")
-                .toUpperCase());
-        setPushMode(pushMode);
-
         getPage().init(request);
 
         // Call the init overridden by the application developer
diff --git a/shared/src/com/vaadin/shared/ApplicationConstants.java b/shared/src/com/vaadin/shared/ApplicationConstants.java
index 6b0c8e7244..04cba79c0c 100644
--- a/shared/src/com/vaadin/shared/ApplicationConstants.java
+++ b/shared/src/com/vaadin/shared/ApplicationConstants.java
@@ -78,4 +78,9 @@ public class ApplicationConstants implements Serializable {
      * in the VAADIN directory.
      */
     public static final String VAADIN_PUSH_JS = "vaadinPush.js";
+
+    /**
+     * Name of the parameter used to transmit the CSRF token.
+     */
+    public static final String CSRF_TOKEN_PARAMETER = "v-csrfToken";
 }