Fix CSP for dapps that require eval. (#7867)

* Add allowJsEval to manifest.

* Enable 'unsafe-eval' if requested in manifest.
This commit is contained in:
Tomasz Drwięga 2018-02-15 11:05:20 +01:00 committed by Afri Schoedon
parent 0a34ad50b4
commit 226215eff6
16 changed files with 41 additions and 52 deletions

View File

@ -14,12 +14,10 @@
// You should have received a copy of the GNU General Public License // You should have received a copy of the GNU General Public License
// along with Parity. If not, see <http://www.gnu.org/licenses/>. // along with Parity. If not, see <http://www.gnu.org/licenses/>.
use endpoint::EndpointInfo;
#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)] #[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
#[serde(deny_unknown_fields)] #[serde(deny_unknown_fields)]
pub struct App { pub struct App {
pub id: String, pub id: Option<String>,
pub name: String, pub name: String,
pub description: String, pub description: String,
pub version: String, pub version: String,
@ -28,32 +26,14 @@ pub struct App {
pub icon_url: String, pub icon_url: String,
#[serde(rename="localUrl")] #[serde(rename="localUrl")]
pub local_url: Option<String>, pub local_url: Option<String>,
#[serde(rename="allowJsEval")]
pub allow_js_eval: Option<bool>,
} }
impl App { impl App {
/// Creates `App` instance from `EndpointInfo` and `id`. pub fn with_id(&self, id: &str) -> Self {
pub fn from_info(id: &str, info: &EndpointInfo) -> Self { let mut app = self.clone();
App { app.id = Some(id.into());
id: id.to_owned(), app
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<EndpointInfo> 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,
}
} }
} }

View File

@ -178,7 +178,7 @@ impl ContentValidator for Dapp {
// First find manifest file // First find manifest file
let (mut manifest, manifest_dir) = Self::find_manifest(&mut zip)?; let (mut manifest, manifest_dir) = Self::find_manifest(&mut zip)?;
// Overwrite id to match hash // Overwrite id to match hash
manifest.id = id; manifest.id = Some(id);
// Unpack zip // Unpack zip
for i in 0..zip.len() { for i in 0..zip.len() {

View File

@ -319,12 +319,14 @@ mod tests {
).allow_dapps(true); ).allow_dapps(true);
let handler = local::Dapp::new(pool, path, EndpointInfo { let handler = local::Dapp::new(pool, path, EndpointInfo {
id: None,
name: "fake".into(), name: "fake".into(),
description: "".into(), description: "".into(),
version: "".into(), version: "".into(),
author: "".into(), author: "".into(),
icon_url: "".into(), icon_url: "".into(),
local_url: Some("".into()), local_url: Some("".into()),
allow_js_eval: None,
}, Default::default(), None); }, Default::default(), None);
// when // when

View File

@ -46,17 +46,18 @@ fn read_manifest(name: &str, mut path: PathBuf) -> EndpointInfo {
// Try to deserialize manifest // Try to deserialize manifest
deserialize_manifest(s) deserialize_manifest(s)
}) })
.map(Into::into)
.unwrap_or_else(|e| { .unwrap_or_else(|e| {
warn!(target: "dapps", "Cannot read manifest file at: {:?}. Error: {:?}", path, e); warn!(target: "dapps", "Cannot read manifest file at: {:?}. Error: {:?}", path, e);
EndpointInfo { EndpointInfo {
id: None,
name: name.into(), name: name.into(),
description: name.into(), description: name.into(),
version: "0.0.0".into(), version: "0.0.0".into(),
author: "?".into(), author: "?".into(),
icon_url: "icon.png".into(), icon_url: "icon.png".into(),
local_url: None, local_url: None,
allow_js_eval: Some(false),
} }
}) })
} }

View File

@ -20,8 +20,13 @@ pub use apps::App as Manifest;
pub const MANIFEST_FILENAME: &'static str = "manifest.json"; pub const MANIFEST_FILENAME: &'static str = "manifest.json";
pub fn deserialize_manifest(manifest: String) -> Result<Manifest, String> { pub fn deserialize_manifest(manifest: String) -> Result<Manifest, String> {
serde_json::from_str::<Manifest>(&manifest).map_err(|e| format!("{:?}", e)) let mut manifest = serde_json::from_str::<Manifest>(&manifest).map_err(|e| format!("{:?}", e))?;
// TODO [todr] Manifest validation (especialy: id (used as path)) 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<String, String> { pub fn serialize_manifest(manifest: &Manifest) -> Result<String, String> {

View File

@ -37,16 +37,7 @@ impl EndpointPath {
} }
} }
#[derive(Debug, PartialEq, Clone)] pub type EndpointInfo = ::apps::App;
pub struct EndpointInfo {
pub name: String,
pub description: String,
pub version: String,
pub author: String,
pub icon_url: String,
pub local_url: Option<String>,
}
pub type Endpoints = BTreeMap<String, Box<Endpoint>>; pub type Endpoints = BTreeMap<String, Box<Endpoint>>;
pub type Response = Box<Future<Item=hyper::Response, Error=hyper::Error> + Send>; pub type Response = Box<Future<Item=hyper::Response, Error=hyper::Error> + Send>;
pub type Request = hyper::Request; pub type Request = hyper::Request;

View File

