Fetching content from HTTPS using rustls (#2024)

* Fetching https

* Fetching dispatcher (HTTP, HTTPS)

* Fetching from github

* Chunked encoding parser

* Abort support

* Fixing tests and review comments

* Cargo.lock order [ci skip]

* Relaxed -> SeqCst

dapps/Cargo.toml

@@ -25,6 +25,7 @@ ethabi = "0.2.2"
 linked-hash-map = "0.3"
 ethcore-rpc = { path = "../rpc" }
 ethcore-util = { path = "../util" }
+https-fetch = { path = "../util/https-fetch" }
 parity-dapps = { git = "https://github.com/ethcore/parity-ui.git", version = "1.4" }
 # List of apps
 parity-dapps-status = { git = "https://github.com/ethcore/parity-ui.git", version = "1.4" }
@@ -37,7 +38,7 @@ clippy = { version = "0.0.85", optional = true}
 serde_codegen = { version = "0.8", optional = true }
 
 [features]
-default = ["serde_codegen", "extra-dapps"]
+default = ["serde_codegen", "extra-dapps", "https-fetch/ca-github-only"]
 extra-dapps = ["parity-dapps-wallet"]
 nightly = ["serde_macros"]
 dev = ["clippy", "ethcore-rpc/dev", "ethcore-util/dev"]

dapps/src/apps/cache.rs

@@ -60,7 +60,7 @@ impl ContentCache {
 				ContentStatus::Fetching(ref abort) => {
 					trace!(target: "dapps", "Aborting {} because of limit.", entry.0);
 					// Mark as aborted
-					abort.store(true, Ordering::Relaxed);
+					abort.store(true, Ordering::SeqCst);
 				},
 				ContentStatus::Ready(ref endpoint) => {
 					trace!(target: "dapps", "Removing {} because of limit.", entry.0);

dapps/src/apps/fetcher.rs

@@ -19,7 +19,7 @@
 //! Uses `URLHint` to resolve addresses into Dapps bundle file location.
 
 use zip;
-use std::{fs, env};
+use std::{fs, env, fmt};
 use std::io::{self, Read, Write};
 use std::path::PathBuf;
 use std::sync::Arc;
@@ -147,6 +147,23 @@ pub enum ValidationError {
 	HashMismatch { expected: H256, got: H256, },
 }
 
+impl fmt::Display for ValidationError {
+	fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> {
+		match *self {
+			ValidationError::Io(ref io) => write!(f, "Unexpected IO error occured: {:?}", io),
+			ValidationError::Zip(ref zip) => write!(f, "Unable to read ZIP archive: {:?}", zip),
+			ValidationError::InvalidDappId => write!(f, "Dapp ID is invalid. It should be 32 bytes hash of content."),
+			ValidationError::ManifestNotFound => write!(f, "Downloaded Dapp bundle did not contain valid manifest.json file."),
+			ValidationError::ManifestSerialization(ref err) => {
+				write!(f, "There was an error during Dapp Manifest serialization: {:?}", err)
+			},
+			ValidationError::HashMismatch { ref expected, ref got } => {
+				write!(f, "Hash of downloaded content did not match. Expected:{:?}, Got:{:?}.", expected, got)
+			},
+		}
+	}
+}
+
 impl From<io::Error> for ValidationError {
 	fn from(err: io::Error) -> Self {
 		ValidationError::Io(err)
@@ -202,8 +219,8 @@ impl ContentValidator for DappInstaller {
 
 	fn validate_and_install(&self, app_path: PathBuf) -> Result<Manifest, ValidationError> {
 		trace!(target: "dapps", "Opening dapp bundle at {:?}", app_path);
-		let mut file = try!(fs::File::open(app_path));
-		let hash = try!(sha3(&mut file));
+		let mut file_reader = io::BufReader::new(try!(fs::File::open(app_path)));
+		let hash = try!(sha3(&mut file_reader));
 		let dapp_id = try!(self.dapp_id.as_str().parse().map_err(|_| ValidationError::InvalidDappId));
 		if dapp_id != hash {
 			return Err(ValidationError::HashMismatch {
@@ -211,6 +228,7 @@ impl ContentValidator for DappInstaller {
 				got: hash,
 			});
 		}
+		let file = file_reader.into_inner();
 		// Unpack archive
 		let mut zip = try!(zip::ZipArchive::new(file));
 		// First find manifest file

dapps/src/apps/urlhint.rs

@@ -33,8 +33,9 @@ pub struct GithubApp {
 
 impl GithubApp {
 	pub fn url(&self) -> String {
+		// Since https fetcher doesn't support redirections we use direct link
 		// format!("https://github.com/{}/{}/archive/{}.zip", self.account, self.repo, self.commit.to_hex())
-		format!("http://github.todr.me/{}/{}/zip/{}", self.account, self.repo, self.commit.to_hex())
+		format!("https://codeload.github.com/{}/{}/zip/{}", self.account, self.repo, self.commit.to_hex())
 	}
 
 	fn commit(bytes: &[u8]) -> Option<[u8;COMMIT_LEN]> {
@@ -300,6 +301,6 @@ mod tests {
 		let url = app.url();
 
 		// then
-		assert_eq!(url, "http://github.todr.me/test/xyz/zip/000102030405060708090a0b0c0d0e0f10111213".to_owned());
+		assert_eq!(url, "https://codeload.github.com/test/xyz/zip/000102030405060708090a0b0c0d0e0f10111213".to_owned());
 	}
 }

dapps/src/handlers/client/fetch_file.rs

@@ -28,6 +28,8 @@ use hyper::client::{Request, Response, DefaultTransport as HttpStream};
 use hyper::header::Connection;
 use hyper::{self, Decoder, Encoder, Next};
 
+use super::FetchError;
+
 #[derive(Debug)]
 pub enum Error {
 	Aborted,
@@ -37,7 +39,7 @@ pub enum Error {
 	HyperError(hyper::Error),
 }
 
-pub type FetchResult = Result<PathBuf, Error>;
+pub type FetchResult = Result<PathBuf, FetchError>;
 pub type OnDone = Box<Fn() + Send>;
 
 pub struct Fetch {
@@ -57,7 +59,7 @@ impl fmt::Debug for Fetch {
 
 impl Drop for Fetch {
     fn drop(&mut self) {
-		let res = self.result.take().unwrap_or(Err(Error::NotStarted));
+		let res = self.result.take().unwrap_or(Err(Error::NotStarted.into()));
 		// Remove file if there was an error
 		if res.is_err() || self.is_aborted() {
 			if let Some(file) = self.file.take() {
@@ -92,10 +94,10 @@ impl Fetch {
 
 impl Fetch {
 	fn is_aborted(&self) -> bool {
-		self.abort.load(Ordering::Relaxed)
+		self.abort.load(Ordering::SeqCst)
 	}
 	fn mark_aborted(&mut self) -> Next {
-		self.result = Some(Err(Error::Aborted));
+		self.result = Some(Err(Error::Aborted.into()));
 		Next::end()
 	}
 }
@@ -121,7 +123,7 @@ impl hyper::client::Handler<HttpStream> for Fetch {
 			return self.mark_aborted();
 		}
 		if *res.status() != StatusCode::Ok {
-			self.result = Some(Err(Error::UnexpectedStatus(*res.status())));
+			self.result = Some(Err(Error::UnexpectedStatus(*res.status()).into()));
 			return Next::end();
 		}
 
@@ -133,7 +135,7 @@ impl hyper::client::Handler<HttpStream> for Fetch {
 				read()
 			},
 			Err(err) => {
-				self.result = Some(Err(Error::IoError(err)));
+				self.result = Some(Err(Error::IoError(err).into()));
 				Next::end()
 			},
 		}
@@ -149,7 +151,7 @@ impl hyper::client::Handler<HttpStream> for Fetch {
             Err(e) => match e.kind() {
                 io::ErrorKind::WouldBlock => Next::read(),
                 _ => {
-					self.result = Some(Err(Error::IoError(e)));
+					self.result = Some(Err(Error::IoError(e).into()));
                     Next::end()
                 }
             }
@@ -157,7 +159,7 @@ impl hyper::client::Handler<HttpStream> for Fetch {
     }
 
     fn on_error(&mut self, err: hyper::Error) -> Next {
-		self.result = Some(Err(Error::HyperError(err)));
+		self.result = Some(Err(Error::HyperError(err).into()));
         Next::remove()
     }
 }

dapps/src/handlers/client/mod.rs

@@ -16,7 +16,98 @@
 
 //! Hyper Client Handlers
 
-mod fetch_file;
+pub mod fetch_file;
+
+use std::env;
+use std::sync::{mpsc, Arc};
+use std::sync::atomic::AtomicBool;
+use std::path::PathBuf;
+
+use hyper;
+use https_fetch as https;
+
+use random_filename;
+use self::fetch_file::{Fetch, Error as HttpFetchError};
+
+pub type FetchResult = Result<PathBuf, FetchError>;
+
+#[derive(Debug)]
+pub enum FetchError {
+	InvalidUrl,
+	Http(HttpFetchError),
+	Https(https::FetchError),
+	Other(String),
+}
+
+impl From<HttpFetchError> for FetchError {
+	fn from(e: HttpFetchError) -> Self {
+		FetchError::Http(e)
+	}
+}
+
+pub struct Client {
+	http_client: hyper::Client<Fetch>,
+	https_client: https::Client,
+}
+
+impl Client {
+	pub fn new() -> Self {
+		Client {
+			http_client: hyper::Client::new().expect("Unable to initialize http client."),
+			https_client: https::Client::new().expect("Unable to initialize https client."),
+		}
+	}
+
+	pub fn close(self) {
+		self.http_client.close();
+		self.https_client.close();
+	}
+
+	pub fn request(&mut self, url: String, abort: Arc<AtomicBool>, on_done: Box<Fn() + Send>) -> Result<mpsc::Receiver<FetchResult>, FetchError> {
+		let is_https = url.starts_with("https://");
+		let url = try!(url.parse().map_err(|_| FetchError::InvalidUrl));
+		trace!(target: "dapps", "Fetching from: {:?}", url);
+		if is_https {
+			let url = try!(Self::convert_url(url));
+
+			let (tx, rx) = mpsc::channel();
+			let temp_path = Self::temp_path();
+			let res = self.https_client.fetch_to_file(url, temp_path.clone(), abort, move |result| {
+				let res = tx.send(
+					result.map(|_| temp_path).map_err(FetchError::Https)
+				);
+				if let Err(_) = res {
+					warn!("Fetch finished, but no one was listening");
+				}
+				on_done();
+			});
+
+			match res {
+				Ok(_) => Ok(rx),
+				Err(e) => Err(FetchError::Other(format!("{:?}", e))),
+			}
+		} else {
+			let (tx, rx) = mpsc::channel();
+			let res = self.http_client.request(url, Fetch::new(tx, abort, on_done));
+
+			match res {
+				Ok(_) => Ok(rx),
+				Err(e) => Err(FetchError::Other(format!("{:?}", e))),
+			}
+		}
+	}
+
+	fn convert_url(url: hyper::Url) -> Result<https::Url, FetchError> {
+		let host = format!("{}", try!(url.host().ok_or(FetchError::InvalidUrl)));
+		let port = try!(url.port_or_known_default().ok_or(FetchError::InvalidUrl));
+		https::Url::new(&host, port, url.path()).map_err(|_| FetchError::InvalidUrl)
+	}
+
+	fn temp_path() -> PathBuf {
+		let mut dir = env::temp_dir();
+		dir.push(random_filename());
+		dir
+	}
+}
 
-pub use self::fetch_file::{Fetch, FetchResult, OnDone};
 

dapps/src/handlers/fetch.rs

@@ -16,18 +16,18 @@
 
 //! Hyper Server Handler that fetches a file during a request (proxy).
 
-use std::{fs, fmt};
+use std::fmt;
 use std::path::PathBuf;
 use std::sync::{mpsc, Arc};
 use std::sync::atomic::AtomicBool;
 use std::time::{Instant, Duration};
 
-use hyper::{header, server, Decoder, Encoder, Next, Method, Control, Client};
+use hyper::{header, server, Decoder, Encoder, Next, Method, Control};
 use hyper::net::HttpStream;
 use hyper::status::StatusCode;
 
 use handlers::ContentHandler;
-use handlers::client::{Fetch, FetchResult};
+use handlers::client::{Client, FetchResult};
 use apps::redirection_address;
 use apps::urlhint::GithubApp;
 use apps::manifest::Manifest;
@@ -45,7 +45,7 @@ enum FetchState {
 }
 
 pub trait ContentValidator {
-	type Error: fmt::Debug;
+	type Error: fmt::Debug + fmt::Display;
 
 	fn validate_and_install(&self, app: PathBuf) -> Result<Manifest, Self::Error>;
 	fn done(&self, Option<&Manifest>);
@@ -55,7 +55,7 @@ pub struct ContentFetcherHandler<H: ContentValidator> {
 	abort: Arc<AtomicBool>,
 	control: Option<Control>,
 	status: FetchState,
-	client: Option<Client<Fetch>>,
+	client: Option<Client>,
 	using_dapps_domains: bool,
 	dapp: H,
 }
@@ -79,7 +79,7 @@ impl<H: ContentValidator> ContentFetcherHandler<H> {
 		using_dapps_domains: bool,
 		handler: H) -> Self {
 
-		let client = Client::new().expect("Failed to create a Client");
+		let client = Client::new();
 		ContentFetcherHandler {
 			abort: abort,
 			control: Some(control),
@@ -90,28 +90,19 @@ impl<H: ContentValidator> ContentFetcherHandler<H> {
 		}
 	}
 
-	fn close_client(client: &mut Option<Client<Fetch>>) {
+	fn close_client(client: &mut Option<Client>) {
 		client.take()
 			.expect("After client is closed we are going into write, hence we can never close it again")
 			.close();
 	}
 
 
-	// TODO [todr] https support
-	fn fetch_app(client: &mut Client<Fetch>, app: &GithubApp, abort: Arc<AtomicBool>, control: Control) -> Result<mpsc::Receiver<FetchResult>, String> {
-		let url = try!(app.url().parse().map_err(|e| format!("{:?}", e)));
-		trace!(target: "dapps", "Fetching from: {:?}", url);
-
-		let (tx, rx) = mpsc::channel();
-		let res = client.request(url, Fetch::new(tx, abort, Box::new(move || {
+	fn fetch_app(client: &mut Client, app: &GithubApp, abort: Arc<AtomicBool>, control: Control) -> Result<mpsc::Receiver<FetchResult>, String> {
+		client.request(app.url(), abort, Box::new(move || {
 			trace!(target: "dapps", "Fetching finished.");
 			// Ignoring control errors
 			let _ = control.ready(Next::read());
-		})));
-		match res {
-			Ok(_) => Ok(rx),
-			Err(e) => Err(format!("{:?}", e)),
-		}
+		})).map_err(|e| format!("{:?}", e))
 	}
 }
 
@@ -177,13 +168,14 @@ impl<H: ContentValidator> server::Handler<HttpStream> for ContentFetcherHandler<
 								trace!(target: "dapps", "Error while validating dapp: {:?}", e);
 								FetchState::Error(ContentHandler::html(
 									StatusCode::BadGateway,
-									format!("<h1>Downloaded bundle does not contain valid app.</h1><pre>{:?}</pre>", e),
+									format!("<h1>Downloaded bundle does not contain valid app.</h1><pre>{}</pre>", e),
 								))
 							},
 							Ok(manifest) => FetchState::Done(manifest)
 						};
 						// Remove temporary zip file
-						let _ = fs::remove_file(path);
+						// TODO [todr] Uncomment me
+						// let _ = fs::remove_file(path);
 						(Some(state), Next::write())
 					},
 					Ok(Err(e)) => {

dapps/src/lib.rs

@@ -58,6 +58,7 @@ extern crate jsonrpc_http_server;
 extern crate mime_guess;
 extern crate rustc_serialize;
 extern crate parity_dapps;
+extern crate https_fetch;
 extern crate ethcore_rpc;
 extern crate ethcore_util as util;
 extern crate linked_hash_map;