0
Fork 0
mirror of https://github.com/withastro/astro.git synced 2025-03-31 23:31:30 -05:00

Encrypt server islands props (#11535)

* Encrypt server islands props

* Comment on the hex algo

* Use @oslojs/encoding

* Rename functions

* Add base to test

* Remove old tests no longer valid

* Run test locally

* Make sure adapters run before manifest

* Add a changeset

* Adjust test adapter

* don't assume adapter is at root

* Add a changeset

* Updates on review comments

* Update oslo

* Add better description of Node adapter change
This commit is contained in:
Matthew Phillips 2024-08-13 08:58:47 -04:00 committed by GitHub
parent 5c9183a8ee
commit 932bd2eb07
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
28 changed files with 199 additions and 48 deletions

View file

@ -0,0 +1,9 @@
---
'@astrojs/node': patch
---
Move polyfills up before awaiting the env module in the Node.js adapter.
Previously the env setting was happening before the polyfills were applied. This means that if the Astro env code (or any dependencies) depended on `crypto`, it would not be polyfilled in time.
Polyfills should be applied ASAP to prevent races. This moves it to the top of the Node adapter.

View file

@ -0,0 +1,9 @@
---
'astro': patch
---
Encrypt server island props
Server island props are not encrypted with a key generated at build-time. This is intended to prevent accidentally leaking secrets caused by exposing secrets through prop-passing. This is not intended to allow a server island to be trusted to skip authentication, or to protect against any other vulnerabilities other than secret leakage.
See the RFC for an explanation: https://github.com/withastro/roadmap/blob/server-islands/proposals/server-islands.md#props-serialization

View file

@ -1,4 +1,6 @@
---
const { secret } = Astro.props;
---
<h2 id="island">I am an island</h2>
<slot />
<h3 id="secret">{secret}</h3>

View file

@ -8,7 +8,7 @@ import Self from '../components/Self.astro';
<!-- Head Stuff -->
</head>
<body>
<Island server:defer>
<Island server:defer secret="test">
<h3 id="children">children</h3>
</Island>
<Self server:defer />

View file

@ -38,6 +38,12 @@ test.describe('Server islands', () => {
await expect(el, 'element rendered').toBeVisible();
});
test('Props are encrypted', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/base/'));
let el = page.locator('#secret');
await expect(el).toHaveText('test');
});
test('Self imported module can server defer', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/base/'));
let el = page.locator('.now');
@ -69,5 +75,11 @@ test.describe('Server islands', () => {
await expect(el, 'element rendered').toBeVisible();
await expect(el, 'should have content').toHaveText('I am an island');
});
test('Props are encrypted', async ({ page, astro }) => {
await page.goto(astro.resolveUrl('/'));
let el = page.locator('#secret');
await expect(el).toHaveText('test');
});
});
});

View file

@ -132,6 +132,7 @@
"@babel/plugin-transform-react-jsx": "^7.25.2",
"@babel/traverse": "^7.25.3",
"@babel/types": "^7.25.2",
"@oslojs/encoding": "^0.4.1",
"@types/babel__core": "^7.20.5",
"@types/cookie": "^0.6.0",
"acorn": "^8.12.1",

View file

@ -3283,6 +3283,7 @@ export interface SSRResult {
cookies: AstroCookies | undefined;
serverIslandNameMap: Map<string, string>;
trailingSlash: AstroConfig['trailingSlash'];
key: Promise<CryptoKey>;
_metadata: SSRMetadata;
}

View file

