servo: Merge #14620 - Implemented nosniff for fetch algorithm (from dpyro:nosniff); r=jdm
authorSumant Manne <sumant.manne@gmail.com>
Fri, 03 Mar 2017 06:56:30 -0800
changeset 345862 c5f07e7744d0ae707051ae57a2d9f642251bfc3f
parent 345861 32604a6dd45e988b485b3f93b2a5dc5410a89b1c
child 345863 0bd17b868a31ce1e53b87f7a619974ad8c796f84
push id31451
push usercbook@mozilla.com
push dateMon, 06 Mar 2017 09:52:09 +0000
treeherdermozilla-central@7099e03837e8 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone54.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #14620 - Implemented nosniff for fetch algorithm (from dpyro:nosniff); r=jdm <!-- Please describe your changes on the following line: --> Implemented [nosniff](https://fetch.spec.whatwg.org/#should-response-to-request-be-blocked-due-to-nosniff?) for [fetch algorithm](https://fetch.spec.whatwg.org). --- <!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: --> - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #14521 (github issue number if applicable). <!-- Either: --> - [X] There are tests for these changes <!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. --> Source-Repo: https://github.com/servo/servo Source-Revision: 5662af9057557ee3db3b9766b0ef0d3fa11dcbb2
servo/components/net/fetch/methods.rs
servo/tests/unit/net/fetch.rs
--- a/servo/components/net/fetch/methods.rs
+++ b/servo/components/net/fetch/methods.rs
@@ -3,31 +3,35 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use blob_loader::load_blob_sync;
 use data_loader::decode;
 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;
 use mime_guess::guess_mime_type;
 use net_traits::{FetchTaskTarget, NetworkError, ReferrerPolicy};
 use net_traits::request::{RedirectMode, Referrer, Request, RequestMode, ResponseTainting};
 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;
 
 pub type Target<'a> = &'a mut (FetchTaskTarget + Send);
 
 pub enum Data {
     Payload(Vec<u8>),
     Done,
@@ -264,48 +268,67 @@ pub fn main_fetch(request: Rc<Request>,
             ResponseTainting::CorsTainting => ResponseType::Cors,
             ResponseTainting::Opaque => ResponseType::Opaque,
         };
         response.to_filtered(response_type)
     } else {
         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()
         };
 
         // Step 15
         if internal_response.url_list.borrow().is_empty() {
             *internal_response.url_list.borrow_mut() = request.url_list.borrow().clone();
         }
 
         // 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);
         response_loaded = true;
 
         // Substep 2
         let ref integrity_metadata = *request.integrity_metadata.borrow();
         if response.termination_reason.is_none() &&
@@ -505,8 +528,99 @@ fn is_null_body_status(status: &Option<S
             StatusCode::SwitchingProtocols | StatusCode::NoContent |
                 StatusCode::ResetContent | StatusCode::NotModified => true,
             _ => false
         },
         _ => false
     }
 }
 
+/// 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<u8>]) -> HyperResult<Self> {
+            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::<XContentTypeOptions>() {
+        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::<ContentType>();
+    // 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
+    };
+}
--- a/servo/tests/unit/net/fetch.rs
+++ b/servo/tests/unit/net/fetch.rs
@@ -26,17 +26,17 @@ use msg::constellation_msg::TEST_PIPELIN
 use net::fetch::cors_cache::CorsCache;
 use net::fetch::methods::FetchContext;
 use net::filemanager_thread::FileManager;
 use net::hsts::HstsEntry;
 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};
 use std::fs::File;
 use std::io::Read;
 use std::rc::Rc;
 use std::sync::{Arc, Mutex};
 use std::sync::atomic::{AtomicUsize, Ordering};
@@ -596,16 +596,61 @@ fn test_fetch_with_sri_sucess() {
     // Set the flag.
     request.local_urls_only = false;
 
     let response = fetch(request, None);
 
     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::<ContentType>());
+            // 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 {
             RequestUri::AbsolutePath(url) =>
                 url.split("/").collect::<String>().parse::<u32>().unwrap_or(0),
             RequestUri::AbsoluteUri(url) =>
                 url.path_segments().unwrap().next_back().unwrap().parse::<u32>().unwrap_or(0),
             _ => panic!()