Skip to content

Commit 2018024

Browse files
committed
Hash password
Fixes issues with unexpected characters breaking things when setting the cookie (like semicolons). This change as-is does not affect the security of code-server itself (we've just replaced the static password with a static hash) but if we were to add a salt in the future it would let us invalidate keys by rehashing with a new salt which could be handy.
1 parent a1d6bcb commit 2018024

File tree

2 files changed

+25
-14
lines changed

2 files changed

+25
-14
lines changed

src/node/server.ts

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ import { TelemetryClient } from "vs/server/src/node/insights";
6363
import { getLocaleFromConfig, getNlsConfiguration } from "vs/server/src/node/nls";
6464
import { Protocol } from "vs/server/src/node/protocol";
6565
import { UpdateService } from "vs/server/src/node/update";
66-
import { AuthType, getMediaMime, getUriTransformer, localRequire, tmpdir } from "vs/server/src/node/util";
66+
import { AuthType, getMediaMime, getUriTransformer, hash, localRequire, tmpdir } from "vs/server/src/node/util";
6767
import { RemoteExtensionLogFileName } from "vs/workbench/services/remote/common/remoteAgentService";
6868
import { IWorkbenchConstructionOptions } from "vs/workbench/workbench.web.api";
6969

@@ -98,7 +98,11 @@ export interface Response {
9898
}
9999

100100
export interface LoginPayload {
101-
password?: string[] | string;
101+
password?: string;
102+
}
103+
104+
export interface AuthPayload {
105+
key?: string[];
102106
}
103107

104108
export class HttpError extends Error {
@@ -137,6 +141,7 @@ export abstract class Server {
137141
host: options.auth === "password" && options.cert ? "0.0.0.0" : "localhost",
138142
...options,
139143
basePath: options.basePath ? options.basePath.replace(/\/+$/, "") : "",
144+
password: options.password ? hash(options.password) : undefined,
140145
};
141146
this.protocol = this.options.cert ? "https" : "http";
142147
if (this.protocol === "https") {
@@ -357,11 +362,11 @@ export abstract class Server {
357362
}
358363

359364
private async tryLogin(request: http.IncomingMessage): Promise<Response> {
360-
const redirect = (password?: string | string[] | true) => {
365+
const redirect = (password: string | true) => {
361366
return {
362367
redirect: "/",
363368
headers: typeof password === "string"
364-
? { "Set-Cookie": `password=${password}; Path=${this.options.basePath || "/"}; HttpOnly; SameSite=strict` }
369+
? { "Set-Cookie": `key=${password}; Path=${this.options.basePath || "/"}; HttpOnly; SameSite=strict` }
365370
: {},
366371
};
367372
};
@@ -371,8 +376,11 @@ export abstract class Server {
371376
}
372377
if (request.method === "POST") {
373378
const data = await this.getData<LoginPayload>(request);
374-
if (this.authenticate(request, data)) {
375-
return redirect(data.password);
379+
const password = this.authenticate(request, {
380+
key: typeof data.password === "string" ? [hash(data.password)] : undefined,
381+
});
382+
if (password) {
383+
return redirect(password);
376384
}
377385
console.error("Failed login attempt", JSON.stringify({
378386
xForwardedFor: request.headers["x-forwarded-for"],
@@ -432,19 +440,18 @@ export abstract class Server {
432440
: Promise.resolve({} as T);
433441
}
434442

435-
private authenticate(request: http.IncomingMessage, payload?: LoginPayload): string | boolean {
436-
if (this.options.auth !== "password") {
443+
private authenticate(request: http.IncomingMessage, payload?: AuthPayload): string | boolean {
444+
if (this.options.auth === "none") {
437445
return true;
438446
}
439447
const safeCompare = localRequire<typeof import("safe-compare")>("safe-compare/index");
440448
if (typeof payload === "undefined") {
441-
payload = this.parseCookies<LoginPayload>(request);
449+
payload = this.parseCookies<AuthPayload>(request);
442450
}
443-
if (this.options.password && payload.password) {
444-
const toTest = Array.isArray(payload.password) ? payload.password : [payload.password];
445-
for (let i = 0; i < toTest.length; ++i) {
446-
if (safeCompare(toTest[i], this.options.password)) {
447-
return toTest[i];
451+
if (this.options.password && payload.key) {
452+
for (let i = 0; i < payload.key.length; ++i) {
453+
if (safeCompare(payload.key[i], this.options.password)) {
454+
return payload.key[i];
448455
}
449456
}
450457
}

src/node/util.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,10 @@ export const generatePassword = async (length: number = 24): Promise<string> =>
6767
return buffer.toString("hex").substring(0, length);
6868
};
6969

70+
export const hash = (str: string): string => {
71+
return crypto.createHash("sha256").update(str).digest("hex");
72+
};
73+
7074
export const getMediaMime = (filePath?: string): string => {
7175
return filePath && (vsGetMediaMime(filePath) || (<{[index: string]: string}>{
7276
".css": "text/css",

0 commit comments

Comments
 (0)