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

Add simple implementation of content-security-policy on network requests #24315

Merged
merged 1 commit into from Oct 17, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

Some generated files are not rendered by default. Learn more.

@@ -21,10 +21,12 @@ servo = [
"url",
"webrender_api",
"xml5ever",
"content-security-policy",
]

[dependencies]
app_units = "0.7"
content-security-policy = {version = "0.3.0", features = ["serde"], optional = true}
crossbeam-channel = { version = "0.3", optional = true }
cssparser = "0.25"
euclid = "0.20"
@@ -48,6 +48,8 @@

extern crate app_units;
#[cfg(feature = "servo")]
extern crate content_security_policy;
#[cfg(feature = "servo")]
extern crate crossbeam_channel;
extern crate cssparser;
extern crate euclid;
@@ -79,6 +81,8 @@ extern crate webrender_api;
#[cfg(feature = "servo")]
extern crate xml5ever;

#[cfg(feature = "servo")]
use content_security_policy as csp;
#[cfg(feature = "servo")]
use serde_bytes::ByteBuf;
use std::hash::{BuildHasher, Hash};
@@ -833,6 +837,8 @@ malloc_size_of_is_0!(app_units::Au);

malloc_size_of_is_0!(cssparser::RGBA, cssparser::TokenSerializationType);

malloc_size_of_is_0!(csp::Destination);