@ -25,6 +25,7 @@ import { getParts, validateSegment } from '../core/routing/manifest/create.js';
import { getPattern } from '../core/routing/manifest/pattern.js';
import type { AstroComponentFactory } from '../runtime/server/index.js';
import { ContainerPipeline } from './pipeline.js';
import { createKey } from '../core/encryption.js';
/**
* Options to be passed when rendering a route
@ -130,6 +131,7 @@ function createManifest(
checkOrigin: false,
middleware: manifest?.middleware ?? middleware ?? defaultMiddleware,
experimentalEnvGetSecretEnabled: false,
key: createKey(),
};
}

View file

@ -1,3 +1,4 @@
import { decodeKey } from '../encryption.js';
import { deserializeRouteData } from '../routing/manifest/serialization.js';
import type { RouteInfo, SSRManifest, SerializedSSRManifest } from './types.js';
@ -18,6 +19,7 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
const inlinedScripts = new Map(serializedManifest.inlinedScripts);
const clientDirectives = new Map(serializedManifest.clientDirectives);
const serverIslandNameMap = new Map(serializedManifest.serverIslandNameMap);
const key = decodeKey(serializedManifest.key);
return {
// in case user middleware exists, this no-op middleware will be reassigned (see plugin-ssr.ts)
@ -31,5 +33,6 @@ export function deserializeManifest(serializedManifest: SerializedSSRManifest):
clientDirectives,
routes,
serverIslandNameMap,
key,
};
}

View file

@ -416,13 +416,15 @@ export class App {
`${this.#baseWithoutTrailingSlash}/${status}${maybeDotHtml}`,
url,
);
const response = await fetch(statusURL.toString());
if(statusURL.toString() !== request.url) {
const response = await fetch(statusURL.toString());
// response for /404.html and 500.html is 200, which is not meaningful
// so we create an override
const override = { status };
// response for /404.html and 500.html is 200, which is not meaningful
// so we create an override
const override = { status };
return this.#mergeResponses(response, originalResponse, override);
return this.#mergeResponses(response, originalResponse, override);
}
}
const mod = await this.#pipeline.getModuleForRoute(errorRouteData);
try {

View file

@ -66,6 +66,7 @@ export type SSRManifest = {
pageMap?: Map<ComponentPath, ImportComponentInstance>;
serverIslandMap?: Map<string, () => Promise<ComponentInstance>>;
serverIslandNameMap?: Map<string, string>;
key: Promise<CryptoKey>;
i18n: SSRManifestI18n | undefined;
middleware: MiddlewareHandler;
checkOrigin: boolean;
@ -90,6 +91,7 @@ export type SerializedSSRManifest = Omit<
| 'inlinedScripts'
| 'clientDirectives'
| 'serverIslandNameMap'
| 'key'
> & {
routes: SerializedRouteInfo[];
assets: string[];
@ -97,4 +99,5 @@ export type SerializedSSRManifest = Omit<
inlinedScripts: [string, string][];
clientDirectives: [string, string][];
serverIslandNameMap: [string, string][];
key: string;
};

View file

@ -77,6 +77,7 @@ export async function generatePages(options: StaticBuildOptions, internals: Buil
internals,
renderers.renderers as SSRLoadedRenderer[],
middleware,
options.key,
);
}
const pipeline = BuildPipeline.create({ internals, manifest, options });
@ -521,6 +522,7 @@ function createBuildManifest(
internals: BuildInternals,
renderers: SSRLoadedRenderer[],
middleware: MiddlewareHandler,
key: Promise<CryptoKey>
): SSRManifest {
let i18nManifest: SSRManifestI18n | undefined = undefined;
if (settings.config.i18n) {
@ -551,6 +553,7 @@ function createBuildManifest(
buildFormat: settings.config.build.format,
middleware,
checkOrigin: settings.config.security?.checkOrigin ?? false,
key,
experimentalEnvGetSecretEnabled: false,
};
}

View file

@ -33,6 +33,7 @@ import { collectPagesData } from './page-data.js';
import { staticBuild, viteBuild } from './static-build.js';
import type { StaticBuildOptions } from './types.js';
import { getTimeStat } from './util.js';
import { createKey } from '../encryption.js';
export interface BuildOptions {
/**
@ -201,6 +202,7 @@ class AstroBuilder {
pageNames,
teardownCompiler: this.teardownCompiler,
viteConfig,
key: createKey(),
};
const { internals, ssrOutputChunkNames, contentFileNames } = await viteBuild(opts);

View file

@ -20,6 +20,7 @@ import { type BuildInternals, cssOrder, mergeInlineCss } from '../internal.js';
import type { AstroBuildPlugin } from '../plugin.js';
import type { StaticBuildOptions } from '../types.js';
import { makePageDataKey } from './util.js';
import { encodeKey } from '../../encryption.js';
const manifestReplace = '@@ASTRO_MANIFEST_REPLACE@@';
const replaceExp = new RegExp(`['"]${manifestReplace}['"]`, 'g');
@ -132,7 +133,8 @@ async function createManifest(
}
const staticFiles = internals.staticFiles;
return buildManifest(buildOpts, internals, Array.from(staticFiles));
const encodedKey = await encodeKey(await buildOpts.key);
return buildManifest(buildOpts, internals, Array.from(staticFiles), encodedKey);
}
/**
@ -150,6 +152,7 @@ function buildManifest(
opts: StaticBuildOptions,
internals: BuildInternals,
staticFiles: string[],
encodedKey: string,
): SerializedSSRManifest {
const { settings } = opts;
@ -277,6 +280,7 @@ function buildManifest(
buildFormat: settings.config.build.format,
checkOrigin: settings.config.security?.checkOrigin ?? false,
serverIslandNameMap: Array.from(settings.serverIslandNameMap),
key: encodedKey,
experimentalEnvGetSecretEnabled:
settings.config.experimental.env !== undefined &&
(settings.adapter?.supportedAstroFeatures.envGetSecret ?? 'unsupported') !== 'unsupported',

View file

@ -37,6 +37,11 @@ function vitePluginSSR(
inputs.add(getVirtualModulePageName(ASTRO_PAGE_MODULE_ID, pageData.component));
}
const adapterServerEntrypoint = options.settings.adapter?.serverEntrypoint;
if(adapterServerEntrypoint) {
inputs.add(adapterServerEntrypoint);
}
inputs.add(SSR_VIRTUAL_MODULE_ID);
return addRollupInput(opts, Array.from(inputs));
},
@ -246,8 +251,8 @@ function generateSSRCode(settings: AstroSettings, adapter: AstroAdapter, middlew
const imports = [
`import { renderers } from '${RENDERERS_MODULE_ID}';`,
`import { manifest as defaultManifest } from '${SSR_MANIFEST_VIRTUAL_MODULE_ID}';`,
`import * as serverEntrypointModule from '${adapter.serverEntrypoint}';`,
`import { manifest as defaultManifest } from '${SSR_MANIFEST_VIRTUAL_MODULE_ID}';`,
edgeMiddleware ? `` : `import { onRequest as middleware } from '${middlewareId}';`,
settings.config.experimental.serverIslands
? `import { serverIslandMap } from '${VIRTUAL_ISLAND_MAP_ID}';`

View file

@ -255,6 +255,8 @@ async function ssrBuild(
return 'renderers.mjs';
} else if (chunkInfo.facadeModuleId === RESOLVED_SSR_MANIFEST_VIRTUAL_MODULE_ID) {
return 'manifest_[hash].mjs';
} else if (chunkInfo.facadeModuleId === settings.adapter?.serverEntrypoint) {
return 'adapter_[hash].mjs';
} else if (
settings.config.experimental.contentCollectionCache &&
chunkInfo.facadeModuleId &&

View file

@ -42,6 +42,7 @@ export interface StaticBuildOptions {
pageNames: string[];
viteConfig: InlineConfig;
teardownCompiler: boolean;
key: Promise<CryptoKey>;
}
type ImportComponentInstance = () => Promise<ComponentInstance>;

View file

@ -0,0 +1,88 @@
import { encodeBase64, decodeBase64, decodeHex, encodeHexUpperCase } from '@oslojs/encoding';
// Chose this algorithm for no particular reason, can change.
// This algo does check against text manipulation though. See
// https://developer.mozilla.org/en-US/docs/Web/API/SubtleCrypto/encrypt#aes-gcm
const ALGORITHM = 'AES-GCM';
/**
* Creates a CryptoKey object that can be used to encrypt any string.
*/
export async function createKey() {
const key = await crypto.subtle.generateKey(
{
name: ALGORITHM,
length: 256,
},
true,
['encrypt', 'decrypt']
);
return key;
}
/**
* Takes a key that has been serialized to an array of bytes and returns a CryptoKey
*/
export async function importKey(bytes: Uint8Array): Promise<CryptoKey> {
const key = await crypto.subtle.importKey('raw', bytes, ALGORITHM, true, ['encrypt', 'decrypt']);
return key;
}
/**
* Encodes a CryptoKey to base64 string, so that it can be embedded in JSON / JavaScript
*/
export async function encodeKey(key: CryptoKey) {
const exported = await crypto.subtle.exportKey('raw', key);
const encodedKey = encodeBase64(new Uint8Array(exported));
return encodedKey;
}
/**
* Decodes a base64 string into bytes and then imports the key.
*/
export async function decodeKey(encoded: string): Promise<CryptoKey> {
const bytes = decodeBase64(encoded);
return crypto.subtle.importKey('raw', bytes, ALGORITHM, true, ['encrypt', 'decrypt']);
}
const encoder = new TextEncoder();
const decoder = new TextDecoder();
// The length of the initialization vector
// See https://developer.mozilla.org/en-US/docs/Web/API/AesGcmParams
const IV_LENGTH = 24;
/**
* Using a CryptoKey, encrypt a string into a base64 string.
*/
export async function encryptString(key: CryptoKey, raw: string) {
const iv = crypto.getRandomValues(new Uint8Array(IV_LENGTH / 2));
const data = encoder.encode(raw);
const buffer = await crypto.subtle.encrypt(
{
name: ALGORITHM,
iv,
},
key,
data
);
// iv is 12, hex brings it to 24
return encodeHexUpperCase(iv) + encodeBase64(new Uint8Array(buffer));
}
/**
* Takes a base64 encoded string, decodes it and returns the decrypted text.
*/
export async function decryptString(key: CryptoKey, encoded: string) {
const iv = decodeHex(encoded.slice(0, IV_LENGTH));
const dataArray = decodeBase64(encoded.slice(IV_LENGTH));
const decryptedBuffer = await crypto.subtle.decrypt(
{
name: ALGORITHM,
iv,
},
key,
dataArray
);
const decryptedString = decoder.decode(decryptedBuffer);
return decryptedString;
}

