From 236f06736e45c2a70f43589c9d447a0a3ef240b5 Mon Sep 17 00:00:00 2001 From: Jesse Luehrs Date: Sun, 12 Apr 2020 00:08:03 -0400 Subject: improve error handling and reporting --- src/bin/rbw-agent/actions.rs | 177 +++++++++++++++++++++++++++++++------------ src/bin/rbw-agent/agent.rs | 42 +++++++--- src/bin/rbw-agent/daemon.rs | 27 ++++--- src/bin/rbw-agent/main.rs | 47 ++++++++++-- src/bin/rbw-agent/sock.rs | 36 ++++++--- src/bin/rbw/actions.rs | 54 +++++++------ src/bin/rbw/commands.rs | 164 +++++++++++++++++++++++++-------------- src/bin/rbw/main.rs | 63 ++++++++++----- src/bin/rbw/sock.rs | 31 +++++--- src/config.rs | 2 + src/db.rs | 4 + src/locked.rs | 2 + src/pinentry.rs | 4 + 13 files changed, 455 insertions(+), 198 deletions(-) diff --git a/src/bin/rbw-agent/actions.rs b/src/bin/rbw-agent/actions.rs index 66d732f..0eec665 100644 --- a/src/bin/rbw-agent/actions.rs +++ b/src/bin/rbw-agent/actions.rs @@ -1,26 +1,43 @@ +use anyhow::Context as _; + pub async fn login( sock: &mut crate::sock::Sock, state: std::sync::Arc>, tty: Option<&str>, -) { +) -> anyhow::Result<()> { let mut state = state.write().await; - let email = config_email().await; + let email = config_email() + .await + .context("failed to read email from config")?; let mut db = rbw::db::Db::load_async(&email) .await .unwrap_or_else(|_| rbw::db::Db::new()); if db.needs_login() { - let url = config_base_url().await; - let url = reqwest::Url::parse(&url).unwrap(); + let url_str = config_base_url() + .await + .context("failed to read base url from config")?; + let url = reqwest::Url::parse(&url_str) + .context("failed to parse base url")?; + let host = if let Some(host) = url.host_str() { + host + } else { + return Err(anyhow::anyhow!( + "couldn't find host in rbw base url {}", + url_str + )); + }; let password = rbw::pinentry::getpin( "Master Password", - &format!("Log in to {}", url.host_str().unwrap()), + &format!("Log in to {}", host), tty, ) .await - .unwrap(); + .context("failed to read password from pinentry")?; let (access_token, refresh_token, iterations, protected_key, keys) = - rbw::actions::login(&email, &password).await.unwrap(); + rbw::actions::login(&email, &password) + .await + .context("failed to log in to bitwarden instance")?; state.priv_key = Some(keys); @@ -28,108 +45,172 @@ pub async fn login( db.refresh_token = Some(refresh_token); db.iterations = Some(iterations); db.protected_key = Some(protected_key); - db.save_async(&email).await.unwrap(); + db.save_async(&email) + .await + .context("failed to save local database")?; - sync(sock).await; + sync(sock).await?; } else { - respond_ack(sock).await; + respond_ack(sock).await?; } + + Ok(()) } pub async fn unlock( sock: &mut crate::sock::Sock, state: std::sync::Arc>, tty: Option<&str>, -) { +) -> anyhow::Result<()> { let mut state = state.write().await; if state.needs_unlock() { - let email = config_email().await; + let email = config_email() + .await + .context("failed to read email from config")?; let password = rbw::pinentry::getpin( "Master Password", "Unlock the local database", tty, ) .await - .unwrap(); + .context("failed to read password from pinentry")?; let db = rbw::db::Db::load_async(&email) .await - .unwrap_or_else(|_| rbw::db::Db::new()); - + .context("failed to load local database")?; + + let iterations = if let Some(iterations) = db.iterations { + iterations + } else { + return Err(anyhow::anyhow!( + "failed to find number of iterations in db" + )); + }; + let protected_key = if let Some(protected_key) = db.protected_key { + protected_key + } else { + return Err(anyhow::anyhow!( + "failed to find protected key in db" + )); + }; let keys = rbw::actions::unlock( &email, &password, - db.iterations.unwrap(), - db.protected_key.as_deref().unwrap(), + iterations, + &protected_key, ) .await - .unwrap(); + .context("failed to unlock database")?; state.priv_key = Some(keys); } - respond_ack(sock).await; + respond_ack(sock).await?; + + Ok(()) } pub async fn lock( sock: &mut crate::sock::Sock, state: std::sync::Arc>, -) { +) -> anyhow::Result<()> { let mut state = state.write().await; state.priv_key = None; - respond_ack(sock).await; + respond_ack(sock).await?; + + Ok(()) } -pub async fn sync(sock: &mut crate::sock::Sock) { - let email = config_email().await; +pub async fn sync(sock: &mut crate::sock::Sock) -> anyhow::Result<()> { + let email = config_email() + .await + .context("failed to read email from config")?; let mut db = rbw::db::Db::load_async(&email) .await - .unwrap_or_else(|_| rbw::db::Db::new()); + .context("failed to load local database")?; - let (protected_key, ciphers) = - rbw::actions::sync(db.access_token.as_ref().unwrap()) - .await - .unwrap(); + let access_token = if let Some(access_token) = &db.access_token { + access_token + } else { + return Err(anyhow::anyhow!("failed to find access token in db")); + }; + let (protected_key, ciphers) = rbw::actions::sync(&access_token) + .await + .context("failed to sync database from server")?; db.protected_key = Some(protected_key); db.ciphers = ciphers; - db.save_async(&email).await.unwrap(); + db.save_async(&email) + .await + .context("failed to save database")?; + + respond_ack(sock).await?; - respond_ack(sock).await; + Ok(()) } pub async fn decrypt( sock: &mut crate::sock::Sock, state: std::sync::Arc>, cipherstring: &str, -) { +) -> anyhow::Result<()> { let state = state.read().await; - let keys = state.priv_key.as_ref().unwrap(); - let cipherstring = - rbw::cipherstring::CipherString::new(cipherstring).unwrap(); - let plaintext = - String::from_utf8(cipherstring.decrypt(keys).unwrap()).unwrap(); - - respond_decrypt(sock, plaintext).await; + let keys = if let Some(keys) = &state.priv_key { + keys + } else { + return Err(anyhow::anyhow!( + "failed to find decryption keys in in-memory state" + )); + }; + let cipherstring = rbw::cipherstring::CipherString::new(cipherstring) + .context("failed to parse encrypted secret")?; + let plaintext = String::from_utf8( + cipherstring + .decrypt(&keys) + .context("failed to decrypt encrypted secret")?, + ) + .context("failed to parse decrypted secret")?; + + respond_decrypt(sock, plaintext).await?; + + Ok(()) } -async fn respond_ack(sock: &mut crate::sock::Sock) { - sock.send(&rbw::agent::Response::Ack).await; +async fn respond_ack(sock: &mut crate::sock::Sock) -> anyhow::Result<()> { + sock.send(&rbw::agent::Response::Ack) + .await + .context("failed to send response")?; + + Ok(()) } -async fn respond_decrypt(sock: &mut crate::sock::Sock, plaintext: String) { +async fn respond_decrypt( + sock: &mut crate::sock::Sock, + plaintext: String, +) -> anyhow::Result<()> { sock.send(&rbw::agent::Response::Decrypt { plaintext }) - .await; + .await + .context("failed to send response")?; + + Ok(()) } -async fn config_email() -> String { - let config = rbw::config::Config::load_async().await.unwrap(); - config.email.unwrap() +async fn config_email() -> anyhow::Result { + let config = rbw::config::Config::load_async() + .await + .context("failed to load config")?; + if let Some(email) = config.email { + Ok(email) + } else { + Err(anyhow::anyhow!("failed to find email address in config")) + } } -async fn config_base_url() -> String { - let config = rbw::config::Config::load_async().await.unwrap(); - config.base_url() +async fn config_base_url() -> anyhow::Result { + let config = rbw::config::Config::load_async() + .await + .context("failed to load config")?; + Ok(config.base_url()) } diff --git a/src/bin/rbw-agent/agent.rs b/src/bin/rbw-agent/agent.rs index 439f458..f092787 100644 --- a/src/bin/rbw-agent/agent.rs +++ b/src/bin/rbw-agent/agent.rs @@ -1,3 +1,4 @@ +use anyhow::Context as _; use tokio::stream::StreamExt as _; pub struct State { @@ -16,28 +17,44 @@ pub struct Agent { } impl Agent { - pub fn new() -> Self { - let config = rbw::config::Config::load().unwrap(); - Self { + pub fn new() -> anyhow::Result { + let config = + rbw::config::Config::load().context("failed to load config")?; + Ok(Self { timeout: tokio::time::delay_for( tokio::time::Duration::from_secs(config.lock_timeout), ), state: std::sync::Arc::new(tokio::sync::RwLock::new(State { priv_key: None, })), - } + }) } - pub async fn run(&mut self, mut listener: tokio::net::UnixListener) { + pub async fn run( + &mut self, + mut listener: tokio::net::UnixListener, + ) -> anyhow::Result<()> { loop { tokio::select! { sock = listener.next() => { - let mut sock - = crate::sock::Sock::new(sock.unwrap().unwrap()); + let sock = if let Some(sock) = sock { + sock + } else { + return Ok(()); + }; + let mut sock = crate::sock::Sock::new( + sock.context("failed to accept incoming connection")? + ); let state = self.state.clone(); tokio::spawn(async move { - let req = sock.recv().await; - handle_request(&req, &mut sock, state.clone()).await; + let res + = handle_request(&mut sock, state.clone()).await; + if let Err(e) = res { + // unwrap is the only option here + sock.send(&rbw::agent::Response::Error { + error: format!("{:#}", e), + }).await.unwrap(); + } }); } _ = &mut self.timeout => { @@ -52,10 +69,13 @@ impl Agent { } async fn handle_request( - req: &rbw::agent::Request, sock: &mut crate::sock::Sock, state: std::sync::Arc>, -) { +) -> anyhow::Result<()> { + let req = sock + .recv() + .await + .context("failed to receive incoming message")?; match &req.action { rbw::agent::Action::Login => { crate::actions::login(sock, state.clone(), req.tty.as_deref()) diff --git a/src/bin/rbw-agent/daemon.rs b/src/bin/rbw-agent/daemon.rs index 64cf298..80c94ad 100644 --- a/src/bin/rbw-agent/daemon.rs +++ b/src/bin/rbw-agent/daemon.rs @@ -1,35 +1,42 @@ +use anyhow::Context as _; + pub struct StartupAck { writer: std::os::unix::io::RawFd, } impl StartupAck { - pub fn ack(&self) { - nix::unistd::write(self.writer, &[0]).unwrap(); - nix::unistd::close(self.writer).unwrap(); + pub fn ack(&self) -> anyhow::Result<()> { + nix::unistd::write(self.writer, &[0])?; + nix::unistd::close(self.writer)?; + Ok(()) } } impl Drop for StartupAck { fn drop(&mut self) { - nix::unistd::close(self.writer).unwrap(); + // best effort close here, can't do better in a destructor + let _ = nix::unistd::close(self.writer); } } -pub fn daemonize() -> StartupAck { +pub fn daemonize() -> anyhow::Result { let runtime_dir = rbw::dirs::runtime_dir(); - std::fs::create_dir_all(&runtime_dir).unwrap(); + std::fs::create_dir_all(&runtime_dir) + .context("failed to create runtime directory")?; - let (r, w) = nix::unistd::pipe().unwrap(); + let (r, w) = nix::unistd::pipe()?; let res = daemonize::Daemonize::new() .pid_file(runtime_dir.join("pidfile")) .exit_action(move || { - nix::unistd::close(w).unwrap(); + // unwraps are necessary because not really a good way to handle + // errors here otherwise + let _ = nix::unistd::close(w); let mut buf = [0; 1]; nix::unistd::read(r, &mut buf).unwrap(); nix::unistd::close(r).unwrap(); }) .start(); - nix::unistd::close(r).unwrap(); + let _ = nix::unistd::close(r); match res { Ok(_) => (), @@ -46,5 +53,5 @@ pub fn daemonize() -> StartupAck { } } - StartupAck { writer: w } + Ok(StartupAck { writer: w }) } diff --git a/src/bin/rbw-agent/main.rs b/src/bin/rbw-agent/main.rs index 235c4f2..622778e 100644 --- a/src/bin/rbw-agent/main.rs +++ b/src/bin/rbw-agent/main.rs @@ -1,24 +1,57 @@ +use anyhow::Context as _; + mod actions; mod agent; mod daemon; mod sock; -fn main() { +async fn tokio_main( + startup_ack: crate::daemon::StartupAck, +) -> anyhow::Result<()> { + let listener = + crate::sock::listen().context("failed to listen on socket")?; + + startup_ack.ack()?; + + let mut agent = crate::agent::Agent::new()?; + agent.run(listener).await?; + + Ok(()) +} + +fn real_main() -> anyhow::Result<()> { env_logger::from_env( env_logger::Env::default().default_filter_or("info"), ) .init(); - let startup_ack = daemon::daemonize(); + let startup_ack = daemon::daemonize().context("failed to daemonize")?; + let (w, r) = std::sync::mpsc::channel(); // can't use tokio::main because we need to daemonize before starting the // tokio runloop, or else things break + // unwrap is fine here because there's no good reason that this should + // ever fail tokio::runtime::Runtime::new().unwrap().block_on(async { - let listener = crate::sock::listen(); + if let Err(e) = tokio_main(startup_ack).await { + // this unwrap is fine because it's the only real option here + w.send(e).unwrap(); + } + }); + + if let Ok(e) = r.recv() { + return Err(e); + } - startup_ack.ack(); + Ok(()) +} + +fn main() { + let res = real_main(); - let mut agent = crate::agent::Agent::new(); - agent.run(listener.unwrap()).await; - }) + if let Err(e) = res { + // XXX log file? + eprintln!("{:#}", e); + std::process::exit(1); + } } diff --git a/src/bin/rbw-agent/sock.rs b/src/bin/rbw-agent/sock.rs index 73ad5c2..6931b13 100644 --- a/src/bin/rbw-agent/sock.rs +++ b/src/bin/rbw-agent/sock.rs @@ -1,3 +1,4 @@ +use anyhow::Context as _; use tokio::io::{AsyncBufReadExt as _, AsyncWriteExt as _}; pub struct Sock(tokio::net::UnixStream); @@ -7,30 +8,45 @@ impl Sock { Self(s) } - pub async fn send(&mut self, res: &rbw::agent::Response) { + pub async fn send( + &mut self, + res: &rbw::agent::Response, + ) -> anyhow::Result<()> { let Self(sock) = self; - sock.write_all(serde_json::to_string(res).unwrap().as_bytes()) + sock.write_all( + serde_json::to_string(res) + .context("failed to serialize message")? + .as_bytes(), + ) + .await + .context("failed to write message to socket")?; + sock.write_all(b"\n") .await - .unwrap(); - sock.write_all(b"\n").await.unwrap(); + .context("failed to write message to socket")?; + Ok(()) } - pub async fn recv(&mut self) -> rbw::agent::Request { + pub async fn recv(&mut self) -> anyhow::Result { let Self(sock) = self; let mut buf = tokio::io::BufStream::new(sock); let mut line = String::new(); - buf.read_line(&mut line).await.unwrap(); - serde_json::from_str(&line).unwrap() + buf.read_line(&mut line) + .await + .context("failed to read message from socket")?; + Ok(serde_json::from_str(&line).context("failed to parse message")?) } } pub fn listen() -> anyhow::Result { let runtime_dir = rbw::dirs::runtime_dir(); - std::fs::create_dir_all(&runtime_dir)?; + std::fs::create_dir_all(&runtime_dir) + .context("failed to create runtime dir")?; let path = runtime_dir.join("socket"); - std::fs::remove_file(&path)?; - let sock = tokio::net::UnixListener::bind(&path)?; + // if the socket already doesn't exist, that's fine + let _ = std::fs::remove_file(&path); + let sock = tokio::net::UnixListener::bind(&path) + .context("failed to listen on socket")?; log::debug!("listening on socket {}", path.to_string_lossy()); Ok(sock) } diff --git a/src/bin/rbw/actions.rs b/src/bin/rbw/actions.rs index cca2cf1..0844696 100644 --- a/src/bin/rbw/actions.rs +++ b/src/bin/rbw/actions.rs @@ -1,60 +1,64 @@ -pub fn login() { - simple_action(rbw::agent::Action::Login, "login"); +pub fn login() -> anyhow::Result<()> { + simple_action(rbw::agent::Action::Login, "login") } -pub fn unlock() { - simple_action(rbw::agent::Action::Unlock, "unlock"); +pub fn unlock() -> anyhow::Result<()> { + simple_action(rbw::agent::Action::Unlock, "unlock") } -pub fn sync() { - simple_action(rbw::agent::Action::Sync, "sync"); +pub fn sync() -> anyhow::Result<()> { + simple_action(rbw::agent::Action::Sync, "sync") } -pub fn lock() { - simple_action(rbw::agent::Action::Lock, "lock"); +pub fn lock() -> anyhow::Result<()> { + simple_action(rbw::agent::Action::Lock, "lock") } -pub fn quit() { - let mut sock = crate::sock::Sock::connect(); +pub fn quit() -> anyhow::Result<()> { + let mut sock = crate::sock::Sock::connect()?; sock.send(&rbw::agent::Request { tty: std::env::var("TTY").ok(), action: rbw::agent::Action::Quit, - }); + })?; + Ok(()) } -pub fn decrypt(cipherstring: &str) -> String { - let mut sock = crate::sock::Sock::connect(); +pub fn decrypt(cipherstring: &str) -> anyhow::Result { + let mut sock = crate::sock::Sock::connect()?; sock.send(&rbw::agent::Request { tty: std::env::var("TTY").ok(), action: rbw::agent::Action::Decrypt { cipherstring: cipherstring.to_string(), }, - }); + })?; - let res = sock.recv(); + let res = sock.recv()?; match res { - rbw::agent::Response::Decrypt { plaintext } => plaintext, + rbw::agent::Response::Decrypt { plaintext } => Ok(plaintext), rbw::agent::Response::Error { error } => { - panic!("failed to decrypt: {}", error) + Err(anyhow::anyhow!("failed to decrypt: {}", error)) } - _ => panic!("unexpected message: {:?}", res), + _ => Err(anyhow::anyhow!("unexpected message: {:?}", res)), } } -fn simple_action(action: rbw::agent::Action, desc: &str) { - let mut sock = crate::sock::Sock::connect(); +fn simple_action( + action: rbw::agent::Action, + desc: &str, +) -> anyhow::Result<()> { + let mut sock = crate::sock::Sock::connect()?; sock.send(&rbw::agent::Request { tty: std::env::var("TTY").ok(), action, - }); + })?; - let res = sock.recv(); + let res = sock.recv()?; match res { - rbw::agent::Response::Ack => (), + rbw::agent::Response::Ack => Ok(()), rbw::agent::Response::Error { error } => { - panic!("failed to {}: {}", desc, error) + Err(anyhow::anyhow!("failed to {}: {}", desc, error)) } - _ => panic!("unexpected message: {:?}", res), + _ => Err(anyhow::anyhow!("unexpected message: {:?}", res)), } } diff --git a/src/bin/rbw/commands.rs b/src/bin/rbw/commands.rs index b76658c..83c6c81 100644 --- a/src/bin/rbw/commands.rs +++ b/src/bin/rbw/commands.rs @@ -1,76 +1,108 @@ -pub fn config_show() { - let config = rbw::config::Config::load().unwrap(); - serde_json::to_writer_pretty(std::io::stdout(), &config).unwrap(); +use anyhow::Context as _; + +pub fn config_show() -> anyhow::Result<()> { + let config = + rbw::config::Config::load().context("failed to load config")?; + serde_json::to_writer_pretty(std::io::stdout(), &config) + .context("failed to write config to stdout")?; println!(); + + Ok(()) } -pub fn config_set(key: &str, value: &str) { +pub fn config_set(key: &str, value: &str) -> anyhow::Result<()> { let mut config = rbw::config::Config::load() .unwrap_or_else(|_| rbw::config::Config::new()); match key { "email" => config.email = Some(value.to_string()), "base_url" => config.base_url = Some(value.to_string()), "identity_url" => config.identity_url = Some(value.to_string()), - "lock_timeout" => config.lock_timeout = value.parse().unwrap(), - _ => unimplemented!(), + "lock_timeout" => { + config.lock_timeout = value + .parse() + .context("failed to parse value for lock_timeout")? + } + _ => return Err(anyhow::anyhow!("invalid config key: {}", key)), } - config.save().unwrap(); + config.save().context("failed to save config file")?; + + Ok(()) } -pub fn login() { - ensure_agent(); - crate::actions::login(); +pub fn login() -> anyhow::Result<()> { + ensure_agent()?; + crate::actions::login()?; + + Ok(()) } -pub fn unlock() { - ensure_agent(); - crate::actions::login(); - crate::actions::unlock(); +pub fn unlock() -> anyhow::Result<()> { + ensure_agent()?; + crate::actions::login()?; + crate::actions::unlock()?; + + Ok(()) } -pub fn sync() { - ensure_agent(); - crate::actions::login(); - crate::actions::sync(); +pub fn sync() -> anyhow::Result<()> { + ensure_agent()?; + crate::actions::login()?; + crate::actions::sync()?; + + Ok(()) } -pub fn list() { - unlock(); +pub fn list() -> anyhow::Result<()> { + unlock()?; - let email = config_email(); - let db = rbw::db::Db::load(&email).unwrap_or_else(|_| rbw::db::Db::new()); + let email = config_email()?; + let db = rbw::db::Db::load(&email) + .context("failed to load password database")?; for cipher in db.ciphers { - println!("{}", crate::actions::decrypt(&cipher.name)); + println!( + "{}", + crate::actions::decrypt(&cipher.name) + .context("failed to decrypt entry name")? + ); } + + Ok(()) } -pub fn get(name: &str, user: Option<&str>) { - unlock(); +pub fn get(name: &str, user: Option<&str>) -> anyhow::Result<()> { + unlock()?; - let email = config_email(); - let db = rbw::db::Db::load(&email).unwrap_or_else(|_| rbw::db::Db::new()); + let email = config_email()?; + let db = rbw::db::Db::load(&email) + .context("failed to load password database")?; for cipher in db.ciphers { - let cipher_name = crate::actions::decrypt(&cipher.name); + let cipher_name = crate::actions::decrypt(&cipher.name) + .context("failed to decrypt entry name")?; if name == cipher_name { - let cipher_user = crate::actions::decrypt(&cipher.login.username); + let cipher_user = crate::actions::decrypt(&cipher.login.username) + .context("failed to decrypt entry username")?; if let Some(user) = user { if user == cipher_user { let pass = - crate::actions::decrypt(&cipher.login.password); + crate::actions::decrypt(&cipher.login.password) + .context("failed to decrypt entry password")?; println!("{}", pass); - return; + return Ok(()); } } else { - let pass = crate::actions::decrypt(&cipher.login.password); + let pass = crate::actions::decrypt(&cipher.login.password) + .context("failed to decrypt entry password")?; println!("{}", pass); - return; + return Ok(()); } } } + + Ok(()) } -pub fn add() { - unlock(); +pub fn add() -> anyhow::Result<()> { + unlock()?; todo!() } @@ -80,62 +112,82 @@ pub fn generate( user: Option<&str>, len: usize, ty: rbw::pwgen::Type, -) { +) -> anyhow::Result<()> { let pw = rbw::pwgen::pwgen(ty, len); + // unwrap is safe because pwgen is guaranteed to always return valid utf8 println!("{}", std::str::from_utf8(pw.data()).unwrap()); if name.is_some() && user.is_some() { - unlock(); + unlock()?; todo!(); } + + Ok(()) } -pub fn edit() { - unlock(); +pub fn edit() -> anyhow::Result<()> { + unlock()?; todo!() } -pub fn remove() { - unlock(); +pub fn remove() -> anyhow::Result<()> { + unlock()?; todo!() } -pub fn lock() { - ensure_agent(); - crate::actions::lock(); +pub fn lock() -> anyhow::Result<()> { + ensure_agent()?; + crate::actions::lock()?; + + Ok(()) } -pub fn purge() { - stop_agent(); +pub fn purge() -> anyhow::Result<()> { + stop_agent()?; + + let email = config_email()?; + rbw::db::Db::remove(&email).context("failed to remove database")?; - let email = config_email(); - rbw::db::Db::remove(&email).unwrap(); + Ok(()) } -pub fn stop_agent() { - crate::actions::quit(); +pub fn stop_agent() -> anyhow::Result<()> { + crate::actions::quit()?; + + Ok(()) } -fn ensure_agent() { +fn ensure_agent() -> anyhow::Result<()> { let agent_path = std::env::var("RBW_AGENT"); let agent_path = agent_path .as_ref() .map(|s| s.as_str()) .unwrap_or("rbw-agent"); - let status = std::process::Command::new(agent_path).status().unwrap(); + let status = std::process::Command::new(agent_path) + .status() + .context("failed to run rbw-agent")?; if !status.success() { if let Some(code) = status.code() { if code != 23 { - panic!("failed to run agent: {}", status); + return Err(anyhow::anyhow!( + "failed to run rbw-agent: {}", + status + )); } } } + + Ok(()) } -fn config_email() -> String { - let config = rbw::config::Config::load().unwrap(); - config.email.unwrap() +fn config_email() -> anyhow::Result { + let config = rbw::config::Config::load()?; + if let Some(email) = config.email { + Ok(email) + } else { + Err(anyhow::anyhow!("failed to find email address in config")) + } } diff --git a/src/bin/rbw/main.rs b/src/bin/rbw/main.rs index 065d143..1260edf 100644 --- a/src/bin/rbw/main.rs +++ b/src/bin/rbw/main.rs @@ -1,3 +1,5 @@ +use anyhow::Context as _; + mod actions; mod commands; mod sock; @@ -54,27 +56,34 @@ fn main() { .subcommand(clap::SubCommand::with_name("stop-agent")) .get_matches(); - match matches.subcommand() { + let res = match matches.subcommand() { ("config", Some(smatches)) => match smatches.subcommand() { - ("show", Some(_)) => commands::config_show(), + ("show", Some(_)) => { + commands::config_show().context("config show") + } + // these unwraps are fine because key and value are both marked + // .required(true) ("set", Some(ssmatches)) => commands::config_set( ssmatches.value_of("key").unwrap(), ssmatches.value_of("value").unwrap(), - ), + ) + .context("config set"), _ => { eprintln!("{}", smatches.usage()); std::process::exit(1); } }, - ("login", Some(_)) => commands::login(), - ("unlock", Some(_)) => commands::unlock(), - ("sync", Some(_)) => commands::sync(), - ("list", Some(_)) => commands::list(), + ("login", Some(_)) => commands::login().context("login"), + ("unlock", Some(_)) => commands::unlock().context("unlock"), + ("sync", Some(_)) => commands::sync().context("sync"), + ("list", Some(_)) => commands::list().context("list"), + // this unwrap is safe because name is marked .required(true) ("get", Some(smatches)) => commands::get( smatches.value_of("name").unwrap(), smatches.value_of("user"), - ), - ("add", Some(_)) => commands::add(), + ) + .context("get"), + ("add", Some(_)) => commands::add().context("add"), ("generate", Some(smatches)) => { let ty = if smatches.is_present("no-symbols") { rbw::pwgen::Type::NoSymbols @@ -87,21 +96,35 @@ fn main() { } else { rbw::pwgen::Type::AllChars }; - commands::generate( - smatches.value_of("name"), - smatches.value_of("user"), - smatches.value_of("len").unwrap().parse().unwrap(), - ty, - ); + // this unwrap is fine because len is marked as .required(true) + let len = smatches.value_of("len").unwrap(); + match len.parse() { + Ok(len) => commands::generate( + smatches.value_of("name"), + smatches.value_of("user"), + len, + ty, + ) + .context("generate"), + Err(e) => Err(e.into()), + } + } + ("edit", Some(_)) => commands::edit().context("edit"), + ("remove", Some(_)) => commands::remove().context("remove"), + ("lock", Some(_)) => commands::lock().context("lock"), + ("purge", Some(_)) => commands::purge().context("purge"), + ("stop-agent", Some(_)) => { + commands::stop_agent().context("stop-agent") } - ("edit", Some(_)) => commands::edit(), - ("remove", Some(_)) => commands::remove(), - ("lock", Some(_)) => commands::lock(), - ("purge", Some(_)) => commands::purge(), - ("stop-agent", Some(_)) => commands::stop_agent(), _ => { eprintln!("{}", matches.usage()); std::process::exit(1); } } + .context("rbw"); + + if let Err(e) = res { + eprintln!("{:#}", e); + std::process::exit(1); + } } diff --git a/src/bin/rbw/sock.rs b/src/bin/rbw/sock.rs index 05597e6..4534e5e 100644 --- a/src/bin/rbw/sock.rs +++ b/src/bin/rbw/sock.rs @@ -1,29 +1,38 @@ +use anyhow::Context as _; use std::io::{BufRead as _, Write as _}; pub struct Sock(std::os::unix::net::UnixStream); impl Sock { - pub fn connect() -> Self { - Self( + pub fn connect() -> anyhow::Result { + Ok(Self( std::os::unix::net::UnixStream::connect( rbw::dirs::runtime_dir().join("socket"), ) - .unwrap(), - ) + .context("failed to connect to rbw-agent")?, + )) } - pub fn send(&mut self, msg: &rbw::agent::Request) { + pub fn send(&mut self, msg: &rbw::agent::Request) -> anyhow::Result<()> { let Self(sock) = self; - sock.write_all(serde_json::to_string(msg).unwrap().as_bytes()) - .unwrap(); - sock.write_all(b"\n").unwrap(); + sock.write_all( + serde_json::to_string(msg) + .context("failed to serialize message to agent")? + .as_bytes(), + ) + .context("failed to send message to agent")?; + sock.write_all(b"\n") + .context("failed to send message to agent")?; + Ok(()) } - pub fn recv(&mut self) -> rbw::agent::Response { + pub fn recv(&mut self) -> anyhow::Result { let Self(sock) = self; let mut buf = std::io::BufReader::new(sock); let mut line = String::new(); - buf.read_line(&mut line).unwrap(); - serde_json::from_str(&line).unwrap() + buf.read_line(&mut line) + .context("failed to read message from agent")?; + Ok(serde_json::from_str(&line) + .context("failed to parse message from agent")?) } } diff --git a/src/config.rs b/src/config.rs index 0e79893..55f6806 100644 --- a/src/config.rs +++ b/src/config.rs @@ -47,6 +47,8 @@ impl Config { pub fn save(&self) -> Result<()> { let filename = Self::filename(); + // unwrap is safe here because Self::filename is explicitly + // constructed as a filename in a directory std::fs::create_dir_all(filename.parent().unwrap()) .context(crate::error::SaveConfig)?; let mut fh = std::fs::File::create(filename) diff --git a/src/db.rs b/src/db.rs index 7e94d73..a750693 100644 --- a/src/db.rs +++ b/src/db.rs @@ -45,6 +45,8 @@ impl Db { // XXX need to make this atomic pub fn save(&self, email: &str) -> Result<()> { let filename = Self::filename(email); + // unwrap is safe here because Self::filename is explicitly + // constructed as a filename in a directory std::fs::create_dir_all(filename.parent().unwrap()) .context(crate::error::SaveDb)?; let mut fh = @@ -61,6 +63,8 @@ impl Db { // XXX need to make this atomic pub async fn save_async(&self, email: &str) -> Result<()> { let filename = Self::filename(email); + // unwrap is safe here because Self::filename is explicitly + // constructed as a filename in a directory tokio::fs::create_dir_all(filename.parent().unwrap()) .await .context(crate::error::SaveDbAsync)?; diff --git a/src/locked.rs b/src/locked.rs index bca30c3..4f55f20 100644 --- a/src/locked.rs +++ b/src/locked.rs @@ -8,6 +8,8 @@ pub struct Vec { impl Default for Vec { fn default() -> Self { let data = Box::new(arrayvec::ArrayVec::<_>::new()); + // XXX it'd be nice to handle this better than .unwrap(), but it'd be + // a lot of effort let lock = region::lock(data.as_ptr(), data.capacity()).unwrap(); Self { data, _lock: lock } } diff --git a/src/pinentry.rs b/src/pinentry.rs index 4a2196e..769b8d0 100644 --- a/src/pinentry.rs +++ b/src/pinentry.rs @@ -17,6 +17,8 @@ pub async fn getpin( opts }; let mut child = opts.spawn().context(crate::error::Spawn)?; + // unwrap is safe because we specified stdin as piped in the command opts + // above let mut stdin = child.stdin.take().unwrap(); stdin @@ -39,6 +41,8 @@ pub async fn getpin( let mut buf = crate::locked::Vec::new(); buf.extend(std::iter::repeat(0)); + // unwrap is safe because we specified stdout as piped in the command opts + // above let len = read_password(buf.data_mut(), child.stdout.as_mut().unwrap()).await?; buf.truncate(len); -- cgit v1.2.3-54-g00ecf