From 076cf9e1c523c5e54f5625a9356f29feaa206d51 Mon Sep 17 00:00:00 2001
From: Sebastian Malton <sebastian@malton.name>
Date: Tue, 9 Nov 2021 09:26:37 -0500
Subject: [PATCH] Make ws:// authentication more timing safe

- Switch to crypto.timingSafeEqual() and Uint8Array (because of the
  crypto function)

Signed-off-by: Sebastian Malton <sebastian@malton.name>
---
 src/main/proxy-functions/shell-api-request.ts | 23 ++++++++++++++-----
 src/renderer/api/terminal-api.ts              |  9 ++++++--
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/src/main/proxy-functions/shell-api-request.ts b/src/main/proxy-functions/shell-api-request.ts
index db88336a2a..828629c6dd 100644
--- a/src/main/proxy-functions/shell-api-request.ts
+++ b/src/main/proxy-functions/shell-api-request.ts
@@ -28,14 +28,17 @@ import URLParse from "url-parse";
 import { ExtendedMap, Singleton } from "../../common/utils";
 import type { ClusterId } from "../../common/cluster-types";
 import { ipcMainHandle } from "../../common/ipc";
-import * as uuid from "uuid";
+import crypto from "crypto";
+import { promisify } from "util";
+
+const randomBytes = promisify(crypto.randomBytes);
 
 export class ShellRequestAuthenticator extends Singleton {
-  private tokens = new ExtendedMap<ClusterId, Map<string, string>>();
+  private tokens = new ExtendedMap<ClusterId, Map<string, Uint8Array>>();
 
   init() {
-    ipcMainHandle("cluster:shell-api", (event, clusterId, tabId) => {
-      const authToken = uuid.v4();
+    ipcMainHandle("cluster:shell-api", async (event, clusterId, tabId) => {
+      const authToken = Uint8Array.from(await randomBytes(128));
 
       this.tokens
         .getOrInsert(clusterId, () => new Map())
@@ -60,9 +63,17 @@ export class ShellRequestAuthenticator extends Singleton {
     }
 
     const authToken = clusterTokens.get(tabId);
+    const buf = Uint8Array.from(token.split(",").map(ar => {
+      const res = +ar;
 
-    // need both conditions to prevent `undefined === undefined` being true here
-    if (typeof authToken === "string" && authToken === token) {
+      if (isNaN(res)) {
+        throw new TypeError("Token must be a CSV list of digits");
+      }
+
+      return res;
+    }));
+
+    if (authToken instanceof Uint8Array && crypto.timingSafeEqual(authToken, buf)) {
       // remove the token because it is a single use token
       clusterTokens.delete(tabId);
 
diff --git a/src/renderer/api/terminal-api.ts b/src/renderer/api/terminal-api.ts
index a1533430db..a6a76a8757 100644
--- a/src/renderer/api/terminal-api.ts
+++ b/src/renderer/api/terminal-api.ts
@@ -75,7 +75,12 @@ export class TerminalApi extends WebSocketApi {
   async connect() {
     this.emitStatus("Connecting ...");
 
-    const shellToken = await ipcRenderer.invoke("cluster:shell-api", getHostedClusterId(), this.query.id);
+    const authTokenArray = await ipcRenderer.invoke("cluster:shell-api", getHostedClusterId(), this.query.id);
+
+    if (!(authTokenArray instanceof Uint8Array)) {
+      throw new TypeError("ShellApi token is not a Uint8Array");
+    }
+
     const { hostname, protocol, port } = location;
     const socketUrl = url.format({
       protocol: protocol.includes("https") ? "wss" : "ws",
@@ -84,7 +89,7 @@ export class TerminalApi extends WebSocketApi {
       pathname: "/api",
       query: {
         ...this.query,
-        shellToken,
+        shellToken: authTokenArray.toString(),
       },
       slashes: true,
     });