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 374889 c5f07e7744d0ae707051ae57a2d9f642251bfc3f
parent 374888 32604a6dd45e988b485b3f93b2a5dc5410a89b1c
child 374890 0bd17b868a31ce1e53b87f7a619974ad8c796f84
push id10863
push userjlorenzo@mozilla.com
push dateMon, 06 Mar 2017 23:02:23 +0000
treeherdermozilla-aurora@0931190cd725 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersjdm
milestone54.0a1
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!()