Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

new_audit: ensure clickjacking mitigation through XFO or CSP #16290

Merged
merged 35 commits into from
Jan 22, 2025
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
2df023e
Add files to gitignore.
sebastian9er Nov 15, 2024
f95d133
Merge remote-tracking branch 'upstream/main'
sebastian9er Nov 15, 2024
e52e1b8
Merge branch 'GoogleChrome:main' into main
sebastian9er Nov 27, 2024
3861727
Merge branch 'main' of https://github.com/sebastian9er/lighthouse
sebastian9er Dec 4, 2024
8717e3c
Commit gitignore
sebastian9er Dec 4, 2024
009af4e
Merge branch 'GoogleChrome:main' into main
sebastian9er Dec 12, 2024
069260b
Add first version for the clickjacking mitigation audit for Lighthouse.
sebastian9er Dec 12, 2024
8b123ab
Revert files requested in the PR.
sebastian9er Dec 12, 2024
aac2789
Revert more files.
sebastian9er Dec 12, 2024
cd5761f
Fix year in the smokehouse-bin.
sebastian9er Dec 12, 2024
6c11434
Fix default config.
sebastian9er Dec 12, 2024
de85518
Update sample json.
sebastian9er Dec 13, 2024
7f10f7f
Test commit to Smokehouse-bin.
sebastian9er Dec 13, 2024
ba82107
Revert smokehouse-bin.
sebastian9er Dec 13, 2024
feabb42
Add Smokehouse tests for the Clickjacking Lighthouse audit.
sebastian9er Dec 13, 2024
6d5b543
Merge branch 'GoogleChrome:main' into lighthouse-clickjacking
sebastian9er Dec 13, 2024
9eaca8f
Adjust Clickjacking audit to pass even if frame-ancestors does not ha…
sebastian9er Dec 18, 2024
268229b
Replace placeholder link with potential chrome dev post (pending inte…
sebastian9er Dec 18, 2024
5d86c36
Update sample json.
sebastian9er Dec 18, 2024
1717db0
Remove unnecessary meta element lookups.
sebastian9er Dec 19, 2024
e00fa47
Merge branch 'main' into lighthouse-clickjacking
sebastian9er Jan 8, 2025
003c7b2
Fix some improvement recommendations discussed in the PR.
sebastian9er Jan 8, 2025
280aac5
Smokehouse-bin test commit.
sebastian9er Jan 9, 2025
6e8b39b
Revert smokehouse-bin.
sebastian9er Jan 9, 2025
2297534
Remove Directive column from Clickjacking audit. Fix tests and re-cre…
sebastian9er Jan 10, 2025
e0bd178
smokehouse revert
adamraine Jan 10, 2025
16bbf1f
Fix tests to reflect wording changes in audit strings. Also, fix lint…
sebastian9er Jan 10, 2025
4e94793
Merge branch 'lighthouse-clickjacking' of https://github.com/sebastia…
sebastian9er Jan 10, 2025
11336ba
Fix smokehouse test.
sebastian9er Jan 10, 2025
8779df7
tests fix
adamraine Jan 14, 2025
57ceae6
sample
adamraine Jan 14, 2025
f64ce1e
oops
adamraine Jan 14, 2025
b98f5f9
Merge branch 'main' into lighthouse-clickjacking
adamraine Jan 14, 2025
42b0afe
update text from docs feedback
adamraine Jan 21, 2025
5a2c5dd
tests
adamraine Jan 21, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions cli/test/smokehouse/core-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
import a11y from './test-definitions/a11y.js';
import byteEfficiency from './test-definitions/byte-efficiency.js';
import byteGzip from './test-definitions/byte-gzip.js';
import clickjackingMissingHeaders from './test-definitions/clickjacking-missing-headers.js';
import clickjackingMitigationPresent from './test-definitions/clickjacking-mitigation-headers-present.js';
import crash from './test-definitions/crash.js';
import cspAllowAll from './test-definitions/csp-allow-all.js';
import cspBlockAll from './test-definitions/csp-block-all.js';
Expand Down Expand Up @@ -69,6 +71,8 @@ const smokeTests = [
a11y,
byteEfficiency,
byteGzip,
clickjackingMissingHeaders,
clickjackingMitigationPresent,
crash,
cspAllowAll,
cspBlockAll,
Expand Down
Empty file modified cli/test/smokehouse/frontends/smokehouse-bin.js
100644 → 100755
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results for a site with missing Clickjacking mitigations
* (through the X-Frame-Options or Content-Security-Policy headers).
*/
const expectations = {
lhr: {
requestedUrl: 'https://example.com/',
finalDisplayedUrl: 'https://example.com/',
audits: {
'clickjacking-mitigation': {
score: 0,
details: {
items: [
{
description: 'No Clickjacking mitigation found.',
severity: 'High',
},
],
},
},
},
},
};

export default {
id: 'clickjacking-missing-headers',
expectations,
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results for a site with present Clickjacking mitigations
* (through the X-Frame-Options or Content-Security-Policy headers).
*/
const expectations = {
lhr: {
requestedUrl: 'https://developer.mozilla.org/en-US/',
finalDisplayedUrl: 'https://developer.mozilla.org/en-US/',
audits: {
'clickjacking-mitigation': {
score: null,
},
},
},
};

export default {
id: 'clickjacking-mitigation-headers-present',
expectations,
};
159 changes: 159 additions & 0 deletions core/audits/clickjacking-mitigation.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

import {Audit} from './audit.js';
import {MainResource} from '../computed/main-resource.js';
import * as i18n from '../lib/i18n/i18n.js';

const UIStrings = {
/** Title of a Lighthouse audit that evaluates whether the set CSP or XFO header is mitigating Clickjacking attacks. "XFO" stands for "X-Frame-Options". "CSP" stands for "Content-Security-Policy". */
title: 'Ensure Clickjacking mitigation through XFO or CSP.',
/** Description of a Lighthouse audit that evaluates whether the set CSP or XFO header is mitigating Clickjacking attacks. This is displayed after a user expands the section to see more. No character length limits. The last sentence starting with 'Learn' becomes link text to additional documentation. "XFO" stands for "X-Frame-Options". "CSP" stands for "Content-Security-Policy". */
description: 'Deployment of either the X-Frame-Options or Content-Security-Policy (with the frame-ancestors directive) header will prevent Clickjacking attacks. While the XFO header is simpler to deploy, the CSP header is more flexible. [Learn more about mitigating Clickjacking with XFO and CSP](https://developer.chrome.com/docs/lighthouse/best-practices/clickjacking-mitigation).',
/** Summary text for the results of a Lighthouse audit that evaluates whether the set CSP or XFO header is mitigating Clickjacking attacks. This is displayed if there is neither a CSP nor XFO header deployed. "XFO" stands for "X-Frame-Options". "CSP" stands for "Content-Security-Policy". */
noClickjackingMitigation: 'No Clickjacking mitigation found.',
/** Label for a column in a data table; entries will be a directive of the XFO or CSP header. "XFO" stands for "X-Frame-Options". "CSP" stands for "Content-Security-Policy". */
columnDirective: 'Directive',
/** Label for a column in a data table; entries will be the severity of an issue with the XFO or CSP header. "XFO" stands for "X-Frame-Options". "CSP" stands for "Content-Security-Policy". */
columnSeverity: 'Severity',
};

const str_ = i18n.createIcuMessageFn(import.meta.url, UIStrings);

class ClickjackingMitigation extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'clickjacking-mitigation',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs', 'URL'],
supportedModes: ['navigation'],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<{cspHeaders: string[], xfoHeaders: string[]}>}
*/
static async getRawCspsAndXfo(artifacts, context) {
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS];
const mainResource =
await MainResource.request({devtoolsLog, URL: artifacts.URL}, context);

const cspHeaders = mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'content-security-policy';
})
.flatMap(h => h.value.split(','))
.filter(rawCsp => rawCsp.replace(/\s/g, ''));
let xfoHeaders = mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'x-frame-options';
})
.flatMap(h => h.value);

// Sanitize the XFO header value.
xfoHeaders = xfoHeaders.map(v => v.toLowerCase().replace(/\s/g, ''));

return {cspHeaders, xfoHeaders};
}

/**
* @param {string | undefined} directive
* @param {LH.IcuMessage | string} findingDescription
* @param {LH.IcuMessage=} severity
* @return {LH.Audit.Details.TableItem}
*/
static findingToTableItem(directive, findingDescription, severity) {
return {
directive: directive,
description: findingDescription,
severity,
};
}

/**
* @param {string[]} cspHeaders
* @param {string[]} xfoHeaders
* @return {{score: number, results: LH.Audit.Details.TableItem[]}}
*/
static constructResults(cspHeaders, xfoHeaders) {
const rawXfo = [...xfoHeaders];
const allowedDirectives = ['deny', 'sameorigin'];

// If there is none of the two headers, return early.
if (!rawXfo.length && !cspHeaders.length) {
return {
score: 0,
results: [{
severity: str_(i18n.UIStrings.itemSeverityHigh),
description: str_(UIStrings.noClickjackingMitigation),
directive: undefined,
}],
};
}

// Check for frame-ancestors in CSP.
if (cspHeaders.length) {
for (const cspDirective of cspHeaders) {
if (cspDirective.includes('frame-ancestors')) {
// Pass the audit if frame-ancestors is present.
return {score: 1, results: []};
}
}
}

for (const actualDirective of xfoHeaders) {
if (allowedDirectives.includes(actualDirective)) {
// DENY or SAMEORIGIN are present.
return {score: 1, results: []};
}
}

return {
score: 0,
results: [{
severity: str_(i18n.UIStrings.itemSeverityHigh),
description: str_(UIStrings.noClickjackingMitigation),
directive: undefined,
}],
};
}

