mirror of
https://github.com/servo/servo.git
synced 2025-07-24 15:50:21 +01:00
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>
This commit is contained in:
parent
4ff45f86b9
commit
baa18e18af
23 changed files with 113 additions and 208 deletions
|
@ -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.
|
||||
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue