Skip to content

Commit

Permalink
fix: miniflare now sets the "Host" header to match the upstream URL (#…
Browse files Browse the repository at this point in the history
…4630)

* fix: miniflare now sets the "Host" header to match the upstream URL

* feat: miniflare exposes unsafeProxySignature config that controls updating host from original URL header

* fix: ensure User Worker gets the correct Host header in wrangler dev local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes cloudflare/next-on-pages#588

* rename "proxy signature" to "proxy shared secret"

* Move proxy shared secret to where it is needed

This avoids prop drilling.

* Use timingSafeEqual for secret comparison

This helps to avoid an attacker guessing the secret via
the timing of the comparison.
  • Loading branch information
petebacondarwin committed Jan 3, 2024
1 parent 9e03272 commit 037de5e
Show file tree
Hide file tree
Showing 12 changed files with 244 additions and 46 deletions.
22 changes: 22 additions & 0 deletions .changeset/curly-colts-beam.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
---
"miniflare": patch
"wrangler": patch
---

fix: ensure User Worker gets the correct Host header in wrangler dev local mode

Some full-stack frameworks, such as Next.js, check that the Host header for a server
side action request matches the host where the application is expected to run.

In `wrangler dev` we have a Proxy Worker in between the browser and the actual User Worker.
This Proxy Worker is forwarding on the request from the browser, but then the actual User
Worker is running on a different host:port combination than that which the browser thinks
it should be on. This was causing the framework to think the request is malicious and blocking
it.

Now we update the request's Host header to that passed from the Proxy Worker in a custom `MF-Original-Url`
header, but only do this if the request also contains a shared secret between the Proxy Worker
and User Worker, which is passed via the `MF-Proxy-Shared-Secret` header. This last feature is to
prevent a malicious website from faking the Host header in a request directly to the User Worker.

Fixes https://github.com/cloudflare/next-on-pages/issues/588
4 changes: 3 additions & 1 deletion fixtures/worker-app/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ export default {
new Request("http://example.com", { method: "POST", body: "foo" })
);

return new Response(`${request.url} ${now()}`);
return new Response(
`${request.url} ${now()} ${request.headers.get("Host")}`
);
},

