Make key material secret

Cargo.lock

@@ -953,7 +953,6 @@ dependencies = [
  "clap",
  "omegaupload-common",
  "reqwest",
- "secrecy",
 ]
 
 [[package]]
@@ -970,6 +969,7 @@ dependencies = [
  "http",
  "lazy_static",
  "rand",
+ "secrecy",
  "serde",
  "thiserror",
  "typenum",
@@ -1368,7 +1368,6 @@ version = "0.8.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "9bd1c54ea06cfd2f6b63219704de0b9b4f72dcc2b8fdef820be6cd799780e91e"
 dependencies = [
- "serde",
  "zeroize",
 ]
 

cli/Cargo.toml

@@ -12,4 +12,3 @@ anyhow = "1"
 atty = "0.2"
 clap = "3.0.0-beta.4"
 reqwest = { version = "0.11", default-features = false, features = ["rustls-tls", "blocking"] }
-secrecy = { version = "0.8", features = ["serde"] }

cli/src/main.rs

@@ -2,18 +2,19 @@
 #![deny(unsafe_code)]
 
 use std::io::{Read, Write};
+use std::path::PathBuf;
 
 use anyhow::{anyhow, bail, Context, Result};
 use atty::Stream;
 use clap::Parser;
 use omegaupload_common::crypto::{open_in_place, seal_in_place};
+use omegaupload_common::secrecy::{ExposeSecret, SecretVec};
 use omegaupload_common::{
     base64, Expiration, ParsedUrl, Url, API_ENDPOINT, EXPIRATION_HEADER_NAME,
 };
 use reqwest::blocking::Client;
 use reqwest::header::EXPIRES;
 use reqwest::StatusCode;
-use secrecy::{ExposeSecret, SecretString};
 
 #[derive(Parser)]
 struct Opts {
@@ -29,9 +30,10 @@ enum Action {
         /// Encrypt the uploaded paste with the provided password, preventing
         /// public access.
         #[clap(short, long)]
-        password: Option<SecretString>,
+        password: bool,
         #[clap(short, long)]
         duration: Option<Expiration>,
+        path: PathBuf,
     },
     Download {
         /// The paste to download.
@@ -47,7 +49,8 @@ fn main() -> Result<()> {
             url,
             password,
             duration,
-        } => handle_upload(url, password, duration),
+            path,
+        } => handle_upload(url, password, duration, path),
         Action::Download { url } => handle_download(url),
     }?;
 
@@ -56,8 +59,9 @@ fn main() -> Result<()> {
 
 fn handle_upload(
     mut url: Url,
-    password: Option<SecretString>,
+    password: bool,
     duration: Option<Expiration>,
+    path: PathBuf,
 ) -> Result<()> {
     url.set_fragment(None);
 
@@ -66,11 +70,17 @@ fn handle_upload(
     }
 
     let (data, key) = {
-        let mut container = Vec::new();
+        let mut container = std::fs::read(path)?;
-        std::io::stdin().read_to_end(&mut container)?;
+        let password = if password {
-        let password = password.as_ref().map(|v| v.expose_secret().as_ref());
+            let mut buffer = vec![];
+            std::io::stdin().read_to_end(&mut buffer)?;
+            Some(SecretVec::new(buffer))
+        } else {
+            None
+        };
+
         let enc_key = seal_in_place(&mut container, password)?;
-        let key = base64::encode(&enc_key);
+        let key = base64::encode(&enc_key.expose_secret().as_ref());
         (container, key)
     };
 
@@ -90,11 +100,11 @@ fn handle_upload(
         .map_err(|_| anyhow!("Failed to get base URL"))?
         .extend(std::iter::once(res.text()?));
 
-    let mut fragment = format!("key:{}", key);
+    let fragment = if password {
-
+        format!("key:{}!pw", key)
-    if password.is_some() {
+    } else {
-        fragment.push_str("!pw");
+        key
-    }
+    };
 
     url.set_fragment(Some(&fragment));
 
@@ -140,7 +150,11 @@ fn handle_download(mut url: ParsedUrl) -> Result<()> {
         password = Some(input);
     }
 
-    open_in_place(&mut data, &url.decryption_key, password.as_deref())?;
+    open_in_place(
+        &mut data,
+        &url.decryption_key,
+        password.map(|v| SecretVec::new(v.into_bytes())),
+    )?;
 
     if atty::is(Stream::Stdout) {
         if let Ok(data) = String::from_utf8(data) {

common/Cargo.toml

@@ -13,6 +13,7 @@ chrono = { version = "0.4", features = ["serde"] }
 headers = "*"
 lazy_static = "1"
 rand = "0.8"
+secrecy = "0.8"
 serde = { version = "1", features = ["derive"] }
 thiserror = "1"
 typenum = "1"

common/src/crypto.rs

@@ -8,10 +8,9 @@ use chacha20poly1305::aead::{AeadInPlace, NewAead};
 use chacha20poly1305::XChaCha20Poly1305;
 use chacha20poly1305::XNonce;
 use rand::{thread_rng, Rng};
+use secrecy::{ExposeSecret, Secret, SecretVec, Zeroize};
 use typenum::Unsigned;
 
-pub use chacha20poly1305::Key;
-
 #[derive(Debug)]
 pub enum Error {
     ChaCha20Poly1305(chacha20poly1305::aead::Error),
@@ -41,6 +40,42 @@ impl Display for Error {
     }
 }
 
+// This struct intentionally prevents implement Clone or Copy
+#[derive(Default)]
+pub struct Key(chacha20poly1305::Key);
+
+impl Key {
+    pub fn new_secret(vec: Vec<u8>) -> Option<Secret<Self>> {
+        chacha20poly1305::Key::from_exact_iter(vec.into_iter())
+            .map(Self)
+            .map(Secret::new)
+    }
+}
+
+impl AsRef<chacha20poly1305::Key> for Key {
+    fn as_ref(&self) -> &chacha20poly1305::Key {
+        &self.0
+    }
+}
+
+impl Deref for Key {
+    type Target = chacha20poly1305::Key;
+    fn deref(&self) -> &Self::Target {
+        &self.0
+    }
+}
+impl DerefMut for Key {
+    fn deref_mut(&mut self) -> &mut Self::Target {
+        &mut self.0
+    }
+}
+
+impl Zeroize for Key {
+    fn zeroize(&mut self) {
+        self.0.zeroize()
+    }
+}
+
 /// Seals the provided message with an optional message. The resulting sealed
 /// message has the nonce used to encrypt the message appended to it as well as
 /// a salt string used to derive the key. In other words, the modified buffer is
@@ -60,16 +95,19 @@ impl Display for Error {
 ///     XChaCha20Poly1305.
 ///  - `rng_key` represents a randomly generated key.
 ///  - `kdf(pw, salt)` represents a key derived from Argon2.
-pub fn seal_in_place(message: &mut Vec<u8>, pw: Option<&str>) -> Result<Key, Error> {
+pub fn seal_in_place(
+    message: &mut Vec<u8>,
+    pw: Option<SecretVec<u8>>,
+) -> Result<Secret<Key>, Error> {
     let (key, nonce) = gen_key_nonce();
-    let cipher = XChaCha20Poly1305::new(&key);
+    let cipher = XChaCha20Poly1305::new(key.expose_secret());
     cipher.encrypt_in_place(&nonce, &[], message)?;
 
     let mut maybe_salt_string = None;
     if let Some(password) = pw {
         let (key, salt_string) = kdf(&password)?;
         maybe_salt_string = Some(salt_string);
-        let cipher = XChaCha20Poly1305::new(&key);
+        let cipher = XChaCha20Poly1305::new(key.expose_secret());
         cipher.encrypt_in_place(&nonce.increment(), &[], message)?;
     }
 
@@ -80,14 +118,18 @@ pub fn seal_in_place(message: &mut Vec<u8>, pw: Option<&str>) -> Result<Key, Err
     Ok(key)
 }
 
-pub fn open_in_place(data: &mut Vec<u8>, key: &Key, password: Option<&str>) -> Result<(), Error> {
+pub fn open_in_place(
+    data: &mut Vec<u8>,
+    key: &Secret<Key>,
+    password: Option<SecretVec<u8>>,
+) -> Result<(), Error> {
     let buffer_len = data.len();
     let pw_key = if let Some(password) = password {
         let salt_buf = data.split_off(buffer_len - Salt::SIZE);
         let argon = Argon2::default();
         let mut pw_key = Key::default();
-        argon.hash_password_into(password.as_bytes(), &salt_buf, &mut pw_key)?;
+        argon.hash_password_into(password.expose_secret(), &salt_buf, &mut pw_key)?;
-        Some(pw_key)
+        Some(Secret::new(pw_key))
     } else {
         None
     };
@@ -97,11 +139,11 @@ pub fn open_in_place(data: &mut Vec<u8>, key: &Key, password: Option<&str>) -> R
     // At this point we should have a buffer that's only the ciphertext.
 
     if let Some(key) = pw_key {
-        let cipher = XChaCha20Poly1305::new(&key);
+        let cipher = XChaCha20Poly1305::new(key.expose_secret());
         cipher.decrypt_in_place(&nonce.increment(), &[], data)?;
     }
 
-    let cipher = XChaCha20Poly1305::new(&key);
+    let cipher = XChaCha20Poly1305::new(key.expose_secret());
     cipher.decrypt_in_place(&nonce, &[], data)?;
 
     Ok(())
@@ -109,13 +151,13 @@ pub fn open_in_place(data: &mut Vec<u8>, key: &Key, password: Option<&str>) -> R
 
 /// Securely generates a random key and nonce.
 #[must_use]
-fn gen_key_nonce() -> (Key, Nonce) {
+fn gen_key_nonce() -> (Secret<Key>, Nonce) {
     let mut rng = thread_rng();
     let mut key = GenericArray::default();
     rng.fill(key.as_mut_slice());
     let mut nonce = Nonce::default();
     rng.fill(nonce.as_mut_slice());
-    (key, nonce)
+    (Secret::new(Key(key)), nonce)
 }
 
 // Type alias; to ensure that we're consistent on what the inner impl is.
@@ -186,11 +228,11 @@ impl AsRef<[u8]> for Salt {
 }
 
 /// Hashes an input to output a usable key.
-fn kdf(password: &str) -> Result<(Key, Salt), argon2::Error> {
+fn kdf(password: &SecretVec<u8>) -> Result<(Secret<Key>, Salt), argon2::Error> {
     let salt = Salt::random();
     let hasher = Argon2::default();
     let mut key = Key::default();
-    hasher.hash_password_into(password.as_ref(), salt.as_ref(), &mut key)?;
+    hasher.hash_password_into(password.expose_secret().as_ref(), salt.as_ref(), &mut key)?;
 
-    Ok((*Key::from_slice(&key), salt))
+    Ok((Secret::new(key), salt))
 }

common/src/lib.rs

@@ -9,6 +9,8 @@ use bytes::Bytes;
 use chrono::{DateTime, Duration, Utc};
 use headers::{Header, HeaderName, HeaderValue};
 use lazy_static::lazy_static;
+pub use secrecy;
+use secrecy::Secret;
 use serde::{Deserialize, Serialize};
 use thiserror::Error;
 pub use url::Url;
@@ -22,18 +24,33 @@ pub const API_ENDPOINT: &str = "/api";
 
 pub struct ParsedUrl {
     pub sanitized_url: Url,
-    pub decryption_key: Key,
+    pub decryption_key: Secret<Key>,
     pub needs_password: bool,
 }
 
 #[derive(Default)]
 pub struct PartialParsedUrl {
-    pub decryption_key: Option<Key>,
+    pub decryption_key: Option<Secret<Key>>,
     pub needs_password: bool,
 }
 
 impl From<&str> for PartialParsedUrl {
     fn from(fragment: &str) -> Self {
+        // Short circuit if the fragment only contains the key.
+
+        // Base64 has an interesting property that the length of an encoded text
+        // is always 4/3rds larger than the original data.
+        if !fragment.contains("key") {
+            let decryption_key = base64::decode(fragment)
+                .ok()
+                .and_then(|k| Key::new_secret(k));
+
+            return Self {
+                decryption_key,
+                needs_password: false,
+            };
+        }
+
         let args = fragment.split('!').filter_map(|kv| {
             let (k, v) = {
                 let mut iter = kv.split(':');
@@ -49,7 +66,7 @@ impl From<&str> for PartialParsedUrl {
         for (key, value) in args {
             match (key, value) {
                 ("key", Some(value)) => {
-                    decryption_key = base64::decode(value).map(|k| *Key::from_slice(&k)).ok();
+                    decryption_key = base64::decode(value).ok().and_then(|k| Key::new_secret(k));
                 }
                 ("pw", _) => {
                     needs_password = true;
@@ -71,10 +88,6 @@ pub enum ParseUrlError {
     BadUrl,
     #[error("Missing decryption key")]
     NeedKey,
-    #[error("Missing nonce")]
-    NeedNonce,
-    #[error("Missing decryption key and nonce")]
-    NeedKeyAndNonce,
 }
 
 impl FromStr for ParsedUrl {
@@ -82,22 +95,19 @@ impl FromStr for ParsedUrl {
 
     fn from_str(s: &str) -> Result<Self, Self::Err> {
         let mut url = Url::from_str(s).map_err(|_| ParseUrlError::BadUrl)?;
-        let fragment = url.fragment().ok_or(ParseUrlError::NeedKeyAndNonce)?;
+        let fragment = url.fragment().ok_or(ParseUrlError::NeedKey)?;
         if fragment.is_empty() {
-            return Err(ParseUrlError::NeedKeyAndNonce);
+            return Err(ParseUrlError::NeedKey);
         }
 
         let PartialParsedUrl {
-            decryption_key,
+            mut decryption_key,
             needs_password,
         } = PartialParsedUrl::from(fragment);
 
         url.set_fragment(None);
 
-        let decryption_key = match &decryption_key {
+        let decryption_key = decryption_key.take().ok_or(ParseUrlError::NeedKey)?;
-            Some(k) => Ok(*k),
-            None => Err(ParseUrlError::NeedKey),
-        }?;
 
         Ok(Self {
             sanitized_url: url,

web/src/decrypt.rs

@@ -5,6 +5,7 @@ use std::sync::Arc;
 use gloo_console::log;
 use js_sys::{Array, Uint8Array};
 use omegaupload_common::crypto::{open_in_place, Key};
+use omegaupload_common::secrecy::{Secret, SecretVec};
 use serde::Serialize;
 use wasm_bindgen::JsCast;
 use web_sys::{Blob, BlobPropertyBag};
@@ -35,8 +36,8 @@ fn now() -> f64 {
 
 pub fn decrypt(
     mut container: Vec<u8>,
-    key: Key,
+    key: Secret<Key>,
-    maybe_password: Option<&str>,
+    maybe_password: Option<SecretVec<u8>>,
 ) -> Result<DecryptedData, PasteCompleteConstructionError> {
     open_in_place(&mut container, &key, maybe_password)
         .map_err(|_| PasteCompleteConstructionError::Decryption)?;

web/src/main.rs

@@ -10,6 +10,7 @@ use http::uri::PathAndQuery;
 use http::{StatusCode, Uri};
 use js_sys::{Array, JsString, Object, Uint8Array};
 use omegaupload_common::crypto::Key;
+use omegaupload_common::secrecy::{Secret, SecretVec};
 use omegaupload_common::{Expiration, PartialParsedUrl};
 use reqwasm::http::Request;
 use wasm_bindgen::prelude::{wasm_bindgen, Closure};
@@ -90,7 +91,7 @@ fn main() {
 
             if let Ok(Some(password)) = pw {
                 if !password.is_empty() {
-                    break Some(password);
+                    break Some(SecretVec::new(password.into_bytes()));
                 }
             }
         }
@@ -101,7 +102,7 @@ fn main() {
     if location().pathname().unwrap() == "/" {
     } else {
         spawn_local(async move {
-            if let Err(e) = fetch_resources(request_uri, key, password.as_deref()).await {
+            if let Err(e) = fetch_resources(request_uri, key, password).await {
                 log!(e.to_string());
             }
         });
@@ -109,7 +110,11 @@ fn main() {
 }
 
 #[allow(clippy::future_not_send)]
-async fn fetch_resources(request_uri: Uri, key: Key, password: Option<&str>) -> Result<()> {
+async fn fetch_resources(
+    request_uri: Uri,
+    key: Secret<Key>,
+    password: Option<SecretVec<u8>>,
+) -> Result<()> {
     match Request::get(&request_uri.to_string()).send().await {
         Ok(resp) if resp.status() == StatusCode::OK => {
             let expires = Expiration::try_from(resp.headers()).map_or_else(