From 0d8d75e850d5d36501df71760996d62ca6ce47ba Mon Sep 17 00:00:00 2001 From: Tom Schuster Date: Tue, 28 Jan 2025 12:30:26 +0000 Subject: [PATCH] Bug 1942622 - Collect telemetry for all internal chrome: CSP violations. r=freddyb Differential Revision: https://phabricator.services.mozilla.com/D235656 --- browser/components/tests/browser/browser.toml | 2 + .../browser_csp_violation_telemetry.js | 45 +++++++++++++ dom/security/metrics.yaml | 63 +++++++++++++++++++ dom/security/nsCSPContext.cpp | 63 ++++++++++++++++++- dom/security/nsCSPContext.h | 5 ++ 5 files changed, 175 insertions(+), 3 deletions(-) create mode 100644 browser/components/tests/browser/browser_csp_violation_telemetry.js diff --git a/browser/components/tests/browser/browser.toml b/browser/components/tests/browser/browser.toml index c76697fa51c5..586ac072e36a 100644 --- a/browser/components/tests/browser/browser.toml +++ b/browser/components/tests/browser/browser.toml @@ -51,3 +51,5 @@ run-if = ["os == 'win'"] ["browser_csp_blocks_event_handlers.js"] run-if = ["debug", "early_beta_or_earlier"] + +["browser_csp_violation_telemetry.js"] diff --git a/browser/components/tests/browser/browser_csp_violation_telemetry.js b/browser/components/tests/browser/browser_csp_violation_telemetry.js new file mode 100644 index 000000000000..d0a58989afe3 --- /dev/null +++ b/browser/components/tests/browser/browser_csp_violation_telemetry.js @@ -0,0 +1,45 @@ +/* Any copyright is dedicated to the Public Domain. + * http://creativecommons.org/publicdomain/zero/1.0/ */ + +"use strict"; + +add_task(async function test_internal_page_telemetry() { + Services.fog.testResetFOG(); + + is( + Glean.security.cspViolationInternalPage.testGetValue(), + null, + `No telemetry should have been recorded yet for cspViolationInternalPage` + ); + + // This page's CSP should disallow inline event handlers. + const ROBOTS_URL = "chrome://browser/content/aboutRobots.xhtml"; + await BrowserTestUtils.withNewTab(ROBOTS_URL, async browser => { + browser.contentDocument.documentElement.setAttribute("onclick", "foobar()"); + await BrowserTestUtils.waitForEvent( + browser.contentDocument, + "securitypolicyviolation" + ); + }); + + let testValue = Glean.security.cspViolationInternalPage.testGetValue(); + is(testValue.length, 1, "Should have telemetry for one violation"); + let extra = testValue[0].extra; + is(extra.directive, "script-src-attr", "violation's `directive` is correct"); + is(extra.selftype, "chromeuri", "violation's `selftype` is correct"); + is(extra.selfdetails, ROBOTS_URL, "violation's `selfdetails` is correct"); + is(extra.sourcetype, "chromeuri", "violation's `sourcetype` is correct"); + ok( + extra.sourcedetails.endsWith("/browser_csp_violation_telemetry.js"), + "violation's `sourcedetails` is correct" + ); + is(extra.blockeduritype, "inline", "violation's `blockeduritype` is correct"); + is( + extra.blockeduridetails, + undefined, + "violation's `blockeduridetails` is correct" + ); + is(extra.linenumber, "18", "violation's `linenumber` is correct"); + is(extra.columnnumber, "45", "violation's `columnnumber` is correct"); + is(extra.sample, "foobar()", "violation's sample is correct"); +}); diff --git a/dom/security/metrics.yaml b/dom/security/metrics.yaml index a4f64c1f3258..f29105a08f05 100644 --- a/dom/security/metrics.yaml +++ b/dom/security/metrics.yaml @@ -315,6 +315,69 @@ security: (Only for violations from chrome:) type: string + csp_violation_internal_page: + type: event + description: > + Information about Content-Security-Policy violations that happen in internal pages like chrome:// + bugs: + - https://bugzil.la/1942622 + data_reviews: + - https://bugzil.la/1942622 + notification_emails: + - tschuster@mozilla.com + - freddy@mozilla.com + expires: never + extra_keys: + directive: + description: > + The same as SecurityPolicyViolationEvent's effectiveDirective. + type: string + selftype: + description: > + The sanitized type of the "self uri", which is roughly similar to the documentURI. + This follows eval_usage_system_context sanitization procedure. + type: string + selfdetails: + description: > + A sanitized version of the "self uri", which is roughly similar to the documentURI, + e.g. the whole chrome:// URL in some cases. + This follows eval_usage_system_context sanitization procedure. + type: string + sourcetype: + description: > + The sanitized type of SecurityPolicyViolationEvent's sourceFile, + e.g. "chromeuri". + This follows eval_usage_system_context sanitization procedure. + type: string + sourcedetails: + description: > + A sanitized version of SecurityPolicyViolationEvent's sourceFile, + e.g. the whole chrome:// URL in some cases. + This follows eval_usage_system_context sanitization procedure. + type: string + blockeduritype: + description: > + The sanitized type of SecurityPolicyViolationEvent's blockedURI. + type: string + blockeduridetails: + description: > + A sanitized version of SecurityPolicyViolationEvent's blockedURI. + type: string + linenumber: + description: > + The same as SecurityPolicyViolationEvent's lineNumber. + type: quantity + columnnumber: + description: > + The same as SecurityPolicyViolationEvent's columnNumber. + type: quantity + sample: + description: > + The same as SecurityPolicyViolationEvent's sample. + (Only for violations from chrome:) + type: string + + eval_usage_parent_process: type: event description: > diff --git a/dom/security/nsCSPContext.cpp b/dom/security/nsCSPContext.cpp index ae63125e710c..d97d4818628e 100644 --- a/dom/security/nsCSPContext.cpp +++ b/dom/security/nsCSPContext.cpp @@ -44,12 +44,13 @@ #include "mozilla/Logging.h" #include "mozilla/Preferences.h" #include "mozilla/StaticPrefs_security.h" -#include "mozilla/dom/CSPReportBinding.h" #include "mozilla/dom/CSPDictionariesBinding.h" +#include "mozilla/dom/CSPReportBinding.h" #include "mozilla/dom/CSPViolationReportBody.h" -#include "mozilla/ipc/PBackgroundSharedTypes.h" #include "mozilla/dom/ReportingUtils.h" #include "mozilla/dom/WindowGlobalParent.h" +#include "mozilla/glean/DomSecurityMetrics.h" +#include "mozilla/ipc/PBackgroundSharedTypes.h" #include "nsINetworkInterceptController.h" #include "nsSandboxFlags.h" #include "nsIScriptElement.h" @@ -1428,6 +1429,59 @@ nsresult nsCSPContext::SendReportsToURIs( return NS_OK; } +void nsCSPContext::RecordInternalViolationTelemetry( + const CSPViolationData& aCSPViolationData, + const SecurityPolicyViolationEventInit& aInit) { + if (!mSelfURI || !mSelfURI->SchemeIs("chrome")) { + return; + } + + nsAutoCString selfURISpec; + mSelfURI->GetSpec(selfURISpec); + + // Temporarily skip this until we remove csp_violation_browser. + if (selfURISpec.EqualsLiteral("chrome://browser/content/browser.xhtml")) { + return; + } + + glean::security::CspViolationInternalPageExtra extra; + extra.directive = Some(NS_ConvertUTF16toUTF8(aInit.mEffectiveDirective)); + + FilenameTypeAndDetails self = + nsContentSecurityUtils::FilenameToFilenameType(selfURISpec, true); + extra.selftype = Some(self.first); + extra.selfdetails = self.second; + + FilenameTypeAndDetails source = + nsContentSecurityUtils::FilenameToFilenameType( + NS_ConvertUTF16toUTF8(aInit.mSourceFile), true); + extra.sourcetype = Some(source.first); + extra.sourcedetails = source.second; + + extra.linenumber = Some(aInit.mLineNumber); + extra.columnnumber = Some(aInit.mColumnNumber); + + // Don't collect samples for code that is probably not shipped by us. + if (source.first.EqualsLiteral("chromeuri") || + source.first.EqualsLiteral("resourceuri") || + source.first.EqualsLiteral("abouturi")) { + // aInit's sample requires the 'report-sample' keyword. + extra.sample = Some(NS_ConvertUTF16toUTF8(aCSPViolationData.mSample)); + } + + if (aInit.mBlockedURI.EqualsLiteral("inline")) { + extra.blockeduritype = Some("inline"_ns); + } else { + FilenameTypeAndDetails blocked = + nsContentSecurityUtils::FilenameToFilenameType( + NS_ConvertUTF16toUTF8(aInit.mBlockedURI), true); + extra.blockeduritype = Some(blocked.first); + extra.blockeduridetails = blocked.second; + } + + glean::security::csp_violation_internal_page.Record(Some(extra)); +} + nsresult nsCSPContext::FireViolationEvent( Element* aTriggeringElement, nsICSPEventListener* aCSPEventListener, const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit) { @@ -1561,7 +1615,10 @@ class CSPReportSenderRunnable final : public Runnable { // 3) log to console (one per policy violation) ReportToConsole(); - // 4) fire violation event + // 4) For internal pages we might send the failure to telemetry. + mCSPContext->RecordInternalViolationTelemetry(mCSPViolationData, init); + + // 5) fire violation event // A frame-ancestors violation has occurred, but we should not dispatch // the violation event to a potentially cross-origin ancestor. if (!mViolatedDirectiveName.EqualsLiteral("frame-ancestors")) { diff --git a/dom/security/nsCSPContext.h b/dom/security/nsCSPContext.h index ea63dc898cbe..d3e4f1308471 100644 --- a/dom/security/nsCSPContext.h +++ b/dom/security/nsCSPContext.h @@ -106,6 +106,11 @@ class nsCSPContext : public nsIContentSecurityPolicy { const mozilla::dom::SecurityPolicyViolationEventInit& aViolationEventInit); + void RecordInternalViolationTelemetry( + const mozilla::dom::CSPViolationData& aCSPViolationData, + const mozilla::dom::SecurityPolicyViolationEventInit& + aViolationEventInit); + nsresult FireViolationEvent( mozilla::dom::Element* aTriggeringElement, nsICSPEventListener* aCSPEventListener,