From 226215eff6362c27fe1df32049836c7c90a5d04e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= Date: Thu, 15 Feb 2018 11:05:20 +0100 Subject: [PATCH] Fix CSP for dapps that require eval. (#7867) * Add allowJsEval to manifest. * Enable 'unsafe-eval' if requested in manifest. --- dapps/src/apps/app.rs | 34 ++++++---------------------- dapps/src/apps/fetcher/installers.rs | 2 +- dapps/src/apps/fetcher/mod.rs | 2 ++ dapps/src/apps/fs.rs | 3 ++- dapps/src/apps/manifest.rs | 9 ++++++-- dapps/src/endpoint.rs | 11 +-------- dapps/src/handlers/content.rs | 2 +- dapps/src/handlers/echo.rs | 2 +- dapps/src/handlers/mod.rs | 9 +++++--- dapps/src/handlers/streaming.rs | 2 +- dapps/src/lib.rs | 2 +- dapps/src/page/builtin.rs | 3 +++ dapps/src/page/handler.rs | 4 +++- dapps/src/page/local.rs | 1 + dapps/src/tests/fetch.rs | 5 ++-- parity/dapps.rs | 2 +- 16 files changed, 41 insertions(+), 52 deletions(-) diff --git a/dapps/src/apps/app.rs b/dapps/src/apps/app.rs index 3b674aa15..c75346124 100644 --- a/dapps/src/apps/app.rs +++ b/dapps/src/apps/app.rs @@ -14,12 +14,10 @@ // You should have received a copy of the GNU General Public License // along with Parity. If not, see . -use endpoint::EndpointInfo; - #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] #[serde(deny_unknown_fields)] pub struct App { - pub id: String, + pub id: Option, pub name: String, pub description: String, pub version: String, @@ -28,32 +26,14 @@ pub struct App { pub icon_url: String, #[serde(rename="localUrl")] pub local_url: Option, + #[serde(rename="allowJsEval")] + pub allow_js_eval: Option, } impl App { - /// Creates `App` instance from `EndpointInfo` and `id`. - pub fn from_info(id: &str, info: &EndpointInfo) -> Self { - App { - id: id.to_owned(), - name: info.name.to_owned(), - description: info.description.to_owned(), - version: info.version.to_owned(), - author: info.author.to_owned(), - icon_url: info.icon_url.to_owned(), - local_url: info.local_url.to_owned(), - } - } -} - -impl Into for App { - fn into(self) -> EndpointInfo { - EndpointInfo { - name: self.name, - description: self.description, - version: self.version, - author: self.author, - icon_url: self.icon_url, - local_url: self.local_url, - } + pub fn with_id(&self, id: &str) -> Self { + let mut app = self.clone(); + app.id = Some(id.into()); + app } } diff --git a/dapps/src/apps/fetcher/installers.rs b/dapps/src/apps/fetcher/installers.rs index 88477cfa2..f47ab855c 100644 --- a/dapps/src/apps/fetcher/installers.rs +++ b/dapps/src/apps/fetcher/installers.rs @@ -178,7 +178,7 @@ impl ContentValidator for Dapp { // First find manifest file let (mut manifest, manifest_dir) = Self::find_manifest(&mut zip)?; // Overwrite id to match hash - manifest.id = id; + manifest.id = Some(id); // Unpack zip for i in 0..zip.len() { diff --git a/dapps/src/apps/fetcher/mod.rs b/dapps/src/apps/fetcher/mod.rs index 2b10351e1..44352f76f 100644 --- a/dapps/src/apps/fetcher/mod.rs +++ b/dapps/src/apps/fetcher/mod.rs @@ -319,12 +319,14 @@ mod tests { ).allow_dapps(true); let handler = local::Dapp::new(pool, path, EndpointInfo { + id: None, name: "fake".into(), description: "".into(), version: "".into(), author: "".into(), icon_url: "".into(), local_url: Some("".into()), + allow_js_eval: None, }, Default::default(), None); // when diff --git a/dapps/src/apps/fs.rs b/dapps/src/apps/fs.rs index c581c2a8a..3d93a2fae 100644 --- a/dapps/src/apps/fs.rs +++ b/dapps/src/apps/fs.rs @@ -46,17 +46,18 @@ fn read_manifest(name: &str, mut path: PathBuf) -> EndpointInfo { // Try to deserialize manifest deserialize_manifest(s) }) - .map(Into::into) .unwrap_or_else(|e| { warn!(target: "dapps", "Cannot read manifest file at: {:?}. Error: {:?}", path, e); EndpointInfo { + id: None, name: name.into(), description: name.into(), version: "0.0.0".into(), author: "?".into(), icon_url: "icon.png".into(), local_url: None, + allow_js_eval: Some(false), } }) } diff --git a/dapps/src/apps/manifest.rs b/dapps/src/apps/manifest.rs index 946114980..e32048219 100644 --- a/dapps/src/apps/manifest.rs +++ b/dapps/src/apps/manifest.rs @@ -20,8 +20,13 @@ pub use apps::App as Manifest; pub const MANIFEST_FILENAME: &'static str = "manifest.json"; pub fn deserialize_manifest(manifest: String) -> Result { - serde_json::from_str::(&manifest).map_err(|e| format!("{:?}", e)) - // TODO [todr] Manifest validation (especialy: id (used as path)) + let mut manifest = serde_json::from_str::(&manifest).map_err(|e| format!("{:?}", e))?; + if manifest.id.is_none() { + return Err("App 'id' is missing.".into()); + } + manifest.allow_js_eval = Some(manifest.allow_js_eval.unwrap_or(false)); + + Ok(manifest) } pub fn serialize_manifest(manifest: &Manifest) -> Result { diff --git a/dapps/src/endpoint.rs b/dapps/src/endpoint.rs index 5491b76de..fd05445c2 100644 --- a/dapps/src/endpoint.rs +++ b/dapps/src/endpoint.rs @@ -37,16 +37,7 @@ impl EndpointPath { } } -#[derive(Debug, PartialEq, Clone)] -pub struct EndpointInfo { - pub name: String, - pub description: String, - pub version: String, - pub author: String, - pub icon_url: String, - pub local_url: Option, -} - +pub type EndpointInfo = ::apps::App; pub type Endpoints = BTreeMap>; pub type Response = Box + Send>; pub type Request = hyper::Request; diff --git a/dapps/src/handlers/content.rs b/dapps/src/handlers/content.rs index 711a985bd..c7eccf474 100644 --- a/dapps/src/handlers/content.rs +++ b/dapps/src/handlers/content.rs @@ -82,7 +82,7 @@ impl Into for ContentHandler { .with_status(self.code) .with_header(header::ContentType(self.mimetype)) .with_body(self.content); - add_security_headers(&mut res.headers_mut(), self.safe_to_embed_on); + add_security_headers(&mut res.headers_mut(), self.safe_to_embed_on, false); res } } diff --git a/dapps/src/handlers/echo.rs b/dapps/src/handlers/echo.rs index 6ebac3d35..375f04790 100644 --- a/dapps/src/handlers/echo.rs +++ b/dapps/src/handlers/echo.rs @@ -40,7 +40,7 @@ impl Into for EchoHandler { .with_header(content_type.unwrap_or(header::ContentType::json())) .with_body(self.request.body()); - add_security_headers(res.headers_mut(), None); + add_security_headers(res.headers_mut(), None, false); res } } diff --git a/dapps/src/handlers/mod.rs b/dapps/src/handlers/mod.rs index e8e8ef874..f78f46c76 100644 --- a/dapps/src/handlers/mod.rs +++ b/dapps/src/handlers/mod.rs @@ -36,7 +36,7 @@ use hyper::header; use {apps, address, Embeddable}; /// Adds security-related headers to the Response. -pub fn add_security_headers(headers: &mut header::Headers, embeddable_on: Embeddable) { +pub fn add_security_headers(headers: &mut header::Headers, embeddable_on: Embeddable, allow_js_eval: bool) { headers.set_raw("X-XSS-Protection", "1; mode=block"); headers.set_raw("X-Content-Type-Options", "nosniff"); @@ -75,9 +75,12 @@ pub fn add_security_headers(headers: &mut header::Headers, embeddable_on: Embedd .map(|&(ref host, port)| address(host, port)) .join(" ") ).unwrap_or_default(); + let eval = if allow_js_eval { " 'unsafe-eval'" } else { "" }; + &format!( - "script-src 'self' {};", - script_src + "script-src 'self' {}{};", + script_src, + eval ) } // Same restrictions as script-src with additional diff --git a/dapps/src/handlers/streaming.rs b/dapps/src/handlers/streaming.rs index 73f7995b2..269e4c5d2 100644 --- a/dapps/src/handlers/streaming.rs +++ b/dapps/src/handlers/streaming.rs @@ -51,7 +51,7 @@ impl StreamingHandler { .with_status(self.status) .with_header(header::ContentType(self.mimetype)) .with_body(body); - add_security_headers(&mut res.headers_mut(), self.safe_to_embed_on); + add_security_headers(&mut res.headers_mut(), self.safe_to_embed_on, false); (reader, res) } diff --git a/dapps/src/lib.rs b/dapps/src/lib.rs index bf9ae6ab9..6f901e4be 100644 --- a/dapps/src/lib.rs +++ b/dapps/src/lib.rs @@ -108,7 +108,7 @@ impl Endpoints { /// Returns a current list of app endpoints. pub fn list(&self) -> Vec { self.endpoints.read().iter().filter_map(|(ref k, ref e)| { - e.info().map(|ref info| apps::App::from_info(k, info)) + e.info().map(|ref info| info.with_id(k)) }).collect() } diff --git a/dapps/src/page/builtin.rs b/dapps/src/page/builtin.rs index 0e93b1c68..827fe27a3 100644 --- a/dapps/src/page/builtin.rs +++ b/dapps/src/page/builtin.rs @@ -117,6 +117,7 @@ impl Endpoint for Dapp { file, cache: PageCache::Disabled, safe_to_embed_on: self.safe_to_embed_on.clone(), + allow_js_eval: self.info.allow_js_eval.clone().unwrap_or(false), }.into_response(); self.pool.spawn(reader).forget(); @@ -128,12 +129,14 @@ impl Endpoint for Dapp { impl From for EndpointInfo { fn from(info: Info) -> Self { EndpointInfo { + id: None, name: info.name.into(), description: info.description.into(), author: info.author.into(), icon_url: info.icon_url.into(), local_url: None, version: info.version.into(), + allow_js_eval: None, } } } diff --git a/dapps/src/page/handler.rs b/dapps/src/page/handler.rs index 576455115..a4bd6d71b 100644 --- a/dapps/src/page/handler.rs +++ b/dapps/src/page/handler.rs @@ -59,6 +59,8 @@ pub struct PageHandler { pub safe_to_embed_on: Embeddable, /// Cache settings for this page. pub cache: PageCache, + /// Allow JS unsafe-eval. + pub allow_js_eval: bool, } impl PageHandler { @@ -93,7 +95,7 @@ impl PageHandler { headers.set(header::ContentType(file.content_type().to_owned())); - add_security_headers(&mut headers, self.safe_to_embed_on); + add_security_headers(&mut headers, self.safe_to_embed_on, self.allow_js_eval); } let initial_content = if file.content_type().to_owned() == mime::TEXT_HTML { diff --git a/dapps/src/page/local.rs b/dapps/src/page/local.rs index 82f1f28f3..a1746efcd 100644 --- a/dapps/src/page/local.rs +++ b/dapps/src/page/local.rs @@ -98,6 +98,7 @@ impl Dapp { file: self.get_file(path), cache: self.cache, safe_to_embed_on: self.embeddable_on.clone(), + allow_js_eval: self.info.as_ref().and_then(|x| x.allow_js_eval).unwrap_or(false), }.into_response(); self.pool.spawn(reader).forget(); diff --git a/dapps/src/tests/fetch.rs b/dapps/src/tests/fetch.rs index c6ae401e7..59eeaf8d6 100644 --- a/dapps/src/tests/fetch.rs +++ b/dapps/src/tests/fetch.rs @@ -181,7 +181,7 @@ fn should_return_fetched_dapp_content() { assert_security_headers_for_embed(&response2.headers); assert_eq!( response2.body, - r#"D2 + r#"EA { "id": "9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a", "name": "Gavcoin", @@ -189,7 +189,8 @@ fn should_return_fetched_dapp_content() { "version": "1.0.0", "author": "", "iconUrl": "icon.png", - "localUrl": null + "localUrl": null, + "allowJsEval": false } 0 diff --git a/parity/dapps.rs b/parity/dapps.rs index 0d66bdc85..5c52b204c 100644 --- a/parity/dapps.rs +++ b/parity/dapps.rs @@ -293,7 +293,7 @@ mod server { self.endpoints.list() .into_iter() .map(|app| rpc_apis::LocalDapp { - id: app.id, + id: app.id.unwrap_or_else(|| "unknown".into()), name: app.name, description: app.description, version: app.version,