From 9149c7abd583c45cf0df83bf445c5b0ae7fa65b9 Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Tue, 27 Apr 2021 13:20:24 +0200 Subject: [PATCH] Add issue for private network request (preflights) This CL adds an issue indicating that private network requests are going to require a preflight request in the future. Bug: chromium:1141824 Change-Id: I7e63872b7612f5a2b6e483a508704f73287c8977 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/2848228 Commit-Queue: Sigurd Schneider Reviewed-by: Lutz Vahl --- config/gni/all_devtools_files.gni | 2 + config/gni/devtools_grd_files.gni | 1 + front_end/models/issues_manager/CorsIssue.ts | 24 ++++-- .../corsInsecurePrivateNetworkPreflight.md | 10 +++ .../panels/issues/CorsIssueDetailsView.ts | 2 + .../cors-private-network-issues_test.ts | 77 ++++++++++++++++++- 6 files changed, 107 insertions(+), 9 deletions(-) create mode 100644 front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md diff --git a/config/gni/all_devtools_files.gni b/config/gni/all_devtools_files.gni index b686155600..5eb84fd899 100644 --- a/config/gni/all_devtools_files.gni +++ b/config/gni/all_devtools_files.gni @@ -222,6 +222,7 @@ all_devtools_files = [ "front_end/models/issues_manager/descriptions/mixedContent.md", "front_end/models/issues_manager/descriptions/sharedArrayBuffer.md", "front_end/models/issues_manager/descriptions/corsInsecurePrivateNetwork.md", + "front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md", "front_end/models/issues_manager/descriptions/SameSiteExcludeContextDowngradeRead.md", "front_end/models/issues_manager/descriptions/SameSiteExcludeContextDowngradeSet.md", "front_end/models/issues_manager/descriptions/SameSiteExcludeNavigationContextDowngrade.md", @@ -440,6 +441,7 @@ devtools_issue_description_files = [ "corsAllowCredentialsRequired.md", "corsHeaderDisallowedByPreflightResponse.md", "corsInsecurePrivateNetwork.md", + "corsInsecurePrivateNetworkPreflight.md", "corsInvalidHeaderValues.md", "corsMethodDisallowedByPreflightResponse.md", "corsOriginMismatch.md", diff --git a/config/gni/devtools_grd_files.gni b/config/gni/devtools_grd_files.gni index 3453d16516..81df5f065e 100644 --- a/config/gni/devtools_grd_files.gni +++ b/config/gni/devtools_grd_files.gni @@ -243,6 +243,7 @@ grd_files_release_sources = [ "front_end/models/issues_manager/descriptions/corsAllowCredentialsRequired.md", "front_end/models/issues_manager/descriptions/corsHeaderDisallowedByPreflightResponse.md", "front_end/models/issues_manager/descriptions/corsInsecurePrivateNetwork.md", + "front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md", "front_end/models/issues_manager/descriptions/corsInvalidHeaderValues.md", "front_end/models/issues_manager/descriptions/corsMethodDisallowedByPreflightResponse.md", "front_end/models/issues_manager/descriptions/corsOriginMismatch.md", diff --git a/front_end/models/issues_manager/CorsIssue.ts b/front_end/models/issues_manager/CorsIssue.ts index 50ba114e4b..90609d688f 100644 --- a/front_end/models/issues_manager/CorsIssue.ts +++ b/front_end/models/issues_manager/CorsIssue.ts @@ -26,6 +26,7 @@ const i18nString = i18n.i18n.getLocalizedString.bind(undefined, str_); // eslint-disable-next-line rulesdir/const_enum export enum IssueCode { InsecurePrivateNetwork = 'CorsIssue::InsecurePrivateNetwork', + InsecurePrivateNetworkPreflight = 'CorsIssue::InsecurePrivateNetworkPreflight', InvalidHeaderValues = 'CorsIssue::InvalidHeaders', WildcardOriginNotAllowed = 'CorsIssue::WildcardOriginWithCredentials', PreflightResponseInvalid = 'CorsIssue::PreflightResponseInvalid', @@ -41,8 +42,8 @@ export enum IssueCode { InvalidResponse = 'CorsIssue::InvalidResponse', } -export function getIssueCode(corsError: Protocol.Network.CorsError): IssueCode { - switch (corsError) { +function getIssueCode(details: Protocol.Audits.CorsIssueDetails): IssueCode { + switch (details.corsErrorStatus.corsError) { case Protocol.Network.CorsError.InvalidAllowMethodsPreflightResponse: case Protocol.Network.CorsError.InvalidAllowHeadersPreflightResponse: case Protocol.Network.CorsError.PreflightMissingAllowOriginHeader: @@ -81,7 +82,8 @@ export function getIssueCode(corsError: Protocol.Network.CorsError): IssueCode { case Protocol.Network.CorsError.InvalidResponse: return IssueCode.InvalidResponse; case Protocol.Network.CorsError.InsecurePrivateNetwork: - return IssueCode.InsecurePrivateNetwork; + return details.clientSecurityState?.initiatorIsSecureContext ? IssueCode.InsecurePrivateNetworkPreflight : + IssueCode.InsecurePrivateNetwork; } } @@ -89,7 +91,7 @@ export class CorsIssue extends Issue { private issueDetails: Protocol.Audits.CorsIssueDetails; constructor(issueDetails: Protocol.Audits.CorsIssueDetails, issuesModel: SDK.IssuesModel.IssuesModel) { - super(getIssueCode(issueDetails.corsErrorStatus.corsError), issuesModel); + super(getIssueCode(issueDetails), issuesModel); this.issueDetails = issueDetails; } @@ -102,11 +104,8 @@ export class CorsIssue extends Issue { } getDescription(): MarkdownIssueDescription|null { - switch (getIssueCode(this.issueDetails.corsErrorStatus.corsError)) { + switch (getIssueCode(this.issueDetails)) { case IssueCode.InsecurePrivateNetwork: - if (this.issueDetails.clientSecurityState?.initiatorIsSecureContext) { - return null; - } return { file: 'corsInsecurePrivateNetwork.md', substitutions: undefined, @@ -115,6 +114,15 @@ export class CorsIssue extends Issue { linkTitle: i18nString(UIStrings.corsForPrivateNetworksRfc), }], }; + case IssueCode.InsecurePrivateNetworkPreflight: + return { + file: 'corsInsecurePrivateNetworkPreflight.md', + substitutions: undefined, + links: [{ + link: 'https://developer.chrome.com/blog/private-network-access-update', + linkTitle: i18nString(UIStrings.corsForPrivateNetworksRfc), + }], + }; case IssueCode.InvalidHeaderValues: return { file: 'corsInvalidHeaderValues.md', diff --git a/front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md b/front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md new file mode 100644 index 0000000000..2e177d100e --- /dev/null +++ b/front_end/models/issues_manager/descriptions/corsInsecurePrivateNetworkPreflight.md @@ -0,0 +1,10 @@ +# Ensure private network requests are only made to resources that allow them + +A site requested a resource from a network that it could only access because of its users' privileged network position. +These requests expose devices and servers to the internet, increasing the risk of a cross-site request forgery (CSRF) attack, and/or information leakage. + +To mitigate these risks, a future version of Chrome will require non-public subresources to opt-into being accessed with a preflight request. + +To fix this issue, ensure that response to the [preflight request](issueCorsPreflightRequest) for the private network resource has the `Access-Control-Allow-Private-Network` header set to `true`. + +Administrators can make use of the `InsecurePrivateNetworkRequestsAllowed` and `InsecurePrivateNetworkRequestsAllowedForUrls` enterprise policies to temporarily disable this restriction on all or certain websites. diff --git a/front_end/panels/issues/CorsIssueDetailsView.ts b/front_end/panels/issues/CorsIssueDetailsView.ts index 9ae24854fb..6c2b2ace1b 100644 --- a/front_end/panels/issues/CorsIssueDetailsView.ts +++ b/front_end/panels/issues/CorsIssueDetailsView.ts @@ -165,6 +165,7 @@ export class CorsIssueDetailsView extends AffectedResourcesView { this.appendColumnTitle(header, i18nString(UIStrings.allowCredentialsValueFromHeader)); break; case IssuesManager.CorsIssue.IssueCode.InsecurePrivateNetwork: + case IssuesManager.CorsIssue.IssueCode.InsecurePrivateNetworkPreflight: this.appendColumnTitle(header, i18nString(UIStrings.resourceAddressSpace)); this.appendColumnTitle(header, i18nString(UIStrings.initiatorAddressSpace)); this.appendColumnTitle(header, i18nString(UIStrings.initiatorContext)); @@ -293,6 +294,7 @@ export class CorsIssueDetailsView extends AffectedResourcesView { this.appendIssueDetailCell(element, details.corsErrorStatus.failedParameter, 'code-example'); break; case IssuesManager.CorsIssue.IssueCode.InsecurePrivateNetwork: + case IssuesManager.CorsIssue.IssueCode.InsecurePrivateNetworkPreflight: this.appendIssueDetailCell(element, details.resourceIPAddressSpace ?? ''); this.appendIssueDetailCell(element, details.clientSecurityState?.initiatorIPAddressSpace ?? ''); this.appendSecureContextCell(element, details.clientSecurityState?.initiatorIsSecureContext); diff --git a/test/e2e/issues/cors-private-network-issues_test.ts b/test/e2e/issues/cors-private-network-issues_test.ts index 6105eb9ad6..3b06bdf68b 100644 --- a/test/e2e/issues/cors-private-network-issues_test.ts +++ b/test/e2e/issues/cors-private-network-issues_test.ts @@ -13,7 +13,7 @@ describe('Cors Private Network issue', async () => { await goToResource('empty.html'); }); - it('should display correct information', async () => { + it('should display correct information for insecure contexts', async () => { await navigateToIssuesTab(); const {frontend} = getBrowserAndPages(); frontend.evaluate(() => { @@ -86,4 +86,79 @@ describe('Cors Private Network issue', async () => { 'insecure', ]); }); + + it('should display correct information for secure contexts', async () => { + await navigateToIssuesTab(); + const {frontend} = getBrowserAndPages(); + frontend.evaluate(() => { + const issue = { + code: 'CorsIssue', + details: { + corsIssueDetails: { + clientSecurityState: { + initiatorIsSecureContext: true, + initiatorIPAddressSpace: 'Public', + privateNetworkRequestPolicy: 'WarnFromInsecureToMorePrivate', + }, + corsErrorStatus: {corsError: 'InsecurePrivateNetwork', failedParameter: ''}, + isWarning: true, + request: {requestId: 'request-1', url: 'http://localhost/'}, + resourceIPAddressSpace: 'Local', + }, + }, + }; + // @ts-ignore + window.addIssueForTest(issue); + const issue2 = { + code: 'CorsIssue', + details: { + corsIssueDetails: { + clientSecurityState: { + initiatorIsSecureContext: true, + initiatorIPAddressSpace: 'Unknown', + privateNetworkRequestPolicy: 'WarnFromInsecureToMorePrivate', + }, + corsErrorStatus: {corsError: 'InsecurePrivateNetwork', failedParameter: ''}, + isWarning: true, + request: {requestId: 'request-1', url: 'http://example.com/'}, + resourceIPAddressSpace: 'Local', + }, + }, + }; + // @ts-ignore + window.addIssueForTest(issue2); + }); + + await expandIssue(); + const issueElement = + await getIssueByTitle('Ensure private network requests are only made to resources that allow them'); + assertNotNull(issueElement); + // TODO(crbug.com/1189877): Remove 2nd space after fixing l10n presubmit check + const section = await getResourcesElement('2 requests', issueElement, '.cors-issue-affected-resource-label'); + await ensureResourceSectionIsExpanded(section); + const table = await extractTableFromResourceSection(section.content); + assertNotNull(table); + assert.strictEqual(table.length, 3); + assert.deepEqual(table[0], [ + 'Request', + 'Status', + 'Resource Address', + 'Initiator Address', + 'Initiator Context', + ]); + assert.deepEqual(table[1], [ + 'localhost/', + 'warning', + 'Local', + 'Public', + 'secure', + ]); + assert.deepEqual(table[2], [ + 'example.com/', + 'warning', + 'Local', + 'Unknown', + 'secure', + ]); + }); });