/**
Expand Down
3 changes: 3 additions & 0 deletions fixtures/worker-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ describe("'wrangler dev' correctly renders pages", () => {
expect(output).toContain("startup log");
expect(output).toContain("request log");

// check host on request in the Worker is as expected
expect(output).toContain(`host' => '${ip}:${port}'`);

// Check logged strings are source mapped
expect(output).toMatch(
/Error: logged error one.+fixtures\/worker-app\/src\/log.ts:7:14/s
Expand Down
2 changes: 1 addition & 1 deletion packages/miniflare/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
"test": "node scripts/build.mjs && ava && rimraf ./.tmp",
"test:ci": "pnpm run test",
"check:lint": "eslint \"{src,test}/**/*.ts\" \"scripts/**/*.{js,mjs}\" \"types/**/*.ts\"",
"lint:fix": "pnpm run lint -- --fix",
"lint:fix": "pnpm run check:lint --fix",
"types:build": "node scripts/types.mjs tsconfig.json && node scripts/types.mjs src/workers/tsconfig.json"
},
"bin": {
Expand Down
10 changes: 10 additions & 0 deletions packages/miniflare/src/plugins/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,10 @@ export const CoreSharedOptionsSchema = z.object({
cf: z.union([z.boolean(), z.string(), z.record(z.any())]).optional(),

liveReload: z.boolean().optional(),
// This is a shared secret between a proxy server and miniflare that can be
// passed in a header to prove that the request came from the proxy and not
// some malicious attacker.
unsafeProxySharedSecret: z.string().optional(),
});

export const CORE_PLUGIN_NAME = "core";
Expand Down Expand Up @@ -651,6 +655,12 @@ export function getGlobalServices({
text: sharedOptions.upstream,
});
}
if (sharedOptions.unsafeProxySharedSecret !== undefined) {
serviceEntryBindings.push({
name: CoreBindings.DATA_PROXY_SHARED_SECRET,
data: encoder.encode(sharedOptions.unsafeProxySharedSecret),
});
}
if (sharedOptions.liveReload) {
const liveReloadScript = LIVE_RELOAD_SCRIPT_TEMPLATE(loopbackPort);
serviceEntryBindings.push({
Expand Down
2 changes: 2 additions & 0 deletions packages/miniflare/src/workers/core/constants.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
export const CoreHeaders = {
CUSTOM_SERVICE: "MF-Custom-Service",
ORIGINAL_URL: "MF-Original-URL",
PROXY_SHARED_SECRET: "MF-Proxy-Shared-Secret",
DISABLE_PRETTY_ERROR: "MF-Disable-Pretty-Error",
ERROR_STACK: "MF-Experimental-Error-Stack",
ROUTE_OVERRIDE: "MF-Route-Override",
Expand All @@ -27,6 +28,7 @@ export const CoreBindings = {
DATA_LIVE_RELOAD_SCRIPT: "MINIFLARE_LIVE_RELOAD_SCRIPT",
DURABLE_OBJECT_NAMESPACE_PROXY: "MINIFLARE_PROXY",
DATA_PROXY_SECRET: "MINIFLARE_PROXY_SECRET",
DATA_PROXY_SHARED_SECRET: "MINIFLARE_PROXY_SHARED_SECRET",
} as const;

export const ProxyOps = {
Expand Down
53 changes: 50 additions & 3 deletions packages/miniflare/src/workers/core/entry.worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
reset,
yellow,
} from "kleur/colors";
import { LogLevel, SharedHeaders } from "miniflare:shared";
import { HttpError, LogLevel, SharedHeaders } from "miniflare:shared";
import { CoreBindings, CoreHeaders } from "./constants";
import { STATUS_CODES } from "./http";
import { WorkerRoute, matchRoutes } from "./routing";
Expand All @@ -23,25 +23,61 @@ type Env = {
[CoreBindings.JSON_LOG_LEVEL]: LogLevel;
[CoreBindings.DATA_LIVE_RELOAD_SCRIPT]: ArrayBuffer;
[CoreBindings.DURABLE_OBJECT_NAMESPACE_PROXY]: DurableObjectNamespace;
[CoreBindings.DATA_PROXY_SHARED_SECRET]?: Uint8Array;
} & {
[K in `${typeof CoreBindings.SERVICE_USER_ROUTE_PREFIX}${string}`]:
| Fetcher
| undefined; // Won't have a `Fetcher` for every possible `string`
};

const encoder = new TextEncoder();

function getUserRequest(
request: Request<unknown, IncomingRequestCfProperties>,
env: Env
) {
// The ORIGINAL_URL header is added to outbound requests from Miniflare,
// triggered either by calling Miniflare.#dispatchFetch(request),
// or as part of a loopback request in a Custom Service.
// The ORIGINAL_URL is extracted from the `request` being sent.
// This is relevant here in the case that a Miniflare implemented Proxy Worker is
// sitting in front of this User Worker, which is hosted on a different URL.
const originalUrl = request.headers.get(CoreHeaders.ORIGINAL_URL);
const upstreamUrl = env[CoreBindings.TEXT_UPSTREAM_URL];
let url = new URL(originalUrl ?? request.url);

// The `upstreamHost` is used to override the `Host` header on the request being handled.
let upstreamHost: string | undefined;

// If the request is signed by an upstream proxy then we can use the one from the ORIGINAL_URL.
// The shared secret is required to prevent a malicious user being able to change the host header without permission.
const proxySharedSecret = request.headers.get(
CoreHeaders.PROXY_SHARED_SECRET
);
if (proxySharedSecret) {
const secretFromHeader = encoder.encode(proxySharedSecret);
const configuredSecret = env[CoreBindings.DATA_PROXY_SHARED_SECRET];
if (
secretFromHeader.byteLength === configuredSecret?.byteLength &&
crypto.subtle.timingSafeEqual(secretFromHeader, configuredSecret)
) {
upstreamHost = url.host;
} else {
throw new HttpError(
400,
`Disallowed header in request: ${CoreHeaders.PROXY_SHARED_SECRET}=${proxySharedSecret}`
);
}
}

// If Miniflare was configured with `upstream`, then we use this to override the url and host in the request.
const upstreamUrl = env[CoreBindings.TEXT_UPSTREAM_URL];
if (upstreamUrl !== undefined) {
// If a custom `upstream` was specified, make sure the URL starts with it
let path = url.pathname + url.search;
// Remove leading slash, so we resolve relative to `upstream`'s path
if (path.startsWith("/")) path = `./${path.substring(1)}`;
url = new URL(path, upstreamUrl);
upstreamHost = url.host;
}

// Note when constructing new `Request`s from `request`, we must always pass
Expand All @@ -56,6 +92,10 @@ function getUserRequest(
if (request.cf === undefined) {
request = new Request(request, { cf: env[CoreBindings.JSON_CF_BLOB] });
}
if (upstreamHost !== undefined) {
request.headers.set("Host", upstreamHost);
}
request.headers.delete(CoreHeaders.PROXY_SHARED_SECRET);
request.headers.delete(CoreHeaders.ORIGINAL_URL);
request.headers.delete(CoreHeaders.DISABLE_PRETTY_ERROR);
return request;
Expand Down Expand Up @@ -209,7 +249,14 @@ export default <ExportedHandler<Env>>{
const disablePrettyErrorPage =
request.headers.get(CoreHeaders.DISABLE_PRETTY_ERROR) !== null;

request = getUserRequest(request, env);
try {
request = getUserRequest(request, env);
} catch (e) {
if (e instanceof HttpError) {
return e.toResponse();
}
throw e;
}
const url = new URL(request.url);
const service = getTargetService(request, url, env);
if (service === undefined) {
Expand Down
99 changes: 95 additions & 4 deletions packages/miniflare/test/index.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -576,15 +576,106 @@ test("Miniflare: custom upstream as origin", async (t) => {
upstream: new URL("/extra/", upstream.http.toString()).toString(),
modules: true,
script: `export default {
fetch(request) {
return fetch(request);
async fetch(request) {
const resp = await (await fetch(request)).text();
return Response.json({
resp,
host: request.headers.get("Host")
});
}
}`,
});
t.teardown(() => mf.dispose());
// Check rewrites protocol, hostname, and port, but keeps pathname and query
const res = await mf.dispatchFetch("https://random:0/path?a=1");
t.is(await res.text(), "upstream: http://upstream/extra/path?a=1");
t.deepEqual(await res.json(), {
resp: "upstream: http://upstream/extra/path?a=1",
host: upstream.http.host,
});
});
test("Miniflare: set origin to original URL if proxy shared secret matches", async (t) => {
const mf = new Miniflare({
unsafeProxySharedSecret: "SOME_PROXY_SHARED_SECRET_VALUE",
modules: true,
script: `export default {
async fetch(request) {
return Response.json({
host: request.headers.get("Host")
});
}
}`,
});
t.teardown(() => mf.dispose());

const res = await mf.dispatchFetch("https://random:0/path?a=1", {
headers: { "MF-Proxy-Shared-Secret": "SOME_PROXY_SHARED_SECRET_VALUE" },
});
t.deepEqual(await res.json(), {
host: "random:0",
});
});
test("Miniflare: keep origin as listening host if proxy shared secret not provided", async (t) => {
const mf = new Miniflare({
modules: true,
script: `export default {
async fetch(request) {
return Response.json({
host: request.headers.get("Host")
});
}
}`,
});
t.teardown(() => mf.dispose());

const res = await mf.dispatchFetch("https://random:0/path?a=1");
t.deepEqual(await res.json(), {
host: (await mf.ready).host,
});
});
test("Miniflare: 400 error on proxy shared secret header when not configured", async (t) => {
const mf = new Miniflare({
modules: true,
script: `export default {
async fetch(request) {
return Response.json({
host: request.headers.get("Host")
});
}
}`,
});
t.teardown(() => mf.dispose());

const res = await mf.dispatchFetch("https://random:0/path?a=1", {
headers: { "MF-Proxy-Shared-Secret": "SOME_PROXY_SHARED_SECRET_VALUE" },
});
t.is(res.status, 400);
t.is(
await res.text(),
"Disallowed header in request: MF-Proxy-Shared-Secret=SOME_PROXY_SHARED_SECRET_VALUE"
);
});
test("Miniflare: 400 error on proxy shared secret header mismatch with configuration", async (t) => {
const mf = new Miniflare({
unsafeProxySharedSecret: "SOME_PROXY_SHARED_SECRET_VALUE",
modules: true,
script: `export default {
async fetch(request) {
return Response.json({
host: request.headers.get("Host")
});
}
}`,
});
t.teardown(() => mf.dispose());

const res = await mf.dispatchFetch("https://random:0/path?a=1", {
headers: { "MF-Proxy-Shared-Secret": "BAD_PROXY_SHARED_SECRET" },
});
t.is(res.status, 400);
t.is(
await res.text(),
"Disallowed header in request: MF-Proxy-Shared-Secret=BAD_PROXY_SHARED_SECRET"
);
});

test("Miniflare: `node:`, `cloudflare:` and `workerd:` modules", async (t) => {
Expand Down Expand Up @@ -1228,7 +1319,7 @@ test("Miniflare: supports wrapped bindings", async (t) => {
await this.STORE.fetch(this.baseURL + key, { method: "DELETE" });
}
}
export default function (env) {
return new MiniKV(env);
}
Expand Down
6 changes: 5 additions & 1 deletion packages/wrangler/src/dev/local.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,11 @@ function useLocalWorker(props: LocalProps) {
hostname: props.localUpstream,
port: props.localUpstream ? "" : undefined, // `localUpstream` was essentially `host`, not `hostname`, so if it was set delete the `port`
},
headers: {}, // no headers needed in local-mode
headers: {
// Passing this signature from Proxy Worker allows the User Worker to trust the request.
"MF-Proxy-Shared-Secret":
event.proxyToUserWorkerAuthenticationSecret,
},
liveReload: props.liveReload,
// in local mode, the logs are already being printed to the console by workerd but only for workers written in "module" format
// workers written in "service-worker" format still need to proxy logs to the ProxyController
Expand Down
18 changes: 16 additions & 2 deletions packages/wrangler/src/dev/miniflare.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import assert from "node:assert";
import { type UUID, randomUUID } from "node:crypto";
import { realpathSync } from "node:fs";
import path from "node:path";
import { Log, LogLevel, TypedEventTarget, Mutex, Miniflare } from "miniflare";
Expand Down Expand Up @@ -509,7 +510,8 @@ export function handleRuntimeStdio(stdout: Readable, stderr: Readable) {

async function buildMiniflareOptions(
log: Log,
config: ConfigBundle
config: ConfigBundle,
proxyToUserWorkerAuthenticationSecret: UUID
): Promise<{ options: MiniflareOptions; internalObjects: CfDurableObject[] }> {
if (config.crons.length > 0) {
logger.warn("Miniflare 3 does not support CRON triggers yet, ignoring...");
Expand Down Expand Up @@ -554,6 +556,7 @@ async function buildMiniflareOptions(
inspectorPort: config.inspect ? config.inspectorPort : undefined,
liveReload: config.liveReload,
upstream,
unsafeProxySharedSecret: proxyToUserWorkerAuthenticationSecret,

log,
verbose: logger.loggerLevel === "debug",
Expand All @@ -580,15 +583,19 @@ async function buildMiniflareOptions(
export interface ReloadedEventOptions {
url: URL;
internalDurableObjects: CfDurableObject[];
proxyToUserWorkerAuthenticationSecret: UUID;
}
export class ReloadedEvent extends Event implements ReloadedEventOptions {
readonly url: URL;
readonly internalDurableObjects: CfDurableObject[];
readonly proxyToUserWorkerAuthenticationSecret: UUID;

constructor(type: "reloaded", options: ReloadedEventOptions) {
super(type);
this.url = options.url;
this.internalDurableObjects = options.internalDurableObjects;
this.proxyToUserWorkerAuthenticationSecret =
options.proxyToUserWorkerAuthenticationSecret;
}
}

Expand All @@ -611,6 +618,10 @@ export type MiniflareServerEventMap = {
export class MiniflareServer extends TypedEventTarget<MiniflareServerEventMap> {
#log = buildLog();
#mf?: Miniflare;
// This is given as a shared secret to the Proxy and User workers
// so that the User Worker can trust aspects of HTTP requests from the Proxy Worker
// if it provides the secret in a `MF-Proxy-Shared-Secret` header.
#proxyToUserWorkerAuthenticationSecret = randomUUID();

// `buildMiniflareOptions()` is asynchronous, meaning if multiple bundle
// updates were submitted, the second may apply before the first. Therefore,
Expand All @@ -622,7 +633,8 @@ export class MiniflareServer extends TypedEventTarget<MiniflareServerEventMap> {
try {
const { options, internalObjects } = await buildMiniflareOptions(
this.#log,
config
config,
this.#proxyToUserWorkerAuthenticationSecret
);
if (opts?.signal?.aborted) return;
if (this.#mf === undefined) {
Expand All @@ -635,6 +647,8 @@ export class MiniflareServer extends TypedEventTarget<MiniflareServerEventMap> {
const event = new ReloadedEvent("reloaded", {
url,
internalDurableObjects: internalObjects,
proxyToUserWorkerAuthenticationSecret:
this.#proxyToUserWorkerAuthenticationSecret,
});
this.dispatchEvent(event);
} catch (error: unknown) {
Expand Down

0 comments on commit 037de5e

Please sign in to comment.