/**
* @param {LH.Artifacts} artifacts
* @param {LH.Audit.Context} context
* @return {Promise<LH.Audit.Product>}
*/
static async audit(artifacts, context) {
const {cspHeaders, xfoHeaders} = await this.getRawCspsAndXfo(artifacts, context);
const {score, results} = this.constructResults(cspHeaders, xfoHeaders);

/** @type {LH.Audit.Details.Table['headings']} */
const headings = [
/* eslint-disable max-len */
{key: 'description', valueType: 'text', subItemsHeading: {key: 'description'}, label: str_(i18n.UIStrings.columnDescription)},
{key: 'directive', valueType: 'code', subItemsHeading: {key: 'directive'}, label: str_(UIStrings.columnDirective)},
{key: 'severity', valueType: 'text', subItemsHeading: {key: 'severity'}, label: str_(UIStrings.columnSeverity)},
/* eslint-enable max-len */
];
const details = Audit.makeTableDetails(headings, results);

return {
score,
notApplicable: !results.length,
details,
};
}
}

export default ClickjackingMitigation;
export {UIStrings};
2 changes: 2 additions & 0 deletions core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ const defaultConfig = {
'csp-xss',
'has-hsts',
'origin-isolation',
'clickjacking-mitigation',
'script-treemap-data',
'accessibility/accesskeys',
'accessibility/aria-allowed-attr',
Expand Down Expand Up @@ -545,6 +546,7 @@ const defaultConfig = {
{id: 'csp-xss', weight: 0, group: 'best-practices-trust-safety'},
{id: 'has-hsts', weight: 0, group: 'best-practices-trust-safety'},
{id: 'origin-isolation', weight: 0, group: 'best-practices-trust-safety'},
{id: 'clickjacking-mitigation', weight: 0, group: 'hidden'},
// User Experience
{id: 'paste-preventing-inputs', weight: 3, group: 'best-practices-ux'},
{id: 'image-aspect-ratio', weight: 1, group: 'best-practices-ux'},
Expand Down
4 changes: 3 additions & 1 deletion core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -732,6 +732,7 @@ function checkKnownFixedCollisions(strings) {
'Directive',
'Directive',
'Directive',
'Directive',
'Document contains a $MARKDOWN_SNIPPET_0$ that triggers $MARKDOWN_SNIPPET_1$',
'Document contains a $MARKDOWN_SNIPPET_0$ that triggers $MARKDOWN_SNIPPET_1$',
'Document has a valid $MARKDOWN_SNIPPET_0$',
Expand All @@ -753,6 +754,7 @@ function checkKnownFixedCollisions(strings) {
'Severity',
'Severity',
'Severity',
'Severity',
'The page was evicted from the cache to allow another page to be cached.',
'The page was evicted from the cache to allow another page to be cached.',
'Use $MARKDOWN_SNIPPET_0$ to detect unused JavaScript code. $LINK_START_0$Learn more$LINK_END_0$',
Expand All @@ -764,7 +766,7 @@ function checkKnownFixedCollisions(strings) {
'Use the $MARKDOWN_SNIPPET_0$ component instead of $MARKDOWN_SNIPPET_1$ to automatically lazy-load images. $LINK_START_0$Learn more$LINK_END_0$.',
'Use the $MARKDOWN_SNIPPET_0$ component instead of $MARKDOWN_SNIPPET_1$ to automatically lazy-load images. $LINK_START_0$Learn more$LINK_END_0$.',
'Use the $MARKDOWN_SNIPPET_0$ component instead of $MARKDOWN_SNIPPET_1$ to automatically optimize image format. $LINK_START_0$Learn more$LINK_END_0$.',
'Use the $MARKDOWN_SNIPPET_0$ component instead of $MARKDOWN_SNIPPET_1$ to automatically optimize image format. $LINK_START_0$Learn more$LINK_END_0$.',
'Use the $MARKDOWN_SNIPPET_0$ component instead of $MARKDOWN_SNIPPET_1$ to automatically optimize image format. $LINK_START_0$Learn more$LINK_END_0$.'
]);
} catch (err) {
console.log('The number of duplicate strings has changed. Consider duplicating the `description` to match existing strings so they\'re translated together or update this assertion if they must absolutely be translated separately');
Expand Down
Loading