View file

@ -318,6 +318,7 @@ export class RenderContext {
? deserializeActionResult(this.locals._actionPayload.actionResult)
: undefined;
// Create the result object that will be passed into the renderPage function.
// This object starts here as an empty shell (not yet the result) but then
// calling the render() function will populate the object with scripts, styles, etc.
@ -344,6 +345,7 @@ export class RenderContext {
styles,
actionResult,
serverIslandNameMap: manifest.serverIslandNameMap ?? new Map(),
key: manifest.key,
trailingSlash: manifest.trailingSlash,
_metadata: {
hasHydrationScript: false,

View file

@ -11,6 +11,7 @@ import {
renderTemplate,
} from '../../runtime/server/index.js';
import { createSlotValueFromString } from '../../runtime/server/render/slot.js';
import { decryptString } from '../encryption.js';
import { getPattern } from '../routing/manifest/pattern.js';
export const SERVER_ISLAND_ROUTE = '/_server-islands/[name]';
@ -48,7 +49,7 @@ export function ensureServerIslandRoute(config: ConfigFields, routeManifest: Man
type RenderOptions = {
componentExport: string;
props: Record<string, any>;
encryptedProps: string;
slots: Record<string, string>;
};
@ -74,7 +75,11 @@ export function createEndpoint(manifest: SSRManifest) {
});
}
const props = data.props;
const key = await manifest.key;
const encryptedProps = data.encryptedProps;
const propString = await decryptString(key, encryptedProps);
const props = JSON.parse(propString);
const componentModule = await imp();
const Component = (componentModule as any)[data.componentExport];

View file

@ -1,8 +1,10 @@
import { encryptString } from '../../../core/encryption.js';
import type { SSRResult } from '../../../@types/astro.js';
import { renderChild } from './any.js';
import type { RenderInstance } from './common.js';
import { type ComponentSlots, renderSlotToString } from './slot.js';
const internalProps = new Set([
'server:component-path',
'server:component-export',
@ -59,6 +61,9 @@ export function renderServerIsland(
}
}
const key = await result.key;
const propsEncrypted = await encryptString(key, JSON.stringify(props));
const hostId = crypto.randomUUID();
const serverIslandUrl = `${result.base}_server-islands/${componentId}${result.trailingSlash === 'always' ? '/' : ''}`;
@ -68,7 +73,7 @@ let componentExport = ${safeJsonStringify(componentExport)};
let script = document.querySelector('script[data-island-id="${hostId}"]');
let data = {
componentExport,
props: ${safeJsonStringify(props)},
encryptedProps: ${safeJsonStringify(propsEncrypted)},
slots: ${safeJsonStringify(renderedSlots)},
};

View file

@ -18,6 +18,7 @@ import { recordServerError } from './error.js';
import { DevPipeline } from './pipeline.js';
import { handleRequest } from './request.js';
import { setRouteError } from './server-state.js';
import { createKey } from '../core/encryption.js';
export interface AstroPluginOptions {
settings: AstroSettings;
@ -129,6 +130,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
domainLookupTable: {},
};
}
return {
hrefRoot: settings.config.root.toString(),
trailingSlash: settings.config.trailingSlash,
@ -148,6 +150,7 @@ export function createDevelopmentManifest(settings: AstroSettings): SSRManifest
i18n: i18nManifest,
checkOrigin: settings.config.security?.checkOrigin ?? false,
experimentalEnvGetSecretEnabled: false,
key: createKey(),
middleware(_, next) {
return next();
},

View file

@ -82,38 +82,6 @@ describe('Server islands', () => {
const serverIslandScript = $('script[data-island-id]');
assert.equal(serverIslandScript.length, 1, 'has the island script');
});
describe('prod', () => {
async function fetchIsland() {
const app = await fixture.loadTestAdapterApp();
const request = new Request('http://example.com/_server-islands/Island', {
method: 'POST',
body: JSON.stringify({
componentExport: 'default',
props: {},
slots: {},
}),
});
return app.render(request);
}
it('Island returns its HTML', async () => {
const response = await fetchIsland();
const html = await response.text();
const $ = cheerio.load(html);
const serverIslandEl = $('h2#island');
assert.equal(serverIslandEl.length, 1);
});
it('Island does not include the doctype', async () => {
const response = await fetchIsland();
const html = await response.text();
console.log(html);
assert.ok(!/doctype/i.test(html), 'html does not include doctype');
});
});
});
});
});

