fix: ensure that allowed scripts are in hashes module

Signed-off-by: Andres Correa Casablanca <andreu@kindspells.dev>
kindspells · Mar 31, 2024 · 1221019 · 1221019
1 parent db10a41
commit 1221019
Show file tree

Hide file tree

Showing 6 changed files with 132 additions and 43 deletions.
diff --git a/.editorconfig b/.editorconfig
@@ -14,6 +14,8 @@ indent_style = tab
 indent_size = 2
 trim_trailing_whitespace = true
 
-[*.md]
+[*.{md,mdx}]
+charset = utf-8
 indent_style = space
 indent_size = 2
+trim_trailing_whitespace = true
diff --git a/@kindspells/astro-shield/e2e/e2e.test.mts b/@kindspells/astro-shield/e2e/e2e.test.mts
@@ -23,6 +23,7 @@ import {
 	it,
 } from 'vitest'
 
+import type { HashesModule } from '#as/core.mjs'
 import { generateSRIHash } from '#as/core.mjs'
 import { doesFileExist } from '#as/fs.mjs'
 
@@ -530,4 +531,24 @@ describe('middleware (hybrid 3)', () => {
 			"default-src 'none'; frame-ancestors 'none'; script-src 'self' 'sha256-X7QGGDHgf6XMoabXvV9pW7gl3ALyZhZlgKq1s3pwmME='; style-src 'self' 'sha256-9U7mv8FibD/D9IbGpXc86pz37l6/w4PCLpFIZuPrzh8=' 'sha256-ZlgyI5Bx/aeAyk/wSIypqeIM5PBhz9IiAek9HIiAjaI='",
 		)
 	})
+
+	it('incorporates the allowed scripts into the generated hashes module', async () => {
+		const hashesModulePath = resolve(hybridDir, 'src', 'generated', 'sri.mjs')
+		assert(await doesFileExist(hashesModulePath))
+
+		const hashesModule = (await import(hashesModulePath)) as HashesModule
+
+		assert(
+			Object.hasOwn(
+				hashesModule.perResourceSriHashes.scripts,
+				'https://code.jquery.com/jquery-3.7.1.slim.min.js',
+			),
+		)
+		assert(
+			Object.hasOwn(
+				hashesModule.perResourceSriHashes.scripts,
+				'https://code.jquery.com/ui/1.13.2/jquery-ui.min.js',
+			),
+		)
+	})
 })