@ -82,7 +82,7 @@ impl Into<hyper::Response> for ContentHandler {
.with_status(self.code) .with_status(self.code)
.with_header(header::ContentType(self.mimetype)) .with_header(header::ContentType(self.mimetype))
.with_body(self.content); .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 res
} }
} }

View File

@ -40,7 +40,7 @@ impl Into<hyper::Response> for EchoHandler {
.with_header(content_type.unwrap_or(header::ContentType::json())) .with_header(content_type.unwrap_or(header::ContentType::json()))
.with_body(self.request.body()); .with_body(self.request.body());
add_security_headers(res.headers_mut(), None); add_security_headers(res.headers_mut(), None, false);
res res
} }
} }

View File

@ -36,7 +36,7 @@ use hyper::header;
use {apps, address, Embeddable}; use {apps, address, Embeddable};
/// Adds security-related headers to the Response. /// 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-XSS-Protection", "1; mode=block");
headers.set_raw("X-Content-Type-Options", "nosniff"); 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)) .map(|&(ref host, port)| address(host, port))
.join(" ") .join(" ")
).unwrap_or_default(); ).unwrap_or_default();
let eval = if allow_js_eval { " 'unsafe-eval'" } else { "" };
&format!( &format!(
"script-src 'self' {};", "script-src 'self' {}{};",
script_src script_src,
eval
) )
} }
// Same restrictions as script-src with additional // Same restrictions as script-src with additional

View File

@ -51,7 +51,7 @@ impl<R: io::Read> StreamingHandler<R> {
.with_status(self.status) .with_status(self.status)
.with_header(header::ContentType(self.mimetype)) .with_header(header::ContentType(self.mimetype))
.with_body(body); .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) (reader, res)
} }

View File

@ -108,7 +108,7 @@ impl Endpoints {
/// Returns a current list of app endpoints. /// Returns a current list of app endpoints.
pub fn list(&self) -> Vec<apps::App> { pub fn list(&self) -> Vec<apps::App> {
self.endpoints.read().iter().filter_map(|(ref k, ref e)| { 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() }).collect()
} }

View File

@ -117,6 +117,7 @@ impl<T: WebApp> Endpoint for Dapp<T> {
file, file,
cache: PageCache::Disabled, cache: PageCache::Disabled,
safe_to_embed_on: self.safe_to_embed_on.clone(), safe_to_embed_on: self.safe_to_embed_on.clone(),
allow_js_eval: self.info.allow_js_eval.clone().unwrap_or(false),
}.into_response(); }.into_response();
self.pool.spawn(reader).forget(); self.pool.spawn(reader).forget();
@ -128,12 +129,14 @@ impl<T: WebApp> Endpoint for Dapp<T> {
impl From<Info> for EndpointInfo { impl From<Info> for EndpointInfo {
fn from(info: Info) -> Self { fn from(info: Info) -> Self {
EndpointInfo { EndpointInfo {
id: None,
name: info.name.into(), name: info.name.into(),
description: info.description.into(), description: info.description.into(),
author: info.author.into(), author: info.author.into(),
icon_url: info.icon_url.into(), icon_url: info.icon_url.into(),
local_url: None, local_url: None,
version: info.version.into(), version: info.version.into(),
allow_js_eval: None,
} }
} }
} }

View File

@ -59,6 +59,8 @@ pub struct PageHandler<T: DappFile> {
pub safe_to_embed_on: Embeddable, pub safe_to_embed_on: Embeddable,
/// Cache settings for this page. /// Cache settings for this page.
pub cache: PageCache, pub cache: PageCache,
/// Allow JS unsafe-eval.
pub allow_js_eval: bool,
} }
impl<T: DappFile> PageHandler<T> { impl<T: DappFile> PageHandler<T> {
@ -93,7 +95,7 @@ impl<T: DappFile> PageHandler<T> {
headers.set(header::ContentType(file.content_type().to_owned())); 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 { let initial_content = if file.content_type().to_owned() == mime::TEXT_HTML {

View File

@ -98,6 +98,7 @@ impl Dapp {
file: self.get_file(path), file: self.get_file(path),
cache: self.cache, cache: self.cache,
safe_to_embed_on: self.embeddable_on.clone(), 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(); }.into_response();
self.pool.spawn(reader).forget(); self.pool.spawn(reader).forget();

View File

@ -181,7 +181,7 @@ fn should_return_fetched_dapp_content() {
assert_security_headers_for_embed(&response2.headers); assert_security_headers_for_embed(&response2.headers);
assert_eq!( assert_eq!(
response2.body, response2.body,
r#"D2 r#"EA
{ {
"id": "9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a", "id": "9c94e154dab8acf859b30ee80fc828fb1d38359d938751b65db71d460588d82a",
"name": "Gavcoin", "name": "Gavcoin",
@ -189,7 +189,8 @@ fn should_return_fetched_dapp_content() {
"version": "1.0.0", "version": "1.0.0",
"author": "", "author": "",
"iconUrl": "icon.png", "iconUrl": "icon.png",
"localUrl": null "localUrl": null,
"allowJsEval": false
} }
0 0

View File

@ -293,7 +293,7 @@ mod server {
self.endpoints.list() self.endpoints.list()
.into_iter() .into_iter()
.map(|app| rpc_apis::LocalDapp { .map(|app| rpc_apis::LocalDapp {
id: app.id, id: app.id.unwrap_or_else(|| "unknown".into()),
name: app.name, name: app.name,
description: app.description, description: app.description,
version: app.version, version: app.version,