From 6a22429632195846029c8f85b4e23b03cfe54388 Mon Sep 17 00:00:00 2001 From: Armin Friedl Date: Sat, 8 Feb 2020 09:56:41 +0100 Subject: [PATCH 1/9] Remove From for Certificate Deserialization from a path is a heavy conversion. This should be done by an explicit `Certificate::new_from_cbor` call. --- coffer-common/src/certificate.rs | 7 ------- coffer-common/src/keyring.rs | 2 +- 2 files changed, 1 insertion(+), 8 deletions(-) diff --git a/coffer-common/src/certificate.rs b/coffer-common/src/certificate.rs index 893d653..54e44a7 100644 --- a/coffer-common/src/certificate.rs +++ b/coffer-common/src/certificate.rs @@ -106,10 +106,3 @@ impl Certificate { Ok(sealedbox::seal(message, pk)) } } - -impl > From for Certificate { - fn from(path: T) -> Self { - Certificate::new_from_cbor(&path) - .expect(&format!{"Could not read certificate from {}", path.as_ref().display()}) - } -} diff --git a/coffer-common/src/keyring.rs b/coffer-common/src/keyring.rs index 7179b15..ec00547 100644 --- a/coffer-common/src/keyring.rs +++ b/coffer-common/src/keyring.rs @@ -53,7 +53,7 @@ impl Keyring { where T: AsRef { Keyring { - certificate: Certificate::from(certificate_path), + certificate: Certificate::new_from_cbor(certificate_path).unwrap(), known_keys: HashMap::new() } } From 7515462433eb669e5f1f24a43394f4564daf11e0 Mon Sep 17 00:00:00 2001 From: Armin Friedl Date: Sat, 8 Feb 2020 10:27:15 +0100 Subject: [PATCH 2/9] Cleaning up clippy warning --- coffer-client/src/main.rs | 2 +- coffer-common/src/keyring.rs | 2 +- coffer-companion/src/certificate.rs | 2 +- coffer-server/src/coffer_map.rs | 8 +++++--- coffer-server/src/server.rs | 2 +- 5 files changed, 9 insertions(+), 7 deletions(-) diff --git a/coffer-client/src/main.rs b/coffer-client/src/main.rs index 6d215bd..cf70e73 100644 --- a/coffer-client/src/main.rs +++ b/coffer-client/src/main.rs @@ -5,7 +5,7 @@ use env_logger; use structopt::StructOpt; use std:: { - net::{SocketAddr, TcpStream}, + net::TcpStream, error::Error, path::PathBuf, io::{Write, Read}, diff --git a/coffer-common/src/keyring.rs b/coffer-common/src/keyring.rs index ec00547..87e50dd 100644 --- a/coffer-common/src/keyring.rs +++ b/coffer-common/src/keyring.rs @@ -44,7 +44,7 @@ pub struct Keyring { impl Keyring { pub fn new(certificate: Certificate) -> Keyring { Keyring { - certificate: certificate, + certificate, known_keys: HashMap::new() } } diff --git a/coffer-companion/src/certificate.rs b/coffer-companion/src/certificate.rs index 32f4826..f9a28ce 100644 --- a/coffer-companion/src/certificate.rs +++ b/coffer-companion/src/certificate.rs @@ -10,7 +10,7 @@ pub fn generate_key(out: PathBuf) { let cert = certificate.to_cbor().unwrap(); let mut writer = File::create(&out) - .expect(&format!{"Could not create out file {}", &out.display()}); + .unwrap_or_else(|_| panic!{"Could not create out file {}", &out.display()}); writer.write_all(&cert).unwrap(); } diff --git a/coffer-server/src/coffer_map.rs b/coffer-server/src/coffer_map.rs index 46acd8a..3941c71 100644 --- a/coffer-server/src/coffer_map.rs +++ b/coffer-server/src/coffer_map.rs @@ -7,7 +7,7 @@ use std::sync::RwLock; use std::sync::RwLockReadGuard; use std::sync::RwLockWriteGuard; -use std::collections::HashMap; +use std::collections::hash_map::{HashMap, Entry}; use coffer_common::coffer::*; @@ -34,8 +34,10 @@ impl Coffer for CofferMap { match lock.get_mut(&key.shard) { Some(shard) => { - if shard.contains_key(&key.key) { Err(CofferError::Msg("Key exists")) } - else { shard.insert(key.key, value); Ok(()) } + match shard.entry(key.key) { + Entry::Occupied(_) => Err(CofferError::Msg("Key exists")), + Entry::Vacant(v) => { v.insert(value); Ok(()) } + } } None => { lock.insert(key.shard.clone(), HashMap::new()); diff --git a/coffer-server/src/server.rs b/coffer-server/src/server.rs index 3139bcb..74dd5ef 100644 --- a/coffer-server/src/server.rs +++ b/coffer-server/src/server.rs @@ -59,7 +59,7 @@ where C: Coffer + Send + Sync + 'static debug!{"Binding to socket {:?}", socket} let mut listener = TcpListener::bind(socket).await - .expect(format!{"Could not bind to socket {}", socket}.as_str()); + .expect("Could not bind to socket"); let server = async move { let mut incoming = listener.incoming(); From e7fbb4e47cfa86e7fc6517baac308d5e7109b1ab Mon Sep 17 00:00:00 2001 From: Armin Friedl Date: Sat, 8 Feb 2020 22:12:04 +0100 Subject: [PATCH 3/9] [client] Unused dependencies, certificate documentation and minor improvements --- Cargo.lock | 2 - coffer-client/Cargo.toml | 2 - coffer-client/src/main.rs | 18 +++++-- coffer-common/src/certificate.rs | 89 ++++++++++++++++++++++---------- 4 files changed, 76 insertions(+), 35 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index a0befe1..6caef01 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -88,9 +88,7 @@ dependencies = [ "exec 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "futures 0.3.1 (registry+https://github.com/rust-lang/crates.io-index)", "log 0.4.8 (registry+https://github.com/rust-lang/crates.io-index)", - "serde 1.0.102 (registry+https://github.com/rust-lang/crates.io-index)", "serde_cbor 0.10.2 (registry+https://github.com/rust-lang/crates.io-index)", - "serde_yaml 0.8.11 (registry+https://github.com/rust-lang/crates.io-index)", "structopt 0.3.4 (registry+https://github.com/rust-lang/crates.io-index)", ] diff --git a/coffer-client/Cargo.toml b/coffer-client/Cargo.toml index a4989f3..e0c56d1 100644 --- a/coffer-client/Cargo.toml +++ b/coffer-client/Cargo.toml @@ -12,8 +12,6 @@ log = "0.4" env_logger="0.7" structopt = "0.3" # Communication -serde = { version = "1.0", features = ["derive"]} -serde_yaml = "0.8" serde_cbor = "0.10.2" # Executing subcommand exec = "0.3.1" diff --git a/coffer-client/src/main.rs b/coffer-client/src/main.rs index cf70e73..ff8b937 100644 --- a/coffer-client/src/main.rs +++ b/coffer-client/src/main.rs @@ -1,9 +1,12 @@ +//! # Coffer client +//! +//! Retrieve a secret shard from a `coffer-server`. Secrets in the shard are set +//! as environment variables for the spawned subcommand `cmd`. + #[allow(unused_imports)] use log::{debug, error, info, trace, warn}; use env_logger; -use structopt::StructOpt; - use std:: { net::TcpStream, error::Error, @@ -12,9 +15,14 @@ use std:: { convert::{TryInto, TryFrom} }; -use coffer_common::certificate::Certificate; -use coffer_common::coffer::{CofferShard, CofferValue}; +use coffer_common::{ + coffer::{CofferShard, CofferValue}, + certificate::Certificate +}; +use structopt::StructOpt; + +/// Client for setting up the environment from coffer server secrets #[derive(StructOpt, Debug)] struct Args { /// Address of the coffer server @@ -75,6 +83,8 @@ fn main() -> Result<(), Box> { Err("Could not spawn sub-command".into()) } +/// Replaces the `coffer-client` process image with +/// the subcommand `cmd` with `args` fn reap_coffer(cmd: &str, args: &[String]) { let mut cmd = exec::Command::new(cmd); diff --git a/coffer-common/src/certificate.rs b/coffer-common/src/certificate.rs index 54e44a7..9431184 100644 --- a/coffer-common/src/certificate.rs +++ b/coffer-common/src/certificate.rs @@ -1,12 +1,20 @@ -//! Common certificate handling and encryption +//! A keypair contianer providing functionality for signing, encryption and +//! decryption +//! +//! # Base libraries +//! The cryptographic operations exposed by this module are based on the +//! [NaCl](http://nacl.cr.yp.to/) fork [libsodium](https://libsodium.org) as +//! exposed by the rust bindings [sodiumoxide](https://crates.io/crates/sodiumoxide). #[allow(unused_imports)] use log::{debug, error, info, trace, warn}; -use std::path::Path; -use std::io::BufReader; -use std::fs::File; -use std::fmt::{Debug, Formatter}; +use std::{ + path::Path, + io::BufReader, + fs::File, + ops::Deref +}; use quick_error::quick_error; @@ -25,22 +33,51 @@ quick_error! { Io(err: std::io::Error) { from() } - SecKey + SecKey { + from(CertificateInner) + } Crypto } } -/// A secure container for certificates +/// A secure container for a keypair /// -/// # Certificate +/// Secure means a best effort approach to: +/// - Prevent swapping memory to disk +/// - Zeroing out memory upon dropping +/// - Prevent other processes and buffer overflows to access the secure memory +/// area /// -/// A certificate consists of a public and a private key in a secure memory -/// area. With a certificate data sealed and opened. +/// These guarantees are currently *not* reliable. If you threat model contains +/// targeted attacks against coffer memory, additional precautions have to be +/// taken. pub struct Certificate { inner: SecKey } +// The SecKeyReadGuard prevents convenience methods for handing out references +// to private/public keys (reference outlives SecKeyReadGuard). Hence below +// macros are shortcut projections that can be used after a read guard is +// created + +// Get the public key +macro_rules! pk { + ($cert:ident) => { + &$cert.inner.read().public_key + }; +} + +// Get the private key +macro_rules! sk { + ($cert:ident) => { + &$cert.inner.read().private_key + }; +} + +// Certificate and its inner SecKey own their +// raw pointer without any thread local behaviour unsafe impl Send for Certificate {} +// After initialization, certificate is read-only unsafe impl Sync for Certificate {} #[derive(Serialize, Deserialize)] @@ -49,13 +86,8 @@ struct CertificateInner { private_key: box_::SecretKey } -impl Debug for CertificateInner { - fn fmt(&self, fmt: &mut Formatter<'_>) -> std::fmt::Result { - write!(fmt, "