diff options
author | Tim van der Lippe <TimvdLippe@users.noreply.github.com> | 2025-04-25 21:59:44 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2025-04-25 19:59:44 +0000 |
commit | baa18e18afa20f3b38c17c830feb76b05f7e64fb (patch) | |
tree | d73a1bf7432f1226b427ef2ea2c9ff1f3acc0420 /components/net | |
parent | 4ff45f86b9af63edafd98685f9d73e8a250ff9aa (diff) | |
download | servo-baa18e18afa20f3b38c17c830feb76b05f7e64fb.tar.gz servo-baa18e18afa20f3b38c17c830feb76b05f7e64fb.zip |
Support CSP report-only header (#36623)
This turned out to be a full rabbit hole. The new header
is parsed in the new `parse_csp_list_from_metadata` which
sets `disposition` to `report.
I was testing this with
`script-src-report-only-policy-works-with-external-hash-policy.html`
which was blocking the script incorrectly. Turns out that there
were multiple bugs in the CSP library, as well as a missing
check in `fetch` to report violations.
Additionally, in several locations we were manually reporting csp
violations, instead of the new `global.report_csp_violations`. As
a result of that, they would double report, since the report-only
header would be appended as a policy and now would report twice.
Now, all callsides use `global.report_csp_violations`. As a nice
side-effect, I added the code to set source file information,
since that was already present for the `eval` check, but nowhere
else.
Part of #36437
Requires servo/rust-content-security-policy#5
---------
Signed-off-by: Tim van der Lippe <tvanderlippe@gmail.com>
Signed-off-by: Tim van der Lippe <TimvdLippe@users.noreply.github.com>
Diffstat (limited to 'components/net')
-rw-r--r-- | components/net/fetch/methods.rs | 67 |
1 files changed, 51 insertions, 16 deletions
diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 53bc2817292..b1ad01b81e0 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -23,8 +23,8 @@ use net_traits::http_status::HttpStatus; use net_traits::policy_container::{PolicyContainer, RequestPolicyContainer}; use net_traits::request::{ BodyChunkRequest, BodyChunkResponse, CredentialsMode, Destination, Initiator, - InsecureRequestsPolicy, Origin, RedirectMode, Referrer, Request, RequestMode, ResponseTainting, - Window, is_cors_safelisted_method, is_cors_safelisted_request_header, + InsecureRequestsPolicy, Origin, ParserMetadata, RedirectMode, Referrer, Request, RequestMode, + ResponseTainting, Window, is_cors_safelisted_method, is_cors_safelisted_request_header, }; use net_traits::response::{Response, ResponseBody, ResponseType}; use net_traits::{ @@ -169,6 +169,29 @@ pub async fn fetch_with_cors_cache( // TODO: We don't implement fetchParams as defined in the spec } +fn convert_request_to_csp_request(request: &Request, origin: &ImmutableOrigin) -> csp::Request { + csp::Request { + url: request.url().into_url(), + origin: origin.clone().into_url_origin(), + redirect_count: request.redirect_count, + destination: request.destination, + initiator: match request.initiator { + Initiator::Download => csp::Initiator::Download, + Initiator::ImageSet => csp::Initiator::ImageSet, + Initiator::Manifest => csp::Initiator::Manifest, + Initiator::Prefetch => csp::Initiator::Prefetch, + _ => csp::Initiator::None, + }, + nonce: request.cryptographic_nonce_metadata.clone(), + integrity_metadata: request.integrity_metadata.clone(), + parser_metadata: match request.parser_metadata { + ParserMetadata::ParserInserted => csp::ParserMetadata::ParserInserted, + ParserMetadata::NotParserInserted => csp::ParserMetadata::NotParserInserted, + ParserMetadata::Default => csp::ParserMetadata::None, + }, + } +} + /// <https://www.w3.org/TR/CSP/#should-block-request> pub fn should_request_be_blocked_by_csp( request: &Request, @@ -178,17 +201,7 @@ pub fn should_request_be_blocked_by_csp( Origin::Client => return (csp::CheckResult::Allowed, Vec::new()), Origin::Origin(origin) => origin, }; - - let csp_request = csp::Request { - url: request.url().into_url(), - origin: origin.clone().into_url_origin(), - redirect_count: request.redirect_count, - destination: request.destination, - initiator: csp::Initiator::None, - nonce: request.cryptographic_nonce_metadata.clone(), - integrity_metadata: request.integrity_metadata.clone(), - parser_metadata: csp::ParserMetadata::None, - }; + let csp_request = convert_request_to_csp_request(request, origin); policy_container .csp_list @@ -197,6 +210,24 @@ pub fn should_request_be_blocked_by_csp( .unwrap_or((csp::CheckResult::Allowed, Vec::new())) } +/// <https://www.w3.org/TR/CSP/#report-for-request> +pub fn report_violations_for_request_by_csp( + request: &Request, + policy_container: &PolicyContainer, +) -> Vec<csp::Violation> { + let origin = match &request.origin { + Origin::Client => return Vec::new(), + Origin::Origin(origin) => origin, + }; + let csp_request = convert_request_to_csp_request(request, origin); + + policy_container + .csp_list + .as_ref() + .map(|c| c.report_violations_for_request(&csp_request)) + .unwrap_or_default() +} + /// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch) pub async fn main_fetch( fetch_params: &mut FetchParams, @@ -232,9 +263,6 @@ pub async fn main_fetch( ))); } - // Step 2.2. - // TODO: Report violations. - // The request should have a valid policy_container associated with it. // TODO: This should not be `Client` here let policy_container = match &request.policy_container { @@ -242,6 +270,13 @@ pub async fn main_fetch( RequestPolicyContainer::PolicyContainer(container) => container.to_owned(), }; + // Step 2.2. + let violations = report_violations_for_request_by_csp(request, &policy_container); + + if !violations.is_empty() { + target.process_csp_violations(request, violations); + } + // Step 3. // TODO: handle request abort. |