diff --git a/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs b/@kindspells/astro-shield/e2e/fixtures/hybrid3/astro.config.mjs
@@ -24,9 +24,15 @@ export default defineConfig({
 	adapter: node({ mode: 'standalone' }),
 	integrations: [
 		shield({
-			enableStatic_SRI: true,
-			enableMiddleware_SRI: true,
-			sriHashesModule,
+			sri: {
+				enableStatic: true,
+				enableMiddleware: true,
+				hashesModule: sriHashesModule,
+				scriptsAllowListUrls: [
+					'https://code.jquery.com/jquery-3.7.1.slim.min.js',
+					'https://code.jquery.com/ui/1.13.2/jquery-ui.min.js',
+				],
+			},
 			securityHeaders: {
 				contentSecurityPolicy: {
 					cspDirectives: {

diff --git a/@kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro b/@kindspells/astro-shield/e2e/fixtures/hybrid3/src/pages/injected.astro
@@ -0,0 +1,32 @@
+---
+/*
+ * SPDX-FileCopyrightText: 2024 KindSpells Labs S.L.
+ *
+ * SPDX-License-Identifier: MIT
+ */
+
+export const prerender = false
+import '../styles/main.css'
+---
+<!DOCTYPE html><html lang="en">
+	<head>
+		<meta charset="utf-8" />
+		<meta name="viewport" content="width=device-width, initial-scale=1" />
+		<title>My Static Test Site</title>
+		<style>h1 { color: red; }</style>
+	</head>
+	<body>
+		<!-- The next script is whitelisted -->
+		<script
+			src="https://code.jquery.com/jquery-3.7.1.slim.min.js"
+			integrity="sha256-kmHvs0B+OpCW5GVHUNjv9rOmY0IvSIRcf7zGUDTDQM8="
+			crossorigin="anonymous"
+		></script>
+        <!-- The next script is whitelisted, but we let Astro-Shield obtain its integrity hash -->
+		<script src="https://code.jquery.com/ui/1.13.2/jquery-ui.min.js" type="module"></script>
+
+		<!-- The next script, although not malicious, is used to simulate an injection -->
+		<script src="https://code.jquery.com/jquery-2.2.4.min.js" integrity="sha256-BbhdlvQf/xTY9gja0Dq3HiwQF8LaCRTXxZKRutelT44="></script>
+		<p>This document simulates a page showing an injected script</p>
+	</body>
+</html>
diff --git a/@kindspells/astro-shield/src/core.mjs b/@kindspells/astro-shield/src/core.mjs
@@ -21,6 +21,13 @@ import { patchHeaders } from './headers.mjs'
  * @typedef {import('./core.js').MiddlewareHashes} MiddlewareHashes
  * @typedef {import('./core.js').Logger} Logger
  * @typedef {import('astro').AstroIntegration} Integration
+ * @typedef {{
+ * 	[k in keyof HashesCollection]: HashesCollection[k] extends Set<string>
+ * 	  ? string[] | undefined
+ * 	  : (k extends 'perPageSriHashes'
+ * 	    ? Record<string, { scripts: string[]; styles: string [] }>
+ * 	    : Record<'scripts' | 'styles', Record<string, string>>)
+ * }} HashesModule
  */
 
 /**
@@ -529,11 +536,11 @@ export const scanForNestedResources = async (logger, dirPath, h) => {
 }
 
 /**
- * @param {Required<Pick<SRIOptions, 'scriptsAllowListUrls' | 'stylesAllowListUrls'>>} sri
+ * @param {Pick<SRIOptions, 'scriptsAllowListUrls' | 'stylesAllowListUrls'>} sri
  * @param {HashesCollection} h
  */
 export const scanAllowLists = async (sri, h) => {
-	for (const scriptUrl of sri.scriptsAllowListUrls) {
+	for (const scriptUrl of sri.scriptsAllowListUrls ?? []) {
 		const resourceResponse = await fetch(scriptUrl, { method: 'GET' })
 		const resourceContent = await resourceResponse.arrayBuffer()
 		const sriHash = generateSRIHash(resourceContent)
@@ -542,7 +549,7 @@ export const scanAllowLists = async (sri, h) => {
 		h.perResourceSriHashes.scripts.set(scriptUrl, sriHash)
 	}
 
-	for (const styleUrl of sri.stylesAllowListUrls) {
+	for (const styleUrl of sri.stylesAllowListUrls ?? []) {
 		const resourceResponse = await fetch(styleUrl, { method: 'GET' })
 		const resourceContent = await resourceResponse.arrayBuffer()
 		const sriHash = generateSRIHash(resourceContent)
@@ -591,14 +598,9 @@ export async function generateSRIHashesModule(
 	}
 
 	if (await doesFileExist(sriHashesModule)) {
-		const hModule = /**
-			@type {{
-					[k in keyof HashesCollection]: HashesCollection[k] extends Set<string>
-							? string[] | undefined
-							: (k extends 'perPageSriHashes'
-								? Record<string, { scripts: string[]; styles: string [] }>
-								: Record<'scripts' | 'styles', Record<string, string>>)
-		}} */ (await import(/* @vite-ignore */ sriHashesModule))
+		const hModule = /** @type {HashesModule} */ (
+			await import(/* @vite-ignore */ sriHashesModule)
+		)
 
 		extResourceHashesChanged = !sriHashesEqual(
 			perResourceHashes,
@@ -683,6 +685,8 @@ export const processStaticFiles = async (logger, { distDir, sri }) => {
 			styles: new Map(),
 		},
 	}
+	await scanAllowLists(sri, h)
+	await scanForNestedResources(logger, distDir, h)
 	await scanDirectory(
 		logger,
 		distDir,
@@ -693,7 +697,6 @@ export const processStaticFiles = async (logger, { distDir, sri }) => {
 		sri,
 	)
 
-	await scanForNestedResources(logger, distDir, h)
 
 	if (!sri.hashesModule) {
 		return
@@ -790,33 +793,58 @@ const loadVirtualMiddlewareModule = async (
 	let extraImports = ''
 	let staticHashesModuleLoader = ''
 
-	if (
-		sri.enableStatic &&
-		sri.hashesModule &&
-		!(await doesFileExist(sri.hashesModule))
-	) {
-		const h = /** @satisfies {HashesCollection} */ {
-			inlineScriptHashes: new Set(),
-			inlineStyleHashes: new Set(),
-			extScriptHashes: new Set(),
-			extStyleHashes: new Set(),
-			perPageSriHashes: new Map(),
-			perResourceSriHashes: {
-				scripts: new Map(),
-				styles: new Map(),
-			},
+	if (sri.enableStatic && sri.hashesModule) {
+		let shouldRegenerateHashesModule = !(await doesFileExist(sri.hashesModule))
+
+		if (!shouldRegenerateHashesModule) {
+			try {
+				const hashesModule = /** @type {HashesModule} */ (
+					await import(sri.hashesModule)
+				)
+
+				for (const allowedScript of sri.scriptsAllowListUrls) {
+					if (
+						!Object.hasOwn(
+							hashesModule.perResourceSriHashes.scripts,
+							allowedScript,
+						)
+					) {
+						shouldRegenerateHashesModule = true
+						break
+					}
+				}
+			} catch (err) {
+				logger.warn(
+					`Failed to load SRI hashes module "${sri.hashesModule}", it will be re-generated:\n\t${err}`,
+				)
+				shouldRegenerateHashesModule = true
+			}
 		}
 
-		// We generate a provisional hashes module. It won't contain the hashes for
-		// resources created by Astro, but it can be useful nonetheless.
-		await scanForNestedResources(logger, publicDir, h)
-		await scanAllowLists(sri, h)
-		await generateSRIHashesModule(
-			logger,
-			h,
-			sri.hashesModule,
-			false, // So we don't get redundant warnings
-		)
+		if (shouldRegenerateHashesModule) {
+			const h = /** @satisfies {HashesCollection} */ {
+				inlineScriptHashes: new Set(),
+				inlineStyleHashes: new Set(),
+				extScriptHashes: new Set(),
+				extStyleHashes: new Set(),
+				perPageSriHashes: new Map(),
+				perResourceSriHashes: {
+					scripts: new Map(),
+					styles: new Map(),
+				},
+			}
+
+			// We generate a provisional hashes module. It won't contain the hashes for
+			// resources created by Astro, but it can be useful nonetheless.
+			await scanForNestedResources(logger, publicDir, h)
+			await scanAllowLists(sri, h)
+			await generateSRIHashesModule(
+				logger,
+				h,
+				sri.hashesModule,
+				false, // So we don't get redundant warnings
+			)
+		}
 	}
 
 	if (

diff --git a/@kindspells/astro-shield/tests/core.test.mts b/@kindspells/astro-shield/tests/core.test.mts
@@ -365,7 +365,7 @@ describe('updateStaticPageSriHashes', () => {
 				<title>My Test Page</title>
 			</head>
 			<body>
-				<script type="module" src="/core.mjs" integrity="sha256-iozyX5cgvSGJZLKhhN7CRl6tn/jC3vYkBm8jfGv4x78="></script>
+				<script type="module" src="/core.mjs" integrity="sha256-zOEqmAz4SCAi+TcSQgdhUuurJfrfnwWqtmdTOP+bBkc="></script>
 			</body>
 		</html>`
 
@@ -382,7 +382,7 @@ describe('updateStaticPageSriHashes', () => {
 		expect(h.extScriptHashes.size).toBe(1)
 		expect(
 			h.extScriptHashes.has(
-				'sha256-iozyX5cgvSGJZLKhhN7CRl6tn/jC3vYkBm8jfGv4x78=',
+				'sha256-zOEqmAz4SCAi+TcSQgdhUuurJfrfnwWqtmdTOP+bBkc=',
 			),
 		).toBe(true)
 		expect(h.inlineScriptHashes.size).toBe(0)