View file

@ -72,7 +72,7 @@ export default function ({
async render(request, { routeData, clientAddress, locals, addCookieHeader } = {}) {
const url = new URL(request.url);
if(this.#manifest.assets.has(url.pathname)) {
const filePath = new URL('../client/' + this.removeBase(url.pathname), import.meta.url);
const filePath = new URL('../../client/' + this.removeBase(url.pathname), import.meta.url);
const data = await fs.promises.readFile(filePath);
return new Response(data);
}

View file

@ -103,7 +103,14 @@ function resolveClientDir(options: Options) {
const clientURLRaw = new URL(options.client);
const serverURLRaw = new URL(options.server);
const rel = path.relative(url.fileURLToPath(serverURLRaw), url.fileURLToPath(clientURLRaw));
const serverEntryURL = new URL(import.meta.url);
// walk up the parent folders until you find the one that is the root of the server entry folder. This is how we find the client folder relatively.
const serverFolder = path.basename(options.server);
let serverEntryFolderURL = path.dirname(import.meta.url);
while(!serverEntryFolderURL.endsWith(serverFolder)) {
serverEntryFolderURL = path.dirname(serverEntryFolderURL);
}
const serverEntryURL = serverEntryFolderURL + '/entry.mjs';
const clientURL = new URL(appendForwardSlash(rel), serverEntryURL);
const client = url.fileURLToPath(clientURL);
return client;

View file

@ -4,6 +4,8 @@ import createMiddleware from './middleware.js';
import { createStandaloneHandler } from './standalone.js';
import startServer from './standalone.js';
import type { Options } from './types.js';
// This needs to run first because some internals depend on `crypto`
applyPolyfills();
// Won't throw if the virtual module is not available because it's not supported in
// the users's astro version or if astro:env is not enabled in the project
@ -11,7 +13,6 @@ await import('astro/env/setup')
.then((mod) => mod.setGetEnv((key) => process.env[key]))
.catch(() => {});
applyPolyfills();
export function createExports(manifest: SSRManifest, options: Options) {
const app = new NodeApp(manifest);
options.trailingSlash = manifest.trailingSlash;

View file

@ -1,6 +1,7 @@
import assert from 'node:assert/strict';
import { after, before, describe, it } from 'node:test';
import { Worker } from 'node:worker_threads';
import { fileURLToPath } from 'node:url';
import * as cheerio from 'cheerio';
import nodejs from '../dist/index.js';
import { loadFixture } from './test-utils.js';
@ -29,7 +30,8 @@ describe('Errors', () => {
it('stays alive after offshoot promise rejections', async () => {
// this test needs to happen in a worker because node test runner adds a listener for unhandled rejections in the main thread
const worker = new Worker('./test/fixtures/errors/dist/server/entry.mjs', {
const url = new URL('./fixtures/errors/dist/server/entry.mjs', import.meta.url);
const worker = new Worker(fileURLToPath(url), {
type: 'module',
env: { ASTRO_NODE_LOGGING: 'enabled' },
});

11
pnpm-lock.yaml generated
View file

@ -582,6 +582,9 @@ importers:
'@babel/types':
specifier: ^7.25.2
version: 7.25.2
'@oslojs/encoding':
specifier: ^0.4.1
version: 0.4.1
'@types/babel__core':
specifier: ^7.20.5
version: 7.20.5
@ -7118,6 +7121,9 @@ packages:
'@octokit/types@13.5.0':
resolution: {integrity: sha512-HdqWTf5Z3qwDVlzCrP8UJquMwunpDiMPt5er+QjGzL4hqr/vBVY/MauQgS1xWxCDT1oMx1EULyqxncdCY/NVSQ==}
'@oslojs/encoding@0.4.1':
resolution: {integrity: sha512-hkjo6MuIK/kQR5CrGNdAPZhS01ZCXuWDRJ187zh6qqF2+yMHZpD9fAYpX8q2bOO6Ryhl3XpCT6kUX76N8hhm4Q==}
'@parse5/tools@0.3.0':
resolution: {integrity: sha512-zxRyTHkqb7WQMV8kTNBKWb1BeOFUKXBXTBWuxg9H9hfvQB3IwP6Iw2U75Ia5eyRxPNltmY7E8YAlz6zWwUnjKg==}
@ -8154,7 +8160,7 @@ packages:
resolution: {integrity: sha512-L3sHRo1pXXEqX8VU28kfgUY+YGsk09hPqZiZmLacNib6XNTCM8ubYeT7ryXQw8asB1sKgcU5lkB7ONug08aB8w==}
concat-map@0.0.1:
resolution: {integrity: sha512-/Srv4dswyQNBfohGpz9o6Yb3Gz3SrUDqBH5rTuhGR7ahtlbYKnVxw2bCFMRljaA7EXHaXZ8wsHdodFvbkhKmqg==}
resolution: {integrity: sha1-2Klr13/Wjfd5OnMDajug1UBdR3s=}
consola@3.2.3:
resolution: {integrity: sha512-I5qxpzLv+sJhTVEoLYNcTW+bThDCPsit0vLNKShZx6rLtpilNpmmeTPaeqJb9ZE9dV3DGaeby6Vuhrw38WjeyQ==}
@ -9333,6 +9339,7 @@ packages:
libsql@0.3.12:
resolution: {integrity: sha512-to30hj8O3DjS97wpbKN6ERZ8k66MN1IaOfFLR6oHqd25GMiPJ/ZX0VaZ7w+TsPmxcFS3p71qArj/hiedCyvXCg==}
cpu: [x64, arm64, wasm32]
os: [darwin, linux, win32]
lilconfig@2.1.0:
@ -12952,6 +12959,8 @@ snapshots:
dependencies:
'@octokit/openapi-types': 22.2.0
'@oslojs/encoding@0.4.1': {}
'@parse5/tools@0.3.0':
dependencies:
parse5: 7.1.2