#[cfg(feature = "url")]
impl MallocSizeOf for url::Host {
fn size_of(&self, ops: &mut MallocSizeOfOps) -> usize {
@@ -17,6 +17,7 @@ doctest = false
base64 = "0.10.1"
brotli = "3"
bytes = "0.4"
content-security-policy = {version = "0.3.0", features = ["serde"]}
This conversation was marked as resolved by notriddle

This comment has been minimized.

Copy link
@nox

nox Oct 7, 2019

Member

Thanks for the publish.

cookie_rs = {package = "cookie", version = "0.11"}
crossbeam-channel = "0.3"
devtools_traits = {path = "../devtools_traits"}
@@ -8,6 +8,7 @@ use crate::filemanager_thread::{fetch_file_in_chunks, FileManager, FILE_CHUNK_SI
use crate::http_loader::{determine_request_referrer, http_fetch, HttpState};
use crate::http_loader::{set_default_accept, set_default_accept_language};
use crate::subresource_integrity::is_response_integrity_valid;
use content_security_policy as csp;
use crossbeam_channel::{unbounded, Receiver, Sender};
use devtools_traits::DevtoolsControlMsg;
use headers::{AccessControlExposeHeaders, ContentType, HeaderMapExt, Range};
@@ -138,6 +139,30 @@ pub fn fetch_with_cors_cache(
main_fetch(request, cache, false, false, target, &mut None, &context);
}

/// https://www.w3.org/TR/CSP/#should-block-request
pub fn should_request_be_blocked_by_csp(request: &Request) -> csp::CheckResult {
let origin = match &request.origin {
Origin::Client => return csp::CheckResult::Allowed,
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: String::new(),
integrity_metadata: request.integrity_metadata.clone(),
parser_metadata: csp::ParserMetadata::None,
};

This comment has been minimized.

Copy link
@nox

nox Oct 10, 2019

Member

Do you think this could be done in a way that doesn't require cloning all those things? What csp::Request was borrowing from its environment? I will understand if you think that makes the content_security_policy crate too complex.

// TODO: Instead of ignoring violations, report them.
request
.csp_list
.as_ref()
.map(|c| c.should_request_be_blocked(&csp_request).0)
.unwrap_or(csp::CheckResult::Allowed)
}

/// [Main fetch](https://fetch.spec.whatwg.org/#concept-main-fetch)
pub fn main_fetch(
request: &mut Request,
@@ -163,8 +188,18 @@ pub fn main_fetch(
}
}

// Step 2.2.
// TODO: Report violations.

// Step 2.4.
if should_request_be_blocked_by_csp(request) == csp::CheckResult::Blocked {
response = Some(Response::network_error(NetworkError::Internal(
"Blocked by Content-Security-Policy".into(),
)))
}

// Step 3.
// TODO: handle content security policy violations.
// TODO: handle request abort.

// Step 4.
// TODO: handle upgrade to a potentially secure URL.
@@ -13,6 +13,7 @@ test = false
doctest = false

[dependencies]
content-security-policy = {version = "0.3.0", features = ["serde"]}
cookie = "0.11"
embedder_traits = { path = "../embedder_traits" }
headers = "0.2"
@@ -4,6 +4,7 @@

use crate::ReferrerPolicy;
use crate::ResourceTimingType;
use content_security_policy::{self as csp, CspList};
use http::HeaderMap;
use hyper::Method;
use msg::constellation_msg::PipelineId;
@@ -20,37 +21,7 @@ pub enum Initiator {
}

/// A request [destination](https://fetch.spec.whatwg.org/#concept-request-destination)
#[derive(Clone, Copy, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
pub enum Destination {
None,
Audio,
Document,
Embed,
Font,
Image,
Manifest,
Object,
Report,
Script,
ServiceWorker,
SharedWorker,
Style,
Track,
Video,
Worker,
Xslt,
}

impl Destination {
/// https://fetch.spec.whatwg.org/#request-destination-script-like
#[inline]
pub fn is_script_like(&self) -> bool {
*self == Destination::Script ||
*self == Destination::ServiceWorker ||
*self == Destination::SharedWorker ||
*self == Destination::Worker
}
}
pub use csp::Destination;
This conversation was marked as resolved by notriddle

This comment has been minimized.

Copy link
@nox

nox Oct 10, 2019

Member

Nice!


/// A request [origin](https://fetch.spec.whatwg.org/#concept-request-origin)
#[derive(Clone, Debug, Deserialize, MallocSizeOf, PartialEq, Serialize)]
@@ -175,6 +146,11 @@ pub struct RequestBuilder {
pub pipeline_id: Option<PipelineId>,
pub redirect_mode: RedirectMode,
pub integrity_metadata: String,
// This is nominally a part of the client's global object.
// It is copied here to avoid having to reach across the thread
// boundary every time a redirect occurs.
#[ignore_malloc_size_of = "Defined in rust-content-security-policy"]
pub csp_list: Option<CspList>,
Comment on lines +149 to +153

This comment has been minimized.

Copy link
@nox

nox Oct 10, 2019

Member

You made malloc_size_of depend on content_security_policy already, is there any way the MallocSizeOf trait could be implemented in any useful way for CspList?

// to keep track of redirects
pub url_list: Vec<ServoUrl>,
pub parser_metadata: ParserMetadata,
@@ -206,6 +182,7 @@ impl RequestBuilder {
url_list: vec![],
parser_metadata: ParserMetadata::Default,
initiator: Initiator::None,
csp_list: None,
}
}

@@ -329,6 +306,7 @@ impl RequestBuilder {
request.url_list = url_list;
request.integrity_metadata = self.integrity_metadata;
request.parser_metadata = self.parser_metadata;
request.csp_list = self.csp_list;
request
}
}
@@ -396,6 +374,11 @@ pub struct Request {
pub response_tainting: ResponseTainting,
/// <https://fetch.spec.whatwg.org/#concept-request-parser-metadata>
pub parser_metadata: ParserMetadata,
// This is nominally a part of the client's global object.
// It is copied here to avoid having to reach across the thread
// boundary every time a redirect occurs.
#[ignore_malloc_size_of = "Defined in rust-content-security-policy"]
pub csp_list: Option<CspList>,
}

impl Request {
@@ -428,6 +411,7 @@ impl Request {
parser_metadata: ParserMetadata::Default,
redirect_count: 0,
response_tainting: ResponseTainting::Basic,
csp_list: None,
}
}

@@ -38,6 +38,7 @@ bitflags = "1.0"
bluetooth_traits = {path = "../bluetooth_traits"}
canvas_traits = {path = "../canvas_traits"}
caseless = "0.2"
content-security-policy = {version = "0.3.0", features = ["serde"]}
cookie = "0.11"
chrono = "0.4"
crossbeam-channel = "0.3"
@@ -54,6 +54,7 @@ use canvas_traits::webgl::{
use canvas_traits::webgl::{WebGLFramebufferId, WebGLMsgSender, WebGLPipeline, WebGLProgramId};
use canvas_traits::webgl::{WebGLReceiver, WebGLRenderbufferId, WebGLSLVersion, WebGLSender};
use canvas_traits::webgl::{WebGLShaderId, WebGLSyncId, WebGLTextureId, WebGLVersion};
use content_security_policy::CspList;
use crossbeam_channel::{Receiver, Sender};
use cssparser::RGBA;
use devtools_traits::{CSSError, TimelineMarkerType, WorkerId};
@@ -170,6 +171,8 @@ unsafe_no_jsmanaged_fields!(*mut JobQueue);

unsafe_no_jsmanaged_fields!(Cow<'static, str>);

unsafe_no_jsmanaged_fields!(CspList);

/// Trace a `JSVal`.
pub fn trace_jsval(tracer: *mut JSTracer, description: &str, val: &Heap<JSVal>) {
unsafe {
@@ -110,6 +110,7 @@ use crate::task::TaskBox;
use crate::task_source::{TaskSource, TaskSourceName};
use crate::timers::OneshotTimerCallback;
use canvas_traits::webgl::{self, WebGLContextId, WebGLMsg};
use content_security_policy::{self as csp, CspList};
use cookie::Cookie;
use devtools_traits::ScriptToDevtoolsControlMsg;
use dom_struct::dom_struct;
@@ -137,6 +138,7 @@ use num_traits::ToPrimitive;
use percent_encoding::percent_decode;
use profile_traits::ipc as profile_ipc;
use profile_traits::time::{TimerMetadata, TimerMetadataFrameType, TimerMetadataReflowType};
use ref_filter_map::ref_filter_map;
use ref_slice::ref_slice;
use script_layout_interface::message::{Msg, ReflowGoal};
use script_traits::{AnimationState, DocumentActivity, MouseButton, MouseEventType};
@@ -148,7 +150,7 @@ use servo_atoms::Atom;
use servo_config::pref;
use servo_media::{ClientContextId, ServoMedia};
use servo_url::{ImmutableOrigin, MutableOrigin, ServoUrl};
use std::borrow::ToOwned;
use std::borrow::Cow;
use std::cell::{Cell, Ref, RefMut};
use std::collections::hash_map::Entry::{Occupied, Vacant};
use std::collections::{HashMap, HashSet, VecDeque};
@@ -398,6 +400,9 @@ pub struct Document {
media_controls: DomRefCell<HashMap<String, Dom<ShadowRoot>>>,
/// List of all WebGL context IDs that need flushing.
dirty_webgl_contexts: DomRefCell<HashSet<WebGLContextId>>,
/// https://html.spec.whatwg.org/multipage/#concept-document-csp-list
#[ignore_malloc_size_of = "Defined in rust-content-security-policy"]
csp_list: DomRefCell<Option<CspList>>,
}

#[derive(JSTraceable, MallocSizeOf)]
@@ -1734,9 +1739,10 @@ impl Document {
pub fn fetch_async(
&self,
load: LoadType,
request: RequestBuilder,
mut request: RequestBuilder,
fetch_target: IpcSender<FetchResponseMsg>,
) {
request.csp_list = self.get_csp_list().map(|x| x.clone());
let mut loader = self.loader.borrow_mut();
loader.fetch_async(load, request, fetch_target);
}
@@ -2806,9 +2812,39 @@ impl Document {
shadow_roots_styles_changed: Cell::new(false),
media_controls: DomRefCell::new(HashMap::new()),
dirty_webgl_contexts: DomRefCell::new(HashSet::new()),
csp_list: DomRefCell::new(None),
}
}

pub fn set_csp_list(&self, csp_list: Option<CspList>) {
*self.csp_list.borrow_mut() = csp_list;
}

pub fn get_csp_list(&self) -> Option<Ref<CspList>> {
ref_filter_map(self.csp_list.borrow(), Option::as_ref)
}

/// https://www.w3.org/TR/CSP/#should-block-inline
pub fn should_elements_inline_type_behavior_be_blocked(
This conversation was marked as resolved by notriddle

This comment has been minimized.

Copy link
@nox

nox Oct 7, 2019

Member

Spec link? Please add spec links everywhere you can, both here and in your crate.

&self,
el: &Element,
type_: csp::InlineCheckType,
source: &str,
) -> csp::CheckResult {
let element = csp::Element {
nonce: el
.get_attribute(&ns!(), &local_name!("nonce"))
.map(|attr| Cow::Owned(attr.value().to_string())),
};
// TODO: Instead of ignoring violations, report them.
self.get_csp_list()
.map(|c| {
c.should_elements_inline_type_behavior_be_blocked(&element, type_, source)
.0
})
.unwrap_or(csp::CheckResult::Allowed)
}

/// Prevent any JS or layout from running until the corresponding call to
/// `remove_script_and_layout_blocker`. Used to isolate periods in which
/// the DOM is in an unstable state and should not be exposed to arbitrary
@@ -38,6 +38,7 @@ use crate::task_source::websocket::WebsocketTaskSource;
use crate::task_source::TaskSourceName;
use crate::timers::{IsInterval, OneshotTimerCallback, OneshotTimerHandle};
use crate::timers::{OneshotTimers, TimerCallback};
use content_security_policy::CspList;
use devtools_traits::{ScriptToDevtoolsControlMsg, WorkerId};
use dom_struct::dom_struct;
use ipc_channel::ipc::IpcSender;
@@ -812,6 +813,15 @@ impl GlobalScope {
pub fn get_user_agent(&self) -> Cow<'static, str> {
self.user_agent.clone()
}

/// https://www.w3.org/TR/CSP/#get-csp-of-object
pub fn get_csp_list(&self) -> Option<CspList> {
if let Some(window) = self.downcast::<Window>() {
return window.Document().get_csp_list().map(|c| c.clone());
This conversation was marked as resolved by nox

This comment has been minimized.

Copy link
@nox

nox Oct 7, 2019

Member

Why clone? And why don't other kinds of globals have any CSP list?

This comment has been minimized.

Copy link
@notriddle

notriddle Oct 9, 2019

Author Contributor

Why clone?

The Document function's return value is too short to return a reference to the CspList. I tried; borrowck throws an error.

And why don't other kinds of globals have any CSP list?

Other kinds of globals do have a CSP list, as described at https://www.w3.org/TR/CSP/#get-csp-of-object. I just haven't implemented it yet.

}
// TODO: Worker and Worklet global scopes.
None
}
}

fn timestamp_in_ms(time: Timespec) -> u64 {
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.