[server] strict same site origin for /api/gitpod endpoint

Author: geropl

components/server/src/express-util.spec.ts

@@ -15,6 +15,7 @@ describe("express-util", function () {
             const result = isAllowedWebsocketDomain(
                 "http://3000-moccasin-ferret-155799b3.ws-eu.gpl-2732-ws-csrf.staging.gitpod.io",
                 HOSTURL_HOSTNAME,
+                false,
             );
             expect(result).to.be.false;
         });
@@ -23,12 +24,43 @@ describe("express-util", function () {
             const result = isAllowedWebsocketDomain(
                 "http://moccasin-ferret-155799b3.ws-eu.gpl-2732-ws-csrf.staging.gitpod.io",
                 HOSTURL_HOSTNAME,
+                false,
             );
             expect(result).to.be.true;
         });
 
         it("should return true for dashboard", function () {
-            const result = isAllowedWebsocketDomain("http://gpl-2732-ws-csrf.staging.gitpod.io", HOSTURL_HOSTNAME);
+            const result = isAllowedWebsocketDomain(
+                "http://gpl-2732-ws-csrf.staging.gitpod.io",
+                HOSTURL_HOSTNAME,
+                false,
+            );
+            expect(result).to.be.true;
+        });
+        it("should return false for workspace-port locations (strict)", function () {
+            const result = isAllowedWebsocketDomain(
+                "http://3000-moccasin-ferret-155799b3.ws-eu.gpl-2732-ws-csrf.staging.gitpod.io",
+                HOSTURL_HOSTNAME,
+                true,
+            );
+            expect(result).to.be.false;
+        });
+
+        it("should return true for workspace locations (strict)", function () {
+            const result = isAllowedWebsocketDomain(
+                "http://moccasin-ferret-155799b3.ws-eu.gpl-2732-ws-csrf.staging.gitpod.io",
+                HOSTURL_HOSTNAME,
+                true,
+            );
+            expect(result).to.be.false;
+        });
+
+        it("should return true for dashboard (strict)", function () {
+            const result = isAllowedWebsocketDomain(
+                "http://gpl-2732-ws-csrf.staging.gitpod.io",
+                HOSTURL_HOSTNAME,
+                true,
+            );
             expect(result).to.be.true;
         });
     });
@@ -38,12 +70,34 @@ describe("express-util", function () {
             const result = isAllowedWebsocketDomain(
                 "https://8000-black-capybara-dy6e3fgz.ws-eu08.gitpod.io",
                 HOSTURL_HOSTNAME,
+                false,
             );
             expect(result).to.be.false;
         });
 
         it("should return true for workspace locations", function () {
-            const result = isAllowedWebsocketDomain("https://bronze-bird-p2q226d8.ws-eu08.gitpod.io", HOSTURL_HOSTNAME);
+            const result = isAllowedWebsocketDomain(
+                "https://bronze-bird-p2q226d8.ws-eu08.gitpod.io",
+                HOSTURL_HOSTNAME,
+                false,
+            );
+            expect(result).to.be.true;
+        });
+        it("should return false for workspace-port locations (strict)", function () {
+            const result = isAllowedWebsocketDomain(
+                "https://8000-black-capybara-dy6e3fgz.ws-eu08.gitpod.io",
+                HOSTURL_HOSTNAME,
+                true,
+            );
+            expect(result).to.be.false;
+        });
+
+        it("should return false for workspace locations (strict)", function () {
+            const result = isAllowedWebsocketDomain(
+                "https://bronze-bird-p2q226d8.ws-eu08.gitpod.io",
+                HOSTURL_HOSTNAME,
+                true,
+            );
             expect(result).to.be.true;
         });
     });

components/server/src/express-util.ts

@@ -7,8 +7,9 @@
 import { URL } from "url";
 import * as express from "express";
 import * as crypto from "crypto";
-import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
 import * as session from "express-session";
+import { log } from "@gitpod/gitpod-protocol/lib/util/logging";
+import { GitpodHostUrl } from "@gitpod/gitpod-protocol/lib/util/gitpod-host-url";
 
 export const query = (...tuples: [string, string][]) => {
     if (tuples.length === 0) {
@@ -17,22 +18,29 @@ export const query = (...tuples: [string, string][]) => {
     return "?" + tuples.map((t) => `${t[0]}=${encodeURIComponent(t[1])}`).join("&");
 };
 
-// We do not precise UUID parsing here, we just want to distinguish three cases:
-//  - the base domain
-//  - a frontend domain (workspace domain)
-//  - a workspace port domain
-// We control all of those values and the base domain, so we don't need to much effort
-export const isAllowedWebsocketDomain = (originHeader: string, gitpodHostName: string): boolean => {
+// Strict: We only allow connections from the base domain, so disallow connections from all other Origins
+//      Only (current) exception: If no Origin header is set, skip the check!
+// Non-Strict: "rely" on subdomain parsing (do we still need this?)
+export const isAllowedWebsocketDomain = (originHeader: string, gitpodHostName: string, strict: boolean): boolean => {
     if (!originHeader) {
+        // TODO(gpl) Can we get rid of this dependency alltogether?
+        log.warn("Origin header check not applied because of empty Origin header!");
         return true;
     }
-    var originHostname = "";
+
     try {
         const originUrl = new URL(originHeader);
-        originHostname = originUrl.hostname;
+        const originHostname = originUrl.hostname;
         if (originHostname === gitpodHostName) {
             return true;
         }
+
+        if (strict) {
+            return false;
+        }
+        // TODO(gpl) This is only used for Bearer-Token authentication.
+        // Does this restriction help at all, or can we remove it entirely?
+        log.warn("Origin header check based on looksLikeWorkspaceHostname");
         if (looksLikeWorkspaceHostname(originUrl, gitpodHostName)) {
             return true;
         } else {

components/server/src/server.ts

@@ -154,13 +154,14 @@ export class Server<C extends GitpodClient, S extends GitpodServer> {
         // Websocket for client connection
         const websocketConnectionHandler = this.websocketConnectionHandler;
         this.eventEmitter.on(Server.EVENT_ON_START, (httpServer) => {
-            // CSRF protection: check "Origin" header, it must be either:
-            //  - gitpod.io (hostUrl.hostname) or
-            //  - a workspace location (ending of hostUrl.hostname)
+            // CSRF protection: check "Origin" header:
+            //  - for cookie/session auth: MUST be gitpod.io (hostUrl.hostname)
+            //  - for Bearer auth: MUST be sth with the same base domain (*.gitpod.io) (is this required?)
+            //  - edge case: empty "Origin" is always permitted (can this be removed?)
             // We rely on the origin header being set correctly (needed by regular clients to use Gitpod:
             // CORS allows subdomains to access gitpod.io)
-            const verifyCSRF = (origin: string) => {
-                let allowedRequest = isAllowedWebsocketDomain(origin, this.config.hostUrl.url.hostname);
+            const verifyOrigin = (origin: string, strict: boolean) => {
+                let allowedRequest = isAllowedWebsocketDomain(origin, this.config.hostUrl.url.hostname, strict);
                 if (!allowedRequest && this.config.insecureNoDomain) {
                     log.warn("Websocket connection CSRF guard disabled");
                     allowedRequest = true;
@@ -172,21 +173,35 @@ export class Server<C extends GitpodClient, S extends GitpodServer> {
              * Verify the web socket handshake request.
              */
             const verifyClient: ws.VerifyClientCallbackAsync = async (info, callback) => {
-                if (!verifyCSRF(info.origin)) {
-                    log.warn("Websocket connection attempt with non-matching Origin header: " + info.origin);
-                    return callback(false, 403);
-                }
+                let authenticatedUsingBearerToken = false;
                 if (info.req.url === "/v1") {
+                    // Connection attempt with Bearer-Token: be less strict for now
+                    if (!verifyOrigin(info.origin, false)) {
+                        log.warn("Websocket connection attempt with non-matching Origin header: " + info.origin);
+                        return callback(false, 403);
+                    }
+
                     try {
                         await this.bearerAuth.auth(info.req as express.Request);
+                        authenticatedUsingBearerToken = true;
                     } catch (e) {
                         if (isBearerAuthError(e)) {
                             return callback(false, 401, e.message);
                         }
                         log.warn("authentication failed: ", e);
                         return callback(false, 500);
                     }
+                    // intentional fall-through to cookie/session based authentication
                 }
+
+                if (!authenticatedUsingBearerToken) {
+                    // Connection attempt with cookie/session based authentication: be strict about where we accept connections from!
+                    if (!verifyOrigin(info.origin, true)) {
+                        log.warn("Websocket connection attempt with non-matching Origin header: " + info.origin);
+                        return callback(false, 403);
+                    }
+                }
+
                 return callback(true);
             };