Commit 0723ce95 authored by SamwiseFilmore's avatar SamwiseFilmore
Browse files

Use error-chain for error handling

There are still a couple of pain points here (EntryReader, for example),
but the general layout works pretty well. With error chain and one or
two handwritten impls I can pass a bare &Path to chain_err and it will
construct the correct error variant and add it to the chain. This is a
significant reduction in boilerplate.

I still need to do some manual testing to generate some error
conditions, but I think this setup will be more robust in the long term.
parent d8de5f9f
......@@ -12,13 +12,14 @@ edition = "2018"
[dependencies]
clap = "2.33.3"
dirs = "3.0.1"
error-chain = "0.12"
hex = { version = "0.4.2", features = ["serde"] }
lazy_static = "1.4.0"
seckey = "0.11.2"
serde = { version = "1.0.115", default_features = false, features = ["derive"] }
sodiumoxide = { version = "0.2.6", default_features = false }
termion = "1.5.5"
thiserror = "1.0.20"
#thiserror = "1.0.20"
toml = "0.5.6"
user-error = "1.2.8"
use std::io;
use std::path::PathBuf;
use error_chain::error_chain;
use user_error::UFE;
use thiserror::Error;
//use thiserror::Error;
error_chain! {
types {
Error, ErrorKind, ResultExt;
}
foreign_links {
Io(io::Error);
Ser(toml::ser::Error);
Deser(toml::de::Error);
}
errors {
KeyInvalid {
description("Key length invalid"),
}
KeyMismatch {
description("Public and secret keys do not match"),
}
NonceInvalid {
description("Invalid nonce length"),
}
PassphraseIncorrect {
description("Incorrect passphrase"),
}
PassphraseMismatch {
description("Passphrases did not match"),
}
Path(path: PathBuf) {
display("{}: ", path.display()),
}
}
skip_msg_variant
}
impl UFE for Error {}
/*
/// An error which includes path context and implements `UFE` for easy display.
#[derive(Debug, Error)]
#[error("File: {path}")]
......@@ -13,8 +57,6 @@ pub struct Error {
pub path: PathBuf,
}
impl UFE for Error {}
/// The main error type that is used by this library internally. For additional
/// contextual information, most public routines use [`Error`](struct.Error.html).
#[non_exhaustive]
......@@ -44,4 +86,5 @@ pub enum ErrorKind {
#[error("Deserialization")]
Deser(#[from] toml::de::Error),
}
*/
......@@ -6,6 +6,7 @@ use std::ops::Deref;
use std::os::unix::fs::OpenOptionsExt;
use std::path::{Path, PathBuf};
use error_chain::bail;
use hex::FromHex;
use lazy_static::lazy_static;
use seckey::SecBytes;
......@@ -17,7 +18,7 @@ use sodiumoxide::crypto::{
};
use termion::input::TermRead;
pub use error::{ErrorKind, Error};
pub use crate::error::{ErrorKind, Error, ResultExt};
lazy_static! {
static ref HOMEDIR: PathBuf = {
......@@ -84,20 +85,14 @@ impl PublicKeyFile {
/// Parse a `PublicKeyFile` from `file` (in toml format).
pub fn open(file: impl AsRef<Path>) -> Result<PublicKeyFile, Error> {
let content = fs::read_to_string(&file)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::from(src),
})?;
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?;
toml::from_str(&content)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::from(src),
})
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )
}
/// Write `self` serialized as toml to `w`.
pub fn write(&self, mut w: impl Write) -> Result<(), ErrorKind> {
pub fn write(&self, mut w: impl Write) -> Result<(), Error> {
w.write_all(toml::to_string(self)?.as_bytes())?;
Ok(())
}
......@@ -106,14 +101,8 @@ impl PublicKeyFile {
pub fn save(&self, file: impl AsRef<Path>) -> Result<(), Error> {
self.write(
File::create(&file)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::from(src)
})?
).map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src,
})
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?
).chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )
}
}
......@@ -133,7 +122,7 @@ impl SKey {
}
}
fn decrypt(&mut self, passwd: Passwd, salt: pwhash::Salt, nonce: secretbox::Nonce) -> Result<(), ErrorKind> {
fn decrypt(&mut self, passwd: Passwd, salt: pwhash::Salt, nonce: secretbox::Nonce) -> Result<(), Error> {
if let SKey::Cipher(ciphertext) = self {
if let Some(passwd_key) = passwd.gen_key(salt) {
let skey_plain = secretbox::open(ciphertext.as_ref(), &nonce, &passwd_key)
......@@ -217,20 +206,14 @@ impl SecretKeyFile {
/// Parse a `SecretKeyFile` from `file` (in toml format).
pub fn open(file: impl AsRef<Path>) -> Result<SecretKeyFile, Error> {
let content = fs::read_to_string(&file)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::Io(src),
})?;
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?;
toml::from_str(&content)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::Deser(src),
})
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )
}
/// Write `self` serialized as toml to `w`.
pub fn write(&self, mut w: impl Write) -> Result<(), ErrorKind> {
pub fn write(&self, mut w: impl Write) -> Result<(), Error> {
w.write_all(toml::to_string(&self)?.as_bytes())?;
Ok(())
}
......@@ -246,14 +229,8 @@ impl SecretKeyFile {
.create(true)
.mode(0o600)
.open(&file)
.map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src: ErrorKind::from(src),
})?
).map_err(|src| Error {
path: file.as_ref().to_path_buf(),
src,
})
.chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )?
).chain_err(|| ErrorKind::Path(file.as_ref().to_path_buf()) )
}
/// Ensure that the internal state of this struct is encrypted.
......@@ -264,7 +241,7 @@ impl SecretKeyFile {
/// Ensure that the internal state of this struct is decrypted.
/// If the internal state is already decrypted, this function is a no-op.
pub fn decrypt(&mut self, passwd: Passwd) -> Result<(), ErrorKind> {
pub fn decrypt(&mut self, passwd: Passwd) -> Result<(), Error> {
self.skey.decrypt(passwd, self.salt, self.nonce)
}
......@@ -313,7 +290,7 @@ impl Passwd {
}
/// Prompt the user for a `Passwd` on stdin.
pub fn prompt(prompt: impl AsRef<str>) -> Result<Passwd, ErrorKind> {
pub fn prompt(prompt: impl AsRef<str>) -> Result<Passwd, Error> {
let stdout = stdout();
let mut stdout = stdout.lock();
let stdin = stdin();
......@@ -336,17 +313,16 @@ impl Passwd {
/// Prompt for a password on stdin and confirm it. For configurable
/// prompts, use [`Passwd::prompt`](struct.Passwd.html#method.prompt).
pub fn prompt_new() -> Result<Passwd, ErrorKind> {
pub fn prompt_new() -> Result<Passwd, Error> {
let passwd = Passwd::prompt(
"Please enter a new passphrase (leave empty to store the key in plaintext): "
)?;
let confirm = Passwd::prompt("Please re-enter the passphrase: ")?;
if passwd != confirm {
Err(ErrorKind::PassphraseMismatch)
} else {
Ok(passwd)
bail!(ErrorKind::PassphraseMismatch);
}
Ok(passwd)
}
/// Get a key for symmetric key encryption from a password.
......@@ -382,10 +358,7 @@ impl Eq for Passwd {}
/// directories will not be created.
pub fn gen_keypair(pkey_path: &Path, skey_path: &Path) -> Result<(PublicKeyFile, SecretKeyFile), Error> {
let passwd = Passwd::prompt_new()
.map_err(|src| Error {
path: skey_path.to_path_buf(),
src,
})?;
.chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?;
let (pkey_file, mut skey_file) = SecretKeyFile::new();
......@@ -399,18 +372,13 @@ pub fn gen_keypair(pkey_path: &Path, skey_path: &Path) -> Result<(PublicKeyFile,
}
fn prompt_skey(skey_path: &Path, prompt: impl AsRef<str>) -> Result<SecretKeyFile, Error> {
let to_file_err = |src| Error {
path: skey_path.to_path_buf(),
src,
};
let mut key_file = SecretKeyFile::open(skey_path)?;
if key_file.is_encrypted() {
let passwd = Passwd::prompt(&format!("{} {}: ", prompt.as_ref(), skey_path.display()))
.map_err(to_file_err)?;
.chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?;
key_file.decrypt(passwd)
.map_err(to_file_err)?;
.chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?;
}
Ok(key_file)
}
......@@ -427,10 +395,7 @@ pub fn re_encrypt(skey_path: &Path) -> Result<(), Error> {
let mut skey_file = prompt_skey(skey_path, "Old passphrase for")?;
let passwd = Passwd::prompt_new()
.map_err(|src| Error {
path: skey_path.to_path_buf(),
src,
})?;
.chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) )?;
skey_file.encrypt(passwd);
skey_file.save(skey_path)
......
......@@ -13,6 +13,7 @@ use pkgar_keys::{
ErrorKind,
gen_keypair,
get_skey,
ResultExt,
SecretKeyFile,
re_encrypt
};
......@@ -53,18 +54,15 @@ fn cli() -> Result<i32, Error> {
"gen" => {
if let Some(keydir) = skey_path.parent() {
fs::create_dir_all(&keydir)
.map_err(|err| Error {
path: keydir.to_path_buf(),
src: ErrorKind::Io(err),
})?;
.chain_err(|| ErrorKind::Path(keydir.to_path_buf()) )?;
}
if ! submatches.is_present("force") {
if skey_path.exists() {
return Err(Error {
path: skey_path,
src: ErrorKind::Io(io::Error::from(io::ErrorKind::AlreadyExists)),
});
return Err(Error::from_kind(ErrorKind::Io(
io::Error::from(io::ErrorKind::AlreadyExists)
)))
.chain_err(|| ErrorKind::Path(skey_path.to_path_buf()) );
}
}
......@@ -89,10 +87,7 @@ fn cli() -> Result<i32, Error> {
pkey.save(file)?;
} else {
pkey.write(io::stdout().lock())
.map_err(|src| Error {
path: PathBuf::from("stdout"),
src,
})?;
.chain_err(|| ErrorKind::Path(PathBuf::from("stdout")) )?;
}
},
"rencrypt" => {
......
......@@ -8,11 +8,12 @@ repository = "https://gitlab.redox-os.org/redox-os/pkgar"
edition = "2018"
[dependencies]
error-chain = "0.12"
plain = "0.2.3"
pkgar-core = { path = "../pkgar-core" }
pkgar-keys = { path = "../pkgar-keys" }
sodiumoxide = { version = "0.2.6", default_features = false }
thiserror = "1.0.20"
#thiserror = "1.0.20"
user-error = "1.2.8"
[dependencies.clap]
......
......@@ -8,7 +8,7 @@ use pkgar_core::{Entry, Header, Mode, PackageSrc};
use pkgar_keys::PublicKeyFile;
use sodiumoxide::crypto::sign;
use crate::{Error, ErrorKind, READ_WRITE_HASH_BUF_SIZE};
use crate::{Error, ErrorKind, READ_WRITE_HASH_BUF_SIZE, ResultExt};
use crate::ext::{copy_and_hash, EntryExt};
use crate::package::PackageFile;
use crate::transaction::Transaction;
......@@ -94,15 +94,13 @@ pub fn create(
.create(true)
.truncate(true)
.open(&archive_path)
.map_err(|e| Error::from(e).path(&archive_path) )?;
.chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?;
// Create a list of entries
let mut entries = Vec::new();
folder_entries(&folder, &folder, &mut entries)
.map_err(|e| Error::from(e)
.reason("Recursing buildroot")
.path(&folder)
)?;
.chain_err(|| ErrorKind::Path(folder.as_ref().to_path_buf()) )
.chain_err(|| "Recursing buildroot" )?;
// Create initial header
let mut header = Header {
......@@ -120,14 +118,13 @@ pub fn create(
entry.offset = data_size;
data_size = data_size.checked_add(entry.size)
.ok_or(pkgar_core::Error::Overflow)?;
//.chain_err(|| ErrorKind::Entry(*entry) )?;
}
let data_offset = header.total_size()?;
archive_file.seek(SeekFrom::Start(data_offset as u64))
.map_err(|e| Error::from(e)
.reason(format!("Seek to {} (data offset)", data_offset))
.path(&archive_path)
)?;
.chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )
.chain_err(|| format!("Seek to {} (data offset)", data_offset) )?;
//TODO: fallocate data_offset + data_size
......@@ -139,47 +136,37 @@ pub fn create(
let path = folder.as_ref().join(relative);
let mode = entry.mode()
.map_err(|e| Error::from(e)
.entry(*entry)
)?;
.map_err(Error::from)
.chain_err(|| ErrorKind::Entry(*entry) )?;
let (total, hash) = match mode.kind() {
Mode::FILE => {
let mut entry_file = fs::OpenOptions::new()
.read(true)
.open(&path)
.map_err(|e| Error::from(e).path(&path) )?;
.chain_err(|| ErrorKind::Path(path.to_path_buf()) )?;
copy_and_hash(&mut entry_file, &mut archive_file, &mut buf)
.map_err(|e| Error::from(e)
.reason(format!("Writing entry to archive: '{}'", relative.display()))
.path(&path)
)?
.chain_err(|| ErrorKind::Path(path.to_path_buf()) )
.chain_err(|| format!("Writing entry to archive: '{}'", relative.display()) )?
},
Mode::SYMLINK => {
let destination = fs::read_link(&path)
.map_err(|e| Error::from(e).path(&path) )?;
.chain_err(|| ErrorKind::Path(path.to_path_buf()) )?;
let mut data = destination.as_os_str().as_bytes();
copy_and_hash(&mut data, &mut archive_file, &mut buf)
.map_err(|e| Error::from(e)
.reason(format!("Writing entry to archive: '{}'", relative.display()))
.path(&path)
)?
.chain_err(|| ErrorKind::Path(path.to_path_buf()) )
.chain_err(|| format!("Writing entry to archive: '{}'", relative.display()) )?
},
_ => return Err(Error::from(
pkgar_core::Error::InvalidMode(mode.bits())
)
.entry(*entry)),
))
.chain_err(|| ErrorKind::Entry(*entry) ),
};
if total != entry.size() {
return Err(ErrorKind::LengthMismatch {
actual: total,
expected: entry.size(),
}
.as_error()
.entry(*entry)
);
return Err(Error::from_kind(ErrorKind::LengthMismatch(total, entry.size())))
.chain_err(|| ErrorKind::Entry(*entry) );
}
entry.blake3.copy_from_slice(hash.as_bytes());
......@@ -195,12 +182,12 @@ pub fn create(
// Write archive header
archive_file.seek(SeekFrom::Start(0))
.map_err(|e| Error::from(e).path(&archive_path) )?;
.chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?;
archive_file.write_all(unsafe {
plain::as_bytes(&header)
})
.map_err(|e| Error::from(e).path(&archive_path) )?;
.chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )?;
// Write each entry header
for entry in &entries {
......@@ -208,10 +195,8 @@ pub fn create(
archive_file.write_all(unsafe {
plain::as_bytes(entry)
})
.map_err(|e| Error::from(e)
.reason(format!("Write entry {}", checked_path.display()))
.path(&archive_path)
)?;
.chain_err(|| ErrorKind::Path(archive_path.as_ref().to_path_buf()) )
.chain_err(|| format!("Write entry {}", checked_path.display()) )?;
}
Ok(())
......@@ -277,7 +262,7 @@ pub fn verify(
.join(entry.check_path()?);
let expected = File::open(&expected_path)
.map_err(|e| Error::from(e).path(expected_path) )?;
.chain_err(|| ErrorKind::Path(expected_path.to_path_buf()) )?;
let (count, hash) = copy_and_hash(expected, io::sink(), &mut buf)?;
......
......@@ -7,7 +7,7 @@ use std::path::{Component, Path};
use blake3::{Hash, Hasher};
use pkgar_core::{Entry, PackageSrc};
use crate::{Error, ErrorKind};
use crate::{Error, ErrorKind, ResultExt};
/// Handy associated functions for `pkgar_core::Entry` that depend on std
pub trait EntryExt {
......@@ -26,10 +26,10 @@ impl EntryExt for Entry {
Component::Normal(_) => {},
invalid => {
let bad_component: &Path = invalid.as_ref();
return Err(ErrorKind::InvalidPathComponent(bad_component.to_path_buf())
.as_error()
.entry(*self)
);
return Err(Error::from_kind(
ErrorKind::InvalidPathComponent(bad_component.to_path_buf())
))
.chain_err(|| ErrorKind::Entry(*self) );
},
}
}
......@@ -38,13 +38,8 @@ impl EntryExt for Entry {
fn verify(&self, blake3: Hash, size: u64) -> Result<(), Error> {
if size != self.size() {
Err(ErrorKind::LengthMismatch {
actual: size,
expected: self.size(),
}
.as_error()
.entry(*self)
)
Err(Error::from_kind(ErrorKind::LengthMismatch(size, self.size())))
.chain_err(|| ErrorKind::Entry(*self) )
} else if blake3 != self.blake3() {
Err(pkgar_core::Error::InvalidBlake3.into())
} else {
......@@ -84,12 +79,14 @@ pub struct EntryReader<'a, Src>
impl<Src, E> Read for EntryReader<'_, Src>
where
Src: PackageSrc<Err = E>,
E: From<pkgar_core::Error> + std::error::Error + Send + Sync + 'static,
E: From<pkgar_core::Error> + std::error::Error,
{
fn read(&mut self, buf: &mut [u8]) -> io::Result<usize> {
let count = self.src.read_entry(self.entry, self.pos, buf)
// This is a little painful, since e is pkgar::Error...
.map_err(|e| io::Error::new(io::ErrorKind::Other, e) )?;
.map_err(|err|
io::Error::new(io::ErrorKind::Other, err.to_string())
)?;
self.pos += count;
Ok(count)
}
......
......@@ -10,12 +10,91 @@ pub use transaction::*;
use std::io;
use std::path::{Path, PathBuf};
use error_chain::error_chain;
use pkgar_core::{Entry, Mode};
use thiserror::Error;
//use thiserror::Error;
use user_error::UFE;
const READ_WRITE_HASH_BUF_SIZE: usize = 4 * 1024 * 1024;
error_chain! {
types {
Error, ErrorKind, ResultExt;
}
links {
Keys(pkgar_keys::Error, pkgar_keys::ErrorKind);
}
foreign_links {
Io(io::Error);
}
errors {
Core(src: pkgar_core::Error) {
display("{}", src),
}
FailedCommit(changed: usize, remaining: usize) {
display(
"Failed to commit transaction. {} files changed, {} files remaining",
changed,
remaining,
),
}
InvalidPathComponent(path: PathBuf) {
display("Invalid path component: {}", path.display()),
}
LengthMismatch(actual: u64, expected: u64) {
display("Entry size mismatch: expected {}, got {}", expected, actual),
}
InvalidModeKind(mode: Mode) {
display("Invalid Mode Kind: {:#o}", mode),
}
Path(path: PathBuf) {
display("Path: {}", path.display()),
}
Entry(entry: Entry) {
display("Entry: {:?}", entry),
}
}
}
impl UFE for Error {}
// Unfortunately error_chain does not handle types that don't implement
// std::error::Error very well.
impl From<pkgar_core::Error> for Error {
fn from(err: pkgar_core::Error) -> Error {
Error::from_kind(ErrorKind::Core(err))
}
}
impl From<&Path> for ErrorKind {
fn from(path: &Path) -> ErrorKind {
ErrorKind::Path(path.to_path_buf())
}
}
/*
// Apparently this conflicts with the first implementation; Just use map_err
// with Error::from for chaining errors to a pkgar_core Result.
impl<T> ResultExt<T> for Result<T, pkgar_core::Error> {
fn chain_err<F, EK>(self, callback: F) -> Result<T, Error>
where F: FnOnce() -> EK,
EK: Into<ErrorKind>,
{
self.map_err(|e|
Error::with_boxed_chain(Box::new(e), callback().into())