From 8aac575019ef95f49e47b6b1d428b3e0c1bc2c5d Mon Sep 17 00:00:00 2001 From: Sumant Manne Date: Fri, 16 Dec 2016 15:04:18 -0600 Subject: [PATCH] Implemented nosniff for fetch algorithm --- components/net/fetch/methods.rs | 138 ++++++++++++++++-- tests/unit/net/fetch.rs | 47 +++++- .../fetch/nosniff/importscripts.html.ini | 5 - .../fetch/nosniff/parsing-nosniff.html.ini | 8 - .../metadata/fetch/nosniff/script.html.ini | 14 -- .../fetch/nosniff/stylesheet.html.ini | 14 -- .../metadata/fetch/nosniff/worker.html.ini | 14 -- ...obalScope_importScripts_NosniffErr.htm.ini | 5 - .../workers/Worker_NosniffErr.htm.ini | 6 - 9 files changed, 172 insertions(+), 79 deletions(-) delete mode 100644 tests/wpt/metadata/fetch/nosniff/importscripts.html.ini delete mode 100644 tests/wpt/metadata/fetch/nosniff/parsing-nosniff.html.ini delete mode 100644 tests/wpt/metadata/fetch/nosniff/script.html.ini delete mode 100644 tests/wpt/metadata/fetch/nosniff/stylesheet.html.ini delete mode 100644 tests/wpt/metadata/fetch/nosniff/worker.html.ini delete mode 100644 tests/wpt/metadata/workers/WorkerGlobalScope_importScripts_NosniffErr.htm.ini delete mode 100644 tests/wpt/metadata/workers/Worker_NosniffErr.htm.ini diff --git a/components/net/fetch/methods.rs b/components/net/fetch/methods.rs index 44a603025b0..fe04acc35c3 100644 --- a/components/net/fetch/methods.rs +++ b/components/net/fetch/methods.rs @@ -8,8 +8,10 @@ use devtools_traits::DevtoolsControlMsg; use fetch::cors_cache::CorsCache; use filemanager_thread::FileManager; use http_loader::{HttpState, determine_request_referrer, http_fetch, set_default_accept_language}; +use hyper::Error; +use hyper::error::Result as HyperResult; use hyper::header::{Accept, AcceptLanguage, ContentLanguage, ContentType}; -use hyper::header::{HeaderView, QualityItem, Referer as RefererHeader, q, qitem}; +use hyper::header::{Header, HeaderFormat, HeaderView, QualityItem, Referer as RefererHeader, q, qitem}; use hyper::method::Method; use hyper::mime::{Mime, SubLevel, TopLevel}; use hyper::status::StatusCode; @@ -19,10 +21,12 @@ use net_traits::request::{RedirectMode, Referrer, Request, RequestMode, Response use net_traits::request::{Type, Origin, Window}; use net_traits::response::{Response, ResponseBody, ResponseType}; use std::borrow::Cow; +use std::fmt; use std::fs::File; use std::io::Read; use std::mem; use std::rc::Rc; +use std::str; use std::sync::mpsc::{Sender, Receiver}; use subresource_integrity::is_response_integrity_valid; @@ -269,13 +273,12 @@ pub fn main_fetch(request: Rc, response }; - let mut response_loaded = false; - { + let internal_error = { // Inner scope to reborrow response // Step 14 - let network_error_res; + let network_error_response; let internal_response = if let Some(error) = response.get_network_error() { - network_error_res = Response::network_error(error.clone()); - &network_error_res + network_error_response = Response::network_error(error.clone()); + &network_error_response } else { response.actual_response() }; @@ -286,21 +289,41 @@ pub fn main_fetch(request: Rc, } // Step 16 - // TODO this step (CSP/blocking) + // TODO Blocking for CSP, mixed content, MIME type + let blocked_error_response; + let internal_response = if !response.is_network_error() && should_block_nosniff(&request, &response) { + // Defer rebinding result + blocked_error_response = Response::network_error(NetworkError::Internal("Blocked by nosniff".into())); + &blocked_error_response + } else { + internal_response + }; // Step 17 - if !response.is_network_error() && (is_null_body_status(&internal_response.status) || + // We check `internal_response` since we did not mutate `response` in the previous step. + let not_network_error = !response.is_network_error() && !internal_response.is_network_error(); + if not_network_error && (is_null_body_status(&internal_response.status) || match *request.method.borrow() { Method::Head | Method::Connect => true, - _ => false }) - { + _ => false }) { // when Fetch is used only asynchronously, we will need to make sure // that nothing tries to write to the body at this point let mut body = internal_response.body.lock().unwrap(); *body = ResponseBody::Empty; } - } - // Step 18 + + internal_response.get_network_error().map(|e| e.clone()) + }; + + // Execute deferred rebinding of response + let response = if let Some(error) = internal_error { + Response::network_error(error) + } else { + response + }; + + // Step 18 + let mut response_loaded = false; let response = if !response.is_network_error() && *request.integrity_metadata.borrow() != "" { // Substep 1 wait_for_response(&response, target, done_chan); @@ -510,3 +533,94 @@ fn is_null_body_status(status: &Option) -> bool { } } +/// https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-nosniff? +fn should_block_nosniff(request: &Request, response: &Response) -> bool { + /// https://fetch.spec.whatwg.org/#x-content-type-options-header + /// This is needed to parse `X-Content-Type-Options` according to spec, + /// which requires that we inspect only the first value. + /// + /// A [unit-like struct](https://doc.rust-lang.org/book/structs.html#unit-like-structs) + /// is sufficient since a valid header implies that we use `nosniff`. + #[derive(Debug, Clone, Copy)] + struct XContentTypeOptions(); + + impl Header for XContentTypeOptions { + fn header_name() -> &'static str { + "X-Content-Type-Options" + } + + /// https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-nosniff%3F #2 + fn parse_header(raw: &[Vec]) -> HyperResult { + raw.first() + .and_then(|v| str::from_utf8(v).ok()) + .and_then(|s| match s.trim().to_lowercase().as_str() { + "nosniff" => Some(XContentTypeOptions()), + _ => None + }) + .ok_or(Error::Header) + } + } + + impl HeaderFormat for XContentTypeOptions { + fn fmt_header(&self, f: &mut fmt::Formatter) -> fmt::Result { + f.write_str("nosniff") + } + } + + match response.headers.get::() { + None => return false, // Step 1 + _ => () // Step 2 & 3 are implemented by the XContentTypeOptions struct + }; + + // Step 4 + // Note: an invalid MIME type will produce a `None`. + let content_type_header = response.headers.get::(); + // Step 5 + let type_ = request.type_; + + /// https://html.spec.whatwg.org/multipage/#scriptingLanguages + #[inline] + fn is_javascript_mime_type(mime_type: &Mime) -> bool { + let javascript_mime_types: [Mime; 16] = [ + mime!(Application / ("ecmascript")), + mime!(Application / ("javascript")), + mime!(Application / ("x-ecmascript")), + mime!(Application / ("x-javascript")), + mime!(Text / ("ecmascript")), + mime!(Text / ("javascript")), + mime!(Text / ("javascript1.0")), + mime!(Text / ("javascript1.1")), + mime!(Text / ("javascript1.2")), + mime!(Text / ("javascript1.3")), + mime!(Text / ("javascript1.4")), + mime!(Text / ("javascript1.5")), + mime!(Text / ("jscript")), + mime!(Text / ("livescript")), + mime!(Text / ("x-ecmascript")), + mime!(Text / ("x-javascript")), + ]; + + javascript_mime_types.contains(mime_type) + } + + let text_css: Mime = mime!(Text / Css); + // Assumes str::starts_with is equivalent to mime::TopLevel + return match type_ { + // Step 6 + Type::Script => { + match content_type_header { + Some(&ContentType(ref mime_type)) => !is_javascript_mime_type(&mime_type), + None => true + } + } + // Step 7 + Type::Style => { + match content_type_header { + Some(&ContentType(ref mime_type)) => mime_type != &text_css, + None => true + } + } + // Step 8 + _ => false + }; +} diff --git a/tests/unit/net/fetch.rs b/tests/unit/net/fetch.rs index 43940f28a9f..066b30bbd63 100644 --- a/tests/unit/net/fetch.rs +++ b/tests/unit/net/fetch.rs @@ -31,7 +31,7 @@ use net::test::HttpState; use net_traits::IncludeSubdomains; use net_traits::NetworkError; use net_traits::ReferrerPolicy; -use net_traits::request::{Origin, RedirectMode, Referrer, Request, RequestMode}; +use net_traits::request::{Origin, RedirectMode, Referrer, Request, RequestMode, Type}; use net_traits::response::{CacheState, Response, ResponseBody, ResponseType}; use servo_config::resource_files::resources_dir_path; use servo_url::{ImmutableOrigin, ServoUrl}; @@ -601,6 +601,51 @@ fn test_fetch_with_sri_sucess() { let _ = server.close(); assert_eq!(response_is_done(&response), true); } + +/// `fetch` should return a network error if there is a header `X-Content-Type-Options: nosniff` +#[test] +fn test_fetch_blocked_nosniff() { + #[inline] + fn test_nosniff_request(request_type: Type, + mime: Mime, + should_error: bool) { + const MESSAGE: &'static [u8] = b""; + const HEADER: &'static str = "X-Content-Type-Options"; + const VALUE: &'static [u8] = b"nosniff"; + + let handler = move |_: HyperRequest, mut response: HyperResponse| { + let mime_header = ContentType(mime.clone()); + response.headers_mut().set(mime_header); + assert!(response.headers().has::()); + // Add the nosniff header + response.headers_mut().set_raw(HEADER, vec![VALUE.to_vec()]); + + response.send(MESSAGE).unwrap(); + }; + + let (mut server, url) = make_server(handler); + + let origin = Origin::Origin(url.origin()); + let mut request = Request::new(url, Some(origin), false, None); + request.type_ = request_type; + let fetch_response = fetch(request, None); + let _ = server.close(); + + assert_eq!(fetch_response.is_network_error(), should_error); + } + + let tests = vec![ + (Type::Script, Mime(TopLevel::Text, SubLevel::Javascript, vec![]), false), + (Type::Script, Mime(TopLevel::Text, SubLevel::Css, vec![]), true), + (Type::Style, Mime(TopLevel::Text, SubLevel::Css, vec![]), false), + ]; + + for test in tests { + let (type_, mime, should_error) = test; + test_nosniff_request(type_, mime, should_error); + } +} + fn setup_server_and_fetch(message: &'static [u8], redirect_cap: u32) -> Response { let handler = move |request: HyperRequest, mut response: HyperResponse| { let redirects = match request.uri { diff --git a/tests/wpt/metadata/fetch/nosniff/importscripts.html.ini b/tests/wpt/metadata/fetch/nosniff/importscripts.html.ini deleted file mode 100644 index 2e04036194e..00000000000 --- a/tests/wpt/metadata/fetch/nosniff/importscripts.html.ini +++ /dev/null @@ -1,5 +0,0 @@ -[importscripts.html] - type: testharness - [Test importScripts()] - expected: FAIL - diff --git a/tests/wpt/metadata/fetch/nosniff/parsing-nosniff.html.ini b/tests/wpt/metadata/fetch/nosniff/parsing-nosniff.html.ini deleted file mode 100644 index 1b49f9f96e1..00000000000 --- a/tests/wpt/metadata/fetch/nosniff/parsing-nosniff.html.ini +++ /dev/null @@ -1,8 +0,0 @@ -[parsing-nosniff.html] - type: testharness - [URL query: first] - expected: FAIL - - [URL query: uppercase] - expected: FAIL - diff --git a/tests/wpt/metadata/fetch/nosniff/script.html.ini b/tests/wpt/metadata/fetch/nosniff/script.html.ini deleted file mode 100644 index 68dd7064313..00000000000 --- a/tests/wpt/metadata/fetch/nosniff/script.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[script.html] - type: testharness - [URL query: ] - expected: FAIL - - [URL query: ?type=] - expected: FAIL - - [URL query: ?type=x] - expected: FAIL - - [URL query: ?type=x/x] - expected: FAIL - diff --git a/tests/wpt/metadata/fetch/nosniff/stylesheet.html.ini b/tests/wpt/metadata/fetch/nosniff/stylesheet.html.ini deleted file mode 100644 index 7b0678423d1..00000000000 --- a/tests/wpt/metadata/fetch/nosniff/stylesheet.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[stylesheet.html] - type: testharness - [URL query: ] - expected: FAIL - - [URL query: ?type=] - expected: FAIL - - [URL query: ?type=x] - expected: FAIL - - [URL query: ?type=x/x] - expected: FAIL - diff --git a/tests/wpt/metadata/fetch/nosniff/worker.html.ini b/tests/wpt/metadata/fetch/nosniff/worker.html.ini deleted file mode 100644 index 0131291f8ac..00000000000 --- a/tests/wpt/metadata/fetch/nosniff/worker.html.ini +++ /dev/null @@ -1,14 +0,0 @@ -[worker.html] - type: testharness - [URL query: ] - expected: FAIL - - [URL query: ?type=] - expected: FAIL - - [URL query: ?type=x] - expected: FAIL - - [URL query: ?type=x/x] - expected: FAIL - diff --git a/tests/wpt/metadata/workers/WorkerGlobalScope_importScripts_NosniffErr.htm.ini b/tests/wpt/metadata/workers/WorkerGlobalScope_importScripts_NosniffErr.htm.ini deleted file mode 100644 index e5bf130ac53..00000000000 --- a/tests/wpt/metadata/workers/WorkerGlobalScope_importScripts_NosniffErr.htm.ini +++ /dev/null @@ -1,5 +0,0 @@ -[WorkerGlobalScope_importScripts_NosniffErr.htm] - type: testharness - [importScripts throws on 'nosniff' violation] - expected: FAIL - diff --git a/tests/wpt/metadata/workers/Worker_NosniffErr.htm.ini b/tests/wpt/metadata/workers/Worker_NosniffErr.htm.ini deleted file mode 100644 index 5ee7bfda10e..00000000000 --- a/tests/wpt/metadata/workers/Worker_NosniffErr.htm.ini +++ /dev/null @@ -1,6 +0,0 @@ -[Worker_NosniffErr.htm] - type: testharness - expected: TIMEOUT - [ Worker with nosniff X-Content-Type-Options header ] - expected: TIMEOUT -