Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

new_audit: use a strong HSTS policy #16257

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
4 changes: 4 additions & 0 deletions cli/test/smokehouse/core-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import fpsMaxPassive from './test-definitions/fps-max-passive.js';
import fpsScaled from './test-definitions/fps-scaled.js';
import fpsOverflowX from './test-definitions/fps-overflow-x.js';
import issuesMixedContent from './test-definitions/issues-mixed-content.js';
import hstsFullyPresent from './test-definitions/hsts-fully-present.js';
import hstsMissingDirectives from './test-definitions/hsts-missing-directives.js';
import lanternFetch from './test-definitions/lantern-fetch.js';
import lanternIdleCallbackLong from './test-definitions/lantern-idle-callback-long.js';
import lanternIdleCallbackShort from './test-definitions/lantern-idle-callback-short.js';
Expand Down Expand Up @@ -79,6 +81,8 @@ const smokeTests = [
fpsOverflowX,
fpsScaled,
issuesMixedContent,
hstsFullyPresent,
hstsMissingDirectives,
lanternFetch,
lanternIdleCallbackLong,
lanternIdleCallbackShort,
Expand Down
26 changes: 26 additions & 0 deletions cli/test/smokehouse/test-definitions/hsts-fully-present.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results a site with full HSTS deployed.
*/
const expectations = {
lhr: {
requestedUrl: 'https://hstspreload.org/',
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
finalDisplayedUrl: 'https://hstspreload.org/',
audits: {
'has-hsts': {
score: null,
},
},
},
};

export default {
id: 'hsts-fully-present',
expectations,
};
26 changes: 26 additions & 0 deletions cli/test/smokehouse/test-definitions/hsts-missing-directives.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* @license
* Copyright 2024 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/

/**
* @type {Smokehouse.ExpectedRunnerResult}
* Expected Lighthouse results a site with HSTS header issues.
*/
const expectations = {
lhr: {
requestedUrl: 'https://developer.mozilla.org/en-US/',
finalDisplayedUrl: 'https://developer.mozilla.org/en-US/',
audits: {
'has-hsts': {
score: 1,
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
},
},
},
};

export default {
id: 'hsts-missing-directives',
expectations,
};
210 changes: 210 additions & 0 deletions core/audits/has-hsts.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,210 @@
/**
* @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 the security of a page's HSTS header. "HSTS" stands for "HTTP Strict Transport Security". */
title: 'Use a strong HSTS policy',
/** Description of a Lighthouse audit that evaluates the security of a page's HSTS header. 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. "HSTS" stands for "HTTP Strict Transport Security". */
description: 'Deployment of the HSTS header significantly ' +
'reduces the risk of downgrading of and eavesdropping attacks on HTTP connections. ' +
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
'reduces the risk of downgrading of and eavesdropping attacks on HTTP connections. ' +
'reduces the risk of downgrading HTTP connections and eavesdropping attacks. ' +

'A rollout in stages, starting with a low max-age is recommended. ' +
'[Learn what HSTS is.](https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Strict-Transport-Security)',
/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if no HSTS header is deployed. "HSTS" stands for "HTTP Strict Transport Security". */
noHsts: 'No HSTS header found',
/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the preload directive is missing. "HSTS" stands for "HTTP Strict Transport Security". */
noPreload: 'No preload directive found',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
noPreload: 'No preload directive found',
noPreload: 'No `preload` directive found',

/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the includeSubDomains directive is missing. "HSTS" stands for "HTTP Strict Transport Security". */
noSubdomain: 'No includeSubDomains directive found',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
noSubdomain: 'No includeSubDomains directive found',
noSubdomain: 'No `includeSubDomains` directive found',

/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the max-age directive is missing. "HSTS" stands for "HTTP Strict Transport Security". */
noMaxAge: 'No max-age directive',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
noMaxAge: 'No max-age directive',
noMaxAge: 'No `max-age` directive',

/** Summary text for the results of a Lighthouse audit that evaluates the HSTS header. This is displayed if the provided duration for the max-age directive is too low. "HSTS" stands for "HTTP Strict Transport Security". */
lowMaxAge: 'Max-age too low',
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
/** Table item value calling out the presence of a syntax error. */
invalidSyntax: 'Invalid syntax',
/** Label for a column in a data table; entries will be a directive of the HSTS header. "HSTS" stands for "HTTP Strict Transport Security". */
columnDirective: 'Directive',
/** Label for a column in a data table; entries will be the severity of an issue with the HSTS header. "HSTS" stands for "HTTP Strict Transport Security". */
columnSeverity: 'Severity',
};

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

class HasHsts extends Audit {
/**
* @return {LH.Audit.Meta}
*/
static get meta() {
return {
id: 'has-hsts',
scoreDisplayMode: Audit.SCORING_MODES.INFORMATIVE,
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
title: str_(UIStrings.title),
description: str_(UIStrings.description),
requiredArtifacts: ['devtoolsLogs', 'URL'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we are only looking at the main document, this audit is only relevant to page navigations.

Suggested change
requiredArtifacts: ['devtoolsLogs', 'URL'],
requiredArtifacts: ['devtoolsLogs', 'URL'],
supportedModes: ['navigation'],

};
}


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

var hstsHeaders =
mainResource.responseHeaders
.filter(h => {
return h.name.toLowerCase() === 'strict-transport-security';
})
.flatMap(h => h.value.split(';'));

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

return {hstsHeaders};
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
}

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

/**
* @param {string[]} hstsHeaders
* @return {{score: number, results: LH.Audit.Details.TableItem[]}}
*/
static constructResults(hstsHeaders) {
const rawHsts = [...hstsHeaders];
const allowedDirectives = ['max-age', 'includesubdomains', 'preload'];
const violations = [];
const warnings = [];
const syntax = [];

if (!rawHsts.length) {
return {
score: 0,
results: [{
severity: str_(i18n.UIStrings.itemSeverityHigh),
description: str_(UIStrings.noHsts),
directive: undefined,
}],
};
}

// No max-age is a violation and renders the HSTS header useless.
if (!hstsHeaders.toString().includes('max-age')) {
violations.push({
severity: str_(i18n.UIStrings.itemSeverityHigh),
description: str_(UIStrings.noMaxAge),
directive: 'max-age'
})
}

if (!hstsHeaders.toString().includes('includesubdomains')) {
// No includeSubdomains might be even wanted. But would be preferred.
warnings.push({
severity: str_(i18n.UIStrings.itemSeverityMedium),
description: str_(UIStrings.noSubdomain),
directive: 'includeSubDomains'
})
}

if (!hstsHeaders.toString().includes('preload')) {
// No preload might be even wanted. But would be preferred.
warnings.push({
severity: str_(i18n.UIStrings.itemSeverityMedium),
description: str_(UIStrings.noPreload),
directive: 'preload'
})
}

for (const actualDirective of hstsHeaders) {
// We recommend 2y max-age. But if it's lower than 1y, it's a violation.
if (actualDirective.includes('max-age') &&
parseInt(actualDirective.split('=')[1], 10) < 31536000) {
violations.push({
severity: str_(i18n.UIStrings.itemSeverityHigh),
description: str_(UIStrings.lowMaxAge),
directive: 'max-age'
})
}

// If there is a directive that's not an official HSTS directive.
if (!allowedDirectives.includes(actualDirective)) {
// max-age=<number> would always trigger.
if (actualDirective.includes('max-age')) {
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
continue;
}
syntax.push({
severity: str_(i18n.UIStrings.itemSeverityLow),
description: str_(UIStrings.invalidSyntax),
directive: actualDirective
})
}
}

const results = [
...violations.map(
f => this.findingToTableItem(
f.directive, f.description,
str_(i18n.UIStrings.itemSeverityHigh))),
...warnings.map(
f => this.findingToTableItem(
f.directive, f.description,
str_(i18n.UIStrings.itemSeverityMedium))),
...syntax.map(
f => this.findingToTableItem(
f.directive, f.description,
str_(i18n.UIStrings.itemSeverityLow))),
];
return {score: violations.length ? 0 : 1, results};
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
}

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

/** @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 HasHsts;
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 @@ -192,6 +192,7 @@ const defaultConfig = {
'valid-source-maps',
'prioritize-lcp-image',
'csp-xss',
'has-hsts',
'script-treemap-data',
'accessibility/accesskeys',
'accessibility/aria-allowed-attr',
Expand Down Expand Up @@ -541,6 +542,7 @@ const defaultConfig = {
{id: 'geolocation-on-start', weight: 1, group: 'best-practices-trust-safety'},
{id: 'notification-on-start', weight: 1, group: 'best-practices-trust-safety'},
{id: 'csp-xss', weight: 0, group: 'best-practices-trust-safety'},
{id: 'has-hsts', weight: 0, group: 'best-practices-trust-safety'},
sebastian9er marked this conversation as resolved.
Show resolved Hide resolved
// 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: 4 additions & 0 deletions core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,8 @@ function checkKnownFixedCollisions(strings) {
'Back/forward cache is disabled due to a keepalive request.',
'Consider uploading your GIF to a service which will make it available to embed as an HTML5 video.',
'Consider uploading your GIF to a service which will make it available to embed as an HTML5 video.',
'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 @@ -745,6 +747,8 @@ function checkKnownFixedCollisions(strings) {
'Pages with an in-flight network request are not currently eligible for back/forward cache.',
'Potential Savings',
'Potential Savings',
'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 Down
Loading
Loading